On Fri, May 31, 2013 at 03:23:41PM +0300, ville.syrjala at linux.intel.com
wrote:
> From: Ville Syrj?l? <ville.syrjala at linux.intel.com>
>
> 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 at linux.intel.com>
> ---
> 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 at ffwll.ch>
>
> 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
