Ben Skeggs
2020-Jul-29 03:40 UTC
[Nouveau] [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
On Wed, 29 Jul 2020 at 12:48, Dave Airlie <airlied at gmail.com> wrote:> > On Tue, 28 Jul 2020 at 04:51, James Jones <jajones at nvidia.com> wrote: > > > > On 7/23/20 9:06 PM, Ben Skeggs wrote: > > > On Sat, 18 Jul 2020 at 13:34, James Jones <jajones at nvidia.com> wrote: > > >> > > >> Accept the DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK() > > >> family of modifiers to handle broken userspace > > >> Xorg modesetting and Mesa drivers. Existing Mesa > > >> drivers are still aware of only these older > > >> format modifiers which do not differentiate > > >> between different variations of the block linear > > >> layout. When the format modifier support flag was > > >> flipped in the nouveau kernel driver, the X.org > > >> modesetting driver began attempting to use its > > >> format modifier-enabled framebuffer path. Because > > >> the set of format modifiers advertised by the > > >> kernel prior to this change do not intersect with > > >> the set of format modifiers advertised by Mesa, > > >> allocating GBM buffers using format modifiers > > >> fails and the modesetting driver falls back to > > >> non-modifier allocation. However, it still later > > >> queries the modifier of the GBM buffer when > > >> creating its DRM-KMS framebuffer object, receives > > >> the old-format modifier from Mesa, and attempts > > >> to create a framebuffer with it. Since the kernel > > >> is still not aware of these formats, this fails. > > >> > > >> Userspace should not be attempting to query format > > >> modifiers of GBM buffers allocated with a non- > > >> format-modifier-aware allocation path, but to > > >> avoid breaking existing userspace behavior, this > > >> change accepts the old-style format modifiers when > > >> creating framebuffers and applying them to planes > > >> by translating them to the equivalent new-style > > >> modifier. To accomplish this, some layout > > >> parameters must be assumed to match properties of > > >> the device targeted by the relevant ioctls. To > > >> avoid perpetuating misuse of the old-style > > >> modifiers, this change does not advertise support > > >> for them. Doing so would imply compatibility > > >> between devices with incompatible memory layouts. > > >> > > >> Tested with Xorg 1.20 modesetting driver, > > >> weston at c46c70dac84a4b3030cd05b380f9f410536690fc, > > >> gnome & KDE wayland desktops from Ubuntu 18.04, > > >> and sway 1.5 > > >> > > >> Reported-by: Kirill A. Shutemov <kirill at shutemov.name> > > >> Fixes: fa4f4c213f5f ("drm/nouveau/kms: Support NVIDIA format modifiers") > > >> Link: https://lkml.org/lkml/2020/6/30/1251 > > >> Signed-off-by: James Jones <jajones at nvidia.com> > > >> --- > > >> drivers/gpu/drm/nouveau/nouveau_display.c | 26 +++++++++++++++++++++-- > > >> 1 file changed, 24 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c > > >> index 496c4621cc78..31543086254b 100644 > > >> --- a/drivers/gpu/drm/nouveau/nouveau_display.c > > >> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c > > >> @@ -191,8 +191,14 @@ nouveau_decode_mod(struct nouveau_drm *drm, > > >> uint32_t *tile_mode, > > >> uint8_t *kind) > > >> { > > >> + struct nouveau_display *disp = nouveau_display(drm->dev); > > >> BUG_ON(!tile_mode || !kind); > > >> > > >> + if ((modifier & (0xffull << 12)) == 0ull) { > > >> + /* Legacy modifier. Translate to this device's 'kind.' */ > > >> + modifier |= disp->format_modifiers[0] & (0xffull << 12); > > >> + } > > > I believe this should be moved into the != MOD_LINEAR case. > > > > Yes, of course, thanks. I need to re-evaluate my testing yet again to > > make sure I hit that case too. Preparing a v3... > > Going to need something here in the next day, two max. > > Linus may wait for another week, but it's not guaranteed.I tested a whole bunch of GPUs before sending nouveau's -next tree, and with the change I suggested to this patch + the other stuff I sent through -fixes already, things seemed to be in OK shape. Ben.> > Dave.
Kirill A. Shutemov
2020-Jul-29 14:47 UTC
[Nouveau] [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
On Wed, Jul 29, 2020 at 01:40:13PM +1000, Ben Skeggs wrote:> On Wed, 29 Jul 2020 at 12:48, Dave Airlie <airlied at gmail.com> wrote: > > > > On Tue, 28 Jul 2020 at 04:51, James Jones <jajones at nvidia.com> wrote: > > > > > > On 7/23/20 9:06 PM, Ben Skeggs wrote: > > > > On Sat, 18 Jul 2020 at 13:34, James Jones <jajones at nvidia.com> wrote: > > > >> > > > >> Accept the DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK() > > > >> family of modifiers to handle broken userspace > > > >> Xorg modesetting and Mesa drivers. Existing Mesa > > > >> drivers are still aware of only these older > > > >> format modifiers which do not differentiate > > > >> between different variations of the block linear > > > >> layout. When the format modifier support flag was > > > >> flipped in the nouveau kernel driver, the X.org > > > >> modesetting driver began attempting to use its > > > >> format modifier-enabled framebuffer path. Because > > > >> the set of format modifiers advertised by the > > > >> kernel prior to this change do not intersect with > > > >> the set of format modifiers advertised by Mesa, > > > >> allocating GBM buffers using format modifiers > > > >> fails and the modesetting driver falls back to > > > >> non-modifier allocation. However, it still later > > > >> queries the modifier of the GBM buffer when > > > >> creating its DRM-KMS framebuffer object, receives > > > >> the old-format modifier from Mesa, and attempts > > > >> to create a framebuffer with it. Since the kernel > > > >> is still not aware of these formats, this fails. > > > >> > > > >> Userspace should not be attempting to query format > > > >> modifiers of GBM buffers allocated with a non- > > > >> format-modifier-aware allocation path, but to > > > >> avoid breaking existing userspace behavior, this > > > >> change accepts the old-style format modifiers when > > > >> creating framebuffers and applying them to planes > > > >> by translating them to the equivalent new-style > > > >> modifier. To accomplish this, some layout > > > >> parameters must be assumed to match properties of > > > >> the device targeted by the relevant ioctls. To > > > >> avoid perpetuating misuse of the old-style > > > >> modifiers, this change does not advertise support > > > >> for them. Doing so would imply compatibility > > > >> between devices with incompatible memory layouts. > > > >> > > > >> Tested with Xorg 1.20 modesetting driver, > > > >> weston at c46c70dac84a4b3030cd05b380f9f410536690fc, > > > >> gnome & KDE wayland desktops from Ubuntu 18.04, > > > >> and sway 1.5 > > > >> > > > >> Reported-by: Kirill A. Shutemov <kirill at shutemov.name> > > > >> Fixes: fa4f4c213f5f ("drm/nouveau/kms: Support NVIDIA format modifiers") > > > >> Link: https://lkml.org/lkml/2020/6/30/1251 > > > >> Signed-off-by: James Jones <jajones at nvidia.com> > > > >> --- > > > >> drivers/gpu/drm/nouveau/nouveau_display.c | 26 +++++++++++++++++++++-- > > > >> 1 file changed, 24 insertions(+), 2 deletions(-) > > > >> > > > >> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c > > > >> index 496c4621cc78..31543086254b 100644 > > > >> --- a/drivers/gpu/drm/nouveau/nouveau_display.c > > > >> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c > > > >> @@ -191,8 +191,14 @@ nouveau_decode_mod(struct nouveau_drm *drm, > > > >> uint32_t *tile_mode, > > > >> uint8_t *kind) > > > >> { > > > >> + struct nouveau_display *disp = nouveau_display(drm->dev); > > > >> BUG_ON(!tile_mode || !kind); > > > >> > > > >> + if ((modifier & (0xffull << 12)) == 0ull) { > > > >> + /* Legacy modifier. Translate to this device's 'kind.' */ > > > >> + modifier |= disp->format_modifiers[0] & (0xffull << 12); > > > >> + } > > > > I believe this should be moved into the != MOD_LINEAR case. > > > > > > Yes, of course, thanks. I need to re-evaluate my testing yet again to > > > make sure I hit that case too. Preparing a v3... > > > > Going to need something here in the next day, two max. > > > > Linus may wait for another week, but it's not guaranteed. > I tested a whole bunch of GPUs before sending nouveau's -next tree, > and with the change I suggested to this patch + the other stuff I sent > through -fixes already, things seemed to be in OK shape.JFYI, the adjusted (moved into != MOD_LINEAR case) patch works fine for me on top of drm-fixes-2020-07-29. -- Kirill A. Shutemov
James Jones
2020-Jul-30 21:06 UTC
[Nouveau] [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
On 7/29/20 7:47 AM, Kirill A. Shutemov wrote:> On Wed, Jul 29, 2020 at 01:40:13PM +1000, Ben Skeggs wrote: >> On Wed, 29 Jul 2020 at 12:48, Dave Airlie <airlied at gmail.com> wrote: >>> >>> On Tue, 28 Jul 2020 at 04:51, James Jones <jajones at nvidia.com> wrote: >>>> >>>> On 7/23/20 9:06 PM, Ben Skeggs wrote: >>>>> On Sat, 18 Jul 2020 at 13:34, James Jones <jajones at nvidia.com> wrote: >>>>>> >>>>>> Accept the DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK() >>>>>> family of modifiers to handle broken userspace >>>>>> Xorg modesetting and Mesa drivers. Existing Mesa >>>>>> drivers are still aware of only these older >>>>>> format modifiers which do not differentiate >>>>>> between different variations of the block linear >>>>>> layout. When the format modifier support flag was >>>>>> flipped in the nouveau kernel driver, the X.org >>>>>> modesetting driver began attempting to use its >>>>>> format modifier-enabled framebuffer path. Because >>>>>> the set of format modifiers advertised by the >>>>>> kernel prior to this change do not intersect with >>>>>> the set of format modifiers advertised by Mesa, >>>>>> allocating GBM buffers using format modifiers >>>>>> fails and the modesetting driver falls back to >>>>>> non-modifier allocation. However, it still later >>>>>> queries the modifier of the GBM buffer when >>>>>> creating its DRM-KMS framebuffer object, receives >>>>>> the old-format modifier from Mesa, and attempts >>>>>> to create a framebuffer with it. Since the kernel >>>>>> is still not aware of these formats, this fails. >>>>>> >>>>>> Userspace should not be attempting to query format >>>>>> modifiers of GBM buffers allocated with a non- >>>>>> format-modifier-aware allocation path, but to >>>>>> avoid breaking existing userspace behavior, this >>>>>> change accepts the old-style format modifiers when >>>>>> creating framebuffers and applying them to planes >>>>>> by translating them to the equivalent new-style >>>>>> modifier. To accomplish this, some layout >>>>>> parameters must be assumed to match properties of >>>>>> the device targeted by the relevant ioctls. To >>>>>> avoid perpetuating misuse of the old-style >>>>>> modifiers, this change does not advertise support >>>>>> for them. Doing so would imply compatibility >>>>>> between devices with incompatible memory layouts. >>>>>> >>>>>> Tested with Xorg 1.20 modesetting driver, >>>>>> weston at c46c70dac84a4b3030cd05b380f9f410536690fc, >>>>>> gnome & KDE wayland desktops from Ubuntu 18.04, >>>>>> and sway 1.5 >>>>>> >>>>>> Reported-by: Kirill A. Shutemov <kirill at shutemov.name> >>>>>> Fixes: fa4f4c213f5f ("drm/nouveau/kms: Support NVIDIA format modifiers") >>>>>> Link: https://lkml.org/lkml/2020/6/30/1251 >>>>>> Signed-off-by: James Jones <jajones at nvidia.com> >>>>>> --- >>>>>> drivers/gpu/drm/nouveau/nouveau_display.c | 26 +++++++++++++++++++++-- >>>>>> 1 file changed, 24 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c >>>>>> index 496c4621cc78..31543086254b 100644 >>>>>> --- a/drivers/gpu/drm/nouveau/nouveau_display.c >>>>>> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c >>>>>> @@ -191,8 +191,14 @@ nouveau_decode_mod(struct nouveau_drm *drm, >>>>>> uint32_t *tile_mode, >>>>>> uint8_t *kind) >>>>>> { >>>>>> + struct nouveau_display *disp = nouveau_display(drm->dev); >>>>>> BUG_ON(!tile_mode || !kind); >>>>>> >>>>>> + if ((modifier & (0xffull << 12)) == 0ull) { >>>>>> + /* Legacy modifier. Translate to this device's 'kind.' */ >>>>>> + modifier |= disp->format_modifiers[0] & (0xffull << 12); >>>>>> + } >>>>> I believe this should be moved into the != MOD_LINEAR case. >>>> >>>> Yes, of course, thanks. I need to re-evaluate my testing yet again to >>>> make sure I hit that case too. Preparing a v3... >>> >>> Going to need something here in the next day, two max. >>> >>> Linus may wait for another week, but it's not guaranteed. >> I tested a whole bunch of GPUs before sending nouveau's -next tree, >> and with the change I suggested to this patch + the other stuff I sent >> through -fixes already, things seemed to be in OK shape. > > JFYI, the adjusted (moved into != MOD_LINEAR case) patch works fine for me > on top of drm-fixes-2020-07-29.Sorry again for the delays (life is terrible lately), but the signed-off version with Ben's suggestion went out this morning, and I specifically tested linear modifiers in addition to retesting all the other test cases mentioned in the patch. Thanks, -James
Apparently Analagous Threads
- [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
- [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
- [PATCH v3] drm/nouveau: Accept 'legacy' format modifiers
- [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
- [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers