2013-03-23 14:45:52

by Torsten Duwe

Subject: [PATCH 0/2] KMS: fix EDID parsing of "detailed timing"

Hi all,

I'm describing a PAL CRT to my HTPC. Since it uses a 50Hz frame
rate, I had to use a "detailed timing" entry when I crafted an EDID
blob. User space tools properly reproduce the mode line I used
originally, but the kernel gets the vsync position and the frame
rate wrong. Fixes follow.

Torsten

2013-03-23 14:45:54

by Torsten Duwe

Subject: [PATCH 2/2] KMS: fix EDID detailed timing frame rate

When KMS has parsed an EDID "detailed timing", it leaves the frame
rate zeroed. Consecutive (debug-) output of that mode thus yields 0
for vsync. This simple fix also speeds up future invocations of
drm_mode_vrefresh(). While it is debatable whether this qualifies
as a -stable fix I'd apply it for consistency's sake;
drm_helper_probe_single_connector_modes() does the same thing already
for all probed modes.

Cc: [email protected]
Signed-off-by: Torsten Duwe <[email protected]>

--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -977,6 +977,7 @@ set_size:
}

mode->type = DRM_MODE_TYPE_DRIVER;
+ mode->vrefresh = drm_mode_vrefresh(mode);
drm_mode_set_name(mode);

return mode;

2013-03-23 14:45:54

by Torsten Duwe

Subject: [PATCH 1/2] KMS: fix EDID detailed timing vsync parsing

EDID spreads some values across multiple bytes; bit-fiddling is needed
to retrieve these. The current code to parse "detailed timings" has a
cut&paste error that results in a vsync offset of at most 15 lines
instead of 63. en.wikipedia.org/wiki/EDID "detailed timing" bytes
10+11 show why that needs to be a left shift.

Cc: [email protected]
Signed-off-by: Torsten Duwe <[email protected]>

--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -896,7 +896,7 @@ static struct drm_display_mode *drm_mode
unsigned vblank = (pt->vactive_vblank_hi & 0xf) << 8 | pt->vblank_lo;
unsigned hsync_offset = (pt->hsync_vsync_offset_pulse_width_hi & 0xc0) << 2 | pt->hsync_offset_lo;
unsigned hsync_pulse_width = (pt->hsync_vsync_offset_pulse_width_hi & 0x30) << 4 | pt->hsync_pulse_width_lo;
- unsigned vsync_offset = (pt->hsync_vsync_offset_pulse_width_hi & 0xc) >> 2 | pt->vsync_offset_pulse_width_lo >> 4;
+ unsigned vsync_offset = (pt->hsync_vsync_offset_pulse_width_hi & 0xc) << 2 | pt->vsync_offset_pulse_width_lo >> 4;
unsigned vsync_pulse_width = (pt->hsync_vsync_offset_pulse_width_hi & 0x3) << 4 | (pt->vsync_offset_pulse_width_lo & 0xf);

/* ignore tiny modes */

2013-03-23 17:47:15

by Linus Torvalds

Subject: Re: [PATCH 1/2] KMS: fix EDID detailed timing vsync parsing

On Sat, Mar 23, 2013 at 7:38 AM, Torsten Duwe <[email protected]> wrote:
>
> EDID spreads some values across multiple bytes; bit-fiddling is needed
> to retrieve these. The current code to parse "detailed timings" has a
> cut&paste error that results in a vsync offset of at most 15 lines
> instead of 63.

Yeah, it clearly just mixes the bits together with that bitwise rather
than actually spreading them out correctly.

Applied.

Linus

2013-03-23 17:50:19

by Linus Torvalds

Subject: Re: [PATCH 1/2] KMS: fix EDID detailed timing vsync parsing

How the heck did you get your *own* email address wrong in your mail
headers? "[email protected]" bounces, apparently your email address is
"[email protected]". Let's see if that bounces..

Linus

2013-03-23 19:45:15

by Torsten Duwe

Subject: Re: [PATCH 1/2] KMS: fix EDID detailed timing vsync parsing

On Sat, 23 Mar 2013, Linus Torvalds wrote:

> How the heck did you get your *own* email address wrong in your mail
> headers? "[email protected]" bounces,

Of course I had to goof up somehow :(
This is not my everyday mail setup, the configuration is suboptimal,
admittedly. Sorry for the inconvenience.

Torsten