[8/8] drm/edid: Dump bogus 18 byte descriptors
diff mbox series

Message ID 20200124200231.10517-8-ville.syrjala@linux.intel.com
State New
Headers show
Series
  • [1/8] drm/edid: Check the number of detailed timing descriptors in the CEA ext block
Related show

Commit Message

Ville Syrjälä Jan. 24, 2020, 8:02 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I'm curious if there are any bogus 18 byte descriptors around.
Let's dump them out if we encounter them.

Not sure we'd actually want this, but at least I get to see
if our CI has anything that hits this :)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

Comments

Alex Deucher Jan. 27, 2020, 10:38 p.m. UTC | #1
On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> I'm curious if there are any bogus 18 byte descriptors around.
> Let's dump them out if we encounter them.
>
> Not sure we'd actually want this, but at least I get to see
> if our CI has anything that hits this :)
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Acked-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/drm_edid.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 8e76efe1654d..4d8303e56536 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2202,6 +2202,12 @@ static bool is_display_descriptor(const u8 d[18], u8 tag)
>                 d[2] == 0x00 && d[3] == tag;
>  }
>
> +static bool is_any_display_descriptor(const u8 d[18])
> +{
> +       return d[0] == 0x00 && d[1] == 0x00 &&
> +               d[2] == 0x00;
> +}
> +
>  static bool is_detailed_timing_descriptor(const u8 d[18])
>  {
>         return d[0] != 0x00 || d[1] != 0x00;
> @@ -2209,6 +2215,15 @@ static bool is_detailed_timing_descriptor(const u8 d[18])
>
>  typedef void detailed_cb(const struct detailed_timing *timing, void *closure);
>
> +static void do_detailed_block(const u8 d[18], detailed_cb *cb, void *closure)
> +{
> +       if (!is_detailed_timing_descriptor(d) &&
> +           !is_any_display_descriptor(d))
> +               DRM_WARN("Unrecognized 18 byte descriptor: %*ph\n", 18, d);
> +
> +       cb((const struct detailed_timing *)d, closure);
> +}
> +
>  static void
>  cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure)
>  {
> @@ -2221,7 +2236,7 @@ cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure)
>
>         n = (127 - d) / 18;
>         for (i = 0; i < n; i++)
> -               cb((const struct detailed_timing *)(det_base + 18 * i), closure);
> +               do_detailed_block(det_base + 18 * i, cb, closure);
>  }
>
>  static void
> @@ -2234,7 +2249,7 @@ vtb_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure)
>                 return; /* unknown version */
>
>         for (i = 0; i < n; i++)
> -               cb((const struct detailed_timing *)(det_base + 18 * i), closure);
> +               do_detailed_block(det_base + 18 * i, cb, closure);
>  }
>
>  static void
> @@ -2247,7 +2262,8 @@ drm_for_each_detailed_block(const u8 *raw_edid, detailed_cb *cb, void *closure)
>                 return;
>
>         for (i = 0; i < EDID_DETAILED_TIMINGS; i++)
> -               cb(&(edid->detailed_timings[i]), closure);
> +               do_detailed_block((const u8 *)&edid->detailed_timings[i],
> +                                 cb, closure);
>
>         for (i = 1; i <= raw_edid[0x7e]; i++) {
>                 const u8 *ext = raw_edid + (i * EDID_LENGTH);
> --
> 2.24.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Shankar, Uma Feb. 3, 2020, 8:38 p.m. UTC | #2
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex
> Deucher
> Sent: Tuesday, January 28, 2020 4:09 AM
> To: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>; Maling list - DRI
> developers <dri-devel@lists.freedesktop.org>
> Subject: Re: [Intel-gfx] [PATCH 8/8] drm/edid: Dump bogus 18 byte descriptors
> 
> On Fri, Jan 24, 2020 at 3:03 PM Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> >
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > I'm curious if there are any bogus 18 byte descriptors around.
> > Let's dump them out if we encounter them.
> >
> > Not sure we'd actually want this, but at least I get to see if our CI
> > has anything that hits this :)
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Acked-by: Alex Deucher <alexander.deucher@amd.com>

Looks good to me as well.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> > ---
> >  drivers/gpu/drm/drm_edid.c | 22 +++++++++++++++++++---
> >  1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 8e76efe1654d..4d8303e56536 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -2202,6 +2202,12 @@ static bool is_display_descriptor(const u8 d[18], u8
> tag)
> >                 d[2] == 0x00 && d[3] == tag;  }
> >
> > +static bool is_any_display_descriptor(const u8 d[18]) {
> > +       return d[0] == 0x00 && d[1] == 0x00 &&
> > +               d[2] == 0x00;
> > +}
> > +
> >  static bool is_detailed_timing_descriptor(const u8 d[18])  {
> >         return d[0] != 0x00 || d[1] != 0x00; @@ -2209,6 +2215,15 @@
> > static bool is_detailed_timing_descriptor(const u8 d[18])
> >
> >  typedef void detailed_cb(const struct detailed_timing *timing, void
> > *closure);
> >
> > +static void do_detailed_block(const u8 d[18], detailed_cb *cb, void
> > +*closure) {
> > +       if (!is_detailed_timing_descriptor(d) &&
> > +           !is_any_display_descriptor(d))
> > +               DRM_WARN("Unrecognized 18 byte descriptor: %*ph\n",
> > +18, d);
> > +
> > +       cb((const struct detailed_timing *)d, closure); }
> > +
> >  static void
> >  cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void
> > *closure)  { @@ -2221,7 +2236,7 @@ cea_for_each_detailed_block(const
> > u8 *ext, detailed_cb *cb, void *closure)
> >
> >         n = (127 - d) / 18;
> >         for (i = 0; i < n; i++)
> > -               cb((const struct detailed_timing *)(det_base + 18 * i), closure);
> > +               do_detailed_block(det_base + 18 * i, cb, closure);
> >  }
> >
> >  static void
> > @@ -2234,7 +2249,7 @@ vtb_for_each_detailed_block(const u8 *ext, detailed_cb
> *cb, void *closure)
> >                 return; /* unknown version */
> >
> >         for (i = 0; i < n; i++)
> > -               cb((const struct detailed_timing *)(det_base + 18 * i), closure);
> > +               do_detailed_block(det_base + 18 * i, cb, closure);
> >  }
> >
> >  static void
> > @@ -2247,7 +2262,8 @@ drm_for_each_detailed_block(const u8 *raw_edid,
> detailed_cb *cb, void *closure)
> >                 return;
> >
> >         for (i = 0; i < EDID_DETAILED_TIMINGS; i++)
> > -               cb(&(edid->detailed_timings[i]), closure);
> > +               do_detailed_block((const u8 *)&edid->detailed_timings[i],
> > +                                 cb, closure);
> >
> >         for (i = 1; i <= raw_edid[0x7e]; i++) {
> >                 const u8 *ext = raw_edid + (i * EDID_LENGTH);
> > --
> > 2.24.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patch
diff mbox series

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 8e76efe1654d..4d8303e56536 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2202,6 +2202,12 @@  static bool is_display_descriptor(const u8 d[18], u8 tag)
 		d[2] == 0x00 && d[3] == tag;
 }
 
+static bool is_any_display_descriptor(const u8 d[18])
+{
+	return d[0] == 0x00 && d[1] == 0x00 &&
+		d[2] == 0x00;
+}
+
 static bool is_detailed_timing_descriptor(const u8 d[18])
 {
 	return d[0] != 0x00 || d[1] != 0x00;
@@ -2209,6 +2215,15 @@  static bool is_detailed_timing_descriptor(const u8 d[18])
 
 typedef void detailed_cb(const struct detailed_timing *timing, void *closure);
 
+static void do_detailed_block(const u8 d[18], detailed_cb *cb, void *closure)
+{
+	if (!is_detailed_timing_descriptor(d) &&
+	    !is_any_display_descriptor(d))
+		DRM_WARN("Unrecognized 18 byte descriptor: %*ph\n", 18, d);
+
+	cb((const struct detailed_timing *)d, closure);
+}
+
 static void
 cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure)
 {
@@ -2221,7 +2236,7 @@  cea_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure)
 
 	n = (127 - d) / 18;
 	for (i = 0; i < n; i++)
-		cb((const struct detailed_timing *)(det_base + 18 * i), closure);
+		do_detailed_block(det_base + 18 * i, cb, closure);
 }
 
 static void
@@ -2234,7 +2249,7 @@  vtb_for_each_detailed_block(const u8 *ext, detailed_cb *cb, void *closure)
 		return; /* unknown version */
 
 	for (i = 0; i < n; i++)
-		cb((const struct detailed_timing *)(det_base + 18 * i), closure);
+		do_detailed_block(det_base + 18 * i, cb, closure);
 }
 
 static void
@@ -2247,7 +2262,8 @@  drm_for_each_detailed_block(const u8 *raw_edid, detailed_cb *cb, void *closure)
 		return;
 
 	for (i = 0; i < EDID_DETAILED_TIMINGS; i++)
-		cb(&(edid->detailed_timings[i]), closure);
+		do_detailed_block((const u8 *)&edid->detailed_timings[i],
+				  cb, closure);
 
 	for (i = 1; i <= raw_edid[0x7e]; i++) {
 		const u8 *ext = raw_edid + (i * EDID_LENGTH);