Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform defines

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 13/07/2023 13:12, Bhadane, Dnyaneshwar wrote:
> 
> 
>> -----Original Message-----
>> From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>> Sent: Thursday, July 13, 2023 5:26 PM
>> To: Jani Nikula <jani.nikula@linux.intel.com>; Bhadane, Dnyaneshwar
>> <dnyaneshwar.bhadane@intel.com>; intel-gfx@lists.freedesktop.org;
>> Ursulin, Tvrtko <tvrtko.ursulin@intel.com>
>> Subject: Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for
>> platform/subplatform defines
>>
>>
>> On 13/07/2023 10:39, Jani Nikula wrote:
>>> On Thu, 13 Jul 2023, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>>>> On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote:
>>>>>> -----Original Message-----
>>>>>> From: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@intel.com>
>>>>>> Sent: Monday, July 10, 2023 4:28 PM
>>>>>> To: intel-gfx@lists.freedesktop.org
>>>>>> Cc: Ursulin, Tvrtko <tvrtko.ursulin@intel.com>;
>>>>>> jani.nikula@linux.intel.com; Srivatsa, Anusha
>>>>>> <anusha.srivatsa@intel.com>; Bhadane, Dnyaneshwar
>>>>>> <dnyaneshwar.bhadane@intel.com>
>>>>>> Subject: [v3] drm/i915/mtl: s/MTL/METEORLAKE for
>>>>>> platform/subplatform defines
>>>>>>
>>>>>> Follow consistent naming convention. Replace MTL with METEORLAKE.
>>>>>> Added defines that are replacing IS_MTL_GRAPHICS_STEP with
>>>>>> IS_METEORLAKE_P_GRAPHICS_STEP and
>> IS_METEORLAKE_M_GRAPHICS_STEP.
>>>>>> Also replaced IS_METEORLAKE_MEDIA_STEP instead of
>> IS_MTL_MEDIA_STEP
>>>>>> and IS_METEORLAKE_DISPLAY_STEP instead of IS_MTL_DISPLAY_STEP.
>>>>>>
>>>>> Hi Tvrtko,
>>>>> Could you please give the feedback on this ? or suggestion regarding the
>> approach.
>>>>
>>>> It's a step in the right direction I just wish we could do all
>>>> churning in one go.
>>>>
>>>> Have you captured IS_CFL and IS_CML in the series? ICL? HSW? Any
>>>> other I am missing?
>>>>
>>>> What have we concluded on Jani's suggestion to split it all to
>>>> IS_<platform> && IS_<subsys>?
>>>
>>> IS_<platform> && IS_<step> is what I was after.
>>
>> Yeah I mistyped. I liked that to so would get my ack.
>>
>>>> If you have a) captured all IS_<tla> and b) Jani acks the series too,
>>>> I guess go ahead.
>>>>
>>>> Hm.. what have we concluded to do with IS_JASPERLAKE_EHL?
>>>
>>> For sure it can't be *that*. It's JSL *or* EHL. Not subplatform.
>>
>> IS_ELKHARTLAKE would indeed work and platform/subplatform can be
>> hidden implementation detail.
>>
>>>> P.S.
>>>> I still think these suck though:
>>>>
>>>> 	if (IS_METEORLAKE_M_GRAPHICS_STEP(i915, STEP_A0, STEP_B0) ||
>>>> 	    IS_METEORLAKE_P_GRAPHICS_STEP(i915, STEP_A0, STEP_B0))
>>>
>>> I still find it appealing to a) go towards shorter acronyms instead of
>>> long names, and b) to separate platform and stepping checks because
>>> they're orthogonal. They're only bundled together for historical
>>> reasons, and to keep the conditions shorter.
>>>
>>> The above could be:
>>>
>>> 	if (IS_MTL(i915) && IS_GRAPHICS_STEP(i915, STEP_A0, STEP_B0))
>>
>> I'd be super pleased with that.
> 
> Could we use the above suggestion for MTL variants for P/M? also replacing MTL with METEORLAKE.
> 
> Using the format:  IS_FULL_PLATFORM_NAME && IS_GRAPHICS_STEP(i915, STEP_A0, STEP_B0).
> 
> It will change to :
> 	For M:	IS_METEORLAKE_M(i915) && IS_GRAPHICS_STEP(i915, STEP_A0, STEP_B0)
> 	For P:	IS_METEORLAKE_P(i915) && IS_GRAPHICS_STEP(i915, STEP_A0, STEP_B0)

You could, but you'd only get a meh from me. :) Why you'd insist to keep 
the two checks? Are we expecting IS_METEROLAKE_<X> at some point?

Regards,

Tvrtko