All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure
@ 2020-05-04 12:35 Oliver Barta
  2020-05-04 18:14 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Oliver Barta @ 2020-05-04 12:35 UTC (permalink / raw)
  To: Ramalingam C, intel-gfx, Jani Nikula, Rodrigo Vivi
  Cc: Daniel Vetter, Ravisankar Madasamy, Sean Paul, Oliver Barta,
	Chris Wilson

From: Oliver Barta <oliver.barta@aptiv.com>

A single Ri mismatch doesn't automatically mean that the link integrity
is broken. Update and check of Ri and Ri' are done asynchronously. In
case an update happens just between the read of Ri' and the check against
Ri there will be a mismatch even if the link integrity is fine otherwise.

Signed-off-by: Oliver Barta <oliver.barta@aptiv.com>
---
 drivers/gpu/drm/i915/display/intel_hdmi.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 010f37240710..3156fde392f2 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1540,7 +1540,7 @@ int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
 }
 
 static
-bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
+bool intel_hdmi_hdcp_check_link_once(struct intel_digital_port *intel_dig_port)
 {
 	struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
 	struct intel_connector *connector =
@@ -1563,8 +1563,7 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
 	if (wait_for((intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder, port)) &
 		      (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC)) ==
 		     (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
-		drm_err(&i915->drm,
-			"Ri' mismatch detected, link check failed (%x)\n",
+		drm_dbg_kms(&i915->drm, "Ri' mismatch detected (%x)\n",
 			intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder,
 							port)));
 		return false;
@@ -1572,6 +1571,20 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
 	return true;
 }
 
+static
+bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
+{
+	struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
+	int retry;
+
+	for (retry = 0; retry < 3; retry++)
+		if (intel_hdmi_hdcp_check_link_once(intel_dig_port))
+			return true;
+
+	drm_err(&i915->drm, "Link check failed\n");
+	return false;
+}
+
 struct hdcp2_hdmi_msg_timeout {
 	u8 msg_id;
 	u16 timeout;
-- 
2.20.1

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

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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: HDCP: retry link integrity check on failure
  2020-05-04 12:35 [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure Oliver Barta
@ 2020-05-04 18:14 ` Patchwork
  2020-05-04 18:38 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2020-05-04 18:14 UTC (permalink / raw)
  To: Oliver Barta; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: HDCP: retry link integrity check on failure
URL   : https://patchwork.freedesktop.org/series/76917/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
a62240c79605 drm/i915: HDCP: retry link integrity check on failure
-:33: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#33: FILE: drivers/gpu/drm/i915/display/intel_hdmi.c:1567:
+		drm_dbg_kms(&i915->drm, "Ri' mismatch detected (%x)\n",
 			intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder,

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

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

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: HDCP: retry link integrity check on failure
  2020-05-04 12:35 [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure Oliver Barta
  2020-05-04 18:14 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2020-05-04 18:38 ` Patchwork
  2020-05-04 20:24 ` [Intel-gfx] [PATCH] " Sean Paul
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2020-05-04 18:38 UTC (permalink / raw)
  To: Oliver Barta; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: HDCP: retry link integrity check on failure
URL   : https://patchwork.freedesktop.org/series/76917/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8422 -> Patchwork_17572
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/index.html

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

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

### IGT changes ###

#### Possible fixes ####

  * igt@i915_selftest@live@perf:
    - fi-bwr-2160:        [INCOMPLETE][1] ([i915#489]) -> [PASS][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/fi-bwr-2160/igt@i915_selftest@live@perf.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/fi-bwr-2160/igt@i915_selftest@live@perf.html

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

  [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34
  [i915#489]: https://gitlab.freedesktop.org/drm/intel/issues/489


Participating hosts (52 -> 45)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8422 -> Patchwork_17572

  CI-20190529: 20190529
  CI_DRM_8422: 0ca0fee447b032331962bc5c717786bdb3594bd9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5628: 652a3fd8966345fa5498904ce80a2027a6782783 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17572: a62240c79605a8b651272b2eb4b9d879f81f0504 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a62240c79605 drm/i915: HDCP: retry link integrity check on failure

== Logs ==

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

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

* Re: [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure
  2020-05-04 12:35 [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure Oliver Barta
  2020-05-04 18:14 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
  2020-05-04 18:38 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-05-04 20:24 ` Sean Paul
  2020-05-05  7:27   ` Oliver Barta
  2020-05-05  7:15 ` [Intel-gfx] ✓ Fi.CI.IGT: success for " Patchwork
  2020-05-05  7:38 ` [Intel-gfx] [PATCH] " Ramalingam C
  4 siblings, 1 reply; 11+ messages in thread
From: Sean Paul @ 2020-05-04 20:24 UTC (permalink / raw)
  To: Oliver Barta
  Cc: Daniel Vetter, Intel Graphics Development, Ravisankar Madasamy,
	Sean Paul, Oliver Barta, Chris Wilson

On Mon, May 4, 2020 at 1:32 PM Oliver Barta <o.barta89@gmail.com> wrote:
>
> From: Oliver Barta <oliver.barta@aptiv.com>
>
> A single Ri mismatch doesn't automatically mean that the link integrity
> is broken. Update and check of Ri and Ri' are done asynchronously. In
> case an update happens just between the read of Ri' and the check against
> Ri there will be a mismatch even if the link integrity is fine otherwise.
>
> Signed-off-by: Oliver Barta <oliver.barta@aptiv.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 010f37240710..3156fde392f2 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1540,7 +1540,7 @@ int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
>  }
>
>  static
> -bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> +bool intel_hdmi_hdcp_check_link_once(struct intel_digital_port *intel_dig_port)
>  {
>         struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
>         struct intel_connector *connector =
> @@ -1563,8 +1563,7 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
>         if (wait_for((intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder, port)) &
>                       (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC)) ==
>                      (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {

Why doesn't the wait_for catch this?

Sean

> -               drm_err(&i915->drm,
> -                       "Ri' mismatch detected, link check failed (%x)\n",
> +               drm_dbg_kms(&i915->drm, "Ri' mismatch detected (%x)\n",
>                         intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder,
>                                                         port)));
>                 return false;
> @@ -1572,6 +1571,20 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
>         return true;
>  }
>
> +static
> +bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> +{
> +       struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
> +       int retry;
> +
> +       for (retry = 0; retry < 3; retry++)
> +               if (intel_hdmi_hdcp_check_link_once(intel_dig_port))
> +                       return true;
> +
> +       drm_err(&i915->drm, "Link check failed\n");
> +       return false;
> +}
> +
>  struct hdcp2_hdmi_msg_timeout {
>         u8 msg_id;
>         u16 timeout;
> --
> 2.20.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] 11+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: HDCP: retry link integrity check on failure
  2020-05-04 12:35 [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure Oliver Barta
                   ` (2 preceding siblings ...)
  2020-05-04 20:24 ` [Intel-gfx] [PATCH] " Sean Paul
@ 2020-05-05  7:15 ` Patchwork
  2020-05-05  7:38 ` [Intel-gfx] [PATCH] " Ramalingam C
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2020-05-05  7:15 UTC (permalink / raw)
  To: Oliver Barta; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: HDCP: retry link integrity check on failure
URL   : https://patchwork.freedesktop.org/series/76917/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8422_full -> Patchwork_17572_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gen9_exec_parse@allowed-all:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([i915#716])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-apl3/igt@gen9_exec_parse@allowed-all.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-apl4/igt@gen9_exec_parse@allowed-all.html

  * igt@i915_suspend@forcewake:
    - shard-iclb:         [PASS][3] -> [INCOMPLETE][4] ([i915#1185])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-iclb5/igt@i915_suspend@forcewake.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-iclb3/igt@i915_suspend@forcewake.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x256-sliding:
    - shard-kbl:          [PASS][5] -> [FAIL][6] ([i915#54] / [i915#93] / [i915#95])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-256x256-sliding.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-256x256-sliding.html

  * igt@kms_flip_tiling@flip-changes-tiling-y:
    - shard-apl:          [PASS][7] -> [FAIL][8] ([i915#95])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-apl2/igt@kms_flip_tiling@flip-changes-tiling-y.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-apl6/igt@kms_flip_tiling@flip-changes-tiling-y.html
    - shard-kbl:          [PASS][9] -> [FAIL][10] ([i915#699] / [i915#93] / [i915#95])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-kbl6/igt@kms_flip_tiling@flip-changes-tiling-y.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-kbl2/igt@kms_flip_tiling@flip-changes-tiling-y.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-render:
    - shard-skl:          [PASS][11] -> [FAIL][12] ([i915#49])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-skl5/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-render.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-skl7/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-render.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-kbl:          [PASS][13] -> [DMESG-WARN][14] ([i915#180]) +3 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-kbl4/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-kbl4/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [PASS][15] -> [DMESG-WARN][16] ([i915#180]) +2 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-apl8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([fdo#108145] / [i915#265])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-skl5/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-skl7/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_psr@psr2_sprite_blt:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#109441]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-iclb2/igt@kms_psr@psr2_sprite_blt.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-iclb7/igt@kms_psr@psr2_sprite_blt.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [PASS][21] -> [DMESG-WARN][22] ([i915#180] / [i915#93] / [i915#95])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-kbl4/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-kbl6/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  
#### Possible fixes ####

  * {igt@gem_ctx_isolation@preservation-s3@bcs0}:
    - shard-kbl:          [DMESG-WARN][23] ([i915#180]) -> [PASS][24] +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-kbl6/igt@gem_ctx_isolation@preservation-s3@bcs0.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-kbl4/igt@gem_ctx_isolation@preservation-s3@bcs0.html
    - shard-apl:          [DMESG-WARN][25] ([i915#180]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-apl1/igt@gem_ctx_isolation@preservation-s3@bcs0.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-apl2/igt@gem_ctx_isolation@preservation-s3@bcs0.html

  * igt@kms_cursor_crc@pipe-b-cursor-suspend:
    - shard-skl:          [INCOMPLETE][27] ([i915#300]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-skl7/igt@kms_cursor_crc@pipe-b-cursor-suspend.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-skl10/igt@kms_cursor_crc@pipe-b-cursor-suspend.html

  * {igt@kms_flip@2x-plain-flip-fb-recreate@ab-hdmi-a1-hdmi-a2}:
    - shard-glk:          [FAIL][29] ([i915#34]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-glk9/igt@kms_flip@2x-plain-flip-fb-recreate@ab-hdmi-a1-hdmi-a2.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-glk6/igt@kms_flip@2x-plain-flip-fb-recreate@ab-hdmi-a1-hdmi-a2.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [FAIL][31] ([fdo#108145] / [i915#265]) -> [PASS][32] +2 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [SKIP][33] ([fdo#109642] / [fdo#111068]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-iclb4/igt@kms_psr2_su@frontbuffer.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-iclb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         [SKIP][35] ([fdo#109441]) -> [PASS][36] +1 similar issue
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-iclb4/igt@kms_psr@psr2_cursor_plane_move.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc6-dpms:
    - shard-tglb:         [SKIP][37] ([i915#468]) -> [FAIL][38] ([i915#454])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-tglb2/igt@i915_pm_dc@dc6-dpms.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-tglb6/igt@i915_pm_dc@dc6-dpms.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [SKIP][39] ([fdo#109642] / [fdo#111068]) -> [FAIL][40] ([i915#608])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8422/shard-iclb4/igt@kms_psr2_su@page_flip.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17572/shard-iclb2/igt@kms_psr2_su@page_flip.html

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

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [i915#1185]: https://gitlab.freedesktop.org/drm/intel/issues/1185
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#300]: https://gitlab.freedesktop.org/drm/intel/issues/300
  [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#46]: https://gitlab.freedesktop.org/drm/intel/issues/46
  [i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#608]: https://gitlab.freedesktop.org/drm/intel/issues/608
  [i915#699]: https://gitlab.freedesktop.org/drm/intel/issues/699
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8422 -> Patchwork_17572

  CI-20190529: 20190529
  CI_DRM_8422: 0ca0fee447b032331962bc5c717786bdb3594bd9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5628: 652a3fd8966345fa5498904ce80a2027a6782783 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17572: a62240c79605a8b651272b2eb4b9d879f81f0504 @ 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_17572/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure
  2020-05-04 20:24 ` [Intel-gfx] [PATCH] " Sean Paul
@ 2020-05-05  7:27   ` Oliver Barta
  2020-05-05 16:22     ` Sean Paul
  0 siblings, 1 reply; 11+ messages in thread
From: Oliver Barta @ 2020-05-05  7:27 UTC (permalink / raw)
  To: Sean Paul
  Cc: Daniel Vetter, Intel Graphics Development, Ravisankar Madasamy,
	Sean Paul, Oliver Barta, Chris Wilson

On Mon, May 4, 2020 at 10:24 PM Sean Paul <sean@poorly.run> wrote:
>
> On Mon, May 4, 2020 at 1:32 PM Oliver Barta <o.barta89@gmail.com> wrote:
> >
> > From: Oliver Barta <oliver.barta@aptiv.com>
> >
> > A single Ri mismatch doesn't automatically mean that the link integrity
> > is broken. Update and check of Ri and Ri' are done asynchronously. In
> > case an update happens just between the read of Ri' and the check against
> > Ri there will be a mismatch even if the link integrity is fine otherwise.
> >
> > Signed-off-by: Oliver Barta <oliver.barta@aptiv.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_hdmi.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index 010f37240710..3156fde392f2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > @@ -1540,7 +1540,7 @@ int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
> >  }
> >
> >  static
> > -bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> > +bool intel_hdmi_hdcp_check_link_once(struct intel_digital_port *intel_dig_port)
> >  {
> >         struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
> >         struct intel_connector *connector =
> > @@ -1563,8 +1563,7 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> >         if (wait_for((intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder, port)) &
> >                       (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC)) ==
> >                      (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
>
> Why doesn't the wait_for catch this?
>
> Sean
>

Hello Sean,

thank you for having a look on my patch. The wait_for can't catch this
because it is Ri' which is outdated compared to Ri. Ri' however needs
to be read over DDC interface which is done only once during the check
sequence. It is not updated during the waiting time.

Oliver

> > -               drm_err(&i915->drm,
> > -                       "Ri' mismatch detected, link check failed (%x)\n",
> > +               drm_dbg_kms(&i915->drm, "Ri' mismatch detected (%x)\n",
> >                         intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder,
> >                                                         port)));
> >                 return false;
> > @@ -1572,6 +1571,20 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> >         return true;
> >  }
> >
> > +static
> > +bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> > +{
> > +       struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
> > +       int retry;
> > +
> > +       for (retry = 0; retry < 3; retry++)
> > +               if (intel_hdmi_hdcp_check_link_once(intel_dig_port))
> > +                       return true;
> > +
> > +       drm_err(&i915->drm, "Link check failed\n");
> > +       return false;
> > +}
> > +
> >  struct hdcp2_hdmi_msg_timeout {
> >         u8 msg_id;
> >         u16 timeout;
> > --
> > 2.20.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] 11+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure
  2020-05-04 12:35 [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure Oliver Barta
                   ` (3 preceding siblings ...)
  2020-05-05  7:15 ` [Intel-gfx] ✓ Fi.CI.IGT: success for " Patchwork
@ 2020-05-05  7:38 ` Ramalingam C
  2020-05-05 12:06   ` Oliver Barta
  4 siblings, 1 reply; 11+ messages in thread
From: Ramalingam C @ 2020-05-05  7:38 UTC (permalink / raw)
  To: Oliver Barta
  Cc: Daniel Vetter, intel-gfx, Chris Wilson, Sean Paul, Oliver Barta,
	Ravisankar Madasamy

On 2020-05-04 at 14:35:24 +0200, Oliver Barta wrote:
> From: Oliver Barta <oliver.barta@aptiv.com>
> 
> A single Ri mismatch doesn't automatically mean that the link integrity
> is broken. Update and check of Ri and Ri' are done asynchronously. In
> case an update happens just between the read of Ri' and the check against
> Ri there will be a mismatch even if the link integrity is fine otherwise.

Thanks for working on this. Btw, did you face this sporadic link check
failure or theoretically you are fixing it?

IMO this change will rule out possible sporadic link check failures as
mentioned in the commit msg. Though I haven't faced this issue at my
testings.

Reviewed-by: Ramalingam C <ramalingam.c@intel.com>

> 
> Signed-off-by: Oliver Barta <oliver.barta@aptiv.com>
> ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 010f37240710..3156fde392f2 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -1540,7 +1540,7 @@ int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
>  }
>  
>  static
> -bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> +bool intel_hdmi_hdcp_check_link_once(struct intel_digital_port *intel_dig_port)
>  {
>  	struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
>  	struct intel_connector *connector =
> @@ -1563,8 +1563,7 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
>  	if (wait_for((intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder, port)) &
>  		      (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC)) ==
>  		     (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
> -		drm_err(&i915->drm,
> -			"Ri' mismatch detected, link check failed (%x)\n",
> +		drm_dbg_kms(&i915->drm, "Ri' mismatch detected (%x)\n",
>  			intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder,
>  							port)));
>  		return false;
> @@ -1572,6 +1571,20 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
>  	return true;
>  }
>  
> +static
> +bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> +{
> +	struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
> +	int retry;
> +
> +	for (retry = 0; retry < 3; retry++)
> +		if (intel_hdmi_hdcp_check_link_once(intel_dig_port))
> +			return true;
> +
> +	drm_err(&i915->drm, "Link check failed\n");
> +	return false;
> +}
> +
>  struct hdcp2_hdmi_msg_timeout {
>  	u8 msg_id;
>  	u16 timeout;
> -- 
> 2.20.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure
  2020-05-05  7:38 ` [Intel-gfx] [PATCH] " Ramalingam C
@ 2020-05-05 12:06   ` Oliver Barta
  2020-05-05 12:12     ` Ramalingam C
  0 siblings, 1 reply; 11+ messages in thread
From: Oliver Barta @ 2020-05-05 12:06 UTC (permalink / raw)
  To: Ramalingam C
  Cc: Daniel Vetter, Intel Graphics Development, Chris Wilson,
	Sean Paul, Oliver Barta, Ravisankar Madasamy

On Tue, May 5, 2020 at 9:38 AM Ramalingam C <ramalingam.c@intel.com> wrote:
>
> On 2020-05-04 at 14:35:24 +0200, Oliver Barta wrote:
> > From: Oliver Barta <oliver.barta@aptiv.com>
> >
> > A single Ri mismatch doesn't automatically mean that the link integrity
> > is broken. Update and check of Ri and Ri' are done asynchronously. In
> > case an update happens just between the read of Ri' and the check against
> > Ri there will be a mismatch even if the link integrity is fine otherwise.
>
> Thanks for working on this. Btw, did you face this sporadic link check
> failure or theoretically you are fixing it?
>
> IMO this change will rule out possible sporadic link check failures as
> mentioned in the commit msg. Though I haven't faced this issue at my
> testings.
>
> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
>

I found it by code inspection, the probability for this to happen is
very low. In order to test the patch I'm decreasing the value of
DRM_HDCP_CHECK_PERIOD_MS to just a few ms. Once you do that it happens
every few seconds.

Thanks,
Oliver

> >
> > Signed-off-by: Oliver Barta <oliver.barta@aptiv.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_hdmi.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index 010f37240710..3156fde392f2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > @@ -1540,7 +1540,7 @@ int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
> >  }
> >
> >  static
> > -bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> > +bool intel_hdmi_hdcp_check_link_once(struct intel_digital_port *intel_dig_port)
> >  {
> >       struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
> >       struct intel_connector *connector =
> > @@ -1563,8 +1563,7 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> >       if (wait_for((intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder, port)) &
> >                     (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC)) ==
> >                    (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
> > -             drm_err(&i915->drm,
> > -                     "Ri' mismatch detected, link check failed (%x)\n",
> > +             drm_dbg_kms(&i915->drm, "Ri' mismatch detected (%x)\n",
> >                       intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder,
> >                                                       port)));
> >               return false;
> > @@ -1572,6 +1571,20 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> >       return true;
> >  }
> >
> > +static
> > +bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> > +{
> > +     struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
> > +     int retry;
> > +
> > +     for (retry = 0; retry < 3; retry++)
> > +             if (intel_hdmi_hdcp_check_link_once(intel_dig_port))
> > +                     return true;
> > +
> > +     drm_err(&i915->drm, "Link check failed\n");
> > +     return false;
> > +}
> > +
> >  struct hdcp2_hdmi_msg_timeout {
> >       u8 msg_id;
> >       u16 timeout;
> > --
> > 2.20.1
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure
  2020-05-05 12:06   ` Oliver Barta
@ 2020-05-05 12:12     ` Ramalingam C
  2020-06-30  8:06       ` Jani Nikula
  0 siblings, 1 reply; 11+ messages in thread
From: Ramalingam C @ 2020-05-05 12:12 UTC (permalink / raw)
  To: Oliver Barta
  Cc: Daniel Vetter, Intel Graphics Development, Chris Wilson,
	Sean Paul, Oliver Barta, Ravisankar Madasamy

On 2020-05-05 at 14:06:51 +0200, Oliver Barta wrote:
> On Tue, May 5, 2020 at 9:38 AM Ramalingam C <ramalingam.c@intel.com> wrote:
> >
> > On 2020-05-04 at 14:35:24 +0200, Oliver Barta wrote:
> > > From: Oliver Barta <oliver.barta@aptiv.com>
> > >
> > > A single Ri mismatch doesn't automatically mean that the link integrity
> > > is broken. Update and check of Ri and Ri' are done asynchronously. In
> > > case an update happens just between the read of Ri' and the check against
> > > Ri there will be a mismatch even if the link integrity is fine otherwise.
> >
> > Thanks for working on this. Btw, did you face this sporadic link check
> > failure or theoretically you are fixing it?
> >
> > IMO this change will rule out possible sporadic link check failures as
> > mentioned in the commit msg. Though I haven't faced this issue at my
> > testings.
> >
> > Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> >
> 
> I found it by code inspection, the probability for this to happen is
> very low. In order to test the patch I'm decreasing the value of
> DRM_HDCP_CHECK_PERIOD_MS to just a few ms. Once you do that it happens
> every few seconds.
Ok. That make sense. Thanks for the explanation.

-Ram
> 
> Thanks,
> Oliver
> 
> > >
> > > Signed-off-by: Oliver Barta <oliver.barta@aptiv.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_hdmi.c | 19 ++++++++++++++++---
> > >  1 file changed, 16 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > index 010f37240710..3156fde392f2 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > @@ -1540,7 +1540,7 @@ int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
> > >  }
> > >
> > >  static
> > > -bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> > > +bool intel_hdmi_hdcp_check_link_once(struct intel_digital_port *intel_dig_port)
> > >  {
> > >       struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
> > >       struct intel_connector *connector =
> > > @@ -1563,8 +1563,7 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> > >       if (wait_for((intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder, port)) &
> > >                     (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC)) ==
> > >                    (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
> > > -             drm_err(&i915->drm,
> > > -                     "Ri' mismatch detected, link check failed (%x)\n",
> > > +             drm_dbg_kms(&i915->drm, "Ri' mismatch detected (%x)\n",
> > >                       intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder,
> > >                                                       port)));
> > >               return false;
> > > @@ -1572,6 +1571,20 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> > >       return true;
> > >  }
> > >
> > > +static
> > > +bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> > > +{
> > > +     struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
> > > +     int retry;
> > > +
> > > +     for (retry = 0; retry < 3; retry++)
> > > +             if (intel_hdmi_hdcp_check_link_once(intel_dig_port))
> > > +                     return true;
> > > +
> > > +     drm_err(&i915->drm, "Link check failed\n");
> > > +     return false;
> > > +}
> > > +
> > >  struct hdcp2_hdmi_msg_timeout {
> > >       u8 msg_id;
> > >       u16 timeout;
> > > --
> > > 2.20.1
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure
  2020-05-05  7:27   ` Oliver Barta
@ 2020-05-05 16:22     ` Sean Paul
  0 siblings, 0 replies; 11+ messages in thread
From: Sean Paul @ 2020-05-05 16:22 UTC (permalink / raw)
  To: Oliver Barta
  Cc: Daniel Vetter, Intel Graphics Development, Ravisankar Madasamy,
	Sean Paul, Oliver Barta, Chris Wilson

On Tue, May 5, 2020 at 3:27 AM Oliver Barta <o.barta89@gmail.com> wrote:
>
> On Mon, May 4, 2020 at 10:24 PM Sean Paul <sean@poorly.run> wrote:
> >
> > On Mon, May 4, 2020 at 1:32 PM Oliver Barta <o.barta89@gmail.com> wrote:
> > >
> > > From: Oliver Barta <oliver.barta@aptiv.com>
> > >
> > > A single Ri mismatch doesn't automatically mean that the link integrity
> > > is broken. Update and check of Ri and Ri' are done asynchronously. In
> > > case an update happens just between the read of Ri' and the check against
> > > Ri there will be a mismatch even if the link integrity is fine otherwise.
> > >
> > > Signed-off-by: Oliver Barta <oliver.barta@aptiv.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_hdmi.c | 19 ++++++++++++++++---
> > >  1 file changed, 16 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > index 010f37240710..3156fde392f2 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > > @@ -1540,7 +1540,7 @@ int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
> > >  }
> > >
> > >  static
> > > -bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> > > +bool intel_hdmi_hdcp_check_link_once(struct intel_digital_port *intel_dig_port)
> > >  {
> > >         struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
> > >         struct intel_connector *connector =
> > > @@ -1563,8 +1563,7 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> > >         if (wait_for((intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder, port)) &
> > >                       (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC)) ==
> > >                      (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
> >
> > Why doesn't the wait_for catch this?
> >
> > Sean
> >
>
> Hello Sean,
>
> thank you for having a look on my patch. The wait_for can't catch this
> because it is Ri' which is outdated compared to Ri. Ri' however needs
> to be read over DDC interface which is done only once during the check
> sequence. It is not updated during the waiting time.
>

Ok, makes sense, thank you.

Reviewed-by: Sean Paul <sean@poorly.run>

> Oliver
>
> > > -               drm_err(&i915->drm,
> > > -                       "Ri' mismatch detected, link check failed (%x)\n",
> > > +               drm_dbg_kms(&i915->drm, "Ri' mismatch detected (%x)\n",
> > >                         intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder,
> > >                                                         port)));
> > >                 return false;
> > > @@ -1572,6 +1571,20 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> > >         return true;
> > >  }
> > >
> > > +static
> > > +bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
> > > +{
> > > +       struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
> > > +       int retry;
> > > +
> > > +       for (retry = 0; retry < 3; retry++)
> > > +               if (intel_hdmi_hdcp_check_link_once(intel_dig_port))
> > > +                       return true;
> > > +
> > > +       drm_err(&i915->drm, "Link check failed\n");
> > > +       return false;
> > > +}
> > > +
> > >  struct hdcp2_hdmi_msg_timeout {
> > >         u8 msg_id;
> > >         u16 timeout;
> > > --
> > > 2.20.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] 11+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure
  2020-05-05 12:12     ` Ramalingam C
@ 2020-06-30  8:06       ` Jani Nikula
  0 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2020-06-30  8:06 UTC (permalink / raw)
  To: Ramalingam C, Oliver Barta
  Cc: Daniel Vetter, Intel Graphics Development, Ravisankar Madasamy,
	Sean Paul, Oliver Barta, Chris Wilson

On Tue, 05 May 2020, Ramalingam C <ramalingam.c@intel.com> wrote:
> On 2020-05-05 at 14:06:51 +0200, Oliver Barta wrote:
>> On Tue, May 5, 2020 at 9:38 AM Ramalingam C <ramalingam.c@intel.com> wrote:
>> >
>> > On 2020-05-04 at 14:35:24 +0200, Oliver Barta wrote:
>> > > From: Oliver Barta <oliver.barta@aptiv.com>
>> > >
>> > > A single Ri mismatch doesn't automatically mean that the link integrity
>> > > is broken. Update and check of Ri and Ri' are done asynchronously. In
>> > > case an update happens just between the read of Ri' and the check against
>> > > Ri there will be a mismatch even if the link integrity is fine otherwise.
>> >
>> > Thanks for working on this. Btw, did you face this sporadic link check
>> > failure or theoretically you are fixing it?
>> >
>> > IMO this change will rule out possible sporadic link check failures as
>> > mentioned in the commit msg. Though I haven't faced this issue at my
>> > testings.
>> >
>> > Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
>> >
>> 
>> I found it by code inspection, the probability for this to happen is
>> very low. In order to test the patch I'm decreasing the value of
>> DRM_HDCP_CHECK_PERIOD_MS to just a few ms. Once you do that it happens
>> every few seconds.
> Ok. That make sense. Thanks for the explanation.

Finally pushed, thanks for the patch and reviews.

Ram, I was kind of expecting you to push this.

BR,
Jani.


>
> -Ram
>> 
>> Thanks,
>> Oliver
>> 
>> > >
>> > > Signed-off-by: Oliver Barta <oliver.barta@aptiv.com>
>> > > ---
>> > >  drivers/gpu/drm/i915/display/intel_hdmi.c | 19 ++++++++++++++++---
>> > >  1 file changed, 16 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> > > index 010f37240710..3156fde392f2 100644
>> > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
>> > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
>> > > @@ -1540,7 +1540,7 @@ int intel_hdmi_hdcp_toggle_signalling(struct intel_digital_port *intel_dig_port,
>> > >  }
>> > >
>> > >  static
>> > > -bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
>> > > +bool intel_hdmi_hdcp_check_link_once(struct intel_digital_port *intel_dig_port)
>> > >  {
>> > >       struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
>> > >       struct intel_connector *connector =
>> > > @@ -1563,8 +1563,7 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
>> > >       if (wait_for((intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder, port)) &
>> > >                     (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC)) ==
>> > >                    (HDCP_STATUS_RI_MATCH | HDCP_STATUS_ENC), 1)) {
>> > > -             drm_err(&i915->drm,
>> > > -                     "Ri' mismatch detected, link check failed (%x)\n",
>> > > +             drm_dbg_kms(&i915->drm, "Ri' mismatch detected (%x)\n",
>> > >                       intel_de_read(i915, HDCP_STATUS(i915, cpu_transcoder,
>> > >                                                       port)));
>> > >               return false;
>> > > @@ -1572,6 +1571,20 @@ bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
>> > >       return true;
>> > >  }
>> > >
>> > > +static
>> > > +bool intel_hdmi_hdcp_check_link(struct intel_digital_port *intel_dig_port)
>> > > +{
>> > > +     struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
>> > > +     int retry;
>> > > +
>> > > +     for (retry = 0; retry < 3; retry++)
>> > > +             if (intel_hdmi_hdcp_check_link_once(intel_dig_port))
>> > > +                     return true;
>> > > +
>> > > +     drm_err(&i915->drm, "Link check failed\n");
>> > > +     return false;
>> > > +}
>> > > +
>> > >  struct hdcp2_hdmi_msg_timeout {
>> > >       u8 msg_id;
>> > >       u16 timeout;
>> > > --
>> > > 2.20.1
>> > >

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

end of thread, other threads:[~2020-06-30  8:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-04 12:35 [Intel-gfx] [PATCH] drm/i915: HDCP: retry link integrity check on failure Oliver Barta
2020-05-04 18:14 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2020-05-04 18:38 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-05-04 20:24 ` [Intel-gfx] [PATCH] " Sean Paul
2020-05-05  7:27   ` Oliver Barta
2020-05-05 16:22     ` Sean Paul
2020-05-05  7:15 ` [Intel-gfx] ✓ Fi.CI.IGT: success for " Patchwork
2020-05-05  7:38 ` [Intel-gfx] [PATCH] " Ramalingam C
2020-05-05 12:06   ` Oliver Barta
2020-05-05 12:12     ` Ramalingam C
2020-06-30  8:06       ` Jani Nikula

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.