[PATCH 0/2] SSC enablement in port clock programming

[PATCH 0/2] SSC enablement in port clock programming

[Suraj Kandpal]

According to specs SSC is enabled during port clock programming based on
below conditions:
-if uhbr 10 or uhbr 20 enable ssc regardless of what display controller
asks of us
-if ubhr 13.5 or legacy rates enable ssc if required.
Currently these conditions are not exactly followed this patch series
fixes that.
One more thing this patch addresses is how SSC was not enabled for c20
PHY at all because ssc_enabled variable is never set for c20 PHY.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>

Suraj Kandpal (2):
  drm/i915/cx0: Fix SSC enablement in PORT_CLOCK_CTL
  drm/i915/cx0: Set ssc_enabled for c20 too

 drivers/gpu/drm/i915/display/intel_cx0_phy.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH 1/2] drm/i915/cx0: Fix SSC enablement in PORT_CLOCK_CTL

[Suraj Kandpal]

SSC for PLL_A is enabled for UHBR10 or UHBR20 regardless of the
need for SSC. This means the ssc_enabled variable had no say
to determine enablement of SSC on PLL A.

Bspec: 64568, 74165, 74489, 74491
Fixes: 237e7be0bf57 ("drm/i915/mtl: For DP2.0 10G and 20G rates use MPLLA")
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index e768dc6a15b3..3fd959a2773c 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -2747,7 +2747,7 @@ static void intel_program_port_clock_ctl(struct intel_encoder *encoder,
 	/* TODO: HDMI FRL */
 	/* DP2.0 10G and 20G rates enable MPLLA*/
 	if (crtc_state->port_clock == 1000000 || crtc_state->port_clock == 2000000)
-		val |= crtc_state->dpll_hw_state.cx0pll.ssc_enabled ? XELPDP_SSC_ENABLE_PLLA : 0;
+		val |= XELPDP_SSC_ENABLE_PLLA;
 	else
 		val |= crtc_state->dpll_hw_state.cx0pll.ssc_enabled ? XELPDP_SSC_ENABLE_PLLB : 0;
 
-- 
2.34.1

[PATCH 2/2] drm/i915/cx0: Set ssc_enabled for c20 too

[Suraj Kandpal]

ssc_enabled does not get set for c20 phy legacy native rates.
This means SSC for MPLLB for legacy rates and UHBR 13.5 is never
set. This patch makes sure we set ssc_enabled for both c10 and c20.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index 3fd959a2773c..a0ae0be77c55 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -2033,21 +2033,27 @@ intel_c10pll_tables_get(struct intel_crtc_state *crtc_state,
 	return NULL;
 }
 
-static void intel_c10pll_update_pll(struct intel_crtc_state *crtc_state,
+static void intel_cx0pll_update_ssc(struct intel_crtc_state *crtc_state,
 				    struct intel_encoder *encoder)
 {
 	struct intel_display *display = to_intel_display(encoder);
 	struct intel_cx0pll_state *pll_state = &crtc_state->dpll_hw_state.cx0pll;
-	int i;
 
 	if (intel_crtc_has_dp_encoder(crtc_state)) {
 		if (intel_panel_use_ssc(display)) {
 			struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-
 			pll_state->ssc_enabled =
 				(intel_dp->dpcd[DP_MAX_DOWNSPREAD] & DP_MAX_DOWNSPREAD_0_5);
 		}
 	}
+}
+
+static void intel_c10pll_update_pll(struct intel_crtc_state *crtc_state,
+				    struct intel_encoder *encoder)
+{
+	struct intel_display *display = to_intel_display(encoder);
+	struct intel_cx0pll_state *pll_state = &crtc_state->dpll_hw_state.cx0pll;
+	int i;
 
 	if (pll_state->ssc_enabled)
 		return;
@@ -2067,6 +2073,8 @@ static int intel_c10pll_calc_state(struct intel_crtc_state *crtc_state,
 	if (!tables)
 		return -EINVAL;
 
+	intel_cx0pll_update_ssc(crtc_state, encoder);
+
 	for (i = 0; tables[i]; i++) {
 		if (crtc_state->port_clock == tables[i]->clock) {
 			crtc_state->dpll_hw_state.cx0pll.c10 = *tables[i];
@@ -2319,6 +2327,8 @@ static int intel_c20pll_calc_state(struct intel_crtc_state *crtc_state,
 	if (!tables)
 		return -EINVAL;
 
+	intel_cx0pll_update_ssc(crtc_state, encoder);
+
 	for (i = 0; tables[i]; i++) {
 		if (crtc_state->port_clock == tables[i]->clock) {
 			crtc_state->dpll_hw_state.cx0pll.c20 = *tables[i];
-- 
2.34.1

✗ i915.CI.BAT: failure for SSC enablement in port clock programming

[Patchwork]

== Series Details ==

Series: SSC enablement in port clock programming
URL   : https://patchwork.freedesktop.org/series/143127/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_15904 -> Patchwork_143127v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_143127v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_143127v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) 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_143127v1/index.html

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

  Additional (1): fi-pnv-d510 
  Missing    (1): fi-snb-2520m 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@reload:
    - bat-arls-5:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15904/bat-arls-5/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_143127v1/bat-arls-5/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@module-reload:
    - bat-dg2-11:         [PASS][3] -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15904/bat-dg2-11/igt@i915_pm_rpm@module-reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_143127v1/bat-dg2-11/igt@i915_pm_rpm@module-reload.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@load:
    - fi-pnv-d510:        NOTRUN -> [ABORT][5] ([i915#13203])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_143127v1/fi-pnv-d510/igt@i915_module_load@load.html

  * igt@i915_module_load@reload:
    - fi-kbl-7567u:       [PASS][6] -> [DMESG-WARN][7] ([i915#11621] / [i915#180] / [i915#1982]) +1 other test dmesg-warn
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15904/fi-kbl-7567u/igt@i915_module_load@reload.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_143127v1/fi-kbl-7567u/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@module-reload:
    - bat-adls-6:         [PASS][8] -> [FAIL][9] ([i915#13401])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15904/bat-adls-6/igt@i915_pm_rpm@module-reload.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_143127v1/bat-adls-6/igt@i915_pm_rpm@module-reload.html
    - bat-dg1-7:          [PASS][10] -> [FAIL][11] ([i915#13401])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15904/bat-dg1-7/igt@i915_pm_rpm@module-reload.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_143127v1/bat-dg1-7/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@sanitycheck:
    - fi-kbl-7567u:       [PASS][12] -> [DMESG-WARN][13] ([i915#11621]) +81 other tests dmesg-warn
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15904/fi-kbl-7567u/igt@i915_selftest@live@sanitycheck.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_143127v1/fi-kbl-7567u/igt@i915_selftest@live@sanitycheck.html

  * igt@i915_selftest@live@workarounds:
    - bat-mtlp-6:         [PASS][14] -> [DMESG-FAIL][15] ([i915#13393]) +1 other test dmesg-fail
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15904/bat-mtlp-6/igt@i915_selftest@live@workarounds.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_143127v1/bat-mtlp-6/igt@i915_selftest@live@workarounds.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-dg2-11:         [PASS][16] -> [SKIP][17] ([i915#9197]) +3 other tests skip
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15904/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_143127v1/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  * igt@kms_pm_rpm@basic-pci-d3-state:
    - fi-kbl-7567u:       [PASS][18] -> [DMESG-WARN][19] ([i915#11621] / [i915#180]) +51 other tests dmesg-warn
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15904/fi-kbl-7567u/igt@kms_pm_rpm@basic-pci-d3-state.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_143127v1/fi-kbl-7567u/igt@kms_pm_rpm@basic-pci-d3-state.html

  
#### Possible fixes ####

  * igt@i915_selftest@live:
    - bat-mtlp-8:         [DMESG-FAIL][20] ([i915#13393]) -> [PASS][21] +1 other test pass
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15904/bat-mtlp-8/igt@i915_selftest@live.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_143127v1/bat-mtlp-8/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - {bat-mtlp-9}:       [DMESG-FAIL][22] ([i915#13393]) -> [PASS][23] +1 other test pass
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15904/bat-mtlp-9/igt@i915_selftest@live@workarounds.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_143127v1/bat-mtlp-9/igt@i915_selftest@live@workarounds.html

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

  [i915#11621]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11621
  [i915#13203]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13203
  [i915#13393]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13393
  [i915#13401]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13401
  [i915#180]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/180
  [i915#1982]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1982
  [i915#9197]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9197


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

  * Linux: CI_DRM_15904 -> Patchwork_143127v1

  CI-20190529: 20190529
  CI_DRM_15904: 856baaf960db044e0ce5e3f5d04d35a7b9879837 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8174: d2004b0623dbccd08502525849b4eef881aa199e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_143127v1: 856baaf960db044e0ce5e3f5d04d35a7b9879837 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

Re: [PATCH 1/2] drm/i915/cx0: Fix SSC enablement in PORT_CLOCK_CTL

[Imre Deak]

On Mon, Jan 06, 2025 at 09:38:20AM +0530, Suraj Kandpal wrote:
> SSC for PLL_A is enabled for UHBR10 or UHBR20 regardless of the
> need for SSC. This means the ssc_enabled variable had no say
> to determine enablement of SSC on PLL A.

I don't see the above in the spec. It suggests that SSC should be
enabled on PLL A for MFD, but in any case SSC can only be enabled
if the sink supports SSC, as indicated by dpll_hw_state.cx0pll.ssc_enabled.

> Bspec: 64568, 74165, 74489, 74491
> Fixes: 237e7be0bf57 ("drm/i915/mtl: For DP2.0 10G and 20G rates use MPLLA")
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index e768dc6a15b3..3fd959a2773c 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -2747,7 +2747,7 @@ static void intel_program_port_clock_ctl(struct intel_encoder *encoder,
>  	/* TODO: HDMI FRL */
>  	/* DP2.0 10G and 20G rates enable MPLLA*/
>  	if (crtc_state->port_clock == 1000000 || crtc_state->port_clock == 2000000)
> -		val |= crtc_state->dpll_hw_state.cx0pll.ssc_enabled ? XELPDP_SSC_ENABLE_PLLA : 0;
> +		val |= XELPDP_SSC_ENABLE_PLLA;
>  	else
>  		val |= crtc_state->dpll_hw_state.cx0pll.ssc_enabled ? XELPDP_SSC_ENABLE_PLLB : 0;
>  
> -- 
> 2.34.1
> 

RE: [PATCH 1/2] drm/i915/cx0: Fix SSC enablement in PORT_CLOCK_CTL

[Kandpal, Suraj]

> -----Original Message-----
> From: Deak, Imre <imre.deak@intel.com>
> Sent: Tuesday, January 7, 2025 8:33 PM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>
> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Nautiyal,
> Ankit K <ankit.k.nautiyal@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>
> Subject: Re: [PATCH 1/2] drm/i915/cx0: Fix SSC enablement in
> PORT_CLOCK_CTL
> 
> On Mon, Jan 06, 2025 at 09:38:20AM +0530, Suraj Kandpal wrote:
> > SSC for PLL_A is enabled for UHBR10 or UHBR20 regardless of the need
> > for SSC. This means the ssc_enabled variable had no say to determine
> > enablement of SSC on PLL A.
> 
> I don't see the above in the spec. It suggests that SSC should be enabled on
> PLL A for MFD, but in any case SSC can only be enabled if the sink supports
> SSC, as indicated by dpll_hw_state.cx0pll.ssc_enabled.

Hi Imre,
You are right
In Bspec 74489 under Non-thunderbolt PLL Enable Sequence
It says SSC enable to be done on PLLA  for MFD when on UHBR10 or UHBR20
(PLLA is only used for C20 PHY UHBR10 and 20.)
and check for ssc_enabled for Native mode to enable SSC but now the issue is
that we aren't checking for MFD mode any particular reason for this ? and how would we
go about checking if we are in MFD mode or not ?
Also the ssc_enabled bool variable never actually gets set for C20 Phy which makes checking the ssc_enabled
Useless, which I fix in the next patch.
Would be great if you could also have a look at that.

Regards,
Suraj Kandpal

> 
> > Bspec: 64568, 74165, 74489, 74491
> > Fixes: 237e7be0bf57 ("drm/i915/mtl: For DP2.0 10G and 20G rates use
> > MPLLA")
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > index e768dc6a15b3..3fd959a2773c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > @@ -2747,7 +2747,7 @@ static void intel_program_port_clock_ctl(struct
> intel_encoder *encoder,
> >  	/* TODO: HDMI FRL */
> >  	/* DP2.0 10G and 20G rates enable MPLLA*/
> >  	if (crtc_state->port_clock == 1000000 || crtc_state->port_clock ==
> 2000000)
> > -		val |= crtc_state->dpll_hw_state.cx0pll.ssc_enabled ?
> XELPDP_SSC_ENABLE_PLLA : 0;
> > +		val |= XELPDP_SSC_ENABLE_PLLA;
> >  	else
> >  		val |= crtc_state->dpll_hw_state.cx0pll.ssc_enabled ?
> > XELPDP_SSC_ENABLE_PLLB : 0;
> >
> > --
> > 2.34.1
> >

Re: [PATCH 1/2] drm/i915/cx0: Fix SSC enablement in PORT_CLOCK_CTL

[Imre Deak]

On Wed, Jan 08, 2025 at 08:04:58AM +0200, Kandpal, Suraj wrote:
> 
> 
> > -----Original Message-----
> > From: Deak, Imre <imre.deak@intel.com>
> > Sent: Tuesday, January 7, 2025 8:33 PM
> > To: Kandpal, Suraj <suraj.kandpal@intel.com>
> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Nautiyal,
> > Ankit K <ankit.k.nautiyal@intel.com>; Shankar, Uma
> > <uma.shankar@intel.com>
> > Subject: Re: [PATCH 1/2] drm/i915/cx0: Fix SSC enablement in
> > PORT_CLOCK_CTL
> >
> > On Mon, Jan 06, 2025 at 09:38:20AM +0530, Suraj Kandpal wrote:
> > > SSC for PLL_A is enabled for UHBR10 or UHBR20 regardless of the need
> > > for SSC. This means the ssc_enabled variable had no say to determine
> > > enablement of SSC on PLL A.
> >
> > I don't see the above in the spec. It suggests that SSC should be enabled on
> > PLL A for MFD, but in any case SSC can only be enabled if the sink supports
> > SSC, as indicated by dpll_hw_state.cx0pll.ssc_enabled.
> 
> Hi Imre,
>
> You are right In Bspec 74489 under Non-thunderbolt PLL Enable Sequence
> It says SSC enable to be done on PLLA  for MFD when on UHBR10 or
> UHBR20 (PLLA is only used for C20 PHY UHBR10 and 20.) and check for
> ssc_enabled for Native mode to enable SSC but now the issue is that we
> aren't checking for MFD mode any particular reason for this ? and how
> would we go about checking if we are in MFD mode or not ?  Also the
> ssc_enabled bool variable never actually gets set for C20 Phy which
> makes checking the ssc_enabled Useless, which I fix in the next patch.

Enabling SSC for DP outputs would require more changes than enabling SSC
in the PLL, AFAICS at least:

- Check the sink if it supports SSC.
- Check VBT if it requires SSC to be enabled.
- If enabling SSC, also enable it in the sink's spread control DPCD register.
- If enabling SSC, adjust the MST BW calculation.

In fact I'm not sure how this works atm on the DG2 SNPS and C10 PHYs,
where SSC is enabled w/o checking/handling all the above.

> Would be great if you could also have a look at that.
> 
> Regards,
> Suraj Kandpal
> 
> >
> > > Bspec: 64568, 74165, 74489, 74491
> > > Fixes: 237e7be0bf57 ("drm/i915/mtl: For DP2.0 10G and 20G rates use
> > > MPLLA")
> > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > index e768dc6a15b3..3fd959a2773c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > @@ -2747,7 +2747,7 @@ static void intel_program_port_clock_ctl(struct
> > intel_encoder *encoder,
> > >     /* TODO: HDMI FRL */
> > >     /* DP2.0 10G and 20G rates enable MPLLA*/
> > >     if (crtc_state->port_clock == 1000000 || crtc_state->port_clock ==
> > 2000000)
> > > -           val |= crtc_state->dpll_hw_state.cx0pll.ssc_enabled ?
> > XELPDP_SSC_ENABLE_PLLA : 0;
> > > +           val |= XELPDP_SSC_ENABLE_PLLA;
> > >     else
> > >             val |= crtc_state->dpll_hw_state.cx0pll.ssc_enabled ?
> > > XELPDP_SSC_ENABLE_PLLB : 0;
> > >
> > > --
> > > 2.34.1
> > >

RE: [PATCH 1/2] drm/i915/cx0: Fix SSC enablement in PORT_CLOCK_CTL

[Kandpal, Suraj]

> -----Original Message-----
> From: Deak, Imre <imre.deak@intel.com>
> Sent: Wednesday, January 8, 2025 10:35 PM
> To: Kandpal, Suraj <suraj.kandpal@intel.com>
> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Nautiyal,
> Ankit K <ankit.k.nautiyal@intel.com>; Shankar, Uma
> <uma.shankar@intel.com>
> Subject: Re: [PATCH 1/2] drm/i915/cx0: Fix SSC enablement in
> PORT_CLOCK_CTL
> 
> On Wed, Jan 08, 2025 at 08:04:58AM +0200, Kandpal, Suraj wrote:
> >
> >
> > > -----Original Message-----
> > > From: Deak, Imre <imre.deak@intel.com>
> > > Sent: Tuesday, January 7, 2025 8:33 PM
> > > To: Kandpal, Suraj <suraj.kandpal@intel.com>
> > > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org;
> > > Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Shankar, Uma
> > > <uma.shankar@intel.com>
> > > Subject: Re: [PATCH 1/2] drm/i915/cx0: Fix SSC enablement in
> > > PORT_CLOCK_CTL
> > >
> > > On Mon, Jan 06, 2025 at 09:38:20AM +0530, Suraj Kandpal wrote:
> > > > SSC for PLL_A is enabled for UHBR10 or UHBR20 regardless of the
> > > > need for SSC. This means the ssc_enabled variable had no say to
> > > > determine enablement of SSC on PLL A.
> > >
> > > I don't see the above in the spec. It suggests that SSC should be
> > > enabled on PLL A for MFD, but in any case SSC can only be enabled if
> > > the sink supports SSC, as indicated by dpll_hw_state.cx0pll.ssc_enabled.
> >
> > Hi Imre,
> >
> > You are right In Bspec 74489 under Non-thunderbolt PLL Enable Sequence
> > It says SSC enable to be done on PLLA  for MFD when on UHBR10 or
> > UHBR20 (PLLA is only used for C20 PHY UHBR10 and 20.) and check for
> > ssc_enabled for Native mode to enable SSC but now the issue is that we
> > aren't checking for MFD mode any particular reason for this ? and how
> > would we go about checking if we are in MFD mode or not ?  Also the
> > ssc_enabled bool variable never actually gets set for C20 Phy which
> > makes checking the ssc_enabled Useless, which I fix in the next patch.
> 
> Enabling SSC for DP outputs would require more changes than enabling SSC in
> the PLL, AFAICS at least:
> 
> - Check the sink if it supports SSC.
> - Check VBT if it requires SSC to be enabled.
> - If enabling SSC, also enable it in the sink's spread control DPCD register.
> - If enabling SSC, adjust the MST BW calculation.
> 
> In fact I'm not sure how this works atm on the DG2 SNPS and C10 PHYs, where
> SSC is enabled w/o checking/handling all the above.

Right now we use the below code to see if SSC needs to be enabled or not
intel_dp->dpcd[DP_MAX_DOWNSPREAD] & DP_MAX_DOWNSPREAD_0_5

Also this code section only takes care of enabling SSC from the PLL side the rest
Would need to be done in the DP code.

Regards,
Suraj Kandpal

> 
> > Would be great if you could also have a look at that.
> >
> > Regards,
> > Suraj Kandpal
> >
> > >
> > > > Bspec: 64568, 74165, 74489, 74491
> > > > Fixes: 237e7be0bf57 ("drm/i915/mtl: For DP2.0 10G and 20G rates
> > > > use
> > > > MPLLA")
> > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > > index e768dc6a15b3..3fd959a2773c 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> > > > @@ -2747,7 +2747,7 @@ static void
> > > > intel_program_port_clock_ctl(struct
> > > intel_encoder *encoder,
> > > >     /* TODO: HDMI FRL */
> > > >     /* DP2.0 10G and 20G rates enable MPLLA*/
> > > >     if (crtc_state->port_clock == 1000000 ||
> > > > crtc_state->port_clock ==
> > > 2000000)
> > > > -           val |= crtc_state->dpll_hw_state.cx0pll.ssc_enabled ?
> > > XELPDP_SSC_ENABLE_PLLA : 0;
> > > > +           val |= XELPDP_SSC_ENABLE_PLLA;
> > > >     else
> > > >             val |= crtc_state->dpll_hw_state.cx0pll.ssc_enabled ?
> > > > XELPDP_SSC_ENABLE_PLLB : 0;
> > > >
> > > > --
> > > > 2.34.1
> > > >