[Intel-gfx] [PATCH 1/2] drm/edid: Add and export function to parse manufacturer id

[Souza, Jose]

On Tue, 2018-11-13 at 22:07 +0200, Jani Nikula wrote:
> On Thu, 08 Nov 2018, Daniel Vetter <daniel at ffwll.ch> wrote:
> > On Thu, Nov 08, 2018 at 08:42:52PM +0000, Souza, Jose wrote:
> > > On Thu, 2018-11-08 at 09:31 +0100, Daniel Vetter wrote:
> > > > On Wed, Nov 07, 2018 at 04:23:52PM -0800, José Roberto de Souza
> > > > wrote:
> > > > > This function will be helpful to drivers that wants to add
> > > > > its own
> > > > > quirks to sinks.
> > > > 
> > > > Why would you want to do that? The point of a shared edid
> > > > parsing
> > > > code is
> > > > that we can share all these quirks ...
> > > > 
> > > > For these kind of patches, always include the driver code that
> > > > makes
> > > > use
> > > > of your new code too. That makes it much easier to answer these
> > > > questions.
> > > 
> > > This will be used to disable or enable with quirks PSR in some
> > > panels
> > > that do not behave like eDP spec states.
> > > As this would be specifc to i915, I guess is better keep the list
> > > only
> > > in i915.
> > > 
> > > What is your opinion about that?
> > 
> > For anything dp, shouldn't we use the OUI instead? Or is that more
> > garbage
> > than the EDID serial?
> 
> The OUI isn't always present, but we can use it when it is. And we
> already have DPCD quirk support for that in drm_dp_helper.c.

I was not aware about OUI, thanks.

OUI is a requirement in DP 1.2+ and PSR was added in DP 1.3, so I can
safely use it.

> 
> > And yes, psr quirking for i915 seems like a reasonable thing to do,
> > using
> > either approach. But definitely include the full picture, including
> > i915
> > patches, in your next round.
> 
> I think all of the quirk matching should be in drm core or helpers.
> For
> most quirks, it's up to the drivers to actually do something with
> that
> information anyway, so it'll still remain i915 specific.

The edid quirks are returned from drm_add_display_info() and it is used
only to updated some connector parameters in drm_add_edid_modes().

Also the use that we have right now is blacklist all panels from a
vendor that have its own proprietary PSR-like implementation, so that
use would not fit in the current drm quirk list.

So for now I can use the OUI to check if the panel vendor match and if
so not enable PSR.

> 
> BR,
> Jani.
> 
> 
> 
> 
> > -Daniel
> > 
> > > > Thanks, Daniel
> > > > 
> > > > 
> > > > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/drm_edid.c | 20 ++++++++++++++++----
> > > > >  include/drm/drm_edid.h     |  1 +
> > > > >  2 files changed, 17 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/drm_edid.c
> > > > > b/drivers/gpu/drm/drm_edid.c
> > > > > index b506e3622b08..1a0ddf3d326b 100644
> > > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > > @@ -1755,6 +1755,21 @@ EXPORT_SYMBOL(drm_edid_duplicate);
> > > > >  
> > > > >  /*** EDID parsing ***/
> > > > >  
> > > > > +/**
> > > > > + * drm_edid_manufacturer_parse - parse the EDID manufacturer
> > > > > id to
> > > > > readable
> > > > > + * characters and set into manufacturer parameter.
> > > > > + * @edid: EDID to get the manufacturer
> > > > > + * @manufacturer: the char buffer to store the id
> > > > > + */
> > > > > +void drm_edid_manufacturer_parse(const struct edid *edid,
> > > > > char
> > > > > manufacturer[3])
> > > > > +{
> > > > > +	manufacturer[0] = ((edid->mfg_id[0] & 0x7c) >> 2) +
> > > > > '@';
> > > > > +	manufacturer[1] = (((edid->mfg_id[0] & 0x3) << 3) |
> > > > > +			  ((edid->mfg_id[1] & 0xe0) >> 5)) +
> > > > > '@';
> > > > > +	manufacturer[2] = (edid->mfg_id[1] & 0x1f) + '@';
> > > > > +}
> > > > > +EXPORT_SYMBOL(drm_edid_manufacturer_parse);
> > > > > +
> > > > >  /**
> > > > >   * edid_vendor - match a string against EDID's obfuscated
> > > > > vendor
> > > > > field
> > > > >   * @edid: EDID to match
> > > > > @@ -1766,10 +1781,7 @@ static bool edid_vendor(const struct
> > > > > edid
> > > > > *edid, const char *vendor)
> > > > >  {
> > > > >  	char edid_vendor[3];
> > > > >  
> > > > > -	edid_vendor[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
> > > > > -	edid_vendor[1] = (((edid->mfg_id[0] & 0x3) << 3) |
> > > > > -			  ((edid->mfg_id[1] & 0xe0) >> 5)) +
> > > > > '@';
> > > > > -	edid_vendor[2] = (edid->mfg_id[1] & 0x1f) + '@';
> > > > > +	drm_edid_manufacturer_parse(edid, edid_vendor);
> > > > >  
> > > > >  	return !strncmp(edid_vendor, vendor, 3);
> > > > >  }
> > > > > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > > > > index e3c404833115..e4f3f7f34d6a 100644
> > > > > --- a/include/drm/drm_edid.h
> > > > > +++ b/include/drm/drm_edid.h
> > > > > @@ -466,6 +466,7 @@ struct edid
> > > > > *drm_get_edid_switcheroo(struct
> > > > > drm_connector *connector,
> > > > >  				     struct i2c_adapter
> > > > > *adapter);
> > > > >  struct edid *drm_edid_duplicate(const struct edid *edid);
> > > > >  int drm_add_edid_modes(struct drm_connector *connector,
> > > > > struct
> > > > > edid *edid);
> > > > > +void drm_edid_manufacturer_parse(const struct edid *edid,
> > > > > char
> > > > > manufacturer[3]);
> > > > >  
> > > > >  u8 drm_match_cea_mode(const struct drm_display_mode
> > > > > *to_match);
> > > > >  enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8
> > > > > video_code);
> > > > > -- 
> > > > > 2.19.1
> > > > > 
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx at lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20181120/6d0ba6e0/attachment.sig>