Re: [PATCH 00/11] Replace acronym with full platform name in defines.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Sent: Tuesday, June 20, 2023 9:31 AM
> To: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@xxxxxxxxx>; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Bhadane, Dnyaneshwar <dnyaneshwar.bhadane@xxxxxxxxx>; Srivatsa,
> Anusha <anusha.srivatsa@xxxxxxxxx>; Tvrtko Ursulin
> <tvrtko.ursulin@xxxxxxxxxxxxxxx>; Joonas Lahtinen
> <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Subject: Re:  [PATCH 00/11] Replace acronym with full platform name
> in defines.
> 
> On Thu, 15 Jun 2023, Dnyaneshwar Bhadane
> <dnyaneshwar.bhadane@xxxxxxxxx> wrote:
> > Replace all occurences of ADL with ALDERLAKE, TGL with TIGERLAKE, MTL
> > with METEORLAKE, RKL with ROCKETLAKE, JSL with JASPERLAKE, KBL with
> > KABYLAKE and SKL with SKYLAKE in platform and subplatform defines.
> > This way there is a consistent pattern to how platforms are referred.
> > While the change is minor and could be combined to have lesser
> > patches, splitting to per subpaltform for easier cherrypicks, if
> > needed.
> 
> First of all, I'll note that changes like these need maintainer acks before merging.
> Simple review for correctness is not enough!
> 
> While discussing this, there was perhaps a slight preference for moving towards
> acronyms for brevity instead of expanding all of them to full names. It can get a
> bit unwieldy.

Yes there was. The main reason being the acronym was more brief/compact. Having said that, thinking out loud it felt having platform defines have full name would make it more clear and readable?  For anyone new wanting to contribute, there will be less confusion.

> 
> For background, the reasons for having IS_<TLA>_DISPLAY_STEP() are
> two-fold: the steppings used to be platform specific, so it made sense to tie
> platform and stepping together, and IS_<LONG_NAME>_DISPLAY_STEP() was
> considered too long combined.
> 
> Nowadays, we've abstracted steppings in code to be independent of platforms,
> so we could use IS_<LONG_NAME>() && IS_DISPLAY_STEP(), and throw out all
> the IS_<TLA>_DISPLAY_STEP() macros. They're orthogonal things, and it actually
> bugs me to have so many platform specific wrappers for the combos.
> 
> If in addition we moved to acronyms, we could have IS_<TLA>() &&
> IS_DISPLAY_STEP(), and it would be pretty short and nice.
> 
> That said, all of these changes are a lot of churn, so I'd rather not make them
> lightly.
Agreed.

Anusha
> 
> BR,
> Jani.
> 
> 
> 
> 
> >
> > v2:
> >  - Fix the checkpatch warning.
> >
> > Anusha Srivatsa (5):
> >   drm/i915/adlp: s/ADLP/ALDERLAKE_P for display and graphics step
> >   drm/i915/rplp: s/ADLP/ALDERLAKE_P for RPLP defines
> >   drm/i915/adln: s/ADLP/ALDERLAKE_P in ADLN defines
> >   drm/i915/adls: s/ADLS/ALDERLAKE_S in platform and subplatform defines
> >   drm/i915/rplu: s/ADLP/ALDERLAKE_P in RPLU defines
> >
> > Dnyaneshwar Bhadane (6):
> >   drm/i915/TGL: s/TGL/TIGERLAKE for platform/subplatform defines
> >   drm/i915/MTL: s/MTL/METEORLAKE for platform/subplatform defines
> >   drm/i915/TGL: s/RKL/ROCKETLAKE for platform/subplatform defines
> >   drm/i915/JSL: s/JSL/JASPERLAKE for platform/subplatform defines
> >   drm/i915/KBL: s/KBL/KABYLAKE for platform/subplatform defines
> >   drm/i915/SKL: s/SKL/SKYLAKE for platform/subplatform defines
> >
> >  drivers/gpu/drm/i915/display/icl_dsi.c        |  4 +-
> >  drivers/gpu/drm/i915/display/intel_cdclk.c    |  8 +--
> >  .../gpu/drm/i915/display/intel_combo_phy.c    |  6 +-
> >  drivers/gpu/drm/i915/display/intel_ddi.c      |  6 +-
> >  .../drm/i915/display/intel_ddi_buf_trans.c    | 10 +--
> >  drivers/gpu/drm/i915/display/intel_display.c  |  6 +-
> >  .../drm/i915/display/intel_display_device.c   |  2 +-
> >  .../drm/i915/display/intel_display_power.c    |  2 +-
> >  drivers/gpu/drm/i915/display/intel_dp.c       |  2 +-
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 20 +++---
> >  drivers/gpu/drm/i915/display/intel_fbc.c      |  2 +-
> >  drivers/gpu/drm/i915/display/intel_hdmi.c     |  2 +-
> >  drivers/gpu/drm/i915/display/intel_pmdemand.c |  2 +-
> >  drivers/gpu/drm/i915/display/intel_psr.c      | 20 +++---
> >  .../drm/i915/display/skl_universal_plane.c    | 10 +--
> >  drivers/gpu/drm/i915/gem/i915_gem_object.c    |  2 +-
> >  drivers/gpu/drm/i915/gt/gen8_engine_cs.c      | 10 +--
> >  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_sseu.c          |  2 +-
> >  drivers/gpu/drm/i915/gt/intel_workarounds.c   | 54 ++++++++--------
> >  drivers/gpu/drm/i915/gt/uc/intel_guc.c        |  4 +-
> >  .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c   |  2 +-
> >  .../gpu/drm/i915/gt/uc/intel_guc_submission.c |  4 +-
> >  drivers/gpu/drm/i915/gt/uc/intel_uc.c         |  2 +-
> >  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      |  2 +-
> >  drivers/gpu/drm/i915/i915_drv.h               | 64 +++++++++----------
> >  drivers/gpu/drm/i915/i915_perf.c              |  4 +-
> >  drivers/gpu/drm/i915/intel_clock_gating.c     |  4 +-
> >  drivers/gpu/drm/i915/intel_step.c             | 10 +--
> >  drivers/gpu/drm/i915/soc/intel_pch.c          |  6 +-
> >  34 files changed, 143 insertions(+), 143 deletions(-)
> 
> --
> Jani Nikula, Intel Open Source Graphics Center




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux