All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list
@ 2013-05-31 12:23 ville.syrjala
  2013-05-31 15:43 ` Daniel Vetter
  2013-06-18 10:10 ` Andy Furniss
  0 siblings, 2 replies; 10+ messages in thread
From: ville.syrjala @ 2013-05-31 12:23 UTC (permalink / raw)
  To: dri-devel

From: Ville Syrjälä <ville.syrjala@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@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.

 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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list
  2013-05-31 12:23 [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list ville.syrjala
@ 2013-05-31 15:43 ` Daniel Vetter
  2013-05-31 18:56   ` Andy Furniss
  2013-06-18 10:10 ` Andy Furniss
  1 sibling, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2013-05-31 15:43 UTC (permalink / raw)
  To: ville.syrjala; +Cc: dri-devel

On Fri, May 31, 2013 at 03:23:41PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@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@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@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@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list
  2013-05-31 15:43 ` Daniel Vetter
@ 2013-05-31 18:56   ` Andy Furniss
  2013-05-31 19:46     ` Daniel Vetter
  0 siblings, 1 reply; 10+ messages in thread
From: Andy Furniss @ 2013-05-31 18:56 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

Daniel Vetter wrote:
> On Fri, May 31, 2013 at 03:23:41PM +0300, ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@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@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.

Apologies, for jumping in here, but assuming I haven't missed anything 
you've done already: do you have any plans for supporting CEA interlaced 
modes?

That's assuming they actually need any more support - I only know that 
they are slightly wrong for me testing with radeon + my TV - are they 
tested and known to work / not work on intel hw?

The symptom I get is not instantly obvious - the second half of the 
bottom line gets rendered and repeated as the top line.

For me the reason that proper support could in theory be useful, is that 
were there in future a way to sync up properly,  my TV will do a nice 
"free" deinterlace. Even on a fast quad core PC doing one as good on cpu 
is not possible for HD 50i.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list
  2013-05-31 18:56   ` Andy Furniss
@ 2013-05-31 19:46     ` Daniel Vetter
  2013-06-01  9:13       ` Andy Furniss
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2013-05-31 19:46 UTC (permalink / raw)
  To: Andy Furniss; +Cc: dri-devel

On Fri, May 31, 2013 at 8:56 PM, Andy Furniss <adf.lists@gmail.com> wrote:
> Daniel Vetter wrote:
>>
>> On Fri, May 31, 2013 at 03:23:41PM +0300, ville.syrjala@linux.intel.com
>> wrote:
>>>
>>> From: Ville Syrjälä <ville.syrjala@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@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.
>
>
> Apologies, for jumping in here, but assuming I haven't missed anything
> you've done already: do you have any plans for supporting CEA interlaced
> modes?
>
> That's assuming they actually need any more support - I only know that they
> are slightly wrong for me testing with radeon + my TV - are they tested and
> known to work / not work on intel hw?
>
> The symptom I get is not instantly obvious - the second half of the bottom
> line gets rendered and repeated as the top line.
>
> For me the reason that proper support could in theory be useful, is that
> were there in future a way to sync up properly,  my TV will do a nice "free"
> deinterlace. Even on a fast quad core PC doing one as good on cpu is not
> possible for HD 50i.

Interlaced modes are supported on all intel platforms (except i8xx, hw
can't do it there) and we have full support for CEA modes.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list
  2013-05-31 19:46     ` Daniel Vetter
@ 2013-06-01  9:13       ` Andy Furniss
  2013-06-23 17:53         ` Alex Deucher
  0 siblings, 1 reply; 10+ messages in thread
From: Andy Furniss @ 2013-06-01  9:13 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

Daniel Vetter wrote:

> Interlaced modes are supported on all intel platforms (except i8xx, hw
> can't do it there) and we have full support for CEA modes.
> -Daniel

OK, thanks, I guess it's an AMD driver issue rather than a more generic 
drm thing then.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list
  2013-05-31 12:23 [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list ville.syrjala
  2013-05-31 15:43 ` Daniel Vetter
@ 2013-06-18 10:10 ` Andy Furniss
  2013-06-18 10:30   ` Ville Syrjälä
  1 sibling, 1 reply; 10+ messages in thread
From: Andy Furniss @ 2013-06-18 10:10 UTC (permalink / raw)
  To: ville.syrjala; +Cc: dri-devel

ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@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@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.

I see this has gone in now, one thing I notice is that xorg/apps/xrandr 
only prints Hz to 1dp so you can't see which mode is which for the 24p 
and 30i cases.

Maybe someone reading has commit access for xorg?



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list
  2013-06-18 10:10 ` Andy Furniss
@ 2013-06-18 10:30   ` Ville Syrjälä
  2013-06-18 10:36     ` Andy Furniss
  0 siblings, 1 reply; 10+ messages in thread
From: Ville Syrjälä @ 2013-06-18 10:30 UTC (permalink / raw)
  To: Andy Furniss; +Cc: dri-devel

On Tue, Jun 18, 2013 at 11:10:30AM +0100, Andy Furniss wrote:
> ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@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@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.
> 
> I see this has gone in now, one thing I notice is that xorg/apps/xrandr 
> only prints Hz to 1dp so you can't see which mode is which for the 24p 
> and 30i cases.
> 
> Maybe someone reading has commit access for xorg?

Not sure if you noticed but I posted some relevant xrandr patches to
xorg-devel. Unfortunately I got no response, and I've been too lazy
to figure out who I need to pester.

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list
  2013-06-18 10:30   ` Ville Syrjälä
@ 2013-06-18 10:36     ` Andy Furniss
  0 siblings, 0 replies; 10+ messages in thread
From: Andy Furniss @ 2013-06-18 10:36 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: dri-devel

Ville Syrjälä wrote:
> On Tue, Jun 18, 2013 at 11:10:30AM +0100, Andy Furniss wrote:
>> ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@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@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.
>>
>> I see this has gone in now, one thing I notice is that xorg/apps/xrandr
>> only prints Hz to 1dp so you can't see which mode is which for the 24p
>> and 30i cases.
>>
>> Maybe someone reading has commit access for xorg?
>
> Not sure if you noticed but I posted some relevant xrandr patches to
> xorg-devel. Unfortunately I got no response, and I've been too lazy
> to figure out who I need to pester.
>

Ahh, sorry I didn't see those, I just has a look at the current code to 
check it was still the same.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list
  2013-06-01  9:13       ` Andy Furniss
@ 2013-06-23 17:53         ` Alex Deucher
  2013-06-23 20:01           ` Andy Furniss
  0 siblings, 1 reply; 10+ messages in thread
From: Alex Deucher @ 2013-06-23 17:53 UTC (permalink / raw)
  To: Andy Furniss; +Cc: dri-devel

On Sat, Jun 1, 2013 at 5:13 AM, Andy Furniss <adf.lists@gmail.com> wrote:
> Daniel Vetter wrote:
>
>> Interlaced modes are supported on all intel platforms (except i8xx, hw
>> can't do it there) and we have full support for CEA modes.
>> -Daniel
>
>
> OK, thanks, I guess it's an AMD driver issue rather than a more generic drm
> thing then.

You might try removing this chunk of code in radeon_atom_mode_fixup()
in atombios_encoders.c

        /* hw bug */
        if ((mode->flags & DRM_MODE_FLAG_INTERLACE)
            && (mode->crtc_vsync_start < (mode->crtc_vdisplay + 2)))
                adjusted_mode->crtc_vsync_start =
adjusted_mode->crtc_vdisplay + 2;


Alex

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list
  2013-06-23 17:53         ` Alex Deucher
@ 2013-06-23 20:01           ` Andy Furniss
  0 siblings, 0 replies; 10+ messages in thread
From: Andy Furniss @ 2013-06-23 20:01 UTC (permalink / raw)
  To: Alex Deucher; +Cc: dri-devel

Alex Deucher wrote:
> On Sat, Jun 1, 2013 at 5:13 AM, Andy Furniss <adf.lists@gmail.com> wrote:
>> Daniel Vetter wrote:
>>
>>> Interlaced modes are supported on all intel platforms (except i8xx, hw
>>> can't do it there) and we have full support for CEA modes.
>>> -Daniel
>>
>>
>> OK, thanks, I guess it's an AMD driver issue rather than a more generic drm
>> thing then.
>
> You might try removing this chunk of code in radeon_atom_mode_fixup()
> in atombios_encoders.c
>
>          /* hw bug */
>          if ((mode->flags & DRM_MODE_FLAG_INTERLACE)
>              && (mode->crtc_vsync_start < (mode->crtc_vdisplay + 2)))
>                  adjusted_mode->crtc_vsync_start =
> adjusted_mode->crtc_vdisplay + 2;

Thanks, but it doesn't make any difference.

To slightly correct what I said earlier when describing what I see -

at 1920x1080i the right quarter of the bottom line is rendering repeated 
x4 as the top line.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-06-23 20:01 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-31 12:23 [PATCH] drm/edid: Add both 60Hz and 59.94Hz CEA modes to connector's mode list ville.syrjala
2013-05-31 15:43 ` Daniel Vetter
2013-05-31 18:56   ` Andy Furniss
2013-05-31 19:46     ` Daniel Vetter
2013-06-01  9:13       ` Andy Furniss
2013-06-23 17:53         ` Alex Deucher
2013-06-23 20:01           ` Andy Furniss
2013-06-18 10:10 ` Andy Furniss
2013-06-18 10:30   ` Ville Syrjälä
2013-06-18 10:36     ` Andy Furniss

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.