Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform defines
On 17/07/2023 07:42, Bhadane, Dnyaneshwar wrote: -Original Message- From: Tvrtko Ursulin Sent: Thursday, July 13, 2023 5:26 PM To: Jani Nikula ; Bhadane, Dnyaneshwar ; intel-gfx@lists.freedesktop.org; Ursulin, Tvrtko 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 wrote: On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote: -Original Message- From: Bhadane, Dnyaneshwar Sent: Monday, July 10, 2023 4:28 PM To: intel-gfx@lists.freedesktop.org Cc: Ursulin, Tvrtko ; jani.nik...@linux.intel.com; Srivatsa, Anusha ; Bhadane, Dnyaneshwar 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_ && IS_? IS_ && IS_ is what I was after. Yeah I mistyped. I liked that to so would get my ack. If you have a) captured all IS_ 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. Changing to acronyms in i915 driver means we need to make all these changes in XE driver as well. That is why we are insisting to go for full name approach. https://gitlab.freedesktop.org/drm/xe/kernel/-/blob/drm-xe-next/drivers/gpu/drm/xe/xe_platform_types.h I didn't get this - platform enum vs macro names? But anyway, I am okay with long names too. Just want to see _all_ of them converted, no overly long IS_JASPERLAKE_ELKHARTLAKE but change to IS_ELKHARTLAKE, and the conversion to "IS_METEORLAKE && IS__STEP". Regards, Tvrtko The above could be: if (IS_MTL(i915) && IS_GRAPHICS_STEP(i915, STEP_A0, STEP_B0)) I'd be super pleased with that. Regards, Tvrtko BR, Jani. I am not convinced we get anything (apart more source code and more binary) by having duplicated conditions. I guess I will have to send that cleanup later. v2: - Replace IS_MTL_GRAPHICS_STEP with IS_METEROLAKE_(P/M)_GRAPHICS_STEP (Tvrtko). - Changed subject prefix mtl instead of MTL (Anusha) v3: - Updated the commit message. (Anusha) Cc: Tvrtko Ursulin Cc: Jani Nikula Cc: Anusha Srivatsa Signed-off-by: Dnyaneshwar Bhadane --- drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- drivers/gpu/drm/i915/display/intel_pmdemand.c | 2 +- drivers/gpu/drm/i915/display/intel_psr.c | 10 ++--- .../drm/i915/display/skl_universal_plane.c| 4 +- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 8 ++-- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- .../drm/i915/gt/intel_execlists_submission.c | 2 +- drivers/gpu/drm/i915/gt/intel_gt_mcr.c| 4 +- drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +- drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +- drivers/gpu/drm/i915/gt/intel_workarounds.c | 44 +--- --- drivers/gpu/drm/i915/gt/uc/intel_guc.c| 4 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 +- drivers/gpu/drm/i915/i915_drv.h | 15 +-- drivers/gpu/drm/i915/i915_perf.c | 4 +- 15 files changed, 60 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 7f8b2d7713c7..6358a8b26172 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1093,7 +1093,7 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, /* Wa_14016291713 */ if ((IS_DISPLAY_VER(i915, 12, 13) || -IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && +IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && crtc_state->has_psr) {
Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform defines
> -Original Message- > From: Tvrtko Ursulin > Sent: Thursday, July 13, 2023 5:26 PM > To: Jani Nikula ; Bhadane, Dnyaneshwar > ; intel-gfx@lists.freedesktop.org; > Ursulin, Tvrtko > 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 wrote: > >> On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote: > >>>> -Original Message- > >>>> From: Bhadane, Dnyaneshwar > >>>> Sent: Monday, July 10, 2023 4:28 PM > >>>> To: intel-gfx@lists.freedesktop.org > >>>> Cc: Ursulin, Tvrtko ; > >>>> jani.nik...@linux.intel.com; Srivatsa, Anusha > >>>> ; Bhadane, Dnyaneshwar > >>>> > >>>> 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_ && IS_? > > > > IS_ && IS_ is what I was after. > > Yeah I mistyped. I liked that to so would get my ack. > > >> If you have a) captured all IS_ 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. Changing to acronyms in i915 driver means we need to make all these changes in XE driver as well. That is why we are insisting to go for full name approach. https://gitlab.freedesktop.org/drm/xe/kernel/-/blob/drm-xe-next/drivers/gpu/drm/xe/xe_platform_types.h > > > > The above could be: > > > > if (IS_MTL(i915) && IS_GRAPHICS_STEP(i915, STEP_A0, STEP_B0)) > > I'd be super pleased with that. > > Regards, > > Tvrtko > > > > > > > BR, > > Jani. > > > > > >> > >> I am not convinced we get anything (apart more source code and more > >> binary) by having duplicated conditions. I guess I will have to send > >> that cleanup later. > >> > >>>> v2: > >>>> - Replace IS_MTL_GRAPHICS_STEP with > >>>> IS_METEROLAKE_(P/M)_GRAPHICS_STEP (Tvrtko). > >>>> - Changed subject prefix mtl instead of MTL (Anusha) > >>>> v3: > >>>> - Updated the commit message. (Anusha) > >>>> > >>>> Cc: Tvrtko Ursulin > >>>> Cc: Jani Nikula > >>>> Cc: Anusha Srivatsa > >>>> Signed-off-by: Dnyaneshwar Bhadane > > >>>> --- > >>>>drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- > >>>>drivers/gpu/drm/i915/display/intel_pmdemand.c | 2 +- > >>>>drivers/gpu/drm/i915/display/intel_psr.c | 10 ++--- > >>>>.../drm/i915/display/skl_universal_plane.c| 4 +- > >>>>drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 8 ++-- > >>>>drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- > >>>>.../drm/i915/gt/intel_execlists_submission.c | 2 +- > >>>>drivers/gpu/drm/i9
Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform defines
On Thu, 13 Jul 2023, Jani Nikula wrote: > On Thu, 13 Jul 2023, "Bhadane, Dnyaneshwar" > wrote: >>> -Original Message- >>> From: Tvrtko Ursulin >>> Sent: Thursday, July 13, 2023 5:55 PM >>> To: Bhadane, Dnyaneshwar ; Jani Nikula >>> ; intel-gfx@lists.freedesktop.org; Ursulin, >>> Tvrtko >>> Cc: Srivatsa, Anusha ; Shankar, Uma >>> >>> 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 >>> >> Sent: Thursday, July 13, 2023 5:26 PM >>> >> To: Jani Nikula ; Bhadane, Dnyaneshwar >>> >> ; intel-gfx@lists.freedesktop.org; >>> >> Ursulin, Tvrtko >>> >> 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 >>> wrote: >>> >>>> On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote: >>> >>>>>> -Original Message- >>> >>>>>> From: Bhadane, Dnyaneshwar >>> >>>>>> Sent: Monday, July 10, 2023 4:28 PM >>> >>>>>> To: intel-gfx@lists.freedesktop.org >>> >>>>>> Cc: Ursulin, Tvrtko ; >>> >>>>>> jani.nik...@linux.intel.com; Srivatsa, Anusha >>> >>>>>> ; Bhadane, Dnyaneshwar >>> >>>>>> >>> >>>>>> 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_ && IS_? >>> >>> >>> >>> IS_ && IS_ is what I was after. >>> >> >>> >> Yeah I mistyped. I liked that to so would get my ack. >>> >> >>> >>>> If you have a) captured all IS_ 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. >>> >>> >>> >>>
Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform defines
On Thu, 13 Jul 2023, "Bhadane, Dnyaneshwar" wrote: >> -Original Message- >> From: Tvrtko Ursulin >> Sent: Thursday, July 13, 2023 5:55 PM >> To: Bhadane, Dnyaneshwar ; Jani Nikula >> ; intel-gfx@lists.freedesktop.org; Ursulin, >> Tvrtko >> Cc: Srivatsa, Anusha ; Shankar, Uma >> >> 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 >> >> Sent: Thursday, July 13, 2023 5:26 PM >> >> To: Jani Nikula ; Bhadane, Dnyaneshwar >> >> ; intel-gfx@lists.freedesktop.org; >> >> Ursulin, Tvrtko >> >> 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 >> wrote: >> >>>> On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote: >> >>>>>> -Original Message- >> >>>>>> From: Bhadane, Dnyaneshwar >> >>>>>> Sent: Monday, July 10, 2023 4:28 PM >> >>>>>> To: intel-gfx@lists.freedesktop.org >> >>>>>> Cc: Ursulin, Tvrtko ; >> >>>>>> jani.nik...@linux.intel.com; Srivatsa, Anusha >> >>>>>> ; Bhadane, Dnyaneshwar >> >>>>>> >> >>>>>> 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_ && IS_? >> >>> >> >>> IS_ && IS_ is what I was after. >> >> >> >> Yeah I mistyped. I liked that to so would get my ack. >> >> >> >>>> If you have a) captured all IS_ 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_PLATF
Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform defines
> -Original Message- > From: Tvrtko Ursulin > Sent: Thursday, July 13, 2023 5:55 PM > To: Bhadane, Dnyaneshwar ; Jani Nikula > ; intel-gfx@lists.freedesktop.org; Ursulin, > Tvrtko > Cc: Srivatsa, Anusha ; Shankar, Uma > > 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 > >> Sent: Thursday, July 13, 2023 5:26 PM > >> To: Jani Nikula ; Bhadane, Dnyaneshwar > >> ; intel-gfx@lists.freedesktop.org; > >> Ursulin, Tvrtko > >> 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 > wrote: > >>>> On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote: > >>>>>> -Original Message- > >>>>>> From: Bhadane, Dnyaneshwar > >>>>>> Sent: Monday, July 10, 2023 4:28 PM > >>>>>> To: intel-gfx@lists.freedesktop.org > >>>>>> Cc: Ursulin, Tvrtko ; > >>>>>> jani.nik...@linux.intel.com; Srivatsa, Anusha > >>>>>> ; Bhadane, Dnyaneshwar > >>>>>> > >>>>>> 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_ && IS_? > >>> > >>> IS_ && IS_ is what I was after. > >> > >> Yeah I mistyped. I liked that to so would get my ack. > >> > >>>> If you have a) captured all IS_ 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_ 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
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 Sent: Thursday, July 13, 2023 5:26 PM To: Jani Nikula ; Bhadane, Dnyaneshwar ; intel-gfx@lists.freedesktop.org; Ursulin, Tvrtko 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 wrote: On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote: -Original Message- From: Bhadane, Dnyaneshwar Sent: Monday, July 10, 2023 4:28 PM To: intel-gfx@lists.freedesktop.org Cc: Ursulin, Tvrtko ; jani.nik...@linux.intel.com; Srivatsa, Anusha ; Bhadane, Dnyaneshwar 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_ && IS_? IS_ && IS_ is what I was after. Yeah I mistyped. I liked that to so would get my ack. If you have a) captured all IS_ 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_ at some point? Regards, Tvrtko
Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform defines
> -Original Message- > From: Tvrtko Ursulin > Sent: Thursday, July 13, 2023 5:26 PM > To: Jani Nikula ; Bhadane, Dnyaneshwar > ; intel-gfx@lists.freedesktop.org; > Ursulin, Tvrtko > 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 wrote: > >> On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote: > >>>> -Original Message- > >>>> From: Bhadane, Dnyaneshwar > >>>> Sent: Monday, July 10, 2023 4:28 PM > >>>> To: intel-gfx@lists.freedesktop.org > >>>> Cc: Ursulin, Tvrtko ; > >>>> jani.nik...@linux.intel.com; Srivatsa, Anusha > >>>> ; Bhadane, Dnyaneshwar > >>>> > >>>> 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_ && IS_? > > > > IS_ && IS_ is what I was after. > > Yeah I mistyped. I liked that to so would get my ack. > > >> If you have a) captured all IS_ 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) Regards , Dnyaneshwar > > Regards, > > Tvrtko > > > > > > > BR, > > Jani. > > > > > >> > >> I am not convinced we get anything (apart more source code and more > >> binary) by having duplicated conditions. I guess I will have to send > >> that cleanup later. > >> > >>>> v2: > >>>> - Replace IS_MTL_GRAPHICS_STEP with > >>>> IS_METEROLAKE_(P/M)_GRAPHICS_STEP (Tvrtko). > >>>> - Changed subject prefix mtl instead of MTL (Anusha) > >>>> v3: > >>>> - Updated the commit message. (Anusha) > >>>> > >>>> Cc: Tvrtko Ursulin > >>>> Cc: Jani Nikula > >>>> Cc: Anusha Srivatsa > >>>> Signed-off-by: Dnyaneshwar Bhadane > > >>>> --- > >>>>drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- > >>>>drivers/gpu/drm/i915/display/intel_pmdemand.c | 2 +- > >>>>drivers/gpu/drm/i915/display/intel_psr.c | 10 ++--- > >>>>.../drm/i915/display/skl_universal_plane.c| 4 +- > >>>>drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 8 ++-- > >>>>drivers/g
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 wrote: On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote: -Original Message- From: Bhadane, Dnyaneshwar Sent: Monday, July 10, 2023 4:28 PM To: intel-gfx@lists.freedesktop.org Cc: Ursulin, Tvrtko ; jani.nik...@linux.intel.com; Srivatsa, Anusha ; Bhadane, Dnyaneshwar 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_ && IS_? IS_ && IS_ is what I was after. Yeah I mistyped. I liked that to so would get my ack. If you have a) captured all IS_ 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. Regards, Tvrtko BR, Jani. I am not convinced we get anything (apart more source code and more binary) by having duplicated conditions. I guess I will have to send that cleanup later. v2: - Replace IS_MTL_GRAPHICS_STEP with IS_METEROLAKE_(P/M)_GRAPHICS_STEP (Tvrtko). - Changed subject prefix mtl instead of MTL (Anusha) v3: - Updated the commit message. (Anusha) Cc: Tvrtko Ursulin Cc: Jani Nikula Cc: Anusha Srivatsa Signed-off-by: Dnyaneshwar Bhadane --- drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- drivers/gpu/drm/i915/display/intel_pmdemand.c | 2 +- drivers/gpu/drm/i915/display/intel_psr.c | 10 ++--- .../drm/i915/display/skl_universal_plane.c| 4 +- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 8 ++-- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- .../drm/i915/gt/intel_execlists_submission.c | 2 +- drivers/gpu/drm/i915/gt/intel_gt_mcr.c| 4 +- drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +- drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +- drivers/gpu/drm/i915/gt/intel_workarounds.c | 44 +-- drivers/gpu/drm/i915/gt/uc/intel_guc.c| 4 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 +- drivers/gpu/drm/i915/i915_drv.h | 15 +-- drivers/gpu/drm/i915/i915_perf.c | 4 +- 15 files changed, 60 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 7f8b2d7713c7..6358a8b26172 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1093,7 +1093,7 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, /* Wa_14016291713 */ if ((IS_DISPLAY_VER(i915, 12, 13) || -IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && +IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && crtc_state->has_psr) { plane_state->no_fbc_reason = "PSR1 enabled (Wa_14016291713)"; return 0; diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c index f7608d363634..8c3158b188ef 100644 --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c @@ -92,7 +92,7 @@ int intel_pmdemand_init(struct drm_i915_private *i915) _state->base, _pmdemand_funcs); - if (IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) + if (IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) /* Wa_14016740474 */ intel_de_rmw(i915, XELPD_CHICKEN_DCPR_3, 0, DMD_RSP_TIMEOUT_DISABLE); diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 62151abe4748..ecd4e36119b2 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++
Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform defines
On Thu, 13 Jul 2023, Tvrtko Ursulin wrote: > On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote: >>> -Original Message- >>> From: Bhadane, Dnyaneshwar >>> Sent: Monday, July 10, 2023 4:28 PM >>> To: intel-gfx@lists.freedesktop.org >>> Cc: Ursulin, Tvrtko ; jani.nik...@linux.intel.com; >>> Srivatsa, Anusha ; Bhadane, Dnyaneshwar >>> >>> 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_ && IS_? IS_ && IS_ is what I was after. > If you have a) captured all IS_ 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. > > Regards, > > Tvrtko > > 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)) BR, Jani. > > I am not convinced we get anything (apart more source code and more > binary) by having duplicated conditions. I guess I will have to send > that cleanup later. > >>> v2: >>> - Replace IS_MTL_GRAPHICS_STEP with >>> IS_METEROLAKE_(P/M)_GRAPHICS_STEP (Tvrtko). >>> - Changed subject prefix mtl instead of MTL (Anusha) >>> v3: >>> - Updated the commit message. (Anusha) >>> >>> Cc: Tvrtko Ursulin >>> Cc: Jani Nikula >>> Cc: Anusha Srivatsa >>> Signed-off-by: Dnyaneshwar Bhadane >>> --- >>> drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- >>> drivers/gpu/drm/i915/display/intel_pmdemand.c | 2 +- >>> drivers/gpu/drm/i915/display/intel_psr.c | 10 ++--- >>> .../drm/i915/display/skl_universal_plane.c| 4 +- >>> drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 8 ++-- >>> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- >>> .../drm/i915/gt/intel_execlists_submission.c | 2 +- >>> drivers/gpu/drm/i915/gt/intel_gt_mcr.c| 4 +- >>> drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +- >>> drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +- >>> drivers/gpu/drm/i915/gt/intel_workarounds.c | 44 +-- >>> drivers/gpu/drm/i915/gt/uc/intel_guc.c| 4 +- >>> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 +- >>> drivers/gpu/drm/i915/i915_drv.h | 15 +-- >>> drivers/gpu/drm/i915/i915_perf.c | 4 +- >>> 15 files changed, 60 insertions(+), 51 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c >>> b/drivers/gpu/drm/i915/display/intel_fbc.c >>> index 7f8b2d7713c7..6358a8b26172 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_fbc.c >>> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c >>> @@ -1093,7 +1093,7 @@ static int intel_fbc_check_plane(struct >>> intel_atomic_state *state, >>> >>> /* Wa_14016291713 */ >>> if ((IS_DISPLAY_VER(i915, 12, 13) || >>> -IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && >>> +IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && >>> crtc_state->has_psr) { >>> plane_state->no_fbc_reason = "PSR1 enabled >>> (Wa_14016291713)"; >>> return 0; >>> diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c >>> b/drivers/gpu/drm/i915/display/intel_pmdemand.c >>> index f7608d363634..8c3158b188ef 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c >>> +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c >>> @@ -92,7 +92,7 @@ int intel_pmdemand_init(struct drm_i915_private *i915) >>> _state->base, >>> _pmdemand_funcs); >>> >>> - if (IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) >>> + if (IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) >>> /* Wa_14016740474 */ >>> intel_de_rmw(i915, XELPD_CHICKEN_DCPR_3, 0, >>> DMD_RSP_TIMEOUT_DISABLE); >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c >>> b/drivers/gpu/drm/i915/display/intel_psr.c >>> index 62151abe4748..ecd4e36119b2 100644
Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform defines
On 10/07/2023 14:44, Bhadane, Dnyaneshwar wrote: -Original Message- From: Bhadane, Dnyaneshwar Sent: Monday, July 10, 2023 4:28 PM To: intel-gfx@lists.freedesktop.org Cc: Ursulin, Tvrtko ; jani.nik...@linux.intel.com; Srivatsa, Anusha ; Bhadane, Dnyaneshwar 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_ && IS_? If you have a) captured all IS_ and b) Jani acks the series too, I guess go ahead. Hm.. what have we concluded to do with IS_JASPERLAKE_EHL? Regards, Tvrtko 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 am not convinced we get anything (apart more source code and more binary) by having duplicated conditions. I guess I will have to send that cleanup later. v2: - Replace IS_MTL_GRAPHICS_STEP with IS_METEROLAKE_(P/M)_GRAPHICS_STEP (Tvrtko). - Changed subject prefix mtl instead of MTL (Anusha) v3: - Updated the commit message. (Anusha) Cc: Tvrtko Ursulin Cc: Jani Nikula Cc: Anusha Srivatsa Signed-off-by: Dnyaneshwar Bhadane --- drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- drivers/gpu/drm/i915/display/intel_pmdemand.c | 2 +- drivers/gpu/drm/i915/display/intel_psr.c | 10 ++--- .../drm/i915/display/skl_universal_plane.c| 4 +- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 8 ++-- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- .../drm/i915/gt/intel_execlists_submission.c | 2 +- drivers/gpu/drm/i915/gt/intel_gt_mcr.c| 4 +- drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +- drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +- drivers/gpu/drm/i915/gt/intel_workarounds.c | 44 +-- drivers/gpu/drm/i915/gt/uc/intel_guc.c| 4 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 +- drivers/gpu/drm/i915/i915_drv.h | 15 +-- drivers/gpu/drm/i915/i915_perf.c | 4 +- 15 files changed, 60 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 7f8b2d7713c7..6358a8b26172 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1093,7 +1093,7 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, /* Wa_14016291713 */ if ((IS_DISPLAY_VER(i915, 12, 13) || -IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && +IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && crtc_state->has_psr) { plane_state->no_fbc_reason = "PSR1 enabled (Wa_14016291713)"; return 0; diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c index f7608d363634..8c3158b188ef 100644 --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c @@ -92,7 +92,7 @@ int intel_pmdemand_init(struct drm_i915_private *i915) _state->base, _pmdemand_funcs); - if (IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) + if (IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) /* Wa_14016740474 */ intel_de_rmw(i915, XELPD_CHICKEN_DCPR_3, 0, DMD_RSP_TIMEOUT_DISABLE); diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 62151abe4748..ecd4e36119b2 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1247,7 +1247,7 @@ static void wm_optimization_wa(struct intel_dp *intel_dp, bool set_wa_bit = false; /* Wa_14015648006 */ - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || + if (IS_METEORLAKE_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || IS_DISPLAY_VER(dev_priv, 11, 13)) set_wa_bit |= crtc_state->wm_level_disabled; @@ -1320,7 +1320,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, * All supported adlp panels have 1-based X granularity, this may * cause issues if non-supported panels are used. */ - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0,
Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform defines
I Like the acronym replacement approach - despite making the macro names longer, it is consistent with how platform is referred everywhere in the driver. For that, Reviewed-by: Anusha Srivatsa > -Original Message- > From: Bhadane, Dnyaneshwar > Sent: Monday, July 10, 2023 6:44 AM > To: intel-gfx@lists.freedesktop.org; Ursulin, Tvrtko > > Cc: jani.nik...@linux.intel.com; Srivatsa, Anusha > Subject: RE: [v3] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform > defines > > > > > -Original Message- > > From: Bhadane, Dnyaneshwar > > Sent: Monday, July 10, 2023 4:28 PM > > To: intel-gfx@lists.freedesktop.org > > Cc: Ursulin, Tvrtko ; > > jani.nik...@linux.intel.com; Srivatsa, Anusha > > ; Bhadane, Dnyaneshwar > > > > 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. > > > v2: > > - Replace IS_MTL_GRAPHICS_STEP with > > IS_METEROLAKE_(P/M)_GRAPHICS_STEP (Tvrtko). > > - Changed subject prefix mtl instead of MTL (Anusha) > > v3: > > - Updated the commit message. (Anusha) > > > > Cc: Tvrtko Ursulin > > Cc: Jani Nikula > > Cc: Anusha Srivatsa > > Signed-off-by: Dnyaneshwar Bhadane > > --- > > drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- > > drivers/gpu/drm/i915/display/intel_pmdemand.c | 2 +- > > drivers/gpu/drm/i915/display/intel_psr.c | 10 ++--- > > .../drm/i915/display/skl_universal_plane.c| 4 +- > > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 8 ++-- > > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- > > .../drm/i915/gt/intel_execlists_submission.c | 2 +- > > drivers/gpu/drm/i915/gt/intel_gt_mcr.c| 4 +- > > drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +- > > drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +- > > drivers/gpu/drm/i915/gt/intel_workarounds.c | 44 +-- > > drivers/gpu/drm/i915/gt/uc/intel_guc.c| 4 +- > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 +- > > drivers/gpu/drm/i915/i915_drv.h | 15 +-- > > drivers/gpu/drm/i915/i915_perf.c | 4 +- > > 15 files changed, 60 insertions(+), 51 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > > b/drivers/gpu/drm/i915/display/intel_fbc.c > > index 7f8b2d7713c7..6358a8b26172 100644 > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > > @@ -1093,7 +1093,7 @@ static int intel_fbc_check_plane(struct > > intel_atomic_state *state, > > > > /* Wa_14016291713 */ > > if ((IS_DISPLAY_VER(i915, 12, 13) || > > -IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && > > +IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && > > crtc_state->has_psr) { > > plane_state->no_fbc_reason = "PSR1 enabled > (Wa_14016291713)"; > > return 0; > > diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c > > b/drivers/gpu/drm/i915/display/intel_pmdemand.c > > index f7608d363634..8c3158b188ef 100644 > > --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c > > +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c > > @@ -92,7 +92,7 @@ int intel_pmdemand_init(struct drm_i915_private *i915) > > _state->base, > > _pmdemand_funcs); > > > > - if (IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) > > + if (IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) > > /* Wa_14016740474 */ > > intel_de_rmw(i915, XELPD_CHICKEN_DCPR_3, 0, > > DMD_RSP_TIMEOUT_DISABLE); > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index 62151abe4748..ecd4e36119b2 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -1247,7 +1247,7 @@ static void wm_optimization_wa(struct intel_dp > > *intel_dp, > > bool set_wa_bit = false; > > > > /* Wa_14015648006 */ > > - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || > > + if (IS_METEORLAKE_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || > > IS_DISPLAY_VER(dev_priv, 11, 13)) > > set_wa_bit |= crtc_state->wm_level_disabled; > > > > @@ -1320,7 +1320,7 @@ static void intel_psr_enable_source(struct > > intel_dp *intel_dp, > > * All supported adlp panels have 1-based X granularity, this > > may > > * cause issues if non-supported panels are used. > > */ > > - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > > +
Re: [Intel-gfx] [v3] drm/i915/mtl: s/MTL/METEORLAKE for platform/subplatform defines
> -Original Message- > From: Bhadane, Dnyaneshwar > Sent: Monday, July 10, 2023 4:28 PM > To: intel-gfx@lists.freedesktop.org > Cc: Ursulin, Tvrtko ; jani.nik...@linux.intel.com; > Srivatsa, Anusha ; Bhadane, Dnyaneshwar > > 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. > v2: > - Replace IS_MTL_GRAPHICS_STEP with > IS_METEROLAKE_(P/M)_GRAPHICS_STEP (Tvrtko). > - Changed subject prefix mtl instead of MTL (Anusha) > v3: > - Updated the commit message. (Anusha) > > Cc: Tvrtko Ursulin > Cc: Jani Nikula > Cc: Anusha Srivatsa > Signed-off-by: Dnyaneshwar Bhadane > --- > drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- > drivers/gpu/drm/i915/display/intel_pmdemand.c | 2 +- > drivers/gpu/drm/i915/display/intel_psr.c | 10 ++--- > .../drm/i915/display/skl_universal_plane.c| 4 +- > drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 8 ++-- > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- > .../drm/i915/gt/intel_execlists_submission.c | 2 +- > drivers/gpu/drm/i915/gt/intel_gt_mcr.c| 4 +- > drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +- > drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 44 +-- > drivers/gpu/drm/i915/gt/uc/intel_guc.c| 4 +- > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 +- > drivers/gpu/drm/i915/i915_drv.h | 15 +-- > drivers/gpu/drm/i915/i915_perf.c | 4 +- > 15 files changed, 60 insertions(+), 51 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c > b/drivers/gpu/drm/i915/display/intel_fbc.c > index 7f8b2d7713c7..6358a8b26172 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbc.c > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c > @@ -1093,7 +1093,7 @@ static int intel_fbc_check_plane(struct > intel_atomic_state *state, > > /* Wa_14016291713 */ > if ((IS_DISPLAY_VER(i915, 12, 13) || > - IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && > + IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && > crtc_state->has_psr) { > plane_state->no_fbc_reason = "PSR1 enabled > (Wa_14016291713)"; > return 0; > diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c > b/drivers/gpu/drm/i915/display/intel_pmdemand.c > index f7608d363634..8c3158b188ef 100644 > --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c > +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c > @@ -92,7 +92,7 @@ int intel_pmdemand_init(struct drm_i915_private *i915) >_state->base, >_pmdemand_funcs); > > - if (IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) > + if (IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) > /* Wa_14016740474 */ > intel_de_rmw(i915, XELPD_CHICKEN_DCPR_3, 0, > DMD_RSP_TIMEOUT_DISABLE); > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > b/drivers/gpu/drm/i915/display/intel_psr.c > index 62151abe4748..ecd4e36119b2 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -1247,7 +1247,7 @@ static void wm_optimization_wa(struct intel_dp > *intel_dp, > bool set_wa_bit = false; > > /* Wa_14015648006 */ > - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || > + if (IS_METEORLAKE_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || > IS_DISPLAY_VER(dev_priv, 11, 13)) > set_wa_bit |= crtc_state->wm_level_disabled; > > @@ -1320,7 +1320,7 @@ static void intel_psr_enable_source(struct intel_dp > *intel_dp, >* All supported adlp panels have 1-based X granularity, this > may >* cause issues if non-supported panels are used. >*/ > - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > + if (IS_METEORLAKE_DISPLAY_STEP(dev_priv, STEP_A0, > STEP_B0)) > intel_de_rmw(dev_priv, > MTL_CHICKEN_TRANS(cpu_transcoder), 0, >ADLP_1_BASED_X_GRANULARITY); > else if (IS_ALDERLAKE_P(dev_priv)) > @@ -1328,7 +1328,7 @@ static void intel_psr_enable_source(struct intel_dp > *intel_dp, >ADLP_1_BASED_X_GRANULARITY); > > /* Wa_16012604467:adlp,mtl[a0,b0] */ > - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) > + if (IS_METEORLAKE_DISPLAY_STEP(dev_priv, STEP_A0, > STEP_B0)) >
[Intel-gfx] [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. v2: - Replace IS_MTL_GRAPHICS_STEP with IS_METEROLAKE_(P/M)_GRAPHICS_STEP (Tvrtko). - Changed subject prefix mtl instead of MTL (Anusha) v3: - Updated the commit message. (Anusha) Cc: Tvrtko Ursulin Cc: Jani Nikula Cc: Anusha Srivatsa Signed-off-by: Dnyaneshwar Bhadane --- drivers/gpu/drm/i915/display/intel_fbc.c | 2 +- drivers/gpu/drm/i915/display/intel_pmdemand.c | 2 +- drivers/gpu/drm/i915/display/intel_psr.c | 10 ++--- .../drm/i915/display/skl_universal_plane.c| 4 +- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 8 ++-- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- .../drm/i915/gt/intel_execlists_submission.c | 2 +- drivers/gpu/drm/i915/gt/intel_gt_mcr.c| 4 +- drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +- drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +- drivers/gpu/drm/i915/gt/intel_workarounds.c | 44 +-- drivers/gpu/drm/i915/gt/uc/intel_guc.c| 4 +- .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 +- drivers/gpu/drm/i915/i915_drv.h | 15 +-- drivers/gpu/drm/i915/i915_perf.c | 4 +- 15 files changed, 60 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c index 7f8b2d7713c7..6358a8b26172 100644 --- a/drivers/gpu/drm/i915/display/intel_fbc.c +++ b/drivers/gpu/drm/i915/display/intel_fbc.c @@ -1093,7 +1093,7 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state, /* Wa_14016291713 */ if ((IS_DISPLAY_VER(i915, 12, 13) || -IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && +IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) && crtc_state->has_psr) { plane_state->no_fbc_reason = "PSR1 enabled (Wa_14016291713)"; return 0; diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c index f7608d363634..8c3158b188ef 100644 --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c @@ -92,7 +92,7 @@ int intel_pmdemand_init(struct drm_i915_private *i915) _state->base, _pmdemand_funcs); - if (IS_MTL_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) + if (IS_METEORLAKE_DISPLAY_STEP(i915, STEP_A0, STEP_C0)) /* Wa_14016740474 */ intel_de_rmw(i915, XELPD_CHICKEN_DCPR_3, 0, DMD_RSP_TIMEOUT_DISABLE); diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 62151abe4748..ecd4e36119b2 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -1247,7 +1247,7 @@ static void wm_optimization_wa(struct intel_dp *intel_dp, bool set_wa_bit = false; /* Wa_14015648006 */ - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || + if (IS_METEORLAKE_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) || IS_DISPLAY_VER(dev_priv, 11, 13)) set_wa_bit |= crtc_state->wm_level_disabled; @@ -1320,7 +1320,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, * All supported adlp panels have 1-based X granularity, this may * cause issues if non-supported panels are used. */ - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) + if (IS_METEORLAKE_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) intel_de_rmw(dev_priv, MTL_CHICKEN_TRANS(cpu_transcoder), 0, ADLP_1_BASED_X_GRANULARITY); else if (IS_ALDERLAKE_P(dev_priv)) @@ -1328,7 +1328,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, ADLP_1_BASED_X_GRANULARITY); /* Wa_16012604467:adlp,mtl[a0,b0] */ - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) + if (IS_METEORLAKE_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) intel_de_rmw(dev_priv, MTL_CLKGATE_DIS_TRANS(cpu_transcoder), 0, MTL_CLKGATE_DIS_TRANS_DMASC_GATING_DIS); @@ -1489,7 +1489,7 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) if (intel_dp->psr.psr2_enabled) { /* Wa_16012604467:adlp,mtl[a0,b0] */ - if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) + if (IS_METEORLAKE_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0))