[PATCH] drm/i915/display: Add power well mapping for WCL
Borah, Chaitanya Kumar
chaitanya.kumar.borah at intel.com
Tue Jul 15 06:11:41 UTC 2025
> -----Original Message-----
> From: Deak, Imre <imre.deak at intel.com>
> Sent: Tuesday, July 15, 2025 2:21 AM
> To: Sousa, Gustavo <gustavo.sousa at intel.com>
> Cc: Borah, Chaitanya Kumar <chaitanya.kumar.borah at intel.com>; intel-
> gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org; Roper, Matthew D
> <matthew.d.roper at intel.com>; Bhadane, Dnyaneshwar
> <dnyaneshwar.bhadane at intel.com>; Dibin Moolakadan Subrahmanian
> <dibin.moolakadan.subrahmanian at intel.com>
> Subject: Re: [PATCH] drm/i915/display: Add power well mapping for WCL
>
> 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.
Correct this leads to a WARN_ON
[ 5.077874] xe 0000:00:02.0: [drm:hsw_wait_for_power_well_enable [xe]] PW_D power well enable timeout
[ 5.077937] ------------[ cut here ]------------
[ 5.078244] xe 0000:00:02.0: [drm] drm_WARN_ON(!timeout_expected)
>
> 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.
>
Thank you Imre, I will take a stab at it.
- Chaitanya
> > 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