Re: [Intel-gfx] [PATCH v2 6/7] drm/i915: rename DISP_STEPPING->DISPLAY_STEP and GT_STEPPING->GT_STEP

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Jani Nikula <jani.nikula@intel.com>,
	Lucas De Marchi <lucas.demarchi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 6/7] drm/i915: rename DISP_STEPPING->DISPLAY_STEP and GT_STEPPING->GT_STEP
Date: Fri, 05 Mar 2021 10:19:20 +0000	[thread overview]
Message-ID: <161493956025.15221.14472624414097711679@build.alporthouse.com> (raw)
In-Reply-To: <87tuq1etts.fsf@intel.com>

Quoting Jani Nikula (2021-02-24 08:46:55)
> On Tue, 23 Feb 2021, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> > On Tue, Feb 23, 2021 at 05:35:11PM +0200, Jani Nikula wrote:
> >>Matter of taste. STEP matches the enums.
> >>
> >>Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >>---
> >> drivers/gpu/drm/i915/display/intel_display_power.c |  2 +-
> >> drivers/gpu/drm/i915/display/intel_psr.c           |  4 ++--
> >> drivers/gpu/drm/i915/display/skl_universal_plane.c |  2 +-
> >> drivers/gpu/drm/i915/gt/intel_workarounds.c        | 10 +++++-----
> >> drivers/gpu/drm/i915/i915_drv.h                    | 10 +++++-----
> >> drivers/gpu/drm/i915/intel_device_info.c           |  2 +-
> >> drivers/gpu/drm/i915/intel_pm.c                    |  2 +-
> >> 7 files changed, 16 insertions(+), 16 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> >>index f00c1750febd..1f7b2700947a 100644
> >>--- a/drivers/gpu/drm/i915/display/intel_display_power.c
> >>+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> >>@@ -5349,7 +5349,7 @@ static void tgl_bw_buddy_init(struct drm_i915_private *dev_priv)
> >>
> >>      if (IS_ALDERLAKE_S(dev_priv) ||
> >>          IS_DG1_REVID(dev_priv, DG1_REVID_A0, DG1_REVID_A0) ||
> >>-         IS_TGL_DISP_STEPPING(dev_priv, STEP_A0, STEP_B0))
> >>+         IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0))
> >>              /* Wa_1409767108:tgl,dg1,adl-s */
> >>              table = wa_1409767108_buddy_page_masks;
> >>      else
> >>diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> >>index 7c6e561f86c1..da5084b54eb6 100644
> >>--- a/drivers/gpu/drm/i915/display/intel_psr.c
> >>+++ b/drivers/gpu/drm/i915/display/intel_psr.c
> >>@@ -548,7 +548,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
> >>
> >>      if (intel_dp->psr.psr2_sel_fetch_enabled) {
> >>              /* WA 1408330847 */
> >>-             if (IS_TGL_DISP_STEPPING(dev_priv, STEP_A0, STEP_A0) ||
> >>+             if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_A0) ||
> >
> > I always hated the DISP vs DISPLAY. It should be in the commit message.
> >
> > But if you are doing the s/STEPPING/STEP/, shouldn't the filename also use
> > step and all the functions/structs?
> 
> To be honest, the rename came as an afterthought, after Aditya (I think)
> added the STEP_X enums.
> 
> For me step everywhere sounds good, I wonder what the native speakers
> think.

IS_DISPLAY_STEPPING(STEP_X) is more flamboyant than
IS_DISPLAY_STEP(STEP_X), but we often make the concession for brevity
and in this case the consistency between macro and enum beats the
inconsistency in English. So STEP reads as a perfectly acceptable synonym
for STEPPING.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx