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 <e...@igel.co.jp>
>>>> ---
>>>>  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
e...@igel.co.jp

Reply via email to