Re: [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux