[RFC PATCH v2 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/20190509054518....
v2: * rebase to the latest master branch (5.16.0+) + "drm/plane: Make format_mod_supported truly optional" patch [2] [2] https://patchwork.freedesktop.org/patch/467940/?series=98255&rev=3
v1: https://www.spinics.net/lists/dri-devel/msg327352.html * The initial patch set
Tomohito Esaki (3): drm: add support modifiers for drivers whose planes only support linear layout drm: set fb_modifiers_not_supported flag in legacy drivers drm: replace allow_fb_modifiers with fb_modifiers_not_supported
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 ++ drivers/gpu/drm/drm_framebuffer.c | 6 ++--- drivers/gpu/drm/drm_ioctl.c | 2 +- drivers/gpu/drm/drm_plane.c | 22 +++++++++---------- 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 +++ 13 files changed, 40 insertions(+), 33 deletions(-)
The LINEAR modifier is advertised as default if a driver doesn't specify modifiers. However, there are legacy drivers such as radeon that do not support modifiers but infer the actual layout of the underlying buffer. Therefore, a new flag not_support_fb_modifires is introduced for these legacy drivers. Allow_fb_modifiers will be replaced with this new flag.
Signed-off-by: Tomohito Esaki etom@igel.co.jp --- drivers/gpu/drm/drm_plane.c | 15 ++++++++++++--- include/drm/drm_mode_config.h | 10 ++++++++++ include/drm/drm_plane.h | 3 +++ 3 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index deeec60a3315..5aa7e241971e 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -237,6 +237,10 @@ static int __drm_universal_plane_init(struct drm_device *dev, const char *name, va_list ap) { struct drm_mode_config *config = &dev->mode_config; + const uint64_t default_modifiers[] = { + DRM_FORMAT_MOD_LINEAR, + DRM_FORMAT_MOD_INVALID + }; unsigned int format_modifier_count = 0; int ret;
@@ -277,6 +281,11 @@ 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 = 1; + } }
/* autoset the cap and check for consistency across all planes */ @@ -341,7 +350,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 +377,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. + * For drivers supporting modifiers, all planes will advertise + * DRM_FORMAT_MOD_LINEAR support, if @format_modifiers is not set. * * Returns: * Zero on success, error code on failure. diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 48b7de80daf5..c56f298c55bd 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -920,6 +920,16 @@ struct drm_mode_config { */ bool allow_fb_modifiers;
+ /** + * @fb_modifiers_not_supported: + * + * This flag is for legacy drivers such as radeon that do not support + * modifiers but infer the actual layout of the underlying buffer. + * Generally, each drivers must support modifiers, this flag should not + * be set. + */ + bool fb_modifiers_not_supported; + /** * @normalize_zpos: * diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index 0c1102dc4d88..cad641b1f797 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. * + * For drivers supporting modifiers, all planes will advertise + * DRM_FORMAT_MOD_LINEAR support, if @format_modifiers is not set. + * * Returns: * Pointer to new plane, or ERR_PTR on failure. */
Set fb_modifiers_not_supported flag in legacy drivers whose planes support non-linear layouts but does not support modifiers, and replace allow_fb_modifiers with fb_modifiers_not_supported.
Signed-off-by: Tomohito Esaki etom@igel.co.jp --- 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 ++ drivers/gpu/drm/nouveau/nouveau_display.c | 6 ++++-- drivers/gpu/drm/radeon/radeon_display.c | 2 ++ 7 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index dc50c05f23fc..cbaea9c6cfda 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -958,7 +958,7 @@ static int amdgpu_display_verify_sizes(struct amdgpu_framebuffer *rfb) int ret; unsigned int i, block_width, block_height, block_size_log2;
- if (!rfb->base.dev->mode_config.allow_fb_modifiers) + if (rfb->base.dev->mode_config.fb_modifiers_not_supported) return 0;
for (i = 0; i < format_info->num_planes; ++i) { @@ -1145,7 +1145,7 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev, if (ret) return ret;
- if (!dev->mode_config.allow_fb_modifiers) { + if (dev->mode_config.fb_modifiers_not_supported) { drm_WARN_ONCE(dev, adev->family >= AMDGPU_FAMILY_AI, "GFX9+ requires FB check based on format modifier\n"); ret = check_tiling_flags_gfx6(rfb); @@ -1153,7 +1153,7 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev, return ret; }
- if (dev->mode_config.allow_fb_modifiers && + if (!dev->mode_config.fb_modifiers_not_supported && !(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) { ret = convert_tiling_flags_to_modifier(rfb); if (ret) { diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c index d1570a462a51..fb61c0814115 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c @@ -2798,6 +2798,8 @@ static int dce_v10_0_sw_init(void *handle) adev_to_drm(adev)->mode_config.preferred_depth = 24; adev_to_drm(adev)->mode_config.prefer_shadow = 1;
+ adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true; + adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base;
r = amdgpu_display_modeset_create_props(adev); diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c index 18a7b3bd633b..17942a11366d 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c @@ -2916,6 +2916,8 @@ static int dce_v11_0_sw_init(void *handle) adev_to_drm(adev)->mode_config.preferred_depth = 24; adev_to_drm(adev)->mode_config.prefer_shadow = 1;
+ adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true; + adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base;
r = amdgpu_display_modeset_create_props(adev); diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c index c7803dc2b2d5..2ec99ec8e1a3 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c @@ -2674,6 +2674,7 @@ static int dce_v6_0_sw_init(void *handle) adev_to_drm(adev)->mode_config.max_height = 16384; adev_to_drm(adev)->mode_config.preferred_depth = 24; adev_to_drm(adev)->mode_config.prefer_shadow = 1; + adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true; adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base;
r = amdgpu_display_modeset_create_props(adev); diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c index b200b9e722d9..8369336cec90 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c @@ -2699,6 +2699,8 @@ static int dce_v8_0_sw_init(void *handle) adev_to_drm(adev)->mode_config.preferred_depth = 24; adev_to_drm(adev)->mode_config.prefer_shadow = 1;
+ adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true; + adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base;
r = amdgpu_display_modeset_create_props(adev); diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 929de41c281f..1ecad7fa3e8a 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -711,10 +711,12 @@ nouveau_display_create(struct drm_device *dev) &disp->disp); if (ret == 0) { nouveau_display_create_properties(dev); - if (disp->disp.object.oclass < NV50_DISP) + if (disp->disp.object.oclass < NV50_DISP) { + dev->mode_config.fb_modifiers_not_supported = true; ret = nv04_display_create(dev); - else + } else { ret = nv50_display_create(dev); + } } } else { ret = 0; diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index 573154268d43..b9a07677a71e 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -1596,6 +1596,8 @@ int radeon_modeset_init(struct radeon_device *rdev) rdev->ddev->mode_config.preferred_depth = 24; rdev->ddev->mode_config.prefer_shadow = 1;
+ rdev->ddev->mode_config.fb_modifiers_not_supported = true; + rdev->ddev->mode_config.fb_base = rdev->mc.aper_base;
ret = radeon_modeset_create_props(rdev);
I think we'll also want to do a conditional disable for DC (drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c) since it only enables modifiers on newer HW. Something like "if (modifiers == NULL) fb_modifiers_not_supported = true;" in amdgpu_dm_plane_init.
On Thu, Jan 13, 2022 at 10:44 AM Tomohito Esaki etom@igel.co.jp wrote:
Set fb_modifiers_not_supported flag in legacy drivers whose planes support non-linear layouts but does not support modifiers, and replace allow_fb_modifiers with fb_modifiers_not_supported.
Signed-off-by: Tomohito Esaki etom@igel.co.jp
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 ++ drivers/gpu/drm/nouveau/nouveau_display.c | 6 ++++-- drivers/gpu/drm/radeon/radeon_display.c | 2 ++ 7 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index dc50c05f23fc..cbaea9c6cfda 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -958,7 +958,7 @@ static int amdgpu_display_verify_sizes(struct amdgpu_framebuffer *rfb) int ret; unsigned int i, block_width, block_height, block_size_log2;
if (!rfb->base.dev->mode_config.allow_fb_modifiers)
if (rfb->base.dev->mode_config.fb_modifiers_not_supported) return 0; for (i = 0; i < format_info->num_planes; ++i) {@@ -1145,7 +1145,7 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev, if (ret) return ret;
if (!dev->mode_config.allow_fb_modifiers) {
if (dev->mode_config.fb_modifiers_not_supported) { drm_WARN_ONCE(dev, adev->family >= AMDGPU_FAMILY_AI, "GFX9+ requires FB check based on format modifier\n"); ret = check_tiling_flags_gfx6(rfb);@@ -1153,7 +1153,7 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev, return ret; }
if (dev->mode_config.allow_fb_modifiers &&
if (!dev->mode_config.fb_modifiers_not_supported && !(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) { ret = convert_tiling_flags_to_modifier(rfb); if (ret) {diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c index d1570a462a51..fb61c0814115 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c @@ -2798,6 +2798,8 @@ static int dce_v10_0_sw_init(void *handle) adev_to_drm(adev)->mode_config.preferred_depth = 24; adev_to_drm(adev)->mode_config.prefer_shadow = 1;
adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; r = amdgpu_display_modeset_create_props(adev);diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c index 18a7b3bd633b..17942a11366d 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c @@ -2916,6 +2916,8 @@ static int dce_v11_0_sw_init(void *handle) adev_to_drm(adev)->mode_config.preferred_depth = 24; adev_to_drm(adev)->mode_config.prefer_shadow = 1;
adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; r = amdgpu_display_modeset_create_props(adev);diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c index c7803dc2b2d5..2ec99ec8e1a3 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c @@ -2674,6 +2674,7 @@ static int dce_v6_0_sw_init(void *handle) adev_to_drm(adev)->mode_config.max_height = 16384; adev_to_drm(adev)->mode_config.preferred_depth = 24; adev_to_drm(adev)->mode_config.prefer_shadow = 1;
adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true; adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; r = amdgpu_display_modeset_create_props(adev);diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c index b200b9e722d9..8369336cec90 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c @@ -2699,6 +2699,8 @@ static int dce_v8_0_sw_init(void *handle) adev_to_drm(adev)->mode_config.preferred_depth = 24; adev_to_drm(adev)->mode_config.prefer_shadow = 1;
adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; r = amdgpu_display_modeset_create_props(adev);diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 929de41c281f..1ecad7fa3e8a 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -711,10 +711,12 @@ nouveau_display_create(struct drm_device *dev) &disp->disp); if (ret == 0) { nouveau_display_create_properties(dev);
if (disp->disp.object.oclass < NV50_DISP)
if (disp->disp.object.oclass < NV50_DISP) {dev->mode_config.fb_modifiers_not_supported = true; ret = nv04_display_create(dev);
else
} else { ret = nv50_display_create(dev);} } } else { ret = 0;diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index 573154268d43..b9a07677a71e 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -1596,6 +1596,8 @@ int radeon_modeset_init(struct radeon_device *rdev) rdev->ddev->mode_config.preferred_depth = 24; rdev->ddev->mode_config.prefer_shadow = 1;
rdev->ddev->mode_config.fb_modifiers_not_supported = true;rdev->ddev->mode_config.fb_base = rdev->mc.aper_base; ret = radeon_modeset_create_props(rdev);-- 2.25.1
Hi,
Thank you for your comment.
On 2022/01/14 2:56, Bas Nieuwenhuizen wrote:
I think we'll also want to do a conditional disable for DC (drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c) since it only enables modifiers on newer HW. Something like "if (modifiers == NULL) fb_modifiers_not_supported = true;" in amdgpu_dm_plane_init.
Ok, I'll add a conditional disable in amdgpu_dm_plane_init().
Thanks, Tomohito Esaki
On Thu, Jan 13, 2022 at 10:44 AM Tomohito Esaki etom@igel.co.jp wrote:
Set fb_modifiers_not_supported flag in legacy drivers whose planes support non-linear layouts but does not support modifiers, and replace allow_fb_modifiers with fb_modifiers_not_supported.
Signed-off-by: Tomohito Esaki etom@igel.co.jp
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 ++ drivers/gpu/drm/nouveau/nouveau_display.c | 6 ++++-- drivers/gpu/drm/radeon/radeon_display.c | 2 ++ 7 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index dc50c05f23fc..cbaea9c6cfda 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -958,7 +958,7 @@ static int amdgpu_display_verify_sizes(struct amdgpu_framebuffer *rfb) int ret; unsigned int i, block_width, block_height, block_size_log2;
if (!rfb->base.dev->mode_config.allow_fb_modifiers)
if (rfb->base.dev->mode_config.fb_modifiers_not_supported) return 0; for (i = 0; i < format_info->num_planes; ++i) {@@ -1145,7 +1145,7 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev, if (ret) return ret;
if (!dev->mode_config.allow_fb_modifiers) {
if (dev->mode_config.fb_modifiers_not_supported) { drm_WARN_ONCE(dev, adev->family >= AMDGPU_FAMILY_AI, "GFX9+ requires FB check based on format modifier\n"); ret = check_tiling_flags_gfx6(rfb);@@ -1153,7 +1153,7 @@ int amdgpu_display_framebuffer_init(struct drm_device *dev, return ret; }
if (dev->mode_config.allow_fb_modifiers &&
if (!dev->mode_config.fb_modifiers_not_supported && !(rfb->base.flags & DRM_MODE_FB_MODIFIERS)) { ret = convert_tiling_flags_to_modifier(rfb); if (ret) {diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c index d1570a462a51..fb61c0814115 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c @@ -2798,6 +2798,8 @@ static int dce_v10_0_sw_init(void *handle) adev_to_drm(adev)->mode_config.preferred_depth = 24; adev_to_drm(adev)->mode_config.prefer_shadow = 1;
adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; r = amdgpu_display_modeset_create_props(adev);diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c index 18a7b3bd633b..17942a11366d 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c @@ -2916,6 +2916,8 @@ static int dce_v11_0_sw_init(void *handle) adev_to_drm(adev)->mode_config.preferred_depth = 24; adev_to_drm(adev)->mode_config.prefer_shadow = 1;
adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; r = amdgpu_display_modeset_create_props(adev);diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c index c7803dc2b2d5..2ec99ec8e1a3 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c @@ -2674,6 +2674,7 @@ static int dce_v6_0_sw_init(void *handle) adev_to_drm(adev)->mode_config.max_height = 16384; adev_to_drm(adev)->mode_config.preferred_depth = 24; adev_to_drm(adev)->mode_config.prefer_shadow = 1;
adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true; adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; r = amdgpu_display_modeset_create_props(adev);diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c index b200b9e722d9..8369336cec90 100644 --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c @@ -2699,6 +2699,8 @@ static int dce_v8_0_sw_init(void *handle) adev_to_drm(adev)->mode_config.preferred_depth = 24; adev_to_drm(adev)->mode_config.prefer_shadow = 1;
adev_to_drm(adev)->mode_config.fb_modifiers_not_supported = true;adev_to_drm(adev)->mode_config.fb_base = adev->gmc.aper_base; r = amdgpu_display_modeset_create_props(adev);diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 929de41c281f..1ecad7fa3e8a 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -711,10 +711,12 @@ nouveau_display_create(struct drm_device *dev) &disp->disp); if (ret == 0) { nouveau_display_create_properties(dev);
if (disp->disp.object.oclass < NV50_DISP)
if (disp->disp.object.oclass < NV50_DISP) {dev->mode_config.fb_modifiers_not_supported = true; ret = nv04_display_create(dev);
else
} else { ret = nv50_display_create(dev);} } } else { ret = 0;diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index 573154268d43..b9a07677a71e 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -1596,6 +1596,8 @@ int radeon_modeset_init(struct radeon_device *rdev) rdev->ddev->mode_config.preferred_depth = 24; rdev->ddev->mode_config.prefer_shadow = 1;
rdev->ddev->mode_config.fb_modifiers_not_supported = true;rdev->ddev->mode_config.fb_base = rdev->mc.aper_base; ret = radeon_modeset_create_props(rdev);-- 2.25.1
Since almost drivers support fb modifiers, allow_fb_modifiers is replaced with fb_modifiers_not_supported and removed.
Signed-off-by: Tomohito Esaki etom@igel.co.jp --- drivers/gpu/drm/drm_framebuffer.c | 6 +++--- drivers/gpu/drm/drm_ioctl.c | 2 +- drivers/gpu/drm/drm_plane.c | 9 --------- drivers/gpu/drm/selftests/test-drm_framebuffer.c | 1 - include/drm/drm_mode_config.h | 16 ---------------- 5 files changed, 4 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index 07f5abc875e9..4562a8b86579 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -309,7 +309,7 @@ drm_internal_framebuffer_create(struct drm_device *dev, }
if (r->flags & DRM_MODE_FB_MODIFIERS && - !dev->mode_config.allow_fb_modifiers) { + dev->mode_config.fb_modifiers_not_supported) { DRM_DEBUG_KMS("driver does not support fb modifiers\n"); return ERR_PTR(-EINVAL); } @@ -594,7 +594,7 @@ int drm_mode_getfb2_ioctl(struct drm_device *dev, r->pixel_format = fb->format->format;
r->flags = 0; - if (dev->mode_config.allow_fb_modifiers) + if (!dev->mode_config.fb_modifiers_not_supported) r->flags |= DRM_MODE_FB_MODIFIERS;
for (i = 0; i < ARRAY_SIZE(r->handles); i++) { @@ -607,7 +607,7 @@ int drm_mode_getfb2_ioctl(struct drm_device *dev, for (i = 0; i < fb->format->num_planes; i++) { r->pitches[i] = fb->pitches[i]; r->offsets[i] = fb->offsets[i]; - if (dev->mode_config.allow_fb_modifiers) + if (!dev->mode_config.fb_modifiers_not_supported) r->modifier[i] = fb->modifier; }
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 8b8744dcf691..51fcf1298023 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -297,7 +297,7 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ req->value = 64; break; case DRM_CAP_ADDFB2_MODIFIERS: - req->value = dev->mode_config.allow_fb_modifiers; + req->value = !dev->mode_config.fb_modifiers_not_supported; break; case DRM_CAP_CRTC_IN_VBLANK_EVENT: req->value = 1; diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 5aa7e241971e..89a3d044ab59 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -288,15 +288,6 @@ static int __drm_universal_plane_init(struct drm_device *dev, } }
- /* 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); - } - plane->modifier_count = format_modifier_count; plane->modifiers = kmalloc_array(format_modifier_count, sizeof(format_modifiers[0]), 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 c56f298c55bd..6fd13d6510f1 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -904,22 +904,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: *
Hi Esaki-san,
On Thu, 13 Jan 2022 at 09:44, Tomohito Esaki etom@igel.co.jp 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].
Thanks for the series, it looks like the right thing to do.
Can you please change the patch ordering though? At the moment there will be a bisection break at patch #1, because the legacy drivers will suddenly start gaining modifier support, before it is removed in patch #2.
I think a better order would be: 1: add fb_modifiers_not_supported flag to core and drivers 2: add default modifiers (and set allow_fb_modifiers) if fb_modifiers_not_supported flag is not set 3: remove allow_fb_modifiers flag
Cheers, Daniel
Hi Daniel-san,
Thank you for your comments.
On 2022/01/13 22:44, Daniel Stone wrote:
Hi Esaki-san,
On Thu, 13 Jan 2022 at 09:44, Tomohito Esaki etom@igel.co.jp 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].
Thanks for the series, it looks like the right thing to do.
Can you please change the patch ordering though? At the moment there will be a bisection break at patch #1, because the legacy drivers will suddenly start gaining modifier support, before it is removed in patch #2.
I think a better order would be: 1: add fb_modifiers_not_supported flag to core and drivers 2: add default modifiers (and set allow_fb_modifiers) if fb_modifiers_not_supported flag is not set 3: remove allow_fb_modifiers flag
I agree to your proposal. I will fix these patches.
dri-devel@lists.freedesktop.org
-
Bas Nieuwenhuizen -
Daniel Stone -
Esaki Tomohito -
Tomohito Esaki