[PATCH] drm/i915/display: Add power well mapping for WCL
Imre Deak
imre.deak at intel.com
Mon Jul 14 20:51:06 UTC 2025
On Mon, Jul 14, 2025 at 04:51:18PM -0300, Gustavo Sousa wrote:
> Quoting Gustavo Sousa (2025-07-14 12:07:50-03:00)
> >Quoting Chaitanya Kumar Borah (2025-07-14 02:53:44-03:00)
> >>WCL has 3 pipes, create power well mapping to reflect
> >
> >I believe display fuses should reflect that, right? I don't have a WCL
> >handy to check that, but I believe so.
> >
> >In that case, I believe a better solution would be to filter out the
> >power well mapping during initialization (__set_power_wells) based on
> >the availability of the associated hardware resource (display pipes in
> >this case).
>
> Hm... Thinking again, I think even this solution wouldn't be very
> robust. If, for some reason, a pipe-specific power well would need to
> power stuff other than the pipe itself, then a simple filtering based
> only on the info about fused-off pipes could be buggy.
>
> For some context, this patch come from the fact that we would get
> timeouts during display initialization, right? I think that comes from
> the fact that we do intel_display_power_get(display, POWER_DOMAIN_INIT)
> during initialization, which tries do get every power well that
> POWER_DOMAIN_INIT maps to, including pipe D's power well, which is not
> included in WCL.
Not registering a power well by using something like
static bool has_power_well(struct intel_display *display, const struct i915_power_well_desc *desc)
{
if (desc->irq_pipe_mask &&
!(desc->irq_pipe_mask & DISPLAY_RUNTIME_INFO(display)->pipe_mask))
return false;
return true;
}
in __set_power_wells() would prevent enabling the power well via any
domain.
> Ideally, we should just make sure that power domains for fused-off pipes
> are cleared in power mappings, but that doesn't really work because
> there is no real hierarchy of power domains encoded in our driver. It is
> just a flat structure that maps power domains directly to power wells.
>
> Now, I'm not sure how involved (or worth it) would it be to convert the
> existing infrastructure to a hierarchical one... I wonder if a simpler
> solution (but that does not involve hardcoding a new mapping) is
> feasible. Perhaps we should treat POWER_DOMAIN_INIT as something
> special?
>
> --
> Gustavo Sousa
>
> >
> >That way, we do not need to hardcode different mappings for different
> >variations of a display arch.
> >
> >--
> >Gustavo Sousa
> >
> >>HW. Rest remains similar to Xe3 power well configuration.
> >>
> >>Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah at intel.com>
> >>---
> >> .../i915/display/intel_display_power_map.c | 58 ++++++++++++++++++-
> >> 1 file changed, 57 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 77268802b55e..23c59b587f78 100644
> >>--- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
> >>+++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
> >>@@ -1717,6 +1717,60 @@ static const struct i915_power_well_desc_list xe3lpd_power_wells[] = {
> >> I915_PW_DESCRIPTORS(xe2lpd_power_wells_pica),
> >> };
> >>
> >>+static const struct i915_power_well_desc wcl_power_wells_main[] = {
> >>+ {
> >>+ .instances = &I915_PW_INSTANCES(
> >>+ I915_PW("PW_2", &xe3lpd_pwdoms_pw_2,
> >>+ .hsw.idx = ICL_PW_CTL_IDX_PW_2,
> >>+ .id = SKL_DISP_PW_2),
> >>+ ),
> >>+ .ops = &hsw_power_well_ops,
> >>+ .has_vga = true,
> >>+ .has_fuses = true,
> >>+ }, {
> >>+ .instances = &I915_PW_INSTANCES(
> >>+ I915_PW("PW_A", &xelpd_pwdoms_pw_a,
> >>+ .hsw.idx = XELPD_PW_CTL_IDX_PW_A),
> >>+ ),
> >>+ .ops = &hsw_power_well_ops,
> >>+ .irq_pipe_mask = BIT(PIPE_A),
> >>+ .has_fuses = true,
> >>+ }, {
> >>+ .instances = &I915_PW_INSTANCES(
> >>+ I915_PW("PW_B", &xe3lpd_pwdoms_pw_b,
> >>+ .hsw.idx = XELPD_PW_CTL_IDX_PW_B),
> >>+ ),
> >>+ .ops = &hsw_power_well_ops,
> >>+ .irq_pipe_mask = BIT(PIPE_B),
> >>+ .has_fuses = true,
> >>+ }, {
> >>+ .instances = &I915_PW_INSTANCES(
> >>+ I915_PW("PW_C", &xe3lpd_pwdoms_pw_c,
> >>+ .hsw.idx = XELPD_PW_CTL_IDX_PW_C),
> >>+ ),
> >>+ .ops = &hsw_power_well_ops,
> >>+ .irq_pipe_mask = BIT(PIPE_C),
> >>+ .has_fuses = true,
> >>+ }, {
> >>+ .instances = &I915_PW_INSTANCES(
> >>+ I915_PW("AUX_A", &icl_pwdoms_aux_a, .xelpdp.aux_ch = AUX_CH_A),
> >>+ I915_PW("AUX_B", &icl_pwdoms_aux_b, .xelpdp.aux_ch = AUX_CH_B),
> >>+ I915_PW("AUX_TC1", &xelpdp_pwdoms_aux_tc1, .xelpdp.aux_ch = AUX_CH_USBC1),
> >>+ I915_PW("AUX_TC2", &xelpdp_pwdoms_aux_tc2, .xelpdp.aux_ch = AUX_CH_USBC2),
> >>+ I915_PW("AUX_TC3", &xelpdp_pwdoms_aux_tc3, .xelpdp.aux_ch = AUX_CH_USBC3),
> >>+ I915_PW("AUX_TC4", &xelpdp_pwdoms_aux_tc4, .xelpdp.aux_ch = AUX_CH_USBC4),
> >>+ ),
> >>+ .ops = &xelpdp_aux_power_well_ops,
> >>+ },
> >>+};
> >>+static const struct i915_power_well_desc_list wcl_power_wells[] = {
> >>+ I915_PW_DESCRIPTORS(i9xx_power_wells_always_on),
> >>+ I915_PW_DESCRIPTORS(icl_power_wells_pw_1),
> >>+ I915_PW_DESCRIPTORS(xe3lpd_power_wells_dcoff),
> >>+ I915_PW_DESCRIPTORS(wcl_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)
> >> {
> >>@@ -1824,7 +1878,9 @@ int intel_display_power_map_init(struct i915_power_domains *power_domains)
> >> return 0;
> >> }
> >>
> >>- if (DISPLAY_VER(display) >= 30)
> >>+ if (DISPLAY_VERx100(display) == 3002)
> >>+ return set_power_wells(power_domains, wcl_power_wells);
> >>+ else if (DISPLAY_VER(display) >= 30)
> >> return set_power_wells(power_domains, xe3lpd_power_wells);
> >> else if (DISPLAY_VER(display) >= 20)
> >> return set_power_wells(power_domains, xe2lpd_power_wells);
> >>--
> >>2.25.1
> >>
More information about the Intel-xe
mailing list