[PATCH] drm/edid: Fix parsing of EDID 1.4 Established Timings III descriptor
On Wed, 30 Mar 2016, Ville Syrjälä wrote: > On Sat, Mar 26, 2016 at 01:18:38PM +, Paul Parsons wrote: >> The EDID 1.4 specification section 3.10.3.9 defines an Established Timings >> III >> descriptor (tag #F7h). The parsing of this descriptor by drm_est3_modes() is >> off by one byte: the offset of the first timing bitmap is 6, not 5. >> >> Signed-off-by: Paul Parsons >> --- >> >> diff -ru a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> --- a/drivers/gpu/drm/drm_edid.c 2016-03-14 04:28:54.0 + >> +++ b/drivers/gpu/drm/drm_edid.c 2016-03-26 12:04:58.963352156 + >> @@ -2215,7 +2215,7 @@ >> { >> int i, j, m, modes = 0; >> struct drm_display_mode *mode; >> -u8 *est = ((u8 *)timing) + 5; >> +u8 *est = ((u8 *)timing) + 6; > > Hmm. The code is very hard to follow due to the weird structs and > whatnot. But yeah this looks correct. > > Reviewed-by: Ville Syrjälä Pushed to our topic/drm-fixes, thanks for the patch and review. BR, Jani. > > Oh and I suppose you could also fix up the EDID version check > to check for 1.4+ before even considering the est3 descriptor > as valid. > >> >> for (i = 0; i < 6; i++) { >> for (j = 7; j >= 0; j--) { >> >> >> >> ___ >> dri-devel mailing list >> dri-devel at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Technology Center
[PATCH] drm/edid: Fix parsing of EDID 1.4 Established Timings III descriptor
On Sat, Mar 26, 2016 at 01:18:38PM +, Paul Parsons wrote: > The EDID 1.4 specification section 3.10.3.9 defines an Established Timings III > descriptor (tag #F7h). The parsing of this descriptor by drm_est3_modes() is > off by one byte: the offset of the first timing bitmap is 6, not 5. > > Signed-off-by: Paul Parsons > --- > > diff -ru a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > --- a/drivers/gpu/drm/drm_edid.c 2016-03-14 04:28:54.0 + > +++ b/drivers/gpu/drm/drm_edid.c 2016-03-26 12:04:58.963352156 + > @@ -2215,7 +2215,7 @@ > { > int i, j, m, modes = 0; > struct drm_display_mode *mode; > - u8 *est = ((u8 *)timing) + 5; > + u8 *est = ((u8 *)timing) + 6; Hmm. The code is very hard to follow due to the weird structs and whatnot. But yeah this looks correct. Reviewed-by: Ville Syrjälä Oh and I suppose you could also fix up the EDID version check to check for 1.4+ before even considering the est3 descriptor as valid. > > for (i = 0; i < 6; i++) { > for (j = 7; j >= 0; j--) { > > > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC
[PATCH] drm/edid: Fix parsing of EDID 1.4 Established Timings III descriptor
On Sun, 27 Mar 2016, Paul Parsons wrote: > The EDID 1.4 specification section 3.10.3.9 defines an Established Timings III > descriptor (tag #F7h). The parsing of this descriptor by drm_est3_modes() is > off by one byte: the offset of the first timing bitmap is 6, not 5. Dupe? Let's keep the discussion in [1]. BR, Jani. [1] http://mid.gmane.org/20160328002258.E75DF6E35D at gabe.freedesktop.org -- Jani Nikula, Intel Open Source Technology Center
[PATCH] drm/edid: Fix parsing of EDID 1.4 Established Timings III descriptor
On Sat, 26 Mar 2016, Paul Parsons wrote: > The EDID 1.4 specification section 3.10.3.9 defines an Established Timings III > descriptor (tag #F7h). The parsing of this descriptor by drm_est3_modes() is > off by one byte: the offset of the first timing bitmap is 6, not 5. > > Signed-off-by: Paul Parsons I'm not all that familiar with the EDID parsing code, but this seems right. Reviewed-by: Jani Nikula I'd appreciate an extra set of eyeballs on this one still. Makes you wonder about the implications though. We've been reading the revision number as a bitmask and otherwise been off by one byte. The interpretation of Established Timings III would have been pretty much random. Where are the bugs? BR, Jani. > --- > > diff -ru a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > --- a/drivers/gpu/drm/drm_edid.c 2016-03-14 04:28:54.0 + > +++ b/drivers/gpu/drm/drm_edid.c 2016-03-26 12:04:58.963352156 + > @@ -2215,7 +2215,7 @@ > { > int i, j, m, modes = 0; > struct drm_display_mode *mode; > - u8 *est = ((u8 *)timing) + 5; > + u8 *est = ((u8 *)timing) + 6; > > for (i = 0; i < 6; i++) { > for (j = 7; j >= 0; j--) { > > > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Technology Center
[PATCH] drm/edid: Fix parsing of EDID 1.4 Established Timings III descriptor
The EDID 1.4 specification section 3.10.3.9 defines an Established Timings III descriptor (tag #F7h). The parsing of this descriptor by drm_est3_modes() is off by one byte: the offset of the first timing bitmap is 6, not 5. Signed-off-by: Paul Parsons --- diff -ru a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c --- a/drivers/gpu/drm/drm_edid.c2016-03-14 04:28:54.0 + +++ b/drivers/gpu/drm/drm_edid.c2016-03-26 12:04:58.963352156 + @@ -2215,7 +2215,7 @@ { int i, j, m, modes = 0; struct drm_display_mode *mode; - u8 *est = ((u8 *)timing) + 5; + u8 *est = ((u8 *)timing) + 6; for (i = 0; i < 6; i++) { for (j = 7; j >= 0; j--) {
[PATCH] drm/edid: Fix parsing of EDID 1.4 Established Timings III descriptor
The EDID 1.4 specification section 3.10.3.9 defines an Established Timings III descriptor (tag #F7h). The parsing of this descriptor by drm_est3_modes() is off by one byte: the offset of the first timing bitmap is 6, not 5. Signed-off-by: Paul Parsons --- diff -ru a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c --- a/drivers/gpu/drm/drm_edid.c2016-03-14 04:28:54.0 + +++ b/drivers/gpu/drm/drm_edid.c2016-03-26 12:04:58.963352156 + @@ -2215,7 +2215,7 @@ { int i, j, m, modes = 0; struct drm_display_mode *mode; - u8 *est = ((u8 *)timing) + 5; + u8 *est = ((u8 *)timing) + 6; for (i = 0; i < 6; i++) { for (j = 7; j >= 0; j--) {