| Message ID | 1515738096-16892-4-git-send-email-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Regards Shashank On 1/12/2018 11:51 AM, Nautiyal, Ankit K wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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 <shashank.sharma@intel.com> > Cc: "Lin, Jia" <lin.a.jia@intel.com> > Cc: Akashdeep Sharma <akashdeep.sharma@intel.com> > Cc: Jim Bride <jim.bride@linux.intel.com> > Cc: Jose Abreu <Jose.Abreu@synopsys.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Emil Velikov <emil.l.velikov@gmail.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > 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, &cea_mode)) > + if (drm_mode_match(to_match, &cea_mode, match_flags)) > return vic; > } while (cea_mode_alternate_timings(vic, &cea_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, &cea_mode)) > + if (drm_mode_match(to_match, &cea_mode, match_flags)) > return vic; > } while (cea_mode_alternate_timings(vic, &cea_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) || > KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && > - drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) > + drm_mode_match(to_match, hdmi_mode, match_flags)) > return vic; > } > return 0;
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ä <ville.syrjala@linux.intel.com> > > > > 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 <shashank.sharma@intel.com> > > Cc: "Lin, Jia" <lin.a.jia@intel.com> > > Cc: Akashdeep Sharma <akashdeep.sharma@intel.com> > > Cc: Jim Bride <jim.bride@linux.intel.com> > > Cc: Jose Abreu <Jose.Abreu@synopsys.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Emil Velikov <emil.l.velikov@gmail.com> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > 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, &cea_mode)) > > + if (drm_mode_match(to_match, &cea_mode, match_flags)) > > return vic; > > } while (cea_mode_alternate_timings(vic, &cea_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, &cea_mode)) > > + if (drm_mode_match(to_match, &cea_mode, match_flags)) > > return vic; > > } while (cea_mode_alternate_timings(vic, &cea_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) || > > KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && > > - drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) > > + drm_mode_match(to_match, hdmi_mode, match_flags)) > > return vic; > > } > > return 0;
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ä <ville.syrjala@linux.intel.com> >>> >>> 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 <shashank.sharma@intel.com> >>> Cc: "Lin, Jia" <lin.a.jia@intel.com> >>> Cc: Akashdeep Sharma <akashdeep.sharma@intel.com> >>> Cc: Jim Bride <jim.bride@linux.intel.com> >>> Cc: Jose Abreu <Jose.Abreu@synopsys.com> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>> Cc: Emil Velikov <emil.l.velikov@gmail.com> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >>> --- >>> 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, &cea_mode)) >>> + if (drm_mode_match(to_match, &cea_mode, match_flags)) >>> return vic; >>> } while (cea_mode_alternate_timings(vic, &cea_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.sharma@intel.com> >> - 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, &cea_mode)) >>> + if (drm_mode_match(to_match, &cea_mode, match_flags)) >>> return vic; >>> } while (cea_mode_alternate_timings(vic, &cea_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) || >>> KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && >>> - drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) >>> + drm_mode_match(to_match, hdmi_mode, match_flags)) >>> return vic; >>> } >>> return 0;
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, &cea_mode)) + if (drm_mode_match(to_match, &cea_mode, match_flags)) return vic; } while (cea_mode_alternate_timings(vic, &cea_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, &cea_mode)) + if (drm_mode_match(to_match, &cea_mode, match_flags)) return vic; } while (cea_mode_alternate_timings(vic, &cea_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) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && - drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) + drm_mode_match(to_match, hdmi_mode, match_flags)) return vic; } return 0;