[Intel-gfx] [PATCH 6/8] drm/edid: Add a FIXME about DispID CEA data block revision

[Shankar, Uma]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: Tuesday, February 4, 2020 7:02 PM
> To: Shankar, Uma <uma.shankar at intel.com>
> Cc: dri-devel at lists.freedesktop.org; intel-gfx at lists.freedesktop.org; Andres
> Rodriguez <andresx7 at gmail.com>
> Subject: Re: [Intel-gfx] [PATCH 6/8] drm/edid: Add a FIXME about DispID CEA data
> block revision
> 
> On Mon, Feb 03, 2020 at 08:15:51PM +0000, Shankar, Uma wrote:
> >
> >
> > > -----Original Message-----
> > > From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf
> > > Of Ville Syrjala
> > > Sent: Saturday, January 25, 2020 1:32 AM
> > > To: dri-devel at lists.freedesktop.org
> > > Cc: intel-gfx at lists.freedesktop.org; Andres Rodriguez
> > > <andresx7 at gmail.com>
> > > Subject: [Intel-gfx] [PATCH 6/8] drm/edid: Add a FIXME about DispID
> > > CEA data block revision
> > >
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > >
> > > I don't understand what the DispID CEA data block revision means.
> > > The spec doesn't say. I guess some DispID must have a value of >= 3
> > > in there or else we generally wouldn't even parse the CEA data
> > > blocks. Or does all this code actually not do anything?
> >
> > This signifies the CTA extension revision (byte 1 of the block). As
> > per the spec, seems like Version 1 is legacy and 2 is deprecated. So version >=3 is
> checked here.
> > Refer section 7.3 of CTA-861-G
> 
> The confusion is about the revision field in the DispID CTA block, not in the CTA
> extension block.

Oh ok, got the ambiguity here. Not sure if we actually get >3 here as value for the block revision,
totally unclear from spec, default being 0. Good to have this comment till we get some clarity on
its significance. Thanks for the clarification.

Reviewed-by: Uma Shankar <uma.shankar at intel.com>

> >
> > > Cc: Andres Rodriguez <andresx7 at gmail.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_edid.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > index
> > > 0369a54e3d32..fd9b724067a7 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -3977,6 +3977,13 @@ cea_db_tag(const u8 *db)  static int
> > > cea_revision(const
> > > u8 *cea)  {
> > > +	/*
> > > +	 * FIXME is this correct for the DispID variant?
> > > +	 * The DispID spec doesn't really specify whether
> > > +	 * this is the revision of the CEA extension or
> > > +	 * the DispID CEA data block. And the only value
> > > +	 * given as an example is 0.
> > > +	 */
> > >  	return cea[1];
> > >  }
> > >
> > > --
> > > 2.24.1
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel