On Fri, Apr 20, 2018 at 07:01:49PM +0530, Nautiyal, Ankit K wrote:
> From: Ankit Nautiyal <ankit.k.nauti...@intel.com>
> 
> We parse the EDID and add all the modes in the connector's modelist.
> This adds CEA modes with aspect ratio information too, regardless of
> whether user space requested this information or not.
> 
> This patch prunes the modes with aspect-ratio information, from a
> connector's modelist, if the user-space has not set the aspect ratio
> DRM client cap. However if such a mode is unique in the list, it is
> kept in the list, with aspect-ratio flags reset.
> 
> Cc: Ville Syrjala <ville.syrj...@linux.intel.com>
> Cc: Shashank Sharma <shashank.sha...@intel.com>
> Cc: Jose Abreu <jose.ab...@synopsys.com>
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nauti...@intel.com>
> 
> V3: As suggested by Ville, modified the mechanism of pruning of modes
>     with aspect-ratio, if the aspect-ratio is not supported. Instead
>     of straight away pruning such a mode, the mode is retained with
>     aspect ratio bits set to zero, provided it is unique.
> V4: rebase
> V5: Addressed review comments from Ville:
>     -used a pointer to store last valid mode.
>     -avoided, modifying of picture_aspect_ratio in kernel mode,
>      instead only flags bits of user mode are reset (if aspect-ratio
>      is not supported).
> V6: As suggested by Ville, corrected the mode pruning logic and
>     elaborated the mode pruning logic and the assumptions taken.
> V7: rebase
> V8: rebase
> V9: rebase
> V10: rebase
> V11: Fixed the issue caused in kms_3d test, and enhanced the pruning
>      logic to correctly identify and prune modes with aspect-ratio,
>      if aspect-ratio cap is not set.
> ---
>  drivers/gpu/drm/drm_connector.c | 56 
> +++++++++++++++++++++++++++++++++++------
>  1 file changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b3cde89..865ee354 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1531,15 +1531,35 @@ static struct drm_encoder 
> *drm_connector_get_encoder(struct drm_connector *conne
>       return connector->encoder;
>  }
>  
> -static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
> -                                      const struct drm_file *file_priv)
> +static bool
> +drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
> +                          const struct drm_display_mode *modelist,
> +                          const struct drm_file *file_priv)
>  {
>       /*
>        * If user-space hasn't configured the driver to expose the stereo 3D
>        * modes, don't expose them.
>        */
> +     struct drm_display_mode *mode_itr;
> +
>       if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode))
>               return false;
> +     /*
> +      * If user-space hasn't configured the driver to expose the modes
> +      * with aspect-ratio, don't expose them. However if such a mode
> +      * is unique, let it be exposed, but reset the aspect-ratio flags
> +      * while preparing the list of user-modes.
> +      */
> +     if (!file_priv->aspect_ratio_allowed &&
> +         mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE) {
> +             list_for_each_entry(mode_itr, &modelist->head, head)
> +                     if (drm_mode_match(mode_itr, mode,
> +                                        DRM_MODE_MATCH_TIMINGS |
> +                                        DRM_MODE_MATCH_CLOCK |
> +                                        DRM_MODE_MATCH_FLAGS |
> +                                        DRM_MODE_MATCH_3D_FLAGS))
> +                             return false;
> +     }
>  
>       return true;
>  }
> @@ -1550,7 +1570,7 @@ int drm_mode_getconnector(struct drm_device *dev, void 
> *data,
>       struct drm_mode_get_connector *out_resp = data;
>       struct drm_connector *connector;
>       struct drm_encoder *encoder;
> -     struct drm_display_mode *mode;
> +     struct drm_display_mode *mode, *tmp, modelist;
>       int mode_count = 0;
>       int encoders_count = 0;
>       int ret = 0;
> @@ -1605,23 +1625,37 @@ int drm_mode_getconnector(struct drm_device *dev, 
> void *data,
>       out_resp->subpixel = connector->display_info.subpixel_order;
>       out_resp->connection = connector->status;
>  
> +     INIT_LIST_HEAD(&modelist.head);

Why are we using a struct drm_display_mode to get a simple list_head?

> +
>       /* delayed so we get modes regardless of pre-fill_modes state */
>       list_for_each_entry(mode, &connector->modes, head)
> -             if (drm_mode_expose_to_userspace(mode, file_priv))
> +             if (drm_mode_expose_to_userspace(mode, &modelist,
> +                                              file_priv)) {
> +                     struct drm_display_mode *tmp_mode;
> +
> +                     tmp_mode = drm_mode_duplicate(dev, mode);

Duplicating every mode seems rather wasteful. I suppose we could
just add another list_head to struct drm_display_mode for this
purpose. An alternative could be to somehow mark each exposed mode
and just walk the original connector mode list.

> +                     list_add_tail(&tmp_mode->head, &modelist.head);
>                       mode_count++;
> +             }
>  
>       /*
>        * This ioctl is called twice, once to determine how much space is
>        * needed, and the 2nd time to fill it.
> +      * The modes that need to be exposed to the user are maintained in the
> +      * 'modelist'. When the ioctl is called first time to determine the,
> +      * space, the modelist gets filled, to find the no. of modes. In the
> +      * 2nd time, the user modes are filled, one by one from the modelist.
>        */
>       if ((out_resp->count_modes >= mode_count) && mode_count) {
>               copied = 0;
>               mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned 
> long)out_resp->modes_ptr;
> -             list_for_each_entry(mode, &connector->modes, head) {
> -                     if (!drm_mode_expose_to_userspace(mode, file_priv))
> -                             continue;
> -
> +             list_for_each_entry(mode, &modelist.head, head) {
>                       drm_mode_convert_to_umode(&u_mode, mode);
> +                     /*
> +                      * Reset aspect ratio flags of user-mode, if modes with
> +                      * aspect-ratio are not supported.
> +                      */
> +                     drm_mode_filter_aspect_ratio_flags(file_priv, &u_mode);
>                       if (copy_to_user(mode_ptr + copied,
>                                        &u_mode, sizeof(u_mode))) {
>                               ret = -EFAULT;
> @@ -1651,6 +1685,12 @@ int drm_mode_getconnector(struct drm_device *dev, void 
> *data,
>       drm_modeset_unlock(&dev->mode_config.connection_mutex);
>  
>  out:
> +     list_for_each_entry_safe(mode, tmp, &modelist.head, head) {
> +             list_del(&mode->head);
> +             drm_mode_destroy(dev, mode);
> +     }
> +     list_del(&modelist.head);
> +
>       drm_connector_put(connector);
>  
>       return ret;
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to