Re: [PATCH v1] drm/i915/dg2: enable G8 with a workaround

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 09 Oct 2024, Raag Jadav <raag.jadav@intel.com> wrote:
> On Wed, Oct 09, 2024 at 04:05:20PM +0300, Jani Nikula wrote:
>> On Wed, 09 Oct 2024, Raag Jadav <raag.jadav@intel.com> wrote:
>> > On Tue, Oct 08, 2024 at 08:24:42PM +0300, Jani Nikula wrote:
>> >> On Mon, 07 Oct 2024, Raag Jadav <raag.jadav@intel.com> wrote:
>> >> > +
>> >> > +/* Wa_14022698589:dg2 */
>> >> > +static void intel_enable_g8(struct intel_uncore *uncore)
>> >> > +{
>> >> > +	if (IS_DG2(uncore->i915)) {
>> >> > +		switch (INTEL_DEVID(uncore->i915)) {
>> >> 
>> >> Even using INTEL_DEVID() is a no-go. There are currently four users, and
>> >> even some of them are too much.
>> >> 
>> >> We try hard to abstract this stuff at a higher level, and there must be
>> >> zero direct PCI ID checks in code other than the table driven device
>> >> identification. Otherwise it's just impossible to figure out where we do
>> >> platform specific stuff for each platform.
>> >
>> > Even if we use pci_match_id(), we'd need an explicit list to match against.
>> 
>> Well, we don't use that for individual workarounds or hacks either. When
>> you think of using something like that, see what git grep says.
>> 
>> > Any better way?
>> 
>> You probably need to turn it into another subplatform, and add it in
>> intel_device_info.c. You're probably going to need to rehash the
>> INTEL_DG2_*_IDS PCI ID macros too. That's how we tell platforms apart at
>> the PCI ID granularity.
>
> Which would be controversial since the ids span across existing subplatforms,
> which we don't want to break.

The i915 subplatform thing supports multiple subplatforms.

> Don't we have quirk mask thing like display?

Please elaborate.

BR,
Jani.





-- 
Jani Nikula, Intel