[Intel-xe] [Intel-gfx] [PATCH v3 16/29] drm/i915/xe2lpd: Add display power well
Lucas De Marchi
lucas.demarchi at intel.com
Tue Sep 12 16:30:50 UTC 2023
On Tue, Sep 12, 2023 at 09:04:17AM -0700, Matt Roper wrote:
>On Mon, Sep 11, 2023 at 09:48:24PM -0700, Lucas De Marchi wrote:
>> From: Ravi Kumar Vodapalli <ravi.kumar.vodapalli at intel.com>
>>
>> Add Display Power Well for LNL platform. It's mostly the same as MTL
>
>It might be better to say "Xe2_LPD" and "Xe_LPD+" instead of LNL/MTL?
ok
>
>> platform so reuse the code. PGPICA1 contains type-C capable port slices
>> which requires the well to power powered up, so add new power well
>> definition for it.
>
>Maybe add a sentence noting that the dc_off fake powerwell will be added
>in a follow-up patch so that people don't think it was overlooked here?
ok
>
>>
>> BSpec: 68886
>> Signed-off-by: Ravi Kumar Vodapalli <ravi.kumar.vodapalli at intel.com>
>> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>> .../i915/display/intel_display_power_map.c | 36 ++++++++++++++-
>> .../i915/display/intel_display_power_well.c | 44 +++++++++++++++++++
>> .../i915/display/intel_display_power_well.h | 1 +
>> .../gpu/drm/i915/display/intel_dp_aux_regs.h | 5 +++
>> 4 files changed, 85 insertions(+), 1 deletion(-)
>>
>> 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 0f1b93d139ca..31c11586ede5 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
>> @@ -1536,6 +1536,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),
>> +};
>> +
>> static void init_power_well_domains(const struct i915_power_well_instance *inst,
>> struct i915_power_well *power_well)
>> {
>> @@ -1643,7 +1675,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 ca0714eba17a..adfe9901bde4 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
>> @@ -1833,6 +1833,43 @@ static bool xelpdp_aux_power_well_enabled(struct drm_i915_private *dev_priv,
>> 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);
>
>Do we need rmw here? Bit 31 is the only writable bit in the register
>(bit 30 is RO and can't be clobbered), so a simple write should suffice?
>Ditto on the disable below.
>
>> +
>> + 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,
>> @@ -1952,3 +1989,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 b4e96baae25a..c869f5661530 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_regs.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_regs.h
>> @@ -86,4 +86,9 @@
>> _XELPDP_DP_AUX_CH_DATA(__xe2lpd_aux_ch_idx(aux_ch), i) : \
>> _XELPDP_DP_AUX_CH_DATA(aux_ch, i))
>>
>> +/* PICA Power Well Control */
>> +#define XE2LPD_PICA_PW_CTL _MMIO(0x16fe04)
>
>Is this the right header for this? It doesn't seem directly related to
>DP AUX.
In Xe2_LPD, those AUX registers are also
I had the same question myself while rebasing this patch as this is
not related to DP. However the register itself seems to have the same
functionality as the other registers above with power request/status.
And if you look at bspec 69009, all of them are under pica for Xe2_LPD,
too.
I'm not sure what was the criteria for the split of this DP header.
It's clearer for things like gt, engine, snps phy, etc, but this one
seems to have been more or less arbitrary.
Any suggestion for a better place? A new display/intel_pica_regs.h
maybe? That may not be as future proof in the cases register move from
one place to the other like happened in Xe2_LPD: see bspec 69009. All
the PORT_BUF_CTL*, PORT_HOTPLUG_CTL, etc should then be in this header,
which doesn't match previous platforms.
If instead of matching HW we try to match where it's used in SW, then
maybe a intel_display_power_well_regs.h, but that too doesn't match
previous platforms.
+Jani
Lucas De Marchi
>
>
>Matt
>
>> +#define XE2LPD_PICA_CTL_POWER_REQUEST REG_BIT(31)
>> +#define XE2LPD_PICA_CTL_POWER_STATUS REG_BIT(30)
>> +
>> #endif /* __INTEL_DP_AUX_REGS_H__ */
>> --
>> 2.40.1
>>
>
>--
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation
More information about the Intel-xe
mailing list