All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
@ 2023-08-28 17:34 Gustavo Sousa
  2023-08-28 18:50 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Gustavo Sousa @ 2023-08-28 17:34 UTC (permalink / raw)
  To: intel-gfx

We have experienced timeout issues when accessing C20 SRAM registers.
Experimentation showed that bumping the message bus timer threshold
helped on getting display Type-C connection on the C20 PHY to work.

While the timeout is still under investigation with the HW team, having
logic to allow forward progress (with the proper warnings) seems useful.
Thus, let's bump the threshold when a timeout is detected.

The maximum value of 0x200 pclk cycles was somewhat arbitrary - 2x the
default value. That value was successfully tested on real hardware that
was displaying timeouts otherwise.

BSpec: 65156
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 41 +++++++++++++++++++
 .../gpu/drm/i915/display/intel_cx0_phy_regs.h | 12 ++++++
 2 files changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index dd489b50ad60..55d2333032e3 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -5,6 +5,7 @@
 
 #include <linux/log2.h>
 #include <linux/math64.h>
+#include <linux/minmax.h>
 #include "i915_reg.h"
 #include "intel_cx0_phy.h"
 #include "intel_cx0_phy_regs.h"
@@ -29,6 +30,8 @@
 #define INTEL_CX0_LANE1		BIT(1)
 #define INTEL_CX0_BOTH_LANES	(INTEL_CX0_LANE1 | INTEL_CX0_LANE0)
 
+#define INTEL_CX0_MSGBUS_TIMER_VAL_MAX	0x200
+
 bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy)
 {
 	if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && phy < PHY_C)
@@ -119,6 +122,43 @@ static void intel_cx0_bus_reset(struct drm_i915_private *i915, enum port port, i
 	intel_clear_response_ready_flag(i915, port, lane);
 }
 
+/*
+ * Check if there was a timeout detected by the hardware in the message bus
+ * and bump the threshold if so.
+ */
+static void intel_cx0_bus_check_and_bump_timer(struct drm_i915_private *i915,
+					       enum port port, int lane)
+{
+	enum phy phy = intel_port_to_phy(i915, port);
+	i915_reg_t reg;
+	u32 val;
+	u32 timer_val;
+
+	reg = XELPDP_PORT_MSGBUS_TIMER(port, lane);
+	val = intel_de_read(i915, reg);
+
+	if (!(val & XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT)) {
+		drm_warn(&i915->drm,
+			 "PHY %c lane %d: hardware did not detect a timeout\n",
+			 phy_name(phy), lane);
+		return;
+	}
+
+	timer_val = REG_FIELD_GET(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK, val);
+
+	if (timer_val == INTEL_CX0_MSGBUS_TIMER_VAL_MAX)
+		return;
+
+	timer_val = min(2 * timer_val, (u32)INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
+	val &= ~XELPDP_PORT_MSGBUS_TIMER_VAL_MASK;
+	val |= REG_FIELD_PREP(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK, timer_val);
+
+	drm_dbg_kms(&i915->drm,
+		    "PHY %c lane %d: increasing msgbus timer threshold to %#x\n",
+		    phy_name(phy), lane, timer_val);
+	intel_de_write(i915, reg, val);
+}
+
 static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port port,
 				  int command, int lane, u32 *val)
 {
@@ -132,6 +172,7 @@ static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port port,
 					 XELPDP_MSGBUS_TIMEOUT_SLOW, val)) {
 		drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting for message ACK. Status: 0x%x\n",
 			    phy_name(phy), *val);
+		intel_cx0_bus_check_and_bump_timer(i915, port, lane);
 		intel_cx0_bus_reset(i915, port, lane);
 		return -ETIMEDOUT;
 	}
diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
index cb5d1be2ba19..17cc3385aabb 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
@@ -110,6 +110,18 @@
 #define   CX0_P4PG_STATE_DISABLE			0xC
 #define   CX0_P2_STATE_RESET				0x2
 
+#define _XELPDP_PORT_MSGBUS_TIMER_LN0_A			0x640d8
+#define _XELPDP_PORT_MSGBUS_TIMER_LN0_B			0x641d8
+#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1		0x16f258
+#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2		0x16f458
+#define XELPDP_PORT_MSGBUS_TIMER(port, lane)		_MMIO(_PICK_EVEN_2RANGES(port, PORT_TC1, \
+										 _XELPDP_PORT_MSGBUS_TIMER_LN0_A, \
+										 _XELPDP_PORT_MSGBUS_TIMER_LN0_B, \
+										 _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1, \
+										 _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2) + (lane) * 4)
+#define   XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT		REG_BIT(31)
+#define   XELPDP_PORT_MSGBUS_TIMER_VAL_MASK		REG_GENMASK(23, 0)
+
 #define _XELPDP_PORT_CLOCK_CTL_A			0x640E0
 #define _XELPDP_PORT_CLOCK_CTL_B			0x641E0
 #define _XELPDP_PORT_CLOCK_CTL_USBC1			0x16F260
-- 
2.41.0


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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/cx0: Check and increase msgbus timeout threshold
  2023-08-28 17:34 [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold Gustavo Sousa
@ 2023-08-28 18:50 ` Patchwork
  2023-08-28 19:08 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2023-08-28 18:50 UTC (permalink / raw)
  To: Gustavo Sousa; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/cx0: Check and increase msgbus timeout threshold
URL   : https://patchwork.freedesktop.org/series/122982/
State : warning

== Summary ==

Error: dim checkpatch failed
7a4474bf6960 drm/i915/cx0: Check and increase msgbus timeout threshold
-:107: WARNING:LONG_LINE: line length of 115 exceeds 100 columns
#107: FILE: drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h:118:
+										 _XELPDP_PORT_MSGBUS_TIMER_LN0_A, \

-:108: WARNING:LONG_LINE: line length of 115 exceeds 100 columns
#108: FILE: drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h:119:
+										 _XELPDP_PORT_MSGBUS_TIMER_LN0_B, \

-:109: WARNING:LONG_LINE: line length of 119 exceeds 100 columns
#109: FILE: drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h:120:
+										 _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1, \

-:110: WARNING:LONG_LINE: line length of 131 exceeds 100 columns
#110: FILE: drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h:121:
+										 _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2) + (lane) * 4)

total: 0 errors, 4 warnings, 0 checks, 83 lines checked



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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/cx0: Check and increase msgbus timeout threshold
  2023-08-28 17:34 [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold Gustavo Sousa
  2023-08-28 18:50 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2023-08-28 19:08 ` Patchwork
  2023-08-28 20:30 ` [Intel-gfx] [PATCH] " Sripada, Radhakrishna
  2023-08-28 22:13 ` [Intel-gfx] ✓ Fi.CI.IGT: success for " Patchwork
  3 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2023-08-28 19:08 UTC (permalink / raw)
  To: Gustavo Sousa; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 7710 bytes --]

== Series Details ==

Series: drm/i915/cx0: Check and increase msgbus timeout threshold
URL   : https://patchwork.freedesktop.org/series/122982/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13571 -> Patchwork_122982v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (38 -> 36)
------------------------------

  Additional (1): bat-adlp-11 
  Missing    (3): fi-kbl-soraka bat-dg2-9 fi-snb-2520m 

New tests
---------

  New tests have been introduced between CI_DRM_13571 and Patchwork_122982v1:

### New IGT tests (7) ###

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-nv12@pipe-b-dp-5:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-b-dp-5:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_pipe_crc_basic@hang-read-crc@pipe-b-dp-5:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence@pipe-b-dp-5:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-b-dp-5:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-b-dp-5:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_pipe_crc_basic@read-crc@pipe-b-dp-5:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-adlp-11:        NOTRUN -> [SKIP][1] ([i915#7456])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-adlp-11/igt@debugfs_test@basic-hwmon.html

  * igt@gem_tiled_pread_basic:
    - bat-adlp-11:        NOTRUN -> [SKIP][2] ([i915#3282])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-adlp-11/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - bat-adlp-11:        NOTRUN -> [ABORT][3] ([i915#8668])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-adlp-11/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_selftest@live@migrate:
    - bat-adlp-9:         [PASS][4] -> [DMESG-FAIL][5] ([i915#7699] / [i915#7913])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/bat-adlp-9/igt@i915_selftest@live@migrate.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-adlp-9/igt@i915_selftest@live@migrate.html

  * igt@i915_selftest@live@mman:
    - bat-rpls-1:         [PASS][6] -> [TIMEOUT][7] ([i915#6794] / [i915#7392])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/bat-rpls-1/igt@i915_selftest@live@mman.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-rpls-1/igt@i915_selftest@live@mman.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - bat-rpls-1:         [PASS][8] -> [WARN][9] ([i915#8747])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/bat-rpls-1/igt@i915_suspend@basic-s2idle-without-i915.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-rpls-1/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@i915_suspend@basic-s3-without-i915:
    - bat-mtlp-8:         NOTRUN -> [SKIP][10] ([i915#6645])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-mtlp-8/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-adlp-11:        NOTRUN -> [SKIP][11] ([i915#4103]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-adlp-11/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - bat-adlp-11:        NOTRUN -> [SKIP][12] ([i915#4093]) +3 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-adlp-11/igt@kms_force_connector_basic@prune-stale-modes.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-dg2-11:         NOTRUN -> [SKIP][13] ([i915#1845]) +3 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  * igt@kms_psr@primary_page_flip:
    - bat-adlp-6:         NOTRUN -> [ABORT][14] ([i915#8442] / [i915#8668])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-adlp-6/igt@kms_psr@primary_page_flip.html
    - bat-adlp-11:        NOTRUN -> [SKIP][15] ([i915#1072]) +3 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-adlp-11/igt@kms_psr@primary_page_flip.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-adlp-11:        NOTRUN -> [SKIP][16] ([i915#3555])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-adlp-11/igt@kms_setmode@basic-clone-single-crtc.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@requests:
    - bat-mtlp-8:         [ABORT][17] ([i915#7982] / [i915#8865]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/bat-mtlp-8/igt@i915_selftest@live@requests.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-mtlp-8/igt@i915_selftest@live@requests.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-a-edp-1:
    - bat-adlp-6:         [ABORT][19] ([i915#7977] / [i915#8469] / [i915#8668]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/bat-adlp-6/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-a-edp-1.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/bat-adlp-6/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-a-edp-1.html

  
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#4093]: https://gitlab.freedesktop.org/drm/intel/issues/4093
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#6794]: https://gitlab.freedesktop.org/drm/intel/issues/6794
  [i915#7392]: https://gitlab.freedesktop.org/drm/intel/issues/7392
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7977]: https://gitlab.freedesktop.org/drm/intel/issues/7977
  [i915#7982]: https://gitlab.freedesktop.org/drm/intel/issues/7982
  [i915#8442]: https://gitlab.freedesktop.org/drm/intel/issues/8442
  [i915#8469]: https://gitlab.freedesktop.org/drm/intel/issues/8469
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
  [i915#8747]: https://gitlab.freedesktop.org/drm/intel/issues/8747
  [i915#8865]: https://gitlab.freedesktop.org/drm/intel/issues/8865


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

  * Linux: CI_DRM_13571 -> Patchwork_122982v1

  CI-20190529: 20190529
  CI_DRM_13571: d3f0e020609215d9097f1b1fad5a13ea0b37548e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7454: 7454
  Patchwork_122982v1: d3f0e020609215d9097f1b1fad5a13ea0b37548e @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

6b1e8f24b9c4 drm/i915/cx0: Check and increase msgbus timeout threshold

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/index.html

[-- Attachment #2: Type: text/html, Size: 9031 bytes --]

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

* Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
  2023-08-28 17:34 [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold Gustavo Sousa
  2023-08-28 18:50 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
  2023-08-28 19:08 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2023-08-28 20:30 ` Sripada, Radhakrishna
  2023-08-28 21:45   ` Gustavo Sousa
  2023-08-28 22:13 ` [Intel-gfx] ✓ Fi.CI.IGT: success for " Patchwork
  3 siblings, 1 reply; 13+ messages in thread
From: Sripada, Radhakrishna @ 2023-08-28 20:30 UTC (permalink / raw)
  To: Sousa, Gustavo, intel-gfx

Hi Gustavo,

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Gustavo
> Sousa
> Sent: Monday, August 28, 2023 10:34 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout
> threshold
> 
> We have experienced timeout issues when accessing C20 SRAM registers.
This wording is misleading. It seems to imply that we directly access SRAM
registers via msg bus but the reads/writes go through intermediate registers
and it is this read/write that is failing. Adding extra clarity here would be helpful.
 
> Experimentation showed that bumping the message bus timer threshold
> helped on getting display Type-C connection on the C20 PHY to work.
> 
> While the timeout is still under investigation with the HW team, having
> logic to allow forward progress (with the proper warnings) seems useful.
> Thus, let's bump the threshold when a timeout is detected.
> 
> The maximum value of 0x200 pclk cycles was somewhat arbitrary - 2x the
> default value. That value was successfully tested on real hardware that
> was displaying timeouts otherwise. 
> 
> BSpec: 65156
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 41 +++++++++++++++++++
>  .../gpu/drm/i915/display/intel_cx0_phy_regs.h | 12 ++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index dd489b50ad60..55d2333032e3 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -5,6 +5,7 @@
> 
>  #include <linux/log2.h>
>  #include <linux/math64.h>
> +#include <linux/minmax.h>
>  #include "i915_reg.h"
>  #include "intel_cx0_phy.h"
>  #include "intel_cx0_phy_regs.h"
> @@ -29,6 +30,8 @@
>  #define INTEL_CX0_LANE1		BIT(1)
>  #define INTEL_CX0_BOTH_LANES	(INTEL_CX0_LANE1 |
> INTEL_CX0_LANE0)
> 
> +#define INTEL_CX0_MSGBUS_TIMER_VAL_MAX	0x200
> +
>  bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy)
>  {
>  	if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && phy < PHY_C)
> @@ -119,6 +122,43 @@ static void intel_cx0_bus_reset(struct drm_i915_private
> *i915, enum port port, i
>  	intel_clear_response_ready_flag(i915, port, lane);
>  }
> 
> +/*
> + * Check if there was a timeout detected by the hardware in the message bus
> + * and bump the threshold if so.
> + */
> +static void intel_cx0_bus_check_and_bump_timer(struct drm_i915_private
> *i915,
> +					       enum port port, int lane)
> +{
> +	enum phy phy = intel_port_to_phy(i915, port);
> +	i915_reg_t reg;
> +	u32 val;
> +	u32 timer_val;
> +
> +	reg = XELPDP_PORT_MSGBUS_TIMER(port, lane);
> +	val = intel_de_read(i915, reg);
> +
> +	if (!(val & XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT)) {
> +		drm_warn(&i915->drm,
> +			 "PHY %c lane %d: hardware did not detect a
> timeout\n",
> +			 phy_name(phy), lane);
> +		return;
> +	}
> +
> +	timer_val =
> REG_FIELD_GET(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK, val);
> +
> +	if (timer_val == INTEL_CX0_MSGBUS_TIMER_VAL_MAX)
> +		return;
> +
> +	timer_val = min(2 * timer_val,
> (u32)INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
     ^ is this cast necessary?

> +	val &= ~XELPDP_PORT_MSGBUS_TIMER_VAL_MASK;
> +	val |= REG_FIELD_PREP(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK,
> timer_val);
We do not use REG_FIELD_PREP in the function. Instead we use it in
register definition in intel_cx0_phy_regs.h.

Thanks,
Radhakrishna Sripada

> +
> +	drm_dbg_kms(&i915->drm,
> +		    "PHY %c lane %d: increasing msgbus timer threshold to
> %#x\n",
> +		    phy_name(phy), lane, timer_val);
> +	intel_de_write(i915, reg, val);
> +}
> +
>  static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port port,
>  				  int command, int lane, u32 *val)
>  {
> @@ -132,6 +172,7 @@ static int intel_cx0_wait_for_ack(struct
> drm_i915_private *i915, enum port port,
>  					 XELPDP_MSGBUS_TIMEOUT_SLOW,
> val)) {
>  		drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting for
> message ACK. Status: 0x%x\n",
>  			    phy_name(phy), *val);
> +		intel_cx0_bus_check_and_bump_timer(i915, port, lane);
>  		intel_cx0_bus_reset(i915, port, lane);
>  		return -ETIMEDOUT;
>  	}
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> index cb5d1be2ba19..17cc3385aabb 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> @@ -110,6 +110,18 @@
>  #define   CX0_P4PG_STATE_DISABLE			0xC
>  #define   CX0_P2_STATE_RESET				0x2
> 
> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_A
> 	0x640d8
> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_B
> 	0x641d8
> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1		0x16f258
> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2		0x16f458
> +#define XELPDP_PORT_MSGBUS_TIMER(port, lane)
> 	_MMIO(_PICK_EVEN_2RANGES(port, PORT_TC1, \
> +
> 	 _XELPDP_PORT_MSGBUS_TIMER_LN0_A, \
> +
> 	 _XELPDP_PORT_MSGBUS_TIMER_LN0_B, \
> +
> 	 _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1, \
> +
> 	 _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2) + (lane) * 4)
> +#define   XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT		REG_BIT(31)
> +#define   XELPDP_PORT_MSGBUS_TIMER_VAL_MASK
> 	REG_GENMASK(23, 0)
> +
>  #define _XELPDP_PORT_CLOCK_CTL_A			0x640E0
>  #define _XELPDP_PORT_CLOCK_CTL_B			0x641E0
>  #define _XELPDP_PORT_CLOCK_CTL_USBC1			0x16F260
> --
> 2.41.0


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

* Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
  2023-08-28 20:30 ` [Intel-gfx] [PATCH] " Sripada, Radhakrishna
@ 2023-08-28 21:45   ` Gustavo Sousa
  2023-08-28 22:53     ` Sripada, Radhakrishna
  2023-08-29 15:12     ` Jani Nikula
  0 siblings, 2 replies; 13+ messages in thread
From: Gustavo Sousa @ 2023-08-28 21:45 UTC (permalink / raw)
  To: Sripada, Radhakrishna, intel-gfx

Quoting Sripada, Radhakrishna (2023-08-28 17:30:19-03:00)
>Hi Gustavo,

Hi, RK.

Thanks for the feedback! Please, see my replies below.

>
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Gustavo
>> Sousa
>> Sent: Monday, August 28, 2023 10:34 AM
>> To: intel-gfx@lists.freedesktop.org
>> Subject: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout
>> threshold
>> 
>> We have experienced timeout issues when accessing C20 SRAM registers.
>This wording is misleading. It seems to imply that we directly access SRAM
>registers via msg bus but the reads/writes go through intermediate registers
>and it is this read/write that is failing. Adding extra clarity here would be helpful.

Hm... Okay. I thought that would already be implied to someone familiar with the
code. What about:

    s/when accessing/when going through the sequence to access/

?

> 
>> Experimentation showed that bumping the message bus timer threshold
>> helped on getting display Type-C connection on the C20 PHY to work.
>> 
>> While the timeout is still under investigation with the HW team, having
>> logic to allow forward progress (with the proper warnings) seems useful.
>> Thus, let's bump the threshold when a timeout is detected.
>> 
>> The maximum value of 0x200 pclk cycles was somewhat arbitrary - 2x the
>> default value. That value was successfully tested on real hardware that
>> was displaying timeouts otherwise. 
>> 
>> BSpec: 65156
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 41 +++++++++++++++++++
>>  .../gpu/drm/i915/display/intel_cx0_phy_regs.h | 12 ++++++
>>  2 files changed, 53 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> index dd489b50ad60..55d2333032e3 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> @@ -5,6 +5,7 @@
>> 
>>  #include <linux/log2.h>
>>  #include <linux/math64.h>
>> +#include <linux/minmax.h>
>>  #include "i915_reg.h"
>>  #include "intel_cx0_phy.h"
>>  #include "intel_cx0_phy_regs.h"
>> @@ -29,6 +30,8 @@
>>  #define INTEL_CX0_LANE1                BIT(1)
>>  #define INTEL_CX0_BOTH_LANES        (INTEL_CX0_LANE1 |
>> INTEL_CX0_LANE0)
>> 
>> +#define INTEL_CX0_MSGBUS_TIMER_VAL_MAX        0x200
>> +
>>  bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy)
>>  {
>>          if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && phy < PHY_C)
>> @@ -119,6 +122,43 @@ static void intel_cx0_bus_reset(struct drm_i915_private
>> *i915, enum port port, i
>>          intel_clear_response_ready_flag(i915, port, lane);
>>  }
>> 
>> +/*
>> + * Check if there was a timeout detected by the hardware in the message bus
>> + * and bump the threshold if so.
>> + */
>> +static void intel_cx0_bus_check_and_bump_timer(struct drm_i915_private
>> *i915,
>> +                                               enum port port, int lane)
>> +{
>> +        enum phy phy = intel_port_to_phy(i915, port);
>> +        i915_reg_t reg;
>> +        u32 val;
>> +        u32 timer_val;
>> +
>> +        reg = XELPDP_PORT_MSGBUS_TIMER(port, lane);
>> +        val = intel_de_read(i915, reg);
>> +
>> +        if (!(val & XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT)) {
>> +                drm_warn(&i915->drm,
>> +                         "PHY %c lane %d: hardware did not detect a
>> timeout\n",
>> +                         phy_name(phy), lane);
>> +                return;
>> +        }
>> +
>> +        timer_val =
>> REG_FIELD_GET(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK, val);
>> +
>> +        if (timer_val == INTEL_CX0_MSGBUS_TIMER_VAL_MAX)
>> +                return;
>> +
>> +        timer_val = min(2 * timer_val,
>> (u32)INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
>     ^ is this cast necessary?

Yes. I remember getting a warning without it. Let me remove it to remember...

...done! I think it complains because the arguments are of different types:

    In file included from drivers/gpu/drm/i915/display/intel_cx0_phy.c:8:
    drivers/gpu/drm/i915/display/intel_cx0_phy.c: In function ‘intel_cx0_bus_check_and_bump_timer’:
    ./include/linux/minmax.h:20:35: error: comparison of distinct pointer types lacks a cast [-Werror]
       20 |         (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
          |                                   ^~
    ./include/linux/minmax.h:26:18: note: in expansion of macro ‘__typecheck’
       26 |                 (__typecheck(x, y) && __no_side_effects(x, y))
          |                  ^~~~~~~~~~~
    ./include/linux/minmax.h:36:31: note: in expansion of macro ‘__safe_cmp’
       36 |         __builtin_choose_expr(__safe_cmp(x, y), \
          |                               ^~~~~~~~~~
    ./include/linux/minmax.h:67:25: note: in expansion of macro ‘__careful_cmp’
       67 | #define min(x, y)       __careful_cmp(x, y, <)
          |                         ^~~~~~~~~~~~~
    drivers/gpu/drm/i915/display/intel_cx0_phy.c:152:21: note: in expansion of macro ‘min’
      152 |         timer_val = min(2 * timer_val, INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
          |

>
>> +        val &= ~XELPDP_PORT_MSGBUS_TIMER_VAL_MASK;
>> +        val |= REG_FIELD_PREP(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK,
>> timer_val);
>We do not use REG_FIELD_PREP in the function. Instead we use it in
>register definition in intel_cx0_phy_regs.h.

I think it makes sense have definitions using REG_FIELD_PREP() in header files
and use those definitions in .c files for fields whose values are are
enumerated.

Now, for fields without enumeration, like this one in discussion, I think it is
fine to use REG_FIELD_PREP() directly. The MASK is already exposed anyway...

Besides, it seems we already have lines of code in *.c files using the pattern
above:

    git grep REG_FIELD_PREP drm-tip/drm-tip -- ':/drivers/gpu/drm/i915/**/*.c'

Let me know what you think. I could add a XELPDP_PORT_MSGBUS_TIMER_VAL() macro
if you think it is necessary. My personal take is that using REG_FIELD_PREP()
for this case is fine.

--
Gustavo Sousa

>
>Thanks,
>Radhakrishna Sripada
>
>> +
>> +        drm_dbg_kms(&i915->drm,
>> +                    "PHY %c lane %d: increasing msgbus timer threshold to
>> %#x\n",
>> +                    phy_name(phy), lane, timer_val);
>> +        intel_de_write(i915, reg, val);
>> +}
>> +
>>  static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port port,
>>                                    int command, int lane, u32 *val)
>>  {
>> @@ -132,6 +172,7 @@ static int intel_cx0_wait_for_ack(struct
>> drm_i915_private *i915, enum port port,
>>                                           XELPDP_MSGBUS_TIMEOUT_SLOW,
>> val)) {
>>                  drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting for
>> message ACK. Status: 0x%x\n",
>>                              phy_name(phy), *val);
>> +                intel_cx0_bus_check_and_bump_timer(i915, port, lane);
>>                  intel_cx0_bus_reset(i915, port, lane);
>>                  return -ETIMEDOUT;
>>          }
>> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>> b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>> index cb5d1be2ba19..17cc3385aabb 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>> @@ -110,6 +110,18 @@
>>  #define   CX0_P4PG_STATE_DISABLE                        0xC
>>  #define   CX0_P2_STATE_RESET                                0x2
>> 
>> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_A
>>         0x640d8
>> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_B
>>         0x641d8
>> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1                0x16f258
>> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2                0x16f458
>> +#define XELPDP_PORT_MSGBUS_TIMER(port, lane)
>>         _MMIO(_PICK_EVEN_2RANGES(port, PORT_TC1, \
>> +
>>          _XELPDP_PORT_MSGBUS_TIMER_LN0_A, \
>> +
>>          _XELPDP_PORT_MSGBUS_TIMER_LN0_B, \
>> +
>>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1, \
>> +
>>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2) + (lane) * 4)
>> +#define   XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT                REG_BIT(31)
>> +#define   XELPDP_PORT_MSGBUS_TIMER_VAL_MASK
>>         REG_GENMASK(23, 0)
>> +
>>  #define _XELPDP_PORT_CLOCK_CTL_A                        0x640E0
>>  #define _XELPDP_PORT_CLOCK_CTL_B                        0x641E0
>>  #define _XELPDP_PORT_CLOCK_CTL_USBC1                        0x16F260
>> --
>> 2.41.0
>

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

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/cx0: Check and increase msgbus timeout threshold
  2023-08-28 17:34 [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold Gustavo Sousa
                   ` (2 preceding siblings ...)
  2023-08-28 20:30 ` [Intel-gfx] [PATCH] " Sripada, Radhakrishna
@ 2023-08-28 22:13 ` Patchwork
  3 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2023-08-28 22:13 UTC (permalink / raw)
  To: Gustavo Sousa; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 43114 bytes --]

== Series Details ==

Series: drm/i915/cx0: Check and increase msgbus timeout threshold
URL   : https://patchwork.freedesktop.org/series/122982/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13571_full -> Patchwork_122982v1_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (9 -> 9)
------------------------------

  No changes in participating hosts

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

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

### IGT changes ###

#### Issues hit ####

  * igt@api_intel_bb@render-ccs:
    - shard-dg2:          NOTRUN -> [FAIL][1] ([i915#6122])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@api_intel_bb@render-ccs.html

  * igt@drm_fdinfo@busy-hang@bcs0:
    - shard-dg2:          NOTRUN -> [SKIP][2] ([i915#8414]) +9 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@drm_fdinfo@busy-hang@bcs0.html

  * igt@gem_ctx_persistence@engines-hostile:
    - shard-snb:          NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1099]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-snb7/igt@gem_ctx_persistence@engines-hostile.html

  * igt@gem_ctx_persistence@hang:
    - shard-dg2:          NOTRUN -> [SKIP][4] ([i915#8555])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@gem_ctx_persistence@hang.html

  * igt@gem_ctx_sseu@invalid-sseu:
    - shard-dg2:          NOTRUN -> [SKIP][5] ([i915#280]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@gem_ctx_sseu@invalid-sseu.html

  * igt@gem_eio@in-flight-contexts-10ms:
    - shard-mtlp:         [PASS][6] -> [ABORT][7] ([i915#7941])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-mtlp-6/igt@gem_eio@in-flight-contexts-10ms.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-1/igt@gem_eio@in-flight-contexts-10ms.html

  * igt@gem_eio@in-flight-contexts-1us:
    - shard-mtlp:         [PASS][8] -> [ABORT][9] ([i915#8503])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-mtlp-4/igt@gem_eio@in-flight-contexts-1us.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-2/igt@gem_eio@in-flight-contexts-1us.html

  * igt@gem_eio@kms:
    - shard-dg1:          [PASS][10] -> [FAIL][11] ([i915#5784])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg1-13/igt@gem_eio@kms.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg1-13/igt@gem_eio@kms.html

  * igt@gem_exec_balancer@bonded-semaphore:
    - shard-dg2:          NOTRUN -> [SKIP][12] ([i915#4812])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@gem_exec_balancer@bonded-semaphore.html

  * igt@gem_exec_flush@basic-batch-kernel-default-uc:
    - shard-mtlp:         [PASS][13] -> [DMESG-FAIL][14] ([i915#8962] / [i915#9121])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-mtlp-2/igt@gem_exec_flush@basic-batch-kernel-default-uc.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-4/igt@gem_exec_flush@basic-batch-kernel-default-uc.html

  * igt@gem_exec_flush@basic-uc-rw-default:
    - shard-dg2:          NOTRUN -> [SKIP][15] ([i915#3539] / [i915#4852]) +2 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@gem_exec_flush@basic-uc-rw-default.html

  * igt@gem_exec_reloc@basic-cpu-gtt-active:
    - shard-dg2:          NOTRUN -> [SKIP][16] ([i915#3281]) +7 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@gem_exec_reloc@basic-cpu-gtt-active.html

  * igt@gem_exec_schedule@preempt-queue-contexts-chain:
    - shard-dg2:          NOTRUN -> [SKIP][17] ([i915#4537] / [i915#4812])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@gem_exec_schedule@preempt-queue-contexts-chain.html

  * igt@gem_exec_suspend@basic-s0@smem:
    - shard-dg2:          [PASS][18] -> [INCOMPLETE][19] ([i915#6311])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg2-10/igt@gem_exec_suspend@basic-s0@smem.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-2/igt@gem_exec_suspend@basic-s0@smem.html

  * igt@gem_fence_thrash@bo-write-verify-none:
    - shard-dg2:          NOTRUN -> [SKIP][20] ([i915#4860])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@gem_fence_thrash@bo-write-verify-none.html

  * igt@gem_lmem_swapping@verify-random:
    - shard-apl:          NOTRUN -> [SKIP][21] ([fdo#109271] / [i915#4613])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-apl1/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_media_fill@media-fill:
    - shard-dg2:          NOTRUN -> [SKIP][22] ([i915#8289])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@gem_media_fill@media-fill.html

  * igt@gem_mmap_gtt@basic-read-write-distinct:
    - shard-dg2:          NOTRUN -> [SKIP][23] ([i915#4077]) +5 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@gem_mmap_gtt@basic-read-write-distinct.html

  * igt@gem_mmap_wc@bad-offset:
    - shard-dg2:          NOTRUN -> [SKIP][24] ([i915#4083]) +1 similar issue
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@gem_mmap_wc@bad-offset.html

  * igt@gem_partial_pwrite_pread@reads:
    - shard-dg2:          NOTRUN -> [SKIP][25] ([i915#3282]) +3 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@gem_partial_pwrite_pread@reads.html

  * igt@gem_partial_pwrite_pread@write-uncached:
    - shard-mtlp:         NOTRUN -> [SKIP][26] ([i915#3282])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-5/igt@gem_partial_pwrite_pread@write-uncached.html

  * igt@gem_pxp@create-regular-context-2:
    - shard-apl:          NOTRUN -> [SKIP][27] ([fdo#109271]) +51 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-apl1/igt@gem_pxp@create-regular-context-2.html

  * igt@gem_pxp@regular-baseline-src-copy-readible:
    - shard-dg2:          NOTRUN -> [SKIP][28] ([i915#4270]) +2 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@gem_pxp@regular-baseline-src-copy-readible.html

  * igt@gem_spin_batch@spin-all-new:
    - shard-dg2:          NOTRUN -> [FAIL][29] ([i915#5889])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@gem_spin_batch@spin-all-new.html

  * igt@gem_tiled_partial_pwrite_pread@writes-after-reads:
    - shard-mtlp:         NOTRUN -> [SKIP][30] ([i915#4077])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-6/igt@gem_tiled_partial_pwrite_pread@writes-after-reads.html

  * igt@gem_userptr_blits@create-destroy-unsync:
    - shard-dg2:          NOTRUN -> [SKIP][31] ([i915#3297]) +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@gem_userptr_blits@create-destroy-unsync.html

  * igt@gen7_exec_parse@cmd-crossing-page:
    - shard-dg2:          NOTRUN -> [SKIP][32] ([fdo#109289])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@gen7_exec_parse@cmd-crossing-page.html

  * igt@gen9_exec_parse@bb-secure:
    - shard-dg2:          NOTRUN -> [SKIP][33] ([i915#2856]) +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@gen9_exec_parse@bb-secure.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp:
    - shard-dg2:          NOTRUN -> [SKIP][34] ([i915#1937])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a:
    - shard-dg1:          [PASS][35] -> [SKIP][36] ([i915#1937])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg1-19/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg1-13/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html

  * igt@i915_pm_rpm@modeset-lpsp-stress:
    - shard-dg2:          [PASS][37] -> [SKIP][38] ([i915#1397])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg2-12/igt@i915_pm_rpm@modeset-lpsp-stress.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-6/igt@i915_pm_rpm@modeset-lpsp-stress.html
    - shard-rkl:          [PASS][39] -> [SKIP][40] ([i915#1397])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-rkl-7/igt@i915_pm_rpm@modeset-lpsp-stress.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-rkl-1/igt@i915_pm_rpm@modeset-lpsp-stress.html

  * igt@i915_pm_rpm@system-suspend:
    - shard-dg2:          [PASS][41] -> [INCOMPLETE][42] ([i915#9142])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg2-10/igt@i915_pm_rpm@system-suspend.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@i915_pm_rpm@system-suspend.html

  * igt@i915_pm_sseu@full-enable:
    - shard-dg2:          NOTRUN -> [SKIP][43] ([i915#4387])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@i915_pm_sseu@full-enable.html

  * igt@i915_query@query-topology-coherent-slice-mask:
    - shard-dg2:          NOTRUN -> [SKIP][44] ([i915#6188])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@i915_query@query-topology-coherent-slice-mask.html

  * igt@i915_suspend@sysfs-reader:
    - shard-snb:          NOTRUN -> [DMESG-FAIL][45] ([fdo#103375])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-snb7/igt@i915_suspend@sysfs-reader.html

  * igt@kms_addfb_basic@clobberred-modifier:
    - shard-dg2:          NOTRUN -> [SKIP][46] ([i915#4212]) +1 similar issue
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@kms_addfb_basic@clobberred-modifier.html

  * igt@kms_async_flips@async-flip-with-page-flip-events@pipe-a-hdmi-a-3-4-mc_ccs:
    - shard-dg2:          NOTRUN -> [SKIP][47] ([i915#8502] / [i915#8709]) +11 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-6/igt@kms_async_flips@async-flip-with-page-flip-events@pipe-a-hdmi-a-3-4-mc_ccs.html

  * igt@kms_atomic_transition@plane-all-modeset-transition-fencing-internal-panels:
    - shard-dg2:          NOTRUN -> [SKIP][48] ([i915#1769] / [i915#3555]) +1 similar issue
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@kms_atomic_transition@plane-all-modeset-transition-fencing-internal-panels.html

  * igt@kms_big_fb@4-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip:
    - shard-mtlp:         [PASS][49] -> [FAIL][50] ([i915#3743])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-mtlp-2/igt@kms_big_fb@4-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-4/igt@kms_big_fb@4-tiled-max-hw-stride-64bpp-rotate-180-hflip-async-flip.html

  * igt@kms_big_fb@y-tiled-addfb-size-offset-overflow:
    - shard-dg2:          NOTRUN -> [SKIP][51] ([i915#5190]) +11 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@kms_big_fb@y-tiled-addfb-size-offset-overflow.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-async-flip:
    - shard-tglu:         [PASS][52] -> [FAIL][53] ([i915#3743])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-tglu-3/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-async-flip.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-tglu-2/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-async-flip.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0:
    - shard-dg2:          NOTRUN -> [SKIP][54] ([i915#4538] / [i915#5190]) +5 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0.html

  * igt@kms_big_joiner@invalid-modeset:
    - shard-dg2:          NOTRUN -> [SKIP][55] ([i915#2705])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@kms_big_joiner@invalid-modeset.html

  * igt@kms_ccs@pipe-a-random-ccs-data-y_tiled_gen12_mc_ccs:
    - shard-dg2:          NOTRUN -> [SKIP][56] ([i915#3689] / [i915#3886] / [i915#5354]) +7 similar issues
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@kms_ccs@pipe-a-random-ccs-data-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-b-bad-rotation-90-yf_tiled_ccs:
    - shard-dg2:          NOTRUN -> [SKIP][57] ([i915#3689] / [i915#5354]) +9 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@kms_ccs@pipe-b-bad-rotation-90-yf_tiled_ccs.html

  * igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][58] ([fdo#109271] / [i915#3886]) +1 similar issue
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-apl1/igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_mc_ccs.html

  * igt@kms_cdclk@plane-scaling@pipe-c-hdmi-a-3:
    - shard-dg2:          NOTRUN -> [SKIP][59] ([i915#4087]) +3 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-6/igt@kms_cdclk@plane-scaling@pipe-c-hdmi-a-3.html

  * igt@kms_chamelium_color@ctm-0-25:
    - shard-dg2:          NOTRUN -> [SKIP][60] ([fdo#111827]) +1 similar issue
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@kms_chamelium_color@ctm-0-25.html

  * igt@kms_chamelium_edid@vga-edid-read:
    - shard-mtlp:         NOTRUN -> [SKIP][61] ([i915#7828])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-6/igt@kms_chamelium_edid@vga-edid-read.html

  * igt@kms_chamelium_frames@hdmi-crc-multiple:
    - shard-dg2:          NOTRUN -> [SKIP][62] ([i915#7828]) +4 similar issues
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@kms_chamelium_frames@hdmi-crc-multiple.html

  * igt@kms_content_protection@dp-mst-lic-type-0:
    - shard-dg2:          NOTRUN -> [SKIP][63] ([i915#3299])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@kms_content_protection@dp-mst-lic-type-0.html

  * igt@kms_content_protection@srm:
    - shard-dg2:          NOTRUN -> [SKIP][64] ([i915#7118]) +1 similar issue
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-6/igt@kms_content_protection@srm.html

  * igt@kms_content_protection@uevent@pipe-a-dp-2:
    - shard-dg2:          NOTRUN -> [FAIL][65] ([i915#1339])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@kms_content_protection@uevent@pipe-a-dp-2.html

  * igt@kms_cursor_crc@cursor-sliding-512x170:
    - shard-dg2:          NOTRUN -> [SKIP][66] ([i915#3359])
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@kms_cursor_crc@cursor-sliding-512x170.html

  * igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic:
    - shard-apl:          NOTRUN -> [SKIP][67] ([fdo#109271] / [fdo#111767])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-apl1/igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - shard-dg2:          NOTRUN -> [SKIP][68] ([i915#4103] / [i915#4213])
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@cursorb-vs-flipb-legacy:
    - shard-dg2:          NOTRUN -> [SKIP][69] ([fdo#109274] / [i915#5354]) +3 similar issues
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@kms_cursor_legacy@cursorb-vs-flipb-legacy.html

  * igt@kms_draw_crc@draw-method-mmap-wc:
    - shard-dg2:          NOTRUN -> [SKIP][70] ([i915#8812])
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@kms_draw_crc@draw-method-mmap-wc.html

  * igt@kms_dsc@dsc-basic:
    - shard-dg2:          NOTRUN -> [SKIP][71] ([i915#3555] / [i915#3840])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@kms_dsc@dsc-basic.html

  * igt@kms_flip@2x-flip-vs-blocking-wf-vblank:
    - shard-snb:          NOTRUN -> [SKIP][72] ([fdo#109271] / [fdo#111767])
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-snb4/igt@kms_flip@2x-flip-vs-blocking-wf-vblank.html

  * igt@kms_flip@2x-wf_vblank-ts-check-interruptible:
    - shard-dg2:          NOTRUN -> [SKIP][73] ([fdo#109274]) +5 similar issues
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@kms_flip@2x-wf_vblank-ts-check-interruptible.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-hdmi-a3:
    - shard-dg2:          [PASS][74] -> [FAIL][75] ([fdo#103375])
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg2-3/igt@kms_flip@flip-vs-suspend-interruptible@a-hdmi-a3.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-1/igt@kms_flip@flip-vs-suspend-interruptible@a-hdmi-a3.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile-upscaling@pipe-a-valid-mode:
    - shard-dg2:          NOTRUN -> [SKIP][76] ([i915#2672]) +2 similar issues
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile-upscaling@pipe-a-valid-mode.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-dg2:          NOTRUN -> [SKIP][77] ([i915#8708]) +11 similar issues
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-indfb-plflip-blt:
    - shard-mtlp:         NOTRUN -> [SKIP][78] ([i915#1825])
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-5/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-indfb-plflip-blt.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render:
    - shard-dg2:          NOTRUN -> [SKIP][79] ([i915#3458]) +13 similar issues
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@psr-2p-primscrn-pri-shrfb-draw-render:
    - shard-dg2:          NOTRUN -> [SKIP][80] ([i915#5354]) +25 similar issues
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@kms_frontbuffer_tracking@psr-2p-primscrn-pri-shrfb-draw-render.html

  * igt@kms_hdmi_inject@inject-audio:
    - shard-tglu:         [PASS][81] -> [SKIP][82] ([i915#433])
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-tglu-9/igt@kms_hdmi_inject@inject-audio.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-tglu-10/igt@kms_hdmi_inject@inject-audio.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-dg2:          NOTRUN -> [SKIP][83] ([i915#3555] / [i915#8228])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-3/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-hdmi-a-1:
    - shard-snb:          NOTRUN -> [DMESG-WARN][84] ([i915#8841]) +3 similar issues
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-snb1/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-a-hdmi-a-1.html

  * igt@kms_plane_scaling@plane-downscale-with-modifiers-factor-0-25@pipe-b-hdmi-a-1:
    - shard-dg1:          NOTRUN -> [SKIP][85] ([i915#5176]) +3 similar issues
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg1-19/igt@kms_plane_scaling@plane-downscale-with-modifiers-factor-0-25@pipe-b-hdmi-a-1.html

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-a-hdmi-a-3:
    - shard-dg2:          NOTRUN -> [SKIP][86] ([i915#5176]) +3 similar issues
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-6/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-a-hdmi-a-3.html

  * igt@kms_plane_scaling@plane-downscale-with-rotation-factor-0-25@pipe-b-hdmi-a-1:
    - shard-rkl:          NOTRUN -> [SKIP][87] ([i915#5176]) +7 similar issues
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-rkl-7/igt@kms_plane_scaling@plane-downscale-with-rotation-factor-0-25@pipe-b-hdmi-a-1.html

  * igt@kms_plane_scaling@planes-downscale-factor-0-25-unity-scaling@pipe-c-hdmi-a-2:
    - shard-dg2:          NOTRUN -> [SKIP][88] ([i915#5235]) +15 similar issues
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-2/igt@kms_plane_scaling@planes-downscale-factor-0-25-unity-scaling@pipe-c-hdmi-a-2.html

  * igt@kms_plane_scaling@planes-downscale-factor-0-25-upscale-factor-0-25@pipe-d-hdmi-a-3:
    - shard-dg1:          NOTRUN -> [SKIP][89] ([i915#5235]) +7 similar issues
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg1-13/igt@kms_plane_scaling@planes-downscale-factor-0-25-upscale-factor-0-25@pipe-d-hdmi-a-3.html

  * igt@kms_plane_scaling@planes-downscale-factor-0-5-unity-scaling@pipe-b-vga-1:
    - shard-snb:          NOTRUN -> [SKIP][90] ([fdo#109271]) +174 similar issues
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-snb7/igt@kms_plane_scaling@planes-downscale-factor-0-5-unity-scaling@pipe-b-vga-1.html

  * igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-25@pipe-b-hdmi-a-2:
    - shard-rkl:          NOTRUN -> [SKIP][91] ([i915#5235]) +3 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-rkl-4/igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-25@pipe-b-hdmi-a-2.html

  * igt@kms_prime@d3hot:
    - shard-dg2:          NOTRUN -> [SKIP][92] ([i915#6524] / [i915#6805])
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@kms_prime@d3hot.html

  * igt@kms_psr2_su@page_flip-nv12:
    - shard-dg2:          NOTRUN -> [SKIP][93] ([i915#658]) +1 similar issue
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@kms_psr2_su@page_flip-nv12.html

  * igt@kms_psr2_su@page_flip-xrgb8888:
    - shard-apl:          NOTRUN -> [SKIP][94] ([fdo#109271] / [i915#658])
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-apl1/igt@kms_psr2_su@page_flip-xrgb8888.html

  * igt@kms_psr@psr2_primary_mmap_gtt:
    - shard-dg2:          NOTRUN -> [SKIP][95] ([i915#1072]) +4 similar issues
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@kms_psr@psr2_primary_mmap_gtt.html

  * igt@kms_rotation_crc@primary-y-tiled-reflect-x-270:
    - shard-dg2:          NOTRUN -> [SKIP][96] ([i915#4235] / [i915#5190])
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@kms_rotation_crc@primary-y-tiled-reflect-x-270.html

  * igt@kms_selftest@drm_damage:
    - shard-dg2:          NOTRUN -> [SKIP][97] ([i915#8661]) +2 similar issues
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@kms_selftest@drm_damage.html

  * igt@kms_setmode@basic@pipe-a-vga-1:
    - shard-snb:          NOTRUN -> [FAIL][98] ([i915#5465]) +1 similar issue
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-snb4/igt@kms_setmode@basic@pipe-a-vga-1.html

  * igt@kms_tiled_display@basic-test-pattern:
    - shard-dg2:          NOTRUN -> [SKIP][99] ([i915#8623])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@kms_tiled_display@basic-test-pattern.html

  * igt@kms_vrr@flip-basic:
    - shard-dg2:          NOTRUN -> [SKIP][100] ([i915#3555]) +2 similar issues
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@kms_vrr@flip-basic.html

  * igt@kms_writeback@writeback-fb-id:
    - shard-dg2:          NOTRUN -> [SKIP][101] ([i915#2437])
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@kms_writeback@writeback-fb-id.html

  * igt@perf_pmu@rc6@other-idle-gt0:
    - shard-dg2:          NOTRUN -> [SKIP][102] ([i915#8516])
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@perf_pmu@rc6@other-idle-gt0.html

  * igt@perf_pmu@render-node-busy-idle@vcs1:
    - shard-dg2:          NOTRUN -> [FAIL][103] ([i915#4349]) +6 similar issues
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@perf_pmu@render-node-busy-idle@vcs1.html

  * igt@v3d/v3d_job_submission@array-job-submission:
    - shard-dg2:          NOTRUN -> [SKIP][104] ([i915#2575]) +8 similar issues
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@v3d/v3d_job_submission@array-job-submission.html

  * igt@vc4/vc4_wait_bo@unused-bo-0ns:
    - shard-dg2:          NOTRUN -> [SKIP][105] ([i915#7711]) +3 similar issues
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@vc4/vc4_wait_bo@unused-bo-0ns.html

  
#### Possible fixes ####

  * igt@gem_ccs@suspend-resume@linear-compressed-compfmt0-smem-lmem0:
    - shard-dg2:          [INCOMPLETE][106] ([i915#6311] / [i915#7297]) -> [PASS][107]
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg2-2/igt@gem_ccs@suspend-resume@linear-compressed-compfmt0-smem-lmem0.html
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@gem_ccs@suspend-resume@linear-compressed-compfmt0-smem-lmem0.html

  * igt@gem_ctx_isolation@preservation-s3@bcs0:
    - shard-dg2:          [INCOMPLETE][108] -> [PASS][109]
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg2-5/igt@gem_ctx_isolation@preservation-s3@bcs0.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@gem_ctx_isolation@preservation-s3@bcs0.html

  * igt@gem_ctx_isolation@preservation-s3@rcs0:
    - shard-dg2:          [FAIL][110] ([fdo#103375]) -> [PASS][111]
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg2-5/igt@gem_ctx_isolation@preservation-s3@rcs0.html
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-12/igt@gem_ctx_isolation@preservation-s3@rcs0.html

  * igt@gem_exec_capture@pi@vcs0:
    - shard-mtlp:         [FAIL][112] ([i915#4475]) -> [PASS][113]
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-mtlp-8/igt@gem_exec_capture@pi@vcs0.html
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-3/igt@gem_exec_capture@pi@vcs0.html

  * igt@gem_exec_capture@pi@vcs1:
    - shard-mtlp:         [FAIL][114] ([i915#4475] / [i915#7765]) -> [PASS][115]
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-mtlp-8/igt@gem_exec_capture@pi@vcs1.html
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-3/igt@gem_exec_capture@pi@vcs1.html

  * igt@gem_exec_fair@basic-deadline:
    - shard-glk:          [FAIL][116] ([i915#2846]) -> [PASS][117]
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-glk3/igt@gem_exec_fair@basic-deadline.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-glk1/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-tglu:         [FAIL][118] ([i915#2842]) -> [PASS][119]
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-tglu-7/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-tglu-10/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_fair@basic-pace@bcs0:
    - shard-rkl:          [FAIL][120] ([i915#2842]) -> [PASS][121] +2 similar issues
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-rkl-7/igt@gem_exec_fair@basic-pace@bcs0.html
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-rkl-4/igt@gem_exec_fair@basic-pace@bcs0.html

  * igt@gem_exec_fair@basic-pace@rcs0:
    - shard-glk:          [FAIL][122] ([i915#2842]) -> [PASS][123]
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-glk5/igt@gem_exec_fair@basic-pace@rcs0.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-glk5/igt@gem_exec_fair@basic-pace@rcs0.html

  * igt@gem_lmem_swapping@smem-oom@lmem0:
    - shard-dg2:          [DMESG-WARN][124] ([i915#4936] / [i915#5493]) -> [PASS][125]
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg2-3/igt@gem_lmem_swapping@smem-oom@lmem0.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-1/igt@gem_lmem_swapping@smem-oom@lmem0.html
    - shard-dg1:          [TIMEOUT][126] ([i915#5493]) -> [PASS][127]
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg1-15/igt@gem_lmem_swapping@smem-oom@lmem0.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg1-17/igt@gem_lmem_swapping@smem-oom@lmem0.html

  * igt@gem_mmap_offset@clear@smem0:
    - shard-dg1:          [FAIL][128] ([i915#7962]) -> [PASS][129]
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg1-14/igt@gem_mmap_offset@clear@smem0.html
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg1-16/igt@gem_mmap_offset@clear@smem0.html

  * igt@i915_pm_rpm@dpms-mode-unset-non-lpsp:
    - shard-dg2:          [SKIP][130] ([i915#1397]) -> [PASS][131] +2 similar issues
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg2-10/igt@i915_pm_rpm@dpms-mode-unset-non-lpsp.html
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@i915_pm_rpm@dpms-mode-unset-non-lpsp.html
    - shard-rkl:          [SKIP][132] ([i915#1397]) -> [PASS][133] +2 similar issues
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-rkl-7/igt@i915_pm_rpm@dpms-mode-unset-non-lpsp.html
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-rkl-4/igt@i915_pm_rpm@dpms-mode-unset-non-lpsp.html

  * igt@i915_pm_rpm@modeset-non-lpsp-stress:
    - shard-dg1:          [SKIP][134] ([i915#1397]) -> [PASS][135]
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg1-19/igt@i915_pm_rpm@modeset-non-lpsp-stress.html
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg1-15/igt@i915_pm_rpm@modeset-non-lpsp-stress.html

  * igt@kms_async_flips@alternate-sync-async-flip@pipe-a-edp-1:
    - shard-mtlp:         [FAIL][136] ([i915#2521]) -> [PASS][137]
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-mtlp-6/igt@kms_async_flips@alternate-sync-async-flip@pipe-a-edp-1.html
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-5/igt@kms_async_flips@alternate-sync-async-flip@pipe-a-edp-1.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip:
    - shard-mtlp:         [FAIL][138] ([i915#3743]) -> [PASS][139] +1 similar issue
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-mtlp-4/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-5/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-apl:          [FAIL][140] ([i915#2346]) -> [PASS][141]
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-apl4/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-apl1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-dg2:          [FAIL][142] ([i915#6880]) -> [PASS][143]
   [142]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg2-10/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [143]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-8/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_plane@pixel-format-source-clamping@pipe-b-planes:
    - shard-mtlp:         [FAIL][144] ([i915#1623]) -> [PASS][145]
   [144]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-mtlp-6/igt@kms_plane@pixel-format-source-clamping@pipe-b-planes.html
   [145]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-mtlp-5/igt@kms_plane@pixel-format-source-clamping@pipe-b-planes.html

  * igt@kms_properties@connector-properties-legacy:
    - shard-dg2:          [FAIL][146] -> [PASS][147]
   [146]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg2-11/igt@kms_properties@connector-properties-legacy.html
   [147]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg2-11/igt@kms_properties@connector-properties-legacy.html

  
#### Warnings ####

  * igt@gem_eio@unwedge-stress:
    - shard-snb:          [FAIL][148] ([i915#8898]) -> [ABORT][149] ([i915#7941] / [i915#8865])
   [148]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-snb2/igt@gem_eio@unwedge-stress.html
   [149]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-snb7/igt@gem_eio@unwedge-stress.html

  * igt@i915_pm_rc6_residency@rc6-idle@rcs0:
    - shard-tglu:         [WARN][150] ([i915#2681]) -> [FAIL][151] ([i915#2681] / [i915#3591])
   [150]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-tglu-7/igt@i915_pm_rc6_residency@rc6-idle@rcs0.html
   [151]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-tglu-10/igt@i915_pm_rc6_residency@rc6-idle@rcs0.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-rkl:          [SKIP][152] ([i915#3955]) -> [SKIP][153] ([fdo#110189] / [i915#3955])
   [152]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-rkl-7/igt@kms_fbcon_fbt@psr-suspend.html
   [153]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-rkl-1/igt@kms_fbcon_fbt@psr-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes:
    - shard-snb:          [DMESG-FAIL][154] ([fdo#103375]) -> [DMESG-WARN][155] ([i915#8841])
   [154]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-snb7/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes.html
   [155]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-snb2/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-a-planes.html

  * igt@kms_psr@sprite_plane_onoff:
    - shard-dg1:          [SKIP][156] ([i915#1072] / [i915#4078]) -> [SKIP][157] ([i915#1072])
   [156]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13571/shard-dg1-18/igt@kms_psr@sprite_plane_onoff.html
   [157]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_122982v1/shard-dg1-17/igt@kms_psr@sprite_plane_onoff.html

  
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#111767]: https://bugs.freedesktop.org/show_bug.cgi?id=111767
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1099]: https://gitlab.freedesktop.org/drm/intel/issues/1099
  [i915#1339]: https://gitlab.freedesktop.org/drm/intel/issues/1339
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#1623]: https://gitlab.freedesktop.org/drm/intel/issues/1623
  [i915#1769]: https://gitlab.freedesktop.org/drm/intel/issues/1769
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1937]: https://gitlab.freedesktop.org/drm/intel/issues/1937
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#2705]: https://gitlab.freedesktop.org/drm/intel/issues/2705
  [i915#280]: https://gitlab.freedesktop.org/drm/intel/issues/280
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2846]: https://gitlab.freedesktop.org/drm/intel/issues/2846
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3299]: https://gitlab.freedesktop.org/drm/intel/issues/3299
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3539]: https://gitlab.freedesktop.org/drm/intel/issues/3539
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3743]: https://gitlab.freedesktop.org/drm/intel/issues/3743
  [i915#3840]: https://gitlab.freedesktop.org/drm/intel/issues/3840
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4087]: https://gitlab.freedesktop.org/drm/intel/issues/4087
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4235]: https://gitlab.freedesktop.org/drm/intel/issues/4235
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#433]: https://gitlab.freedesktop.org/drm/intel/issues/433
  [i915#4349]: https://gitlab.freedesktop.org/drm/intel/issues/4349
  [i915#4387]: https://gitlab.freedesktop.org/drm/intel/issues/4387
  [i915#4475]: https://gitlab.freedesktop.org/drm/intel/issues/4475
  [i915#4537]: https://gitlab.freedesktop.org/drm/intel/issues/4537
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4812]: https://gitlab.freedesktop.org/drm/intel/issues/4812
  [i915#4852]: https://gitlab.freedesktop.org/drm/intel/issues/4852
  [i915#4860]: https://gitlab.freedesktop.org/drm/intel/issues/4860
  [i915#4936]: https://gitlab.freedesktop.org/drm/intel/issues/4936
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5465]: https://gitlab.freedesktop.org/drm/intel/issues/5465
  [i915#5493]: https://gitlab.freedesktop.org/drm/intel/issues/5493
  [i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
  [i915#5889]: https://gitlab.freedesktop.org/drm/intel/issues/5889
  [i915#6122]: https://gitlab.freedesktop.org/drm/intel/issues/6122
  [i915#6188]: https://gitlab.freedesktop.org/drm/intel/issues/6188
  [i915#6311]: https://gitlab.freedesktop.org/drm/intel/issues/6311
  [i915#6524]: https://gitlab.freedesktop.org/drm/intel/issues/6524
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6805]: https://gitlab.freedesktop.org/drm/intel/issues/6805
  [i915#6880]: https://gitlab.freedesktop.org/drm/intel/issues/6880
  [i915#7118]: https://gitlab.freedesktop.org/drm/intel/issues/7118
  [i915#7297]: https://gitlab.freedesktop.org/drm/intel/issues/7297
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7765]: https://gitlab.freedesktop.org/drm/intel/issues/7765
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7941]: https://gitlab.freedesktop.org/drm/intel/issues/7941
  [i915#7962]: https://gitlab.freedesktop.org/drm/intel/issues/7962
  [i915#8228]: https://gitlab.freedesktop.org/drm/intel/issues/8228
  [i915#8289]: https://gitlab.freedesktop.org/drm/intel/issues/8289
  [i915#8414]: https://gitlab.freedesktop.org/drm/intel/issues/8414
  [i915#8502]: https://gitlab.freedesktop.org/drm/intel/issues/8502
  [i915#8503]: https://gitlab.freedesktop.org/drm/intel/issues/8503
  [i915#8516]: https://gitlab.freedesktop.org/drm/intel/issues/8516
  [i915#8555]: https://gitlab.freedesktop.org/drm/intel/issues/8555
  [i915#8623]: https://gitlab.freedesktop.org/drm/intel/issues/8623
  [i915#8661]: https://gitlab.freedesktop.org/drm/intel/issues/8661
  [i915#8708]: https://gitlab.freedesktop.org/drm/intel/issues/8708
  [i915#8709]: https://gitlab.freedesktop.org/drm/intel/issues/8709
  [i915#8812]: https://gitlab.freedesktop.org/drm/intel/issues/8812
  [i915#8841]: https://gitlab.freedesktop.org/drm/intel/issues/8841
  [i915#8865]: https://gitlab.freedesktop.org/drm/intel/issues/8865
  [i915#8898]: https://gitlab.freedesktop.org/drm/intel/issues/8898
  [i915#8962]: https://gitlab.freedesktop.org/drm/intel/issues/8962
  [i915#9121]: https://gitlab.freedesktop.org/drm/intel/issues/9121
  [i915#9142]: https://gitlab.freedesktop.org/drm/intel/issues/9142


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

  * Linux: CI_DRM_13571 -> Patchwork_122982v1
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_13571: d3f0e020609215d9097f1b1fad5a13ea0b37548e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7454: 7454
  Patchwork_122982v1: d3f0e020609215d9097f1b1fad5a13ea0b37548e @ 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_122982v1/index.html

[-- Attachment #2: Type: text/html, Size: 49841 bytes --]

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

* Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
  2023-08-28 21:45   ` Gustavo Sousa
@ 2023-08-28 22:53     ` Sripada, Radhakrishna
  2023-08-29  9:35       ` Kahola, Mika
  2023-08-29 15:12     ` Jani Nikula
  1 sibling, 1 reply; 13+ messages in thread
From: Sripada, Radhakrishna @ 2023-08-28 22:53 UTC (permalink / raw)
  To: Sousa, Gustavo, intel-gfx

Hi Gustavo,

> -----Original Message-----
> From: Sousa, Gustavo <gustavo.sousa@intel.com>
> Sent: Monday, August 28, 2023 2:46 PM
> To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus
> timeout threshold
> 
> Quoting Sripada, Radhakrishna (2023-08-28 17:30:19-03:00)
> >Hi Gustavo,
> 
> Hi, RK.
> 
> Thanks for the feedback! Please, see my replies below.
> 
> >
> >> -----Original Message-----
> >> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Gustavo
> >> Sousa
> >> Sent: Monday, August 28, 2023 10:34 AM
> >> To: intel-gfx@lists.freedesktop.org
> >> Subject: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus
> timeout
> >> threshold
> >>
> >> We have experienced timeout issues when accessing C20 SRAM registers.
> >This wording is misleading. It seems to imply that we directly access SRAM
> >registers via msg bus but the reads/writes go through intermediate registers
> >and it is this read/write that is failing. Adding extra clarity here would be helpful.
> 
> Hm... Okay. I thought that would already be implied to someone familiar with the
> code. What about:
> 
>     s/when accessing/when going through the sequence to access/
This is better to indicate that it is not direct access.

> 
> ?
> 
> >
> >> Experimentation showed that bumping the message bus timer threshold
> >> helped on getting display Type-C connection on the C20 PHY to work.
> >>
> >> While the timeout is still under investigation with the HW team, having
> >> logic to allow forward progress (with the proper warnings) seems useful.
> >> Thus, let's bump the threshold when a timeout is detected.
> >>
> >> The maximum value of 0x200 pclk cycles was somewhat arbitrary - 2x the
> >> default value. That value was successfully tested on real hardware that
> >> was displaying timeouts otherwise.
> >>
> >> BSpec: 65156
> >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 41 +++++++++++++++++++
> >>  .../gpu/drm/i915/display/intel_cx0_phy_regs.h | 12 ++++++
> >>  2 files changed, 53 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >> index dd489b50ad60..55d2333032e3 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >> @@ -5,6 +5,7 @@
> >>
> >>  #include <linux/log2.h>
> >>  #include <linux/math64.h>
> >> +#include <linux/minmax.h>
> >>  #include "i915_reg.h"
> >>  #include "intel_cx0_phy.h"
> >>  #include "intel_cx0_phy_regs.h"
> >> @@ -29,6 +30,8 @@
> >>  #define INTEL_CX0_LANE1                BIT(1)
> >>  #define INTEL_CX0_BOTH_LANES        (INTEL_CX0_LANE1 |
> >> INTEL_CX0_LANE0)
> >>
> >> +#define INTEL_CX0_MSGBUS_TIMER_VAL_MAX        0x200
> >> +
> >>  bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy)
> >>  {
> >>          if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && phy < PHY_C)
> >> @@ -119,6 +122,43 @@ static void intel_cx0_bus_reset(struct
> drm_i915_private
> >> *i915, enum port port, i
> >>          intel_clear_response_ready_flag(i915, port, lane);
> >>  }
> >>
> >> +/*
> >> + * Check if there was a timeout detected by the hardware in the message bus
> >> + * and bump the threshold if so.
> >> + */
> >> +static void intel_cx0_bus_check_and_bump_timer(struct drm_i915_private
> >> *i915,
> >> +                                               enum port port, int lane)
> >> +{
> >> +        enum phy phy = intel_port_to_phy(i915, port);
> >> +        i915_reg_t reg;
> >> +        u32 val;
> >> +        u32 timer_val;
> >> +
> >> +        reg = XELPDP_PORT_MSGBUS_TIMER(port, lane);
> >> +        val = intel_de_read(i915, reg);
> >> +
> >> +        if (!(val & XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT)) {
> >> +                drm_warn(&i915->drm,
> >> +                         "PHY %c lane %d: hardware did not detect a
> >> timeout\n",
> >> +                         phy_name(phy), lane);
> >> +                return;
> >> +        }
> >> +
> >> +        timer_val =
> >> REG_FIELD_GET(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK, val);
> >> +
> >> +        if (timer_val == INTEL_CX0_MSGBUS_TIMER_VAL_MAX)
> >> +                return;
> >> +
> >> +        timer_val = min(2 * timer_val,
> >> (u32)INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
> >     ^ is this cast necessary?
> 
> Yes. I remember getting a warning without it. Let me remove it to remember...
Got it thanks for the quick check.

> 
> ...done! I think it complains because the arguments are of different types:
> 
>     In file included from drivers/gpu/drm/i915/display/intel_cx0_phy.c:8:
>     drivers/gpu/drm/i915/display/intel_cx0_phy.c: In function
> ‘intel_cx0_bus_check_and_bump_timer’:
>     ./include/linux/minmax.h:20:35: error: comparison of distinct pointer types
> lacks a cast [-Werror]
>        20 |         (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>           |                                   ^~
>     ./include/linux/minmax.h:26:18: note: in expansion of macro ‘__typecheck’
>        26 |                 (__typecheck(x, y) && __no_side_effects(x, y))
>           |                  ^~~~~~~~~~~
>     ./include/linux/minmax.h:36:31: note: in expansion of macro ‘__safe_cmp’
>        36 |         __builtin_choose_expr(__safe_cmp(x, y), \
>           |                               ^~~~~~~~~~
>     ./include/linux/minmax.h:67:25: note: in expansion of macro ‘__careful_cmp’
>        67 | #define min(x, y)       __careful_cmp(x, y, <)
>           |                         ^~~~~~~~~~~~~
>     drivers/gpu/drm/i915/display/intel_cx0_phy.c:152:21: note: in expansion of
> macro ‘min’
>       152 |         timer_val = min(2 * timer_val,
> INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
>           |
> 
> >
> >> +        val &= ~XELPDP_PORT_MSGBUS_TIMER_VAL_MASK;
> >> +        val |= REG_FIELD_PREP(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK,
> >> timer_val);
> >We do not use REG_FIELD_PREP in the function. Instead we use it in
> >register definition in intel_cx0_phy_regs.h.
> 
> I think it makes sense have definitions using REG_FIELD_PREP() in header files
> and use those definitions in .c files for fields whose values are are
> enumerated.
> 
> Now, for fields without enumeration, like this one in discussion, I think it is
> fine to use REG_FIELD_PREP() directly. The MASK is already exposed anyway...
> 
> Besides, it seems we already have lines of code in *.c files using the pattern
> above:
> 
>     git grep REG_FIELD_PREP drm-tip/drm-tip -- ':/drivers/gpu/drm/i915/**/*.c'
> 
> Let me know what you think. I could add a XELPDP_PORT_MSGBUS_TIMER_VAL()
> macro
> if you think it is necessary. My personal take is that using REG_FIELD_PREP()
> for this case is fine.
Let us conform with the norms for the macro-fields used in this file instead of starting
a new pattern.  

--Radhakrishna(RK) Sripada
> 
> --
> Gustavo Sousa
> 
> >
> >Thanks,
> >Radhakrishna Sripada
> >
> >> +
> >> +        drm_dbg_kms(&i915->drm,
> >> +                    "PHY %c lane %d: increasing msgbus timer threshold to
> >> %#x\n",
> >> +                    phy_name(phy), lane, timer_val);
> >> +        intel_de_write(i915, reg, val);
> >> +}
> >> +
> >>  static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port
> port,
> >>                                    int command, int lane, u32 *val)
> >>  {
> >> @@ -132,6 +172,7 @@ static int intel_cx0_wait_for_ack(struct
> >> drm_i915_private *i915, enum port port,
> >>                                           XELPDP_MSGBUS_TIMEOUT_SLOW,
> >> val)) {
> >>                  drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting for
> >> message ACK. Status: 0x%x\n",
> >>                              phy_name(phy), *val);
> >> +                intel_cx0_bus_check_and_bump_timer(i915, port, lane);
> >>                  intel_cx0_bus_reset(i915, port, lane);
> >>                  return -ETIMEDOUT;
> >>          }
> >> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> >> b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> >> index cb5d1be2ba19..17cc3385aabb 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> >> @@ -110,6 +110,18 @@
> >>  #define   CX0_P4PG_STATE_DISABLE                        0xC
> >>  #define   CX0_P2_STATE_RESET                                0x2
> >>
> >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_A
> >>         0x640d8
> >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_B
> >>         0x641d8
> >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1                0x16f258
> >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2                0x16f458
> >> +#define XELPDP_PORT_MSGBUS_TIMER(port, lane)
> >>         _MMIO(_PICK_EVEN_2RANGES(port, PORT_TC1, \
> >> +
> >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_A, \
> >> +
> >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_B, \
> >> +
> >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1, \
> >> +
> >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2) + (lane) * 4)
> >> +#define   XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT                REG_BIT(31)
> >> +#define   XELPDP_PORT_MSGBUS_TIMER_VAL_MASK
> >>         REG_GENMASK(23, 0)
> >> +
> >>  #define _XELPDP_PORT_CLOCK_CTL_A                        0x640E0
> >>  #define _XELPDP_PORT_CLOCK_CTL_B                        0x641E0
> >>  #define _XELPDP_PORT_CLOCK_CTL_USBC1                        0x16F260
> >> --
> >> 2.41.0
> >

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

* Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
  2023-08-28 22:53     ` Sripada, Radhakrishna
@ 2023-08-29  9:35       ` Kahola, Mika
  2023-08-29 11:45         ` Gustavo Sousa
  0 siblings, 1 reply; 13+ messages in thread
From: Kahola, Mika @ 2023-08-29  9:35 UTC (permalink / raw)
  To: Sripada, Radhakrishna, Sousa, Gustavo, intel-gfx

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Sripada, Radhakrishna
> Sent: Tuesday, August 29, 2023 1:54 AM
> To: Sousa, Gustavo <gustavo.sousa@intel.com>; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
> 
> Hi Gustavo,
> 
> > -----Original Message-----
> > From: Sousa, Gustavo <gustavo.sousa@intel.com>
> > Sent: Monday, August 28, 2023 2:46 PM
> > To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase
> > msgbus timeout threshold
> >
> > Quoting Sripada, Radhakrishna (2023-08-28 17:30:19-03:00)
> > >Hi Gustavo,
> >
> > Hi, RK.
> >
> > Thanks for the feedback! Please, see my replies below.
> >
> > >
> > >> -----Original Message-----
> > >> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
> > >> Of
> > Gustavo
> > >> Sousa
> > >> Sent: Monday, August 28, 2023 10:34 AM
> > >> To: intel-gfx@lists.freedesktop.org
> > >> Subject: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase
> > >> msgbus
> > timeout
> > >> threshold
> > >>
> > >> We have experienced timeout issues when accessing C20 SRAM registers.
> > >This wording is misleading. It seems to imply that we directly access
> > >SRAM registers via msg bus but the reads/writes go through
> > >intermediate registers and it is this read/write that is failing. Adding extra clarity here would be helpful.
> >
> > Hm... Okay. I thought that would already be implied to someone
> > familiar with the code. What about:
> >
> >     s/when accessing/when going through the sequence to access/
> This is better to indicate that it is not direct access.
> 
> >
> > ?
> >
> > >
> > >> Experimentation showed that bumping the message bus timer threshold
> > >> helped on getting display Type-C connection on the C20 PHY to work.
> > >>
> > >> While the timeout is still under investigation with the HW team,
> > >> having logic to allow forward progress (with the proper warnings) seems useful.
> > >> Thus, let's bump the threshold when a timeout is detected.
> > >>
> > >> The maximum value of 0x200 pclk cycles was somewhat arbitrary - 2x
> > >> the default value. That value was successfully tested on real
> > >> hardware that was displaying timeouts otherwise.
> > >>
> > >> BSpec: 65156
> > >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> > >> ---
> > >>  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 41
> > >> +++++++++++++++++++  .../gpu/drm/i915/display/intel_cx0_phy_regs.h
> > >> | 12 ++++++
> > >>  2 files changed, 53 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > >> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > >> index dd489b50ad60..55d2333032e3 100644
> > >> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > >> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > >> @@ -5,6 +5,7 @@
> > >>
> > >>  #include <linux/log2.h>
> > >>  #include <linux/math64.h>
> > >> +#include <linux/minmax.h>
> > >>  #include "i915_reg.h"
> > >>  #include "intel_cx0_phy.h"
> > >>  #include "intel_cx0_phy_regs.h"
> > >> @@ -29,6 +30,8 @@
> > >>  #define INTEL_CX0_LANE1                BIT(1)
> > >>  #define INTEL_CX0_BOTH_LANES        (INTEL_CX0_LANE1 |
> > >> INTEL_CX0_LANE0)
> > >>
> > >> +#define INTEL_CX0_MSGBUS_TIMER_VAL_MAX        0x200
> > >> +
> > >>  bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy)
> > >> {
> > >>          if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && phy <
> > >> PHY_C) @@ -119,6 +122,43 @@ static void intel_cx0_bus_reset(struct
> > drm_i915_private
> > >> *i915, enum port port, i
> > >>          intel_clear_response_ready_flag(i915, port, lane);  }
> > >>
> > >> +/*
> > >> + * Check if there was a timeout detected by the hardware in the
> > >> +message bus
> > >> + * and bump the threshold if so.

Just a thought but wouldn't it be simpler if we just set the timeout to it's maximum and discard the check here?

-Mika- 

> > >> + */
> > >> +static void intel_cx0_bus_check_and_bump_timer(struct
> > >> +drm_i915_private
> > >> *i915,
> > >> +                                               enum port port, int
> > >> +lane) {
> > >> +        enum phy phy = intel_port_to_phy(i915, port);
> > >> +        i915_reg_t reg;
> > >> +        u32 val;
> > >> +        u32 timer_val;
> > >> +
> > >> +        reg = XELPDP_PORT_MSGBUS_TIMER(port, lane);
> > >> +        val = intel_de_read(i915, reg);
> > >> +
> > >> +        if (!(val & XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT)) {
> > >> +                drm_warn(&i915->drm,
> > >> +                         "PHY %c lane %d: hardware did not detect
> > >> + a
> > >> timeout\n",
> > >> +                         phy_name(phy), lane);
> > >> +                return;
> > >> +        }
> > >> +
> > >> +        timer_val =
> > >> REG_FIELD_GET(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK, val);
> > >> +
> > >> +        if (timer_val == INTEL_CX0_MSGBUS_TIMER_VAL_MAX)
> > >> +                return;
> > >> +
> > >> +        timer_val = min(2 * timer_val,
> > >> (u32)INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
> > >     ^ is this cast necessary?
> >
> > Yes. I remember getting a warning without it. Let me remove it to remember...
> Got it thanks for the quick check.
> 
> >
> > ...done! I think it complains because the arguments are of different types:
> >
> >     In file included from drivers/gpu/drm/i915/display/intel_cx0_phy.c:8:
> >     drivers/gpu/drm/i915/display/intel_cx0_phy.c: In function
> > ‘intel_cx0_bus_check_and_bump_timer’:
> >     ./include/linux/minmax.h:20:35: error: comparison of distinct
> > pointer types lacks a cast [-Werror]
> >        20 |         (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> >           |                                   ^~
> >     ./include/linux/minmax.h:26:18: note: in expansion of macro ‘__typecheck’
> >        26 |                 (__typecheck(x, y) && __no_side_effects(x, y))
> >           |                  ^~~~~~~~~~~
> >     ./include/linux/minmax.h:36:31: note: in expansion of macro ‘__safe_cmp’
> >        36 |         __builtin_choose_expr(__safe_cmp(x, y), \
> >           |                               ^~~~~~~~~~
> >     ./include/linux/minmax.h:67:25: note: in expansion of macro ‘__careful_cmp’
> >        67 | #define min(x, y)       __careful_cmp(x, y, <)
> >           |                         ^~~~~~~~~~~~~
> >     drivers/gpu/drm/i915/display/intel_cx0_phy.c:152:21: note: in
> > expansion of macro ‘min’
> >       152 |         timer_val = min(2 * timer_val,
> > INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
> >           |
> >
> > >
> > >> +        val &= ~XELPDP_PORT_MSGBUS_TIMER_VAL_MASK;
> > >> +        val |= REG_FIELD_PREP(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK,
> > >> timer_val);
> > >We do not use REG_FIELD_PREP in the function. Instead we use it in
> > >register definition in intel_cx0_phy_regs.h.
> >
> > I think it makes sense have definitions using REG_FIELD_PREP() in
> > header files and use those definitions in .c files for fields whose
> > values are are enumerated.
> >
> > Now, for fields without enumeration, like this one in discussion, I
> > think it is fine to use REG_FIELD_PREP() directly. The MASK is already exposed anyway...
> >
> > Besides, it seems we already have lines of code in *.c files using the
> > pattern
> > above:
> >
> >     git grep REG_FIELD_PREP drm-tip/drm-tip -- ':/drivers/gpu/drm/i915/**/*.c'
> >
> > Let me know what you think. I could add a
> > XELPDP_PORT_MSGBUS_TIMER_VAL() macro if you think it is necessary. My
> > personal take is that using REG_FIELD_PREP() for this case is fine.
> Let us conform with the norms for the macro-fields used in this file instead of starting a new pattern.
> 
> --Radhakrishna(RK) Sripada
> >
> > --
> > Gustavo Sousa
> >
> > >
> > >Thanks,
> > >Radhakrishna Sripada
> > >
> > >> +
> > >> +        drm_dbg_kms(&i915->drm,
> > >> +                    "PHY %c lane %d: increasing msgbus timer
> > >> + threshold to
> > >> %#x\n",
> > >> +                    phy_name(phy), lane, timer_val);
> > >> +        intel_de_write(i915, reg, val); }
> > >> +
> > >>  static int intel_cx0_wait_for_ack(struct drm_i915_private *i915,
> > >> enum port
> > port,
> > >>                                    int command, int lane, u32 *val)
> > >> { @@ -132,6 +172,7 @@ static int intel_cx0_wait_for_ack(struct
> > >> drm_i915_private *i915, enum port port,
> > >>
> > >> XELPDP_MSGBUS_TIMEOUT_SLOW,
> > >> val)) {
> > >>                  drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting
> > >> for message ACK. Status: 0x%x\n",
> > >>                              phy_name(phy), *val);
> > >> +                intel_cx0_bus_check_and_bump_timer(i915, port,
> > >> + lane);
> > >>                  intel_cx0_bus_reset(i915, port, lane);
> > >>                  return -ETIMEDOUT;
> > >>          }
> > >> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> > >> b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> > >> index cb5d1be2ba19..17cc3385aabb 100644
> > >> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> > >> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> > >> @@ -110,6 +110,18 @@
> > >>  #define   CX0_P4PG_STATE_DISABLE                        0xC
> > >>  #define   CX0_P2_STATE_RESET                                0x2
> > >>
> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_A
> > >>         0x640d8
> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_B
> > >>         0x641d8
> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1                0x16f258
> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2                0x16f458
> > >> +#define XELPDP_PORT_MSGBUS_TIMER(port, lane)
> > >>         _MMIO(_PICK_EVEN_2RANGES(port, PORT_TC1, \
> > >> +
> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_A, \
> > >> +
> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_B, \
> > >> +
> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1, \
> > >> +
> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2) + (lane) * 4)
> > >> +#define   XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT                REG_BIT(31)
> > >> +#define   XELPDP_PORT_MSGBUS_TIMER_VAL_MASK
> > >>         REG_GENMASK(23, 0)
> > >> +
> > >>  #define _XELPDP_PORT_CLOCK_CTL_A                        0x640E0
> > >>  #define _XELPDP_PORT_CLOCK_CTL_B                        0x641E0
> > >>  #define _XELPDP_PORT_CLOCK_CTL_USBC1                        0x16F260
> > >> --
> > >> 2.41.0
> > >

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

* Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
  2023-08-29  9:35       ` Kahola, Mika
@ 2023-08-29 11:45         ` Gustavo Sousa
  2023-08-30 12:18           ` Gustavo Sousa
  0 siblings, 1 reply; 13+ messages in thread
From: Gustavo Sousa @ 2023-08-29 11:45 UTC (permalink / raw)
  To: Kahola, Mika, Sripada, Radhakrishna, intel-gfx

Quoting Kahola, Mika (2023-08-29 06:35:17-03:00)
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Sripada, Radhakrishna
>> Sent: Tuesday, August 29, 2023 1:54 AM
>> To: Sousa, Gustavo <gustavo.sousa@intel.com>; intel-gfx@lists.freedesktop.org
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
>> 
>> Hi Gustavo,
>> 
>> > -----Original Message-----
>> > From: Sousa, Gustavo <gustavo.sousa@intel.com>
>> > Sent: Monday, August 28, 2023 2:46 PM
>> > To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
>> > gfx@lists.freedesktop.org
>> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase
>> > msgbus timeout threshold
>> >
>> > Quoting Sripada, Radhakrishna (2023-08-28 17:30:19-03:00)
>> > >Hi Gustavo,
>> >
>> > Hi, RK.
>> >
>> > Thanks for the feedback! Please, see my replies below.
>> >
>> > >
>> > >> -----Original Message-----
>> > >> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
>> > >> Of
>> > Gustavo
>> > >> Sousa
>> > >> Sent: Monday, August 28, 2023 10:34 AM
>> > >> To: intel-gfx@lists.freedesktop.org
>> > >> Subject: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase
>> > >> msgbus
>> > timeout
>> > >> threshold
>> > >>
>> > >> We have experienced timeout issues when accessing C20 SRAM registers.
>> > >This wording is misleading. It seems to imply that we directly access
>> > >SRAM registers via msg bus but the reads/writes go through
>> > >intermediate registers and it is this read/write that is failing. Adding extra clarity here would be helpful.
>> >
>> > Hm... Okay. I thought that would already be implied to someone
>> > familiar with the code. What about:
>> >
>> >     s/when accessing/when going through the sequence to access/
>> This is better to indicate that it is not direct access.
>> 
>> >
>> > ?
>> >
>> > >
>> > >> Experimentation showed that bumping the message bus timer threshold
>> > >> helped on getting display Type-C connection on the C20 PHY to work.
>> > >>
>> > >> While the timeout is still under investigation with the HW team,
>> > >> having logic to allow forward progress (with the proper warnings) seems useful.
>> > >> Thus, let's bump the threshold when a timeout is detected.
>> > >>
>> > >> The maximum value of 0x200 pclk cycles was somewhat arbitrary - 2x
>> > >> the default value. That value was successfully tested on real
>> > >> hardware that was displaying timeouts otherwise.
>> > >>
>> > >> BSpec: 65156
>> > >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> > >> ---
>> > >>  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 41
>> > >> +++++++++++++++++++  .../gpu/drm/i915/display/intel_cx0_phy_regs.h
>> > >> | 12 ++++++
>> > >>  2 files changed, 53 insertions(+)
>> > >>
>> > >> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> > >> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> > >> index dd489b50ad60..55d2333032e3 100644
>> > >> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> > >> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>> > >> @@ -5,6 +5,7 @@
>> > >>
>> > >>  #include <linux/log2.h>
>> > >>  #include <linux/math64.h>
>> > >> +#include <linux/minmax.h>
>> > >>  #include "i915_reg.h"
>> > >>  #include "intel_cx0_phy.h"
>> > >>  #include "intel_cx0_phy_regs.h"
>> > >> @@ -29,6 +30,8 @@
>> > >>  #define INTEL_CX0_LANE1                BIT(1)
>> > >>  #define INTEL_CX0_BOTH_LANES        (INTEL_CX0_LANE1 |
>> > >> INTEL_CX0_LANE0)
>> > >>
>> > >> +#define INTEL_CX0_MSGBUS_TIMER_VAL_MAX        0x200
>> > >> +
>> > >>  bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy)
>> > >> {
>> > >>          if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && phy <
>> > >> PHY_C) @@ -119,6 +122,43 @@ static void intel_cx0_bus_reset(struct
>> > drm_i915_private
>> > >> *i915, enum port port, i
>> > >>          intel_clear_response_ready_flag(i915, port, lane);  }
>> > >>
>> > >> +/*
>> > >> + * Check if there was a timeout detected by the hardware in the
>> > >> +message bus
>> > >> + * and bump the threshold if so.
>
>Just a thought but wouldn't it be simpler if we just set the timeout to it's maximum and discard the check here?

I'm not sure which exactly check you are talking about here:

  1) The check on the XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT.

     I think this could give us useful debugging info, for example, if our code
     thinks the message bus timed out while the bus hardware did not say so. I
     would prefer to keep this one, if you are okay with it.

  2) The check on the current value of the threshold and the exponential
     increase up to the maximum.

     I would agree that I did a bit of over-engineering here. I guess I could
     simply to a rmw using INTEL_CX0_MSGBUS_TIMER_VAL_MAX directly as you
     suggested; and maybe rename INTEL_CX0_MSGBUS_TIMER_VAL_MAX to
     INTEL_CX0_MSGBUS_TIMER_BUMPED_VAL, just not to give the impression that
     0x200 is the maximum accepted by the hardware.

What do you think?

--
Gustavo Sousa

>
>-Mika- 
>
>> > >> + */
>> > >> +static void intel_cx0_bus_check_and_bump_timer(struct
>> > >> +drm_i915_private
>> > >> *i915,
>> > >> +                                               enum port port, int
>> > >> +lane) {
>> > >> +        enum phy phy = intel_port_to_phy(i915, port);
>> > >> +        i915_reg_t reg;
>> > >> +        u32 val;
>> > >> +        u32 timer_val;
>> > >> +
>> > >> +        reg = XELPDP_PORT_MSGBUS_TIMER(port, lane);
>> > >> +        val = intel_de_read(i915, reg);
>> > >> +
>> > >> +        if (!(val & XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT)) {
>> > >> +                drm_warn(&i915->drm,
>> > >> +                         "PHY %c lane %d: hardware did not detect
>> > >> + a
>> > >> timeout\n",
>> > >> +                         phy_name(phy), lane);
>> > >> +                return;
>> > >> +        }
>> > >> +
>> > >> +        timer_val =
>> > >> REG_FIELD_GET(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK, val);
>> > >> +
>> > >> +        if (timer_val == INTEL_CX0_MSGBUS_TIMER_VAL_MAX)
>> > >> +                return;
>> > >> +
>> > >> +        timer_val = min(2 * timer_val,
>> > >> (u32)INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
>> > >     ^ is this cast necessary?
>> >
>> > Yes. I remember getting a warning without it. Let me remove it to remember...
>> Got it thanks for the quick check.
>> 
>> >
>> > ...done! I think it complains because the arguments are of different types:
>> >
>> >     In file included from drivers/gpu/drm/i915/display/intel_cx0_phy.c:8:
>> >     drivers/gpu/drm/i915/display/intel_cx0_phy.c: In function
>> > ‘intel_cx0_bus_check_and_bump_timer’:
>> >     ./include/linux/minmax.h:20:35: error: comparison of distinct
>> > pointer types lacks a cast [-Werror]
>> >        20 |         (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>> >           |                                   ^~
>> >     ./include/linux/minmax.h:26:18: note: in expansion of macro ‘__typecheck’
>> >        26 |                 (__typecheck(x, y) && __no_side_effects(x, y))
>> >           |                  ^~~~~~~~~~~
>> >     ./include/linux/minmax.h:36:31: note: in expansion of macro ‘__safe_cmp’
>> >        36 |         __builtin_choose_expr(__safe_cmp(x, y), \
>> >           |                               ^~~~~~~~~~
>> >     ./include/linux/minmax.h:67:25: note: in expansion of macro ‘__careful_cmp’
>> >        67 | #define min(x, y)       __careful_cmp(x, y, <)
>> >           |                         ^~~~~~~~~~~~~
>> >     drivers/gpu/drm/i915/display/intel_cx0_phy.c:152:21: note: in
>> > expansion of macro ‘min’
>> >       152 |         timer_val = min(2 * timer_val,
>> > INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
>> >           |
>> >
>> > >
>> > >> +        val &= ~XELPDP_PORT_MSGBUS_TIMER_VAL_MASK;
>> > >> +        val |= REG_FIELD_PREP(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK,
>> > >> timer_val);
>> > >We do not use REG_FIELD_PREP in the function. Instead we use it in
>> > >register definition in intel_cx0_phy_regs.h.
>> >
>> > I think it makes sense have definitions using REG_FIELD_PREP() in
>> > header files and use those definitions in .c files for fields whose
>> > values are are enumerated.
>> >
>> > Now, for fields without enumeration, like this one in discussion, I
>> > think it is fine to use REG_FIELD_PREP() directly. The MASK is already exposed anyway...
>> >
>> > Besides, it seems we already have lines of code in *.c files using the
>> > pattern
>> > above:
>> >
>> >     git grep REG_FIELD_PREP drm-tip/drm-tip -- ':/drivers/gpu/drm/i915/**/*.c'
>> >
>> > Let me know what you think. I could add a
>> > XELPDP_PORT_MSGBUS_TIMER_VAL() macro if you think it is necessary. My
>> > personal take is that using REG_FIELD_PREP() for this case is fine.
>> Let us conform with the norms for the macro-fields used in this file instead of starting a new pattern.
>> 
>> --Radhakrishna(RK) Sripada
>> >
>> > --
>> > Gustavo Sousa
>> >
>> > >
>> > >Thanks,
>> > >Radhakrishna Sripada
>> > >
>> > >> +
>> > >> +        drm_dbg_kms(&i915->drm,
>> > >> +                    "PHY %c lane %d: increasing msgbus timer
>> > >> + threshold to
>> > >> %#x\n",
>> > >> +                    phy_name(phy), lane, timer_val);
>> > >> +        intel_de_write(i915, reg, val); }
>> > >> +
>> > >>  static int intel_cx0_wait_for_ack(struct drm_i915_private *i915,
>> > >> enum port
>> > port,
>> > >>                                    int command, int lane, u32 *val)
>> > >> { @@ -132,6 +172,7 @@ static int intel_cx0_wait_for_ack(struct
>> > >> drm_i915_private *i915, enum port port,
>> > >>
>> > >> XELPDP_MSGBUS_TIMEOUT_SLOW,
>> > >> val)) {
>> > >>                  drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting
>> > >> for message ACK. Status: 0x%x\n",
>> > >>                              phy_name(phy), *val);
>> > >> +                intel_cx0_bus_check_and_bump_timer(i915, port,
>> > >> + lane);
>> > >>                  intel_cx0_bus_reset(i915, port, lane);
>> > >>                  return -ETIMEDOUT;
>> > >>          }
>> > >> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>> > >> b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>> > >> index cb5d1be2ba19..17cc3385aabb 100644
>> > >> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>> > >> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>> > >> @@ -110,6 +110,18 @@
>> > >>  #define   CX0_P4PG_STATE_DISABLE                        0xC
>> > >>  #define   CX0_P2_STATE_RESET                                0x2
>> > >>
>> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_A
>> > >>         0x640d8
>> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_B
>> > >>         0x641d8
>> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1                0x16f258
>> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2                0x16f458
>> > >> +#define XELPDP_PORT_MSGBUS_TIMER(port, lane)
>> > >>         _MMIO(_PICK_EVEN_2RANGES(port, PORT_TC1, \
>> > >> +
>> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_A, \
>> > >> +
>> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_B, \
>> > >> +
>> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1, \
>> > >> +
>> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2) + (lane) * 4)
>> > >> +#define   XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT                REG_BIT(31)
>> > >> +#define   XELPDP_PORT_MSGBUS_TIMER_VAL_MASK
>> > >>         REG_GENMASK(23, 0)
>> > >> +
>> > >>  #define _XELPDP_PORT_CLOCK_CTL_A                        0x640E0
>> > >>  #define _XELPDP_PORT_CLOCK_CTL_B                        0x641E0
>> > >>  #define _XELPDP_PORT_CLOCK_CTL_USBC1                        0x16F260
>> > >> --
>> > >> 2.41.0
>> > >

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

* Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
  2023-08-28 21:45   ` Gustavo Sousa
  2023-08-28 22:53     ` Sripada, Radhakrishna
@ 2023-08-29 15:12     ` Jani Nikula
  2023-08-30 12:17       ` Gustavo Sousa
  1 sibling, 1 reply; 13+ messages in thread
From: Jani Nikula @ 2023-08-29 15:12 UTC (permalink / raw)
  To: Gustavo Sousa, Sripada, Radhakrishna, intel-gfx

On Mon, 28 Aug 2023, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>>> +#define INTEL_CX0_MSGBUS_TIMER_VAL_MAX        0x200

Either make this 0x200U (for unsigned)...

>>> +
>>>  bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy)
>>>  {
>>>          if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && phy < PHY_C)
>>> @@ -119,6 +122,43 @@ static void intel_cx0_bus_reset(struct drm_i915_private
>>> *i915, enum port port, i
>>>          intel_clear_response_ready_flag(i915, port, lane);
>>>  }
>>> 
>>> +/*
>>> + * Check if there was a timeout detected by the hardware in the message bus
>>> + * and bump the threshold if so.
>>> + */
>>> +static void intel_cx0_bus_check_and_bump_timer(struct drm_i915_private
>>> *i915,
>>> +                                               enum port port, int lane)
>>> +{
>>> +        enum phy phy = intel_port_to_phy(i915, port);
>>> +        i915_reg_t reg;
>>> +        u32 val;
>>> +        u32 timer_val;
>>> +
>>> +        reg = XELPDP_PORT_MSGBUS_TIMER(port, lane);
>>> +        val = intel_de_read(i915, reg);
>>> +
>>> +        if (!(val & XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT)) {
>>> +                drm_warn(&i915->drm,
>>> +                         "PHY %c lane %d: hardware did not detect a
>>> timeout\n",
>>> +                         phy_name(phy), lane);
>>> +                return;
>>> +        }
>>> +
>>> +        timer_val =
>>> REG_FIELD_GET(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK, val);
>>> +
>>> +        if (timer_val == INTEL_CX0_MSGBUS_TIMER_VAL_MAX)
>>> +                return;
>>> +
>>> +        timer_val = min(2 * timer_val,
>>> (u32)INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
>>     ^ is this cast necessary?
>
> Yes. I remember getting a warning without it. Let me remove it to remember...

...or use min_t() instead of adding the cast here.

BR,
Jani.


>
> ...done! I think it complains because the arguments are of different types:
>
>     In file included from drivers/gpu/drm/i915/display/intel_cx0_phy.c:8:
>     drivers/gpu/drm/i915/display/intel_cx0_phy.c: In function ‘intel_cx0_bus_check_and_bump_timer’:
>     ./include/linux/minmax.h:20:35: error: comparison of distinct pointer types lacks a cast [-Werror]
>        20 |         (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>           |                                   ^~
>     ./include/linux/minmax.h:26:18: note: in expansion of macro ‘__typecheck’
>        26 |                 (__typecheck(x, y) && __no_side_effects(x, y))
>           |                  ^~~~~~~~~~~
>     ./include/linux/minmax.h:36:31: note: in expansion of macro ‘__safe_cmp’
>        36 |         __builtin_choose_expr(__safe_cmp(x, y), \
>           |                               ^~~~~~~~~~
>     ./include/linux/minmax.h:67:25: note: in expansion of macro ‘__careful_cmp’
>        67 | #define min(x, y)       __careful_cmp(x, y, <)
>           |                         ^~~~~~~~~~~~~
>     drivers/gpu/drm/i915/display/intel_cx0_phy.c:152:21: note: in expansion of macro ‘min’
>       152 |         timer_val = min(2 * timer_val, INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
>           |
>
>>
>>> +        val &= ~XELPDP_PORT_MSGBUS_TIMER_VAL_MASK;
>>> +        val |= REG_FIELD_PREP(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK,
>>> timer_val);
>>We do not use REG_FIELD_PREP in the function. Instead we use it in
>>register definition in intel_cx0_phy_regs.h.
>
> I think it makes sense have definitions using REG_FIELD_PREP() in header files
> and use those definitions in .c files for fields whose values are are
> enumerated.
>
> Now, for fields without enumeration, like this one in discussion, I think it is
> fine to use REG_FIELD_PREP() directly. The MASK is already exposed anyway...
>
> Besides, it seems we already have lines of code in *.c files using the pattern
> above:
>
>     git grep REG_FIELD_PREP drm-tip/drm-tip -- ':/drivers/gpu/drm/i915/**/*.c'
>
> Let me know what you think. I could add a XELPDP_PORT_MSGBUS_TIMER_VAL() macro
> if you think it is necessary. My personal take is that using REG_FIELD_PREP()
> for this case is fine.
>
> --
> Gustavo Sousa
>
>>
>>Thanks,
>>Radhakrishna Sripada
>>
>>> +
>>> +        drm_dbg_kms(&i915->drm,
>>> +                    "PHY %c lane %d: increasing msgbus timer threshold to
>>> %#x\n",
>>> +                    phy_name(phy), lane, timer_val);
>>> +        intel_de_write(i915, reg, val);
>>> +}
>>> +
>>>  static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port port,
>>>                                    int command, int lane, u32 *val)
>>>  {
>>> @@ -132,6 +172,7 @@ static int intel_cx0_wait_for_ack(struct
>>> drm_i915_private *i915, enum port port,
>>>                                           XELPDP_MSGBUS_TIMEOUT_SLOW,
>>> val)) {
>>>                  drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting for
>>> message ACK. Status: 0x%x\n",
>>>                              phy_name(phy), *val);
>>> +                intel_cx0_bus_check_and_bump_timer(i915, port, lane);
>>>                  intel_cx0_bus_reset(i915, port, lane);
>>>                  return -ETIMEDOUT;
>>>          }
>>> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>>> b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>>> index cb5d1be2ba19..17cc3385aabb 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>>> @@ -110,6 +110,18 @@
>>>  #define   CX0_P4PG_STATE_DISABLE                        0xC
>>>  #define   CX0_P2_STATE_RESET                                0x2
>>> 
>>> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_A
>>>         0x640d8
>>> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_B
>>>         0x641d8
>>> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1                0x16f258
>>> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2                0x16f458
>>> +#define XELPDP_PORT_MSGBUS_TIMER(port, lane)
>>>         _MMIO(_PICK_EVEN_2RANGES(port, PORT_TC1, \
>>> +
>>>          _XELPDP_PORT_MSGBUS_TIMER_LN0_A, \
>>> +
>>>          _XELPDP_PORT_MSGBUS_TIMER_LN0_B, \
>>> +
>>>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1, \
>>> +
>>>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2) + (lane) * 4)
>>> +#define   XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT                REG_BIT(31)
>>> +#define   XELPDP_PORT_MSGBUS_TIMER_VAL_MASK
>>>         REG_GENMASK(23, 0)
>>> +
>>>  #define _XELPDP_PORT_CLOCK_CTL_A                        0x640E0
>>>  #define _XELPDP_PORT_CLOCK_CTL_B                        0x641E0
>>>  #define _XELPDP_PORT_CLOCK_CTL_USBC1                        0x16F260
>>> --
>>> 2.41.0
>>

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
  2023-08-29 15:12     ` Jani Nikula
@ 2023-08-30 12:17       ` Gustavo Sousa
  0 siblings, 0 replies; 13+ messages in thread
From: Gustavo Sousa @ 2023-08-30 12:17 UTC (permalink / raw)
  To: Sripada, Radhakrishna, Jani Nikula, intel-gfx

Quoting Jani Nikula (2023-08-29 12:12:19-03:00)
>On Mon, 28 Aug 2023, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>>>> +#define INTEL_CX0_MSGBUS_TIMER_VAL_MAX        0x200
>
>Either make this 0x200U (for unsigned)...
>
>>>> +
>>>>  bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy)
>>>>  {
>>>>          if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && phy < PHY_C)
>>>> @@ -119,6 +122,43 @@ static void intel_cx0_bus_reset(struct drm_i915_private
>>>> *i915, enum port port, i
>>>>          intel_clear_response_ready_flag(i915, port, lane);
>>>>  }
>>>> 
>>>> +/*
>>>> + * Check if there was a timeout detected by the hardware in the message bus
>>>> + * and bump the threshold if so.
>>>> + */
>>>> +static void intel_cx0_bus_check_and_bump_timer(struct drm_i915_private
>>>> *i915,
>>>> +                                               enum port port, int lane)
>>>> +{
>>>> +        enum phy phy = intel_port_to_phy(i915, port);
>>>> +        i915_reg_t reg;
>>>> +        u32 val;
>>>> +        u32 timer_val;
>>>> +
>>>> +        reg = XELPDP_PORT_MSGBUS_TIMER(port, lane);
>>>> +        val = intel_de_read(i915, reg);
>>>> +
>>>> +        if (!(val & XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT)) {
>>>> +                drm_warn(&i915->drm,
>>>> +                         "PHY %c lane %d: hardware did not detect a
>>>> timeout\n",
>>>> +                         phy_name(phy), lane);
>>>> +                return;
>>>> +        }
>>>> +
>>>> +        timer_val =
>>>> REG_FIELD_GET(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK, val);
>>>> +
>>>> +        if (timer_val == INTEL_CX0_MSGBUS_TIMER_VAL_MAX)
>>>> +                return;
>>>> +
>>>> +        timer_val = min(2 * timer_val,
>>>> (u32)INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
>>>     ^ is this cast necessary?
>>
>> Yes. I remember getting a warning without it. Let me remove it to remember...
>
>...or use min_t() instead of adding the cast here.

The v2 of this patch removed the usage of min(), but thanks for the tips!

--
Gustavo Sousa

>
>BR,
>Jani.
>
>
>>
>> ...done! I think it complains because the arguments are of different types:
>>
>>     In file included from drivers/gpu/drm/i915/display/intel_cx0_phy.c:8:
>>     drivers/gpu/drm/i915/display/intel_cx0_phy.c: In function ‘intel_cx0_bus_check_and_bump_timer’:
>>     ./include/linux/minmax.h:20:35: error: comparison of distinct pointer types lacks a cast [-Werror]
>>        20 |         (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>>           |                                   ^~
>>     ./include/linux/minmax.h:26:18: note: in expansion of macro ‘__typecheck’
>>        26 |                 (__typecheck(x, y) && __no_side_effects(x, y))
>>           |                  ^~~~~~~~~~~
>>     ./include/linux/minmax.h:36:31: note: in expansion of macro ‘__safe_cmp’
>>        36 |         __builtin_choose_expr(__safe_cmp(x, y), \
>>           |                               ^~~~~~~~~~
>>     ./include/linux/minmax.h:67:25: note: in expansion of macro ‘__careful_cmp’
>>        67 | #define min(x, y)       __careful_cmp(x, y, <)
>>           |                         ^~~~~~~~~~~~~
>>     drivers/gpu/drm/i915/display/intel_cx0_phy.c:152:21: note: in expansion of macro ‘min’
>>       152 |         timer_val = min(2 * timer_val, INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
>>           |
>>
>>>
>>>> +        val &= ~XELPDP_PORT_MSGBUS_TIMER_VAL_MASK;
>>>> +        val |= REG_FIELD_PREP(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK,
>>>> timer_val);
>>>We do not use REG_FIELD_PREP in the function. Instead we use it in
>>>register definition in intel_cx0_phy_regs.h.
>>
>> I think it makes sense have definitions using REG_FIELD_PREP() in header files
>> and use those definitions in .c files for fields whose values are are
>> enumerated.
>>
>> Now, for fields without enumeration, like this one in discussion, I think it is
>> fine to use REG_FIELD_PREP() directly. The MASK is already exposed anyway...
>>
>> Besides, it seems we already have lines of code in *.c files using the pattern
>> above:
>>
>>     git grep REG_FIELD_PREP drm-tip/drm-tip -- ':/drivers/gpu/drm/i915/**/*.c'
>>
>> Let me know what you think. I could add a XELPDP_PORT_MSGBUS_TIMER_VAL() macro
>> if you think it is necessary. My personal take is that using REG_FIELD_PREP()
>> for this case is fine.
>>
>> --
>> Gustavo Sousa
>>
>>>
>>>Thanks,
>>>Radhakrishna Sripada
>>>
>>>> +
>>>> +        drm_dbg_kms(&i915->drm,
>>>> +                    "PHY %c lane %d: increasing msgbus timer threshold to
>>>> %#x\n",
>>>> +                    phy_name(phy), lane, timer_val);
>>>> +        intel_de_write(i915, reg, val);
>>>> +}
>>>> +
>>>>  static int intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port port,
>>>>                                    int command, int lane, u32 *val)
>>>>  {
>>>> @@ -132,6 +172,7 @@ static int intel_cx0_wait_for_ack(struct
>>>> drm_i915_private *i915, enum port port,
>>>>                                           XELPDP_MSGBUS_TIMEOUT_SLOW,
>>>> val)) {
>>>>                  drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting for
>>>> message ACK. Status: 0x%x\n",
>>>>                              phy_name(phy), *val);
>>>> +                intel_cx0_bus_check_and_bump_timer(i915, port, lane);
>>>>                  intel_cx0_bus_reset(i915, port, lane);
>>>>                  return -ETIMEDOUT;
>>>>          }
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>>>> b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>>>> index cb5d1be2ba19..17cc3385aabb 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>>>> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>>>> @@ -110,6 +110,18 @@
>>>>  #define   CX0_P4PG_STATE_DISABLE                        0xC
>>>>  #define   CX0_P2_STATE_RESET                                0x2
>>>> 
>>>> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_A
>>>>         0x640d8
>>>> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_B
>>>>         0x641d8
>>>> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1                0x16f258
>>>> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2                0x16f458
>>>> +#define XELPDP_PORT_MSGBUS_TIMER(port, lane)
>>>>         _MMIO(_PICK_EVEN_2RANGES(port, PORT_TC1, \
>>>> +
>>>>          _XELPDP_PORT_MSGBUS_TIMER_LN0_A, \
>>>> +
>>>>          _XELPDP_PORT_MSGBUS_TIMER_LN0_B, \
>>>> +
>>>>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1, \
>>>> +
>>>>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2) + (lane) * 4)
>>>> +#define   XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT                REG_BIT(31)
>>>> +#define   XELPDP_PORT_MSGBUS_TIMER_VAL_MASK
>>>>         REG_GENMASK(23, 0)
>>>> +
>>>>  #define _XELPDP_PORT_CLOCK_CTL_A                        0x640E0
>>>>  #define _XELPDP_PORT_CLOCK_CTL_B                        0x641E0
>>>>  #define _XELPDP_PORT_CLOCK_CTL_USBC1                        0x16F260
>>>> --
>>>> 2.41.0
>>>
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
  2023-08-29 11:45         ` Gustavo Sousa
@ 2023-08-30 12:18           ` Gustavo Sousa
  2023-08-31 10:23             ` Kahola, Mika
  0 siblings, 1 reply; 13+ messages in thread
From: Gustavo Sousa @ 2023-08-30 12:18 UTC (permalink / raw)
  To: Kahola, Mika, Sripada, Radhakrishna, intel-gfx

Quoting Gustavo Sousa (2023-08-29 08:45:41-03:00)
>Quoting Kahola, Mika (2023-08-29 06:35:17-03:00)
>>> -----Original Message-----
>>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Sripada, Radhakrishna
>>> Sent: Tuesday, August 29, 2023 1:54 AM
>>> To: Sousa, Gustavo <gustavo.sousa@intel.com>; intel-gfx@lists.freedesktop.org
>>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
>>> 
>>> Hi Gustavo,
>>> 
>>> > -----Original Message-----
>>> > From: Sousa, Gustavo <gustavo.sousa@intel.com>
>>> > Sent: Monday, August 28, 2023 2:46 PM
>>> > To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
>>> > gfx@lists.freedesktop.org
>>> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase
>>> > msgbus timeout threshold
>>> >
>>> > Quoting Sripada, Radhakrishna (2023-08-28 17:30:19-03:00)
>>> > >Hi Gustavo,
>>> >
>>> > Hi, RK.
>>> >
>>> > Thanks for the feedback! Please, see my replies below.
>>> >
>>> > >
>>> > >> -----Original Message-----
>>> > >> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
>>> > >> Of
>>> > Gustavo
>>> > >> Sousa
>>> > >> Sent: Monday, August 28, 2023 10:34 AM
>>> > >> To: intel-gfx@lists.freedesktop.org
>>> > >> Subject: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase
>>> > >> msgbus
>>> > timeout
>>> > >> threshold
>>> > >>
>>> > >> We have experienced timeout issues when accessing C20 SRAM registers.
>>> > >This wording is misleading. It seems to imply that we directly access
>>> > >SRAM registers via msg bus but the reads/writes go through
>>> > >intermediate registers and it is this read/write that is failing. Adding extra clarity here would be helpful.
>>> >
>>> > Hm... Okay. I thought that would already be implied to someone
>>> > familiar with the code. What about:
>>> >
>>> >     s/when accessing/when going through the sequence to access/
>>> This is better to indicate that it is not direct access.
>>> 
>>> >
>>> > ?
>>> >
>>> > >
>>> > >> Experimentation showed that bumping the message bus timer threshold
>>> > >> helped on getting display Type-C connection on the C20 PHY to work.
>>> > >>
>>> > >> While the timeout is still under investigation with the HW team,
>>> > >> having logic to allow forward progress (with the proper warnings) seems useful.
>>> > >> Thus, let's bump the threshold when a timeout is detected.
>>> > >>
>>> > >> The maximum value of 0x200 pclk cycles was somewhat arbitrary - 2x
>>> > >> the default value. That value was successfully tested on real
>>> > >> hardware that was displaying timeouts otherwise.
>>> > >>
>>> > >> BSpec: 65156
>>> > >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>> > >> ---
>>> > >>  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 41
>>> > >> +++++++++++++++++++  .../gpu/drm/i915/display/intel_cx0_phy_regs.h
>>> > >> | 12 ++++++
>>> > >>  2 files changed, 53 insertions(+)
>>> > >>
>>> > >> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>>> > >> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>>> > >> index dd489b50ad60..55d2333032e3 100644
>>> > >> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>>> > >> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>>> > >> @@ -5,6 +5,7 @@
>>> > >>
>>> > >>  #include <linux/log2.h>
>>> > >>  #include <linux/math64.h>
>>> > >> +#include <linux/minmax.h>
>>> > >>  #include "i915_reg.h"
>>> > >>  #include "intel_cx0_phy.h"
>>> > >>  #include "intel_cx0_phy_regs.h"
>>> > >> @@ -29,6 +30,8 @@
>>> > >>  #define INTEL_CX0_LANE1                BIT(1)
>>> > >>  #define INTEL_CX0_BOTH_LANES        (INTEL_CX0_LANE1 |
>>> > >> INTEL_CX0_LANE0)
>>> > >>
>>> > >> +#define INTEL_CX0_MSGBUS_TIMER_VAL_MAX        0x200
>>> > >> +
>>> > >>  bool intel_is_c10phy(struct drm_i915_private *i915, enum phy phy)
>>> > >> {
>>> > >>          if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && phy <
>>> > >> PHY_C) @@ -119,6 +122,43 @@ static void intel_cx0_bus_reset(struct
>>> > drm_i915_private
>>> > >> *i915, enum port port, i
>>> > >>          intel_clear_response_ready_flag(i915, port, lane);  }
>>> > >>
>>> > >> +/*
>>> > >> + * Check if there was a timeout detected by the hardware in the
>>> > >> +message bus
>>> > >> + * and bump the threshold if so.
>>
>>Just a thought but wouldn't it be simpler if we just set the timeout to it's maximum and discard the check here?
>
>I'm not sure which exactly check you are talking about here:
>
>  1) The check on the XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT.
>
>     I think this could give us useful debugging info, for example, if our code
>     thinks the message bus timed out while the bus hardware did not say so. I
>     would prefer to keep this one, if you are okay with it.
>
>  2) The check on the current value of the threshold and the exponential
>     increase up to the maximum.
>
>     I would agree that I did a bit of over-engineering here. I guess I could
>     simply to a rmw using INTEL_CX0_MSGBUS_TIMER_VAL_MAX directly as you
>     suggested; and maybe rename INTEL_CX0_MSGBUS_TIMER_VAL_MAX to
>     INTEL_CX0_MSGBUS_TIMER_BUMPED_VAL, just not to give the impression that
>     0x200 is the maximum accepted by the hardware.
>
>What do you think?

I've sent a v2 addressing (2).

--
Gustavo Sousa

>
>--
>Gustavo Sousa
>
>>
>>-Mika- 
>>
>>> > >> + */
>>> > >> +static void intel_cx0_bus_check_and_bump_timer(struct
>>> > >> +drm_i915_private
>>> > >> *i915,
>>> > >> +                                               enum port port, int
>>> > >> +lane) {
>>> > >> +        enum phy phy = intel_port_to_phy(i915, port);
>>> > >> +        i915_reg_t reg;
>>> > >> +        u32 val;
>>> > >> +        u32 timer_val;
>>> > >> +
>>> > >> +        reg = XELPDP_PORT_MSGBUS_TIMER(port, lane);
>>> > >> +        val = intel_de_read(i915, reg);
>>> > >> +
>>> > >> +        if (!(val & XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT)) {
>>> > >> +                drm_warn(&i915->drm,
>>> > >> +                         "PHY %c lane %d: hardware did not detect
>>> > >> + a
>>> > >> timeout\n",
>>> > >> +                         phy_name(phy), lane);
>>> > >> +                return;
>>> > >> +        }
>>> > >> +
>>> > >> +        timer_val =
>>> > >> REG_FIELD_GET(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK, val);
>>> > >> +
>>> > >> +        if (timer_val == INTEL_CX0_MSGBUS_TIMER_VAL_MAX)
>>> > >> +                return;
>>> > >> +
>>> > >> +        timer_val = min(2 * timer_val,
>>> > >> (u32)INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
>>> > >     ^ is this cast necessary?
>>> >
>>> > Yes. I remember getting a warning without it. Let me remove it to remember...
>>> Got it thanks for the quick check.
>>> 
>>> >
>>> > ...done! I think it complains because the arguments are of different types:
>>> >
>>> >     In file included from drivers/gpu/drm/i915/display/intel_cx0_phy.c:8:
>>> >     drivers/gpu/drm/i915/display/intel_cx0_phy.c: In function
>>> > ‘intel_cx0_bus_check_and_bump_timer’:
>>> >     ./include/linux/minmax.h:20:35: error: comparison of distinct
>>> > pointer types lacks a cast [-Werror]
>>> >        20 |         (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>>> >           |                                   ^~
>>> >     ./include/linux/minmax.h:26:18: note: in expansion of macro ‘__typecheck’
>>> >        26 |                 (__typecheck(x, y) && __no_side_effects(x, y))
>>> >           |                  ^~~~~~~~~~~
>>> >     ./include/linux/minmax.h:36:31: note: in expansion of macro ‘__safe_cmp’
>>> >        36 |         __builtin_choose_expr(__safe_cmp(x, y), \
>>> >           |                               ^~~~~~~~~~
>>> >     ./include/linux/minmax.h:67:25: note: in expansion of macro ‘__careful_cmp’
>>> >        67 | #define min(x, y)       __careful_cmp(x, y, <)
>>> >           |                         ^~~~~~~~~~~~~
>>> >     drivers/gpu/drm/i915/display/intel_cx0_phy.c:152:21: note: in
>>> > expansion of macro ‘min’
>>> >       152 |         timer_val = min(2 * timer_val,
>>> > INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
>>> >           |
>>> >
>>> > >
>>> > >> +        val &= ~XELPDP_PORT_MSGBUS_TIMER_VAL_MASK;
>>> > >> +        val |= REG_FIELD_PREP(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK,
>>> > >> timer_val);
>>> > >We do not use REG_FIELD_PREP in the function. Instead we use it in
>>> > >register definition in intel_cx0_phy_regs.h.
>>> >
>>> > I think it makes sense have definitions using REG_FIELD_PREP() in
>>> > header files and use those definitions in .c files for fields whose
>>> > values are are enumerated.
>>> >
>>> > Now, for fields without enumeration, like this one in discussion, I
>>> > think it is fine to use REG_FIELD_PREP() directly. The MASK is already exposed anyway...
>>> >
>>> > Besides, it seems we already have lines of code in *.c files using the
>>> > pattern
>>> > above:
>>> >
>>> >     git grep REG_FIELD_PREP drm-tip/drm-tip -- ':/drivers/gpu/drm/i915/**/*.c'
>>> >
>>> > Let me know what you think. I could add a
>>> > XELPDP_PORT_MSGBUS_TIMER_VAL() macro if you think it is necessary. My
>>> > personal take is that using REG_FIELD_PREP() for this case is fine.
>>> Let us conform with the norms for the macro-fields used in this file instead of starting a new pattern.
>>> 
>>> --Radhakrishna(RK) Sripada
>>> >
>>> > --
>>> > Gustavo Sousa
>>> >
>>> > >
>>> > >Thanks,
>>> > >Radhakrishna Sripada
>>> > >
>>> > >> +
>>> > >> +        drm_dbg_kms(&i915->drm,
>>> > >> +                    "PHY %c lane %d: increasing msgbus timer
>>> > >> + threshold to
>>> > >> %#x\n",
>>> > >> +                    phy_name(phy), lane, timer_val);
>>> > >> +        intel_de_write(i915, reg, val); }
>>> > >> +
>>> > >>  static int intel_cx0_wait_for_ack(struct drm_i915_private *i915,
>>> > >> enum port
>>> > port,
>>> > >>                                    int command, int lane, u32 *val)
>>> > >> { @@ -132,6 +172,7 @@ static int intel_cx0_wait_for_ack(struct
>>> > >> drm_i915_private *i915, enum port port,
>>> > >>
>>> > >> XELPDP_MSGBUS_TIMEOUT_SLOW,
>>> > >> val)) {
>>> > >>                  drm_dbg_kms(&i915->drm, "PHY %c Timeout waiting
>>> > >> for message ACK. Status: 0x%x\n",
>>> > >>                              phy_name(phy), *val);
>>> > >> +                intel_cx0_bus_check_and_bump_timer(i915, port,
>>> > >> + lane);
>>> > >>                  intel_cx0_bus_reset(i915, port, lane);
>>> > >>                  return -ETIMEDOUT;
>>> > >>          }
>>> > >> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>>> > >> b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>>> > >> index cb5d1be2ba19..17cc3385aabb 100644
>>> > >> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>>> > >> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
>>> > >> @@ -110,6 +110,18 @@
>>> > >>  #define   CX0_P4PG_STATE_DISABLE                        0xC
>>> > >>  #define   CX0_P2_STATE_RESET                                0x2
>>> > >>
>>> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_A
>>> > >>         0x640d8
>>> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_B
>>> > >>         0x641d8
>>> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1                0x16f258
>>> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2                0x16f458
>>> > >> +#define XELPDP_PORT_MSGBUS_TIMER(port, lane)
>>> > >>         _MMIO(_PICK_EVEN_2RANGES(port, PORT_TC1, \
>>> > >> +
>>> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_A, \
>>> > >> +
>>> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_B, \
>>> > >> +
>>> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1, \
>>> > >> +
>>> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2) + (lane) * 4)
>>> > >> +#define   XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT                REG_BIT(31)
>>> > >> +#define   XELPDP_PORT_MSGBUS_TIMER_VAL_MASK
>>> > >>         REG_GENMASK(23, 0)
>>> > >> +
>>> > >>  #define _XELPDP_PORT_CLOCK_CTL_A                        0x640E0
>>> > >>  #define _XELPDP_PORT_CLOCK_CTL_B                        0x641E0
>>> > >>  #define _XELPDP_PORT_CLOCK_CTL_USBC1                        0x16F260
>>> > >> --
>>> > >> 2.41.0
>>> > >

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

* Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
  2023-08-30 12:18           ` Gustavo Sousa
@ 2023-08-31 10:23             ` Kahola, Mika
  0 siblings, 0 replies; 13+ messages in thread
From: Kahola, Mika @ 2023-08-31 10:23 UTC (permalink / raw)
  To: Sousa, Gustavo, Sripada, Radhakrishna, intel-gfx

> -----Original Message-----
> From: Sousa, Gustavo <gustavo.sousa@intel.com>
> Sent: Wednesday, August 30, 2023 3:19 PM
> To: Kahola, Mika <mika.kahola@intel.com>; Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold
> 
> Quoting Gustavo Sousa (2023-08-29 08:45:41-03:00)
> >Quoting Kahola, Mika (2023-08-29 06:35:17-03:00)
> >>> -----Original Message-----
> >>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
> >>> Of Sripada, Radhakrishna
> >>> Sent: Tuesday, August 29, 2023 1:54 AM
> >>> To: Sousa, Gustavo <gustavo.sousa@intel.com>;
> >>> intel-gfx@lists.freedesktop.org
> >>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase
> >>> msgbus timeout threshold
> >>>
> >>> Hi Gustavo,
> >>>
> >>> > -----Original Message-----
> >>> > From: Sousa, Gustavo <gustavo.sousa@intel.com>
> >>> > Sent: Monday, August 28, 2023 2:46 PM
> >>> > To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
> >>> > gfx@lists.freedesktop.org
> >>> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase
> >>> > msgbus timeout threshold
> >>> >
> >>> > Quoting Sripada, Radhakrishna (2023-08-28 17:30:19-03:00)
> >>> > >Hi Gustavo,
> >>> >
> >>> > Hi, RK.
> >>> >
> >>> > Thanks for the feedback! Please, see my replies below.
> >>> >
> >>> > >
> >>> > >> -----Original Message-----
> >>> > >> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On
> >>> > >> Behalf Of
> >>> > Gustavo
> >>> > >> Sousa
> >>> > >> Sent: Monday, August 28, 2023 10:34 AM
> >>> > >> To: intel-gfx@lists.freedesktop.org
> >>> > >> Subject: [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase
> >>> > >> msgbus
> >>> > timeout
> >>> > >> threshold
> >>> > >>
> >>> > >> We have experienced timeout issues when accessing C20 SRAM registers.
> >>> > >This wording is misleading. It seems to imply that we directly
> >>> > >access SRAM registers via msg bus but the reads/writes go through
> >>> > >intermediate registers and it is this read/write that is failing. Adding extra clarity here would be helpful.
> >>> >
> >>> > Hm... Okay. I thought that would already be implied to someone
> >>> > familiar with the code. What about:
> >>> >
> >>> >     s/when accessing/when going through the sequence to access/
> >>> This is better to indicate that it is not direct access.
> >>>
> >>> >
> >>> > ?
> >>> >
> >>> > >
> >>> > >> Experimentation showed that bumping the message bus timer
> >>> > >> threshold helped on getting display Type-C connection on the C20 PHY to work.
> >>> > >>
> >>> > >> While the timeout is still under investigation with the HW
> >>> > >> team, having logic to allow forward progress (with the proper warnings) seems useful.
> >>> > >> Thus, let's bump the threshold when a timeout is detected.
> >>> > >>
> >>> > >> The maximum value of 0x200 pclk cycles was somewhat arbitrary -
> >>> > >> 2x the default value. That value was successfully tested on
> >>> > >> real hardware that was displaying timeouts otherwise.
> >>> > >>
> >>> > >> BSpec: 65156
> >>> > >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> >>> > >> ---
> >>> > >>  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 41
> >>> > >> +++++++++++++++++++
> >>> > >> +++++++++++++++++++ .../gpu/drm/i915/display/intel_cx0_phy_regs
> >>> > >> +++++++++++++++++++ .h
> >>> > >> | 12 ++++++
> >>> > >>  2 files changed, 53 insertions(+)
> >>> > >>
> >>> > >> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >>> > >> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >>> > >> index dd489b50ad60..55d2333032e3 100644
> >>> > >> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >>> > >> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >>> > >> @@ -5,6 +5,7 @@
> >>> > >>
> >>> > >>  #include <linux/log2.h>
> >>> > >>  #include <linux/math64.h>
> >>> > >> +#include <linux/minmax.h>
> >>> > >>  #include "i915_reg.h"
> >>> > >>  #include "intel_cx0_phy.h"
> >>> > >>  #include "intel_cx0_phy_regs.h"
> >>> > >> @@ -29,6 +30,8 @@
> >>> > >>  #define INTEL_CX0_LANE1                BIT(1)
> >>> > >>  #define INTEL_CX0_BOTH_LANES        (INTEL_CX0_LANE1 |
> >>> > >> INTEL_CX0_LANE0)
> >>> > >>
> >>> > >> +#define INTEL_CX0_MSGBUS_TIMER_VAL_MAX        0x200
> >>> > >> +
> >>> > >>  bool intel_is_c10phy(struct drm_i915_private *i915, enum phy
> >>> > >> phy) {
> >>> > >>          if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0) && phy <
> >>> > >> PHY_C) @@ -119,6 +122,43 @@ static void
> >>> > >> intel_cx0_bus_reset(struct
> >>> > drm_i915_private
> >>> > >> *i915, enum port port, i
> >>> > >>          intel_clear_response_ready_flag(i915, port, lane);  }
> >>> > >>
> >>> > >> +/*
> >>> > >> + * Check if there was a timeout detected by the hardware in
> >>> > >> +the message bus
> >>> > >> + * and bump the threshold if so.
> >>
> >>Just a thought but wouldn't it be simpler if we just set the timeout to it's maximum and discard the check here?
> >
> >I'm not sure which exactly check you are talking about here:
> >
> >  1) The check on the XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT.
> >
> >     I think this could give us useful debugging info, for example, if our code
> >     thinks the message bus timed out while the bus hardware did not say so. I
> >     would prefer to keep this one, if you are okay with it.
> >
> >  2) The check on the current value of the threshold and the exponential
> >     increase up to the maximum.
> >
> >     I would agree that I did a bit of over-engineering here. I guess I could
> >     simply to a rmw using INTEL_CX0_MSGBUS_TIMER_VAL_MAX directly as you
> >     suggested; and maybe rename INTEL_CX0_MSGBUS_TIMER_VAL_MAX to
> >     INTEL_CX0_MSGBUS_TIMER_BUMPED_VAL, just not to give the impression that
> >     0x200 is the maximum accepted by the hardware.
> >
> >What do you think?
> 
> I've sent a v2 addressing (2).

Yes, you're right, we would benefit from this timeout check. I think your approach to directly bump up timeout to it's max is a way to go.

I will have a look at the v2 patch.

-Mika-
> 
> --
> Gustavo Sousa
> 
> >
> >--
> >Gustavo Sousa
> >
> >>
> >>-Mika-
> >>
> >>> > >> + */
> >>> > >> +static void intel_cx0_bus_check_and_bump_timer(struct
> >>> > >> +drm_i915_private
> >>> > >> *i915,
> >>> > >> +                                               enum port port,
> >>> > >> +int
> >>> > >> +lane) {
> >>> > >> +        enum phy phy = intel_port_to_phy(i915, port);
> >>> > >> +        i915_reg_t reg;
> >>> > >> +        u32 val;
> >>> > >> +        u32 timer_val;
> >>> > >> +
> >>> > >> +        reg = XELPDP_PORT_MSGBUS_TIMER(port, lane);
> >>> > >> +        val = intel_de_read(i915, reg);
> >>> > >> +
> >>> > >> +        if (!(val & XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT)) {
> >>> > >> +                drm_warn(&i915->drm,
> >>> > >> +                         "PHY %c lane %d: hardware did not
> >>> > >> + detect a
> >>> > >> timeout\n",
> >>> > >> +                         phy_name(phy), lane);
> >>> > >> +                return;
> >>> > >> +        }
> >>> > >> +
> >>> > >> +        timer_val =
> >>> > >> REG_FIELD_GET(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK, val);
> >>> > >> +
> >>> > >> +        if (timer_val == INTEL_CX0_MSGBUS_TIMER_VAL_MAX)
> >>> > >> +                return;
> >>> > >> +
> >>> > >> +        timer_val = min(2 * timer_val,
> >>> > >> (u32)INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
> >>> > >     ^ is this cast necessary?
> >>> >
> >>> > Yes. I remember getting a warning without it. Let me remove it to remember...
> >>> Got it thanks for the quick check.
> >>>
> >>> >
> >>> > ...done! I think it complains because the arguments are of different types:
> >>> >
> >>> >     In file included from drivers/gpu/drm/i915/display/intel_cx0_phy.c:8:
> >>> >     drivers/gpu/drm/i915/display/intel_cx0_phy.c: In function
> >>> > ‘intel_cx0_bus_check_and_bump_timer’:
> >>> >     ./include/linux/minmax.h:20:35: error: comparison of distinct
> >>> > pointer types lacks a cast [-Werror]
> >>> >        20 |         (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> >>> >           |                                   ^~
> >>> >     ./include/linux/minmax.h:26:18: note: in expansion of macro ‘__typecheck’
> >>> >        26 |                 (__typecheck(x, y) && __no_side_effects(x, y))
> >>> >           |                  ^~~~~~~~~~~
> >>> >     ./include/linux/minmax.h:36:31: note: in expansion of macro ‘__safe_cmp’
> >>> >        36 |         __builtin_choose_expr(__safe_cmp(x, y), \
> >>> >           |                               ^~~~~~~~~~
> >>> >     ./include/linux/minmax.h:67:25: note: in expansion of macro ‘__careful_cmp’
> >>> >        67 | #define min(x, y)       __careful_cmp(x, y, <)
> >>> >           |                         ^~~~~~~~~~~~~
> >>> >     drivers/gpu/drm/i915/display/intel_cx0_phy.c:152:21: note: in
> >>> > expansion of macro ‘min’
> >>> >       152 |         timer_val = min(2 * timer_val,
> >>> > INTEL_CX0_MSGBUS_TIMER_VAL_MAX);
> >>> >           |
> >>> >
> >>> > >
> >>> > >> +        val &= ~XELPDP_PORT_MSGBUS_TIMER_VAL_MASK;
> >>> > >> +        val |=
> >>> > >> + REG_FIELD_PREP(XELPDP_PORT_MSGBUS_TIMER_VAL_MASK,
> >>> > >> timer_val);
> >>> > >We do not use REG_FIELD_PREP in the function. Instead we use it
> >>> > >in register definition in intel_cx0_phy_regs.h.
> >>> >
> >>> > I think it makes sense have definitions using REG_FIELD_PREP() in
> >>> > header files and use those definitions in .c files for fields
> >>> > whose values are are enumerated.
> >>> >
> >>> > Now, for fields without enumeration, like this one in discussion,
> >>> > I think it is fine to use REG_FIELD_PREP() directly. The MASK is already exposed anyway...
> >>> >
> >>> > Besides, it seems we already have lines of code in *.c files using
> >>> > the pattern
> >>> > above:
> >>> >
> >>> >     git grep REG_FIELD_PREP drm-tip/drm-tip -- ':/drivers/gpu/drm/i915/**/*.c'
> >>> >
> >>> > Let me know what you think. I could add a
> >>> > XELPDP_PORT_MSGBUS_TIMER_VAL() macro if you think it is necessary.
> >>> > My personal take is that using REG_FIELD_PREP() for this case is fine.
> >>> Let us conform with the norms for the macro-fields used in this file instead of starting a new pattern.
> >>>
> >>> --Radhakrishna(RK) Sripada
> >>> >
> >>> > --
> >>> > Gustavo Sousa
> >>> >
> >>> > >
> >>> > >Thanks,
> >>> > >Radhakrishna Sripada
> >>> > >
> >>> > >> +
> >>> > >> +        drm_dbg_kms(&i915->drm,
> >>> > >> +                    "PHY %c lane %d: increasing msgbus timer
> >>> > >> + threshold to
> >>> > >> %#x\n",
> >>> > >> +                    phy_name(phy), lane, timer_val);
> >>> > >> +        intel_de_write(i915, reg, val); }
> >>> > >> +
> >>> > >>  static int intel_cx0_wait_for_ack(struct drm_i915_private
> >>> > >> *i915, enum port
> >>> > port,
> >>> > >>                                    int command, int lane, u32
> >>> > >> *val) { @@ -132,6 +172,7 @@ static int
> >>> > >> intel_cx0_wait_for_ack(struct drm_i915_private *i915, enum port
> >>> > >> port,
> >>> > >>
> >>> > >> XELPDP_MSGBUS_TIMEOUT_SLOW,
> >>> > >> val)) {
> >>> > >>                  drm_dbg_kms(&i915->drm, "PHY %c Timeout
> >>> > >> waiting for message ACK. Status: 0x%x\n",
> >>> > >>                              phy_name(phy), *val);
> >>> > >> +                intel_cx0_bus_check_and_bump_timer(i915, port,
> >>> > >> + lane);
> >>> > >>                  intel_cx0_bus_reset(i915, port, lane);
> >>> > >>                  return -ETIMEDOUT;
> >>> > >>          }
> >>> > >> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> >>> > >> b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> >>> > >> index cb5d1be2ba19..17cc3385aabb 100644
> >>> > >> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> >>> > >> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> >>> > >> @@ -110,6 +110,18 @@
> >>> > >>  #define   CX0_P4PG_STATE_DISABLE                        0xC
> >>> > >>  #define   CX0_P2_STATE_RESET                                0x2
> >>> > >>
> >>> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_A
> >>> > >>         0x640d8
> >>> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_B
> >>> > >>         0x641d8
> >>> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1                0x16f258
> >>> > >> +#define _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2                0x16f458
> >>> > >> +#define XELPDP_PORT_MSGBUS_TIMER(port, lane)
> >>> > >>         _MMIO(_PICK_EVEN_2RANGES(port, PORT_TC1, \
> >>> > >> +
> >>> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_A, \
> >>> > >> +
> >>> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_B, \
> >>> > >> +
> >>> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC1, \
> >>> > >> +
> >>> > >>          _XELPDP_PORT_MSGBUS_TIMER_LN0_USBC2) + (lane) * 4)
> >>> > >> +#define   XELPDP_PORT_MSGBUS_TIMER_TIMED_OUT                REG_BIT(31)
> >>> > >> +#define   XELPDP_PORT_MSGBUS_TIMER_VAL_MASK
> >>> > >>         REG_GENMASK(23, 0)
> >>> > >> +
> >>> > >>  #define _XELPDP_PORT_CLOCK_CTL_A                        0x640E0
> >>> > >>  #define _XELPDP_PORT_CLOCK_CTL_B                        0x641E0
> >>> > >>  #define _XELPDP_PORT_CLOCK_CTL_USBC1                        0x16F260
> >>> > >> --
> >>> > >> 2.41.0
> >>> > >

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

end of thread, other threads:[~2023-08-31 10:23 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-28 17:34 [Intel-gfx] [PATCH] drm/i915/cx0: Check and increase msgbus timeout threshold Gustavo Sousa
2023-08-28 18:50 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2023-08-28 19:08 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2023-08-28 20:30 ` [Intel-gfx] [PATCH] " Sripada, Radhakrishna
2023-08-28 21:45   ` Gustavo Sousa
2023-08-28 22:53     ` Sripada, Radhakrishna
2023-08-29  9:35       ` Kahola, Mika
2023-08-29 11:45         ` Gustavo Sousa
2023-08-30 12:18           ` Gustavo Sousa
2023-08-31 10:23             ` Kahola, Mika
2023-08-29 15:12     ` Jani Nikula
2023-08-30 12:17       ` Gustavo Sousa
2023-08-28 22:13 ` [Intel-gfx] ✓ Fi.CI.IGT: success for " 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.