Signed-off-by: Adam Jackson ajax@redhat.com --- Documentation/kernel-parameters.txt | 4 ++++ drivers/gpu/drm/drm_edid.c | 5 ++++- 2 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index e2202e9..17d50d2 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -713,6 +713,10 @@ and is between 256 and 4096 characters. It is defined in the file edd= [EDD] Format: {"off" | "on" | "skip[mbr]"}
+ edid_threshold=<int> [DRM] + Set the minimum number of bytes of an EDID header + that must be valid, out of 8. The default is 6. + eisa_irq_edge= [PARISC,HW] See header of drivers/parisc/eisa.c.
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 7188674..6a1b5a3 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -62,6 +62,9 @@ /* use +hsync +vsync for detailed mode */ #define EDID_QUIRK_DETAILED_SYNC_PP (1 << 6)
+static unsigned int edid_threshold = 6; +MODULE_PARM_DESC(edid_threshold, "EDID header fixup threshold (default: 6)"); +module_param_named(edid_threshold, edid_threshold, int, 0600);
#define LEVEL_DMT 0 #define LEVEL_GTF 1 @@ -135,7 +138,7 @@ drm_edid_block_valid(u8 *raw_edid) score++;
if (score == 8) ; - else if (score >= 6) { + else if (score >= edid_threshold) { DRM_DEBUG("Fixing EDID header, your hardware may be failing\n"); memcpy(raw_edid, edid_header, sizeof(edid_header)); } else {
So much broken hardware, so few bullets.
Signed-off-by: Adam Jackson ajax@redhat.com --- Documentation/kernel-parameters.txt | 3 +++ drivers/gpu/drm/drm_edid.c | 12 ++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 17d50d2..5814331 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -713,6 +713,9 @@ and is between 256 and 4096 characters. It is defined in the file edd= [EDD] Format: {"off" | "on" | "skip[mbr]"}
+ edid_force_checksum=<bool> [DRM] + Forcibly correct EDID checksum errors. Default is off. + edid_threshold=<int> [DRM] Set the minimum number of bytes of an EDID header that must be valid, out of 8. The default is 6. diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 6a1b5a3..55b498e 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -66,6 +66,10 @@ static unsigned int edid_threshold = 6; MODULE_PARM_DESC(edid_threshold, "EDID header fixup threshold (default: 6)"); module_param_named(edid_threshold, edid_threshold, int, 0600);
+static bool edid_force_checksum = 0; +MODULE_PARM_DESC(edid_force_checksum, "Forcibly correct EDID checksum"); +module_param_named(edid_force_checksum, edid_force_checksum, int, 0600); + #define LEVEL_DMT 0 #define LEVEL_GTF 1 #define LEVEL_GTF2 2 @@ -149,8 +153,12 @@ drm_edid_block_valid(u8 *raw_edid) for (i = 0; i < EDID_LENGTH; i++) csum += raw_edid[i]; if (csum) { - DRM_ERROR("EDID checksum is invalid, remainder is %d\n", csum); - goto bad; + if (edid_force_checksum) { + raw_edid[0x7f] -= csum; + } else { + DRM_ERROR("EDID checksum is invalid (%d)\n", csum); + goto bad; + } }
/* per-block-type checks */
On Tue, Apr 27, 2010 at 5:03 AM, Adam Jackson ajax@redhat.com wrote:
Signed-off-by: Adam Jackson ajax@redhat.com
not sure about the kernel-parameters additions, since the option would be drm.edid_threshhold= and its a module option, though now that we can accept module options on the command line, we could start adding them to parameters file. However you'd need to add the drm. bit to the front of the option.
How much broken hw we seeing that this fixes? maybe some bug references, as I hate having special options that people have to know just to see somehting on screen.
Dave.
Documentation/kernel-parameters.txt | 4 ++++ drivers/gpu/drm/drm_edid.c | 5 ++++- 2 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index e2202e9..17d50d2 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -713,6 +713,10 @@ and is between 256 and 4096 characters. It is defined in the file edd= [EDD] Format: {"off" | "on" | "skip[mbr]"}
- edid_threshold=<int> [DRM]
- Set the minimum number of bytes of an EDID header
- that must be valid, out of 8. The default is 6.
eisa_irq_edge= [PARISC,HW] See header of drivers/parisc/eisa.c.
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 7188674..6a1b5a3 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -62,6 +62,9 @@ /* use +hsync +vsync for detailed mode */ #define EDID_QUIRK_DETAILED_SYNC_PP (1 << 6)
+static unsigned int edid_threshold = 6; +MODULE_PARM_DESC(edid_threshold, "EDID header fixup threshold (default: 6)"); +module_param_named(edid_threshold, edid_threshold, int, 0600);
#define LEVEL_DMT 0 #define LEVEL_GTF 1 @@ -135,7 +138,7 @@ drm_edid_block_valid(u8 *raw_edid) score++;
if (score == 8) ;
- else if (score >= 6) {
- else if (score >= edid_threshold) {
DRM_DEBUG("Fixing EDID header, your hardware may be failing\n"); memcpy(raw_edid, edid_header, sizeof(edid_header)); } else { -- 1.7.0.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 2010-04-27 at 15:01 +1000, Dave Airlie wrote:
On Tue, Apr 27, 2010 at 5:03 AM, Adam Jackson ajax@redhat.com wrote:
Signed-off-by: Adam Jackson ajax@redhat.com
not sure about the kernel-parameters additions, since the option would be drm.edid_threshhold= and its a module option, though now that we can accept module options on the command line, we could start adding them to parameters file. However you'd need to add the drm. bit to the front of the option.
The doc already kind of mentions that, but it's kind of wrong. It only talks about "passing options with modprobe" and "passing options to built-in modules on kcmdline", but not about "passing options to loadable modules on kcmdline". I'll follow up on that to lkml.
How much broken hw we seeing that this fixes? maybe some bug references, as I hate having special options that people have to know just to see somehting on screen.
I've seen at least two cases in bug reports. I could dig up a reference I suppose.
That's a valid point though: we currently don't distinguish between "connected but with broken EDID" and "disconnected". I think it sounds reasonable to treat the former as connector_status_unknown instead. That way, if nothing else is connected, X will still light you up at the fallback size. Sound right?
- ajax
On Tue, Apr 27, 2010 at 10:30 AM, Adam Jackson ajax@redhat.com wrote:
On Tue, 2010-04-27 at 15:01 +1000, Dave Airlie wrote:
On Tue, Apr 27, 2010 at 5:03 AM, Adam Jackson ajax@redhat.com wrote:
Signed-off-by: Adam Jackson ajax@redhat.com
not sure about the kernel-parameters additions, since the option would be drm.edid_threshhold= and its a module option, though now that we can accept module options on the command line, we could start adding them to parameters file. However you'd need to add the drm. bit to the front of the option.
The doc already kind of mentions that, but it's kind of wrong. It only talks about "passing options with modprobe" and "passing options to built-in modules on kcmdline", but not about "passing options to loadable modules on kcmdline". I'll follow up on that to lkml.
How much broken hw we seeing that this fixes? maybe some bug references, as I hate having special options that people have to know just to see somehting on screen.
I've seen at least two cases in bug reports. I could dig up a reference I suppose.
That's a valid point though: we currently don't distinguish between "connected but with broken EDID" and "disconnected". I think it sounds reasonable to treat the former as connector_status_unknown instead. That way, if nothing else is connected, X will still light you up at the fallback size. Sound right?
Or maybe just mention the EDID is bad and may cause problems, but then use as much of it as possible. That has the best chance of a satisfied user and a native mode being selected.
Alex
- ajax
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 2010-04-27 at 10:47 -0400, Alex Deucher wrote:
On Tue, Apr 27, 2010 at 10:30 AM, Adam Jackson ajax@redhat.com wrote:
That's a valid point though: we currently don't distinguish between "connected but with broken EDID" and "disconnected". I think it sounds reasonable to treat the former as connector_status_unknown instead. That way, if nothing else is connected, X will still light you up at the fallback size. Sound right?
Or maybe just mention the EDID is bad and may cause problems, but then use as much of it as possible. That has the best chance of a satisfied user and a native mode being selected.
That would require a lot more parser work to make it robust in the face of suspicious-looking subsections. Not that we should never do that, but that I think it's the wrong strategy in general. I've really tended to shy away from being too permissive in what we accept from EDID, because the failure mode is picking a mode that the display doesn't support and coming up blank. I'd much rather play it safe if we're not 100% sure, especially since you can jam the correct mode in later with RANDR if you come up in a conservative mode.
One thing that _would_ be cool is figuring out the common ways of writing to EDID EEPROMs and exposing some I-know-what-I'm-doing way of trying them, so we could repair bad displays. Also, DDC/CI sometimes gives you a way of knowing whether the monitor has synced, which would let us know whether a given mode was bad or not.
- ajax
dri-devel@lists.freedesktop.org
-
Adam Jackson -
Alex Deucher -
Dave Airlie