* [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).