RE: [PATCH 1/3] drm/i915/pch: Make LPT LP a dedicated PCH type

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 11 Feb 2025, "Garg, Nemesa" <nemesa.garg@intel.com> wrote:
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani
>> Nikula
>> Sent: Friday, February 7, 2025 3:48 PM
>> To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula@intel.com>
>> Subject: [PATCH 1/3] drm/i915/pch: Make LPT LP a dedicated PCH type
>>
>> Add PCH type PCH_LPT_LP and rename PCH_LPT to PCH_LPT_H for
>> consistency. Keep the existing HAS_PCH_LPT*() macros, but express them in
>> terms of the PCH types instead of looking at the device IDs directly. This
>> makes the PCH checks independent of the PCH device IDs.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> LGTM,
> Reviewed-by: Nemesa Garg <nemesa.garg@intel.com>

Thanks for the review, series pushed to din.

BR,
Jani.

>> ---
>>  drivers/gpu/drm/i915/display/intel_backlight.c |  2 +-
>>  drivers/gpu/drm/i915/soc/intel_pch.c           |  8 ++++----
>>  drivers/gpu/drm/i915/soc/intel_pch.h           | 14 ++++++--------
>>  3 files changed, 11 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c
>> b/drivers/gpu/drm/i915/display/intel_backlight.c
>> index 477b21af71a4..178dc6c8de80 100644
>> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
>> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
>> @@ -1829,7 +1829,7 @@ void intel_backlight_init_funcs(struct intel_panel
>> *panel)
>>               panel->backlight.pwm_funcs = &bxt_pwm_funcs;
>>       } else if (INTEL_PCH_TYPE(i915) >= PCH_CNP) {
>>               panel->backlight.pwm_funcs = &cnp_pwm_funcs;
>> -     } else if (INTEL_PCH_TYPE(i915) >= PCH_LPT) {
>> +     } else if (INTEL_PCH_TYPE(i915) >= PCH_LPT_H) {
>>               if (HAS_PCH_LPT(i915))
>>                       panel->backlight.pwm_funcs = &lpt_pwm_funcs;
>>               else
>> diff --git a/drivers/gpu/drm/i915/soc/intel_pch.c
>> b/drivers/gpu/drm/i915/soc/intel_pch.c
>> index 9f7c9dbc178e..0370ccb6943c 100644
>> --- a/drivers/gpu/drm/i915/soc/intel_pch.c
>> +++ b/drivers/gpu/drm/i915/soc/intel_pch.c
>> @@ -33,14 +33,14 @@ intel_pch_type(const struct drm_i915_private
>> *dev_priv, unsigned short id)
>>                           !IS_HASWELL(dev_priv) &&
>> !IS_BROADWELL(dev_priv));
>>               drm_WARN_ON(&dev_priv->drm,
>>                           IS_HASWELL_ULT(dev_priv) ||
>> IS_BROADWELL_ULT(dev_priv));
>> -             return PCH_LPT;
>> +             return PCH_LPT_H;
>>       case INTEL_PCH_LPT_LP_DEVICE_ID_TYPE:
>>               drm_dbg_kms(&dev_priv->drm, "Found LynxPoint LP
>> PCH\n");
>>               drm_WARN_ON(&dev_priv->drm,
>>                           !IS_HASWELL(dev_priv) &&
>> !IS_BROADWELL(dev_priv));
>>               drm_WARN_ON(&dev_priv->drm,
>>                           !IS_HASWELL_ULT(dev_priv) &&
>> !IS_BROADWELL_ULT(dev_priv));
>> -             return PCH_LPT;
>> +             return PCH_LPT_LP;
>>       case INTEL_PCH_WPT_DEVICE_ID_TYPE:
>>               drm_dbg_kms(&dev_priv->drm, "Found WildcatPoint
>> PCH\n");
>>               drm_WARN_ON(&dev_priv->drm,
>> @@ -48,7 +48,7 @@ intel_pch_type(const struct drm_i915_private *dev_priv,
>> unsigned short id)
>>               drm_WARN_ON(&dev_priv->drm,
>>                           IS_HASWELL_ULT(dev_priv) ||
>> IS_BROADWELL_ULT(dev_priv));
>>               /* WPT is LPT compatible */
>> -             return PCH_LPT;
>> +             return PCH_LPT_H;
>>       case INTEL_PCH_WPT_LP_DEVICE_ID_TYPE:
>>               drm_dbg_kms(&dev_priv->drm, "Found WildcatPoint LP
>> PCH\n");
>>               drm_WARN_ON(&dev_priv->drm,
>> @@ -56,7 +56,7 @@ intel_pch_type(const struct drm_i915_private *dev_priv,
>> unsigned short id)
>>               drm_WARN_ON(&dev_priv->drm,
>>                           !IS_HASWELL_ULT(dev_priv) &&
>> !IS_BROADWELL_ULT(dev_priv));
>>               /* WPT is LPT compatible */
>> -             return PCH_LPT;
>> +             return PCH_LPT_LP;
>>       case INTEL_PCH_SPT_DEVICE_ID_TYPE:
>>               drm_dbg_kms(&dev_priv->drm, "Found SunrisePoint
>> PCH\n");
>>               drm_WARN_ON(&dev_priv->drm,
>> diff --git a/drivers/gpu/drm/i915/soc/intel_pch.h
>> b/drivers/gpu/drm/i915/soc/intel_pch.h
>> index 89e89ede265d..bc926df14c45 100644
>> --- a/drivers/gpu/drm/i915/soc/intel_pch.h
>> +++ b/drivers/gpu/drm/i915/soc/intel_pch.h
>> @@ -19,7 +19,8 @@ enum intel_pch {
>>       PCH_NONE = 0,   /* No PCH present */
>>       PCH_IBX,        /* Ibexpeak PCH */
>>       PCH_CPT,        /* Cougarpoint/Pantherpoint PCH */
>> -     PCH_LPT,        /* Lynxpoint/Wildcatpoint PCH */
>> +     PCH_LPT_H,      /* Lynxpoint/Wildcatpoint H PCH */
>> +     PCH_LPT_LP,     /* Lynxpoint/Wildcatpoint LP PCH */
>>       PCH_SPT,        /* Sunrisepoint/Kaby Lake PCH */
>>       PCH_CNP,        /* Cannon/Comet Lake PCH */
>>       PCH_ICP,        /* Ice Lake/Jasper Lake PCH */
>> @@ -72,13 +73,10 @@ enum intel_pch {
>>  #define HAS_PCH_ICP(dev_priv)
>>       (INTEL_PCH_TYPE(dev_priv) == PCH_ICP)
>>  #define HAS_PCH_CNP(dev_priv)
>>       (INTEL_PCH_TYPE(dev_priv) == PCH_CNP)
>>  #define HAS_PCH_SPT(dev_priv)
>>       (INTEL_PCH_TYPE(dev_priv) == PCH_SPT)
>> -#define HAS_PCH_LPT(dev_priv)
>>       (INTEL_PCH_TYPE(dev_priv) == PCH_LPT)
>> -#define HAS_PCH_LPT_LP(dev_priv) \
>> -     (INTEL_PCH_ID(dev_priv) == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE ||
>> \
>> -      INTEL_PCH_ID(dev_priv) == INTEL_PCH_WPT_LP_DEVICE_ID_TYPE)
>> -#define HAS_PCH_LPT_H(dev_priv) \
>> -     (INTEL_PCH_ID(dev_priv) == INTEL_PCH_LPT_DEVICE_ID_TYPE || \
>> -      INTEL_PCH_ID(dev_priv) == INTEL_PCH_WPT_DEVICE_ID_TYPE)
>> +#define HAS_PCH_LPT_H(dev_priv)
>>       (INTEL_PCH_TYPE(dev_priv) == PCH_LPT_H)
>> +#define HAS_PCH_LPT_LP(dev_priv)             (INTEL_PCH_TYPE(dev_priv)
>> == PCH_LPT_LP)
>> +#define HAS_PCH_LPT(dev_priv)
>>       (INTEL_PCH_TYPE(dev_priv) == PCH_LPT_H || \
>> +                                              INTEL_PCH_TYPE(dev_priv)
>> == PCH_LPT_LP)
>>  #define HAS_PCH_CPT(dev_priv)
>>       (INTEL_PCH_TYPE(dev_priv) == PCH_CPT)
>>  #define HAS_PCH_IBX(dev_priv)
>>       (INTEL_PCH_TYPE(dev_priv) == PCH_IBX)
>>  #define HAS_PCH_NOP(dev_priv)
>>       (INTEL_PCH_TYPE(dev_priv) == PCH_NOP)
>> --
>> 2.39.5
>

-- 
Jani Nikula, Intel