Bug 677232 - [PATCH] Fix EDID macros H_SYNC_WIDTH and H_SYNC_OFFSET
Summary: [PATCH] Fix EDID macros H_SYNC_WIDTH and H_SYNC_OFFSET
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Fedora
Classification: Fedora
Component: kernel
Version: 15
Hardware: All
OS: Linux
unspecified
low
Target Milestone: ---
Assignee: Kernel Maintainer List
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-02-14 07:10 UTC by Martin Decky
Modified: 2011-08-03 13:06 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-08-03 13:06:42 UTC
Type: ---


Attachments (Terms of Use)
Proposed patch (830 bytes, patch)
2011-02-14 07:10 UTC, Martin Decky
no flags Details | Diff

Description Martin Decky 2011-02-14 07:10:25 UTC
Created attachment 478557 [details]
Proposed patch

Description of problem:

The macros H_SYNC_WIDTH and H_SYNC_OFFSET (in drivers/video/edid.h in the kernel source tree) for getting the values for horizontal sync width and offset do not respect the EDID specification. Both these values are 10 bit values according to [1], not 6 bit values. The lower 8 bits are stored in byte 8 and byte 9 of the DTD respectively, while two additional bits for each value are stored in byte 11. The original macros just slammed the bits together.

[1] VESA Enhanced Extended Display Identification Data Standard,
    Release A, Revision 2, page 34, notes 7 & 8, VESA, Sep 2006

Version-Release number of selected component (if applicable):

Every Linux kernel since 2005.

How reproducible:

Although the original macros usually work fine because the higher two bits are rarely used, it can cause problems on plasma panels running at 1920x1080 @ 50 Hz with drivers that rely on the correct parsing of the EDID data by fbmon.c.

The display timing in 1920x1080 @ 50 Hz is usually such that if the higher two bits are slammed with the lower 8 bits, the video output is shifted to the right (the sync width is too small).

Steps to Reproduce:

1. Use any framebuffer driver which relies on correct EDID data (e.g. udlfb from linux-next).
2. Connect to a monitor which uses the higher 2 bits in the respective EDID fields.
  
Actual results:

The retrieved EDID values are obviously wrong, thus the display output is misplaced (some monitors might even report "wrong signal").

Expected results:

The EDID data should be parsed correctly and the display output parameters should be set according to the supplied EDID data.

Additional info:

This patch has been submitted to the linux-fbdev.org. However, it is a very trivial patch and therefore could be applied downstream immediately (IMHO).

Comment 1 Jon Masters 2011-02-16 07:44:36 UTC
Did you raise this upstream or have a link to upstream conversation?

(What you're saying sounds plausible but I haven't verified any of it, as I don't have time other than to suggest if this is a breakage in EDID parsing that affects other kernels, it should be raised upstream on linux-kernel.org)

Jon.

Comment 2 Martin Decky 2011-02-16 08:49:14 UTC
Like I have written, I have already sent this patch upstream, specifically to the linux-fbdev.org mailing list, see it in the archive and patchwork

http://marc.info/?l=linux-fbdev&m=129762711305257&w=2
https://patchwork.kernel.org/patch/553331/

Paul Mundt, the unofficial maintainer of the framebuffer subsystem and the owner of git://git.kernel.org/pub/scm/linux/kernel/git/lethal/fbdev-2.6.git (which is regularly pulled into linux-next), also received the patch.

I don't believe going directly to linux-kernel.org is reasonable course of action in this case. If every tiny changeset to the upstream Linux kernel would go through a single mailing list, the development process would be dead by now.

And seriously, would the fact that the patch is posted there actually _create_ you the time you need to google for the document [1], go to page 34, read the notes 7 and 8 and confirm that my patch is OK? I mean, are those 10 minutes you would need to finish those four steps hinted in the previous sentence so expensive?

Comment 3 Chuck Ebbert 2011-02-16 16:28:46 UTC
I asked an expert and he says it looks like vsync has the same problem.

Comment 4 Martin Decky 2011-02-16 18:18:17 UTC
(In reply to comment #3)
> I asked an expert and he says it looks like vsync has the same problem.

I don't think so. Again by reading the same document [1], page 34, this time notes 9 and 10:

<quote>
9. Vertical Front Porch in Lines (from blanking start to start of sync) is represented by a 6 bit number (Bits 3 & 2 of byte 11 and the upper nibble of byte 10) - Range is 0 lines to 63 lines.

10. Vertical Sync Pulse Width in Lines (from the end of the front porch to the start of the back porch) is represented by a 6 bit number (Bits 1 & 0 of byte 11 and the lower nibble of byte 10 - Range is 0 lines to 63 lines.
</quote>

This is really not very surprising. You have 4 bits from byte 10 (the lower or upper nibble) and 2 bits from byte 11. There is no special provision for computing any additional bits.

Comment 5 Martin Decky 2011-04-07 19:02:44 UTC
The patch has been merged upstream into kernel 2.6.39-rc2 (see http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=dfc906daeec03b3f2d306ae260d398d97ba232c5).

Unfortunately, it didn't make it into 2.6.38 which is probably going to be in Fedora 15. Can you please add this patch to the Fedora's 2.6.38 kernel for F15? This really shouldn't be such a big deal ..

Comment 6 Martin Decky 2011-08-03 13:06:42 UTC
As we currently have the kernel 2.6.40 (a.k.a. 3.0) in F15, this bug is no longer relevant.


Note You need to log in before you can comment on or make changes to this bug.