drm: Only try to set formats supported by the hardware

When userland uses DRM_IOCTL_MODE_ADDFB to add a framebuffer, the DRM subsystem tries to find a pixel format from the supplied depth and bpp-values. It does this by calling drm_driver_legacy_fb_format(). Unfortunately drm_driver_legacy_fb_format() can return formats not supported by the underlying hardware. This series of patches remedies this problem in patch 1.

In order to use the same logic for determining the pixel format, when a fbdev adds a framebuffer as userland does, patches 2 to 11 migrates fbdev users of drm_mode_legacy_fb_format() to drm_driver_legacy_fb_format().

This series has been tested with the nouveau and modesetting drivers on a NVIDIA NV96, the modesetting driver on Beagleboard Black, and with the Intel and modesetting drivers on an Intel HD Graphics 4000 chipset.

This is an evolved version of the changes proposed in "drm: Don't return unsupported formats in drm_mode_legacy_fb_format" [1] following the suggestions of Thomas Zimmermann.

[1] https://lore.kernel.org/all/[email protected]/

Frej Drejhammar (11):
drm: Only return supported formats from drm_driver_legacy_fb_format drm/fbdev_generic: Use drm_driver_legacy_fb_format() for fbdev drm/armada: Use drm_driver_legacy_fb_format() for fbdev drm/exynos: Use drm_driver_legacy_fb_format() for fbdev drm/gma500: Use drm_driver_legacy_fb_format() for fbdev drm/i915: Use drm_driver_legacy_fb_format() for fbdev drm/msm: Use drm_driver_legacy_fb_format() for fbdev drm/omapdrm: Use drm_driver_legacy_fb_format() for fbdev drm/radeon: Use drm_driver_legacy_fb_format() for fbdev drm/tegra: Use drm_driver_legacy_fb_format() for fbdev drm/xe: Use drm_driver_legacy_fb_format() for fbdev

 drivers/gpu/drm/armada/armada_fbdev.c         |  5 +-
 drivers/gpu/drm/drm_fb_helper.c               |  2 +-
 drivers/gpu/drm/drm_fbdev_dma.c               |  4 +-
 drivers/gpu/drm/drm_fbdev_generic.c           |  4 +-
 drivers/gpu/drm/drm_fourcc.c                  | 83 +++++++++++++++++++
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c     |  6 +-
 drivers/gpu/drm/gma500/fbdev.c                |  2 +-
 drivers/gpu/drm/i915/display/intel_fbdev_fb.c |  6 +-
 drivers/gpu/drm/msm/msm_fbdev.c               |  4 +-
 drivers/gpu/drm/omapdrm/omap_fbdev.c          |  6 +-
 drivers/gpu/drm/radeon/radeon_fbdev.c         |  6 +-
 drivers/gpu/drm/tegra/fbdev.c                 |  5 +-
 drivers/gpu/drm/xe/display/intel_fbdev_fb.c   |  5 +-
 13 files changed, 119 insertions(+), 19 deletions(-)

base-commit: 119b225f01e4d3ce974cd3b4d982c76a380c796d

2.44.0

This patch extends drm_driver_legacy_fb_format() to only return formats which are supported by the current drm-device. The motivation for this change is to fix a regression introduced by commit c91acda3a380 ("drm/gem: Check for valid formats") which stops the Xorg modesetting driver from working on the Beagleboard Black (it uses the tilcdc kernel driver).

When the Xorg modesetting driver starts up, it tries to determine the default bpp for the device. It does this by allocating a dumb 32bpp frame buffer (using DRM_IOCTL_MODE_CREATE_DUMB) and then calling drmModeAddFB() with that frame buffer asking for a 24-bit depth and 32 bpp. As the modesetting driver uses drmModeAddFB() which doesn't supply a format, the kernel's drm_driver_legacy_fb_format() is called, in drm_mode_addfb(), to provide a format matching the requested depth and bpp. If the drmModeAddFB() call fails, the modesetting driver forces both depth and bpp to 24. If drmModeAddFB() succeeds, depth is assumed to be 24 and bpp 32. The dummy frame buffer is then removed (using drmModeRmFB()).

If the modesetting driver finds that both the default bpp and depth are 24, it forces the use of a 32bpp shadow buffer and a 24bpp front buffer. Following this, the driver reads the user-specified color depth option and tries to create a framebuffer of that depth, but if the use of a shadow buffer has been forced, the bpp and depth of it overrides the user-supplied option.

The Xorg modesetting driver on top of the tilcdc kernel driver used to work on the Beagleboard Black if a 16 bit color depth was configured. The hardware in the Beagleboard Black supports the RG16, BG24, and XB24 formats. When drm_mode_legacy_fb_format() was called to request a format for a 24-bit depth and 32 bpp, it would return the unsupported RG24 format which drmModeAddFB() would happily accept (as there was no check for a valid format). As a shadow buffer wasn't forced, the modesetting driver would try the user specified 16 bit color depth and drm_mode_legacy_fb_format() would return RG16 which is supported by the hardware. Color depths of 24 bits were not supported, as the unsupported RG24 would be detected when drmModeSetCrtc() was called.

Following commit c91acda3a380 ("drm/gem: Check for valid formats"), which adds a check for a valid (supported by the hardware) format to the code path for the kernel part of drmModeAddFB(), the modesetting driver fails to configure and add a frame buffer. This is because the call to create a 24-bit depth and 32 bpp framebuffer during detection of the default bpp will now fail and a 24-bit depth and 24 bpp front buffer will be forced. As drm_mode_legacy_fb_format() will return RG24 which isn't supported, the creation of that framebuffer will also fail.

To fix the regression, this patch extends drm_driver_legacy_fb_format() to list all formats with a particular bpp and color depth known to the kernel, and have it probe the current drm-device for a supported format. This fixes the regression and, as a bonus, a color depth of 24 bits on the Beagleboard Black is now working.

This patch has, in addition to the Beagleboard Black, also been tested with the nouveau and modesetting drivers on a NVIDIA NV96, and with the Intel and modesetting drivers on an Intel HD Graphics 4000 chipset.

---

This is an evolved version of the changes proposed in "drm: Don't return unsupported formats in drm_mode_legacy_fb_format" [1] following the suggestions of Thomas Zimmermann.

[1] https://lore.kernel.org/all/[email protected]/

---
 drivers/gpu/drm/drm_fourcc.c | 83 ++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

index 193cf8ed7912..285388bed990 100644
@@ -29,6 +29,7 @@
 
 #include <drm/drm_device.h>
 #include <drm/drm_fourcc.h>
+#include <drm/drm_plane.h>
 
 /**
  * drm_mode_legacy_fb_format - compute drm fourcc code from legacy description

@@ -105,6 +106,87 @@ uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
 }
 EXPORT_SYMBOL(drm_mode_legacy_fb_format);
 
+/*
+ * Internal helper to find a supported format among a list of
+ * potentially supported formats.
+ *
+ * Traverses the variadic arguments until a format supported by dev or
+ * an DRM_FORMAT_INVALID argument is found. If a supported format is
+ * found it is returned, otherwise DRM_FORMAT_INVALID is returned.
+ */
+static uint32_t drm_pick_supported_format(struct drm_device *dev, ...)
+{
+	va_list va;
+	uint32_t fmt = DRM_FORMAT_INVALID;
+	uint32_t to_try;
+
+	va_start(va, dev);
+
+	for (to_try = va_arg(va, uint32_t);
+	     to_try != DRM_FORMAT_INVALID;
+	     to_try = va_arg(va, uint32_t)) {
+		if (drm_any_plane_has_format(dev, to_try, 0)) {
+			fmt = to_try;
+			break;
+		}
+	}
+
+	va_end(va);
+
+	return fmt;
+}
+
+/*
+ * Internal helper to find a format which has the same depth and bpp
+ * as the input format and is supported by the drm device.
+ */
+static uint32_t drm_supported_format(struct drm_device *dev, uint32_t fmt)
+{
+	switch (fmt) {
+	case DRM_FORMAT_XRGB1555:
+		return drm_pick_supported_format(dev,
+						 fmt,
+						 DRM_FORMAT_RGBX5551,
+						 DRM_FORMAT_BGRX5551,
+						 DRM_FORMAT_INVALID);
+	case DRM_FORMAT_RGB565:
+		return drm_pick_supported_format(dev,
+						 fmt,
+						 DRM_FORMAT_BGR565,
+						 DRM_FORMAT_INVALID);
+	case DRM_FORMAT_RGB888:
+		return drm_pick_supported_format(dev,
+						 fmt,
+						 DRM_FORMAT_BGR888,
+						 DRM_FORMAT_INVALID);
+	case DRM_FORMAT_XRGB8888:
+		return drm_pick_supported_format(dev,
+						 fmt,
+						 DRM_FORMAT_XBGR8888,
+						 DRM_FORMAT_RGBX8888,
+						 DRM_FORMAT_BGRX8888,
+						 DRM_FORMAT_INVALID);
+	case DRM_FORMAT_XRGB2101010:
+		return drm_pick_supported_format(dev,
+						 fmt,
+						 DRM_FORMAT_XBGR2101010,
+						 DRM_FORMAT_RGBX1010102,
+						 DRM_FORMAT_BGRX1010102,
+						 DRM_FORMAT_INVALID);
+	case DRM_FORMAT_ARGB8888:
+		return drm_pick_supported_format(dev,
+						 fmt,
+						 DRM_FORMAT_ABGR8888,
+						 DRM_FORMAT_RGBA8888,
+						 DRM_FORMAT_BGRA8888,
+						 DRM_FORMAT_INVALID);
+	default:
+		return drm_pick_supported_format(dev,
+						 fmt,
+						 DRM_FORMAT_INVALID);
+	}
+}
+
 /**
  * drm_driver_legacy_fb_format - compute drm fourcc code from legacy description
  * @dev: DRM device

@@ -121,6 +203,7 @@ uint32_t drm_driver_legacy_fb_format(struct drm_device *dev,
 {
 	uint32_t fmt = drm_mode_legacy_fb_format(bpp, depth);
 
+	fmt = drm_supported_format(dev, fmt);
 	if (dev->mode_config.quirk_addfb_prefer_host_byte_order) {
 		if (fmt == DRM_FORMAT_XRGB8888)
 			fmt = DRM_FORMAT_HOST_XRGB8888;

2.44.0

Switch to using drm_driver_legacy_fb_format() instead of drm_mode_legacy_fb_format() to use the same logic as for the DRM_IOCTL_MODE_ADDFB ioctl when selecting a framebuffer format.

---

This is an evolved version of the changes proposed in "drm: Don't return unsupported formats in drm_mode_legacy_fb_format" [1] following the suggestions of Thomas Zimmermann.

[1] https://lore.kernel.org/all/[email protected]/

---
 drivers/gpu/drm/drm_fb_helper.c     | 2 +-
 drivers/gpu/drm/drm_fbdev_dma.c     | 4 +++-
 drivers/gpu/drm/drm_fbdev_generic.c | 4 +++-
 3 files changed, 7 insertions(+), 3 deletions(-)

index d612133e2cf7..61c22e6c72af 100644
@@ -1453,7 +1453,7 @@ static uint32_t drm_fb_helper_find_format(struct drm_fb_helper *fb_helper, const
 	 * the framebuffer emulation can only deal with such
 	 * formats, specifically RGB/BGA formats.
 	 */
-	format = drm_mode_legacy_fb_format(bpp, depth);
+	format = drm_driver_legacy_fb_format(dev, bpp, depth);
 	if (!format)
 		goto err;
 

index 6c9427bb4053..f6c89b62b177 100644
@@ -90,7 +90,9 @@ static int drm_fbdev_dma_helper_fb_probe(struct drm_fb_helper *fb_helper,
 		    sizes->surface_width, sizes->surface_height,
 		    sizes->surface_bpp);
 
-	format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
+	format = drm_driver_legacy_fb_format(dev,
+					     sizes->surface_bpp,
+					     sizes->surface_depth);
 	buffer = drm_client_framebuffer_create(client, sizes->surface_width,
 					       sizes->surface_height, format);
 	if (IS_ERR(buffer))

index d647d89764cb..0a567f37d127 100644
@@ -84,7 +84,9 @@ static int drm_fbdev_generic_helper_fb_probe(struct drm_fb_helper *fb_helper,
 		    sizes->surface_width, sizes->surface_height,
 		    sizes->surface_bpp);
 
-	format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
+	format = drm_driver_legacy_fb_format(dev,
+					     sizes->surface_bpp,
+					     sizes->surface_depth);
 	buffer = drm_client_framebuffer_create(client, sizes->surface_width,
 					       sizes->surface_height, format);
 	if (IS_ERR(buffer))

2.44.0

Switch to using drm_driver_legacy_fb_format() instead of drm_mode_legacy_fb_format() to use the same logic as for the DRM_IOCTL_MODE_ADDFB ioctl when selecting a framebuffer format.

---

This is an evolved version of the changes proposed in "drm: Don't return unsupported formats in drm_mode_legacy_fb_format" [1] following the suggestions of Thomas Zimmermann.

[1] https://lore.kernel.org/all/[email protected]/

---
 drivers/gpu/drm/armada/armada_fbdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index d223176912b6..3a7258de323f 100644
@@ -54,8 +54,9 @@ static int armada_fbdev_create(struct drm_fb_helper *fbh,
 	mode.width = sizes->surface_width;
 	mode.height = sizes->surface_height;
 	mode.pitches[0] = armada_pitch(mode.width, sizes->surface_bpp);
-	mode.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
-					sizes->surface_depth);
+	mode.pixel_format = drm_driver_legacy_fb_format(dev,
+							sizes->surface_bpp,
+							sizes->surface_depth);
 
 	size = mode.pitches[0] * mode.height;
 	obj = armada_gem_alloc_private_object(dev, size);

2.44.0

Switch to using drm_driver_legacy_fb_format() instead of drm_mode_legacy_fb_format() to use the same logic as for the DRM_IOCTL_MODE_ADDFB ioctl when selecting a framebuffer format.

---

This is an evolved version of the changes proposed in "drm: Don't return unsupported formats in drm_mode_legacy_fb_format" [1] following the suggestions of Thomas Zimmermann.

[1] https://lore.kernel.org/all/[email protected]/

---
 drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index a379c8ca435a..d47bb5e89ff2 100644
@@ -104,8 +104,10 @@ static int exynos_drm_fbdev_create(struct drm_fb_helper *helper,
 	mode_cmd.width = sizes->surface_width;
 	mode_cmd.height = sizes->surface_height;
 	mode_cmd.pitches[0] = sizes->surface_width * (sizes->surface_bpp >> 3);
-	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
-							  sizes->surface_depth);
+	mode_cmd.pixel_format =
+		drm_driver_legacy_fb_format(dev,
+					    sizes->surface_bpp,
+					    sizes->surface_depth);
 
 	size = mode_cmd.pitches[0] * mode_cmd.height;
 

2.44.0

Switch to using drm_driver_legacy_fb_format() instead of drm_mode_legacy_fb_format() to use the same logic as for the DRM_IOCTL_MODE_ADDFB ioctl when selecting a framebuffer format.

---

This is an evolved version of the changes proposed in "drm: Don't return unsupported formats in drm_mode_legacy_fb_format" [1] following the suggestions of Thomas Zimmermann.

[1] https://lore.kernel.org/all/[email protected]/

---
 drivers/gpu/drm/gma500/fbdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 98b44974d42d..4a5eaf407432 100644
@@ -189,7 +189,7 @@ static int psb_fbdev_fb_probe(struct drm_fb_helper *fb_helper,
 	mode_cmd.width = sizes->surface_width;
 	mode_cmd.height = sizes->surface_height;
 	mode_cmd.pitches[0] = ALIGN(mode_cmd.width * DIV_ROUND_UP(bpp, 8), 64);
-	mode_cmd.pixel_format = drm_mode_legacy_fb_format(bpp, depth);
+	mode_cmd.pixel_format = drm_driver_legacy_fb_format(dev, bpp, depth);
 
 	size = mode_cmd.pitches[0] * mode_cmd.height;
 	size = ALIGN(size, PAGE_SIZE);

2.44.0

Switch to using drm_driver_legacy_fb_format() instead of drm_mode_legacy_fb_format() to use the same logic as for the DRM_IOCTL_MODE_ADDFB ioctl when selecting a framebuffer format.

---

This is an evolved version of the changes proposed in "drm: Don't return unsupported formats in drm_mode_legacy_fb_format" [1] following the suggestions of Thomas Zimmermann.

[1] https://lore.kernel.org/all/[email protected]/

---
 drivers/gpu/drm/i915/display/intel_fbdev_fb.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index 0665f943f65f..1579c433c2c6 100644
@@ -30,8 +30,10 @@ struct drm_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
 
 	mode_cmd.pitches[0] = ALIGN(mode_cmd.width *
 				    DIV_ROUND_UP(sizes->surface_bpp, 8), 64);
-	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
-							  sizes->surface_depth);
+	mode_cmd.pixel_format =
+		drm_driver_legacy_fb_format(dev,
+					    sizes->surface_bpp,
+					    sizes->surface_depth);
 
 	size = mode_cmd.pitches[0] * mode_cmd.height;
 	size = PAGE_ALIGN(size);

2.44.0

Switch to using drm_driver_legacy_fb_format() instead of drm_mode_legacy_fb_format() to use the same logic as for the DRM_IOCTL_MODE_ADDFB ioctl when selecting a framebuffer format.

---

This is an evolved version of the changes proposed in "drm: Don't return unsupported formats in drm_mode_legacy_fb_format" [1] following the suggestions of Thomas Zimmermann.

[1] https://lore.kernel.org/all/[email protected]/

---
 drivers/gpu/drm/msm/msm_fbdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index 030bedac632d..05bedf210572 100644
@@ -77,7 +77,9 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
 	uint32_t format;
 	int ret, pitch;
 
-	format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
+	format = drm_driver_legacy_fb_format(dev,
+					     sizes->surface_bpp,
+					     sizes->surface_depth);
 
 	DBG("create fbdev: %dx%d@%d (%dx%d)", sizes->surface_width,
 			sizes->surface_height, sizes->surface_bpp,

2.44.0

Switch to using drm_driver_legacy_fb_format() instead of drm_mode_legacy_fb_format() to use the same logic as for the DRM_IOCTL_MODE_ADDFB ioctl when selecting a framebuffer format.

---

This is an evolved version of the changes proposed in "drm: Don't return unsupported formats in drm_mode_legacy_fb_format" [1] following the suggestions of Thomas Zimmermann.

[1] https://lore.kernel.org/all/[email protected]/

---
 drivers/gpu/drm/omapdrm/omap_fbdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index 6b08b137af1a..08ecced62459 100644
@@ -139,8 +139,10 @@ static int omap_fbdev_create(struct drm_fb_helper *helper,
 			sizes->surface_height, sizes->surface_bpp,
 			sizes->fb_width, sizes->fb_height);
 
-	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
-			sizes->surface_depth);
+	mode_cmd.pixel_format =
+		drm_driver_legacy_fb_format(dev,
+					    sizes->surface_bpp,
+					    sizes->surface_depth);
 
 	mode_cmd.width = sizes->surface_width;
 	mode_cmd.height = sizes->surface_height;

2.44.0

Switch to using drm_driver_legacy_fb_format() instead of drm_mode_legacy_fb_format() to use the same logic as for the DRM_IOCTL_MODE_ADDFB ioctl when selecting a framebuffer format.

---

This is an evolved version of the changes proposed in "drm: Don't return unsupported formats in drm_mode_legacy_fb_format" [1] following the suggestions of Thomas Zimmermann.

[1] https://lore.kernel.org/all/[email protected]/

---
 drivers/gpu/drm/radeon/radeon_fbdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

index 02bf25759059..7f4617183982 100644
@@ -221,8 +221,10 @@ static int radeon_fbdev_fb_helper_fb_probe(struct drm_fb_helper *fb_helper,
 	if ((sizes->surface_bpp == 24) && ASIC_IS_AVIVO(rdev))
 		sizes->surface_bpp = 32;
 
-	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
-							  sizes->surface_depth);
+	mode_cmd.pixel_format =
+		drm_driver_legacy_fb_format(fb_helper->dev,
+					    sizes->surface_bpp,
+					    sizes->surface_depth);
 
 	ret = radeon_fbdev_create_pinned_object(fb_helper, &mode_cmd, &gobj);
 	if (ret) {

2.44.0

Switch to using drm_driver_legacy_fb_format() instead of drm_mode_legacy_fb_format() to use the same logic as for the DRM_IOCTL_MODE_ADDFB ioctl when selecting a framebuffer format.

---

This is an evolved version of the changes proposed in "drm: Don't return unsupported formats in drm_mode_legacy_fb_format" [1] following the suggestions of Thomas Zimmermann.

[1] https://lore.kernel.org/all/[email protected]/

---
 drivers/gpu/drm/tegra/fbdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index db6eaac3d30e..700ab02235f5 100644
@@ -87,8 +87,9 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper,
 	cmd.pitches[0] = round_up(sizes->surface_width * bytes_per_pixel,
 				  tegra->pitch_align);
 
-	cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
-						     sizes->surface_depth);
+	cmd.pixel_format = drm_driver_legacy_fb_format(drm,
+						       sizes->surface_bpp,
+						       sizes->surface_depth);
 
 	size = cmd.pitches[0] * cmd.height;
 

2.44.0

Switch to using drm_driver_legacy_fb_format() instead of drm_mode_legacy_fb_format() to use the same logic as for the DRM_IOCTL_MODE_ADDFB ioctl when selecting a framebuffer format.

---

This is an evolved version of the changes proposed in "drm: Don't return unsupported formats in drm_mode_legacy_fb_format" [1] following the suggestions of Thomas Zimmermann.

[1] https://lore.kernel.org/all/[email protected]/

---
 drivers/gpu/drm/xe/display/intel_fbdev_fb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index 51ae3561fd0d..5bf0841a9f27 100644
@@ -32,8 +32,9 @@ struct drm_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
 
 	mode_cmd.pitches[0] = ALIGN(mode_cmd.width *
 				    DIV_ROUND_UP(sizes->surface_bpp, 8), XE_PAGE_SIZE);
-	mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
-							  sizes->surface_depth);
+	mode_cmd.pixel_format = drm_driver_legacy_fb_format(dev,
+							    sizes->surface_bpp,
+							    sizes->surface_depth);
 
 	size = mode_cmd.pitches[0] * mode_cmd.height;
 	size = PAGE_ALIGN(size);

2.44.0