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

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

[José Roberto de Souza]

This function will be helpful to drivers that wants to add its own
quirks to sinks.

Signed-off-by: José Roberto de Souza <jose.souza@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/edid: Parse manufacturer id only once per sink

[José Roberto de Souza]

It was parsing the manufacturer id of the sink for each entry in
quirk list.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/drm_edid.c | 21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 1a0ddf3d326b..69209fcc40f0 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1770,22 +1770,6 @@ void drm_edid_manufacturer_parse(const struct edid *edid, char manufacturer[3])
 }
 EXPORT_SYMBOL(drm_edid_manufacturer_parse);
 
-/**
- * edid_vendor - match a string against EDID's obfuscated vendor field
- * @edid: EDID to match
- * @vendor: vendor string
- *
- * Returns true if @vendor is in @edid, false otherwise
- */
-static bool edid_vendor(const struct edid *edid, const char *vendor)
-{
-	char edid_vendor[3];
-
-	drm_edid_manufacturer_parse(edid, edid_vendor);
-
-	return !strncmp(edid_vendor, vendor, 3);
-}
-
 /**
  * edid_get_quirks - return quirk flags for a given EDID
  * @edid: EDID to process
@@ -1795,12 +1779,15 @@ static bool edid_vendor(const struct edid *edid, const char *vendor)
 static u32 edid_get_quirks(const struct edid *edid)
 {
 	const struct edid_quirk *quirk;
+	char edid_vendor[3];
 	int i;
 
+	drm_edid_manufacturer_parse(edid, edid_vendor);
+
 	for (i = 0; i < ARRAY_SIZE(edid_quirk_list); i++) {
 		quirk = &edid_quirk_list[i];
 
-		if (edid_vendor(edid, quirk->vendor) &&
+		if (!strncmp(edid_vendor, quirk->vendor, 3) &&
 		    (EDID_PRODUCT_ID(edid) == quirk->product_id))
 			return quirk->quirks;
 	}
-- 
2.19.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/edid: Add and export function to parse manufacturer id

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/edid: Add and export function to parse manufacturer id
URL   : https://patchwork.freedesktop.org/series/52197/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5103 -> Patchwork_10764 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10764 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10764, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52197/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10764:

  === IGT changes ===

    ==== Warnings ====

    igt@drv_selftest@live_guc:
      fi-skl-iommu:       PASS -> SKIP +1

    
== Known issues ==

  Here are the changes found in Patchwork_10764 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-skl-iommu:       PASS -> INCOMPLETE (fdo#108602)

    igt@gem_exec_suspend@basic-s3:
      fi-icl-u:           PASS -> INCOMPLETE (fdo#107713)

    igt@kms_chamelium@common-hpd-after-suspend:
      fi-skl-6700k2:      PASS -> WARN (fdo#108680)

    igt@kms_frontbuffer_tracking@basic:
      fi-icl-u2:          PASS -> FAIL (fdo#103167)

    
    ==== Possible fixes ====

    igt@gem_ctx_create@basic-files:
      fi-icl-u2:          DMESG-WARN (fdo#107724) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-hsw-4770r:       DMESG-WARN (fdo#105602) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#108602 https://bugs.freedesktop.org/show_bug.cgi?id=108602
  fdo#108680 https://bugs.freedesktop.org/show_bug.cgi?id=108680


== Participating hosts (54 -> 47) ==

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-snb-2520m fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_5103 -> Patchwork_10764

  CI_DRM_5103: 23c1138030ad65402f698ab0b356e2f55722bc77 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4712: a3ede1b535ac8137f6949c468edd7054453d5dae @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10764: efbb9fc9f7de18543259c376eaf16a9baf1309e7 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

efbb9fc9f7de drm/edid: Parse manufacturer id only once per sink
7393f3658c68 drm/edid: Add and export function to parse manufacturer id

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10764/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

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.

Thanks, Daniel


> 
> Signed-off-by: José Roberto de Souza <jose.souza@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 3533 bytes --]

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?

> 
> Thanks, Daniel
> 
> 
> > Signed-off-by: José Roberto de Souza <jose.souza@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Daniel Vetter]

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?

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.
-Daniel

> 
> > 
> > Thanks, Daniel
> > 
> > 
> > > Signed-off-by: José Roberto de Souza <jose.souza@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@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Jani Nikula]

On Thu, 08 Nov 2018, Daniel Vetter <daniel@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.

> 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.

BR,
Jani.




> -Daniel
>
>> 
>> > 
>> > Thanks, Daniel
>> > 
>> > 
>> > > Signed-off-by: José Roberto de Souza <jose.souza@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@lists.freedesktop.org
>> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 5756 bytes --]

On Tue, 2018-11-13 at 22:07 +0200, Jani Nikula wrote:
> On Thu, 08 Nov 2018, Daniel Vetter <daniel@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@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@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✓ Fi.CI.IGT: success for series starting with [1/2] drm/edid: Add and export function to parse manufacturer id

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/edid: Add and export function to parse manufacturer id
URL   : https://patchwork.freedesktop.org/series/52197/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5103_full -> Patchwork_10764_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10764_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10764_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10764_full:

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_10764_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@fence-restore-tiled2untiled:
      shard-skl:          PASS -> INCOMPLETE (fdo#104108, fdo#107773)

    igt@drv_suspend@shrink:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411, fdo#106886)
      shard-glk:          PASS -> INCOMPLETE (fdo#103359, fdo#106886, k.org#198133)

    igt@gem_cpu_reloc@full:
      shard-skl:          NOTRUN -> INCOMPLETE (fdo#108073)

    igt@gem_exec_schedule@pi-ringfull-bsd:
      shard-skl:          NOTRUN -> FAIL (fdo#103158)

    igt@kms_chv_cursor_fail@pipe-a-64x64-right-edge:
      shard-skl:          NOTRUN -> FAIL (fdo#104671)

    igt@kms_cursor_crc@cursor-64x21-onscreen:
      shard-glk:          PASS -> FAIL (fdo#103232) +1

    igt@kms_cursor_crc@cursor-64x64-sliding:
      shard-skl:          NOTRUN -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-size-change:
      shard-apl:          PASS -> FAIL (fdo#103232) +1

    igt@kms_flip@flip-vs-expired-vblank:
      shard-skl:          PASS -> FAIL (fdo#105363)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
      shard-glk:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
      shard-apl:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-farfromfence:
      shard-skl:          NOTRUN -> FAIL (fdo#105682)

    igt@kms_frontbuffer_tracking@fbc-indfb-scaledprimary:
      shard-skl:          PASS -> FAIL (fdo#105682)

    igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc:
      shard-skl:          NOTRUN -> FAIL (fdo#103167) +2

    igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +2

    igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
      shard-skl:          PASS -> FAIL (fdo#108145, fdo#107815)

    igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
      shard-glk:          PASS -> FAIL (fdo#103166) +2

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    igt@perf@polling:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    igt@pm_rpm@gem-mmap-cpu:
      shard-skl:          PASS -> INCOMPLETE (fdo#107807)

    
    ==== Possible fixes ====

    igt@drm_import_export@import-close-race-flink:
      shard-skl:          TIMEOUT (fdo#108667) -> PASS

    igt@gem_exec_reuse@baggage:
      shard-apl:          DMESG-WARN (fdo#108690) -> PASS

    igt@kms_color@pipe-b-ctm-max:
      shard-apl:          FAIL (fdo#108147) -> PASS

    igt@kms_cursor_crc@cursor-256x85-onscreen:
      shard-apl:          FAIL (fdo#103232) -> PASS +1

    igt@kms_cursor_legacy@cursorb-vs-flipb-varying-size:
      shard-glk:          DMESG-WARN (fdo#105763, fdo#106538) -> PASS

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
      shard-glk:          FAIL (fdo#103167) -> PASS +4

    igt@pm_rpm@drm-resources-equal:
      shard-skl:          INCOMPLETE (fdo#107807) -> PASS

    igt@pm_rpm@modeset-non-lpsp-stress-no-wait:
      shard-skl:          INCOMPLETE (fdo#107807) -> SKIP

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#108073 https://bugs.freedesktop.org/show_bug.cgi?id=108073
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108147 https://bugs.freedesktop.org/show_bug.cgi?id=108147
  fdo#108667 https://bugs.freedesktop.org/show_bug.cgi?id=108667
  fdo#108690 https://bugs.freedesktop.org/show_bug.cgi?id=108690
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5103 -> Patchwork_10764

  CI_DRM_5103: 23c1138030ad65402f698ab0b356e2f55722bc77 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4712: a3ede1b535ac8137f6949c468edd7054453d5dae @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10764: efbb9fc9f7de18543259c376eaf16a9baf1309e7 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10764/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx