All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
@ 2018-11-27 20:51 Manasi Navare
  2018-11-27 21:44 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev2) Patchwork
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Manasi Navare @ 2018-11-27 20:51 UTC (permalink / raw)
  To: intel-gfx

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
v7 (From Manasi):
* Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
* Exit early (Jani N)

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
Tested-by: Manasi Navare <manasi.d.navare@intel.com>
Acked-by: Manasi Navare <manasi.d.navare@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 41 +++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 70ae3d57316b..a9eb14a4ab27 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3802,6 +3802,45 @@ intel_dp_link_down(struct intel_encoder *encoder,
 	}
 }
 
+static void
+intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
+{
+	u8 dpcd_ext[6];
+
+	/*
+	 * 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))
+		return;
+
+	DRM_DEBUG_KMS("DPCD: Reading extended receiver capabilities\n");
+
+	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
+			     &dpcd_ext, sizeof(dpcd_ext)) != sizeof(dpcd_ext)) {
+		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, dpcd_ext, sizeof(dpcd_ext))) {
+		DRM_DEBUG_KMS("Extended Receiver Cap DPCD match the base DPCD\n");
+		return;
+	}
+
+	DRM_DEBUG_KMS("Base DPCD: %*ph\n", (int)sizeof(dpcd_ext), intel_dp->dpcd);
+	DRM_DEBUG_KMS("Extended Receiver Cap DPCD: %*ph\n",
+		      (int)sizeof(dpcd_ext), dpcd_ext);
+	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
+}
+
+
 bool
 intel_dp_read_dpcd(struct intel_dp *intel_dp)
 {
@@ -3809,6 +3848,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.19.1

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

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

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev2)
  2018-11-27 20:51 [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
@ 2018-11-27 21:44 ` Patchwork
  2018-11-27 22:10 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-11-27 21:44 UTC (permalink / raw)
  To: Atwood, Matthew S; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

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

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

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

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

* ✓ Fi.CI.BAT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev2)
  2018-11-27 20:51 [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
  2018-11-27 21:44 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev2) Patchwork
@ 2018-11-27 22:10 ` Patchwork
  2018-11-28  9:09 ` [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Jani Nikula
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-11-27 22:10 UTC (permalink / raw)
  To: Atwood, Matthew S; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_5212 -> Patchwork_10921 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ctx_create@basic-files:
      fi-bsw-kefka:       PASS -> FAIL ([fdo#108656])

    igt@gem_exec_suspend@basic-s4-devices:
      fi-ivb-3520m:       PASS -> FAIL ([fdo#108880])
      fi-blb-e6850:       PASS -> INCOMPLETE ([fdo#107718])

    igt@i915_selftest@live_hangcheck:
      fi-bwr-2160:        PASS -> DMESG-FAIL ([fdo#108735])

    igt@kms_flip@basic-flip-vs-modeset:
      fi-skl-6700hq:      PASS -> DMESG-WARN ([fdo#105998])

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

    
    ==== Possible fixes ====

    igt@gem_ctx_create@basic-files:
      fi-bsw-n3050:       DMESG-FAIL ([fdo#108656]) -> PASS

    
  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#108656 https://bugs.freedesktop.org/show_bug.cgi?id=108656
  fdo#108735 https://bugs.freedesktop.org/show_bug.cgi?id=108735
  fdo#108880 https://bugs.freedesktop.org/show_bug.cgi?id=108880


== Participating hosts (51 -> 43) ==

  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-gdg-551 fi-icl-u3 


== Build changes ==

    * Linux: CI_DRM_5212 -> Patchwork_10921

  CI_DRM_5212: db8d567e7cf70aeca866e85972078cd5c9b59cfb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4734: 0a961239c27bfbd60c045e6255b2970d4bf84411 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10921: d147afa0255b8eb26dfb36632bb788ef4a118f92 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d147afa0255b drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT

== Logs ==

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

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

* Re: [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-11-27 20:51 [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
  2018-11-27 21:44 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev2) Patchwork
  2018-11-27 22:10 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-11-28  9:09 ` Jani Nikula
  2018-11-28 19:18   ` Manasi Navare
  2018-11-28 10:02 ` ✓ Fi.CI.IGT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev2) Patchwork
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Jani Nikula @ 2018-11-28  9:09 UTC (permalink / raw)
  To: Manasi Navare, intel-gfx

On Tue, 27 Nov 2018, Manasi Navare <manasi.d.navare@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
> v7 (From Manasi):
> * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
> * Exit early (Jani N)
>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 41 +++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 70ae3d57316b..a9eb14a4ab27 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3802,6 +3802,45 @@ intel_dp_link_down(struct intel_encoder *encoder,
>  	}
>  }
>  
> +static void
> +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> +{
> +	u8 dpcd_ext[6];
> +
> +	/*
> +	 * 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))
> +		return;
> +
> +	DRM_DEBUG_KMS("DPCD: Reading extended receiver capabilities\n");

Superfluous debug logging.

> +
> +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> +			     &dpcd_ext, sizeof(dpcd_ext)) != sizeof(dpcd_ext)) {
> +		DRM_ERROR("DPCD failed read at extended capabilities\n");

Most of our dpcd failures are logged using DRM_DEBUG_KMS. The ones that
log DRM_ERROR seem to be very recent additions deviating from the debug
loggin practice. There isn't much the user can do, really.

> +		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");

Okay, seems like a rare event.

> +		return;
> +	}
> +	if (!memcmp(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext))) {
> +		DRM_DEBUG_KMS("Extended Receiver Cap DPCD match the base DPCD\n");

I don't think this debug logging is needed.

> +		return;
> +	}
> +
> +	DRM_DEBUG_KMS("Base DPCD: %*ph\n", (int)sizeof(dpcd_ext), intel_dp->dpcd);

Using sizeof(dpcd_ext) when printing something else is a red flag. You
could log the whole dpcd here.

	DRM_DEBUG_KMS("DPCD: %*ph (base)\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);

> +	DRM_DEBUG_KMS("Extended Receiver Cap DPCD: %*ph\n",
> +		      (int)sizeof(dpcd_ext), dpcd_ext);

The caller will log the *updated* DPCD right after this returns, you
don't need to log dpcd_ext.

> +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
> +}
> +
> +

Superfluous newline.

>  bool
>  intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  {
> @@ -3809,6 +3848,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);

One other alternative is to have
intel_dp_extended_receiver_capabilities() return true if the cap exists
and is different from current DPCD, and *all* DPCD logging would be done
here. Both the old and the new. Like so:

	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);
	if (intel_dp_extended_receiver_capabilities(intel_dp))
		DRM_DEBUG_KMS("DPCD: %*ph\n (ext)", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);

BR,
Jani.

>  
>  	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] 18+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev2)
  2018-11-27 20:51 [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
                   ` (2 preceding siblings ...)
  2018-11-28  9:09 ` [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Jani Nikula
@ 2018-11-28 10:02 ` Patchwork
  2018-11-29 18:34 ` [PATCH v8] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-11-28 10:02 UTC (permalink / raw)
  To: Atwood, Matthew S; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_5212_full -> Patchwork_10921_full =

== Summary - WARNING ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ctx_isolation@rcs0-s3:
      shard-skl:          PASS -> INCOMPLETE ([fdo#104108], [fdo#107773])

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
      shard-snb:          PASS -> DMESG-WARN ([fdo#107956])

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
      shard-glk:          NOTRUN -> DMESG-WARN ([fdo#107956]) +1

    igt@kms_cursor_crc@cursor-256x256-random:
      shard-glk:          PASS -> FAIL ([fdo#103232]) +2

    igt@kms_cursor_crc@cursor-256x256-suspend:
      shard-apl:          PASS -> FAIL ([fdo#103191], [fdo#103232])

    igt@kms_cursor_crc@cursor-256x85-sliding:
      shard-apl:          PASS -> FAIL ([fdo#103232]) +2

    igt@kms_cursor_crc@cursor-64x21-onscreen:
      {shard-iclb}:       NOTRUN -> FAIL ([fdo#103232])

    igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-untiled:
      shard-skl:          PASS -> FAIL ([fdo#103184])

    igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-untiled:
      {shard-iclb}:       PASS -> WARN ([fdo#108336]) +3

    igt@kms_flip@2x-plain-flip-ts-check-interruptible:
      shard-glk:          PASS -> FAIL ([fdo#100368])

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-skl:          PASS -> FAIL ([fdo#102887])

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-render:
      {shard-iclb}:       PASS -> DMESG-FAIL ([fdo#107720], [fdo#107724])

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

    igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
      {shard-iclb}:       PASS -> DMESG-FAIL ([fdo#107724]) +9

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-pwrite:
      shard-skl:          NOTRUN -> FAIL ([fdo#103167])

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-gtt:
      {shard-iclb}:       NOTRUN -> FAIL ([fdo#103167])

    igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-onoff:
      {shard-iclb}:       PASS -> DMESG-WARN ([fdo#107724], [fdo#108336]) +25

    igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-pwrite:
      {shard-iclb}:       PASS -> FAIL ([fdo#103167]) +1

    igt@kms_plane@pixel-format-pipe-c-planes:
      shard-skl:          NOTRUN -> DMESG-WARN ([fdo#106885])

    igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
      {shard-iclb}:       PASS -> DMESG-FAIL ([fdo#103166], [fdo#107724])

    igt@kms_plane@plane-position-covered-pipe-c-planes:
      shard-apl:          PASS -> FAIL ([fdo#103166]) +2

    igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
      shard-skl:          NOTRUN -> FAIL ([fdo#107815], [fdo#108145])

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

    igt@kms_psr@no_drrs:
      {shard-iclb}:       PASS -> FAIL ([fdo#108341])

    igt@kms_vblank@pipe-a-ts-continuation-idle:
      {shard-iclb}:       PASS -> DMESG-WARN ([fdo#107724]) +39

    igt@kms_vblank@pipe-b-ts-continuation-suspend:
      shard-kbl:          PASS -> INCOMPLETE ([fdo#103665])

    igt@pm_rpm@gem-execbuf:
      shard-skl:          PASS -> INCOMPLETE ([fdo#107803], [fdo#107807])

    igt@pm_rpm@legacy-planes:
      {shard-iclb}:       PASS -> DMESG-WARN ([fdo#108654])

    igt@pm_rpm@modeset-lpsp-stress-no-wait:
      {shard-iclb}:       PASS -> INCOMPLETE ([fdo#108840])

    igt@pm_rpm@universal-planes-dpms:
      shard-skl:          PASS -> INCOMPLETE ([fdo#107807])

    
    ==== Possible fixes ====

    igt@i915_suspend@fence-restore-untiled:
      shard-glk:          INCOMPLETE ([fdo#103359], [k.org#198133]) -> PASS

    igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
      shard-glk:          FAIL ([fdo#108145]) -> PASS

    igt@kms_cursor_crc@cursor-256x256-dpms:
      shard-apl:          FAIL ([fdo#103232]) -> PASS +1

    igt@kms_cursor_crc@cursor-256x256-offscreen:
      shard-skl:          FAIL ([fdo#103232]) -> PASS

    igt@kms_cursor_legacy@flip-vs-cursor-legacy:
      shard-glk:          FAIL ([fdo#102670]) -> PASS

    igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-untiled:
      {shard-iclb}:       WARN ([fdo#108336]) -> PASS +3

    igt@kms_draw_crc@draw-method-xrgb8888-pwrite-xtiled:
      shard-skl:          FAIL ([fdo#107791]) -> PASS

    igt@kms_draw_crc@fill-fb:
      shard-skl:          FAIL ([fdo#103184]) -> PASS +1

    igt@kms_flip@flip-vs-expired-vblank:
      shard-apl:          INCOMPLETE ([fdo#103927]) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-render:
      shard-skl:          FAIL ([fdo#105682]) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-blt:
      shard-skl:          FAIL ([fdo#103167]) -> PASS +2

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
      shard-glk:          FAIL ([fdo#103167]) -> PASS +3

    igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
      {shard-iclb}:       DMESG-FAIL ([fdo#107724]) -> PASS +3

    igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen:
      {shard-iclb}:       FAIL ([fdo#103167]) -> PASS +3

    igt@kms_pipe_crc_basic@read-crc-pipe-a:
      shard-skl:          FAIL ([fdo#107362]) -> PASS

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

    igt@kms_plane_lowres@pipe-c-tiling-none:
      {shard-iclb}:       DMESG-WARN ([fdo#107724], [fdo#108336]) -> PASS +6

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

    igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
      {shard-iclb}:       FAIL ([fdo#103166]) -> PASS +2

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

    igt@kms_vblank@pipe-c-wait-forked-hang:
      {shard-iclb}:       DMESG-WARN ([fdo#107724]) -> PASS +11

    igt@pm_rpm@system-suspend-execbuf:
      {shard-iclb}:       INCOMPLETE ([fdo#107713], [fdo#108840]) -> PASS

    igt@pm_rpm@universal-planes:
      {shard-iclb}:       DMESG-WARN ([fdo#108654], [fdo#108756]) -> PASS

    
    ==== Warnings ====

    igt@i915_suspend@shrink:
      shard-hsw:          DMESG-WARN ([fdo#108784]) -> INCOMPLETE ([fdo#103540], [fdo#106886])

    igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
      {shard-iclb}:       DMESG-WARN ([fdo#107724], [fdo#108336]) -> FAIL ([fdo#107725])

    igt@kms_cursor_crc@cursor-128x128-suspend:
      {shard-iclb}:       DMESG-FAIL ([fdo#103232], [fdo#107724]) -> FAIL ([fdo#103232])

    igt@kms_cursor_crc@cursor-256x256-onscreen:
      {shard-iclb}:       DMESG-WARN ([fdo#107724], [fdo#108336]) -> FAIL ([fdo#103232])

    igt@kms_cursor_crc@cursor-256x85-sliding:
      {shard-iclb}:       FAIL ([fdo#103232]) -> DMESG-WARN ([fdo#107724], [fdo#108336]) +1

    igt@kms_fbcon_fbt@psr:
      {shard-iclb}:       FAIL ([fdo#107882]) -> DMESG-FAIL ([fdo#107724])

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-cpu:
      {shard-iclb}:       FAIL ([fdo#103167]) -> DMESG-FAIL ([fdo#107724])

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

  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#106885 https://bugs.freedesktop.org/show_bug.cgi?id=106885
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107713 https://bugs.freedesktop.org/show_bug.cgi?id=107713
  fdo#107720 https://bugs.freedesktop.org/show_bug.cgi?id=107720
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#107725 https://bugs.freedesktop.org/show_bug.cgi?id=107725
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107791 https://bugs.freedesktop.org/show_bug.cgi?id=107791
  fdo#107803 https://bugs.freedesktop.org/show_bug.cgi?id=107803
  fdo#107807 https://bugs.freedesktop.org/show_bug.cgi?id=107807
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#107882 https://bugs.freedesktop.org/show_bug.cgi?id=107882
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108336 https://bugs.freedesktop.org/show_bug.cgi?id=108336
  fdo#108341 https://bugs.freedesktop.org/show_bug.cgi?id=108341
  fdo#108654 https://bugs.freedesktop.org/show_bug.cgi?id=108654
  fdo#108756 https://bugs.freedesktop.org/show_bug.cgi?id=108756
  fdo#108784 https://bugs.freedesktop.org/show_bug.cgi?id=108784
  fdo#108840 https://bugs.freedesktop.org/show_bug.cgi?id=108840
  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 (7 -> 7) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5212 -> Patchwork_10921

  CI_DRM_5212: db8d567e7cf70aeca866e85972078cd5c9b59cfb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4734: 0a961239c27bfbd60c045e6255b2970d4bf84411 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10921: d147afa0255b8eb26dfb36632bb788ef4a118f92 @ 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_10921/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-11-28  9:09 ` [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Jani Nikula
@ 2018-11-28 19:18   ` Manasi Navare
  0 siblings, 0 replies; 18+ messages in thread
From: Manasi Navare @ 2018-11-28 19:18 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Wed, Nov 28, 2018 at 11:09:46AM +0200, Jani Nikula wrote:
> On Tue, 27 Nov 2018, Manasi Navare <manasi.d.navare@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
> > v7 (From Manasi):
> > * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
> > * Exit early (Jani N)
> >
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> > Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 41 +++++++++++++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 70ae3d57316b..a9eb14a4ab27 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3802,6 +3802,45 @@ intel_dp_link_down(struct intel_encoder *encoder,
> >  	}
> >  }
> >  
> > +static void
> > +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> > +{
> > +	u8 dpcd_ext[6];
> > +
> > +	/*
> > +	 * 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))
> > +		return;
> > +
> > +	DRM_DEBUG_KMS("DPCD: Reading extended receiver capabilities\n");
> 
> Superfluous debug logging.

Will get rid of this

> 
> > +
> > +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> > +			     &dpcd_ext, sizeof(dpcd_ext)) != sizeof(dpcd_ext)) {
> > +		DRM_ERROR("DPCD failed read at extended capabilities\n");
> 
> Most of our dpcd failures are logged using DRM_DEBUG_KMS. The ones that
> log DRM_ERROR seem to be very recent additions deviating from the debug
> loggin practice. There isn't much the user can do, really.

Here this change from DEBUG_KMS to DRM_ERROR was as per Rodrigo's comment
on the initial patch. (https://patchwork.freedesktop.org/patch/240452/) 
Also IMO it should be an error since it will give unexpected results as we were
unable to get the true extended capabilities.

> 
> > +		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");
> 
> Okay, seems like a rare event.

Again this check and logging comes from Rodrigo's review comments on the initial patch.

https://patchwork.freedesktop.org/patch/240452/

> 
> > +		return;
> > +	}
> > +	if (!memcmp(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext))) {
> > +		DRM_DEBUG_KMS("Extended Receiver Cap DPCD match the base DPCD\n");
> 
> I don't think this debug logging is needed.

Sure will get rid of this.

> 
> > +		return;
> > +	}
> > +
> > +	DRM_DEBUG_KMS("Base DPCD: %*ph\n", (int)sizeof(dpcd_ext), intel_dp->dpcd);
> 
> Using sizeof(dpcd_ext) when printing something else is a red flag. You
> could log the whole dpcd here.
> 
> 	DRM_DEBUG_KMS("DPCD: %*ph (base)\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);

Yes will do this.

> 
> > +	DRM_DEBUG_KMS("Extended Receiver Cap DPCD: %*ph\n",
> > +		      (int)sizeof(dpcd_ext), dpcd_ext);
> 
> The caller will log the *updated* DPCD right after this returns, you
> don't need to log dpcd_ext.

Okay agreed, I will get rid of this debug print since the caller already prints it.


> 
> > +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
> > +}
> > +
> > +
> 
> Superfluous newline.

Will remove.

> 
> >  bool
> >  intel_dp_read_dpcd(struct intel_dp *intel_dp)
> >  {
> > @@ -3809,6 +3848,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);
> 
> One other alternative is to have
> intel_dp_extended_receiver_capabilities() return true if the cap exists
> and is different from current DPCD, and *all* DPCD logging would be done
> here. Both the old and the new. Like so:
> 
> 	DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);
> 	if (intel_dp_extended_receiver_capabilities(intel_dp))
> 		DRM_DEBUG_KMS("DPCD: %*ph\n (ext)", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd);
>

I like the firts idea better where we print the old DPCD before memcpy and caller prints the DPCD (new/old)
anyways after return so will stick to that.

Manasi
 
> BR,
> Jani.
> 
> >  
> >  	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] 18+ messages in thread

* [PATCH v8] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-11-27 20:51 [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
                   ` (3 preceding siblings ...)
  2018-11-28 10:02 ` ✓ Fi.CI.IGT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev2) Patchwork
@ 2018-11-29 18:34 ` Manasi Navare
  2018-11-29 19:28   ` Rodrigo Vivi
  2018-11-29 22:00   ` [PATCH v9] " Manasi Navare
  2018-11-29 19:07 ` ✓ Fi.CI.BAT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev3) Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 2 replies; 18+ messages in thread
From: Manasi Navare @ 2018-11-29 18:34 UTC (permalink / raw)
  To: intel-gfx

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
v7 (From Manasi):
* Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
* Exit early (Jani N)
v8 (From Manasi):
* Get rid of superfluous debug prints (Jani N)
* Print entire base DPCD before memcpy (Jani N)

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
Tested-by: Manasi Navare <manasi.d.navare@intel.com>
Acked-by: Manasi Navare <manasi.d.navare@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 35 +++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 70ae3d57316b..5698441abe47 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3802,6 +3802,39 @@ intel_dp_link_down(struct intel_encoder *encoder,
 	}
 }
 
+static void
+intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
+{
+	u8 dpcd_ext[6];
+
+	/*
+	 * 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))
+		return;
+
+	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
+			     &dpcd_ext, sizeof(dpcd_ext)) != sizeof(dpcd_ext)) {
+		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, dpcd_ext, sizeof(dpcd_ext)))
+		return;
+
+	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
+		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
+	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
+}
+
 bool
 intel_dp_read_dpcd(struct intel_dp *intel_dp)
 {
@@ -3809,6 +3842,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.19.1

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

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

* ✓ Fi.CI.BAT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev3)
  2018-11-27 20:51 [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
                   ` (4 preceding siblings ...)
  2018-11-29 18:34 ` [PATCH v8] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
@ 2018-11-29 19:07 ` Patchwork
  2018-11-29 23:03 ` ✓ Fi.CI.BAT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev4) Patchwork
  2018-11-30 17:11 ` ✓ Fi.CI.IGT: " Patchwork
  7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-11-29 19:07 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5224 -> Patchwork_10962
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362] +1

  
#### Possible fixes ####

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     FAIL [fdo#103167] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362


Participating hosts (50 -> 44)
------------------------------

  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


Build changes
-------------

    * Linux: CI_DRM_5224 -> Patchwork_10962

  CI_DRM_5224: 67ee0d0da79f5b32636d496fd2127da1eecf6262 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10962: e037e3a1eb538b0337c5334507e875f96d4e189d @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e037e3a1eb53 drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT

== Logs ==

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

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

* Re: [PATCH v8] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-11-29 18:34 ` [PATCH v8] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
@ 2018-11-29 19:28   ` Rodrigo Vivi
  2018-11-29 19:43     ` Manasi Navare
  2018-11-29 22:00   ` [PATCH v9] " Manasi Navare
  1 sibling, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2018-11-29 19:28 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

On Thu, Nov 29, 2018 at 10:34:05AM -0800, Manasi Navare 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
> v7 (From Manasi):
> * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
> * Exit early (Jani N)
> v8 (From Manasi):

it seems you should have signed-off this commit.

> * Get rid of superfluous debug prints (Jani N)
> * Print entire base DPCD before memcpy (Jani N)
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

This changed a lot that I'm not sure my rv-b should remain ;)

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 35 +++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 70ae3d57316b..5698441abe47 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3802,6 +3802,39 @@ intel_dp_link_down(struct intel_encoder *encoder,
>  	}
>  }
>  
> +static void
> +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> +{
> +	u8 dpcd_ext[6];
> +
> +	/*
> +	 * 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))
> +		return;
> +
> +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> +			     &dpcd_ext, sizeof(dpcd_ext)) != sizeof(dpcd_ext)) {
> +		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;
> +	}

no strong feeling, just yet-another bikesheding:
but the mix with blank lines and no blank lines is at least strange to my OCD.

> +	if (!memcmp(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext)))
> +		return;
> +

> +	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
> +		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
> +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));

hm... I'm afraid the full copy might be dangerous.

0x2215 to 0x22FF is
"RESERVED for Extended Receiver V Capability Read all 0s"

On this range of the original one we have
0x0021 which contains DP_MST_CAP bit that we use in our code.

This full copy seems to have the potential to break MST if not
more cases.

I prefer the individual copy of the bits that we know that we need.

but if you prefer the full copy it seems that it would be better
to limit memcopy to 0x0015

Thanks,
Rodrigo.

> +}
> +
>  bool
>  intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  {
> @@ -3809,6 +3842,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.19.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v8] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-11-29 19:28   ` Rodrigo Vivi
@ 2018-11-29 19:43     ` Manasi Navare
  2018-11-29 20:04       ` Rodrigo Vivi
  0 siblings, 1 reply; 18+ messages in thread
From: Manasi Navare @ 2018-11-29 19:43 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

On Thu, Nov 29, 2018 at 11:28:34AM -0800, Rodrigo Vivi wrote:
> On Thu, Nov 29, 2018 at 10:34:05AM -0800, Manasi Navare 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
> > v7 (From Manasi):
> > * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
> > * Exit early (Jani N)
> > v8 (From Manasi):
> 
> it seems you should have signed-off this commit.

Oh yes will do

> 
> > * Get rid of superfluous debug prints (Jani N)
> > * Print entire base DPCD before memcpy (Jani N)
> > 
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> > Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> This changed a lot that I'm not sure my rv-b should remain ;)

Yes agree, i will remove that

> 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 35 +++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 70ae3d57316b..5698441abe47 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3802,6 +3802,39 @@ intel_dp_link_down(struct intel_encoder *encoder,
> >  	}
> >  }
> >  
> > +static void
> > +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> > +{
> > +	u8 dpcd_ext[6];
> > +
> > +	/*
> > +	 * 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))
> > +		return;
> > +
> > +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> > +			     &dpcd_ext, sizeof(dpcd_ext)) != sizeof(dpcd_ext)) {
> > +		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;
> > +	}
> 
> no strong feeling, just yet-another bikesheding:
> but the mix with blank lines and no blank lines is at least strange to my OCD.

Guess the newlines are just residues from spins, will make it uniform

> 
> > +	if (!memcmp(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext)))
> > +		return;
> > +
> 
> > +	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
> > +		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
> > +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
> 
> hm... I'm afraid the full copy might be dangerous.

We are not doing full copy here since the size of dpcd_ext is only 6 bytes.
We are only restricting it from 0x2200 to 0x2205

And the reason we are doing all 6 is because memcmp returned that something changed.

Manasi

> 
> 0x2215 to 0x22FF is
> "RESERVED for Extended Receiver V Capability Read all 0s"
> 
> On this range of the original one we have
> 0x0021 which contains DP_MST_CAP bit that we use in our code.
> 
> This full copy seems to have the potential to break MST if not
> more cases.
> 
> I prefer the individual copy of the bits that we know that we need.
> 
> but if you prefer the full copy it seems that it would be better
> to limit memcopy to 0x0015
> 
> Thanks,
> Rodrigo.
> 
> > +}
> > +
> >  bool
> >  intel_dp_read_dpcd(struct intel_dp *intel_dp)
> >  {
> > @@ -3809,6 +3842,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.19.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v8] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-11-29 19:43     ` Manasi Navare
@ 2018-11-29 20:04       ` Rodrigo Vivi
  0 siblings, 0 replies; 18+ messages in thread
From: Rodrigo Vivi @ 2018-11-29 20:04 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

On Thu, Nov 29, 2018 at 11:43:18AM -0800, Manasi Navare wrote:
> On Thu, Nov 29, 2018 at 11:28:34AM -0800, Rodrigo Vivi wrote:
> > On Thu, Nov 29, 2018 at 10:34:05AM -0800, Manasi Navare 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
> > > v7 (From Manasi):
> > > * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
> > > * Exit early (Jani N)
> > > v8 (From Manasi):
> > 
> > it seems you should have signed-off this commit.
> 
> Oh yes will do
> 
> > 
> > > * Get rid of superfluous debug prints (Jani N)
> > > * Print entire base DPCD before memcpy (Jani N)
> > > 
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > > Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> > > Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > This changed a lot that I'm not sure my rv-b should remain ;)
> 
> Yes agree, i will remove that
> 
> > 
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 35 +++++++++++++++++++++++++++++++++
> > >  1 file changed, 35 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 70ae3d57316b..5698441abe47 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -3802,6 +3802,39 @@ intel_dp_link_down(struct intel_encoder *encoder,
> > >  	}
> > >  }
> > >  
> > > +static void
> > > +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> > > +{
> > > +	u8 dpcd_ext[6];
> > > +
> > > +	/*
> > > +	 * 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))
> > > +		return;
> > > +
> > > +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> > > +			     &dpcd_ext, sizeof(dpcd_ext)) != sizeof(dpcd_ext)) {
> > > +		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;
> > > +	}
> > 
> > no strong feeling, just yet-another bikesheding:
> > but the mix with blank lines and no blank lines is at least strange to my OCD.
> 
> Guess the newlines are just residues from spins, will make it uniform
> 
> > 
> > > +	if (!memcmp(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext)))
> > > +		return;
> > > +
> > 
> > > +	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
> > > +		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
> > > +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
> > 
> > hm... I'm afraid the full copy might be dangerous.
> 
> We are not doing full copy here since the size of dpcd_ext is only 6 bytes.
> We are only restricting it from 0x2200 to 0x2205
> 
> And the reason we are doing all 6 is because memcmp returned that something changed.
> 
> Manasi
> 
> > 
> > 0x2215 to 0x22FF is
> > "RESERVED for Extended Receiver V Capability Read all 0s"
> > 
> > On this range of the original one we have
> > 0x0021 which contains DP_MST_CAP bit that we use in our code.
> > 
> > This full copy seems to have the potential to break MST if not
> > more cases.
> > 
> > I prefer the individual copy of the bits that we know that we need.
> > 
> > but if you prefer the full copy it seems that it would be better
> > to limit memcopy to 0x0015

Duh! I replied and went out for lunch.
while walking to the cafeteria I realized that I didn't check
the size local dpcd_ext... but it was too late! ;)

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

> > 
> > Thanks,
> > Rodrigo.
> > 
> > > +}
> > > +
> > >  bool
> > >  intel_dp_read_dpcd(struct intel_dp *intel_dp)
> > >  {
> > > @@ -3809,6 +3842,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.19.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v9] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-11-29 18:34 ` [PATCH v8] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
  2018-11-29 19:28   ` Rodrigo Vivi
@ 2018-11-29 22:00   ` Manasi Navare
  2018-11-29 22:54     ` Atwood, Matthew S
  2018-12-06 15:50     ` Manasi Navare
  1 sibling, 2 replies; 18+ messages in thread
From: Manasi Navare @ 2018-11-29 22:00 UTC (permalink / raw)
  To: intel-gfx

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
v7 (From Manasi):
* Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
* Exit early (Jani N)
v8 (From Manasi):
* Get rid of superfluous debug prints (Jani N)
* Print entire base DPCD before memcpy (Jani N)
v9 (From Manasi):
* Add uniform newlines (Rodrigo)

Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
Tested-by: Manasi Navare <manasi.d.navare@intel.com>
Acked-by: Manasi Navare <manasi.d.navare@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 38 +++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 38a6e82153fd..b7c4d38089b5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3991,6 +3991,42 @@ intel_dp_link_down(struct intel_encoder *encoder,
 	}
 }
 
+static void
+intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
+{
+	u8 dpcd_ext[6];
+
+	/*
+	 * 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))
+		return;
+
+	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
+			     &dpcd_ext, sizeof(dpcd_ext)) != sizeof(dpcd_ext)) {
+		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, dpcd_ext, sizeof(dpcd_ext)))
+		return;
+
+	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
+		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
+
+	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
+}
+
 bool
 intel_dp_read_dpcd(struct intel_dp *intel_dp)
 {
@@ -3998,6 +4034,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.19.1

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

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

* Re: [PATCH v9] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-11-29 22:00   ` [PATCH v9] " Manasi Navare
@ 2018-11-29 22:54     ` Atwood, Matthew S
  2018-11-29 23:37       ` Rodrigo Vivi
  2018-12-06 15:50     ` Manasi Navare
  1 sibling, 1 reply; 18+ messages in thread
From: Atwood, Matthew S @ 2018-11-29 22:54 UTC (permalink / raw)
  To: intel-gfx, Navare, Manasi D

On Thu, 2018-11-29 at 14:00 -0800, Manasi Navare 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
> v7 (From Manasi):
> * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
> * Exit early (Jani N)
> v8 (From Manasi):
> * Get rid of superfluous debug prints (Jani N)
> * Print entire base DPCD before memcpy (Jani N)
> v9 (From Manasi):
> * Add uniform newlines (Rodrigo)
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 38
> +++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 38a6e82153fd..b7c4d38089b5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3991,6 +3991,42 @@ intel_dp_link_down(struct intel_encoder
> *encoder,
>  	}
>  }
>  
> +static void
> +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> +{
> +	u8 dpcd_ext[6];
> +
> +	/*
> +	 * 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))
I strongly disagree with removing the debug statements. While the spec
may be clear, real world products have real world gotchas that can
silently fail for a long time. The print statements would affect less
then 1% of panels. Why can't we support more verbose debugging
statements here?
> +		return;
> +
> +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> +			     &dpcd_ext, sizeof(dpcd_ext)) !=
> sizeof(dpcd_ext)) {
> +		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, dpcd_ext, sizeof(dpcd_ext)))
> +		return;
> +
> +	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
> +		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
I'f we're doing a Base DPCD dump to dmesg, might as well do the new one
too and have it all in one place.
> +
> +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
I disagree with this method. I specifically did each register that
*could* change to avoid panels that may not follow spec. While this is
more spec compliant, I'd prefer an approach that doesnt allow the panel
to do things improperly.
> +}
> +
>  bool
>  intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  {
> @@ -3998,6 +4034,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;
Manasi, thanks for babysitting this patch while I was on vacation.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev4)
  2018-11-27 20:51 [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
                   ` (5 preceding siblings ...)
  2018-11-29 19:07 ` ✓ Fi.CI.BAT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev3) Patchwork
@ 2018-11-29 23:03 ` Patchwork
  2018-11-30 17:11 ` ✓ Fi.CI.IGT: " Patchwork
  7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-11-29 23:03 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5226 -> Patchwork_10967
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-bsw-n3050:       FAIL [fdo#108656] -> PASS

  * igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656


Participating hosts (49 -> 42)
------------------------------

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


Build changes
-------------

    * Linux: CI_DRM_5226 -> Patchwork_10967

  CI_DRM_5226: 70ad55640e884b31fb19a0e73fa68cb9c99a69f0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10967: e002ba17a0ab963273e666ea63114caadc8f1bda @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e002ba17a0ab drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT

== Logs ==

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

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

* Re: [PATCH v9] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-11-29 22:54     ` Atwood, Matthew S
@ 2018-11-29 23:37       ` Rodrigo Vivi
  2018-12-03 23:54         ` Atwood, Matthew S
  0 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2018-11-29 23:37 UTC (permalink / raw)
  To: Atwood, Matthew S; +Cc: intel-gfx

On Thu, Nov 29, 2018 at 10:54:50PM +0000, Atwood, Matthew S wrote:
> On Thu, 2018-11-29 at 14:00 -0800, Manasi Navare 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
> > v7 (From Manasi):
> > * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
> > * Exit early (Jani N)
> > v8 (From Manasi):
> > * Get rid of superfluous debug prints (Jani N)
> > * Print entire base DPCD before memcpy (Jani N)
> > v9 (From Manasi):
> > * Add uniform newlines (Rodrigo)
> > 
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> > Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 38
> > +++++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 38a6e82153fd..b7c4d38089b5 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3991,6 +3991,42 @@ intel_dp_link_down(struct intel_encoder
> > *encoder,
> >  	}
> >  }
> >  
> > +static void
> > +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> > +{
> > +	u8 dpcd_ext[6];
> > +
> > +	/*
> > +	 * 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))
> I strongly disagree with removing the debug statements. While the spec
> may be clear, real world products have real world gotchas that can
> silently fail for a long time. The print statements would affect less
> then 1% of panels. Why can't we support more verbose debugging
> statements here?

Well, I'm also in favor of the more verbose approach. Specially with
so many bad panels we got out there already.

But in the end if we print all the Base DPCD I believe we
will have all information we need anyway right?

> > +		return;
> > +
> > +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> > +			     &dpcd_ext, sizeof(dpcd_ext)) !=
> > sizeof(dpcd_ext)) {
> > +		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, dpcd_ext, sizeof(dpcd_ext)))
> > +		return;
> > +
> > +	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
> > +		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
> I'f we're doing a Base DPCD dump to dmesg, might as well do the new one
> too and have it all in one place.

I initially had the same feeling here, but then I noticed that
the new one is printed right after this function is called.
So I believe this is a clean enough way. But any patch can be on
top.

> > +
> > +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
> I disagree with this method. I specifically did each register that
> *could* change to avoid panels that may not follow spec. While this is
> more spec compliant, I'd prefer an approach that doesnt allow the panel
> to do things improperly.

I don't have strong feelings on one or the other approach.
But the situation with the author disagreeing with own patch
doesn't seem right.

> > +}
> > +
> >  bool
> >  intel_dp_read_dpcd(struct intel_dp *intel_dp)
> >  {
> > @@ -3998,6 +4034,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;
> Manasi, thanks for babysitting this patch while I was on vacation.

maybe we should split in 2 patches for a clean and accurate
history? :/

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

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

* ✓ Fi.CI.IGT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev4)
  2018-11-27 20:51 [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
                   ` (6 preceding siblings ...)
  2018-11-29 23:03 ` ✓ Fi.CI.BAT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev4) Patchwork
@ 2018-11-30 17:11 ` Patchwork
  7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-11-30 17:11 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5226_full -> Patchwork_10967_full
====================================================

Summary
-------

  **WARNING**

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

### IGT changes ###

#### Warnings ####

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

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@pi-ringfull-vebox:
    - shard-skl:          NOTRUN -> FAIL [fdo#103158]

  * igt@gem_ppgtt@blt-vs-render-ctxn:
    - shard-skl:          NOTRUN -> TIMEOUT [fdo#108039]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956] +1

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_content_protection@atomic:
    - shard-apl:          NOTRUN -> FAIL [fdo#108597]

  * igt@kms_cursor_crc@cursor-128x128-sliding:
    - shard-skl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-128x42-onscreen:
    - shard-apl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          PASS -> FAIL [fdo#104873]

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
    - shard-apl:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-apl:          PASS -> FAIL [fdo#103167] / [fdo#105682]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-render:
    - shard-glk:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-pwrite:
    - {shard-iclb}:       PASS -> FAIL [fdo#103167] +1

  * igt@kms_panel_fitting@legacy:
    - shard-skl:          NOTRUN -> FAIL [fdo#105456]

  * igt@kms_plane@pixel-format-pipe-c-planes:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885]

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +3

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-basic:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-apl:          NOTRUN -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-apl:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_properties@connector-properties-atomic:
    - shard-skl:          NOTRUN -> FAIL [fdo#108642]

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]

  * igt@pm_rpm@dpms-mode-unset-non-lpsp:
    - shard-skl:          SKIP -> INCOMPLETE [fdo#107807]

  * igt@pm_rpm@modeset-non-lpsp-stress-no-wait:
    - {shard-iclb}:       SKIP -> INCOMPLETE [fdo#108840]

  * igt@pm_rpm@reg-read-ioctl:
    - {shard-iclb}:       PASS -> DMESG-WARN [fdo#107724] +3

  * igt@pm_rpm@universal-planes:
    - {shard-iclb}:       PASS -> DMESG-WARN [fdo#108654] / [fdo#108756]

  * {igt@runner@aborted}:
    - {shard-iclb}:       NOTRUN -> FAIL [fdo#108756]

  
#### Possible fixes ####

  * igt@gem_userptr_blits@readonly-unsync:
    - shard-skl:          TIMEOUT [fdo#108887] -> PASS

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - {shard-iclb}:       DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-apl:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_chv_cursor_fail@pipe-a-128x128-bottom-edge:
    - shard-skl:          FAIL [fdo#104671] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-offscreen:
    - shard-skl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-size-change:
    - shard-apl:          FAIL [fdo#103232] -> PASS

  * igt@kms_draw_crc@draw-method-xrgb8888-pwrite-ytiled:
    - shard-skl:          FAIL -> PASS

  * igt@kms_flip@bo-too-big:
    - shard-apl:          DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS +5

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-onoff:
    - {shard-iclb}:       FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@fbcpsr-tilingchange:
    - shard-skl:          FAIL [fdo#105682] -> PASS +2

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-blt:
    - shard-skl:          FAIL [fdo#103167] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - {shard-iclb}:       FAIL [fdo#103166] -> PASS +1
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane_scaling@pipe-c-scaler-with-rotation:
    - {shard-iclb}:       DMESG-WARN [fdo#107724] -> PASS

  * igt@kms_rotation_crc@primary-rotation-180:
    - shard-skl:          FAIL [fdo#103925] / [fdo#107815] -> PASS

  * igt@kms_setmode@basic:
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  * igt@pm_rpm@modeset-non-lpsp-stress-no-wait:
    - shard-skl:          INCOMPLETE [fdo#107807] -> SKIP

  * igt@pm_rpm@pm-caching:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS

  * igt@pm_rps@min-max-config-idle:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

  
#### Warnings ####

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-apl:          DMESG-FAIL [fdo#103558] / [fdo#105602] / [fdo#108145] -> FAIL [fdo#108145]

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

  [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#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103925]: https://bugs.freedesktop.org/show_bug.cgi?id=103925
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108039]: https://bugs.freedesktop.org/show_bug.cgi?id=108039
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108597]: https://bugs.freedesktop.org/show_bug.cgi?id=108597
  [fdo#108642]: https://bugs.freedesktop.org/show_bug.cgi?id=108642
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108756]: https://bugs.freedesktop.org/show_bug.cgi?id=108756
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#108887]: https://bugs.freedesktop.org/show_bug.cgi?id=108887
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


Build changes
-------------

    * Linux: CI_DRM_5226 -> Patchwork_10967

  CI_DRM_5226: 70ad55640e884b31fb19a0e73fa68cb9c99a69f0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4736: 285ebfb3b7adc56586031afa5150c4e5ad40c229 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10967: e002ba17a0ab963273e666ea63114caadc8f1bda @ 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_10967/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v9] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-11-29 23:37       ` Rodrigo Vivi
@ 2018-12-03 23:54         ` Atwood, Matthew S
  0 siblings, 0 replies; 18+ messages in thread
From: Atwood, Matthew S @ 2018-12-03 23:54 UTC (permalink / raw)
  To: Vivi, Rodrigo; +Cc: intel-gfx

On Thu, 2018-11-29 at 15:37 -0800, Rodrigo Vivi wrote:
> On Thu, Nov 29, 2018 at 10:54:50PM +0000, Atwood, Matthew S wrote:
> > On Thu, 2018-11-29 at 14:00 -0800, Manasi Navare 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
> > > v7 (From Manasi):
> > > * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext)
> > > (Jani N)
> > > * Exit early (Jani N)
> > > v8 (From Manasi):
> > > * Get rid of superfluous debug prints (Jani N)
> > > * Print entire base DPCD before memcpy (Jani N)
> > > v9 (From Manasi):
> > > * Add uniform newlines (Rodrigo)
> > > 
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > > Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> > > Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 38
> > > +++++++++++++++++++++++++++++++++
> > >  1 file changed, 38 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 38a6e82153fd..b7c4d38089b5 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -3991,6 +3991,42 @@ intel_dp_link_down(struct intel_encoder
> > > *encoder,
> > >  	}
> > >  }
> > >  
> > > +static void
> > > +intel_dp_extended_receiver_capabilities(struct intel_dp
> > > *intel_dp)
> > > +{
> > > +	u8 dpcd_ext[6];
> > > +
> > > +	/*
> > > +	 * 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))
> > 
> > I strongly disagree with removing the debug statements. While the
> > spec
> > may be clear, real world products have real world gotchas that can
> > silently fail for a long time. The print statements would affect
> > less
> > then 1% of panels. Why can't we support more verbose debugging
> > statements here?
> 
> Well, I'm also in favor of the more verbose approach. Specially with
> so many bad panels we got out there already.
> 
> But in the end if we print all the Base DPCD I believe we
> will have all information we need anyway right?
Sure.
> 
> > > +		return;
> > > +
> > > +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> > > +			     &dpcd_ext, sizeof(dpcd_ext)) !=
> > > sizeof(dpcd_ext)) {
> > > +		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, dpcd_ext, sizeof(dpcd_ext)))
> > > +		return;
> > > +
> > > +	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
> > > +		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
> > 
> > I'f we're doing a Base DPCD dump to dmesg, might as well do the new
> > one
> > too and have it all in one place.
> 
> I initially had the same feeling here, but then I noticed that
> the new one is printed right after this function is called.
> So I believe this is a clean enough way. But any patch can be on
> top.
You're right this is fine
> 
> > > +
> > > +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
> > 
> > I disagree with this method. I specifically did each register that
> > *could* change to avoid panels that may not follow spec. While this
> > is
> > more spec compliant, I'd prefer an approach that doesnt allow the
> > panel
> > to do things improperly.
> 
> I don't have strong feelings on one or the other approach.
> But the situation with the author disagreeing with own patch
> doesn't seem right.
I'm fine with merging it.
> 
> > > +}
> > > +
> > >  bool
> > >  intel_dp_read_dpcd(struct intel_dp *intel_dp)
> > >  {
> > > @@ -3998,6 +4034,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;
> > 
> > Manasi, thanks for babysitting this patch while I was on vacation.
> 
> maybe we should split in 2 patches for a clean and accurate
> history? :/
Rather just have this done now I think. 
> 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v9] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT
  2018-11-29 22:00   ` [PATCH v9] " Manasi Navare
  2018-11-29 22:54     ` Atwood, Matthew S
@ 2018-12-06 15:50     ` Manasi Navare
  1 sibling, 0 replies; 18+ messages in thread
From: Manasi Navare @ 2018-12-06 15:50 UTC (permalink / raw)
  To: intel-gfx

Thanks for the patch and the review, pushed to dinq

Manasi`

On Thu, Nov 29, 2018 at 02:00:58PM -0800, Manasi Navare 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
> v7 (From Manasi):
> * Memcpy, memcmp and debig logging based on sizeof(dpcd_ext) (Jani N)
> * Exit early (Jani N)
> v8 (From Manasi):
> * Get rid of superfluous debug prints (Jani N)
> * Print entire base DPCD before memcpy (Jani N)
> v9 (From Manasi):
> * Add uniform newlines (Rodrigo)
> 
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> Tested-by: Manasi Navare <manasi.d.navare@intel.com>
> Acked-by: Manasi Navare <manasi.d.navare@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 38 +++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 38a6e82153fd..b7c4d38089b5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3991,6 +3991,42 @@ intel_dp_link_down(struct intel_encoder *encoder,
>  	}
>  }
>  
> +static void
> +intel_dp_extended_receiver_capabilities(struct intel_dp *intel_dp)
> +{
> +	u8 dpcd_ext[6];
> +
> +	/*
> +	 * 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))
> +		return;
> +
> +	if (drm_dp_dpcd_read(&intel_dp->aux, DP_DP13_DPCD_REV,
> +			     &dpcd_ext, sizeof(dpcd_ext)) != sizeof(dpcd_ext)) {
> +		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, dpcd_ext, sizeof(dpcd_ext)))
> +		return;
> +
> +	DRM_DEBUG_KMS("Base DPCD: %*ph\n",
> +		      (int)sizeof(intel_dp->dpcd), intel_dp->dpcd);
> +
> +	memcpy(intel_dp->dpcd, dpcd_ext, sizeof(dpcd_ext));
> +}
> +
>  bool
>  intel_dp_read_dpcd(struct intel_dp *intel_dp)
>  {
> @@ -3998,6 +4034,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.19.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2018-12-06 15:47 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-27 20:51 [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
2018-11-27 21:44 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev2) Patchwork
2018-11-27 22:10 ` ✓ Fi.CI.BAT: success " Patchwork
2018-11-28  9:09 ` [PATCH v7] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Jani Nikula
2018-11-28 19:18   ` Manasi Navare
2018-11-28 10:02 ` ✓ Fi.CI.IGT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev2) Patchwork
2018-11-29 18:34 ` [PATCH v8] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT Manasi Navare
2018-11-29 19:28   ` Rodrigo Vivi
2018-11-29 19:43     ` Manasi Navare
2018-11-29 20:04       ` Rodrigo Vivi
2018-11-29 22:00   ` [PATCH v9] " Manasi Navare
2018-11-29 22:54     ` Atwood, Matthew S
2018-11-29 23:37       ` Rodrigo Vivi
2018-12-03 23:54         ` Atwood, Matthew S
2018-12-06 15:50     ` Manasi Navare
2018-11-29 19:07 ` ✓ Fi.CI.BAT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev3) Patchwork
2018-11-29 23:03 ` ✓ Fi.CI.BAT: success for drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT (rev4) Patchwork
2018-11-30 17:11 ` ✓ Fi.CI.IGT: " Patchwork

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.