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/[email protected]/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
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 <[email protected]>
Reviewed-by: Simon Ser <[email protected]>
---
v2:
- Remove unused "done:" label to fix compile warning
Reported-by: kernel test robot <[email protected]>
- 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
Fix minor typo: "valdiate" -> "validate".
Signed-off-by: José Expósito <[email protected]>
Reviewed-by: Simon Ser <[email protected]>
---
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
The "drm_plane_funcs.format_mod_supported" can be removed in favor of
the default implementation.
Signed-off-by: José Expósito <[email protected]>
---
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
The "drm_plane_funcs.format_mod_supported" can be removed in favor of
the default implementation.
Signed-off-by: José Expósito <[email protected]>
---
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
The "drm_plane_funcs.format_mod_supported" can be removed in favor of
the default implementation.
Signed-off-by: José Expósito <[email protected]>
---
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
The "drm_plane_funcs.format_mod_supported" can be removed in favor of
the default implementation.
Signed-off-by: José Expósito <[email protected]>
---
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
Pushed patches 1 & 2 to drm-misc-next. Thanks for your contribution!
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
On Friday, January 7th, 2022 at 18:26, José Expósito <[email protected]> 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.
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 :-)