Re: [REBASE 3/5] drm: Expose modes with aspect ratio, only if requested

[Ville Syrjälä]

On Fri, Nov 24, 2017 at 02:36:17PM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 11/21/2017 10:41 PM, Ville Syrjälä wrote:
> > On Fri, Nov 17, 2017 at 03:00:30PM +0530, Shashank Sharma wrote:
> >> From: aknautiy 
> >>
> >> We parse the EDID and add all the modes in the connector's
> >> modelist. This adds CEA modes with aspect ratio information
> >> too, regadless of if 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.
> >>
> >> Cc: Ville Syrjala 
> >> Cc: Shashank Sharma 
> >> Cc: Jose Abreu 
> >>
> >> Signed-off-by: aknautiy 
> >> ---
> >>   drivers/gpu/drm/drm_connector.c | 7 +++
> >>   1 file changed, 7 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_connector.c 
> >> b/drivers/gpu/drm/drm_connector.c
> >> index 704fc89..a246bb5 100644
> >> --- a/drivers/gpu/drm/drm_connector.c
> >> +++ b/drivers/gpu/drm/drm_connector.c
> >> @@ -1285,6 +1285,13 @@ static bool drm_mode_expose_to_userspace(const 
> >> struct drm_display_mode *mode,
> >> */
> >>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.
> >> +   */
> >> +  if (!file_priv->aspect_ratio_allowed &&
> >> +  mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE)
> >> +  return false;
> > I don't think we can just blindly drop the modes. We would have to
> > expose them with the aspect ratio cleared. That could lead to
> > duplicates, but I'm thinking that shouldn't be a real problem for
> > userspace. Having to filteri out the duplicates would certainly
> > complicate things a bit.
> Yes, Agree. Even I was thinking that the right way should be to:
> - add a drm_mode_equal_no_aspect function (like 
> drm_mode_equal_no_clock_no_stereo).

Or just drm_mode_match() with the right flags ;)

> - clear the aspect ratio information from the mode, when not asked for.
> - check the sorted connector->modes list for duplicates for this mode, 
> using above function.
>  - if mode exists, remove it from the list
>  - if not, keep it in the list

Hmm. Since the list should be sorted I guess this won't even have to
traverse the list mutliple times. We can just keep skipping modes as
long they match the last mode we've already decided to expose.

> 
> Sounds like a plan ?
> 
> - Shashank
> >>   
> >>return true;
> >>   }
> >> -- 
> >> 2.7.4

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [REBASE 3/5] drm: Expose modes with aspect ratio, only if requested

[Sharma, Shashank]

Regards

Shashank


On 11/21/2017 10:41 PM, Ville Syrjälä wrote:

On Fri, Nov 17, 2017 at 03:00:30PM +0530, Shashank Sharma wrote:

From: aknautiy 

We parse the EDID and add all the modes in the connector's
modelist. This adds CEA modes with aspect ratio information
too, regadless of if 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.

Cc: Ville Syrjala 
Cc: Shashank Sharma 
Cc: Jose Abreu 

Signed-off-by: aknautiy 
---
  drivers/gpu/drm/drm_connector.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 704fc89..a246bb5 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1285,6 +1285,13 @@ static bool drm_mode_expose_to_userspace(const struct 
drm_display_mode *mode,
 */
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.
+*/
+   if (!file_priv->aspect_ratio_allowed &&
+   mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE)
+   return false;

I don't think we can just blindly drop the modes. We would have to
expose them with the aspect ratio cleared. That could lead to
duplicates, but I'm thinking that shouldn't be a real problem for
userspace. Having to filteri out the duplicates would certainly
complicate things a bit.

Yes, Agree. Even I was thinking that the right way should be to:
- add a drm_mode_equal_no_aspect function (like 
drm_mode_equal_no_clock_no_stereo).

- clear the aspect ratio information from the mode, when not asked for.
- check the sorted connector->modes list for duplicates for this mode, 
using above function.

- if mode exists, remove it from the list
- if not, keep it in the list

Sounds like a plan ?

- Shashank
  
  	return true;

  }
--
2.7.4


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [REBASE 3/5] drm: Expose modes with aspect ratio, only if requested

[Ville Syrjälä]

On Fri, Nov 17, 2017 at 03:00:30PM +0530, Shashank Sharma wrote:
> From: aknautiy 
> 
> We parse the EDID and add all the modes in the connector's
> modelist. This adds CEA modes with aspect ratio information
> too, regadless of if 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.
> 
> Cc: Ville Syrjala 
> Cc: Shashank Sharma 
> Cc: Jose Abreu 
> 
> Signed-off-by: aknautiy 
> ---
>  drivers/gpu/drm/drm_connector.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 704fc89..a246bb5 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1285,6 +1285,13 @@ static bool drm_mode_expose_to_userspace(const struct 
> drm_display_mode *mode,
>*/
>   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.
> +  */
> + if (!file_priv->aspect_ratio_allowed &&
> + mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE)
> + return false;

I don't think we can just blindly drop the modes. We would have to
expose them with the aspect ratio cleared. That could lead to
duplicates, but I'm thinking that shouldn't be a real problem for
userspace. Having to filteri out the duplicates would certainly
complicate things a bit.

>  
>   return true;
>  }
> -- 
> 2.7.4

-- 
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[REBASE 3/5] drm: Expose modes with aspect ratio, only if requested

[Shashank Sharma]

From: aknautiy 

We parse the EDID and add all the modes in the connector's
modelist. This adds CEA modes with aspect ratio information
too, regadless of if 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.

Cc: Ville Syrjala 
Cc: Shashank Sharma 
Cc: Jose Abreu 

Signed-off-by: aknautiy 
---
 drivers/gpu/drm/drm_connector.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 704fc89..a246bb5 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1285,6 +1285,13 @@ static bool drm_mode_expose_to_userspace(const struct 
drm_display_mode *mode,
 */
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.
+*/
+   if (!file_priv->aspect_ratio_allowed &&
+   mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE)
+   return false;
 
return true;
 }
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel