> -----Original Message----- > From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> > Sent: Thursday, July 13, 2023 5:55 PM > To: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@xxxxxxxxx>; Jani Nikula > <jani.nikula@xxxxxxxxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Ursulin, > Tvrtko <tvrtko.ursulin@xxxxxxxxx> > Cc: Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx>; Shankar, Uma > <uma.shankar@xxxxxxxxx> > Subject: Re: [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 <tvrtko.ursulin@xxxxxxxxxxxxxxx> > >> Sent: Thursday, July 13, 2023 5:26 PM > >> To: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>; Bhadane, Dnyaneshwar > >> <dnyaneshwar.bhadane@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; > >> Ursulin, Tvrtko <tvrtko.ursulin@xxxxxxxxx> > >> Subject: Re: [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@xxxxxxxxxxxxxxx> > wrote: > >>>> On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote: > >>>>>> -----Original Message----- > >>>>>> From: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@xxxxxxxxx> > >>>>>> Sent: Monday, July 10, 2023 4:28 PM > >>>>>> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > >>>>>> Cc: Ursulin, Tvrtko <tvrtko.ursulin@xxxxxxxxx>; > >>>>>> jani.nikula@xxxxxxxxxxxxxxx; Srivatsa, Anusha > >>>>>> <anusha.srivatsa@xxxxxxxxx>; Bhadane, Dnyaneshwar > >>>>>> <dnyaneshwar.bhadane@xxxxxxxxx> > >>>>>> 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
