* [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
@ 2020-07-18 3:33 James Jones
2020-07-24 4:06 ` [Nouveau] " Ben Skeggs
0 siblings, 1 reply; 7+ messages in thread
From: James Jones @ 2020-07-18 3:33 UTC (permalink / raw)
To: Dave Airlie, Daniel Vetter, Ben Skeggs, Kirill A . Shutemov
Cc: Nouveau, James Jones, dri-devel
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);
+ }
+
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [Nouveau] [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
2020-07-18 3:33 [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers James Jones
@ 2020-07-24 4:06 ` Ben Skeggs
2020-07-27 18:51 ` James Jones
0 siblings, 1 reply; 7+ messages in thread
From: Ben Skeggs @ 2020-07-24 4:06 UTC (permalink / raw)
To: James Jones
Cc: Nouveau, dri-devel, Ben Skeggs, Daniel Vetter, Kirill A . Shutemov
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Nouveau] [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
2020-07-24 4:06 ` [Nouveau] " Ben Skeggs
@ 2020-07-27 18:51 ` James Jones
2020-07-29 2:48 ` Dave Airlie
0 siblings, 1 reply; 7+ messages in thread
From: James Jones @ 2020-07-27 18:51 UTC (permalink / raw)
To: Ben Skeggs
Cc: Nouveau, dri-devel, Ben Skeggs, Daniel Vetter, Kirill A . Shutemov
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Nouveau] [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
2020-07-27 18:51 ` James Jones
@ 2020-07-29 2:48 ` Dave Airlie
2020-07-29 3:40 ` Ben Skeggs
0 siblings, 1 reply; 7+ messages in thread
From: Dave Airlie @ 2020-07-29 2:48 UTC (permalink / raw)
To: James Jones
Cc: Ben Skeggs, Nouveau, dri-devel, Ben Skeggs, Daniel Vetter,
Kirill A . Shutemov
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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Nouveau] [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
2020-07-29 2:48 ` Dave Airlie
@ 2020-07-29 3:40 ` Ben Skeggs
2020-07-29 14:47 ` Kirill A. Shutemov
0 siblings, 1 reply; 7+ messages in thread
From: Ben Skeggs @ 2020-07-29 3:40 UTC (permalink / raw)
To: Dave Airlie
Cc: Nouveau, James Jones, dri-devel, Ben Skeggs, Daniel Vetter,
Kirill A . Shutemov
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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Nouveau] [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
2020-07-29 3:40 ` Ben Skeggs
@ 2020-07-29 14:47 ` Kirill A. Shutemov
2020-07-30 21:06 ` James Jones
0 siblings, 1 reply; 7+ messages in thread
From: Kirill A. Shutemov @ 2020-07-29 14:47 UTC (permalink / raw)
To: Ben Skeggs; +Cc: Daniel Vetter, James Jones, dri-devel, Ben Skeggs, Nouveau
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.
--
Kirill A. Shutemov
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Nouveau] [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers
2020-07-29 14:47 ` Kirill A. Shutemov
@ 2020-07-30 21:06 ` James Jones
0 siblings, 0 replies; 7+ messages in thread
From: James Jones @ 2020-07-30 21:06 UTC (permalink / raw)
To: Kirill A. Shutemov, Ben Skeggs
Cc: Daniel Vetter, Ben Skeggs, dri-devel, Nouveau
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-30 21:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-18 3:33 [PATCH v2] drm/nouveau: Accept 'legacy' format modifiers James Jones
2020-07-24 4:06 ` [Nouveau] " Ben Skeggs
2020-07-27 18:51 ` James Jones
2020-07-29 2:48 ` Dave Airlie
2020-07-29 3:40 ` Ben Skeggs
2020-07-29 14:47 ` Kirill A. Shutemov
2020-07-30 21:06 ` James Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).