On Wed, Aug 23, 2023 at 12:44:56PM -0700, Matt Roper wrote:
> On Wed, Aug 23, 2023 at 10:07:19AM -0700, Lucas De Marchi wrote:
> > From: Ravi Kumar Vodapalli <ravi.kumar.vodapa...@intel.com>
> >
> > Add Display Power Well for LNL platform, mostly it is same as MTL
> > platform so reused the code
> >
> > Changes are:
> > 1. AUX_CH_CTL and AUX_CH_DATA1 are different from MTL so added extra
> > logic xelpdp_aux_power_well_ops functions.
> > 2. PGPICA1 contains type-C capable port slices which requires the well
> > to power powered up, so added new power well definition for PGPICA1
> >
> > BSpec: 68886
> > Signed-off-by: Ravi Kumar Vodapalli <ravi.kumar.vodapa...@intel.com>
> > Signed-off-by: Gustavo Sousa <gustavo.so...@intel.com>
> > Signed-off-by: Lucas De Marchi <lucas.demar...@intel.com>
> > ---
> > .../i915/display/intel_display_power_map.c | 36 ++++++++++-
> > .../i915/display/intel_display_power_well.c | 63 ++++++++++++++++++-
> > .../i915/display/intel_display_power_well.h | 1 +
> > .../gpu/drm/i915/display/intel_dp_aux_regs.h | 27 ++++++++
> > 4 files changed, 123 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > index 5ad04cd42c15..cef3b313c9f5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> > @@ -1545,6 +1545,38 @@ static const struct i915_power_well_desc_list
> > xelpdp_power_wells[] = {
> > I915_PW_DESCRIPTORS(xelpdp_power_wells_main),
> > };
> >
> > +I915_DECL_PW_DOMAINS(xe2lpd_pwdoms_pica_tc,
> > + POWER_DOMAIN_PORT_DDI_LANES_TC1,
> > + POWER_DOMAIN_PORT_DDI_LANES_TC2,
> > + POWER_DOMAIN_PORT_DDI_LANES_TC3,
> > + POWER_DOMAIN_PORT_DDI_LANES_TC4,
> > + POWER_DOMAIN_AUX_USBC1,
> > + POWER_DOMAIN_AUX_USBC2,
> > + POWER_DOMAIN_AUX_USBC3,
> > + POWER_DOMAIN_AUX_USBC4,
> > + POWER_DOMAIN_AUX_TBT1,
> > + POWER_DOMAIN_AUX_TBT2,
> > + POWER_DOMAIN_AUX_TBT3,
> > + POWER_DOMAIN_AUX_TBT4,
> > + POWER_DOMAIN_INIT);
> > +
> > +static const struct i915_power_well_desc xe2lpd_power_wells_pica[] = {
> > + {
> > + .instances = &I915_PW_INSTANCES(I915_PW("PICA_TC",
> > + &xe2lpd_pwdoms_pica_tc,
> > + .id = DISP_PW_ID_NONE),
> > + ),
> > + .ops = &xe2lpd_pica_power_well_ops,
> > + },
> > +};
> > +
> > +static const struct i915_power_well_desc_list xe2lpd_power_wells[] = {
> > + I915_PW_DESCRIPTORS(i9xx_power_wells_always_on),
> > + I915_PW_DESCRIPTORS(icl_power_wells_pw_1),
> > + I915_PW_DESCRIPTORS(xelpdp_power_wells_main),
> > + I915_PW_DESCRIPTORS(xe2lpd_power_wells_pica),
> > +};
>
> Are we missing a "dc_off" power well here? This patch might have
> originally been written before we separated dc_off out from main.
>
> Assuming the DC state requirements are the same for Xe2_LPD as they were
> for Xe_LPD and Xe_LPD+ (I haven't checked), then adding
>
> I915_PW_DESCRIPTORS(xelpd_power_wells_dc_off),
>
> immediately before xelpdp_power_wells_main should be sufficient.
Okay, this is actually taken care of by the next patch in the series.
Disregard this comment.
Matt
>
> > +
> > static void init_power_well_domains(const struct i915_power_well_instance
> > *inst,
> > struct i915_power_well *power_well)
> > {
> > @@ -1652,7 +1684,9 @@ int intel_display_power_map_init(struct
> > i915_power_domains *power_domains)
> > return 0;
> > }
> >
> > - if (DISPLAY_VER(i915) >= 14)
> > + if (DISPLAY_VER(i915) >= 20)
> > + return set_power_wells(power_domains, xe2lpd_power_wells);
> > + else if (DISPLAY_VER(i915) >= 14)
> > return set_power_wells(power_domains, xelpdp_power_wells);
> > else if (IS_DG2(i915))
> > return set_power_wells(power_domains, xehpd_power_wells);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> > b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> > index 916009894d89..e1fb0bd7b3bf 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
> > @@ -1795,7 +1795,11 @@ static void xelpdp_aux_power_well_enable(struct
> > drm_i915_private *dev_priv,
> > {
> > enum aux_ch aux_ch =
> > i915_power_well_instance(power_well)->xelpdp.aux_ch;
> >
> > - intel_de_rmw(dev_priv, XELPDP_DP_AUX_CH_CTL(aux_ch),
> > + i915_reg_t aux_ch_ctl = DISPLAY_VER(dev_priv) >= 20 ?
> > + XE2LPD_DP_AUX_CH_CTL(aux_ch) :
> > + XELPDP_DP_AUX_CH_CTL(aux_ch);
> > +
> > + intel_de_rmw(dev_priv, aux_ch_ctl,
> > XELPDP_DP_AUX_CH_CTL_POWER_REQUEST,
> > XELPDP_DP_AUX_CH_CTL_POWER_REQUEST);
> >
> > @@ -1813,7 +1817,11 @@ static void xelpdp_aux_power_well_disable(struct
> > drm_i915_private *dev_priv,
> > {
> > enum aux_ch aux_ch =
> > i915_power_well_instance(power_well)->xelpdp.aux_ch;
> >
> > - intel_de_rmw(dev_priv, XELPDP_DP_AUX_CH_CTL(aux_ch),
> > + i915_reg_t aux_ch_ctl = DISPLAY_VER(dev_priv) >= 20 ?
> > + XE2LPD_DP_AUX_CH_CTL(aux_ch) :
> > + XELPDP_DP_AUX_CH_CTL(aux_ch);
> > +
> > + intel_de_rmw(dev_priv, aux_ch_ctl,
> > XELPDP_DP_AUX_CH_CTL_POWER_REQUEST,
> > 0);
> > usleep_range(10, 30);
> > @@ -1823,11 +1831,53 @@ static bool xelpdp_aux_power_well_enabled(struct
> > drm_i915_private *dev_priv,
> > struct i915_power_well *power_well)
> > {
> > enum aux_ch aux_ch =
> > i915_power_well_instance(power_well)->xelpdp.aux_ch;
> > + i915_reg_t aux_ch_ctl;
> >
> > - return intel_de_read(dev_priv, XELPDP_DP_AUX_CH_CTL(aux_ch)) &
> > + aux_ch_ctl = DISPLAY_VER(dev_priv) >= 20 ?
> > + XE2LPD_DP_AUX_CH_CTL(aux_ch) :
> > + XELPDP_DP_AUX_CH_CTL(aux_ch);
> > +
> > + return intel_de_read(dev_priv, aux_ch_ctl) &
> > XELPDP_DP_AUX_CH_CTL_POWER_STATUS;
> > }
> >
> > +static void xe2lpd_pica_power_well_enable(struct drm_i915_private
> > *dev_priv,
> > + struct i915_power_well *power_well)
> > +{
> > + intel_de_rmw(dev_priv, XE2LPD_PICA_PW_CTL,
> > + XE2LPD_PICA_CTL_POWER_REQUEST,
> > + XE2LPD_PICA_CTL_POWER_REQUEST);
> > +
> > + if (intel_de_wait_for_set(dev_priv, XE2LPD_PICA_PW_CTL,
> > + XE2LPD_PICA_CTL_POWER_STATUS, 1)) {
> > + drm_dbg_kms(&dev_priv->drm, "pica power well enable timeout\n");
> > +
> > + drm_WARN(&dev_priv->drm, 1, "Power well PICA timeout when
> > enabled");
> > + }
> > +}
> > +
> > +static void xe2lpd_pica_power_well_disable(struct drm_i915_private
> > *dev_priv,
> > + struct i915_power_well *power_well)
> > +{
> > + intel_de_rmw(dev_priv, XE2LPD_PICA_PW_CTL,
> > + XE2LPD_PICA_CTL_POWER_REQUEST,
> > + 0);
> > +
> > + if (intel_de_wait_for_clear(dev_priv, XE2LPD_PICA_PW_CTL,
> > + XE2LPD_PICA_CTL_POWER_STATUS, 1)) {
> > + drm_dbg_kms(&dev_priv->drm, "pica power well disable
> > timeout\n");
> > +
> > + drm_WARN(&dev_priv->drm, 1, "Power well PICA timeout when
> > disabled");
> > + }
> > +}
> > +
> > +static bool xe2lpd_pica_power_well_enabled(struct drm_i915_private
> > *dev_priv,
> > + struct i915_power_well *power_well)
> > +{
> > + return intel_de_read(dev_priv, XE2LPD_PICA_PW_CTL) &
> > + XE2LPD_PICA_CTL_POWER_STATUS;
> > +}
> > +
> > const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
> > .sync_hw = i9xx_power_well_sync_hw_noop,
> > .enable = i9xx_always_on_power_well_noop,
> > @@ -1947,3 +1997,10 @@ const struct i915_power_well_ops
> > xelpdp_aux_power_well_ops = {
> > .disable = xelpdp_aux_power_well_disable,
> > .is_enabled = xelpdp_aux_power_well_enabled,
> > };
> > +
> > +const struct i915_power_well_ops xe2lpd_pica_power_well_ops = {
> > + .sync_hw = i9xx_power_well_sync_hw_noop,
> > + .enable = xe2lpd_pica_power_well_enable,
> > + .disable = xe2lpd_pica_power_well_disable,
> > + .is_enabled = xe2lpd_pica_power_well_enabled,
> > +};
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.h
> > b/drivers/gpu/drm/i915/display/intel_display_power_well.h
> > index a8736588314d..9357a9a73c06 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power_well.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.h
> > @@ -176,5 +176,6 @@ extern const struct i915_power_well_ops
> > icl_aux_power_well_ops;
> > extern const struct i915_power_well_ops icl_ddi_power_well_ops;
> > extern const struct i915_power_well_ops tgl_tc_cold_off_ops;
> > extern const struct i915_power_well_ops xelpdp_aux_power_well_ops;
> > +extern const struct i915_power_well_ops xe2lpd_pica_power_well_ops;
> >
> > #endif
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_regs.h
> > b/drivers/gpu/drm/i915/display/intel_dp_aux_regs.h
> > index 5185345277c7..d855f3730381 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_regs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_regs.h
> > @@ -83,4 +83,31 @@
> > #define DP_AUX_CH_CTL_SYNC_PULSE_SKL_MASK REG_GENMASK(4, 0) /*
> > skl+ */
> > #define DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)
> > REG_FIELD_PREP(DP_AUX_CH_CTL_SYNC_PULSE_SKL_MASK, (c) - 1)
> >
> > +#define _XE2LPD_DPA_AUX_CH_CTL 0x16FA10
> > +#define _XE2LPD_DPB_AUX_CH_CTL 0x16FC10
> > +#define _XE2LPD_DPA_AUX_CH_DATA1 0x16FA14
> > +#define _XE2LPD_DPB_AUX_CH_DATA1 0x16FC14
>
> We're generally trying to standardize on lowercase hex for register
> offsets these days.
>
> > +#define XE2LPD_DP_AUX_CH_CTL(aux_ch) _MMIO(_PICK(aux_ch, \
> > + _XE2LPD_DPA_AUX_CH_CTL, \
> > + _XE2LPD_DPB_AUX_CH_CTL, \
> > + 0, /* port/aux_ch C is
> > non-existent */ \
> > +
> > _XELPDP_USBC1_AUX_CH_CTL, \
> > +
> > _XELPDP_USBC2_AUX_CH_CTL, \
> > +
> > _XELPDP_USBC3_AUX_CH_CTL, \
> > +
> > _XELPDP_USBC4_AUX_CH_CTL))
> > +#define XE2LPD_DP_AUX_CH_DATA(aux_ch, i) _MMIO(_PICK(aux_ch, \
> > +
> > _XE2LPD_DPA_AUX_CH_DATA1, \
> > +
> > _XE2LPD_DPB_AUX_CH_DATA1, \
> > + 0, /* port/aux_ch C is
> > non-existent */ \
> > +
> > _XELPDP_USBC1_AUX_CH_DATA1, \
> > +
> > _XELPDP_USBC2_AUX_CH_DATA1, \
> > +
> > _XELPDP_USBC3_AUX_CH_DATA1, \
> > +
> > _XELPDP_USBC4_AUX_CH_DATA1) + (i) * 4)
>
> It looks like these PICA registers are following the same general layout
> change as the ones we modified a couple patches ago ("drm/i915/xe2lpd:
> Move registers to PICA"). We should probably handle these the same way
> for consistency (or maybe even squash the register movement here into
> that previous patch?).
>
> > +
> > +/* PICA Power Well Control register for Xe2 platforms*/
> > +#define XE2LPD_PICA_PW_CTL _MMIO(0x16FE04)
> > +
>
> Unwanted blank line?
>
>
> Matt
>
> > +#define XE2LPD_PICA_CTL_POWER_REQUEST BIT(31)
> > +#define XE2LPD_PICA_CTL_POWER_STATUS BIT(30)
> > +
> > #endif /* __INTEL_DP_AUX_REGS_H__ */
> > --
> > 2.40.1
> >
>
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
