Quoting Chaitanya Kumar Borah (2025-07-17 02:16:03-03:00)
>Some power wells are only relevant for certain display pipes. Add a check
>to ensure we only allocate and initialize power wells whose associated
>pipes are available on the platform.
>
>This avoids unnecessary mapping of power wells, particularly when platforms
>support a subset of pipes described in the power well descriptors.
>
>Suggested-by: Imre Deak <imre.d...@intel.com>
>Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.bo...@intel.com>
>---
> .../i915/display/intel_display_power_map.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 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 77268802b55e..ca73e4084354 100644
>--- a/drivers/gpu/drm/i915/display/intel_display_power_map.c
>+++ b/drivers/gpu/drm/i915/display/intel_display_power_map.c
>@@ -1748,6 +1748,16 @@ static void init_power_well_domains(const struct
>i915_power_well_instance *inst,
> for_each_power_well_instance_in_desc_list((_descs)->list,
> (_descs)->count, \
> (_desc), (_inst))
>
>+static bool
>+is_power_well_available(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))
According to irq_pipe_mask's documentation, that member contains a "mask
of pipes whose IRQ logic is backed by the pw". I think we are
overloading the meaning of that field with this logic.
* Do we have guarantees that irq_pipe_mask will always be associated
with the power well that powers the pipe?
* If the power well that has irq_pipe_mask is also used to power
something else than the pipes, we could have issues if pipes in that
mask are fused off.
I'm leaning more toward a solution that makes POWER_DOMAIN_INIT map to
POWER_DOMAIN_PIPE_* based on DISPLAY_RUNTIME_INFO(display)->pipe_mask. I
have some idea of how to do that without rewriting code to use a
hierarchical structure (which IMO would be ideal, but takes more
effort).
The idea is to, during runtime and initialization of the mapping, set
the bit respective to POWER_DOMAIN_INIT in each power well that has the
bit for POWER_DOMAIN_PIPE_* set for non-fused off pipes. That would
also require removing the POWER_DOMAIN_INIT from the static mapping for
power wells directly responsible for POWER_DOMAIN_PIPE_*.
--
Gustavo Sousa
>+ return false;
>+
>+ return true;
>+}
>+
> static int
> __set_power_wells(struct i915_power_domains *power_domains,
> const struct i915_power_well_desc_list *power_well_descs,
>@@ -1763,8 +1773,10 @@ __set_power_wells(struct i915_power_domains
>*power_domains,
> int power_well_count = 0;
> int plt_idx = 0;
>
>- for_each_power_well_instance(power_well_descs, power_well_descs_sz,
>desc_list, desc, inst)
>- power_well_count++;
>+ for_each_power_well_instance(power_well_descs, power_well_descs_sz,
>desc_list, desc, inst) {
>+ if (is_power_well_available(display, desc))
>+ power_well_count++;
>+ }
>
> power_domains->power_well_count = power_well_count;
> power_domains->power_wells =
>@@ -1778,6 +1790,9 @@ __set_power_wells(struct i915_power_domains
>*power_domains,
> struct i915_power_well *pw =
> &power_domains->power_wells[plt_idx];
> enum i915_power_well_id id = inst->id;
>
>+ if (!is_power_well_available(display, desc))
>+ continue;
>+
> pw->desc = desc;
> drm_WARN_ON(display->drm,
> overflows_type(inst - desc->instances->list,
> pw->instance_idx));
>--
>2.25.1
>
