Re: [PATCH 05/23] drm/xe/xe3p_lpm: Stop reading the CTC_MODE register

["Vivekanandan, Balasubramani" <balasubramani.vivekanandan@intel.com>]

On 14.10.2025 22:41, Lucas De Marchi wrote:
> On Tue, Oct 14, 2025 at 09:52:11AM -0700, Matt Roper wrote:
> > On Tue, Oct 14, 2025 at 05:28:48PM +0530, Shekhar Chauhan wrote:
> > > 
> > > On 10/14/2025 8:54, Lucas De Marchi wrote:
> > > > From: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> > > >
> > > > CTC_MODE register is removed by Xe3p.  Stop reading that register on
> > > > Xe3p and later platforms.
> > > >
> > > > Bspec: 62395
> > > 
> > > Again, maybe I'm referencing the Bspec in a wrong way, but I do see this
> > > register existing for IP 35. Maybe a possible tagging issue in the Bspec?
> > > Can someone comment here, if I'm missing something.
> > 
> > So looking a bit closer based on your comment, I think maybe this should
> > actually be a GRAPHICS_VER() test rather than a GT_VER() test.  It gets
> > a bit confusing since the bspec website still tries to print stuff out
> > in a platform-based manner, which makes it hard to follow exactly which
> > IP versions and GTs are actually relevant.  In this case the first
> > REMOVEDBY() tag listed is what documents the removal of the CTC_MODE
> > register, but it appears that tag only applies to primary GT; the media
> > GT is not included by the change referenced.  And then if you look
> > through the jumble of platform names that come after that, it does
> > indeed appear that media is still listed as having the register, but the
> > primary GT no longer appears in the list for platforms with Xe3p IP.
> > 
> > So maybe this is another case where we should be using GRAPHICS_VER
> > instead of GT_VER.  I'm not sure it really matters too much though since
> > we don't actually use the register for anything, except to complain
> > about a condition we expect to never show up.
> 
> maybe at this point we should just drop the entire thing?
> It's not supported for xe2/xe3, it's dropped for xe3p. It's supported
> for unofficially supported platforms, but we don't really know what to
> do in that case.
> 
> So I think it'd be safe to just drop the entire function and now worry
> about what is the exact gt version. Thoughts?

+1 from me to complete remove it.

Regards,
Bala

> 
> Lucas De Marchi
> 
> > 
> > 
> > Matt
> > 
> > > 
> > > -shekhar
> > > 
> > > > Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
> > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/xe/xe_gt_clock.c | 4 ++++
> > > >   1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/xe/xe_gt_clock.c b/drivers/gpu/drm/xe/xe_gt_clock.c
> > > > index 4f011d1573c65..26052710be8a7 100644
> > > > --- a/drivers/gpu/drm/xe/xe_gt_clock.c
> > > > +++ b/drivers/gpu/drm/xe/xe_gt_clock.c
> > > > @@ -57,6 +57,10 @@ static void read_crystal_clock(struct xe_gt *gt, u32 rpm_config_reg, u32 *freq,
> > > >   static void check_ctc_mode(struct xe_gt *gt)
> > > >   {
> > > > +	/* CTC_MODE register is removed by Xe3p */
> > > > +	if (GT_VER(gt) >= 35)
> > > > +		return;
> > > > +
> > > >   	/*
> > > >   	 * CTC_MODE[0] = 1 is definitely not supported for Xe2 and later
> > > >   	 * platforms.  In theory it could be a valid setting for pre-Xe2
> > > >
> > 
> > -- 
> > Matt Roper
> > Graphics Software Engineer
> > Linux GPU Platform Enablement
> > Intel Corporation