Hi Imre,
> On Mon, May 16, 2022 at 01:54:02AM -0700, Vivek Kasireddy wrote:
> > Although, doing a modeset on any disconnected connector might be futile,
> > it can be particularly problematic if the connector is a Type-C port
> > without a sink. And, the spec only says "Display software must not use
> > a disconnected port" while referring to the Type-C DDI seqeuence, it does
> > not spell out what happens if such an attempt is made. Experimental results
> > have shown that this can lead to serious issues including a system hang.
> > Therefore, reject the atomic modeset if we detect that the Type-C port
> > is not connected.
> >
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_atomic.c | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> > index 40da7910f845..40576964b8c1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> > @@ -114,6 +114,8 @@ int intel_digital_connector_atomic_set_property(struct
> drm_connector *connector,
> > int intel_digital_connector_atomic_check(struct drm_connector *conn,
> > struct drm_atomic_state *state)
> > {
> > + struct drm_device *dev = conn->dev;
> > + struct drm_i915_private *dev_priv = to_i915(dev);
> > struct drm_connector_state *new_state =
> > drm_atomic_get_new_connector_state(state, conn);
> > struct intel_digital_connector_state *new_conn_state =
> > @@ -122,6 +124,10 @@ int intel_digital_connector_atomic_check(struct
> drm_connector *conn,
> > drm_atomic_get_old_connector_state(state, conn);
> > struct intel_digital_connector_state *old_conn_state =
> > to_intel_digital_connector_state(old_state);
> > + struct intel_encoder *encoder =
> > + intel_attached_encoder(to_intel_connector(conn));
> > + struct intel_digital_port *dig_port =
> > + encoder ? enc_to_dig_port(encoder) : NULL;
> > struct drm_crtc_state *crtc_state;
> >
> > intel_hdcp_atomic_check(conn, old_state, new_state);
> > @@ -131,6 +137,20 @@ int intel_digital_connector_atomic_check(struct
> drm_connector *conn,
> >
> > crtc_state = drm_atomic_get_new_crtc_state(state, new_state->crtc);
> >
> > + /*
> > + * The spec says that it is not safe to use a disconnected Type-C port.
> > + * Therefore, check to see if this connector is connected and reject
> > + * the modeset if there is no sink detected.
> > + */
> > + if (dig_port && !dig_port->connected(encoder) &&
>
> This check is racy, as right after dig_port->connected() returns true,
> the port can become disconnected.
[Kasireddy, Vivek] Given that, do you think the only way to reliably determine
if the Type-C port has a sink is to check the live status and ignore dig_port->tc_mode?
If that is the case, should I just add a function pointer to dig_port to call
tc_port_live_status_mask()? Or, should I just change intel_tc_port_connected()
to ignore dig_port->tc_mode like below:
@@ -764,8 +764,7 @@ bool intel_tc_port_connected(struct intel_encoder *encoder)
intel_tc_port_lock(dig_port);
- is_connected = tc_port_live_status_mask(dig_port) &
- BIT(dig_port->tc_mode);
+ is_connected = tc_port_live_status_mask(dig_port);
Or, are there any other elegant ways that you can think of to determine whether
a tc port has a sink or not?
Thanks,
Vivek
>
> > + intel_phy_is_tc(dev_priv,
> > + intel_port_to_phy(dev_priv, encoder->port))) {
> > + drm_dbg_atomic(&dev_priv->drm,
> > + "[CONNECTOR:%d:%s] is not connected; rejecting the
> modeset\n",
> > + conn->base.id, conn->name);
> > + return -EINVAL;
> > + }
> > +
> > /*
> > * These properties are handled by fastset, and might not end
> > * up in a modeset.
> > --
> > 2.35.1
> >
