> -----Original Message-----
> From: Tvrtko Ursulin <[email protected]>
> Sent: Thursday, July 13, 2023 5:55 PM
> To: Bhadane, Dnyaneshwar <[email protected]>; Jani Nikula
> <[email protected]>; [email protected]; Ursulin,
> Tvrtko <[email protected]>
> Cc: Srivatsa, Anusha <[email protected]>; Shankar, Uma
> <[email protected]>
> Subject: Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for
> platform/subplatform defines
>
>
> On 13/07/2023 13:12, Bhadane, Dnyaneshwar wrote:
> >
> >
> >> -----Original Message-----
> >> From: Tvrtko Ursulin <[email protected]>
> >> Sent: Thursday, July 13, 2023 5:26 PM
> >> To: Jani Nikula <[email protected]>; Bhadane, Dnyaneshwar
> >> <[email protected]>; [email protected];
> >> Ursulin, Tvrtko <[email protected]>
> >> 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 <[email protected]>
> wrote:
> >>>> On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote:
> >>>>>> -----Original Message-----
> >>>>>> From: Bhadane, Dnyaneshwar <[email protected]>
> >>>>>> Sent: Monday, July 10, 2023 4:28 PM
> >>>>>> To: [email protected]
> >>>>>> Cc: Ursulin, Tvrtko <[email protected]>;
> >>>>>> [email protected]; Srivatsa, Anusha
> >>>>>> <[email protected]>; Bhadane, Dnyaneshwar
> >>>>>> <[email protected]>
> >>>>>> 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?
For example FILE PATH: drivers/gpu/drm/i915/gt/intel_workarounds.c
Multiple occurrences of IS_MTL_GRAPHICS_STEP(i915, M/P, STEP_B0, STEP_FOREVER)
Where P and M are passed explicitly. That why we can not check IS_METEORLAKE()
as single check.
IS_MTL_GRAPHICS_STEP(i915, M, STEP_B0, STEP_FOREVER) ||
IS_MTL_GRAPHICS_STEP(i915, P, STEP_B0, STEP_FOREVER))
The IS_GRAPHICS_STEP is generic macro and used by other platforms also.
On changing the IS_GRAPHICS_STEP only for MTL variants is lead to affect the
other
platform. The IS_METEORLAKE_P(i915) or IS_METEORLAKE_M(i915) solves the
problem.
to differentiate the MTL platform variant.
Regards,
Dnyaneshwar.
>
> Regards,
>
> Tvrtko
