On Tue, Jul 18, 2023 at 11:53:17AM +0100, Tvrtko Ursulin wrote:

On 18/07/2023 09:11, Dnyaneshwar Bhadane wrote:
From: Anusha Srivatsa <[email protected]>

Driver refers to the platform Alderlake S as ADLS in places
and ALDERLAKE_S in some. Making the consistent change
to avoid confusion of the right naming convention for
the platform.

v2:
- Unrolled wrapper IS_ADLS_GRAPHICS_STEP and Replace
with IS_ALDERLAKE_S() && IS_GRAPHICS_STEP() (Jani/Tvrtko).

Signed-off-by: Anusha Srivatsa <[email protected]>
---
 drivers/gpu/drm/i915/display/intel_display_device.c | 2 +-
 drivers/gpu/drm/i915/gt/uc/intel_uc.c               | 2 +-
 drivers/gpu/drm/i915/i915_drv.h                     | 6 +++---
 drivers/gpu/drm/i915/intel_step.c                   | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c 
b/drivers/gpu/drm/i915/display/intel_display_device.c
index 3fd30e7f0062..252dd8446410 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.c
+++ b/drivers/gpu/drm/i915/display/intel_display_device.c
@@ -797,7 +797,7 @@ void intel_display_device_info_runtime_init(struct 
drm_i915_private *i915)
        enum pipe pipe;
        /* Wa_14011765242: adl-s A0,A1 */
-       if (IS_ADLS_DISPLAY_STEP(i915, STEP_A0, STEP_A2))
+       if (IS_ALDERLAKE_S(i915) && IS_DISPLAY_STEP(i915, STEP_A0, STEP_A2))
                for_each_pipe(i915, pipe)
                        display_runtime->num_scalers[pipe] = 0;
        else if (DISPLAY_VER(i915) >= 11) {
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 18250fb64bd8..eb28705b88bd 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -43,7 +43,7 @@ static void uc_expand_default_options(struct intel_uc *uc)
        }
        /* Intermediate platforms are HuC authentication only */
-       if (IS_ALDERLAKE_S(i915) && !IS_ADLS_RPLS(i915)) {
+       if (IS_ALDERLAKE_S(i915) && !IS_ALDERLAKE_S_RPLS(i915)) {
                i915->params.enable_guc = ENABLE_GUC_LOAD_HUC;
                return;
        }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 942004afdd2f..e15471bbad5a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -585,7 +585,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
        IS_SUBPLATFORM(i915, INTEL_DG2, INTEL_SUBPLATFORM_G11)
 #define IS_DG2_G12(i915) \
        IS_SUBPLATFORM(i915, INTEL_DG2, INTEL_SUBPLATFORM_G12)
-#define IS_ADLS_RPLS(i915) \
+#define IS_ALDERLAKE_S_RPLS(i915) \

I don't know what we should/could do with these Alderlake macros.. I mean all three of:

IS_ALDERLAKE_S_RPLS
IS_ALDERLAKE_P_RPLP
IS_ALDERLAKE_RPLU

It becomes too long to expand the TLA suffix..

Is there scope to turn this around and simplify in code like:

 IS_RAPTORLAKE_S/P/U ?

Not sure at all, just throwing out wild ideas.. There aren't many call sites for those three but despite that I don't see any easy cheats to tidy it.

I agree. I think "promoting" it like you mention is great and avoids the
super long macro. IS_ADLS_RPLS() was already ambiguous if you don't know
rpl is treated as a subplatform (is it ADLS || RPLS?). That's kind of
different than the DG2 case since there isn't a G10/G11/G12 platform by
itself.


thanks
Lucas De Marchi


Regards,

Tvrtko

        IS_SUBPLATFORM(i915, INTEL_ALDERLAKE_S, INTEL_SUBPLATFORM_RPL)
 #define IS_ALDERLAKE_P_N(i915) \
        IS_SUBPLATFORM(i915, INTEL_ALDERLAKE_P, INTEL_SUBPLATFORM_N)
@@ -664,11 +664,11 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
         IS_DISPLAY_STEP(__i915, since, until))
-#define IS_ADLS_DISPLAY_STEP(__i915, since, until) \
+#define IS_ALDERLAKE_S_DISPLAY_STEP(__i915, since, until) \
        (IS_ALDERLAKE_S(__i915) && \
         IS_DISPLAY_STEP(__i915, since, until))
-#define IS_ADLS_GRAPHICS_STEP(__i915, since, until) \
+#define IS_ALDERLAKE_GRAPHICS_STEP(__i915, since, until) \
        (IS_ALDERLAKE_S(__i915) && \
         IS_GRAPHICS_STEP(__i915, since, until))
diff --git a/drivers/gpu/drm/i915/intel_step.c 
b/drivers/gpu/drm/i915/intel_step.c
index 545102d14ba4..5904aa5640e1 100644
--- a/drivers/gpu/drm/i915/intel_step.c
+++ b/drivers/gpu/drm/i915/intel_step.c
@@ -201,7 +201,7 @@ void intel_step_init(struct drm_i915_private *i915)
        } else if (IS_ALDERLAKE_P(i915)) {
                revids = adlp_revids;
                size = ARRAY_SIZE(adlp_revids);
-       } else if (IS_ADLS_RPLS(i915)) {
+       } else if (IS_ALDERLAKE_S_RPLS(i915)) {
                revids = adls_rpls_revids;
                size = ARRAY_SIZE(adls_rpls_revids);
        } else if (IS_ALDERLAKE_S(i915)) {

Reply via email to