On Wed, Oct 31, 2018 at 01:28:07AM +0200, Souza, Jose wrote: > 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. Ok, will change these. > > > + > > /* > > * 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
