subject:"\[PATCH v3 3\/8\] drm\/edid\: Fix cea mode aspect ratio handling"

Re: [PATCH v3 3/8] drm/edid: Fix cea mode aspect ratio handling

2018-01-18 Thread Sharma, Shashank


Regards

Shashank


On 1/17/2018 8:59 PM, Ville Syrjälä wrote:

On Wed, Jan 17, 2018 at 02:35:40PM +0530, Sharma, Shashank wrote:

Regards

Shashank


On 1/12/2018 11:51 AM, Nautiyal, Ankit K wrote:

From: Ville Syrjälä 

commit 6dffd431e229 ("drm: Add aspect ratio parsing in DRM layer")
cause us to not send out any VICs in the AVI infoframes. That commit
was since reverted, but if and when we add aspect ratio handing back
we need to be more careful.

Let's handle this by considering the aspect ratio as a requirement
for cea mode matching only if the passed in mode actually has a
non-zero aspect ratio field. This will keep userspace that doesn't
provide an aspect ratio working as before by matching it to the
first otherwise equal cea mode. And once userspace starts to
provide the aspect ratio it will be considerd a hard requirement
for the match.

Also change the hdmi mode matching to use drm_mode_match() for
consistency, but we don't match on aspect ratio there since the
spec doesn't list a specific aspect ratio for those modes.

Cc: Shashank Sharma 
Cc: "Lin, Jia" 
Cc: Akashdeep Sharma 
Cc: Jim Bride 
Cc: Jose Abreu 
Cc: Daniel Vetter 
Cc: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
   drivers/gpu/drm/drm_edid.c | 18 ++
   1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 1818a71..3d57c41 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2908,11 +2908,15 @@ cea_mode_alternate_timings(u8 vic, struct 
drm_display_mode *mode)
   static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode 
*to_match,
 unsigned int clock_tolerance)
   {
+   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
DRM_MODE_MATCH_FLAGS;
u8 vic;
   
   	if (!to_match->clock)

return 0;
   
+	if (to_match->picture_aspect_ratio)

+   match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
struct drm_display_mode cea_mode = edid_cea_modes[vic];
unsigned int clock1, clock2;
@@ -2926,7 +2930,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct 
drm_display_mode *to_m
continue;
   
   		do {

-   if (drm_mode_equal_no_clocks_no_stereo(to_match, 
_mode))
+   if (drm_mode_match(to_match, _mode, match_flags))
return vic;
} while (cea_mode_alternate_timings(vic, _mode));
}
@@ -2943,11 +2947,15 @@ static u8 drm_match_cea_mode_clock_tolerance(const 
struct drm_display_mode *to_m
*/
   u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
   {
+   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
DRM_MODE_MATCH_FLAGS;
u8 vic;
   
   	if (!to_match->clock)

return 0;
   
+	if (to_match->picture_aspect_ratio)

+   match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+

How about stereo flags ? CEA modes can be 3d and in that case we might
want to add DRM_MODE_MATCH_3D_FLAGS here

There are no separate VICs for stereo vs. no stereo. So ignoring the
stereo flags here seems like the correct thing to do.

Agree,
Reviewed-by: Shashank Sharma 

- Shashank

for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
struct drm_display_mode cea_mode = edid_cea_modes[vic];
unsigned int clock1, clock2;
@@ -2961,7 +2969,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode 
*to_match)
continue;
   
   		do {

-   if (drm_mode_equal_no_clocks_no_stereo(to_match, 
_mode))
+   if (drm_mode_match(to_match, _mode, match_flags))
return vic;
} while (cea_mode_alternate_timings(vic, _mode));
}
@@ -3008,6 +3016,7 @@ hdmi_mode_alternate_clock(const struct drm_display_mode 
*hdmi_mode)
   static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode 
*to_match,
  unsigned int clock_tolerance)
   {
+   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
DRM_MODE_MATCH_FLAGS;
u8 vic;
   
   	if (!to_match->clock)

@@ -3025,7 +3034,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const 
struct drm_display_mode *to_
abs(to_match->clock - clock2) > clock_tolerance)
continue;
   
-		if (drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode))

+   if (drm_mode_match(to_match, hdmi_mode, match_flags))
return vic;
}
   
@@

Re: [PATCH v3 3/8] drm/edid: Fix cea mode aspect ratio handling

2018-01-17 Thread Ville Syrjälä

On Wed, Jan 17, 2018 at 02:35:40PM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 1/12/2018 11:51 AM, Nautiyal, Ankit K wrote:
> > From: Ville Syrjälä 
> >
> > commit 6dffd431e229 ("drm: Add aspect ratio parsing in DRM layer")
> > cause us to not send out any VICs in the AVI infoframes. That commit
> > was since reverted, but if and when we add aspect ratio handing back
> > we need to be more careful.
> >
> > Let's handle this by considering the aspect ratio as a requirement
> > for cea mode matching only if the passed in mode actually has a
> > non-zero aspect ratio field. This will keep userspace that doesn't
> > provide an aspect ratio working as before by matching it to the
> > first otherwise equal cea mode. And once userspace starts to
> > provide the aspect ratio it will be considerd a hard requirement
> > for the match.
> >
> > Also change the hdmi mode matching to use drm_mode_match() for
> > consistency, but we don't match on aspect ratio there since the
> > spec doesn't list a specific aspect ratio for those modes.
> >
> > Cc: Shashank Sharma 
> > Cc: "Lin, Jia" 
> > Cc: Akashdeep Sharma 
> > Cc: Jim Bride 
> > Cc: Jose Abreu 
> > Cc: Daniel Vetter 
> > Cc: Emil Velikov 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >   drivers/gpu/drm/drm_edid.c | 18 ++
> >   1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 1818a71..3d57c41 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -2908,11 +2908,15 @@ cea_mode_alternate_timings(u8 vic, struct 
> > drm_display_mode *mode)
> >   static u8 drm_match_cea_mode_clock_tolerance(const struct 
> > drm_display_mode *to_match,
> >  unsigned int clock_tolerance)
> >   {
> > +   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
> > DRM_MODE_MATCH_FLAGS;
> > u8 vic;
> >   
> > if (!to_match->clock)
> > return 0;
> >   
> > +   if (to_match->picture_aspect_ratio)
> > +   match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
> > +
> > for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> > struct drm_display_mode cea_mode = edid_cea_modes[vic];
> > unsigned int clock1, clock2;
> > @@ -2926,7 +2930,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const 
> > struct drm_display_mode *to_m
> > continue;
> >   
> > do {
> > -   if (drm_mode_equal_no_clocks_no_stereo(to_match, 
> > _mode))
> > +   if (drm_mode_match(to_match, _mode, match_flags))
> > return vic;
> > } while (cea_mode_alternate_timings(vic, _mode));
> > }
> > @@ -2943,11 +2947,15 @@ static u8 drm_match_cea_mode_clock_tolerance(const 
> > struct drm_display_mode *to_m
> >*/
> >   u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
> >   {
> > +   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
> > DRM_MODE_MATCH_FLAGS;
> > u8 vic;
> >   
> > if (!to_match->clock)
> > return 0;
> >   
> > +   if (to_match->picture_aspect_ratio)
> > +   match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
> > +
> How about stereo flags ? CEA modes can be 3d and in that case we might 
> want to add DRM_MODE_MATCH_3D_FLAGS here

There are no separate VICs for stereo vs. no stereo. So ignoring the
stereo flags here seems like the correct thing to do.

> 
> - Shashank
> > for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
> > struct drm_display_mode cea_mode = edid_cea_modes[vic];
> > unsigned int clock1, clock2;
> > @@ -2961,7 +2969,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode 
> > *to_match)
> > continue;
> >   
> > do {
> > -   if (drm_mode_equal_no_clocks_no_stereo(to_match, 
> > _mode))
> > +   if (drm_mode_match(to_match, _mode, match_flags))
> > return vic;
> > } while (cea_mode_alternate_timings(vic, _mode));
> > }
> > @@ -3008,6 +3016,7 @@ hdmi_mode_alternate_clock(const struct 
> > drm_display_mode *hdmi_mode)
> >   static u8 drm_match_hdmi_mode_clock_tolerance(const struct 
> > drm_display_mode *to_match,
> >   unsigned int clock_tolerance)
> >   {
> > +   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
> > DRM_MODE_MATCH_FLAGS;
> > u8 vic;
> >   
> > if (!to_match->clock)
> > @@ -3025,7 +3034,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const 
> > struct drm_display_mode *to_
> > abs(to_match->clock - clock2) > clock_tolerance)
> > continue;
> >

Re: [PATCH v3 3/8] drm/edid: Fix cea mode aspect ratio handling

2018-01-17 Thread Sharma, Shashank


Regards

Shashank


On 1/12/2018 11:51 AM, Nautiyal, Ankit K wrote:

From: Ville Syrjälä 

commit 6dffd431e229 ("drm: Add aspect ratio parsing in DRM layer")
cause us to not send out any VICs in the AVI infoframes. That commit
was since reverted, but if and when we add aspect ratio handing back
we need to be more careful.

Let's handle this by considering the aspect ratio as a requirement
for cea mode matching only if the passed in mode actually has a
non-zero aspect ratio field. This will keep userspace that doesn't
provide an aspect ratio working as before by matching it to the
first otherwise equal cea mode. And once userspace starts to
provide the aspect ratio it will be considerd a hard requirement
for the match.

Also change the hdmi mode matching to use drm_mode_match() for
consistency, but we don't match on aspect ratio there since the
spec doesn't list a specific aspect ratio for those modes.

Cc: Shashank Sharma 
Cc: "Lin, Jia" 
Cc: Akashdeep Sharma 
Cc: Jim Bride 
Cc: Jose Abreu 
Cc: Daniel Vetter 
Cc: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
  drivers/gpu/drm/drm_edid.c | 18 ++
  1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 1818a71..3d57c41 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2908,11 +2908,15 @@ cea_mode_alternate_timings(u8 vic, struct 
drm_display_mode *mode)
  static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode 
*to_match,
 unsigned int clock_tolerance)
  {
+   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
DRM_MODE_MATCH_FLAGS;
u8 vic;
  
  	if (!to_match->clock)

return 0;
  
+	if (to_match->picture_aspect_ratio)

+   match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
struct drm_display_mode cea_mode = edid_cea_modes[vic];
unsigned int clock1, clock2;
@@ -2926,7 +2930,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct 
drm_display_mode *to_m
continue;
  
  		do {

-   if (drm_mode_equal_no_clocks_no_stereo(to_match, 
_mode))
+   if (drm_mode_match(to_match, _mode, match_flags))
return vic;
} while (cea_mode_alternate_timings(vic, _mode));
}
@@ -2943,11 +2947,15 @@ static u8 drm_match_cea_mode_clock_tolerance(const 
struct drm_display_mode *to_m
   */
  u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
  {
+   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
DRM_MODE_MATCH_FLAGS;
u8 vic;
  
  	if (!to_match->clock)

return 0;
  
+	if (to_match->picture_aspect_ratio)

+   match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
How about stereo flags ? CEA modes can be 3d and in that case we might 
want to add DRM_MODE_MATCH_3D_FLAGS here


- Shashank

for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
struct drm_display_mode cea_mode = edid_cea_modes[vic];
unsigned int clock1, clock2;
@@ -2961,7 +2969,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode 
*to_match)
continue;
  
  		do {

-   if (drm_mode_equal_no_clocks_no_stereo(to_match, 
_mode))
+   if (drm_mode_match(to_match, _mode, match_flags))
return vic;
} while (cea_mode_alternate_timings(vic, _mode));
}
@@ -3008,6 +3016,7 @@ hdmi_mode_alternate_clock(const struct drm_display_mode 
*hdmi_mode)
  static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode 
*to_match,
  unsigned int clock_tolerance)
  {
+   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
DRM_MODE_MATCH_FLAGS;
u8 vic;
  
  	if (!to_match->clock)

@@ -3025,7 +3034,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const 
struct drm_display_mode *to_
abs(to_match->clock - clock2) > clock_tolerance)
continue;
  
-		if (drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode))

+   if (drm_mode_match(to_match, hdmi_mode, match_flags))
return vic;
}
  
@@ -3042,6 +3051,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_

   */
  static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
  {
+   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
DRM_MODE_MATCH_FLAGS;
u8 vic;
  
  	if (!to_match->clock)

@@ -3057,7 +3067,7 @@ static

[PATCH v3 3/8] drm/edid: Fix cea mode aspect ratio handling

2018-01-11 Thread Nautiyal, Ankit K

From: Ville Syrjälä 

commit 6dffd431e229 ("drm: Add aspect ratio parsing in DRM layer")
cause us to not send out any VICs in the AVI infoframes. That commit
was since reverted, but if and when we add aspect ratio handing back
we need to be more careful.

Let's handle this by considering the aspect ratio as a requirement
for cea mode matching only if the passed in mode actually has a
non-zero aspect ratio field. This will keep userspace that doesn't
provide an aspect ratio working as before by matching it to the
first otherwise equal cea mode. And once userspace starts to
provide the aspect ratio it will be considerd a hard requirement
for the match.

Also change the hdmi mode matching to use drm_mode_match() for
consistency, but we don't match on aspect ratio there since the
spec doesn't list a specific aspect ratio for those modes.

Cc: Shashank Sharma 
Cc: "Lin, Jia" 
Cc: Akashdeep Sharma 
Cc: Jim Bride 
Cc: Jose Abreu 
Cc: Daniel Vetter 
Cc: Emil Velikov 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_edid.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 1818a71..3d57c41 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2908,11 +2908,15 @@ cea_mode_alternate_timings(u8 vic, struct 
drm_display_mode *mode)
 static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode 
*to_match,
 unsigned int clock_tolerance)
 {
+   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
DRM_MODE_MATCH_FLAGS;
u8 vic;
 
if (!to_match->clock)
return 0;
 
+   if (to_match->picture_aspect_ratio)
+   match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
struct drm_display_mode cea_mode = edid_cea_modes[vic];
unsigned int clock1, clock2;
@@ -2926,7 +2930,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct 
drm_display_mode *to_m
continue;
 
do {
-   if (drm_mode_equal_no_clocks_no_stereo(to_match, 
_mode))
+   if (drm_mode_match(to_match, _mode, match_flags))
return vic;
} while (cea_mode_alternate_timings(vic, _mode));
}
@@ -2943,11 +2947,15 @@ static u8 drm_match_cea_mode_clock_tolerance(const 
struct drm_display_mode *to_m
  */
 u8 drm_match_cea_mode(const struct drm_display_mode *to_match)
 {
+   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
DRM_MODE_MATCH_FLAGS;
u8 vic;
 
if (!to_match->clock)
return 0;
 
+   if (to_match->picture_aspect_ratio)
+   match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) {
struct drm_display_mode cea_mode = edid_cea_modes[vic];
unsigned int clock1, clock2;
@@ -2961,7 +2969,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode 
*to_match)
continue;
 
do {
-   if (drm_mode_equal_no_clocks_no_stereo(to_match, 
_mode))
+   if (drm_mode_match(to_match, _mode, match_flags))
return vic;
} while (cea_mode_alternate_timings(vic, _mode));
}
@@ -3008,6 +3016,7 @@ hdmi_mode_alternate_clock(const struct drm_display_mode 
*hdmi_mode)
 static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode 
*to_match,
  unsigned int clock_tolerance)
 {
+   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
DRM_MODE_MATCH_FLAGS;
u8 vic;
 
if (!to_match->clock)
@@ -3025,7 +3034,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const 
struct drm_display_mode *to_
abs(to_match->clock - clock2) > clock_tolerance)
continue;
 
-   if (drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode))
+   if (drm_mode_match(to_match, hdmi_mode, match_flags))
return vic;
}
 
@@ -3042,6 +3051,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const 
struct drm_display_mode *to_
  */
 static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match)
 {
+   unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | 
DRM_MODE_MATCH_FLAGS;
u8 vic;
 
if (!to_match->clock)
@@ -3057,7 +3067,7 @@ static u8 drm_match_hdmi_mode(const struct 
drm_display_mode *to_match)
 
if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) ||

Re: [PATCH v3 3/8] drm/edid: Fix cea mode aspect ratio handling

Re: [PATCH v3 3/8] drm/edid: Fix cea mode aspect ratio handling

Re: [PATCH v3 3/8] drm/edid: Fix cea mode aspect ratio handling

[PATCH v3 3/8] drm/edid: Fix cea mode aspect ratio handling

4 matches

Site Navigation

Mail list logo

Footer information