All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
@ 2018-09-13 21:17 matthew.s.atwood
  2018-09-13 22:12 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: matthew.s.atwood @ 2018-09-13 21:17 UTC (permalink / raw)
  To: intel-gfx, manasi.d.navare, rodrigo.vivi

From: Matt Atwood <matthew.s.atwood@intel.com>

According to DP spec (2.9.3.1 of DP 1.4) if
EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in DPCD
02200h through 0220Fh shall contain the DPRX's true capability. These
values will match 00000h through 0000Fh, except for DPCD_REV,
MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT.

Read from DPCD once for all 3 values as this is an expensive operation.
Spec mentions that all of address space 02200h through 0220Fh should
contain the right information however currently only 3 values can
differ.

There is no address space in the intel_dp->dpcd struct for addresses
02200h through 0220Fh, and since so much of the data is a identical,
simply overwrite the values stored in 00000h through 0000Fh with the
values that can be overwritten from addresses 02200h through 0220Fh.

This patch helps with backward compatibility for devices pre DP1.3.

v2: read only dpcd values which can be affected, remove incorrect check,
split into drm include changes into separate patch, commit message,
verbose debugging statements during overwrite.
v3: white space fixes
v4: make path dependent on DPCD revision > 1.2
v5: split into function, removed DPCD rev check
v6: add debugging prints for early exit conditions

Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 58 +++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dde92e4af5d3..1190635e4135 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3731,6 +3731,62 @@ intel_dp_link_down(struct intel_encoder *encoder,
 	}
 }
 
+static void
+intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
+{
+	/*
+	 * Prior to DP1.3 the bit represented by
+	 * DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT was reserved.
+	 * if it is set DP_DPCD_REV at 0000h could be at a value less than
+	 * the true capability of the panel. The only way to check is to
+	 * then compare 0000h and 2200h.
+	 */
+	if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
+	    DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) {
+		uint8_t dpcd_ext[6];
+
+		DRM_DEBUG_KMS("DPCD: Extended Receiver Capability Field Present, accessing 02200h through 022FFh\n");
+
+		if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
+				     &dpcd_ext, sizeof(dpcd_ext)) < 0) {
+			DRM_ERROR("DPCD failed read at extended capabilities\n");
+			return;
+		}
+
+		if (intel_dp->dpcd[DP_DPCD_REV] > dpcd_ext[DP_DPCD_REV]) {
+			DRM_DEBUG_KMS("DPCD extended DPCD rev less than base DPCD rev\n");
+			return;
+		}
+
+		if (memcmp(&intel_dp->dpcd[DP_DPCD_REV], &dpcd_ext[DP_DPCD_REV],
+			   sizeof(u8))) {
+			DRM_DEBUG_KMS("DPCD: new value for DPCD Revision previous value %2x new value %2x\n",
+				      intel_dp->dpcd[DP_DPCD_REV],
+				      dpcd_ext[DP_DPCD_REV]);
+			memcpy(&intel_dp->dpcd[DP_DPCD_REV],
+			       &dpcd_ext[DP_DPCD_REV], sizeof(u8));
+		}
+		if (memcmp(&intel_dp->dpcd[DP_MAX_LINK_RATE],
+			   &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8))) {
+			DRM_DEBUG_KMS("DPCD: new value for DPCD Max Link Rate previous value %2x new value %2x\n",
+				      intel_dp->dpcd[DP_MAX_LINK_RATE],
+				      dpcd_ext[DP_MAX_LINK_RATE]);
+			memcpy(&intel_dp->dpcd[DP_MAX_LINK_RATE],
+			       &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8));
+		}
+		if (memcmp(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
+			   &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT], sizeof(u8))) {
+			DRM_DEBUG_KMS("DPCD: new value for DPCD Downstream Port Present  previous value %2x new value %2x\n",
+				      intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
+				      dpcd_ext[DP_DOWNSTREAMPORT_PRESENT]);
+			memcpy(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
+			       &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT],
+			       sizeof(u8));
+		}
+	}
+}
+
+
 bool
 intel_dp_read_dpcd(struct intel_dp *intel_dp)
 {
@@ -3738,6 +3794,8 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
 			     sizeof(intel_dp->dpcd)) < 0)
 		return false; /* aux transfer failed */
 
+	intel_dp_extended_receiver_capabilities(intel_dp);
+
 	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);
 
 	return intel_dp->dpcd[DP_DPCD_REV] != 0;
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-09-13 21:17 [PATCH] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT matthew.s.atwood
@ 2018-09-13 22:12 ` Patchwork
  2018-09-13 22:32 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-09-13 22:12 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
URL   : https://patchwork.freedesktop.org/series/49669/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
dd7da1191c62 drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
-:98: CHECK:LINE_SPACING: Please don't use multiple blank lines
#98: FILE: drivers/gpu/drm/i915/intel_dp.c:3916:
+
+

total: 0 errors, 0 warnings, 1 checks, 70 lines checked

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-09-13 21:17 [PATCH] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT matthew.s.atwood
  2018-09-13 22:12 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-09-13 22:32 ` Patchwork
  2018-09-14  1:12 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-09-13 22:32 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
URL   : https://patchwork.freedesktop.org/series/49669/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4820 -> Patchwork_10179 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

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

    igt@kms_psr@primary_page_flip:
      fi-kbl-7560u:       PASS -> FAIL (fdo#107336)

    
    ==== Possible fixes ====

    igt@drv_module_reload@basic-reload:
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-WARN (fdo#102614) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-a:
      fi-ilk-650:         DMESG-WARN (fdo#106387) -> PASS

    igt@pm_rpm@module-reload:
      {fi-cnl-u}:         FAIL -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718


== Participating hosts (49 -> 45) ==

  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4820 -> Patchwork_10179

  CI_DRM_4820: a804643cba4646e18e0e31c9363fdd3c92dca3c0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10179: dd7da1191c62804f60c30154b1446a44aafb44d1 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

dd7da1191c62 drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT

== Logs ==

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-09-13 21:17 [PATCH] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT matthew.s.atwood
  2018-09-13 22:12 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
  2018-09-13 22:32 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-09-14  1:12 ` Patchwork
  2018-09-18 21:19 ` [PATCH] " Manasi Navare
  2018-09-26  9:42 ` Jani Nikula
  4 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2018-09-14  1:12 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
URL   : https://patchwork.freedesktop.org/series/49669/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4820_full -> Patchwork_10179_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10179_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10179_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_10179_full:

  === IGT changes ===

    ==== Warnings ====

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

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

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

    igt@kms_frontbuffer_tracking@fbc-modesetfrombusy:
      shard-glk:          PASS -> INCOMPLETE (fdo#103359, k.org#198133)

    
    ==== Possible fixes ====

    igt@kms_fbcon_fbt@fbc-suspend:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-pgflip-blt:
      shard-glk:          DMESG-FAIL -> PASS

    igt@kms_plane_scaling@pipe-b-scaler-with-rotation:
      shard-glk:          DMESG-WARN (fdo#105763) -> PASS +3

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

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  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 (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4820 -> Patchwork_10179

  CI_DRM_4820: a804643cba4646e18e0e31c9363fdd3c92dca3c0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4640: 9a8da36e708f9ed15b20689dfe305e41f9a19008 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10179: dd7da1191c62804f60c30154b1446a44aafb44d1 @ 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_10179/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-09-13 21:17 [PATCH] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT matthew.s.atwood
                   ` (2 preceding siblings ...)
  2018-09-14  1:12 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-09-18 21:19 ` Manasi Navare
  2018-09-18 21:36   ` Rodrigo Vivi
  2018-09-26  9:42 ` Jani Nikula
  4 siblings, 1 reply; 8+ messages in thread
From: Manasi Navare @ 2018-09-18 21:19 UTC (permalink / raw)
  To: matthew.s.atwood; +Cc: intel-gfx, rodrigo.vivi

Thanks for the patch. I have tested this on DP 1.4 sink device
and it works properly to read the DPCDs from different offset and
use the true capabilities. Without this patch, the sink behaves as a
legacy DP 1.2 sink.

So with that:
Tested-by: Manasi Navare <manasi.d.navare@intel.com>
Acked-by: Manasi Navare <manasi.d.navare@intel.com>

Manasi

On Thu, Sep 13, 2018 at 02:17:20PM -0700, matthew.s.atwood@intel.com wrote:
> From: Matt Atwood <matthew.s.atwood@intel.com>
> 
> According to DP spec (2.9.3.1 of DP 1.4) if
> EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in DPCD
> 02200h through 0220Fh shall contain the DPRX's true capability. These
> values will match 00000h through 0000Fh, except for DPCD_REV,
> MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT.
> 
> Read from DPCD once for all 3 values as this is an expensive operation.
> Spec mentions that all of address space 02200h through 0220Fh should
> contain the right information however currently only 3 values can
> differ.
> 
> There is no address space in the intel_dp->dpcd struct for addresses
> 02200h through 0220Fh, and since so much of the data is a identical,
> simply overwrite the values stored in 00000h through 0000Fh with the
> values that can be overwritten from addresses 02200h through 0220Fh.
> 
> This patch helps with backward compatibility for devices pre DP1.3.
> 
> v2: read only dpcd values which can be affected, remove incorrect check,
> split into drm include changes into separate patch, commit message,
> verbose debugging statements during overwrite.
> v3: white space fixes
> v4: make path dependent on DPCD revision > 1.2
> v5: split into function, removed DPCD rev check
> v6: add debugging prints for early exit conditions
> 
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 58 +++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index dde92e4af5d3..1190635e4135 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3731,6 +3731,62 @@ intel_dp_link_down(struct intel_encoder *encoder,
>  	}
>  }
>  
> +static void
> +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> +{
> +	/*
> +	 * Prior to DP1.3 the bit represented by
> +	 * DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT was reserved.
> +	 * if it is set DP_DPCD_REV at 0000h could be at a value less than
> +	 * the true capability of the panel. The only way to check is to
> +	 * then compare 0000h and 2200h.
> +	 */
> +	if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> +	    DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) {
> +		uint8_t dpcd_ext[6];
> +
> +		DRM_DEBUG_KMS("DPCD: Extended Receiver Capability Field Present, accessing 02200h through 022FFh\n");
> +
> +		if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> +				     &dpcd_ext, sizeof(dpcd_ext)) < 0) {
> +			DRM_ERROR("DPCD failed read at extended capabilities\n");
> +			return;
> +		}
> +
> +		if (intel_dp->dpcd[DP_DPCD_REV] > dpcd_ext[DP_DPCD_REV]) {
> +			DRM_DEBUG_KMS("DPCD extended DPCD rev less than base DPCD rev\n");
> +			return;
> +		}
> +
> +		if (memcmp(&intel_dp->dpcd[DP_DPCD_REV], &dpcd_ext[DP_DPCD_REV],
> +			   sizeof(u8))) {
> +			DRM_DEBUG_KMS("DPCD: new value for DPCD Revision previous value %2x new value %2x\n",
> +				      intel_dp->dpcd[DP_DPCD_REV],
> +				      dpcd_ext[DP_DPCD_REV]);
> +			memcpy(&intel_dp->dpcd[DP_DPCD_REV],
> +			       &dpcd_ext[DP_DPCD_REV], sizeof(u8));
> +		}
> +		if (memcmp(&intel_dp->dpcd[DP_MAX_LINK_RATE],
> +			   &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8))) {
> +			DRM_DEBUG_KMS("DPCD: new value for DPCD Max Link Rate previous value %2x new value %2x\n",
> +				      intel_dp->dpcd[DP_MAX_LINK_RATE],
> +				      dpcd_ext[DP_MAX_LINK_RATE]);
> +			memcpy(&intel_dp->dpcd[DP_MAX_LINK_RATE],
> +			       &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8));
> +		}
> +		if (memcmp(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> +			   &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT], sizeof(u8))) {
> +			DRM_DEBUG_KMS("DPCD: new value for DPCD Downstream Port Present  previous value %2x new value %2x\n",
> +				      intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> +				      dpcd_ext[DP_DOWNSTREAMPORT_PRESENT]);
> +			memcpy(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> +			       &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT],
> +			       sizeof(u8));
> +		}
> +	}
> +}
> +
> +
>  bool
>  intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  {
> @@ -3738,6 +3794,8 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  			     sizeof(intel_dp->dpcd)) < 0)
>  		return false; /* aux transfer failed */
>  
> +	intel_dp_extended_receiver_capabilities(intel_dp);
> +
>  	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);
>  
>  	return intel_dp->dpcd[DP_DPCD_REV] != 0;
> -- 
> 2.17.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-09-18 21:19 ` [PATCH] " Manasi Navare
@ 2018-09-18 21:36   ` Rodrigo Vivi
  0 siblings, 0 replies; 8+ messages in thread
From: Rodrigo Vivi @ 2018-09-18 21:36 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

On Tue, Sep 18, 2018 at 02:19:17PM -0700, Manasi Navare wrote:
> Thanks for the patch. I have tested this on DP 1.4 sink device
> and it works properly to read the DPCDs from different offset and
> use the true capabilities. Without this patch, the sink behaves as a
> legacy DP 1.2 sink.
> 
> So with that:
> Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> Acked-by: Manasi Navare <manasi.d.navare@intel.com>

thanks

also:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Manasi
> 
> On Thu, Sep 13, 2018 at 02:17:20PM -0700, matthew.s.atwood@intel.com wrote:
> > From: Matt Atwood <matthew.s.atwood@intel.com>
> > 
> > According to DP spec (2.9.3.1 of DP 1.4) if
> > EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in DPCD
> > 02200h through 0220Fh shall contain the DPRX's true capability. These
> > values will match 00000h through 0000Fh, except for DPCD_REV,
> > MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT.
> > 
> > Read from DPCD once for all 3 values as this is an expensive operation.
> > Spec mentions that all of address space 02200h through 0220Fh should
> > contain the right information however currently only 3 values can
> > differ.
> > 
> > There is no address space in the intel_dp->dpcd struct for addresses
> > 02200h through 0220Fh, and since so much of the data is a identical,
> > simply overwrite the values stored in 00000h through 0000Fh with the
> > values that can be overwritten from addresses 02200h through 0220Fh.
> > 
> > This patch helps with backward compatibility for devices pre DP1.3.
> > 
> > v2: read only dpcd values which can be affected, remove incorrect check,
> > split into drm include changes into separate patch, commit message,
> > verbose debugging statements during overwrite.
> > v3: white space fixes
> > v4: make path dependent on DPCD revision > 1.2
> > v5: split into function, removed DPCD rev check
> > v6: add debugging prints for early exit conditions
> > 
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 58 +++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index dde92e4af5d3..1190635e4135 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3731,6 +3731,62 @@ intel_dp_link_down(struct intel_encoder *encoder,
> >  	}
> >  }
> >  
> > +static void
> > +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> > +{
> > +	/*
> > +	 * Prior to DP1.3 the bit represented by
> > +	 * DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT was reserved.
> > +	 * if it is set DP_DPCD_REV at 0000h could be at a value less than
> > +	 * the true capability of the panel. The only way to check is to
> > +	 * then compare 0000h and 2200h.
> > +	 */
> > +	if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> > +	    DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) {
> > +		uint8_t dpcd_ext[6];
> > +
> > +		DRM_DEBUG_KMS("DPCD: Extended Receiver Capability Field Present, accessing 02200h through 022FFh\n");
> > +
> > +		if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> > +				     &dpcd_ext, sizeof(dpcd_ext)) < 0) {
> > +			DRM_ERROR("DPCD failed read at extended capabilities\n");
> > +			return;
> > +		}
> > +
> > +		if (intel_dp->dpcd[DP_DPCD_REV] > dpcd_ext[DP_DPCD_REV]) {
> > +			DRM_DEBUG_KMS("DPCD extended DPCD rev less than base DPCD rev\n");
> > +			return;
> > +		}
> > +
> > +		if (memcmp(&intel_dp->dpcd[DP_DPCD_REV], &dpcd_ext[DP_DPCD_REV],
> > +			   sizeof(u8))) {
> > +			DRM_DEBUG_KMS("DPCD: new value for DPCD Revision previous value %2x new value %2x\n",
> > +				      intel_dp->dpcd[DP_DPCD_REV],
> > +				      dpcd_ext[DP_DPCD_REV]);
> > +			memcpy(&intel_dp->dpcd[DP_DPCD_REV],
> > +			       &dpcd_ext[DP_DPCD_REV], sizeof(u8));
> > +		}
> > +		if (memcmp(&intel_dp->dpcd[DP_MAX_LINK_RATE],
> > +			   &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8))) {
> > +			DRM_DEBUG_KMS("DPCD: new value for DPCD Max Link Rate previous value %2x new value %2x\n",
> > +				      intel_dp->dpcd[DP_MAX_LINK_RATE],
> > +				      dpcd_ext[DP_MAX_LINK_RATE]);
> > +			memcpy(&intel_dp->dpcd[DP_MAX_LINK_RATE],
> > +			       &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8));
> > +		}
> > +		if (memcmp(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> > +			   &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT], sizeof(u8))) {
> > +			DRM_DEBUG_KMS("DPCD: new value for DPCD Downstream Port Present  previous value %2x new value %2x\n",
> > +				      intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> > +				      dpcd_ext[DP_DOWNSTREAMPORT_PRESENT]);
> > +			memcpy(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> > +			       &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT],
> > +			       sizeof(u8));
> > +		}
> > +	}
> > +}
> > +
> > +
> >  bool
> >  intel_dp_read_dpcd(struct intel_dp *intel_dp)
> >  {
> > @@ -3738,6 +3794,8 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
> >  			     sizeof(intel_dp->dpcd)) < 0)
> >  		return false; /* aux transfer failed */
> >  
> > +	intel_dp_extended_receiver_capabilities(intel_dp);
> > +
> >  	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);
> >  
> >  	return intel_dp->dpcd[DP_DPCD_REV] != 0;
> > -- 
> > 2.17.1
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-09-13 21:17 [PATCH] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT matthew.s.atwood
                   ` (3 preceding siblings ...)
  2018-09-18 21:19 ` [PATCH] " Manasi Navare
@ 2018-09-26  9:42 ` Jani Nikula
  2018-09-26  9:57   ` Manasi Navare
  4 siblings, 1 reply; 8+ messages in thread
From: Jani Nikula @ 2018-09-26  9:42 UTC (permalink / raw)
  To: matthew.s.atwood, intel-gfx, manasi.d.navare, rodrigo.vivi

On Thu, 13 Sep 2018, matthew.s.atwood@intel.com wrote:
> From: Matt Atwood <matthew.s.atwood@intel.com>
>
> According to DP spec (2.9.3.1 of DP 1.4) if
> EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in DPCD
> 02200h through 0220Fh shall contain the DPRX's true capability. These
> values will match 00000h through 0000Fh, except for DPCD_REV,
> MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT.
>
> Read from DPCD once for all 3 values as this is an expensive operation.
> Spec mentions that all of address space 02200h through 0220Fh should
> contain the right information however currently only 3 values can
> differ.
>
> There is no address space in the intel_dp->dpcd struct for addresses
> 02200h through 0220Fh, and since so much of the data is a identical,
> simply overwrite the values stored in 00000h through 0000Fh with the
> values that can be overwritten from addresses 02200h through 0220Fh.
>
> This patch helps with backward compatibility for devices pre DP1.3.
>
> v2: read only dpcd values which can be affected, remove incorrect check,
> split into drm include changes into separate patch, commit message,
> verbose debugging statements during overwrite.
> v3: white space fixes
> v4: make path dependent on DPCD revision > 1.2
> v5: split into function, removed DPCD rev check
> v6: add debugging prints for early exit conditions
>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>

So I did the backmerge to get the DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT
to dinq, and was about to merge... but I gotta nitpick here.

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 58 +++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index dde92e4af5d3..1190635e4135 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3731,6 +3731,62 @@ intel_dp_link_down(struct intel_encoder *encoder,
>  	}
>  }
>  
> +static void
> +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> +{
> +	/*
> +	 * Prior to DP1.3 the bit represented by
> +	 * DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT was reserved.
> +	 * if it is set DP_DPCD_REV at 0000h could be at a value less than
> +	 * the true capability of the panel. The only way to check is to
> +	 * then compare 0000h and 2200h.
> +	 */
> +	if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> +	    DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) {

Reverse the condition, early exit, and make the rest of the function so
much easier to read by decreasing indent.

> +		uint8_t dpcd_ext[6];

u8

> +
> +		DRM_DEBUG_KMS("DPCD: Extended Receiver Capability Field Present, accessing 02200h through 022FFh\n");

Logging the offsets is excessive.

> +
> +		if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> +				     &dpcd_ext, sizeof(dpcd_ext)) < 0) {

!= sizeof(dpcd_ext) is more accurate.

> +			DRM_ERROR("DPCD failed read at extended capabilities\n");
> +			return;
> +		}
> +
> +		if (intel_dp->dpcd[DP_DPCD_REV] > dpcd_ext[DP_DPCD_REV]) {
> +			DRM_DEBUG_KMS("DPCD extended DPCD rev less than base DPCD rev\n");
> +			return;
> +		}
> +
> +		if (memcmp(&intel_dp->dpcd[DP_DPCD_REV], &dpcd_ext[DP_DPCD_REV],
> +			   sizeof(u8))) {
> +			DRM_DEBUG_KMS("DPCD: new value for DPCD Revision previous value %2x new value %2x\n",
> +				      intel_dp->dpcd[DP_DPCD_REV],
> +				      dpcd_ext[DP_DPCD_REV]);
> +			memcpy(&intel_dp->dpcd[DP_DPCD_REV],
> +			       &dpcd_ext[DP_DPCD_REV], sizeof(u8));
> +		}

I presume earlier versions of the patch had memcmp/memcpy on the whole
dpcd_ext, but if for some reason not present in the changelog you really
need to do this piecemeal, please do not use memcmp/memcpy on
sizeof(u8)! Ditto below.

BR,
Jani.

> +		if (memcmp(&intel_dp->dpcd[DP_MAX_LINK_RATE],
> +			   &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8))) {
> +			DRM_DEBUG_KMS("DPCD: new value for DPCD Max Link Rate previous value %2x new value %2x\n",
> +				      intel_dp->dpcd[DP_MAX_LINK_RATE],
> +				      dpcd_ext[DP_MAX_LINK_RATE]);
> +			memcpy(&intel_dp->dpcd[DP_MAX_LINK_RATE],
> +			       &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8));
> +		}
> +		if (memcmp(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> +			   &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT], sizeof(u8))) {
> +			DRM_DEBUG_KMS("DPCD: new value for DPCD Downstream Port Present  previous value %2x new value %2x\n",
> +				      intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> +				      dpcd_ext[DP_DOWNSTREAMPORT_PRESENT]);
> +			memcpy(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> +			       &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT],
> +			       sizeof(u8));
> +		}
> +	}
> +}
> +
> +
>  bool
>  intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  {
> @@ -3738,6 +3794,8 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  			     sizeof(intel_dp->dpcd)) < 0)
>  		return false; /* aux transfer failed */
>  
> +	intel_dp_extended_receiver_capabilities(intel_dp);
> +
>  	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);
>  
>  	return intel_dp->dpcd[DP_DPCD_REV] != 0;

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-09-26  9:42 ` Jani Nikula
@ 2018-09-26  9:57   ` Manasi Navare
  0 siblings, 0 replies; 8+ messages in thread
From: Manasi Navare @ 2018-09-26  9:57 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, rodrigo.vivi

Thanks Jani for backmerging the drm patch.

On Wed, Sep 26, 2018 at 12:42:16PM +0300, Jani Nikula wrote:
> On Thu, 13 Sep 2018, matthew.s.atwood@intel.com wrote:
> > From: Matt Atwood <matthew.s.atwood@intel.com>
> >
> > According to DP spec (2.9.3.1 of DP 1.4) if
> > EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in DPCD
> > 02200h through 0220Fh shall contain the DPRX's true capability. These
> > values will match 00000h through 0000Fh, except for DPCD_REV,
> > MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT.
> >
> > Read from DPCD once for all 3 values as this is an expensive operation.
> > Spec mentions that all of address space 02200h through 0220Fh should
> > contain the right information however currently only 3 values can
> > differ.
> >
> > There is no address space in the intel_dp->dpcd struct for addresses
> > 02200h through 0220Fh, and since so much of the data is a identical,
> > simply overwrite the values stored in 00000h through 0000Fh with the
> > values that can be overwritten from addresses 02200h through 0220Fh.
> >
> > This patch helps with backward compatibility for devices pre DP1.3.
> >
> > v2: read only dpcd values which can be affected, remove incorrect check,
> > split into drm include changes into separate patch, commit message,
> > verbose debugging statements during overwrite.
> > v3: white space fixes
> > v4: make path dependent on DPCD revision > 1.2
> > v5: split into function, removed DPCD rev check
> > v6: add debugging prints for early exit conditions
> >
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> 
> So I did the backmerge to get the DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT
> to dinq, and was about to merge... but I gotta nitpick here.
> 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 58 +++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index dde92e4af5d3..1190635e4135 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3731,6 +3731,62 @@ intel_dp_link_down(struct intel_encoder *encoder,
> >  	}
> >  }
> >  
> > +static void
> > +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> > +{
> > +	/*
> > +	 * Prior to DP1.3 the bit represented by
> > +	 * DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT was reserved.
> > +	 * if it is set DP_DPCD_REV at 0000h could be at a value less than
> > +	 * the true capability of the panel. The only way to check is to
> > +	 * then compare 0000h and 2200h.
> > +	 */
> > +	if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> > +	    DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) {
> 
> Reverse the condition, early exit, and make the rest of the function so
> much easier to read by decreasing indent.
> 

Yes that makes sense, so just exit early if DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is not set

> > +		uint8_t dpcd_ext[6];
> 
> u8
> 
> > +
> > +		DRM_DEBUG_KMS("DPCD: Extended Receiver Capability Field Present, accessing 02200h through 022FFh\n");
> 
> Logging the offsets is excessive.
> 
> > +
> > +		if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> > +				     &dpcd_ext, sizeof(dpcd_ext)) < 0) {
> 
> != sizeof(dpcd_ext) is more accurate.
>

+1 for this change.
 
> > +			DRM_ERROR("DPCD failed read at extended capabilities\n");
> > +			return;
> > +		}
> > +
> > +		if (intel_dp->dpcd[DP_DPCD_REV] > dpcd_ext[DP_DPCD_REV]) {
> > +			DRM_DEBUG_KMS("DPCD extended DPCD rev less than base DPCD rev\n");
> > +			return;
> > +		}
> > +
> > +		if (memcmp(&intel_dp->dpcd[DP_DPCD_REV], &dpcd_ext[DP_DPCD_REV],
> > +			   sizeof(u8))) {
> > +			DRM_DEBUG_KMS("DPCD: new value for DPCD Revision previous value %2x new value %2x\n",
> > +				      intel_dp->dpcd[DP_DPCD_REV],
> > +				      dpcd_ext[DP_DPCD_REV]);
> > +			memcpy(&intel_dp->dpcd[DP_DPCD_REV],
> > +			       &dpcd_ext[DP_DPCD_REV], sizeof(u8));
> > +		}
> 
> I presume earlier versions of the patch had memcmp/memcpy on the whole
> dpcd_ext, but if for some reason not present in the changelog you really
> need to do this piecemeal, please do not use memcmp/memcpy on
> sizeof(u8)! Ditto below.
> 
> BR,
> Jani.
> 
> > +		if (memcmp(&intel_dp->dpcd[DP_MAX_LINK_RATE],
> > +			   &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8))) {
> > +			DRM_DEBUG_KMS("DPCD: new value for DPCD Max Link Rate previous value %2x new value %2x\n",
> > +				      intel_dp->dpcd[DP_MAX_LINK_RATE],
> > +				      dpcd_ext[DP_MAX_LINK_RATE]);
> > +			memcpy(&intel_dp->dpcd[DP_MAX_LINK_RATE],
> > +			       &dpcd_ext[DP_MAX_LINK_RATE], sizeof(u8));

The other feedback here from Ville was that, the value of DP_MX_LINK_RATE set here
should be validated against the valid values in drm_dp_bw_code_to_link_rate() otherwise if the faulty panels set it to
a bad value, the driver will default to lowest rate of RBR/1.62Gbps

Manasi

> > +		}
> > +		if (memcmp(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> > +			   &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT], sizeof(u8))) {
> > +			DRM_DEBUG_KMS("DPCD: new value for DPCD Downstream Port Present  previous value %2x new value %2x\n",
> > +				      intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> > +				      dpcd_ext[DP_DOWNSTREAMPORT_PRESENT]);
> > +			memcpy(&intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
> > +			       &dpcd_ext[DP_DOWNSTREAMPORT_PRESENT],
> > +			       sizeof(u8));
> > +		}
> > +	}
> > +}
> > +
> > +
> >  bool
> >  intel_dp_read_dpcd(struct intel_dp *intel_dp)
> >  {
> > @@ -3738,6 +3794,8 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
> >  			     sizeof(intel_dp->dpcd)) < 0)
> >  		return false; /* aux transfer failed */
> >  
> > +	intel_dp_extended_receiver_capabilities(intel_dp);
> > +
> >  	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);
> >  
> >  	return intel_dp->dpcd[DP_DPCD_REV] != 0;
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-09-26 10:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-13 21:17 [PATCH] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT matthew.s.atwood
2018-09-13 22:12 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-09-13 22:32 ` ✓ Fi.CI.BAT: success " Patchwork
2018-09-14  1:12 ` ✓ Fi.CI.IGT: " Patchwork
2018-09-18 21:19 ` [PATCH] " Manasi Navare
2018-09-18 21:36   ` Rodrigo Vivi
2018-09-26  9:42 ` Jani Nikula
2018-09-26  9:57   ` Manasi Navare

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.