| Message ID | 20200718033352.1810-1-jajones@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/nouveau: Accept 'legacy' format modifiers | expand |
On Sat, 18 Jul 2020 at 13:34, James Jones <jajones@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@c46c70dac84a4b3030cd05b380f9f410536690fc, > gnome & KDE wayland desktops from Ubuntu 18.04, > and sway 1.5 > > Reported-by: Kirill A. Shutemov <kirill@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@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. > + > if (modifier == DRM_FORMAT_MOD_LINEAR) { > /* tile_mode will not be used in this case */ > *tile_mode = 0; > @@ -227,6 +233,16 @@ nouveau_framebuffer_get_layout(struct drm_framebuffer *fb, > } > } > > +static const u64 legacy_modifiers[] = { > + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(0), > + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(1), > + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(2), > + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(3), > + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(4), > + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(5), > + DRM_FORMAT_MOD_INVALID > +}; > + > static int > nouveau_validate_decode_mod(struct nouveau_drm *drm, > uint64_t modifier, > @@ -247,8 +263,14 @@ nouveau_validate_decode_mod(struct nouveau_drm *drm, > (disp->format_modifiers[mod] != modifier); > mod++); > > - if (disp->format_modifiers[mod] == DRM_FORMAT_MOD_INVALID) > - return -EINVAL; > + if (disp->format_modifiers[mod] == DRM_FORMAT_MOD_INVALID) { > + for (mod = 0; > + (legacy_modifiers[mod] != DRM_FORMAT_MOD_INVALID) && > + (legacy_modifiers[mod] != modifier); > + mod++); > + if (legacy_modifiers[mod] == DRM_FORMAT_MOD_INVALID) > + return -EINVAL; > + } > > nouveau_decode_mod(drm, modifier, tile_mode, kind); > > -- > 2.17.1 > > _______________________________________________ > Nouveau mailing list > Nouveau@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
On 7/23/20 9:06 PM, Ben Skeggs wrote: > On Sat, 18 Jul 2020 at 13:34, James Jones <jajones@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@c46c70dac84a4b3030cd05b380f9f410536690fc, >> gnome & KDE wayland desktops from Ubuntu 18.04, >> and sway 1.5 >> >> Reported-by: Kirill A. Shutemov <kirill@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@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... Thanks, -James >> + >> if (modifier == DRM_FORMAT_MOD_LINEAR) { >> /* tile_mode will not be used in this case */ >> *tile_mode = 0; >> @@ -227,6 +233,16 @@ nouveau_framebuffer_get_layout(struct drm_framebuffer *fb, >> } >> } >> >> +static const u64 legacy_modifiers[] = { >> + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(0), >> + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(1), >> + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(2), >> + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(3), >> + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(4), >> + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(5), >> + DRM_FORMAT_MOD_INVALID >> +}; >> + >> static int >> nouveau_validate_decode_mod(struct nouveau_drm *drm, >> uint64_t modifier, >> @@ -247,8 +263,14 @@ nouveau_validate_decode_mod(struct nouveau_drm *drm, >> (disp->format_modifiers[mod] != modifier); >> mod++); >> >> - if (disp->format_modifiers[mod] == DRM_FORMAT_MOD_INVALID) >> - return -EINVAL; >> + if (disp->format_modifiers[mod] == DRM_FORMAT_MOD_INVALID) { >> + for (mod = 0; >> + (legacy_modifiers[mod] != DRM_FORMAT_MOD_INVALID) && >> + (legacy_modifiers[mod] != modifier); >> + mod++); >> + if (legacy_modifiers[mod] == DRM_FORMAT_MOD_INVALID) >> + return -EINVAL; >> + } >> >> nouveau_decode_mod(drm, modifier, tile_mode, kind); >> >> -- >> 2.17.1 >> >> _______________________________________________ >> Nouveau mailing list >> Nouveau@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/nouveau
On Tue, 28 Jul 2020 at 04:51, James Jones <jajones@nvidia.com> wrote: > > On 7/23/20 9:06 PM, Ben Skeggs wrote: > > On Sat, 18 Jul 2020 at 13:34, James Jones <jajones@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@c46c70dac84a4b3030cd05b380f9f410536690fc, > >> gnome & KDE wayland desktops from Ubuntu 18.04, > >> and sway 1.5 > >> > >> Reported-by: Kirill A. Shutemov <kirill@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@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. Dave.
On Wed, 29 Jul 2020 at 12:48, Dave Airlie <airlied@gmail.com> wrote: > > On Tue, 28 Jul 2020 at 04:51, James Jones <jajones@nvidia.com> wrote: > > > > On 7/23/20 9:06 PM, Ben Skeggs wrote: > > > On Sat, 18 Jul 2020 at 13:34, James Jones <jajones@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@c46c70dac84a4b3030cd05b380f9f410536690fc, > > >> gnome & KDE wayland desktops from Ubuntu 18.04, > > >> and sway 1.5 > > >> > > >> Reported-by: Kirill A. Shutemov <kirill@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@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.
On Wed, Jul 29, 2020 at 01:40:13PM +1000, Ben Skeggs wrote: > On Wed, 29 Jul 2020 at 12:48, Dave Airlie <airlied@gmail.com> wrote: > > > > On Tue, 28 Jul 2020 at 04:51, James Jones <jajones@nvidia.com> wrote: > > > > > > On 7/23/20 9:06 PM, Ben Skeggs wrote: > > > > On Sat, 18 Jul 2020 at 13:34, James Jones <jajones@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@c46c70dac84a4b3030cd05b380f9f410536690fc, > > > >> gnome & KDE wayland desktops from Ubuntu 18.04, > > > >> and sway 1.5 > > > >> > > > >> Reported-by: Kirill A. Shutemov <kirill@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@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.
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@gmail.com> wrote: >>> >>> On Tue, 28 Jul 2020 at 04:51, James Jones <jajones@nvidia.com> wrote: >>>> >>>> On 7/23/20 9:06 PM, Ben Skeggs wrote: >>>>> On Sat, 18 Jul 2020 at 13:34, James Jones <jajones@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@c46c70dac84a4b3030cd05b380f9f410536690fc, >>>>>> gnome & KDE wayland desktops from Ubuntu 18.04, >>>>>> and sway 1.5 >>>>>> >>>>>> Reported-by: Kirill A. Shutemov <kirill@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@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
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); + } + if (modifier == DRM_FORMAT_MOD_LINEAR) { /* tile_mode will not be used in this case */ *tile_mode = 0; @@ -227,6 +233,16 @@ nouveau_framebuffer_get_layout(struct drm_framebuffer *fb, } } +static const u64 legacy_modifiers[] = { + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(0), + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(1), + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(2), + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(3), + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(4), + DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK(5), + DRM_FORMAT_MOD_INVALID +}; + static int nouveau_validate_decode_mod(struct nouveau_drm *drm, uint64_t modifier, @@ -247,8 +263,14 @@ nouveau_validate_decode_mod(struct nouveau_drm *drm, (disp->format_modifiers[mod] != modifier); mod++); - if (disp->format_modifiers[mod] == DRM_FORMAT_MOD_INVALID) - return -EINVAL; + if (disp->format_modifiers[mod] == DRM_FORMAT_MOD_INVALID) { + for (mod = 0; + (legacy_modifiers[mod] != DRM_FORMAT_MOD_INVALID) && + (legacy_modifiers[mod] != modifier); + mod++); + if (legacy_modifiers[mod] == DRM_FORMAT_MOD_INVALID) + return -EINVAL; + } nouveau_decode_mod(drm, modifier, tile_mode, kind);
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@c46c70dac84a4b3030cd05b380f9f410536690fc, gnome & KDE wayland desktops from Ubuntu 18.04, and sway 1.5 Reported-by: Kirill A. Shutemov <kirill@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@nvidia.com> --- drivers/gpu/drm/nouveau/nouveau_display.c | 26 +++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)