[PATCH] drm: edid revision 0 is valid

[PATCH] drm: edid revision 0 is valid

[Kyle McMartin]

From: Kyle McMartin <kyle@redhat.com>

edid->revision == 0 should be valid (at least, so the error message
indicates. :) and wikipedia seems to indicate that EDID 1.0 existed.

We can dump the entire check, since edid->revision is a u8, so
it can't ever be less than 0.

Marko reports in RH bz#476735 that his monitor claims to be
EDID 1.0, and therefore hits the check and is stuck at 800x600 because
of it.

Reported-by: Marko Ristola <marko.ristola@kolumbus.fi>
Signed-off-by: Kyle McMartin <kyle@redhat.com>
---
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5a4d324..ec14dd8 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -125,7 +125,7 @@ static bool edid_is_valid(struct edid *edid)
 		DRM_ERROR("EDID has major version %d, instead of 1\n", edid->version);
 		goto bad;
 	}
-	if (edid->revision <= 0 || edid->revision > 3) {
+	if (edid->revision > 3) {
 		DRM_ERROR("EDID has minor version %d, which is not between 0-3\n", edid->revision);
 		goto bad;
 	}

Re: [PATCH] drm: edid revision 0 is valid

[Jesse Barnes]

On Tuesday, February 24, 2009 5:31:53 pm Kyle McMartin wrote:
> From: Kyle McMartin <kyle@redhat.com>
>
> edid->revision == 0 should be valid (at least, so the error message
> indicates. :) and wikipedia seems to indicate that EDID 1.0 existed.
>
> We can dump the entire check, since edid->revision is a u8, so
> it can't ever be less than 0.
>
> Marko reports in RH bz#476735 that his monitor claims to be
> EDID 1.0, and therefore hits the check and is stuck at 800x600 because
> of it.
>
> Reported-by: Marko Ristola <marko.ristola@kolumbus.fi>
> Signed-off-by: Kyle McMartin <kyle@redhat.com>

Heh, yeah this makes the code correct *and* match the message.  Thanks.

Acked-by: Jesse Barnes <jbarnes@virtuousgek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] drm: edid revision 0 is valid

[Jamie Lokier]

Kyle McMartin wrote:
> -	if (edid->revision <= 0 || edid->revision > 3) {
> +	if (edid->revision > 3) {
>  		DRM_ERROR("EDID has minor version %d, which is not between 0-3\n", edid->revision);
>  		goto bad;

Ahem.  EDID version 1.4 exists too, in fact it's the current version.
EDID 1.5+ doesn't exist, but it should be backward compatible when it does.

I'm doing a lot of work on another EDID parser, including full DMT and
CEA-861 mode tables, monitor hotplug, all quirks from Xorg and the
kernel, EDID 1.4 support w/ CVT calculation, etc. right now for an
embedded Linux video project.  Would there be any interest
incorporating parts of that work?

-- Jamie

Re: [PATCH] drm: edid revision 0 is valid

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

On Wed, 2009-03-11 at 02:56 +0000, Jamie Lokier wrote:
> Kyle McMartin wrote:
> > -	if (edid->revision <= 0 || edid->revision > 3) {
> > +	if (edid->revision > 3) {
> >  		DRM_ERROR("EDID has minor version %d, which is not between 0-3\n", edid->revision);
> >  		goto bad;
> 
> Ahem.  EDID version 1.4 exists too, in fact it's the current version.
> EDID 1.5+ doesn't exist, but it should be backward compatible when it does.
> 
> I'm doing a lot of work on another EDID parser, including full DMT and
> CEA-861 mode tables, monitor hotplug, all quirks from Xorg and the
> kernel, EDID 1.4 support w/ CVT calculation, etc. right now for an
> embedded Linux video project.  Would there be any interest
> incorporating parts of that work?

There's work going on for CEA-861 and syncing KMS against UMS.  But
certainly we love fixes when we don't even have to write them ourselves.

-- 
Eric Anholt
eric@anholt.net                         eric.anholt@intel.com



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]