Wrong vsync offset calculation in drm_edid.c

[Paul Menzel]

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 
> 
> 
> diff -Naur a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> --- a/drivers/gpu/drm/drm_edid.c2013-02-19 00:58:34.0 +0100
> +++ b/drivers/gpu/drm/drm_edid.c2013-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
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: 

Re: Wrong vsync offset calculation in drm_edid.c

[Paul Menzel]

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 
> 
> 
> diff -Naur a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> --- a/drivers/gpu/drm/drm_edid.c2013-02-19 00:58:34.0 +0100
> +++ b/drivers/gpu/drm/drm_edid.c2013-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


signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Wrong vsync offset calculation in drm_edid.c

[Peter Blum]

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 


diff -Naur a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
--- a/drivers/gpu/drm/drm_edid.c2013-02-19 00:58:34.0 +0100
+++ b/drivers/gpu/drm/drm_edid.c2013-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;
-   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 */

Wrong vsync offset calculation in drm_edid.c

[Peter Blum]

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 


diff -Naur a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
--- a/drivers/gpu/drm/drm_edid.c2013-02-19 00:58:34.0 +0100
+++ b/drivers/gpu/drm/drm_edid.c2013-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;
-   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 */




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