| Message ID | 20181002175054.15010-6-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/10] drm: Do not call drm_dp_cec_set_edid() while registering DP connectors | expand |
On Tue, Oct 02, 2018 at 10:50:50AM -0700, José Roberto de Souza wrote: > Before enter in power saving states or before unload the driver > spec states that display driver is required to to mark TC ports as > safe so hardware can do the disconnection flow without wait for > display driver handshake. > When driver is resumed or loaded again, if the TC live state is > still set as connected driver will mark again TC port as not safe as > required by spec. > > BSpec: 2175 > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 2e242270e270..58616690f45f 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -3640,6 +3640,7 @@ static void gen11_irq_reset(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > int pipe; > + u32 val; > > I915_WRITE(GEN11_GFX_MSTR_IRQ, 0); > POSTING_READ(GEN11_GFX_MSTR_IRQ); > @@ -3661,6 +3662,15 @@ static void gen11_irq_reset(struct drm_device *dev) > > if (HAS_PCH_ICP(dev_priv)) > GEN3_IRQ_RESET(SDE); > + > + /* > + * Mark TC ports as safe so hardware can do the disconnect flow without > + * wait for driver to do the handshake > + */ > + val = I915_READ(PORT_TX_DFLEXDPCSSS); > + for (pipe = 0; pipe < 4; pipe++) > + val &= ~(DP_PHY_MODE_STATUS_NOT_SAFE(pipe)); > + I915_WRITE(PORT_TX_DFLEXDPCSSS, val); Why would we have to do this here? The driver should relinquish control if and when it has shut down the pipes etc. Sounds like a bug if we're hanging on when we have no need for the port. > } > > void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, > -- > 2.19.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 2018-10-02 at 23:35 +0300, Ville Syrjälä wrote: > On Tue, Oct 02, 2018 at 10:50:50AM -0700, José Roberto de Souza > wrote: > > Before enter in power saving states or before unload the driver > > spec states that display driver is required to to mark TC ports as > > safe so hardware can do the disconnection flow without wait for > > display driver handshake. > > When driver is resumed or loaded again, if the TC live state is > > still set as connected driver will mark again TC port as not safe > > as > > required by spec. > > > > BSpec: 2175 > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index 2e242270e270..58616690f45f 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -3640,6 +3640,7 @@ static void gen11_irq_reset(struct drm_device > > *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > int pipe; > > + u32 val; > > > > I915_WRITE(GEN11_GFX_MSTR_IRQ, 0); > > POSTING_READ(GEN11_GFX_MSTR_IRQ); > > @@ -3661,6 +3662,15 @@ static void gen11_irq_reset(struct > > drm_device *dev) > > > > if (HAS_PCH_ICP(dev_priv)) > > GEN3_IRQ_RESET(SDE); > > + > > + /* > > + * Mark TC ports as safe so hardware can do the disconnect flow > > without > > + * wait for driver to do the handshake > > + */ > > + val = I915_READ(PORT_TX_DFLEXDPCSSS); > > + for (pipe = 0; pipe < 4; pipe++) > > + val &= ~(DP_PHY_MODE_STATUS_NOT_SAFE(pipe)); > > + I915_WRITE(PORT_TX_DFLEXDPCSSS, val); > > Why would we have to do this here? The driver should relinquish > control > if and when it has shut down the pipes etc. Sounds like a bug if > we're > hanging on when we have no need for the port. Right now we take control and only release it when port is disconnected. You mean release it in intel_dp_link_down()? I guess it could be moved to that function but we would need: - make icl_tc_port_connected() not grab control in any further calls to intel_digital_port_connected() when driver is not using it - to add special case to handle MST - release control when a error happen when doing the modeset In my opinion that would make it way more complicated with no benefit. Also keeping it in gen11_irq_reset() would give back to HW the control when loading the module, fixing possible problems with a BIOS that do not release control of it before handing out system to bootloader. > > > } > > > > void gen8_irq_power_well_post_enable(struct drm_i915_private > > *dev_priv, > > -- > > 2.19.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Oct 10, 2018 at 12:55:07AM +0000, Souza, Jose wrote: > On Tue, 2018-10-02 at 23:35 +0300, Ville Syrjälä wrote: > > On Tue, Oct 02, 2018 at 10:50:50AM -0700, José Roberto de Souza > > wrote: > > > Before enter in power saving states or before unload the driver > > > spec states that display driver is required to to mark TC ports as > > > safe so hardware can do the disconnection flow without wait for > > > display driver handshake. > > > When driver is resumed or loaded again, if the TC live state is > > > still set as connected driver will mark again TC port as not safe > > > as > > > required by spec. > > > > > > BSpec: 2175 > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > > b/drivers/gpu/drm/i915/i915_irq.c > > > index 2e242270e270..58616690f45f 100644 > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > @@ -3640,6 +3640,7 @@ static void gen11_irq_reset(struct drm_device > > > *dev) > > > { > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > int pipe; > > > + u32 val; > > > > > > I915_WRITE(GEN11_GFX_MSTR_IRQ, 0); > > > POSTING_READ(GEN11_GFX_MSTR_IRQ); > > > @@ -3661,6 +3662,15 @@ static void gen11_irq_reset(struct > > > drm_device *dev) > > > > > > if (HAS_PCH_ICP(dev_priv)) > > > GEN3_IRQ_RESET(SDE); > > > + > > > + /* > > > + * Mark TC ports as safe so hardware can do the disconnect flow > > > without > > > + * wait for driver to do the handshake > > > + */ > > > + val = I915_READ(PORT_TX_DFLEXDPCSSS); > > > + for (pipe = 0; pipe < 4; pipe++) > > > + val &= ~(DP_PHY_MODE_STATUS_NOT_SAFE(pipe)); > > > + I915_WRITE(PORT_TX_DFLEXDPCSSS, val); > > > > Why would we have to do this here? The driver should relinquish > > control > > if and when it has shut down the pipes etc. Sounds like a bug if > > we're > > hanging on when we have no need for the port. > > Right now we take control and only release it when port is > disconnected. Disconnection is totally asynchronous. Someone could be using the port/aux for anything when the disconnect irq happens. Presumably bad things will happen if we just go and yank the control away when someone is doing stuff. I believe even the spec tells us to properly shut things down during the disconnect flow and make sure eg. the aux power wells have been fully shut down before relinquishing control.
On Wed, 2018-10-10 at 20:15 +0300, Ville Syrjälä wrote: > On Wed, Oct 10, 2018 at 12:55:07AM +0000, Souza, Jose wrote: > > On Tue, 2018-10-02 at 23:35 +0300, Ville Syrjälä wrote: > > > On Tue, Oct 02, 2018 at 10:50:50AM -0700, José Roberto de Souza > > > wrote: > > > > Before enter in power saving states or before unload the driver > > > > spec states that display driver is required to to mark TC ports > > > > as > > > > safe so hardware can do the disconnection flow without wait for > > > > display driver handshake. > > > > When driver is resumed or loaded again, if the TC live state is > > > > still set as connected driver will mark again TC port as not > > > > safe > > > > as > > > > required by spec. > > > > > > > > BSpec: 2175 > > > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++ > > > > 1 file changed, 10 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > > > b/drivers/gpu/drm/i915/i915_irq.c > > > > index 2e242270e270..58616690f45f 100644 > > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > > @@ -3640,6 +3640,7 @@ static void gen11_irq_reset(struct > > > > drm_device > > > > *dev) > > > > { > > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > > int pipe; > > > > + u32 val; > > > > > > > > I915_WRITE(GEN11_GFX_MSTR_IRQ, 0); > > > > POSTING_READ(GEN11_GFX_MSTR_IRQ); > > > > @@ -3661,6 +3662,15 @@ static void gen11_irq_reset(struct > > > > drm_device *dev) > > > > > > > > if (HAS_PCH_ICP(dev_priv)) > > > > GEN3_IRQ_RESET(SDE); > > > > + > > > > + /* > > > > + * Mark TC ports as safe so hardware can do the > > > > disconnect flow > > > > without > > > > + * wait for driver to do the handshake > > > > + */ > > > > + val = I915_READ(PORT_TX_DFLEXDPCSSS); > > > > + for (pipe = 0; pipe < 4; pipe++) > > > > + val &= ~(DP_PHY_MODE_STATUS_NOT_SAFE(pipe)); > > > > + I915_WRITE(PORT_TX_DFLEXDPCSSS, val); > > > > > > Why would we have to do this here? The driver should relinquish > > > control > > > if and when it has shut down the pipes etc. Sounds like a bug if > > > we're > > > hanging on when we have no need for the port. > > > > Right now we take control and only release it when port is > > disconnected. > > Disconnection is totally asynchronous. Someone could be using the > port/aux for anything when the disconnect irq happens. Presumably > bad things will happen if we just go and yank the control away > when someone is doing stuff. I believe even the spec tells us > to properly shut things down during the disconnect flow and make > sure eg. the aux power wells have been fully shut down before > relinquishing control. In my understanding at the point of the gen11_irq_reset() call all DDI DDI ports and aux channels are already off. >
On Wed, Oct 10, 2018 at 05:23:30PM +0000, Souza, Jose wrote: > On Wed, 2018-10-10 at 20:15 +0300, Ville Syrjälä wrote: > > On Wed, Oct 10, 2018 at 12:55:07AM +0000, Souza, Jose wrote: > > > On Tue, 2018-10-02 at 23:35 +0300, Ville Syrjälä wrote: > > > > On Tue, Oct 02, 2018 at 10:50:50AM -0700, José Roberto de Souza > > > > wrote: > > > > > Before enter in power saving states or before unload the driver > > > > > spec states that display driver is required to to mark TC ports > > > > > as > > > > > safe so hardware can do the disconnection flow without wait for > > > > > display driver handshake. > > > > > When driver is resumed or loaded again, if the TC live state is > > > > > still set as connected driver will mark again TC port as not > > > > > safe > > > > > as > > > > > required by spec. > > > > > > > > > > BSpec: 2175 > > > > > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++ > > > > > 1 file changed, 10 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > > > > b/drivers/gpu/drm/i915/i915_irq.c > > > > > index 2e242270e270..58616690f45f 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > > > @@ -3640,6 +3640,7 @@ static void gen11_irq_reset(struct > > > > > drm_device > > > > > *dev) > > > > > { > > > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > > > int pipe; > > > > > + u32 val; > > > > > > > > > > I915_WRITE(GEN11_GFX_MSTR_IRQ, 0); > > > > > POSTING_READ(GEN11_GFX_MSTR_IRQ); > > > > > @@ -3661,6 +3662,15 @@ static void gen11_irq_reset(struct > > > > > drm_device *dev) > > > > > > > > > > if (HAS_PCH_ICP(dev_priv)) > > > > > GEN3_IRQ_RESET(SDE); > > > > > + > > > > > + /* > > > > > + * Mark TC ports as safe so hardware can do the > > > > > disconnect flow > > > > > without > > > > > + * wait for driver to do the handshake > > > > > + */ > > > > > + val = I915_READ(PORT_TX_DFLEXDPCSSS); > > > > > + for (pipe = 0; pipe < 4; pipe++) > > > > > + val &= ~(DP_PHY_MODE_STATUS_NOT_SAFE(pipe)); > > > > > + I915_WRITE(PORT_TX_DFLEXDPCSSS, val); > > > > > > > > Why would we have to do this here? The driver should relinquish > > > > control > > > > if and when it has shut down the pipes etc. Sounds like a bug if > > > > we're > > > > hanging on when we have no need for the port. > > > > > > Right now we take control and only release it when port is > > > disconnected. > > > > Disconnection is totally asynchronous. Someone could be using the > > port/aux for anything when the disconnect irq happens. Presumably > > bad things will happen if we just go and yank the control away > > when someone is doing stuff. I believe even the spec tells us > > to properly shut things down during the disconnect flow and make > > sure eg. the aux power wells have been fully shut down before > > relinquishing control. > > In my understanding at the point of the gen11_irq_reset() call all DDI > DDI ports and aux channels are already off. I'm not talking about gen11_irq_reset(). But if we are then that gets called during driver load too and everything could be up and running. Though I'm not sure what the BIOS/GOP will do w.r.t. the safe mode knob. Anyways, I don't think poking at some display stuff from irq_reset() is a particularly clean apporoach. The irq code should only concern itself with irqs. If we properly track which mode the port is in then I presume we'd just put it back into the tbt mode when the last typec mode user goes away. Or if we try to keep it in typec mode even when there are no users (as some kind of optimimization perhaps?) then we should probably switch it back to tbt mode during some display suspend/shutdown sequence.
On Wed, 2018-10-10 at 20:52 +0300, Ville Syrjälä wrote: > On Wed, Oct 10, 2018 at 05:23:30PM +0000, Souza, Jose wrote: > > On Wed, 2018-10-10 at 20:15 +0300, Ville Syrjälä wrote: > > > On Wed, Oct 10, 2018 at 12:55:07AM +0000, Souza, Jose wrote: > > > > On Tue, 2018-10-02 at 23:35 +0300, Ville Syrjälä wrote: > > > > > On Tue, Oct 02, 2018 at 10:50:50AM -0700, José Roberto de > > > > > Souza > > > > > wrote: > > > > > > Before enter in power saving states or before unload the > > > > > > driver > > > > > > spec states that display driver is required to to mark TC > > > > > > ports > > > > > > as > > > > > > safe so hardware can do the disconnection flow without wait > > > > > > for > > > > > > display driver handshake. > > > > > > When driver is resumed or loaded again, if the TC live > > > > > > state is > > > > > > still set as connected driver will mark again TC port as > > > > > > not > > > > > > safe > > > > > > as > > > > > > required by spec. > > > > > > > > > > > > BSpec: 2175 > > > > > > > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > > > --- > > > > > > drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++ > > > > > > 1 file changed, 10 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > > > > > b/drivers/gpu/drm/i915/i915_irq.c > > > > > > index 2e242270e270..58616690f45f 100644 > > > > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > > > > @@ -3640,6 +3640,7 @@ static void gen11_irq_reset(struct > > > > > > drm_device > > > > > > *dev) > > > > > > { > > > > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > > > > int pipe; > > > > > > + u32 val; > > > > > > > > > > > > I915_WRITE(GEN11_GFX_MSTR_IRQ, 0); > > > > > > POSTING_READ(GEN11_GFX_MSTR_IRQ); > > > > > > @@ -3661,6 +3662,15 @@ static void gen11_irq_reset(struct > > > > > > drm_device *dev) > > > > > > > > > > > > if (HAS_PCH_ICP(dev_priv)) > > > > > > GEN3_IRQ_RESET(SDE); > > > > > > + > > > > > > + /* > > > > > > + * Mark TC ports as safe so hardware can do the > > > > > > disconnect flow > > > > > > without > > > > > > + * wait for driver to do the handshake > > > > > > + */ > > > > > > + val = I915_READ(PORT_TX_DFLEXDPCSSS); > > > > > > + for (pipe = 0; pipe < 4; pipe++) > > > > > > + val &= ~(DP_PHY_MODE_STATUS_NOT_SAFE(pipe)); > > > > > > + I915_WRITE(PORT_TX_DFLEXDPCSSS, val); > > > > > > > > > > Why would we have to do this here? The driver should > > > > > relinquish > > > > > control > > > > > if and when it has shut down the pipes etc. Sounds like a bug > > > > > if > > > > > we're > > > > > hanging on when we have no need for the port. > > > > > > > > Right now we take control and only release it when port is > > > > disconnected. > > > > > > Disconnection is totally asynchronous. Someone could be using the > > > port/aux for anything when the disconnect irq happens. Presumably > > > bad things will happen if we just go and yank the control away > > > when someone is doing stuff. I believe even the spec tells us > > > to properly shut things down during the disconnect flow and make > > > sure eg. the aux power wells have been fully shut down before > > > relinquishing control. > > > > In my understanding at the point of the gen11_irq_reset() call all > > DDI > > DDI ports and aux channels are already off. > > I'm not talking about gen11_irq_reset(). But if we are then that gets > called during driver load too and everything could be up and running. > Though I'm not sure what the BIOS/GOP will do w.r.t. the safe mode > knob. BIOS should do the same connection flow to use sinks in tc ports. > > Anyways, I don't think poking at some display stuff from irq_reset() > is a particularly clean apporoach. The irq code should only concern Move it to a function? Or move everything reseting display irq to a function like is done for gen11_gt_irq_reset(). > itself with irqs. If we properly track which mode the port is in then > I presume we'd just put it back into the tbt mode when the last > typec mode user goes away. Or if we try to keep it in typec mode even > when there are no users (as some kind of optimimization perhaps?) > then > we should probably switch it back to tbt mode during some display > suspend/shutdown sequence. We don't control what mode the connector is in, HW is the one that tell us if it is in: type-c, TBT, legacy or disconnected state our only job is grab and release this knob. >
On Wed, Oct 10, 2018 at 06:27:07PM +0000, Souza, Jose wrote: > On Wed, 2018-10-10 at 20:52 +0300, Ville Syrjälä wrote: > > On Wed, Oct 10, 2018 at 05:23:30PM +0000, Souza, Jose wrote: > > > On Wed, 2018-10-10 at 20:15 +0300, Ville Syrjälä wrote: > > > > On Wed, Oct 10, 2018 at 12:55:07AM +0000, Souza, Jose wrote: > > > > > On Tue, 2018-10-02 at 23:35 +0300, Ville Syrjälä wrote: > > > > > > On Tue, Oct 02, 2018 at 10:50:50AM -0700, José Roberto de > > > > > > Souza > > > > > > wrote: > > > > > > > Before enter in power saving states or before unload the > > > > > > > driver > > > > > > > spec states that display driver is required to to mark TC > > > > > > > ports > > > > > > > as > > > > > > > safe so hardware can do the disconnection flow without wait > > > > > > > for > > > > > > > display driver handshake. > > > > > > > When driver is resumed or loaded again, if the TC live > > > > > > > state is > > > > > > > still set as connected driver will mark again TC port as > > > > > > > not > > > > > > > safe > > > > > > > as > > > > > > > required by spec. > > > > > > > > > > > > > > BSpec: 2175 > > > > > > > > > > > > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > > > > --- > > > > > > > drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++ > > > > > > > 1 file changed, 10 insertions(+) > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > > > > > > b/drivers/gpu/drm/i915/i915_irq.c > > > > > > > index 2e242270e270..58616690f45f 100644 > > > > > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > > > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > > > > > @@ -3640,6 +3640,7 @@ static void gen11_irq_reset(struct > > > > > > > drm_device > > > > > > > *dev) > > > > > > > { > > > > > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > > > > > int pipe; > > > > > > > + u32 val; > > > > > > > > > > > > > > I915_WRITE(GEN11_GFX_MSTR_IRQ, 0); > > > > > > > POSTING_READ(GEN11_GFX_MSTR_IRQ); > > > > > > > @@ -3661,6 +3662,15 @@ static void gen11_irq_reset(struct > > > > > > > drm_device *dev) > > > > > > > > > > > > > > if (HAS_PCH_ICP(dev_priv)) > > > > > > > GEN3_IRQ_RESET(SDE); > > > > > > > + > > > > > > > + /* > > > > > > > + * Mark TC ports as safe so hardware can do the > > > > > > > disconnect flow > > > > > > > without > > > > > > > + * wait for driver to do the handshake > > > > > > > + */ > > > > > > > + val = I915_READ(PORT_TX_DFLEXDPCSSS); > > > > > > > + for (pipe = 0; pipe < 4; pipe++) > > > > > > > + val &= ~(DP_PHY_MODE_STATUS_NOT_SAFE(pipe)); > > > > > > > + I915_WRITE(PORT_TX_DFLEXDPCSSS, val); > > > > > > > > > > > > Why would we have to do this here? The driver should > > > > > > relinquish > > > > > > control > > > > > > if and when it has shut down the pipes etc. Sounds like a bug > > > > > > if > > > > > > we're > > > > > > hanging on when we have no need for the port. > > > > > > > > > > Right now we take control and only release it when port is > > > > > disconnected. > > > > > > > > Disconnection is totally asynchronous. Someone could be using the > > > > port/aux for anything when the disconnect irq happens. Presumably > > > > bad things will happen if we just go and yank the control away > > > > when someone is doing stuff. I believe even the spec tells us > > > > to properly shut things down during the disconnect flow and make > > > > sure eg. the aux power wells have been fully shut down before > > > > relinquishing control. > > > > > > In my understanding at the point of the gen11_irq_reset() call all > > > DDI > > > DDI ports and aux channels are already off. > > > > I'm not talking about gen11_irq_reset(). But if we are then that gets > > called during driver load too and everything could be up and running. > > Though I'm not sure what the BIOS/GOP will do w.r.t. the safe mode > > knob. > > BIOS should do the same connection flow to use sinks in tc ports. > > > > > Anyways, I don't think poking at some display stuff from irq_reset() > > is a particularly clean apporoach. The irq code should only concern > > Move it to a function? Or move everything reseting display irq to a > function like is done for gen11_gt_irq_reset(). > > > itself with irqs. If we properly track which mode the port is in then > > I presume we'd just put it back into the tbt mode when the last > > typec mode user goes away. Or if we try to keep it in typec mode even > > when there are no users (as some kind of optimimization perhaps?) > > then > > we should probably switch it back to tbt mode during some display > > suspend/shutdown sequence. > > We don't control what mode the connector is in, HW is the one that tell > us if it is in: type-c, TBT, legacy or disconnected state our only job > is grab and release this knob. So s/tbt mode/safe mode/ or whatever. Semantics.
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2e242270e270..58616690f45f 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3640,6 +3640,7 @@ static void gen11_irq_reset(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; int pipe; + u32 val; I915_WRITE(GEN11_GFX_MSTR_IRQ, 0); POSTING_READ(GEN11_GFX_MSTR_IRQ); @@ -3661,6 +3662,15 @@ static void gen11_irq_reset(struct drm_device *dev) if (HAS_PCH_ICP(dev_priv)) GEN3_IRQ_RESET(SDE); + + /* + * Mark TC ports as safe so hardware can do the disconnect flow without + * wait for driver to do the handshake + */ + val = I915_READ(PORT_TX_DFLEXDPCSSS); + for (pipe = 0; pipe < 4; pipe++) + val &= ~(DP_PHY_MODE_STATUS_NOT_SAFE(pipe)); + I915_WRITE(PORT_TX_DFLEXDPCSSS, val); } void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv,
Before enter in power saving states or before unload the driver spec states that display driver is required to to mark TC ports as safe so hardware can do the disconnection flow without wait for display driver handshake. When driver is resumed or loaded again, if the TC live state is still set as connected driver will mark again TC port as not safe as required by spec. BSpec: 2175 Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++ 1 file changed, 10 insertions(+)