Em Qui, 2018-06-21 às 15:04 -0700, Srivatsa, Anusha escreveu: > > -----Original Message----- > > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On > > Behalf Of > > Paulo Zanoni > > Sent: Monday, May 21, 2018 5:26 PM > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: Zanoni, Paulo R <paulo.r.zanoni@xxxxxxxxx> > > Subject: [PATCH 20/24] drm/i915/icl: implement the > > tc/legacy HPD > > {dis, }connect flow for DP > > > > Implement the DFLEXDPPMS/DFLEXDPCSSS dance for DisplayPort. These > > functions need to be called during HPD assert/deassert, but due to > > how our driver > > works it's much simpler if we always call them when > > icl_digital_port_connected() is called, which means we won't call > > them exactly > > once per HPD event. This should also cover the connected boot case, > > whatever > > the BIOS does. > > > > We're still missing the HDMI case, which should be implemented in > > the next > > patch. > > > > Also notice that, today, the BSpec pages for the DFLEXDPPMS and > > DFLEXDPCSSS > > registers are wrong, so you should only trust the flows described > > by the "Gen11 > > TypeC Programming" page in our spec. > > > > Cc: Animesh Manna <animesh.manna@xxxxxxxxx> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 6 > > +++++ drivers/gpu/drm/i915/intel_dp.c | > > 57 ++++++++++++++++++++++++++++++++++++++++- > > 2 files changed, 62 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 24308d4435f5..42cbace4c61e 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -10027,4 +10027,10 @@ enum skl_power_gate { > > _ICL_PHY_MISC_B) > > #define ICL_PHY_MISC_DE_IO_COMP_PWR_DOWN (1 << 23) > > > > +#define PORT_TX_DFLEXDPPMS > > _MMIO(0x163890) > > +#define DP_PHY_MODE_STATUS_COMPLETED(tc_port) (1 > > << > > (tc_port)) > > + > > +#define PORT_TX_DFLEXDPCSSS > > _MMIO(0x163894) > > +#define DP_PHY_MODE_STATUS_NOT_SAFE(tc_port) (1 > > << (tc_port)) > > + > > #endif /* _I915_REG_H_ */ > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index f3d5b9eed625..f25f871e7c22 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -4762,6 +4762,56 @@ static void icl_update_tc_port_type(struct > > drm_i915_private *dev_priv, > > type_str); > > } > > > > +static bool icl_tc_phy_mode_status_connect(struct drm_i915_private > > *dev_priv, > > + struct > > intel_digital_port *dig_port) { > > + enum tc_port tc_port = intel_port_to_tc(dev_priv, > > dig_port->base.port); > > + u32 val; > > + > > + if (dig_port->tc_type != TC_PORT_LEGACY && > > + dig_port->tc_type != TC_PORT_TYPEC) > > + return true; > > Shouldn’t this return false? Types that don't need to run this sequence can be considered connected, so we return true. > > > + val = I915_READ(PORT_TX_DFLEXDPPMS); > > + if (!(val & DP_PHY_MODE_STATUS_COMPLETED(tc_port))) { > > + DRM_ERROR("DP PHY for TC port %d not ready\n", > > tc_port); > > + return false; > > + } > > + > > + /* > > + * This function may be called many times in a row without > > an HPD event > > + * in between, so try to avoid the write when we can. > > + */ > > + val = I915_READ(PORT_TX_DFLEXDPCSSS); > > + if (!(val & DP_PHY_MODE_STATUS_NOT_SAFE(tc_port))) { > > + val |= DP_PHY_MODE_STATUS_NOT_SAFE(tc_port); > > + I915_WRITE(PORT_TX_DFLEXDPCSSS, val); > > + } > > + > > + return true; > > +} > > + > > +static void icl_tc_phy_mode_status_disconnect(struct > > drm_i915_private > > *dev_priv, > > + struct > > intel_digital_port *dig_port) { > > + enum tc_port tc_port = intel_port_to_tc(dev_priv, > > dig_port->base.port); > > + u32 val; > > + > > + if (dig_port->tc_type != TC_PORT_LEGACY && > > + dig_port->tc_type != TC_PORT_TYPEC) > > + return; > > + > > + /* > > + * This function may be called many times in a row without > > an HPD event > > + * in between, so try to avoid the write when we can. > > + */ > > So, in the sequences to enable, it does tell that enabling suitable > aux power domains is optional. But in the disable sequence, disable > AUX_PWR is mentioned as a non-optional step. > In which case it has to be before we set DFLEXDPCSSS register to 0. > > Is it being addressed in another patch? It should be disabled here because we're not using the port anymore at this point. > > The rest of the patch looks good. > > Anusha > > > + val = I915_READ(PORT_TX_DFLEXDPCSSS); > > + if (val & DP_PHY_MODE_STATUS_NOT_SAFE(tc_port)) { > > + val &= ~DP_PHY_MODE_STATUS_NOT_SAFE(tc_port); > > + I915_WRITE(PORT_TX_DFLEXDPCSSS, val); > > + } > > +} > > + > > static bool icl_tc_port_connected(struct drm_i915_private > > *dev_priv, > > struct intel_digital_port > > *intel_dig_port) { @@ > > -4782,12 +4832,17 @@ static bool icl_tc_port_connected(struct > > drm_i915_private *dev_priv, > > if (cpu_isr & tbt_bit) > > is_tbt = true; > > > > - if (!is_legacy && !is_typec && !is_tbt) > > + if (!is_legacy && !is_typec && !is_tbt) { > > + icl_tc_phy_mode_status_disconnect(dev_priv, > > intel_dig_port); > > return false; > > + } > > > > icl_update_tc_port_type(dev_priv, intel_dig_port, is_legacy, > > is_typec, > > is_tbt); > > > > + if (!icl_tc_phy_mode_status_connect(dev_priv, > > intel_dig_port)) > > + return false; > > + > > return true; > > } > > > > -- > > 2.14.3 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
