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?
Regards,
Tvrtko
