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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On Wed, 08 May 2024, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Wed, May 08, 2024 at 02:45:10PM +0300, Jani Nikula wrote:
>> On Wed, 08 May 2024, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> 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.


>> >> 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", 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.

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.


Jani Nikula, Intel

[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux