On Tue, 2018-10-30 at 17:40 +0200, Imre Deak wrote: > Most of the AUX_CH_CTL flags are concerned with DP AUX transfer > parameters. As opposed to this the flag specifying the thunderbolt > vs. > non-thunderbolt mode of the port is not related to AUX transfers at > all > (rather it's repurposed to enable either TBT or non-TBT PHY HW > blocks). > The programming has to be done before enabling the corresponding AUX > power well, so make it part of the power well code. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108548 > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Reviewed-by: José Roberto de Souza <jose.souza@xxxxxxxxx> If respinning this patch please consider the comments bellow but nice catch. > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_runtime_pm.c | 69 > +++++++++++++++++++++++++++++---- > 2 files changed, 62 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h > index c57b701f72a7..dbf894835cb2 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -921,6 +921,7 @@ struct i915_power_well_desc { > /* The pw is backing the VGA functionality */ > bool has_vga:1; > bool has_fuses:1; > + bool is_tc_tbt; > } hsw; > }; > const struct i915_power_well_ops *ops; > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 5f5416eb9644..eed17440a4a7 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -465,6 +465,44 @@ icl_combo_phy_aux_power_well_disable(struct > drm_i915_private *dev_priv, > hsw_wait_for_power_well_disable(dev_priv, power_well); > } > > +#define ICL_AUX_PW_TO_CH(pw_idx) \ > + ((pw_idx) - ICL_PW_CTL_IDX_AUX_A + AUX_CH_A) > + > +static void > +icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv, > + struct i915_power_well *power_well) > +{ > + const struct i915_power_well_regs *regs = power_well->desc- > >hsw.regs; > + int pw_idx = power_well->desc->hsw.idx; > + enum aux_ch aux_ch = ICL_AUX_PW_TO_CH(pw_idx); > + u32 val; > + > + val = I915_READ(DP_AUX_CH_CTL(aux_ch)); > + val &= ~DP_AUX_CH_CTL_TBT_IO; > + if (power_well->desc->hsw.is_tc_tbt) > + val |= DP_AUX_CH_CTL_TBT_IO; > + I915_WRITE(DP_AUX_CH_CTL(aux_ch), val); > + > + val = I915_READ(regs->driver); > + I915_WRITE(regs->driver, val | HSW_PWR_WELL_CTL_REQ(pw_idx)); > + > + hsw_wait_for_power_well_enable(dev_priv, power_well); Minor but you could call hsw_power_well_enable() after write to DP_AUX_CH_CTL instead of duplicate code. > +} > + > +static void > +icl_tc_phy_aux_power_well_disable(struct drm_i915_private *dev_priv, > + struct i915_power_well *power_well) > +{ > + const struct i915_power_well_regs *regs = power_well->desc- > >hsw.regs; > + int pw_idx = power_well->desc->hsw.idx; > + u32 val; > + > + val = I915_READ(regs->driver); > + I915_WRITE(regs->driver, val & ~HSW_PWR_WELL_CTL_REQ(pw_idx)); > + > + hsw_wait_for_power_well_disable(dev_priv, power_well); > +} Minor too you could use the hsw_power_well_disable() instead of duplicate code. > + > /* > * We should only use the power well if we explicitly asked the > hardware to > * enable it, so check if it's enabled and also check if we've > requested it to > @@ -2725,6 +2763,13 @@ static const struct i915_power_well_ops > icl_combo_phy_aux_power_well_ops = { > .is_enabled = hsw_power_well_enabled, > }; > > +static const struct i915_power_well_ops > icl_tc_phy_aux_power_well_ops = { > + .sync_hw = hsw_power_well_sync_hw, > + .enable = icl_tc_phy_aux_power_well_enable, > + .disable = icl_tc_phy_aux_power_well_disable, > + .is_enabled = hsw_power_well_enabled, > +}; > + > static const struct i915_power_well_regs icl_aux_power_well_regs = { > .bios = ICL_PWR_WELL_CTL_AUX1, > .driver = ICL_PWR_WELL_CTL_AUX2, > @@ -2870,81 +2915,89 @@ static const struct i915_power_well_desc > icl_power_wells[] = { > { > .name = "AUX C", > .domains = ICL_AUX_C_IO_POWER_DOMAINS, > - .ops = &hsw_power_well_ops, > + .ops = &icl_tc_phy_aux_power_well_ops, > .id = DISP_PW_ID_NONE, > { > .hsw.regs = &icl_aux_power_well_regs, > .hsw.idx = ICL_PW_CTL_IDX_AUX_C, > + .hsw.is_tc_tbt = false, > }, > }, > { > .name = "AUX D", > .domains = ICL_AUX_D_IO_POWER_DOMAINS, > - .ops = &hsw_power_well_ops, > + .ops = &icl_tc_phy_aux_power_well_ops, > .id = DISP_PW_ID_NONE, > { > .hsw.regs = &icl_aux_power_well_regs, > .hsw.idx = ICL_PW_CTL_IDX_AUX_D, > + .hsw.is_tc_tbt = false, > }, > }, > { > .name = "AUX E", > .domains = ICL_AUX_E_IO_POWER_DOMAINS, > - .ops = &hsw_power_well_ops, > + .ops = &icl_tc_phy_aux_power_well_ops, > .id = DISP_PW_ID_NONE, > { > .hsw.regs = &icl_aux_power_well_regs, > .hsw.idx = ICL_PW_CTL_IDX_AUX_E, > + .hsw.is_tc_tbt = false, > }, > }, > { > .name = "AUX F", > .domains = ICL_AUX_F_IO_POWER_DOMAINS, > - .ops = &hsw_power_well_ops, > + .ops = &icl_tc_phy_aux_power_well_ops, > .id = DISP_PW_ID_NONE, > { > .hsw.regs = &icl_aux_power_well_regs, > .hsw.idx = ICL_PW_CTL_IDX_AUX_F, > + .hsw.is_tc_tbt = false, > }, > }, > { > .name = "AUX TBT1", > .domains = ICL_AUX_TBT1_IO_POWER_DOMAINS, > - .ops = &hsw_power_well_ops, > + .ops = &icl_tc_phy_aux_power_well_ops, > .id = DISP_PW_ID_NONE, > { > .hsw.regs = &icl_aux_power_well_regs, > .hsw.idx = ICL_PW_CTL_IDX_AUX_TBT1, > + .hsw.is_tc_tbt = true, > }, > }, > { > .name = "AUX TBT2", > .domains = ICL_AUX_TBT2_IO_POWER_DOMAINS, > - .ops = &hsw_power_well_ops, > + .ops = &icl_tc_phy_aux_power_well_ops, > .id = DISP_PW_ID_NONE, > { > .hsw.regs = &icl_aux_power_well_regs, > .hsw.idx = ICL_PW_CTL_IDX_AUX_TBT2, > + .hsw.is_tc_tbt = true, > }, > }, > { > .name = "AUX TBT3", > .domains = ICL_AUX_TBT3_IO_POWER_DOMAINS, > - .ops = &hsw_power_well_ops, > + .ops = &icl_tc_phy_aux_power_well_ops, > .id = DISP_PW_ID_NONE, > { > .hsw.regs = &icl_aux_power_well_regs, > .hsw.idx = ICL_PW_CTL_IDX_AUX_TBT3, > + .hsw.is_tc_tbt = true, > }, > }, > { > .name = "AUX TBT4", > .domains = ICL_AUX_TBT4_IO_POWER_DOMAINS, > - .ops = &hsw_power_well_ops, > + .ops = &icl_tc_phy_aux_power_well_ops, > .id = DISP_PW_ID_NONE, > { > .hsw.regs = &icl_aux_power_well_regs, > .hsw.idx = ICL_PW_CTL_IDX_AUX_TBT4, > + .hsw.is_tc_tbt = true, > }, > }, > { _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
