diff mbox

Wrong vsync offset calculation in drm_edid.c

Message ID 5138B520.40903@lxco.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Blum March 7, 2013, 3:41 p.m. UTC
This patch is a bug fix for the file drm_edid.c of the kernel 3.8.

The vsync offset is calculated wrong from the EDID set because of the 
wrong shift direction.
We could measure the bad old setting and also the good new setting after 
the bug fix.


Signed-off-by: Peter Blum <Peter.Blum@lxco.com>


-       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 */

Comments

Paul Menzel March 8, 2013, 9:05 a.m. UTC | #1
Dear Peter,


thank you for your patch. It has some formal issues though.


Am Donnerstag, den 07.03.2013, 16:41 +0100 schrieb Peter Blum:


1. Your subject line does not contain the tag [PATCH].
2. You should CC the maintainers.

> This patch is a bug fix for the file drm_edid.c of the kernel 3.8.

Is that bug also present in Linux 3.9 (Linus’ master branch), which is
the current development branch. Patches have to be submitted against
that branch.

> The vsync offset is calculated wrong from the EDID set because of the

s,wrong,incorrectly,

> wrong shift direction.
> We could measure the bad old setting and also the good new setting after 
> the bug fix.
> 
> 
> Signed-off-by: Peter Blum <Peter.Blum@lxco.com>
> 
> 
> diff -Naur a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> --- a/drivers/gpu/drm/drm_edid.c        2013-02-19 00:58:34.000000000 +0100
> +++ b/drivers/gpu/drm/drm_edid.c        2013-03-07 12:04:24.527609783 +0100
> @@ -893,7 +893,7 @@
>          unsigned vblank = (pt->vactive_vblank_hi & 0xf) << 8 | 
> pt->vblank_lo;

3. Your MUA Thunderbird mangled the patch by automatically breaking
lines. You should disable that or use `git send-email`.

>          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;

Not sure where it is determined how to shift that. But if it works for
you, good.

>          unsigned vsync_pulse_width = 
> (pt->hsync_vsync_offset_pulse_width_hi & 0x3) << 4 | 
> (pt->vsync_offset_pulse_width_lo & 0xf);
> 
>          /* ignore tiny modes */

Please send a [PATCH v2] with a comment, what changed in v2 after `---`
right before the diff line.

    $ git commit --amend # to edit commit message
    $ git format-patch -1 --subject-prefix="PATCH v2"


Thanks,

Paul
diff mbox

Patch

diff -Naur a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
--- a/drivers/gpu/drm/drm_edid.c        2013-02-19 00:58:34.000000000 +0100
+++ b/drivers/gpu/drm/drm_edid.c        2013-03-07 12:04:24.527609783 +0100
@@ -893,7 +893,7 @@ 
         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;