[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 at intel.com>
> > Sent: Tuesday, January 7, 2025 8:33 PM
> > To: Kandpal, Suraj <suraj.kandpal at intel.com>
> > Cc: intel-xe at lists.freedesktop.org; intel-gfx at lists.freedesktop.org; Nautiyal,
> > Ankit K <ankit.k.nautiyal at intel.com>; Shankar, Uma
> > <uma.shankar at 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 at 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
> > >