On Fri, May 31, 2013 at 03:23:41PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Having both modes can be beneficial for video playback cases. If you can > match the video framerate exactly, and the audio and video clocks come > from the same source, you should be able to avoid dropped/repeated > frames without expensive operations such as resampling the audio to > match video output rate. > > Rather than add both variants based on the CEA extension short video > descriptors in do_cea_modes(), add only one variant there. Once all > the EDID has been fully probed, do a loop over the entire probed mode > list, during which we add the other variants for all modes that match > CEA modes. This allows us to match modes that didn't come via the CEA > short video descriptors. For example one Samsung TV here doesn't have > the 640x480-60 mode as a SVD, but instead it's specified via a detailed > timing descriptor. > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > A few people requested this. Originally I was a bit opposed to it, but > when I thought about it a bit more I figured if the audio and video > clocks come from the same source (or happen to be close enough w/o > significant drift), this could provide a better A/V sync w/o resampling > tricks. Yeah, I think this should be useful for a bunch of people. I've recently chatted with a few xbmc folks on #irc and one thing they've requested is mode fine-tuning. For DP we should have plenty of precision, but for HDMI we'd need to (slightly) frob the vtotal ever so often to compensate. With some runtime-tuning a la npt we should have perfect a/v sync without any audio resampling. Anyway, on the patch: Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > drivers/gpu/drm/drm_edid.c | 102 ++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 88 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 9e62bbe..e8d17ee 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -2321,6 +2321,31 @@ u8 *drm_find_cea_extension(struct edid *edid) > } > EXPORT_SYMBOL(drm_find_cea_extension); > > +/* > + * Calculate the alternate clock for the CEA mode > + * (60Hz vs. 59.94Hz etc.) > + */ > +static unsigned int > +cea_mode_alternate_clock(const struct drm_display_mode *cea_mode) > +{ > + unsigned int clock = cea_mode->clock; > + > + if (cea_mode->vrefresh % 6 != 0) > + return clock; > + > + /* > + * edid_cea_modes contains the 59.94Hz > + * variant for 240 and 480 line modes, > + * and the 60Hz variant otherwise. > + */ > + if (cea_mode->vdisplay == 240 || cea_mode->vdisplay == 480) > + clock = clock * 1001 / 1000; > + else > + clock = DIV_ROUND_UP(clock * 1000, 1001); > + > + return clock; > +} > + > /** > * drm_match_cea_mode - look for a CEA mode matching given mode > * @to_match: display mode > @@ -2339,21 +2364,9 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) > const struct drm_display_mode *cea_mode = &edid_cea_modes[mode]; > unsigned int clock1, clock2; > > - clock1 = clock2 = cea_mode->clock; > - > /* Check both 60Hz and 59.94Hz */ > - if (cea_mode->vrefresh % 6 == 0) { > - /* > - * edid_cea_modes contains the 59.94Hz > - * variant for 240 and 480 line modes, > - * and the 60Hz variant otherwise. > - */ > - if (cea_mode->vdisplay == 240 || > - cea_mode->vdisplay == 480) > - clock1 = clock1 * 1001 / 1000; > - else > - clock2 = DIV_ROUND_UP(clock2 * 1000, 1001); > - } > + clock1 = cea_mode->clock; > + clock2 = cea_mode_alternate_clock(cea_mode); > > if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || > KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && > @@ -2364,6 +2377,66 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) > } > EXPORT_SYMBOL(drm_match_cea_mode); > > +static int > +add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) > +{ > + struct drm_device *dev = connector->dev; > + struct drm_display_mode *mode, *tmp; > + LIST_HEAD(list); > + int modes = 0; > + > + /* Don't add CEA modes if the CEA extension block is missing */ > + if (!drm_find_cea_extension(edid)) > + return 0; > + > + /* > + * Go through all probed modes and create a new mode > + * with the alternate clock for certain CEA modes. > + */ > + list_for_each_entry(mode, &connector->probed_modes, head) { > + const struct drm_display_mode *cea_mode; > + struct drm_display_mode *newmode; > + u8 cea_mode_idx = drm_match_cea_mode(mode) - 1; > + unsigned int clock1, clock2; > + > + if (cea_mode_idx >= ARRAY_SIZE(edid_cea_modes)) > + continue; > + > + cea_mode = &edid_cea_modes[cea_mode_idx]; > + > + clock1 = cea_mode->clock; > + clock2 = cea_mode_alternate_clock(cea_mode); > + > + if (clock1 == clock2) > + continue; > + > + if (mode->clock != clock1 && mode->clock != clock2) > + continue; > + > + newmode = drm_mode_duplicate(dev, cea_mode); > + if (!newmode) > + continue; > + > + /* > + * The current mode could be either variant. Make > + * sure to pick the "other" clock for the new mode. > + */ > + if (mode->clock != clock1) > + newmode->clock = clock1; > + else > + newmode->clock = clock2; > + > + list_add_tail(&newmode->head, &list); > + } > + > + list_for_each_entry_safe(mode, tmp, &list, head) { > + list_del(&mode->head); > + drm_mode_probed_add(connector, mode); > + modes++; > + } > + > + return modes; > +} > > static int > do_cea_modes (struct drm_connector *connector, u8 *db, u8 len) > @@ -2946,6 +3019,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) > if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) > num_modes += add_inferred_modes(connector, edid); > num_modes += add_cea_modes(connector, edid); > + num_modes += add_alternate_cea_modes(connector, edid); > > if (quirks & (EDID_QUIRK_PREFER_LARGE_60 | EDID_QUIRK_PREFER_LARGE_75)) > edid_fixup_preferred(connector, quirks); > -- > 1.8.1.5 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel