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. 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 <[email protected]> >>--- >> .../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 >>
