On Mon, Nov 16, 2015 at 09:05:12PM +0200, ville.syrjala at linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala at linux.intel.com> > > Rather than using drm_match_cea_mode() to see if the EDID detailed > timings are supposed to represent one of the CEA/HDMI modes, add a > special version of that function that takes in an explicit clock > tolerance value (in kHz). When looking at the detailed timings specify > the tolerance as 5kHz due to the 10kHz clock resolution limit inherent > in detailed timings. > > drm_match_cea_mode() uses the normal KHZ2PICOS() matching of clocks, > which only allows smaller errors for lower clocks (eg. for 25200 it > won't allow any error) and a bigger error for higher clocks (eg. for > 297000 it actually matches 296913-297000). So it doesn't really match > what we want for the fixup. Using the explicit +-5kHz is much better > for this use case. > > Not sure if we should change the normal mode matching to also use > something else besides KHZ2PICOS() since it allows a different > proportion of error depending on the clock. I believe VESA CVT > allows a maximum deviation of .5%, so using that for normal mode > matching might be a good idea?
Ping. Any thoughts from anyone? > > Cc: Adam Jackson <ajax at redhat.com> > Tested-by: nathan.d.ciobanu at linux.intel.com > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92217 > Fixes: fa3a7340eaa1 ("drm/edid: Fix up clock for CEA/HDMI modes specified via > detailed timings") > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com> > --- > drivers/gpu/drm/drm_edid.c | 62 > +++++++++++++++++++++++++++++++++++++++++++-- > drivers/gpu/drm/drm_modes.c | 19 +++++++++++++- > include/drm/drm_modes.h | 2 ++ > 3 files changed, 80 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index d5d2c03fd136..c214f1246cb4 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -2545,6 +2545,33 @@ cea_mode_alternate_clock(const struct drm_display_mode > *cea_mode) > return clock; > } > > +static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode > *to_match, > + unsigned int clock_tolerance) > +{ > + u8 mode; > + > + if (!to_match->clock) > + return 0; > + > + for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) { > + const struct drm_display_mode *cea_mode = &edid_cea_modes[mode]; > + unsigned int clock1, clock2; > + > + /* Check both 60Hz and 59.94Hz */ > + clock1 = cea_mode->clock; > + clock2 = cea_mode_alternate_clock(cea_mode); > + > + if (abs(to_match->clock - clock1) > clock_tolerance && > + abs(to_match->clock - clock2) > clock_tolerance) > + continue; > + > + if (drm_mode_equal_no_clocks(to_match, cea_mode)) > + return mode + 1; > + } > + > + return 0; > +} > + > /** > * drm_match_cea_mode - look for a CEA mode matching given mode > * @to_match: display mode > @@ -2609,6 +2636,33 @@ hdmi_mode_alternate_clock(const struct > drm_display_mode *hdmi_mode) > return cea_mode_alternate_clock(hdmi_mode); > } > > +static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode > *to_match, > + unsigned int clock_tolerance) > +{ > + u8 mode; > + > + if (!to_match->clock) > + return 0; > + > + for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) { > + const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode]; > + unsigned int clock1, clock2; > + > + /* Make sure to also match alternate clocks */ > + clock1 = hdmi_mode->clock; > + clock2 = hdmi_mode_alternate_clock(hdmi_mode); > + > + if (abs(to_match->clock - clock1) > clock_tolerance && > + abs(to_match->clock - clock2) > clock_tolerance) > + continue; > + > + if (drm_mode_equal_no_clocks(to_match, hdmi_mode)) > + return mode + 1; > + } > + > + return 0; > +} > + > /* > * drm_match_hdmi_mode - look for a HDMI mode matching given mode > * @to_match: display mode > @@ -3119,14 +3173,18 @@ static void fixup_detailed_cea_mode_clock(struct > drm_display_mode *mode) > u8 mode_idx; > const char *type; > > - mode_idx = drm_match_cea_mode(mode) - 1; > + /* > + * allow 5kHz clock difference either way to account for > + * the 10kHz clock resolution limit of detailed timings. > + */ > + mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1; > if (mode_idx < ARRAY_SIZE(edid_cea_modes)) { > type = "CEA"; > cea_mode = &edid_cea_modes[mode_idx]; > clock1 = cea_mode->clock; > clock2 = cea_mode_alternate_clock(cea_mode); > } else { > - mode_idx = drm_match_hdmi_mode(mode) - 1; > + mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1; > if (mode_idx < ARRAY_SIZE(edid_4k_modes)) { > type = "HDMI"; > cea_mode = &edid_4k_modes[mode_idx]; > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > index 9480464434cf..4d2a24121dc2 100644 > --- a/drivers/gpu/drm/drm_modes.c > +++ b/drivers/gpu/drm/drm_modes.c > @@ -917,13 +917,30 @@ bool drm_mode_equal(const struct drm_display_mode > *mode1, const struct drm_displ > } else if (mode1->clock != mode2->clock) > return false; > > + return drm_mode_equal_no_clocks(mode1, mode2); > +} > +EXPORT_SYMBOL(drm_mode_equal); > + > +/** > + * drm_mode_equal_no_clocks - test modes for equality > + * @mode1: first mode > + * @mode2: second mode > + * > + * Check to see if @mode1 and @mode2 are equivalent, but > + * don't check the pixel clocks. > + * > + * Returns: > + * True if the modes are equal, false otherwise. > + */ > +bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, const > struct drm_display_mode *mode2) > +{ > if ((mode1->flags & DRM_MODE_FLAG_3D_MASK) != > (mode2->flags & DRM_MODE_FLAG_3D_MASK)) > return false; > > return drm_mode_equal_no_clocks_no_stereo(mode1, mode2); > } > -EXPORT_SYMBOL(drm_mode_equal); > +EXPORT_SYMBOL(drm_mode_equal_no_clocks); > > /** > * drm_mode_equal_no_clocks_no_stereo - test modes for equality > diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h > index 08a8cac9e555..f9115aee43f4 100644 > --- a/include/drm/drm_modes.h > +++ b/include/drm/drm_modes.h > @@ -222,6 +222,8 @@ struct drm_display_mode *drm_mode_duplicate(struct > drm_device *dev, > const struct drm_display_mode > *mode); > bool drm_mode_equal(const struct drm_display_mode *mode1, > const struct drm_display_mode *mode2); > +bool drm_mode_equal_no_clocks(const struct drm_display_mode *mode1, > + const struct drm_display_mode *mode2); > bool drm_mode_equal_no_clocks_no_stereo(const struct drm_display_mode *mode1, > const struct drm_display_mode *mode2); > > -- > 2.4.10 -- Ville Syrjälä Intel OTC
