> -----Original Message-----
> From: Deak, Imre <imre.deak@xxxxxxxxx>
> Sent: Wednesday, January 8, 2025 10:35 PM
> To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>
> Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Nautiyal,
> Ankit K <ankit.k.nautiyal@xxxxxxxxx>; Shankar, Uma
> <uma.shankar@xxxxxxxxx>
> 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@xxxxxxxxx>
> > > Sent: Tuesday, January 7, 2025 8:33 PM
> > > To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>
> > > Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx;
> > > Nautiyal, Ankit K <ankit.k.nautiyal@xxxxxxxxx>; Shankar, Uma
> > > <uma.shankar@xxxxxxxxx>
> > > 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@xxxxxxxxx>
> > > > ---
> > > > 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
> > > >
