Re: [PATCH] drm/nouveau: Accept 'legacy' format modifiers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/17/20 12:47 PM, Daniel Vetter wrote:
On Fri, Jul 17, 2020 at 11:57:57AM -0700, James Jones wrote:
Accept the DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK()
family of modifiers to handle broken userspace
Xorg modesetting and Mesa drivers.

Tested with Xorg 1.20 modesetting driver,
weston@c46c70dac84a4b3030cd05b380f9f410536690fc,
gnome & KDE wayland desktops from Ubuntu 18.04,
and sway 1.5

Just bikeshed, but maybe a few more words on what exactly is broken and
how this works around it. Specifically why we only accept these, but don't
advertise them.

Added quite a few words.


Signed-off-by: James Jones <jajones@xxxxxxxxxx>

Needs Fixes: line here. Also nice to mention the bug reporter/link.

Done in v2.

---
  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);
+	}

Hm I tried to understand what this magic does by looking at drm_fourcc.h,
but the drm_fourcc_canonicalize_nvidia_format_mod() in there implements
something else. Is that function wrong, or should we use it here instead?

> Or is there something else going on entirely?

This may be slightly clearer with the expanded change description:

Canonicalize assumes the old modifiers are only used by certain Tegra revisions, because the Mesa patches were supposed to land and obliterate all uses beyond that. That assumption means it can assume the specific page kind (0xfe) used by the display-engine-compatible layout on those specific devices. There is no way to generally canonicalize a legacy modifier without referencing a specific device type, as is indirectly done here.

This code does a limited device-specific canonicalization: It substitutes the display-appropriate page kind used by this specific device, ensuring we derive this correct page kind later in the function. I iterated on the best way to accomplish this a few times, and this was the least-invasive thing I came up with, but it does require a pretty thorough understanding of the NVIDIA modifier macros.

Thanks for the quick review.

-James


Cheers, Daniel

+
  	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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux