2022-01-27 11:23:18

by Tomohito Esaki

[permalink] [raw]
Subject: [RFC PATCH v5 0/3] Add support modifiers for drivers whose planes only support linear layout

Some drivers whose planes only support linear layout fb do not support format
modifiers.
These drivers should support modifiers, however the DRM core should handle this
rather than open-coding in every driver.

In this patch series, these drivers expose format modifiers based on the
following suggestion[1].

On Thu, Nov 18, 2021 at 01:02:11PM +0000, Daniel Stone wrote:
> I think the best way forward here is:
> - add a new mode_config.cannot_support_modifiers flag, and enable
> this in radeon (plus any other drivers in the same boat)
> - change drm_universal_plane_init() to advertise the LINEAR modifier
> when NULL is passed as the modifier list (including installing a
> default .format_mod_supported hook)
> - remove the mode_config.allow_fb_modifiers hook and always
> advertise modifier support, unless
> mode_config.cannot_support_modifiers is set


[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/[email protected]/#24602575

v5:
* rebase to the latest master branch (5.17-rc1+)
+ "drm/plane: Make format_mod_supported truly optional" patch [2]
[2] https://patchwork.freedesktop.org/patch/467940/?series=98255&rev=3

* change default_modifiers array from non-static to static
* remove terminator in default_modifiers array
* use ARRAY_SIZE to get the format_modifier_count
* keep a sanity check in plane init func
* modify several kerneldocs

v4: https://www.spinics.net/lists/dri-devel/msg329508.html
* modify documentation for fb_modifiers_not_supported flag in kerneldoc

v3: https://www.spinics.net/lists/dri-devel/msg329102.html
* change the order as follows:
1. add fb_modifiers_not_supported flag
2. add default modifiers
3. remove allow_fb_modifiers flag
* add a conditional disable in amdgpu_dm_plane_init()

v2: https://www.spinics.net/lists/dri-devel/msg328939.html
* rebase to the latest master branch (5.16.0+)
+ "drm/plane: Make format_mod_supported truly optional" patch [2]

v1: https://www.spinics.net/lists/dri-devel/msg327352.html
* The initial patch set

Tomohito Esaki (3):
drm: introduce fb_modifiers_not_supported flag in mode_config
drm: add support modifiers for drivers whose planes only support
linear layout
drm: remove allow_fb_modifiers

drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 6 ++---
drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 ++
drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 ++
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 1 +
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 ++
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
drivers/gpu/drm/drm_framebuffer.c | 6 ++---
drivers/gpu/drm/drm_ioctl.c | 2 +-
drivers/gpu/drm/drm_plane.c | 23 +++++++++++--------
drivers/gpu/drm/nouveau/nouveau_display.c | 6 +++--
drivers/gpu/drm/radeon/radeon_display.c | 2 ++
.../gpu/drm/selftests/test-drm_framebuffer.c | 1 -
include/drm/drm_mode_config.h | 18 +++++----------
include/drm/drm_plane.h | 3 +++
14 files changed, 45 insertions(+), 32 deletions(-)

--
2.25.1


2022-01-27 11:23:26

by Tomohito Esaki

[permalink] [raw]
Subject: [RFC PATCH v5 2/3] drm: add support modifiers for drivers whose planes only support linear layout

The LINEAR modifier is advertised as default if a driver doesn't specify
modifiers.

Signed-off-by: Tomohito Esaki <[email protected]>
---
drivers/gpu/drm/drm_plane.c | 23 +++++++++++++----------
include/drm/drm_plane.h | 3 +++
2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index deeec60a3315..bf0daa8d9bbd 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -237,6 +237,9 @@ static int __drm_universal_plane_init(struct drm_device *dev,
const char *name, va_list ap)
{
struct drm_mode_config *config = &dev->mode_config;
+ static const uint64_t default_modifiers[] = {
+ DRM_FORMAT_MOD_LINEAR,
+ };
unsigned int format_modifier_count = 0;
int ret;

@@ -277,16 +280,16 @@ static int __drm_universal_plane_init(struct drm_device *dev,

while (*temp_modifiers++ != DRM_FORMAT_MOD_INVALID)
format_modifier_count++;
+ } else {
+ if (!dev->mode_config.fb_modifiers_not_supported) {
+ format_modifiers = default_modifiers;
+ format_modifier_count = ARRAY_SIZE(default_modifiers);
+ }
}

/* autoset the cap and check for consistency across all planes */
- if (format_modifier_count) {
- drm_WARN_ON(dev, !config->allow_fb_modifiers &&
- !list_empty(&config->plane_list));
- config->allow_fb_modifiers = true;
- } else {
- drm_WARN_ON(dev, config->allow_fb_modifiers);
- }
+ drm_WARN_ON(dev, config->fb_modifiers_not_supported &&
+ format_modifier_count);

plane->modifier_count = format_modifier_count;
plane->modifiers = kmalloc_array(format_modifier_count,
@@ -341,7 +344,7 @@ static int __drm_universal_plane_init(struct drm_device *dev,
drm_object_attach_property(&plane->base, config->prop_src_h, 0);
}

- if (config->allow_fb_modifiers)
+ if (format_modifier_count)
create_in_format_blob(dev, plane);

return 0;
@@ -368,8 +371,8 @@ static int __drm_universal_plane_init(struct drm_device *dev,
* drm_universal_plane_init() to let the DRM managed resource infrastructure
* take care of cleanup and deallocation.
*
- * Drivers supporting modifiers must set @format_modifiers on all their planes,
- * even those that only support DRM_FORMAT_MOD_LINEAR.
+ * Drivers that only support the DRM_FORMAT_MOD_LINEAR modifier support may set
+ * @format_modifiers to NULL. The plane will advertise the linear modifier.
*
* Returns:
* Zero on success, error code on failure.
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 0c1102dc4d88..a0390b6ad3b4 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -803,6 +803,9 @@ void *__drmm_universal_plane_alloc(struct drm_device *dev,
*
* The @drm_plane_funcs.destroy hook must be NULL.
*
+ * Drivers that only support the DRM_FORMAT_MOD_LINEAR modifier support may set
+ * @format_modifiers to NULL. The plane will advertise the linear modifier.
+ *
* Returns:
* Pointer to new plane, or ERR_PTR on failure.
*/
--
2.25.1

2022-01-27 11:23:28

by Tomohito Esaki

[permalink] [raw]
Subject: [RFC PATCH v5 3/3] drm: remove allow_fb_modifiers

The allow_fb_modifiers flag is unnecessary since it has been replaced
with fb_modifiers_not_supported flag.

Signed-off-by: Tomohito Esaki <[email protected]>
---
drivers/gpu/drm/selftests/test-drm_framebuffer.c | 1 -
include/drm/drm_mode_config.h | 16 ----------------
2 files changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/selftests/test-drm_framebuffer.c b/drivers/gpu/drm/selftests/test-drm_framebuffer.c
index 61b44d3a6a61..f6d66285c5fc 100644
--- a/drivers/gpu/drm/selftests/test-drm_framebuffer.c
+++ b/drivers/gpu/drm/selftests/test-drm_framebuffer.c
@@ -323,7 +323,6 @@ static struct drm_device mock_drm_device = {
.max_width = MAX_WIDTH,
.min_height = MIN_HEIGHT,
.max_height = MAX_HEIGHT,
- .allow_fb_modifiers = true,
.funcs = &mock_config_funcs,
},
};
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 4a93dac91cf9..6b5e01295348 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -917,22 +917,6 @@ struct drm_mode_config {
*/
bool async_page_flip;

- /**
- * @allow_fb_modifiers:
- *
- * Whether the driver supports fb modifiers in the ADDFB2.1 ioctl call.
- * Note that drivers should not set this directly, it is automatically
- * set in drm_universal_plane_init().
- *
- * IMPORTANT:
- *
- * If this is set the driver must fill out the full implicit modifier
- * information in their &drm_mode_config_funcs.fb_create hook for legacy
- * userspace which does not set modifiers. Otherwise the GETFB2 ioctl is
- * broken for modifier aware userspace.
- */
- bool allow_fb_modifiers;
-
/**
* @fb_modifiers_not_supported:
*
--
2.25.1

2022-01-27 17:25:20

by Daniel Vetter

[permalink] [raw]
Subject: Re: [RFC PATCH v5 0/3] Add support modifiers for drivers whose planes only support linear layout

On Thu, Jan 27, 2022 at 12:25:36PM +0900, Tomohito Esaki wrote:
> Some drivers whose planes only support linear layout fb do not support format
> modifiers.
> These drivers should support modifiers, however the DRM core should handle this
> rather than open-coding in every driver.
>
> In this patch series, these drivers expose format modifiers based on the
> following suggestion[1].
>
> On Thu, Nov 18, 2021 at 01:02:11PM +0000, Daniel Stone wrote:
> > I think the best way forward here is:
> > - add a new mode_config.cannot_support_modifiers flag, and enable
> > this in radeon (plus any other drivers in the same boat)
> > - change drm_universal_plane_init() to advertise the LINEAR modifier
> > when NULL is passed as the modifier list (including installing a
> > default .format_mod_supported hook)
> > - remove the mode_config.allow_fb_modifiers hook and always
> > advertise modifier support, unless
> > mode_config.cannot_support_modifiers is set
>
>
> [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/[email protected]/#24602575

Two procedural things:
- There's an r-b on all the patches from Andy from the last round, please
include that.
- Please also include a changelog per-patch (at least going forward), that
helps with judging where a patch series is.

But aside from this I think this looks ready.

Cheers, Daniel

>
> v5:
> * rebase to the latest master branch (5.17-rc1+)
> + "drm/plane: Make format_mod_supported truly optional" patch [2]
> [2] https://patchwork.freedesktop.org/patch/467940/?series=98255&rev=3
>
> * change default_modifiers array from non-static to static
> * remove terminator in default_modifiers array
> * use ARRAY_SIZE to get the format_modifier_count
> * keep a sanity check in plane init func
> * modify several kerneldocs
>
> v4: https://www.spinics.net/lists/dri-devel/msg329508.html
> * modify documentation for fb_modifiers_not_supported flag in kerneldoc
>
> v3: https://www.spinics.net/lists/dri-devel/msg329102.html
> * change the order as follows:
> 1. add fb_modifiers_not_supported flag
> 2. add default modifiers
> 3. remove allow_fb_modifiers flag
> * add a conditional disable in amdgpu_dm_plane_init()
>
> v2: https://www.spinics.net/lists/dri-devel/msg328939.html
> * rebase to the latest master branch (5.16.0+)
> + "drm/plane: Make format_mod_supported truly optional" patch [2]
>
> v1: https://www.spinics.net/lists/dri-devel/msg327352.html
> * The initial patch set
>
> Tomohito Esaki (3):
> drm: introduce fb_modifiers_not_supported flag in mode_config
> drm: add support modifiers for drivers whose planes only support
> linear layout
> drm: remove allow_fb_modifiers
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 6 ++---
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 2 ++
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 2 ++
> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 1 +
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 2 ++
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
> drivers/gpu/drm/drm_framebuffer.c | 6 ++---
> drivers/gpu/drm/drm_ioctl.c | 2 +-
> drivers/gpu/drm/drm_plane.c | 23 +++++++++++--------
> drivers/gpu/drm/nouveau/nouveau_display.c | 6 +++--
> drivers/gpu/drm/radeon/radeon_display.c | 2 ++
> .../gpu/drm/selftests/test-drm_framebuffer.c | 1 -
> include/drm/drm_mode_config.h | 18 +++++----------
> include/drm/drm_plane.h | 3 +++
> 14 files changed, 45 insertions(+), 32 deletions(-)
>
> --
> 2.25.1
>

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch