Skip to content

Commit 0b385be

Browse files
vsyrjalajlahtine-intel
authored andcommitted
drm/i915: Don't explode when the dig port we don't have an AUX CH
The icl+ power well code currently assumes that every AUX power well maps to an encoder which is using said power well. That is by no menas guaranteed as we: - only register encoders for ports declared in the VBT - combo PHY HDMI-only encoder no longer get an AUX CH since commit 9856308 ("drm/i915: Only populate aux_ch if really needed") However we have places such as intel_power_domains_sanitize_state() that blindly traverse all the possible power wells. So these bits of code may very well encounbter an aux power well with no associated encoder. In this particular case the BIOS seems to have left one AUX power well enabled even though we're dealing with a HDMI only encoder on a combo PHY. We then proceed to turn off said power well and explode when we can't find a matching encoder. As a short term fix we should be able to just skip the PHY related parts of the power well programming since we know this situation can only happen with combo PHYs. Another option might be to go back to always picking an AUX CH for all encoders. However I'm a bit wary about that since we might in theory end up conflicting with the VBT AUX CH assignment. Also that wouldn't help with encoders not declared in the VBT, should we ever need to poke the corresponding power wells. Longer term we need to figure out what the actual relationship is between the PHY vs. AUX CH vs. AUX power well. Currently this is entirely unclear. Cc: [email protected] Fixes: 9856308 ("drm/i915: Only populate aux_ch if really needed") Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10184 Signed-off-by: Ville Syrjälä <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] Reviewed-by: Imre Deak <[email protected]> (cherry picked from commit 6a8c66b) Signed-off-by: Joonas Lahtinen <[email protected]>
1 parent d6a209d commit 0b385be

File tree

1 file changed

+14
-3
lines changed

1 file changed

+14
-3
lines changed

drivers/gpu/drm/i915/display/intel_display_power_well.c

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,14 @@ static enum phy icl_aux_pw_to_phy(struct drm_i915_private *i915,
246246
enum aux_ch aux_ch = icl_aux_pw_to_ch(power_well);
247247
struct intel_digital_port *dig_port = aux_ch_to_digital_port(i915, aux_ch);
248248

249-
return intel_port_to_phy(i915, dig_port->base.port);
249+
/*
250+
* FIXME should we care about the (VBT defined) dig_port->aux_ch
251+
* relationship or should this be purely defined by the hardware layout?
252+
* Currently if the port doesn't appear in the VBT, or if it's declared
253+
* as HDMI-only and routed to a combo PHY, the encoder either won't be
254+
* present at all or it will not have an aux_ch assigned.
255+
*/
256+
return dig_port ? intel_port_to_phy(i915, dig_port->base.port) : PHY_NONE;
250257
}
251258

252259
static void hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,
@@ -414,7 +421,8 @@ icl_combo_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
414421

415422
intel_de_rmw(dev_priv, regs->driver, 0, HSW_PWR_WELL_CTL_REQ(pw_idx));
416423

417-
if (DISPLAY_VER(dev_priv) < 12)
424+
/* FIXME this is a mess */
425+
if (phy != PHY_NONE)
418426
intel_de_rmw(dev_priv, ICL_PORT_CL_DW12(phy),
419427
0, ICL_LANE_ENABLE_AUX);
420428

@@ -437,7 +445,10 @@ icl_combo_phy_aux_power_well_disable(struct drm_i915_private *dev_priv,
437445

438446
drm_WARN_ON(&dev_priv->drm, !IS_ICELAKE(dev_priv));
439447

440-
intel_de_rmw(dev_priv, ICL_PORT_CL_DW12(phy), ICL_LANE_ENABLE_AUX, 0);
448+
/* FIXME this is a mess */
449+
if (phy != PHY_NONE)
450+
intel_de_rmw(dev_priv, ICL_PORT_CL_DW12(phy),
451+
ICL_LANE_ENABLE_AUX, 0);
441452

442453
intel_de_rmw(dev_priv, regs->driver, HSW_PWR_WELL_CTL_REQ(pw_idx), 0);
443454

0 commit comments

Comments
 (0)