Re: [Intel-gfx] [PATCH 2/2] drm/i915: Do not get aux power for disconnected DP ports
On Wed, Sep 26, 2018 at 10:42:25PM +, Souza, Jose wrote:
> On Tue, 2018-09-25 at 15:17 +0300, Ville Syrjälä wrote:
> > On Mon, Sep 24, 2018 at 06:16:49PM -0700, José Roberto de Souza
> > wrote:
> > > For ICL type-c ports there is a aux power restriction, it can only
> > > be
> > > enabled while there is sink connected.
> >
> > That can't be entirely true because it's super racy. I was talking
> > with Imre about this mess at some point, and IIRC we came up with
> > some half decent theories on how we might handle this mess in a
> > sane manner. IIRC it was something along the lines of: the tbt vs.
> > other mode knob defines which power well we can enable, and so
> > while we're doing something with the port in one mode or the other
> > we must not try to use the port in the other mode. Ie. we need to
> > properly track which mode the port is in and if there's an event
> > that requires switching modes we have to wait until the previous
> > use of the other mode has stopped.
>
> I can't think how would we use the same TBT3 port in TBT3 and USB-C
> mode without going to the disconnection and connection flow.
There's no thunderbolt support added yet, for instance all the TypeC
ports are set up statically to work in USB-C mode. So a proper way to
switch from one mode to other with all the proper waits for any pending
connect/disconnect event needs to be implemented as described in BSpec.
The above also means that when switching from one mode to the other we
have to wait for any pending operation keeping the AUX power domain for
the other mode enabled (meaning detect and AUX transfers for any other
reason).
> This statment about aux power restriction is on BSec also in the past
> we were having some timeouts warnings while waiting for USB-C/TBT aux
> power wells to be turn or off, Imre fixed that by not adding
> POWER_DOMAIN_INIT to those power well domains.
>
> >
> > >
> > > BSpec: 21750
> > >
> > > Cc: Maarten Lankhorst
> > > Signed-off-by: José Roberto de Souza
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 19 ++-
> > > 1 file changed, 14 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 6b4c19123f2a..48fd38cd4ba4 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -5019,19 +5019,27 @@ intel_dp_long_pulse(struct intel_connector
> > > *connector,
> > > struct intel_dp *intel_dp = intel_attached_dp(
> > > >base);
> > > enum drm_connector_status status;
> > > u8 sink_irq_vector = 0;
> > > + bool got_aux_power;
> > >
> > > WARN_ON(!drm_modeset_is_locked(_priv-
> > > >drm.mode_config.connection_mutex));
> > >
> > > + /* Can't disconnect eDP */
> > > + if (!intel_dp_is_edp(intel_dp) &&
> > > + !intel_digital_port_connected(_to_dig_port(intel_dp)-
> > > >base)) {
> > > + status = connector_status_disconnected;
> > > + got_aux_power = false;
> > > + goto port_disconnected;
> > > + }
> > > +
> > > intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> > > + got_aux_power = true;
> > >
> > > - /* Can't disconnect eDP */
> > > if (intel_dp_is_edp(intel_dp))
> > > status = edp_detect(intel_dp);
> > > - else if
> > > (intel_digital_port_connected(_to_dig_port(intel_dp)->base))
> > > - status = intel_dp_detect_dpcd(intel_dp);
> > > else
> > > - status = connector_status_disconnected;
> > > + status = intel_dp_detect_dpcd(intel_dp);
> > >
> > > +port_disconnected:
> > > if (status == connector_status_disconnected) {
> > > memset(_dp->compliance, 0, sizeof(intel_dp-
> > > >compliance));
> > >
> > > @@ -5122,7 +5130,8 @@ intel_dp_long_pulse(struct intel_connector
> > > *connector,
> > > if (status != connector_status_connected && !intel_dp->is_mst)
> > > intel_dp_unset_edid(intel_dp);
> > >
> > > - intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> > > + if (got_aux_power)
> > > + intel_display_power_put(dev_priv, intel_dp-
> > > >aux_power_domain);
> > > return status;
> > > }
> > >
> > > --
> > > 2.19.0
> > >
> > > ___
> > > Intel-gfx mailing list
> > > intel-...@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Do not get aux power for disconnected DP ports
On Tue, 2018-09-25 at 15:17 +0300, Ville Syrjälä wrote:
> On Mon, Sep 24, 2018 at 06:16:49PM -0700, José Roberto de Souza
> wrote:
> > For ICL type-c ports there is a aux power restriction, it can only
> > be
> > enabled while there is sink connected.
>
> That can't be entirely true because it's super racy. I was talking
> with Imre about this mess at some point, and IIRC we came up with
> some half decent theories on how we might handle this mess in a
> sane manner. IIRC it was something along the lines of: the tbt vs.
> other mode knob defines which power well we can enable, and so
> while we're doing something with the port in one mode or the other
> we must not try to use the port in the other mode. Ie. we need to
> properly track which mode the port is in and if there's an event
> that requires switching modes we have to wait until the previous
> use of the other mode has stopped.
I can't think how would we use the same TBT3 port in TBT3 and USB-C
mode without going to the disconnection and connection flow.
This statment about aux power restriction is on BSec also in the past
we were having some timeouts warnings while waiting for USB-C/TBT aux
power wells to be turn or off, Imre fixed that by not adding
POWER_DOMAIN_INIT to those power well domains.
>
> >
> > BSpec: 21750
> >
> > Cc: Maarten Lankhorst
> > Signed-off-by: José Roberto de Souza
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 19 ++-
> > 1 file changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 6b4c19123f2a..48fd38cd4ba4 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -5019,19 +5019,27 @@ intel_dp_long_pulse(struct intel_connector
> > *connector,
> > struct intel_dp *intel_dp = intel_attached_dp(
> > >base);
> > enum drm_connector_status status;
> > u8 sink_irq_vector = 0;
> > + bool got_aux_power;
> >
> > WARN_ON(!drm_modeset_is_locked(_priv-
> > >drm.mode_config.connection_mutex));
> >
> > + /* Can't disconnect eDP */
> > + if (!intel_dp_is_edp(intel_dp) &&
> > + !intel_digital_port_connected(_to_dig_port(intel_dp)-
> > >base)) {
> > + status = connector_status_disconnected;
> > + got_aux_power = false;
> > + goto port_disconnected;
> > + }
> > +
> > intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> > + got_aux_power = true;
> >
> > - /* Can't disconnect eDP */
> > if (intel_dp_is_edp(intel_dp))
> > status = edp_detect(intel_dp);
> > - else if
> > (intel_digital_port_connected(_to_dig_port(intel_dp)->base))
> > - status = intel_dp_detect_dpcd(intel_dp);
> > else
> > - status = connector_status_disconnected;
> > + status = intel_dp_detect_dpcd(intel_dp);
> >
> > +port_disconnected:
> > if (status == connector_status_disconnected) {
> > memset(_dp->compliance, 0, sizeof(intel_dp-
> > >compliance));
> >
> > @@ -5122,7 +5130,8 @@ intel_dp_long_pulse(struct intel_connector
> > *connector,
> > if (status != connector_status_connected && !intel_dp->is_mst)
> > intel_dp_unset_edid(intel_dp);
> >
> > - intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> > + if (got_aux_power)
> > + intel_display_power_put(dev_priv, intel_dp-
> > >aux_power_domain);
> > return status;
> > }
> >
> > --
> > 2.19.0
> >
> > ___
> > Intel-gfx mailing list
> > intel-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Do not get aux power for disconnected DP ports
On Mon, Sep 24, 2018 at 06:16:49PM -0700, José Roberto de Souza wrote:
> For ICL type-c ports there is a aux power restriction, it can only be
> enabled while there is sink connected.
That can't be entirely true because it's super racy. I was talking
with Imre about this mess at some point, and IIRC we came up with
some half decent theories on how we might handle this mess in a
sane manner. IIRC it was something along the lines of: the tbt vs.
other mode knob defines which power well we can enable, and so
while we're doing something with the port in one mode or the other
we must not try to use the port in the other mode. Ie. we need to
properly track which mode the port is in and if there's an event
that requires switching modes we have to wait until the previous
use of the other mode has stopped.
>
> BSpec: 21750
>
> Cc: Maarten Lankhorst
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/intel_dp.c | 19 ++-
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 6b4c19123f2a..48fd38cd4ba4 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5019,19 +5019,27 @@ intel_dp_long_pulse(struct intel_connector *connector,
> struct intel_dp *intel_dp = intel_attached_dp(>base);
> enum drm_connector_status status;
> u8 sink_irq_vector = 0;
> + bool got_aux_power;
>
>
> WARN_ON(!drm_modeset_is_locked(_priv->drm.mode_config.connection_mutex));
>
> + /* Can't disconnect eDP */
> + if (!intel_dp_is_edp(intel_dp) &&
> + !intel_digital_port_connected(_to_dig_port(intel_dp)->base)) {
> + status = connector_status_disconnected;
> + got_aux_power = false;
> + goto port_disconnected;
> + }
> +
> intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> + got_aux_power = true;
>
> - /* Can't disconnect eDP */
> if (intel_dp_is_edp(intel_dp))
> status = edp_detect(intel_dp);
> - else if (intel_digital_port_connected(_to_dig_port(intel_dp)->base))
> - status = intel_dp_detect_dpcd(intel_dp);
> else
> - status = connector_status_disconnected;
> + status = intel_dp_detect_dpcd(intel_dp);
>
> +port_disconnected:
> if (status == connector_status_disconnected) {
> memset(_dp->compliance, 0, sizeof(intel_dp->compliance));
>
> @@ -5122,7 +5130,8 @@ intel_dp_long_pulse(struct intel_connector *connector,
> if (status != connector_status_connected && !intel_dp->is_mst)
> intel_dp_unset_edid(intel_dp);
>
> - intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> + if (got_aux_power)
> + intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> return status;
> }
>
> --
> 2.19.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
