Hello Laurent-san, On 2019/05/12 3:10, Laurent Pinchart wrote: > Hello Esaki-san, > > On Thu, May 09, 2019 at 06:25:19PM +0900, Esaki Tomohito wrote: >> Hi Laurent-san >> >>> What's the purpose of this, as it adds no new functionality to the >>> driver ? Why is this change needed ? >> >> Weston compositor (v5.0.0 or later) uses the DRM API to get the >> supported modifiers and determines if the sprite plane can be used by >> comparing the modifiers with the client specified modifier. >> In currently driver, since the weston doesn't know supported modifiers, >> that cannot determine if the received dmabuf can be passed through to >> sprite plane. >> Since there are R-Car GPU which support linear modifier, the sprite >> plane cannot be used in a compositor similar to the weston if client >> specify linear modifier. > > I don't think the right solution is to expose the linear modifier from > all drivers that don't otherwise support modifiers. We should instead > fix it either in Weston, and treat drivers that don't support the > modifiers API as supporting the linear modifier only, or in the DRM/KMS > core by reporting the linear modifier for drivers that don't explicitly > support modifiers. I would like to ask for an opinion to wayland community. I will add you to Cc in that mail. Thanks! >> On 2019/05/09 16:14, Laurent Pinchart wrote: >>> On Thu, May 09, 2019 at 02:45:18PM +0900, Tomohito Esaki wrote: >>>> Add support for the linear modifier. Since the rcar-du device supports >>>> only linear modifier, this driver doesn't support other modifiers. >>> >>> What's the purpose of this, as it adds no new functionality to the >>> driver ? Why is this change needed ? >>> >>>> Signed-off-by: Tomohito Esaki <etom@xxxxxxxxxx> >>>> --- >>>> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 11 +++++++++++ >>>> drivers/gpu/drm/rcar-du/rcar_du_plane.c | 15 ++++++++++++++- >>>> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 15 ++++++++++++++- >>>> 3 files changed, 39 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c >>>> index 3b7d50a8fb9b..9c5e15a5ab1c 100644 >>>> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c >>>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c >>>> @@ -214,6 +214,16 @@ rcar_du_fb_create(struct drm_device *dev, struct drm_file *file_priv, >>>> return ERR_PTR(-EINVAL); >>>> } >>>> >>>> + /* >>>> + * Support only LINEAR modifier. >>>> + */ >>>> + if ((mode_cmd->flags & DRM_MODE_FB_MODIFIERS) && >>>> + mode_cmd->modifier[0] != DRM_FORMAT_MOD_LINEAR) { >>>> + dev_dbg(dev->dev, "unsupported fb modifier 0x%llx\n", >>>> + mode_cmd->modifier[0]); >>>> + return ERR_PTR(-EINVAL); >>>> + } >>>> + >>>> if (rcdu->info->gen < 3) { >>>> /* >>>> * On Gen2 the DU limits the pitch to 4095 pixels and requires >>>> @@ -529,6 +539,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) >>>> dev->mode_config.min_width = 0; >>>> dev->mode_config.min_height = 0; >>>> dev->mode_config.normalize_zpos = true; >>>> + dev->mode_config.allow_fb_modifiers = true; >>>> dev->mode_config.funcs = &rcar_du_mode_config_funcs; >>>> dev->mode_config.helper_private = &rcar_du_mode_config_helper; >>>> >>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c >>>> index c6430027169f..32135ad387fa 100644 >>>> --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c >>>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c >>>> @@ -726,6 +726,13 @@ static int rcar_du_plane_atomic_get_property(struct drm_plane *plane, >>>> return 0; >>>> } >>>> >>>> +static bool rcar_du_plane_format_mod_supported(struct drm_plane *plane, >>>> + uint32_t format, >>>> + uint64_t modifier) >>>> +{ >>>> + return modifier == DRM_FORMAT_MOD_LINEAR; >>>> +} >>>> + >>>> static const struct drm_plane_funcs rcar_du_plane_funcs = { >>>> .update_plane = drm_atomic_helper_update_plane, >>>> .disable_plane = drm_atomic_helper_disable_plane, >>>> @@ -735,6 +742,7 @@ static const struct drm_plane_funcs rcar_du_plane_funcs = { >>>> .atomic_destroy_state = rcar_du_plane_atomic_destroy_state, >>>> .atomic_set_property = rcar_du_plane_atomic_set_property, >>>> .atomic_get_property = rcar_du_plane_atomic_get_property, >>>> + .format_mod_supported = rcar_du_plane_format_mod_supported, >>>> }; >>>> >>>> static const uint32_t formats[] = { >>>> @@ -750,6 +758,11 @@ static const uint32_t formats[] = { >>>> DRM_FORMAT_NV16, >>>> }; >>>> >>>> +static const uint64_t modifiers[] = { >>>> + DRM_FORMAT_MOD_LINEAR, >>>> + DRM_FORMAT_MOD_INVALID, >>>> +}; >>>> + >>>> int rcar_du_planes_init(struct rcar_du_group *rgrp) >>>> { >>>> struct rcar_du_device *rcdu = rgrp->dev; >>>> @@ -776,7 +789,7 @@ int rcar_du_planes_init(struct rcar_du_group *rgrp) >>>> ret = drm_universal_plane_init(rcdu->ddev, &plane->plane, crtcs, >>>> &rcar_du_plane_funcs, formats, >>>> ARRAY_SIZE(formats), >>>> - NULL, type, NULL); >>>> + modifiers, type, NULL); >>>> if (ret < 0) >>>> return ret; >>>> >>>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c >>>> index 0878accbd134..9d1382b02717 100644 >>>> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c >>>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c >>>> @@ -139,6 +139,11 @@ static const u32 formats_kms[] = { >>>> DRM_FORMAT_YVU444, >>>> }; >>>> >>>> +static const uint64_t modifiers_kms[] = { >>>> + DRM_FORMAT_MOD_LINEAR, >>>> + DRM_FORMAT_MOD_INVALID, >>>> +}; >>>> + >>>> static const u32 formats_v4l2[] = { >>>> V4L2_PIX_FMT_RGB332, >>>> V4L2_PIX_FMT_ARGB444, >>>> @@ -344,6 +349,13 @@ static void rcar_du_vsp_plane_reset(struct drm_plane *plane) >>>> state->state.zpos = plane->type == DRM_PLANE_TYPE_PRIMARY ? 0 : 1; >>>> } >>>> >>>> +static bool rcar_du_vsp_plane_format_mod_supported(struct drm_plane *plane, >>>> + uint32_t format, >>>> + uint64_t modifier) >>>> +{ >>>> + return modifier == DRM_FORMAT_MOD_LINEAR; >>>> +} >>>> + >>>> static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = { >>>> .update_plane = drm_atomic_helper_update_plane, >>>> .disable_plane = drm_atomic_helper_disable_plane, >>>> @@ -351,6 +363,7 @@ static const struct drm_plane_funcs rcar_du_vsp_plane_funcs = { >>>> .destroy = drm_plane_cleanup, >>>> .atomic_duplicate_state = rcar_du_vsp_plane_atomic_duplicate_state, >>>> .atomic_destroy_state = rcar_du_vsp_plane_atomic_destroy_state, >>>> + .format_mod_supported = rcar_du_vsp_plane_format_mod_supported, >>>> }; >>>> >>>> int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np, >>>> @@ -397,7 +410,7 @@ int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np, >>>> &rcar_du_vsp_plane_funcs, >>>> formats_kms, >>>> ARRAY_SIZE(formats_kms), >>>> - NULL, type, NULL); >>>> + modifiers_kms, type, NULL); >>>> if (ret < 0) >>>> return ret; >>>> > -- -------------------------- Tomohito Esaki etom@xxxxxxxxxx