Re: [PATCH 1/5] drm/i915: don't include CML PCI IDs in CFL

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, May 10, 2024 at 01:24:12PM +0300, Jani Nikula wrote:
> On Wed, 08 May 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Wed, May 08, 2024 at 02:45:10PM +0300, Jani Nikula wrote:
> >> On Wed, 08 May 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> >> > On Tue, May 07, 2024 at 09:47:16AM -0400, Rodrigo Vivi wrote:
> >> >> On Tue, May 07, 2024 at 03:56:48PM +0300, Jani Nikula wrote:
> >> >> > It's confusing for INTEL_CFL_IDS() to include all CML PCI IDs. Even if
> >> >> > we treat them the same in a lot of places, CML is a platform of its own,
> >> >> > and the lists of PCI IDs should not conflate them.
> 
> [snip]
> 
> >> >> Why only CML and not AML and WHL as well?
> >> >
> >> > Why do we even have CML as a separate platform? The only difference 
> >> > I can see is is that we do allow_read_ctx_timestamp() for CML but
> >> > not for CFL. Does that even make sense?
> >> 
> >> git blame tells me:
> >> 
> >> 5f4ae2704d59 ("drm/i915: Identify Cometlake platform")
> >> dbc7e72897a4 ("drm/i915/gt: Make the CTX_TIMESTAMP readable on !rcs")
> >
> > Right. That explains why we need it on CML+. But is there some reason
> > we  can't just do it on CFL as well, even if not strictly necessary?
> > I would assume that setting FORCE_TO_NONPRIV on an already
> > non-privileged register should be totally fine.
> 
> I have absolutely no idea.
> 
> I'm somewhat thinking "CML being a separate platform" is a separate
> problem from "CFL PCI ID macros including CML".
> 
> I'm starting to think the PCI ID macros should be grouped by "does the
> platform have a name of its own",

That and/or "does bspec have a separate 'Confgurations <platform>' page?"

> not by how those macros are actually
> used by the driver. Keeping them separate at the PCI ID macro level just
> reduces the pain in maintaining the PCI IDs, and lets us wiggle stuff
> around in the driver how we see fit.

Aye.

> 
> And that spins back to Rodrigo's question, "Why only CML and not AML and
> WHL as well?". Yeah, indeed.
> 
> If we decide to stop treating CML as a separate platform in the
> *driver*, that's another matter.

Sure. Seeing it just got me wondering...

-- 
Ville Syrjälä
Intel