[PATCH v3 0/6] Add missing format_mod_supported functions

[PATCH v3 0/6] Add missing format_mod_supported functions

[José Expósito]

Hi all,

This patchset supersedes [1]. Now the title is a bit misleading, but
I left it this way to (hopefully) facilitate the maintainers' work.

A little context: Originally, I sent a patch adding modifiers to the
VKMS driver and Simon Ser kindly reviewed it and pointed out that
"format_mod_supported" was missing [2].
I asked if the docs were incorrect or if it was a bug in
"create_in_format_blob".

In the first version of this series, Simon Ser and Dmitry Baryshkov
agreed [1] that the code should behave as documented and
"create_in_format_blob" should be changed.

The second version implemented the required changes and drops the
"format_mod_supported" in the drivers that can use the default
implementation. [3]

This third version fixes a compiler warning and adds the reviewed
by tags.

Thanks,
José Expósito

[1] https://lore.kernel.org/dri-devel/CAA8EJpqJ-tWmb5Ba6XSK59toCtLb3nRRmVH8da4Ud_rrRYytmw@mail.gmail.com/T/
[2] https://lore.kernel.org/dri-devel/20211216170532.GA16349@elementary/T/
[3] https://lore.kernel.org/dri-devel/20211222090552.25972-1-jose.exposito89@gmail.com/T/

José Expósito (6):
  drm/plane: Make format_mod_supported truly optional
  drm/plane: Fix typo in format_mod_supported documentation
  drm/simple-kms: Drop format_mod_supported function
  drm/i915/display: Drop format_mod_supported function
  drm: mxsfb: Drop format_mod_supported function
  drm/stm: ltdc: Drop format_mod_supported function

 drivers/gpu/drm/drm_plane.c                 |  9 ++-------
 drivers/gpu/drm/drm_simple_kms_helper.c     |  8 --------
 drivers/gpu/drm/i915/display/intel_cursor.c |  8 --------
 drivers/gpu/drm/mxsfb/mxsfb_kms.c           |  8 --------
 drivers/gpu/drm/stm/ltdc.c                  | 11 -----------
 include/drm/drm_plane.h                     |  2 +-
 6 files changed, 3 insertions(+), 43 deletions(-)

-- 
2.25.1

[PATCH v3 1/6] drm/plane: Make format_mod_supported truly optional

[José Expósito]

The documentation for "drm_plane_funcs.format_mod_supported" reads:

  This *optional* hook is used for the DRM to determine if the given
  format/modifier combination is valid for the plane. This allows the
  DRM to generate the correct format bitmask (which formats apply to
  which modifier), and to validate modifiers at atomic_check time.

  *If not present*, then any modifier in the plane's modifier
  list is allowed with any of the plane's formats.

However, where the function is not present, an invalid IN_FORMATS blob
property with modifiers but no formats is exposed to user-space.

This breaks the latest Weston [1]. For testing purposes, I extracted the
affected code to a standalone program [2].

Make "create_in_format_blob" behave as documented.

[1] https://gitlab.freedesktop.org/wayland/weston/-/blob/9.0/libweston/backend-drm/kms.c#L431
[2] https://github.com/JoseExposito/drm-sandbox/blob/main/in_formats.c

Signed-off-by: José Expósito <jose.exposito89@gmail.com>
Reviewed-by: Simon Ser <contact@emersion.fr>

---

v2:

 - Remove unused "done:" label to fix compile warning
   Reported-by: kernel test robot <lkp@intel.com>

 - Add Reviewed-by (thanks to Simon Ser)
---
 drivers/gpu/drm/drm_plane.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 82afb854141b..deeec60a3315 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -202,17 +202,13 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
 
 	memcpy(formats_ptr(blob_data), plane->format_types, formats_size);
 
-	/* If we can't determine support, just bail */
-	if (!plane->funcs->format_mod_supported)
-		goto done;
-
 	mod = modifiers_ptr(blob_data);
 	for (i = 0; i < plane->modifier_count; i++) {
 		for (j = 0; j < plane->format_count; j++) {
-			if (plane->funcs->format_mod_supported(plane,
+			if (!plane->funcs->format_mod_supported ||
+			    plane->funcs->format_mod_supported(plane,
 							       plane->format_types[j],
 							       plane->modifiers[i])) {
-
 				mod->formats |= 1ULL << j;
 			}
 		}
@@ -223,7 +219,6 @@ static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane
 		mod++;
 	}
 
-done:
 	drm_object_attach_property(&plane->base, config->modifiers_property,
 				   blob->base.id);
 
-- 
2.25.1

[PATCH v3 2/6] drm/plane: Fix typo in format_mod_supported documentation

[José Expósito]

Fix minor typo: "valdiate" -> "validate".

Signed-off-by: José Expósito <jose.exposito89@gmail.com>
Reviewed-by: Simon Ser <contact@emersion.fr>
---
 include/drm/drm_plane.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 0c1102dc4d88..06759badf99f 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -516,7 +516,7 @@ struct drm_plane_funcs {
 	 * This optional hook is used for the DRM to determine if the given
 	 * format/modifier combination is valid for the plane. This allows the
 	 * DRM to generate the correct format bitmask (which formats apply to
-	 * which modifier), and to valdiate modifiers at atomic_check time.
+	 * which modifier), and to validate modifiers at atomic_check time.
 	 *
 	 * If not present, then any modifier in the plane's modifier
 	 * list is allowed with any of the plane's formats.
-- 
2.25.1

[PATCH v3 3/6] drm/simple-kms: Drop format_mod_supported function

[José Expósito]

The "drm_plane_funcs.format_mod_supported" can be removed in favor of
the default implementation.

Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 drivers/gpu/drm/drm_simple_kms_helper.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index 72989ed1baba..2c6aa67c6956 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -284,13 +284,6 @@ static void drm_simple_kms_plane_cleanup_fb(struct drm_plane *plane,
 	pipe->funcs->cleanup_fb(pipe, state);
 }
 
-static bool drm_simple_kms_format_mod_supported(struct drm_plane *plane,
-						uint32_t format,
-						uint64_t modifier)
-{
-	return modifier == DRM_FORMAT_MOD_LINEAR;
-}
-
 static const struct drm_plane_helper_funcs drm_simple_kms_plane_helper_funcs = {
 	.prepare_fb = drm_simple_kms_plane_prepare_fb,
 	.cleanup_fb = drm_simple_kms_plane_cleanup_fb,
@@ -339,7 +332,6 @@ static const struct drm_plane_funcs drm_simple_kms_plane_funcs = {
 	.reset			= drm_simple_kms_plane_reset,
 	.atomic_duplicate_state	= drm_simple_kms_plane_duplicate_state,
 	.atomic_destroy_state	= drm_simple_kms_plane_destroy_state,
-	.format_mod_supported   = drm_simple_kms_format_mod_supported,
 };
 
 /**
-- 
2.25.1

[PATCH v3 4/6] drm/i915/display: Drop format_mod_supported function

[José Expósito]

The "drm_plane_funcs.format_mod_supported" can be removed in favor of
the default implementation.

Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 drivers/gpu/drm/i915/display/intel_cursor.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 11842f212613..6a5e022f5e21 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -602,13 +602,6 @@ static bool i9xx_cursor_get_hw_state(struct intel_plane *plane,
 	return ret;
 }
 
-static bool intel_cursor_format_mod_supported(struct drm_plane *_plane,
-					      u32 format, u64 modifier)
-{
-	return modifier == DRM_FORMAT_MOD_LINEAR &&
-		format == DRM_FORMAT_ARGB8888;
-}
-
 static int
 intel_legacy_cursor_update(struct drm_plane *_plane,
 			   struct drm_crtc *_crtc,
@@ -745,7 +738,6 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = {
 	.destroy = intel_plane_destroy,
 	.atomic_duplicate_state = intel_plane_duplicate_state,
 	.atomic_destroy_state = intel_plane_destroy_state,
-	.format_mod_supported = intel_cursor_format_mod_supported,
 };
 
 struct intel_plane *
-- 
2.25.1

[PATCH v3 5/6] drm: mxsfb: Drop format_mod_supported function

[José Expósito]

The "drm_plane_funcs.format_mod_supported" can be removed in favor of
the default implementation.

Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 drivers/gpu/drm/mxsfb/mxsfb_kms.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
index 0655582ae8ed..df32e1c3cc5d 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c
@@ -554,13 +554,6 @@ static void mxsfb_plane_overlay_atomic_update(struct drm_plane *plane,
 	writel(ctrl, mxsfb->base + LCDC_AS_CTRL);
 }
 
-static bool mxsfb_format_mod_supported(struct drm_plane *plane,
-				       uint32_t format,
-				       uint64_t modifier)
-{
-	return modifier == DRM_FORMAT_MOD_LINEAR;
-}
-
 static const struct drm_plane_helper_funcs mxsfb_plane_primary_helper_funcs = {
 	.atomic_check = mxsfb_plane_atomic_check,
 	.atomic_update = mxsfb_plane_primary_atomic_update,
@@ -572,7 +565,6 @@ static const struct drm_plane_helper_funcs mxsfb_plane_overlay_helper_funcs = {
 };
 
 static const struct drm_plane_funcs mxsfb_plane_funcs = {
-	.format_mod_supported	= mxsfb_format_mod_supported,
 	.update_plane		= drm_atomic_helper_update_plane,
 	.disable_plane		= drm_atomic_helper_disable_plane,
 	.destroy		= drm_plane_cleanup,
-- 
2.25.1

[PATCH v3 6/6] drm/stm: ltdc: Drop format_mod_supported function

[José Expósito]

The "drm_plane_funcs.format_mod_supported" can be removed in favor of
the default implementation.

Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 drivers/gpu/drm/stm/ltdc.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index dbdee954692a..ef909e50f0e4 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -925,16 +925,6 @@ static void ltdc_plane_atomic_print_state(struct drm_printer *p,
 	fpsi->counter = 0;
 }
 
-static bool ltdc_plane_format_mod_supported(struct drm_plane *plane,
-					    u32 format,
-					    u64 modifier)
-{
-	if (modifier == DRM_FORMAT_MOD_LINEAR)
-		return true;
-
-	return false;
-}
-
 static const struct drm_plane_funcs ltdc_plane_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
@@ -943,7 +933,6 @@ static const struct drm_plane_funcs ltdc_plane_funcs = {
 	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 	.atomic_print_state = ltdc_plane_atomic_print_state,
-	.format_mod_supported = ltdc_plane_format_mod_supported,
 };
 
 static const struct drm_plane_helper_funcs ltdc_plane_helper_funcs = {
-- 
2.25.1

Re: [PATCH v3 2/6] drm/plane: Fix typo in format_mod_supported documentation

[Simon Ser]

Pushed patches 1 & 2 to drm-misc-next. Thanks for your contribution!

Re: [PATCH v3 2/6] drm/plane: Fix typo in format_mod_supported documentation

[José Expósito]

Hi Simon,

On Wed, Jan 05, 2022 at 11:54:43PM +0000, Simon Ser wrote:
> Pushed patches 1 & 2 to drm-misc-next. Thanks for your contribution!

Thanks a lot for the review and for applying the changes, appreciate it.

Is there something that needs to improve in the other 4 patches?
Or just waiting on maintainers input?

Thanks,
José Expósito

Re: [PATCH v3 2/6] drm/plane: Fix typo in format_mod_supported documentation

[Simon Ser]

On Friday, January 7th, 2022 at 18:26, José Expósito <jose.exposito89@gmail.com> wrote:

> Is there something that needs to improve in the other 4 patches?
> Or just waiting on maintainers input?

Yeah, these patches look good to me. Feel free to add my R-b.

Maintainers for these drivers still need to review/ack/apply them.

Re: [PATCH v3 2/6] drm/plane: Fix typo in format_mod_supported documentation

[Philippe CORNU]

On 1/7/22 6:26 PM, José Expósito wrote:
> Hi Simon,
> 
> On Wed, Jan 05, 2022 at 11:54:43PM +0000, Simon Ser wrote:
>> Pushed patches 1 & 2 to drm-misc-next. Thanks for your contribution!
> 
> Thanks a lot for the review and for applying the changes, appreciate it.
> 
> Is there something that needs to improve in the other 4 patches?
> Or just waiting on maintainers input?
> 
> Thanks,
> José Expósito
> 

Hi José,
for the drm/stm part,
Applied on drm-misc-next.
Many thanks for your patches,
Philippe :-)