intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode
@ 2021-06-22  9:34 Tejas Upadhyay
  2021-06-22 12:44 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gen11: Disable cursor clock gating in HDR mode (rev2) Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Tejas Upadhyay @ 2021-06-22  9:34 UTC (permalink / raw)
  To: intel-gfx

Display underrun in HDR mode when cursor is enabled.
RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h.
As per W/A 1604331009, Disable cursor clock gating in HDR mode.

Bspec : 33451

Changes since V3:
	- Disable WA when not in HDR mode or cursor plane not active - Ville
	- Extract required args from crtc_state - Ville
	- Create HDR mode API using bdw_set_pipemisc ref - Ville
	- Tested with HDR video as well full setmode, WA applies and disables
Changes since V2:
        - Made it general gen11 WA
        - Removed WA needed check
        - Added cursor plane active check
        - Once WA enable, software will not disable
Changes since V1:
        - Modified way CLKGATE_DIS_PSL bit 28 was modified

Cc: Souza Jose <jose.souza@intel.com>
Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 27 ++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h              |  5 ++++
 2 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 6be1b31af07b..e1ea03b918df 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -358,6 +358,13 @@ static void intel_update_czclk(struct drm_i915_private *dev_priv)
 		dev_priv->czclk_freq);
 }
 
+static bool
+is_hdr_mode(const struct intel_crtc_state *crtc_state)
+{
+	return (crtc_state->active_planes & ~(icl_hdr_plane_mask() |
+		BIT(PLANE_CURSOR))) == 0;
+}
+
 /* WA Display #0827: Gen9:all */
 static void
 skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe, bool enable)
@@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe,
 		               intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) & ~DPFR_GATING_DIS);
 }
 
+/* Wa_1604331009:icl,jsl,ehl */
+	static void
+icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+
+	if (is_hdr_mode(crtc_state) &&
+	    crtc_state->active_planes & BIT(PLANE_CURSOR) &&
+	    IS_GEN(dev_priv, 11))
+		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe),
+			     CURSOR_GATING_DIS, CURSOR_GATING_DIS);
+	else
+		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe),
+			     CURSOR_GATING_DIS, 0);
+}
+
 static bool
 is_trans_port_sync_slave(const struct intel_crtc_state *crtc_state)
 {
@@ -2939,6 +2963,9 @@ static void intel_pre_plane_update(struct intel_atomic_state *state,
 	    needs_scalerclk_wa(new_crtc_state))
 		icl_wa_scalerclkgating(dev_priv, pipe, true);
 
+	/* Wa_1604331009:icl,jsl,ehl */
+	icl_wa_cursorclkgating(new_crtc_state);
+
 	/*
 	 * Vblank time updates from the shadow to live plane control register
 	 * are blocked if the memory self-refresh mode is active at that
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c857fafb8a30..703d708c773e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4235,6 +4235,11 @@ enum {
 #define INF_UNIT_LEVEL_CLKGATE		_MMIO(0x9560)
 #define   CGPSF_CLKGATE_DIS		(1 << 3)
 
+/*
+ * GEN11 clock gating regs
+ */
+#define   CURSOR_GATING_DIS            BIT(28)
+
 /*
  * Display engine regs
  */
-- 
2.31.1

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

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

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gen11: Disable cursor clock gating in HDR mode (rev2)
  2021-06-22  9:34 [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode Tejas Upadhyay
@ 2021-06-22 12:44 ` Patchwork
  2021-06-25  7:03 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gen11: Disable cursor clock gating in HDR mode (rev3) Patchwork
  2021-09-01 13:48 ` [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode Ville Syrjälä
  2 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2021-06-22 12:44 UTC (permalink / raw)
  To: Tejas Upadhyay; +Cc: intel-gfx


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

== Series Details ==

Series: drm/i915/gen11: Disable cursor clock gating in HDR mode (rev2)
URL   : https://patchwork.freedesktop.org/series/91674/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10261 -> Patchwork_20427
====================================================

Summary
-------

  **FAILURE**

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

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

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - fi-bdw-5557u:       NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20427/fi-bdw-5557u/igt@runner@aborted.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_exec_suspend@basic-s3:
    - {fi-ehl-2}:         [PASS][2] -> [INCOMPLETE][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10261/fi-ehl-2/igt@gem_exec_suspend@basic-s3.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20427/fi-ehl-2/igt@gem_exec_suspend@basic-s3.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-gfx:
    - fi-skl-6700k2:      NOTRUN -> [SKIP][4] ([fdo#109271]) +24 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20427/fi-skl-6700k2/igt@amdgpu/amd_basic@cs-gfx.html
    - fi-kbl-soraka:      NOTRUN -> [SKIP][5] ([fdo#109271]) +12 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20427/fi-kbl-soraka/igt@amdgpu/amd_basic@cs-gfx.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-skl-6700k2:      NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#533])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20427/fi-skl-6700k2/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@runner@aborted:
    - fi-hsw-4770:        NOTRUN -> [FAIL][7] ([i915#192] / [i915#193] / [i915#194] / [i915#2505])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20427/fi-hsw-4770/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-skl-6700k2:      [INCOMPLETE][8] ([i915#146] / [i915#2405]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10261/fi-skl-6700k2/igt@kms_chamelium@common-hpd-after-suspend.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20427/fi-skl-6700k2/igt@kms_chamelium@common-hpd-after-suspend.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#192]: https://gitlab.freedesktop.org/drm/intel/issues/192
  [i915#193]: https://gitlab.freedesktop.org/drm/intel/issues/193
  [i915#194]: https://gitlab.freedesktop.org/drm/intel/issues/194
  [i915#2405]: https://gitlab.freedesktop.org/drm/intel/issues/2405
  [i915#2505]: https://gitlab.freedesktop.org/drm/intel/issues/2505
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533


Participating hosts (44 -> 37)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-apl-guc fi-snb-2520m fi-ctg-p8600 fi-bdw-samus 


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

  * Linux: CI_DRM_10261 -> Patchwork_20427

  CI-20190529: 20190529
  CI_DRM_10261: 132b189b72a94328f17fd70321bfe63e5b4208e9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6117: 3ba0a02404f243d6d8f232c6215163cc4b0fd699 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20427: 8aaf8e7a39d13775853b775d6992fb44d5b10125 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

8aaf8e7a39d1 drm/i915/gen11: Disable cursor clock gating in HDR mode

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 5392 bytes --]

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

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

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

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gen11: Disable cursor clock gating in HDR mode (rev3)
  2021-06-22  9:34 [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode Tejas Upadhyay
  2021-06-22 12:44 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gen11: Disable cursor clock gating in HDR mode (rev2) Patchwork
@ 2021-06-25  7:03 ` Patchwork
  2021-09-01 13:48 ` [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode Ville Syrjälä
  2 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2021-06-25  7:03 UTC (permalink / raw)
  To: Tejas Upadhyay; +Cc: intel-gfx


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

== Series Details ==

Series: drm/i915/gen11: Disable cursor clock gating in HDR mode (rev3)
URL   : https://patchwork.freedesktop.org/series/91674/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10279 -> Patchwork_20464
====================================================

Summary
-------

  **FAILURE**

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

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

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - fi-bdw-5557u:       NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20464/fi-bdw-5557u/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       [PASS][2] -> [FAIL][3] ([i915#1372])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10279/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20464/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html

  * igt@runner@aborted:
    - fi-hsw-4770:        NOTRUN -> [FAIL][4] ([i915#192] / [i915#193] / [i915#194] / [i915#2505])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20464/fi-hsw-4770/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - {fi-tgl-dsi}:       [DMESG-WARN][5] ([i915#1982] / [k.org#205379]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10279/fi-tgl-dsi/igt@i915_module_load@reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20464/fi-tgl-dsi/igt@i915_module_load@reload.html

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

  [i915#1372]: https://gitlab.freedesktop.org/drm/intel/issues/1372
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#192]: https://gitlab.freedesktop.org/drm/intel/issues/192
  [i915#193]: https://gitlab.freedesktop.org/drm/intel/issues/193
  [i915#194]: https://gitlab.freedesktop.org/drm/intel/issues/194
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2505]: https://gitlab.freedesktop.org/drm/intel/issues/2505
  [k.org#205379]: https://bugzilla.kernel.org/show_bug.cgi?id=205379


Participating hosts (43 -> 37)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-kbl-guc fi-bsw-nick fi-bdw-samus 


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

  * Linux: CI_DRM_10279 -> Patchwork_20464

  CI-20190529: 20190529
  CI_DRM_10279: a996e82cbdc77fc789d0a385602e02f7e2478a1e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6119: a306810ebbc8984bde38a57ef0c33eea394f4e18 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20464: 6281662b5a6d2db92e48e8bb587b64ff35047eff @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6281662b5a6d drm/i915/gen11: Disable cursor clock gating in HDR mode

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 4230 bytes --]

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

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

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

* Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode
  2021-06-22  9:34 [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode Tejas Upadhyay
  2021-06-22 12:44 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gen11: Disable cursor clock gating in HDR mode (rev2) Patchwork
  2021-06-25  7:03 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gen11: Disable cursor clock gating in HDR mode (rev3) Patchwork
@ 2021-09-01 13:48 ` Ville Syrjälä
  2021-09-02 11:07   ` Surendrakumar Upadhyay, TejaskumarX
  2 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2021-09-01 13:48 UTC (permalink / raw)
  To: Tejas Upadhyay; +Cc: intel-gfx

On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote:
> Display underrun in HDR mode when cursor is enabled.
> RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h.
> As per W/A 1604331009, Disable cursor clock gating in HDR mode.
> 
> Bspec : 33451
> 
> Changes since V3:
> 	- Disable WA when not in HDR mode or cursor plane not active - Ville
> 	- Extract required args from crtc_state - Ville
> 	- Create HDR mode API using bdw_set_pipemisc ref - Ville
> 	- Tested with HDR video as well full setmode, WA applies and disables
> Changes since V2:
>         - Made it general gen11 WA
>         - Removed WA needed check
>         - Added cursor plane active check
>         - Once WA enable, software will not disable
> Changes since V1:
>         - Modified way CLKGATE_DIS_PSL bit 28 was modified
> 
> Cc: Souza Jose <jose.souza@intel.com>
> Signed-off-by: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 27 ++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_reg.h              |  5 ++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 6be1b31af07b..e1ea03b918df 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -358,6 +358,13 @@ static void intel_update_czclk(struct drm_i915_private *dev_priv)
>  		dev_priv->czclk_freq);
>  }
>  
> +static bool
> +is_hdr_mode(const struct intel_crtc_state *crtc_state)
> +{
> +	return (crtc_state->active_planes & ~(icl_hdr_plane_mask() |
> +		BIT(PLANE_CURSOR))) == 0;
> +}

Please use this in bdw_set_pipemisc() as well. This could be
a separate prep patch actually.

> +
>  /* WA Display #0827: Gen9:all */
>  static void
>  skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe, bool enable)
> @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe,
>  		               intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) & ~DPFR_GATING_DIS);
>  }
>  
> +/* Wa_1604331009:icl,jsl,ehl */
> +	static void
> +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +
> +	if (is_hdr_mode(crtc_state) &&
> +	    crtc_state->active_planes & BIT(PLANE_CURSOR) &&
> +	    IS_GEN(dev_priv, 11))
> +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe),
> +			     CURSOR_GATING_DIS, CURSOR_GATING_DIS);
> +	else
> +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe),
> +			     CURSOR_GATING_DIS, 0);
> +}
> +
>  static bool
>  is_trans_port_sync_slave(const struct intel_crtc_state *crtc_state)
>  {
> @@ -2939,6 +2963,9 @@ static void intel_pre_plane_update(struct intel_atomic_state *state,
>  	    needs_scalerclk_wa(new_crtc_state))
>  		icl_wa_scalerclkgating(dev_priv, pipe, true);
>  
> +	/* Wa_1604331009:icl,jsl,ehl */
> +	icl_wa_cursorclkgating(new_crtc_state);

This looks a bit wrong. We shouldn't turn the clock gating back on
until after HDR mode has been disabled.

So please model this after skl_wa_827() and icl_wa_scalerclkgating()
so that a) the ordering is correct, and b) the code between all three
w/as looks consistent.

> +
>  	/*
>  	 * Vblank time updates from the shadow to live plane control register
>  	 * are blocked if the memory self-refresh mode is active at that
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c857fafb8a30..703d708c773e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4235,6 +4235,11 @@ enum {
>  #define INF_UNIT_LEVEL_CLKGATE		_MMIO(0x9560)
>  #define   CGPSF_CLKGATE_DIS		(1 << 3)
>  
> +/*
> + * GEN11 clock gating regs
> + */
> +#define   CURSOR_GATING_DIS            BIT(28)

This looks misplaced. It should be next to the other bits of whatever
register this is. Also pls use REG_BIT() instead of BIT().

> +
>  /*
>   * Display engine regs
>   */
> -- 
> 2.31.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel

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

* Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode
  2021-09-01 13:48 ` [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode Ville Syrjälä
@ 2021-09-02 11:07   ` Surendrakumar Upadhyay, TejaskumarX
  2021-09-02 12:59     ` Ville Syrjälä
  0 siblings, 1 reply; 11+ messages in thread
From: Surendrakumar Upadhyay, TejaskumarX @ 2021-09-02 11:07 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx



> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: 01 September 2021 19:19
> To: Surendrakumar Upadhyay, TejaskumarX
> <tejaskumarx.surendrakumar.upadhyay@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock
> gating in HDR mode
> 
> On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote:
> > Display underrun in HDR mode when cursor is enabled.
> > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h.
> > As per W/A 1604331009, Disable cursor clock gating in HDR mode.
> >
> > Bspec : 33451
> >
> > Changes since V3:
> > 	- Disable WA when not in HDR mode or cursor plane not active - Ville
> > 	- Extract required args from crtc_state - Ville
> > 	- Create HDR mode API using bdw_set_pipemisc ref - Ville
> > 	- Tested with HDR video as well full setmode, WA applies and
> disables
> > Changes since V2:
> >         - Made it general gen11 WA
> >         - Removed WA needed check
> >         - Added cursor plane active check
> >         - Once WA enable, software will not disable Changes since V1:
> >         - Modified way CLKGATE_DIS_PSL bit 28 was modified
> >
> > Cc: Souza Jose <jose.souza@intel.com>
> > Signed-off-by: Tejas Upadhyay
> > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 27 ++++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_reg.h              |  5 ++++
> >  2 files changed, 32 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 6be1b31af07b..e1ea03b918df 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct
> drm_i915_private *dev_priv)
> >  		dev_priv->czclk_freq);
> >  }
> >
> > +static bool
> > +is_hdr_mode(const struct intel_crtc_state *crtc_state) {
> > +	return (crtc_state->active_planes & ~(icl_hdr_plane_mask() |
> > +		BIT(PLANE_CURSOR))) == 0;
> > +}
> 
> Please use this in bdw_set_pipemisc() as well. This could be a separate prep
> patch actually.
> 
> > +
> >  /* WA Display #0827: Gen9:all */
> >  static void
> >  skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe, bool
> > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct
> drm_i915_private *dev_priv, enum pipe pipe,
> >  		               intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) &
> > ~DPFR_GATING_DIS);  }
> >
> > +/* Wa_1604331009:icl,jsl,ehl */
> > +	static void
> > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) {
> > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > +
> > +	if (is_hdr_mode(crtc_state) &&
> > +	    crtc_state->active_planes & BIT(PLANE_CURSOR) &&
> > +	    IS_GEN(dev_priv, 11))
> > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe),
> > +			     CURSOR_GATING_DIS, CURSOR_GATING_DIS);
> > +	else
> > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe),
> > +			     CURSOR_GATING_DIS, 0);
> > +}
> > +
> >  static bool
> >  is_trans_port_sync_slave(const struct intel_crtc_state *crtc_state)
> > { @@ -2939,6 +2963,9 @@ static void intel_pre_plane_update(struct
> > intel_atomic_state *state,
> >  	    needs_scalerclk_wa(new_crtc_state))
> >  		icl_wa_scalerclkgating(dev_priv, pipe, true);
> >
> > +	/* Wa_1604331009:icl,jsl,ehl */
> > +	icl_wa_cursorclkgating(new_crtc_state);
> 
> This looks a bit wrong. We shouldn't turn the clock gating back on until after
> HDR mode has been disabled.
> 
> So please model this after skl_wa_827() and icl_wa_scalerclkgating() so that
> a) the ordering is correct, and b) the code between all three w/as looks
> consistent.

I did not get what you are suggesting here. Can you please put psudo? Currently as far as I see icl_wa_cursorclkgating is already modelled  after skl_wa_827() and icl_wa_scalerclkgating(). Are referring same 
Or something else?

Thanks,
Tejas
> 
> > +
> >  	/*
> >  	 * Vblank time updates from the shadow to live plane control register
> >  	 * are blocked if the memory self-refresh mode is active at that
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index c857fafb8a30..703d708c773e
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4235,6 +4235,11 @@ enum {
> >  #define INF_UNIT_LEVEL_CLKGATE		_MMIO(0x9560)
> >  #define   CGPSF_CLKGATE_DIS		(1 << 3)
> >
> > +/*
> > + * GEN11 clock gating regs
> > + */
> > +#define   CURSOR_GATING_DIS            BIT(28)
> 
> This looks misplaced. It should be next to the other bits of whatever register
> this is. Also pls use REG_BIT() instead of BIT().
> 
> > +
> >  /*
> >   * Display engine regs
> >   */
> > --
> > 2.31.1
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel

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

* Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode
  2021-09-02 11:07   ` Surendrakumar Upadhyay, TejaskumarX
@ 2021-09-02 12:59     ` Ville Syrjälä
  2021-09-02 13:04       ` Surendrakumar Upadhyay, TejaskumarX
  0 siblings, 1 reply; 11+ messages in thread
From: Ville Syrjälä @ 2021-09-02 12:59 UTC (permalink / raw)
  To: Surendrakumar Upadhyay, TejaskumarX; +Cc: intel-gfx

On Thu, Sep 02, 2021 at 11:07:06AM +0000, Surendrakumar Upadhyay, TejaskumarX wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: 01 September 2021 19:19
> > To: Surendrakumar Upadhyay, TejaskumarX
> > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock
> > gating in HDR mode
> > 
> > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote:
> > > Display underrun in HDR mode when cursor is enabled.
> > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h.
> > > As per W/A 1604331009, Disable cursor clock gating in HDR mode.
> > >
> > > Bspec : 33451
> > >
> > > Changes since V3:
> > > 	- Disable WA when not in HDR mode or cursor plane not active - Ville
> > > 	- Extract required args from crtc_state - Ville
> > > 	- Create HDR mode API using bdw_set_pipemisc ref - Ville
> > > 	- Tested with HDR video as well full setmode, WA applies and
> > disables
> > > Changes since V2:
> > >         - Made it general gen11 WA
> > >         - Removed WA needed check
> > >         - Added cursor plane active check
> > >         - Once WA enable, software will not disable Changes since V1:
> > >         - Modified way CLKGATE_DIS_PSL bit 28 was modified
> > >
> > > Cc: Souza Jose <jose.souza@intel.com>
> > > Signed-off-by: Tejas Upadhyay
> > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_display.c | 27 ++++++++++++++++++++
> > >  drivers/gpu/drm/i915/i915_reg.h              |  5 ++++
> > >  2 files changed, 32 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 6be1b31af07b..e1ea03b918df 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct
> > drm_i915_private *dev_priv)
> > >  		dev_priv->czclk_freq);
> > >  }
> > >
> > > +static bool
> > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) {
> > > +	return (crtc_state->active_planes & ~(icl_hdr_plane_mask() |
> > > +		BIT(PLANE_CURSOR))) == 0;
> > > +}
> > 
> > Please use this in bdw_set_pipemisc() as well. This could be a separate prep
> > patch actually.
> > 
> > > +
> > >  /* WA Display #0827: Gen9:all */
> > >  static void
> > >  skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe, bool
> > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct
> > drm_i915_private *dev_priv, enum pipe pipe,
> > >  		               intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) &
> > > ~DPFR_GATING_DIS);  }
> > >
> > > +/* Wa_1604331009:icl,jsl,ehl */
> > > +	static void
> > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) {
> > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > +
> > > +	if (is_hdr_mode(crtc_state) &&
> > > +	    crtc_state->active_planes & BIT(PLANE_CURSOR) &&
> > > +	    IS_GEN(dev_priv, 11))
> > > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe),
> > > +			     CURSOR_GATING_DIS, CURSOR_GATING_DIS);
> > > +	else
> > > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe),
> > > +			     CURSOR_GATING_DIS, 0);
> > > +}
> > > +
> > >  static bool
> > >  is_trans_port_sync_slave(const struct intel_crtc_state *crtc_state)
> > > { @@ -2939,6 +2963,9 @@ static void intel_pre_plane_update(struct
> > > intel_atomic_state *state,
> > >  	    needs_scalerclk_wa(new_crtc_state))
> > >  		icl_wa_scalerclkgating(dev_priv, pipe, true);
> > >
> > > +	/* Wa_1604331009:icl,jsl,ehl */
> > > +	icl_wa_cursorclkgating(new_crtc_state);
> > 
> > This looks a bit wrong. We shouldn't turn the clock gating back on until after
> > HDR mode has been disabled.
> > 
> > So please model this after skl_wa_827() and icl_wa_scalerclkgating() so that
> > a) the ordering is correct, and b) the code between all three w/as looks
> > consistent.
> 
> I did not get what you are suggesting here. Can you please put psudo? Currently as far as I see icl_wa_cursorclkgating is already modelled  after skl_wa_827() and icl_wa_scalerclkgating(). Are referring same 
> Or something else?

It should look something like:

intel_pre_plane_update()
{
	if (!needs_cursorclk_wa(old_crtc_state) &&
	    needs_cursorclk_wa(new_crtc_state))
		icl_wa_cursorclkgating(..., true);
}

intel_post_plane_update()
{
	if (needs_cursorclk_wa(old_crtc_state) &&
	    !needs_cursorclk_wa(new_crtc_state))
		icl_wa_cursorclkgating(..., false);
}


-- 
Ville Syrjälä
Intel

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

* Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode
  2021-09-02 12:59     ` Ville Syrjälä
@ 2021-09-02 13:04       ` Surendrakumar Upadhyay, TejaskumarX
  2021-09-02 13:07         ` Surendrakumar Upadhyay, TejaskumarX
  0 siblings, 1 reply; 11+ messages in thread
From: Surendrakumar Upadhyay, TejaskumarX @ 2021-09-02 13:04 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Souza, Jose



> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: 02 September 2021 18:29
> To: Surendrakumar Upadhyay, TejaskumarX
> <tejaskumarx.surendrakumar.upadhyay@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock
> gating in HDR mode
> 
> On Thu, Sep 02, 2021 at 11:07:06AM +0000, Surendrakumar Upadhyay,
> TejaskumarX wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Sent: 01 September 2021 19:19
> > > To: Surendrakumar Upadhyay, TejaskumarX
> > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor
> > > clock gating in HDR mode
> > >
> > > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote:
> > > > Display underrun in HDR mode when cursor is enabled.
> > > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h.
> > > > As per W/A 1604331009, Disable cursor clock gating in HDR mode.
> > > >
> > > > Bspec : 33451
> > > >
> > > > Changes since V3:
> > > > 	- Disable WA when not in HDR mode or cursor plane not active - Ville
> > > > 	- Extract required args from crtc_state - Ville
> > > > 	- Create HDR mode API using bdw_set_pipemisc ref - Ville
> > > > 	- Tested with HDR video as well full setmode, WA applies and
> > > disables
> > > > Changes since V2:
> > > >         - Made it general gen11 WA
> > > >         - Removed WA needed check
> > > >         - Added cursor plane active check
> > > >         - Once WA enable, software will not disable Changes since V1:
> > > >         - Modified way CLKGATE_DIS_PSL bit 28 was modified
> > > >
> > > > Cc: Souza Jose <jose.souza@intel.com>
> > > > Signed-off-by: Tejas Upadhyay
> > > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_display.c | 27
> ++++++++++++++++++++
> > > >  drivers/gpu/drm/i915/i915_reg.h              |  5 ++++
> > > >  2 files changed, 32 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 6be1b31af07b..e1ea03b918df 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct
> > > drm_i915_private *dev_priv)
> > > >  		dev_priv->czclk_freq);
> > > >  }
> > > >
> > > > +static bool
> > > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) {
> > > > +	return (crtc_state->active_planes & ~(icl_hdr_plane_mask() |
> > > > +		BIT(PLANE_CURSOR))) == 0;
> > > > +}
> > >
> > > Please use this in bdw_set_pipemisc() as well. This could be a
> > > separate prep patch actually.
> > >
> > > > +
> > > >  /* WA Display #0827: Gen9:all */
> > > >  static void
> > > >  skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe,
> > > > bool
> > > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct
> > > drm_i915_private *dev_priv, enum pipe pipe,
> > > >  		               intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe)) &
> > > > ~DPFR_GATING_DIS);  }
> > > >
> > > > +/* Wa_1604331009:icl,jsl,ehl */
> > > > +	static void
> > > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) {
> > > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > > +
> > > > +	if (is_hdr_mode(crtc_state) &&
> > > > +	    crtc_state->active_planes & BIT(PLANE_CURSOR) &&
> > > > +	    IS_GEN(dev_priv, 11))
> > > > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe),
> > > > +			     CURSOR_GATING_DIS, CURSOR_GATING_DIS);
> > > > +	else
> > > > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc->pipe),
> > > > +			     CURSOR_GATING_DIS, 0);
> > > > +}
> > > > +
> > > >  static bool
> > > >  is_trans_port_sync_slave(const struct intel_crtc_state
> > > > *crtc_state) { @@ -2939,6 +2963,9 @@ static void
> > > > intel_pre_plane_update(struct intel_atomic_state *state,
> > > >  	    needs_scalerclk_wa(new_crtc_state))
> > > >  		icl_wa_scalerclkgating(dev_priv, pipe, true);
> > > >
> > > > +	/* Wa_1604331009:icl,jsl,ehl */
> > > > +	icl_wa_cursorclkgating(new_crtc_state);
> > >
> > > This looks a bit wrong. We shouldn't turn the clock gating back on
> > > until after HDR mode has been disabled.
> > >
> > > So please model this after skl_wa_827() and icl_wa_scalerclkgating()
> > > so that
> > > a) the ordering is correct, and b) the code between all three w/as
> > > looks consistent.
> >
> > I did not get what you are suggesting here. Can you please put psudo?
> > Currently as far as I see icl_wa_cursorclkgating is already modelled  after
> skl_wa_827() and icl_wa_scalerclkgating(). Are referring same Or something
> else?
> 
> It should look something like:
> 
> intel_pre_plane_update()
> {
> 	if (!needs_cursorclk_wa(old_crtc_state) &&
> 	    needs_cursorclk_wa(new_crtc_state))
> 		icl_wa_cursorclkgating(..., true);
> }
> 
> intel_post_plane_update()
> {
> 	if (needs_cursorclk_wa(old_crtc_state) &&
> 	    !needs_cursorclk_wa(new_crtc_state))
> 		icl_wa_cursorclkgating(..., false);
> }
> 

Tejas : In the previous version it was done this way only. But after review comments from Jose I had to change it. See "Changes since V2" section. Also you can check in previous versions of patch.

> 
> --
> Ville Syrjälä
> Intel

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

* Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode
  2021-09-02 13:04       ` Surendrakumar Upadhyay, TejaskumarX
@ 2021-09-02 13:07         ` Surendrakumar Upadhyay, TejaskumarX
  2021-09-06  6:11           ` Surendrakumar Upadhyay, TejaskumarX
  0 siblings, 1 reply; 11+ messages in thread
From: Surendrakumar Upadhyay, TejaskumarX @ 2021-09-02 13:07 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Souza, Jose, Pandey, Hariom

+ Hariom

> -----Original Message-----
> From: Surendrakumar Upadhyay, TejaskumarX
> Sent: 02 September 2021 18:34
> To: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Souza, Jose <jose.souza@intel.com>
> Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock
> gating in HDR mode
> 
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: 02 September 2021 18:29
> > To: Surendrakumar Upadhyay, TejaskumarX
> > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor
> > clock gating in HDR mode
> >
> > On Thu, Sep 02, 2021 at 11:07:06AM +0000, Surendrakumar Upadhyay,
> > TejaskumarX wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Sent: 01 September 2021 19:19
> > > > To: Surendrakumar Upadhyay, TejaskumarX
> > > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > > Cc: intel-gfx@lists.freedesktop.org
> > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor
> > > > clock gating in HDR mode
> > > >
> > > > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote:
> > > > > Display underrun in HDR mode when cursor is enabled.
> > > > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h.
> > > > > As per W/A 1604331009, Disable cursor clock gating in HDR mode.
> > > > >
> > > > > Bspec : 33451
> > > > >
> > > > > Changes since V3:
> > > > > 	- Disable WA when not in HDR mode or cursor plane not active - Ville
> > > > > 	- Extract required args from crtc_state - Ville
> > > > > 	- Create HDR mode API using bdw_set_pipemisc ref - Ville
> > > > > 	- Tested with HDR video as well full setmode, WA applies and
> > > > disables
> > > > > Changes since V2:
> > > > >         - Made it general gen11 WA
> > > > >         - Removed WA needed check
> > > > >         - Added cursor plane active check
> > > > >         - Once WA enable, software will not disable Changes since V1:
> > > > >         - Modified way CLKGATE_DIS_PSL bit 28 was modified
> > > > >
> > > > > Cc: Souza Jose <jose.souza@intel.com>
> > > > > Signed-off-by: Tejas Upadhyay
> > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/display/intel_display.c | 27
> > ++++++++++++++++++++
> > > > >  drivers/gpu/drm/i915/i915_reg.h              |  5 ++++
> > > > >  2 files changed, 32 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > index 6be1b31af07b..e1ea03b918df 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct
> > > > drm_i915_private *dev_priv)
> > > > >  		dev_priv->czclk_freq);
> > > > >  }
> > > > >
> > > > > +static bool
> > > > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) {
> > > > > +	return (crtc_state->active_planes & ~(icl_hdr_plane_mask() |
> > > > > +		BIT(PLANE_CURSOR))) == 0;
> > > > > +}
> > > >
> > > > Please use this in bdw_set_pipemisc() as well. This could be a
> > > > separate prep patch actually.
> > > >
> > > > > +
> > > > >  /* WA Display #0827: Gen9:all */  static void
> > > > > skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe,
> > > > > bool
> > > > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct
> > > > drm_i915_private *dev_priv, enum pipe pipe,
> > > > >  		               intel_de_read(dev_priv, CLKGATE_DIS_PSL(pipe))
> > > > > & ~DPFR_GATING_DIS);  }
> > > > >
> > > > > +/* Wa_1604331009:icl,jsl,ehl */
> > > > > +	static void
> > > > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) {
> > > > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > > > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > > > +
> > > > > +	if (is_hdr_mode(crtc_state) &&
> > > > > +	    crtc_state->active_planes & BIT(PLANE_CURSOR) &&
> > > > > +	    IS_GEN(dev_priv, 11))
> > > > > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc-
> >pipe),
> > > > > +			     CURSOR_GATING_DIS,
> CURSOR_GATING_DIS);
> > > > > +	else
> > > > > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc-
> >pipe),
> > > > > +			     CURSOR_GATING_DIS, 0);
> > > > > +}
> > > > > +
> > > > >  static bool
> > > > >  is_trans_port_sync_slave(const struct intel_crtc_state
> > > > > *crtc_state) { @@ -2939,6 +2963,9 @@ static void
> > > > > intel_pre_plane_update(struct intel_atomic_state *state,
> > > > >  	    needs_scalerclk_wa(new_crtc_state))
> > > > >  		icl_wa_scalerclkgating(dev_priv, pipe, true);
> > > > >
> > > > > +	/* Wa_1604331009:icl,jsl,ehl */
> > > > > +	icl_wa_cursorclkgating(new_crtc_state);
> > > >
> > > > This looks a bit wrong. We shouldn't turn the clock gating back on
> > > > until after HDR mode has been disabled.
> > > >
> > > > So please model this after skl_wa_827() and
> > > > icl_wa_scalerclkgating() so that
> > > > a) the ordering is correct, and b) the code between all three w/as
> > > > looks consistent.
> > >
> > > I did not get what you are suggesting here. Can you please put psudo?
> > > Currently as far as I see icl_wa_cursorclkgating is already modelled
> > > after
> > skl_wa_827() and icl_wa_scalerclkgating(). Are referring same Or
> > something else?
> >
> > It should look something like:
> >
> > intel_pre_plane_update()
> > {
> > 	if (!needs_cursorclk_wa(old_crtc_state) &&
> > 	    needs_cursorclk_wa(new_crtc_state))
> > 		icl_wa_cursorclkgating(..., true);
> > }
> >
> > intel_post_plane_update()
> > {
> > 	if (needs_cursorclk_wa(old_crtc_state) &&
> > 	    !needs_cursorclk_wa(new_crtc_state))
> > 		icl_wa_cursorclkgating(..., false);
> > }
> >
> 
> Tejas : In the previous version it was done this way only. But after review
> comments from Jose I had to change it. See "Changes since V2" section. Also
> you can check in previous versions of patch.
> 
> >
> > --
> > Ville Syrjälä
> > Intel

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

* Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode
  2021-09-02 13:07         ` Surendrakumar Upadhyay, TejaskumarX
@ 2021-09-06  6:11           ` Surendrakumar Upadhyay, TejaskumarX
  2021-09-07 16:11             ` Surendrakumar Upadhyay, TejaskumarX
  0 siblings, 1 reply; 11+ messages in thread
From: Surendrakumar Upadhyay, TejaskumarX @ 2021-09-06  6:11 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Souza, Jose, Pandey, Hariom

Hi Ville/Jose,

I hope you both discussed as in next version I will incorporate all changes as per Ville's final suggestion. Please let me know if you guys think otherwise.

Thanks,
Tejas

> -----Original Message-----
> From: Surendrakumar Upadhyay, TejaskumarX
> Sent: 02 September 2021 18:37
> To: 'Ville Syrjälä' <ville.syrjala@linux.intel.com>
> Cc: 'intel-gfx@lists.freedesktop.org' <intel-gfx@lists.freedesktop.org>; Souza,
> Jose <jose.souza@intel.com>; Pandey, Hariom <hariom.pandey@intel.com>
> Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock
> gating in HDR mode
> 
> + Hariom
> 
> > -----Original Message-----
> > From: Surendrakumar Upadhyay, TejaskumarX
> > Sent: 02 September 2021 18:34
> > To: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; Souza, Jose
> > <jose.souza@intel.com>
> > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor
> > clock gating in HDR mode
> >
> >
> >
> > > -----Original Message-----
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Sent: 02 September 2021 18:29
> > > To: Surendrakumar Upadhyay, TejaskumarX
> > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor
> > > clock gating in HDR mode
> > >
> > > On Thu, Sep 02, 2021 at 11:07:06AM +0000, Surendrakumar Upadhyay,
> > > TejaskumarX wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Sent: 01 September 2021 19:19
> > > > > To: Surendrakumar Upadhyay, TejaskumarX
> > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > > > Cc: intel-gfx@lists.freedesktop.org
> > > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable
> > > > > cursor clock gating in HDR mode
> > > > >
> > > > > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote:
> > > > > > Display underrun in HDR mode when cursor is enabled.
> > > > > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h.
> > > > > > As per W/A 1604331009, Disable cursor clock gating in HDR mode.
> > > > > >
> > > > > > Bspec : 33451
> > > > > >
> > > > > > Changes since V3:
> > > > > > 	- Disable WA when not in HDR mode or cursor plane not
> active - Ville
> > > > > > 	- Extract required args from crtc_state - Ville
> > > > > > 	- Create HDR mode API using bdw_set_pipemisc ref - Ville
> > > > > > 	- Tested with HDR video as well full setmode, WA applies and
> > > > > disables
> > > > > > Changes since V2:
> > > > > >         - Made it general gen11 WA
> > > > > >         - Removed WA needed check
> > > > > >         - Added cursor plane active check
> > > > > >         - Once WA enable, software will not disable Changes since V1:
> > > > > >         - Modified way CLKGATE_DIS_PSL bit 28 was modified
> > > > > >
> > > > > > Cc: Souza Jose <jose.souza@intel.com>
> > > > > > Signed-off-by: Tejas Upadhyay
> > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/display/intel_display.c | 27
> > > ++++++++++++++++++++
> > > > > >  drivers/gpu/drm/i915/i915_reg.h              |  5 ++++
> > > > > >  2 files changed, 32 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > index 6be1b31af07b..e1ea03b918df 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct
> > > > > drm_i915_private *dev_priv)
> > > > > >  		dev_priv->czclk_freq);
> > > > > >  }
> > > > > >
> > > > > > +static bool
> > > > > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) {
> > > > > > +	return (crtc_state->active_planes & ~(icl_hdr_plane_mask() |
> > > > > > +		BIT(PLANE_CURSOR))) == 0;
> > > > > > +}
> > > > >
> > > > > Please use this in bdw_set_pipemisc() as well. This could be a
> > > > > separate prep patch actually.
> > > > >
> > > > > > +
> > > > > >  /* WA Display #0827: Gen9:all */  static void
> > > > > > skl_wa_827(struct drm_i915_private *dev_priv, enum pipe pipe,
> > > > > > bool
> > > > > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct
> > > > > drm_i915_private *dev_priv, enum pipe pipe,
> > > > > >  		               intel_de_read(dev_priv,
> > > > > > CLKGATE_DIS_PSL(pipe)) & ~DPFR_GATING_DIS);  }
> > > > > >
> > > > > > +/* Wa_1604331009:icl,jsl,ehl */
> > > > > > +	static void
> > > > > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) {
> > > > > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > > > > +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > > > > +
> > > > > > +	if (is_hdr_mode(crtc_state) &&
> > > > > > +	    crtc_state->active_planes & BIT(PLANE_CURSOR) &&
> > > > > > +	    IS_GEN(dev_priv, 11))
> > > > > > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc-
> > >pipe),
> > > > > > +			     CURSOR_GATING_DIS,
> > CURSOR_GATING_DIS);
> > > > > > +	else
> > > > > > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc-
> > >pipe),
> > > > > > +			     CURSOR_GATING_DIS, 0); }
> > > > > > +
> > > > > >  static bool
> > > > > >  is_trans_port_sync_slave(const struct intel_crtc_state
> > > > > > *crtc_state) { @@ -2939,6 +2963,9 @@ static void
> > > > > > intel_pre_plane_update(struct intel_atomic_state *state,
> > > > > >  	    needs_scalerclk_wa(new_crtc_state))
> > > > > >  		icl_wa_scalerclkgating(dev_priv, pipe, true);
> > > > > >
> > > > > > +	/* Wa_1604331009:icl,jsl,ehl */
> > > > > > +	icl_wa_cursorclkgating(new_crtc_state);
> > > > >
> > > > > This looks a bit wrong. We shouldn't turn the clock gating back
> > > > > on until after HDR mode has been disabled.
> > > > >
> > > > > So please model this after skl_wa_827() and
> > > > > icl_wa_scalerclkgating() so that
> > > > > a) the ordering is correct, and b) the code between all three
> > > > > w/as looks consistent.
> > > >
> > > > I did not get what you are suggesting here. Can you please put psudo?
> > > > Currently as far as I see icl_wa_cursorclkgating is already
> > > > modelled after
> > > skl_wa_827() and icl_wa_scalerclkgating(). Are referring same Or
> > > something else?
> > >
> > > It should look something like:
> > >
> > > intel_pre_plane_update()
> > > {
> > > 	if (!needs_cursorclk_wa(old_crtc_state) &&
> > > 	    needs_cursorclk_wa(new_crtc_state))
> > > 		icl_wa_cursorclkgating(..., true); }
> > >
> > > intel_post_plane_update()
> > > {
> > > 	if (needs_cursorclk_wa(old_crtc_state) &&
> > > 	    !needs_cursorclk_wa(new_crtc_state))
> > > 		icl_wa_cursorclkgating(..., false); }
> > >
> >
> > Tejas : In the previous version it was done this way only. But after
> > review comments from Jose I had to change it. See "Changes since V2"
> > section. Also you can check in previous versions of patch.
> >
> > >
> > > --
> > > Ville Syrjälä
> > > Intel

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

* Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode
  2021-09-06  6:11           ` Surendrakumar Upadhyay, TejaskumarX
@ 2021-09-07 16:11             ` Surendrakumar Upadhyay, TejaskumarX
  2021-09-22 10:49               ` Surendrakumar Upadhyay, TejaskumarX
  0 siblings, 1 reply; 11+ messages in thread
From: Surendrakumar Upadhyay, TejaskumarX @ 2021-09-07 16:11 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Souza, Jose, Pandey, Hariom

Hi Ville,

I have posted single patch for HDR mode here https://patchwork.freedesktop.org/series/94428/#rev1 . Please review and ack. I will post " drm/i915/gen11: Disable cursor clock gating in HDR mode " on top of that patch.

Thanks,
Tejas

> -----Original Message-----
> From: Surendrakumar Upadhyay, TejaskumarX
> Sent: 06 September 2021 11:41
> To: 'Ville Syrjälä' <ville.syrjala@linux.intel.com>
> Cc: 'intel-gfx@lists.freedesktop.org' <intel-gfx@lists.freedesktop.org>; Souza,
> Jose <jose.souza@intel.com>; Pandey, Hariom <hariom.pandey@intel.com>
> Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock
> gating in HDR mode
> 
> Hi Ville/Jose,
> 
> I hope you both discussed as in next version I will incorporate all changes as
> per Ville's final suggestion. Please let me know if you guys think otherwise.
> 
> Thanks,
> Tejas
> 
> > -----Original Message-----
> > From: Surendrakumar Upadhyay, TejaskumarX
> > Sent: 02 September 2021 18:37
> > To: 'Ville Syrjälä' <ville.syrjala@linux.intel.com>
> > Cc: 'intel-gfx@lists.freedesktop.org'
> > <intel-gfx@lists.freedesktop.org>; Souza, Jose <jose.souza@intel.com>;
> > Pandey, Hariom <hariom.pandey@intel.com>
> > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor
> > clock gating in HDR mode
> >
> > + Hariom
> >
> > > -----Original Message-----
> > > From: Surendrakumar Upadhyay, TejaskumarX
> > > Sent: 02 September 2021 18:34
> > > To: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: intel-gfx@lists.freedesktop.org; Souza, Jose
> > > <jose.souza@intel.com>
> > > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor
> > > clock gating in HDR mode
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Sent: 02 September 2021 18:29
> > > > To: Surendrakumar Upadhyay, TejaskumarX
> > > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > > Cc: intel-gfx@lists.freedesktop.org
> > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor
> > > > clock gating in HDR mode
> > > >
> > > > On Thu, Sep 02, 2021 at 11:07:06AM +0000, Surendrakumar Upadhyay,
> > > > TejaskumarX wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > Sent: 01 September 2021 19:19
> > > > > > To: Surendrakumar Upadhyay, TejaskumarX
> > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > > > > Cc: intel-gfx@lists.freedesktop.org
> > > > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable
> > > > > > cursor clock gating in HDR mode
> > > > > >
> > > > > > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote:
> > > > > > > Display underrun in HDR mode when cursor is enabled.
> > > > > > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h.
> > > > > > > As per W/A 1604331009, Disable cursor clock gating in HDR mode.
> > > > > > >
> > > > > > > Bspec : 33451
> > > > > > >
> > > > > > > Changes since V3:
> > > > > > > 	- Disable WA when not in HDR mode or cursor plane not
> > active - Ville
> > > > > > > 	- Extract required args from crtc_state - Ville
> > > > > > > 	- Create HDR mode API using bdw_set_pipemisc ref - Ville
> > > > > > > 	- Tested with HDR video as well full setmode, WA applies
> > > > > > > and
> > > > > > disables
> > > > > > > Changes since V2:
> > > > > > >         - Made it general gen11 WA
> > > > > > >         - Removed WA needed check
> > > > > > >         - Added cursor plane active check
> > > > > > >         - Once WA enable, software will not disable Changes since V1:
> > > > > > >         - Modified way CLKGATE_DIS_PSL bit 28 was modified
> > > > > > >
> > > > > > > Cc: Souza Jose <jose.souza@intel.com>
> > > > > > > Signed-off-by: Tejas Upadhyay
> > > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/i915/display/intel_display.c | 27
> > > > ++++++++++++++++++++
> > > > > > >  drivers/gpu/drm/i915/i915_reg.h              |  5 ++++
> > > > > > >  2 files changed, 32 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > index 6be1b31af07b..e1ea03b918df 100644
> > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct
> > > > > > drm_i915_private *dev_priv)
> > > > > > >  		dev_priv->czclk_freq);
> > > > > > >  }
> > > > > > >
> > > > > > > +static bool
> > > > > > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) {
> > > > > > > +	return (crtc_state->active_planes & ~(icl_hdr_plane_mask() |
> > > > > > > +		BIT(PLANE_CURSOR))) == 0; }
> > > > > >
> > > > > > Please use this in bdw_set_pipemisc() as well. This could be a
> > > > > > separate prep patch actually.
> > > > > >
> > > > > > > +
> > > > > > >  /* WA Display #0827: Gen9:all */  static void
> > > > > > > skl_wa_827(struct drm_i915_private *dev_priv, enum pipe
> > > > > > > pipe, bool
> > > > > > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct
> > > > > > drm_i915_private *dev_priv, enum pipe pipe,
> > > > > > >  		               intel_de_read(dev_priv,
> > > > > > > CLKGATE_DIS_PSL(pipe)) & ~DPFR_GATING_DIS);  }
> > > > > > >
> > > > > > > +/* Wa_1604331009:icl,jsl,ehl */
> > > > > > > +	static void
> > > > > > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state) {
> > > > > > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > > > > > +	struct drm_i915_private *dev_priv =
> > > > > > > +to_i915(crtc->base.dev);
> > > > > > > +
> > > > > > > +	if (is_hdr_mode(crtc_state) &&
> > > > > > > +	    crtc_state->active_planes & BIT(PLANE_CURSOR) &&
> > > > > > > +	    IS_GEN(dev_priv, 11))
> > > > > > > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc-
> > > >pipe),
> > > > > > > +			     CURSOR_GATING_DIS,
> > > CURSOR_GATING_DIS);
> > > > > > > +	else
> > > > > > > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc-
> > > >pipe),
> > > > > > > +			     CURSOR_GATING_DIS, 0); }
> > > > > > > +
> > > > > > >  static bool
> > > > > > >  is_trans_port_sync_slave(const struct intel_crtc_state
> > > > > > > *crtc_state) { @@ -2939,6 +2963,9 @@ static void
> > > > > > > intel_pre_plane_update(struct intel_atomic_state *state,
> > > > > > >  	    needs_scalerclk_wa(new_crtc_state))
> > > > > > >  		icl_wa_scalerclkgating(dev_priv, pipe, true);
> > > > > > >
> > > > > > > +	/* Wa_1604331009:icl,jsl,ehl */
> > > > > > > +	icl_wa_cursorclkgating(new_crtc_state);
> > > > > >
> > > > > > This looks a bit wrong. We shouldn't turn the clock gating
> > > > > > back on until after HDR mode has been disabled.
> > > > > >
> > > > > > So please model this after skl_wa_827() and
> > > > > > icl_wa_scalerclkgating() so that
> > > > > > a) the ordering is correct, and b) the code between all three
> > > > > > w/as looks consistent.
> > > > >
> > > > > I did not get what you are suggesting here. Can you please put psudo?
> > > > > Currently as far as I see icl_wa_cursorclkgating is already
> > > > > modelled after
> > > > skl_wa_827() and icl_wa_scalerclkgating(). Are referring same Or
> > > > something else?
> > > >
> > > > It should look something like:
> > > >
> > > > intel_pre_plane_update()
> > > > {
> > > > 	if (!needs_cursorclk_wa(old_crtc_state) &&
> > > > 	    needs_cursorclk_wa(new_crtc_state))
> > > > 		icl_wa_cursorclkgating(..., true); }
> > > >
> > > > intel_post_plane_update()
> > > > {
> > > > 	if (needs_cursorclk_wa(old_crtc_state) &&
> > > > 	    !needs_cursorclk_wa(new_crtc_state))
> > > > 		icl_wa_cursorclkgating(..., false); }
> > > >
> > >
> > > Tejas : In the previous version it was done this way only. But after
> > > review comments from Jose I had to change it. See "Changes since V2"
> > > section. Also you can check in previous versions of patch.
> > >
> > > >
> > > > --
> > > > Ville Syrjälä
> > > > Intel

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

* Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode
  2021-09-07 16:11             ` Surendrakumar Upadhyay, TejaskumarX
@ 2021-09-22 10:49               ` Surendrakumar Upadhyay, TejaskumarX
  0 siblings, 0 replies; 11+ messages in thread
From: Surendrakumar Upadhyay, TejaskumarX @ 2021-09-22 10:49 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Souza, Jose, Pandey, Hariom

Hi Ville,

Can you please help with reviewing below patch?

Thanks,
Tejas
> -----Original Message-----
> From: Surendrakumar Upadhyay, TejaskumarX
> Sent: 07 September 2021 21:42
> To: 'Ville Syrjälä' <ville.syrjala@linux.intel.com>
> Cc: 'intel-gfx@lists.freedesktop.org' <intel-gfx@lists.freedesktop.org>; Souza,
> Jose <jose.souza@intel.com>; Pandey, Hariom <hariom.pandey@intel.com>
> Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock
> gating in HDR mode
> 
> Hi Ville,
> 
> I have posted single patch for HDR mode here
> https://patchwork.freedesktop.org/series/94428/#rev1 . Please review and
> ack. I will post " drm/i915/gen11: Disable cursor clock gating in HDR mode "
> on top of that patch.
> 
> Thanks,
> Tejas
> 
> > -----Original Message-----
> > From: Surendrakumar Upadhyay, TejaskumarX
> > Sent: 06 September 2021 11:41
> > To: 'Ville Syrjälä' <ville.syrjala@linux.intel.com>
> > Cc: 'intel-gfx@lists.freedesktop.org'
> > <intel-gfx@lists.freedesktop.org>; Souza, Jose <jose.souza@intel.com>;
> > Pandey, Hariom <hariom.pandey@intel.com>
> > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor
> > clock gating in HDR mode
> >
> > Hi Ville/Jose,
> >
> > I hope you both discussed as in next version I will incorporate all
> > changes as per Ville's final suggestion. Please let me know if you guys think
> otherwise.
> >
> > Thanks,
> > Tejas
> >
> > > -----Original Message-----
> > > From: Surendrakumar Upadhyay, TejaskumarX
> > > Sent: 02 September 2021 18:37
> > > To: 'Ville Syrjälä' <ville.syrjala@linux.intel.com>
> > > Cc: 'intel-gfx@lists.freedesktop.org'
> > > <intel-gfx@lists.freedesktop.org>; Souza, Jose
> > > <jose.souza@intel.com>; Pandey, Hariom <hariom.pandey@intel.com>
> > > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor
> > > clock gating in HDR mode
> > >
> > > + Hariom
> > >
> > > > -----Original Message-----
> > > > From: Surendrakumar Upadhyay, TejaskumarX
> > > > Sent: 02 September 2021 18:34
> > > > To: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: intel-gfx@lists.freedesktop.org; Souza, Jose
> > > > <jose.souza@intel.com>
> > > > Subject: RE: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor
> > > > clock gating in HDR mode
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Sent: 02 September 2021 18:29
> > > > > To: Surendrakumar Upadhyay, TejaskumarX
> > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > > > Cc: intel-gfx@lists.freedesktop.org
> > > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable
> > > > > cursor clock gating in HDR mode
> > > > >
> > > > > On Thu, Sep 02, 2021 at 11:07:06AM +0000, Surendrakumar
> > > > > Upadhyay, TejaskumarX wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > > Sent: 01 September 2021 19:19
> > > > > > > To: Surendrakumar Upadhyay, TejaskumarX
> > > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > > > > > Cc: intel-gfx@lists.freedesktop.org
> > > > > > > Subject: Re: [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable
> > > > > > > cursor clock gating in HDR mode
> > > > > > >
> > > > > > > On Tue, Jun 22, 2021 at 03:04:24PM +0530, Tejas Upadhyay wrote:
> > > > > > > > Display underrun in HDR mode when cursor is enabled.
> > > > > > > > RTL fix will be implemented CLKGATE_DIS_PSL_A bit 28-46520h.
> > > > > > > > As per W/A 1604331009, Disable cursor clock gating in HDR
> mode.
> > > > > > > >
> > > > > > > > Bspec : 33451
> > > > > > > >
> > > > > > > > Changes since V3:
> > > > > > > > 	- Disable WA when not in HDR mode or cursor plane not
> > > active - Ville
> > > > > > > > 	- Extract required args from crtc_state - Ville
> > > > > > > > 	- Create HDR mode API using bdw_set_pipemisc ref - Ville
> > > > > > > > 	- Tested with HDR video as well full setmode, WA applies
> > > > > > > > and
> > > > > > > disables
> > > > > > > > Changes since V2:
> > > > > > > >         - Made it general gen11 WA
> > > > > > > >         - Removed WA needed check
> > > > > > > >         - Added cursor plane active check
> > > > > > > >         - Once WA enable, software will not disable Changes since
> V1:
> > > > > > > >         - Modified way CLKGATE_DIS_PSL bit 28 was modified
> > > > > > > >
> > > > > > > > Cc: Souza Jose <jose.souza@intel.com>
> > > > > > > > Signed-off-by: Tejas Upadhyay
> > > > > > > > <tejaskumarx.surendrakumar.upadhyay@intel.com>
> > > > > > > > ---
> > > > > > > >  drivers/gpu/drm/i915/display/intel_display.c | 27
> > > > > ++++++++++++++++++++
> > > > > > > >  drivers/gpu/drm/i915/i915_reg.h              |  5 ++++
> > > > > > > >  2 files changed, 32 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > > index 6be1b31af07b..e1ea03b918df 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > > @@ -358,6 +358,13 @@ static void intel_update_czclk(struct
> > > > > > > drm_i915_private *dev_priv)
> > > > > > > >  		dev_priv->czclk_freq);
> > > > > > > >  }
> > > > > > > >
> > > > > > > > +static bool
> > > > > > > > +is_hdr_mode(const struct intel_crtc_state *crtc_state) {
> > > > > > > > +	return (crtc_state->active_planes & ~(icl_hdr_plane_mask() |
> > > > > > > > +		BIT(PLANE_CURSOR))) == 0; }
> > > > > > >
> > > > > > > Please use this in bdw_set_pipemisc() as well. This could be
> > > > > > > a separate prep patch actually.
> > > > > > >
> > > > > > > > +
> > > > > > > >  /* WA Display #0827: Gen9:all */  static void
> > > > > > > > skl_wa_827(struct drm_i915_private *dev_priv, enum pipe
> > > > > > > > pipe, bool
> > > > > > > > enable) @@ -383,6 +390,23 @@ icl_wa_scalerclkgating(struct
> > > > > > > drm_i915_private *dev_priv, enum pipe pipe,
> > > > > > > >  		               intel_de_read(dev_priv,
> > > > > > > > CLKGATE_DIS_PSL(pipe)) & ~DPFR_GATING_DIS);  }
> > > > > > > >
> > > > > > > > +/* Wa_1604331009:icl,jsl,ehl */
> > > > > > > > +	static void
> > > > > > > > +icl_wa_cursorclkgating(const struct intel_crtc_state *crtc_state)
> {
> > > > > > > > +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > > > > > > +	struct drm_i915_private *dev_priv =
> > > > > > > > +to_i915(crtc->base.dev);
> > > > > > > > +
> > > > > > > > +	if (is_hdr_mode(crtc_state) &&
> > > > > > > > +	    crtc_state->active_planes & BIT(PLANE_CURSOR) &&
> > > > > > > > +	    IS_GEN(dev_priv, 11))
> > > > > > > > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc-
> > > > >pipe),
> > > > > > > > +			     CURSOR_GATING_DIS,
> > > > CURSOR_GATING_DIS);
> > > > > > > > +	else
> > > > > > > > +		intel_de_rmw(dev_priv, CLKGATE_DIS_PSL(crtc-
> > > > >pipe),
> > > > > > > > +			     CURSOR_GATING_DIS, 0); }
> > > > > > > > +
> > > > > > > >  static bool
> > > > > > > >  is_trans_port_sync_slave(const struct intel_crtc_state
> > > > > > > > *crtc_state) { @@ -2939,6 +2963,9 @@ static void
> > > > > > > > intel_pre_plane_update(struct intel_atomic_state *state,
> > > > > > > >  	    needs_scalerclk_wa(new_crtc_state))
> > > > > > > >  		icl_wa_scalerclkgating(dev_priv, pipe, true);
> > > > > > > >
> > > > > > > > +	/* Wa_1604331009:icl,jsl,ehl */
> > > > > > > > +	icl_wa_cursorclkgating(new_crtc_state);
> > > > > > >
> > > > > > > This looks a bit wrong. We shouldn't turn the clock gating
> > > > > > > back on until after HDR mode has been disabled.
> > > > > > >
> > > > > > > So please model this after skl_wa_827() and
> > > > > > > icl_wa_scalerclkgating() so that
> > > > > > > a) the ordering is correct, and b) the code between all
> > > > > > > three w/as looks consistent.
> > > > > >
> > > > > > I did not get what you are suggesting here. Can you please put
> psudo?
> > > > > > Currently as far as I see icl_wa_cursorclkgating is already
> > > > > > modelled after
> > > > > skl_wa_827() and icl_wa_scalerclkgating(). Are referring same Or
> > > > > something else?
> > > > >
> > > > > It should look something like:
> > > > >
> > > > > intel_pre_plane_update()
> > > > > {
> > > > > 	if (!needs_cursorclk_wa(old_crtc_state) &&
> > > > > 	    needs_cursorclk_wa(new_crtc_state))
> > > > > 		icl_wa_cursorclkgating(..., true); }
> > > > >
> > > > > intel_post_plane_update()
> > > > > {
> > > > > 	if (needs_cursorclk_wa(old_crtc_state) &&
> > > > > 	    !needs_cursorclk_wa(new_crtc_state))
> > > > > 		icl_wa_cursorclkgating(..., false); }
> > > > >
> > > >
> > > > Tejas : In the previous version it was done this way only. But
> > > > after review comments from Jose I had to change it. See "Changes since
> V2"
> > > > section. Also you can check in previous versions of patch.
> > > >
> > > > >
> > > > > --
> > > > > Ville Syrjälä
> > > > > Intel

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

end of thread, other threads:[~2021-09-22 10:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-22  9:34 [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode Tejas Upadhyay
2021-06-22 12:44 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gen11: Disable cursor clock gating in HDR mode (rev2) Patchwork
2021-06-25  7:03 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gen11: Disable cursor clock gating in HDR mode (rev3) Patchwork
2021-09-01 13:48 ` [Intel-gfx] [PATCH V4] drm/i915/gen11: Disable cursor clock gating in HDR mode Ville Syrjälä
2021-09-02 11:07   ` Surendrakumar Upadhyay, TejaskumarX
2021-09-02 12:59     ` Ville Syrjälä
2021-09-02 13:04       ` Surendrakumar Upadhyay, TejaskumarX
2021-09-02 13:07         ` Surendrakumar Upadhyay, TejaskumarX
2021-09-06  6:11           ` Surendrakumar Upadhyay, TejaskumarX
2021-09-07 16:11             ` Surendrakumar Upadhyay, TejaskumarX
2021-09-22 10:49               ` Surendrakumar Upadhyay, TejaskumarX

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).