[Intel-gfx] [PATCH] drm/i915/tc: Implement the TC cold exit sequence
Imre Deak
imre.deak at intel.com
Sat Oct 19 10:49:42 UTC 2019
On Sat, Oct 19, 2019 at 02:59:14AM +0300, Souza, Jose wrote:
> On Thu, 2019-10-03 at 17:50 +0300, Imre Deak wrote:
> > On Mon, Sep 30, 2019 at 05:55:36PM -0700, José Roberto de Souza
> > wrote:
> > > This is required for legacy/static TC ports as IOM is not aware of
> > > the connection and will not trigger the TC cold exit.
> > >
> > > BSpec: 21750
> > > BSpsc: 49294
> > > Cc: Imre Deak <imre.deak at intel.com>
> > > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_tc.c | 34 ++++++++++++++++++++-
> > > ----
> > > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > > 2 files changed, 29 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c
> > > b/drivers/gpu/drm/i915/display/intel_tc.c
> > > index 7773169b7331..09b78027bdd5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > > @@ -7,6 +7,7 @@
> > > #include "intel_display.h"
> > > #include "intel_display_types.h"
> > > #include "intel_dp_mst.h"
> > > +#include "intel_sideband.h"
> > > #include "intel_tc.h"
> > >
> > > static const char *tc_port_mode_name(enum tc_port_mode mode)
> > > @@ -169,6 +170,22 @@ static void tc_port_fixup_legacy_flag(struct
> > > intel_digital_port *dig_port,
> > > dig_port->tc_legacy_port = !dig_port->tc_legacy_port;
> > > }
> > >
> > > +static int tc_cold_exit_request(struct drm_i915_private *dev_priv)
> > > +{
> > > + int ret;
> > > +
> > > + do {
> > > + ret = sandybridge_pcode_write_timeout(dev_priv,
> > > + ICL_PCODE_EXIT_TC
> > > COLD, 0,
> > > + 250, 1);
> > > +
> > > + } while (ret == -EAGAIN);
> > > +
> > > + DRM_DEBUG_KMS("tccold exit %s\n", ret == 0 ? "succeeded" :
> > > "failed");
> > > +
> > > + return ret;
> > > +}
> > > +
> > > static u32 tc_port_live_status_mask(struct intel_digital_port
> > > *dig_port)
> > > {
> > > struct drm_i915_private *i915 = to_i915(dig_port-
> > > >base.base.dev);
> > > @@ -177,13 +194,21 @@ static u32 tc_port_live_status_mask(struct
> > > intel_digital_port *dig_port)
> > > u32 mask = 0;
> > > u32 val;
> > >
> > > + if (intel_uncore_read(uncore, SDEISR) &
> > > SDE_TC_HOTPLUG_ICP(tc_port))
> > > + mask |= BIT(TC_PORT_LEGACY);
> > > +
> > > val = intel_uncore_read(uncore,
> > > PORT_TX_DFLEXDPSP(dig_port-
> > > >tc_phy_fia));
> > >
> > > if (val == 0xffffffff) {
> > > - DRM_DEBUG_KMS("Port %s: PHY in TCCOLD, nothing
> > > connected\n",
> > > - dig_port->tc_port_name);
> > > - return mask;
> > > + if (mask)
> > > + tc_cold_exit_request(i915);
>
>
> Sorry for the delay, was OOO.
>
> >
> > The semantic of the mbox command this needs is inherently racy on
> > systems with preemption, the instruction to use it is:
> >
> > """
> > Issue GT Driver mailbox command to exit TCCOLD, then complete steps
> > b-d
> > within 500 milliseconds to prevent re-entry.
> > """
> >
> > We'd have to reach the point where we enable the AUX power in 500ms,
> > which can't be guaranteed. Moreover TCCOLD could be entered just
> > after
> > reading PORT_TX_DFLEXDPSP, so we may not detect it.
> >
> > What we can do - after discussing with HW folks - is to first request
> > AUX power and then issue the mbox command, which prevents TCCOLD
> > re-entry until releasing the AUX power request. This also needs
> > ignoring
> > the timeout of the AUX power enabling ACK, since it will only be
> > ACKed
> > after exiting TCCOLD.
>
>
> Huum looks more robust indeed.
>
> >
> > So I think we should block TCCOLD this way in __intel_tc_port_lock():
> >
> > if tc_link_refcount==0:
> > intel_display_power_get(POWER_DOMAIN_AUX_<port>)
> > tc_cold_exit_request()
>
> So for non-legacy the IOM request to exit TCCOLD could have timed out
> at this point. But for most cases this will not happen, why not here
> read one FIA register and check if == 0xffffffff if so we call
> tc_cold_exit_request().
That would be racy, TCCOLD could be entered right after doing the FIA
reg check. Getting the AUX power ref alone won't prevent a TCCOLD entry
until the PCODE command has completed.
>
> > if intel_tc_port_needs_reset():
> > intel_tc_port_reset_mode()
> > if tc_mode != LEGACY:
> > intel_display_power_put(POWER_DOMAIN_AUX_<port>
>
> Also why power_put() for non-legacy? a preemption could happen after
> lock() and cause a TCCOLD before the needed reads/writes to FIA is
> done.
The new PCODE command was only added for legacy and the AUX power
reference makes a difference only in that mode. In DP-alt mode the
DPCSSS.DPPMSTC flag prevents TCCOLD.
In TBT mode the TBT AUX power ref would not prevent TCCOLD. In that mode
we don't access FIA regs afterwards and
"the TBT controller prevents TCCOLD while it uses the connection".
>
> Better remove this block and only power_put() when removing the last
> reference on intel_tc_port_unlock()
>
> > )
> >
> > then unblock TCCOLD in intel_tc_port_unlock():
> >
> > if tc_link_refcount==0 and tc_mode == LEGACY:
> > intel_display_power_put(POWER_DOMAIN_AUX_<port>)
> >
> > > +
> > > + if (val == 0xffffffff) {
> > > + DRM_DEBUG_KMS("Port %s: PHY in TCCOLD, nothing
> > > connected\n",
> > > + dig_port->tc_port_name);
> > > + return mask;
> > > + }
> > > }
> > >
> > > if (val & TC_LIVE_STATE_TBT(dig_port->tc_phy_fia_idx))
> > > @@ -191,9 +216,6 @@ static u32 tc_port_live_status_mask(struct
> > > intel_digital_port *dig_port)
> > > if (val & TC_LIVE_STATE_TC(dig_port->tc_phy_fia_idx))
> > > mask |= BIT(TC_PORT_DP_ALT);
> > >
> > > - if (intel_uncore_read(uncore, SDEISR) &
> > > SDE_TC_HOTPLUG_ICP(tc_port))
> > > - mask |= BIT(TC_PORT_LEGACY);
> > > -
> > > /* The sink can be connected only in a single mode. */
> > > if (!WARN_ON(hweight32(mask) > 1))
> > > tc_port_fixup_legacy_flag(dig_port, mask);
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 058aa5ca8b73..35c3724b7fef 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -8860,6 +8860,7 @@ enum {
> > > #define ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point) (((poin
> > > t) << 16) | (0x1 << 8))
> > > #define GEN6_PCODE_READ_D_COMP 0x10
> > > #define GEN6_PCODE_WRITE_D_COMP 0x11
> > > +#define ICL_PCODE_EXIT_TCCOLD 0x12
> > > #define HSW_PCODE_DE_WRITE_FREQ_REQ 0x17
> > > #define DISPLAY_IPS_CONTROL 0x19
> > > /* See also IPS_CTL */
> > > --
> > > 2.23.0
> > >
More information about the Intel-gfx
mailing list