| Message ID | 20180926025422.2154-1-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Tue, Sep 25, 2018 at 07:54:22PM -0700, Dhinakaran Pandiyan wrote: > A crtc modeset lock was added for link retraining but > intel_dp_retrain_link() knows to take the necessary locks since > commit c85d200e8321 ("drm/i915: Move SST DP link retraining into the > ->post_hotplug() hook") > v2: Drop AUX power domain reference in the early return path > > Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into the ->post_hotplug() hook") This patch failed on cherry-pick for drm-intel-fixes targeting 4.19. If it still makes sense to have it there please provide a backported version to drm-intel-fixes or let me know if it has dependency on other patches? Thanks, Rodrigo. > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++-------------- > 1 file changed, 10 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 09229fc66dec..ef28cc0f122c 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -5079,8 +5079,15 @@ intel_dp_long_pulse(struct intel_connector *connector, > * Some external monitors do not signal loss of link synchronization > * with an IRQ_HPD, so force a link status check. > */ > - if (!intel_dp_is_edp(intel_dp)) > - intel_dp_retrain_link(encoder, ctx); > + if (!intel_dp_is_edp(intel_dp)) { > + int ret; > + > + ret = intel_dp_retrain_link(encoder, ctx); > + if (ret) { > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain); > + return ret; > + } > + } > > /* > * Clearing NACK and defer counts to get their exact values > @@ -5130,19 +5137,8 @@ intel_dp_detect(struct drm_connector *connector, > connector->base.id, connector->name); > > /* If full detect is not performed yet, do a full detect */ > - if (!intel_dp->detect_done) { > - struct drm_crtc *crtc; > - int ret; > - > - crtc = connector->state->crtc; > - if (crtc) { > - ret = drm_modeset_lock(&crtc->mutex, ctx); > - if (ret) > - return ret; > - } > - > + if (!intel_dp->detect_done) > status = intel_dp_long_pulse(intel_dp->attached_connector, ctx); > - } > > intel_dp->detect_done = false; > > -- > 2.14.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Oct 01, 2018 at 12:48:28PM -0700, Rodrigo Vivi wrote: > On Tue, Sep 25, 2018 at 07:54:22PM -0700, Dhinakaran Pandiyan wrote: > > A crtc modeset lock was added for link retraining but > > intel_dp_retrain_link() knows to take the necessary locks since > > commit c85d200e8321 ("drm/i915: Move SST DP link retraining into the > > ->post_hotplug() hook") > > v2: Drop AUX power domain reference in the early return path > > > > Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into the ->post_hotplug() hook") > > This patch failed on cherry-pick for drm-intel-fixes targeting 4.19. > > If it still makes sense to have it there please provide a backported version > to drm-intel-fixes or let me know if it has dependency on other patches? This is more of an optimization to not disturb page flips and whatnot so much with detect. The double lock is otherwise harmless due to ww_mutex. > > Thanks, > Rodrigo. > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++-------------- > > 1 file changed, 10 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 09229fc66dec..ef28cc0f122c 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -5079,8 +5079,15 @@ intel_dp_long_pulse(struct intel_connector *connector, > > * Some external monitors do not signal loss of link synchronization > > * with an IRQ_HPD, so force a link status check. > > */ > > - if (!intel_dp_is_edp(intel_dp)) > > - intel_dp_retrain_link(encoder, ctx); > > + if (!intel_dp_is_edp(intel_dp)) { > > + int ret; > > + > > + ret = intel_dp_retrain_link(encoder, ctx); > > + if (ret) { > > + intel_display_power_put(dev_priv, intel_dp->aux_power_domain); > > + return ret; > > + } > > + } > > > > /* > > * Clearing NACK and defer counts to get their exact values > > @@ -5130,19 +5137,8 @@ intel_dp_detect(struct drm_connector *connector, > > connector->base.id, connector->name); > > > > /* If full detect is not performed yet, do a full detect */ > > - if (!intel_dp->detect_done) { > > - struct drm_crtc *crtc; > > - int ret; > > - > > - crtc = connector->state->crtc; > > - if (crtc) { > > - ret = drm_modeset_lock(&crtc->mutex, ctx); > > - if (ret) > > - return ret; > > - } > > - > > + if (!intel_dp->detect_done) > > status = intel_dp_long_pulse(intel_dp->attached_connector, ctx); > > - } > > > > intel_dp->detect_done = false; > > > > -- > > 2.14.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 2018-10-01 at 23:10 +0300, Ville Syrjälä wrote: > On Mon, Oct 01, 2018 at 12:48:28PM -0700, Rodrigo Vivi wrote: > > On Tue, Sep 25, 2018 at 07:54:22PM -0700, Dhinakaran Pandiyan > > wrote: > > > A crtc modeset lock was added for link retraining but > > > intel_dp_retrain_link() knows to take the necessary locks since > > > commit c85d200e8321 ("drm/i915: Move SST DP link retraining into > > > the > > > ->post_hotplug() hook") > > > v2: Drop AUX power domain reference in the early return path > > > > > > Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into > > > the ->post_hotplug() hook") > > > > This patch failed on cherry-pick for drm-intel-fixes targeting > > 4.19. > > > > If it still makes sense to have it there please provide a > > backported version > > to drm-intel-fixes or let me know if it has dependency on other > > patches? > > This is more of an optimization to not disturb page flips and whatnot > so much with detect. The double lock is otherwise harmless due to > ww_mutex. > Yeah, let's not backport this. -DK > > > > Thanks, > > Rodrigo. > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com > > > > > > > --- > > > drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++-------------- > > > 1 file changed, 10 insertions(+), 14 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > b/drivers/gpu/drm/i915/intel_dp.c > > > index 09229fc66dec..ef28cc0f122c 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -5079,8 +5079,15 @@ intel_dp_long_pulse(struct intel_connector > > > *connector, > > > * Some external monitors do not signal loss of link > > > synchronization > > > * with an IRQ_HPD, so force a link status check. > > > */ > > > - if (!intel_dp_is_edp(intel_dp)) > > > - intel_dp_retrain_link(encoder, ctx); > > > + if (!intel_dp_is_edp(intel_dp)) { > > > + int ret; > > > + > > > + ret = intel_dp_retrain_link(encoder, ctx); > > > + if (ret) { > > > + intel_display_power_put(dev_priv, intel_dp- > > > >aux_power_domain); > > > + return ret; > > > + } > > > + } > > > > > > /* > > > * Clearing NACK and defer counts to get their exact values > > > @@ -5130,19 +5137,8 @@ intel_dp_detect(struct drm_connector > > > *connector, > > > connector->base.id, connector->name); > > > > > > /* If full detect is not performed yet, do a full detect */ > > > - if (!intel_dp->detect_done) { > > > - struct drm_crtc *crtc; > > > - int ret; > > > - > > > - crtc = connector->state->crtc; > > > - if (crtc) { > > > - ret = drm_modeset_lock(&crtc->mutex, ctx); > > > - if (ret) > > > - return ret; > > > - } > > > - > > > + if (!intel_dp->detect_done) > > > status = intel_dp_long_pulse(intel_dp- > > > >attached_connector, ctx); > > > - } > > > > > > intel_dp->detect_done = false; > > > > > > -- > > > 2.14.1 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
On Mon, Oct 01, 2018 at 03:01:18PM -0700, Dhinakaran Pandiyan wrote: > On Mon, 2018-10-01 at 23:10 +0300, Ville Syrjälä wrote: > > On Mon, Oct 01, 2018 at 12:48:28PM -0700, Rodrigo Vivi wrote: > > > On Tue, Sep 25, 2018 at 07:54:22PM -0700, Dhinakaran Pandiyan > > > wrote: > > > > A crtc modeset lock was added for link retraining but > > > > intel_dp_retrain_link() knows to take the necessary locks since > > > > commit c85d200e8321 ("drm/i915: Move SST DP link retraining into > > > > the > > > > ->post_hotplug() hook") > > > > v2: Drop AUX power domain reference in the early return path > > > > > > > > Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into > > > > the ->post_hotplug() hook") > > > > > > This patch failed on cherry-pick for drm-intel-fixes targeting > > > 4.19. > > > > > > If it still makes sense to have it there please provide a > > > backported version > > > to drm-intel-fixes or let me know if it has dependency on other > > > patches? > > > > This is more of an optimization to not disturb page flips and whatnot > > so much with detect. The double lock is otherwise harmless due to > > ww_mutex. > > > Yeah, let's not backport this. cool, thanks. > > -DK > > > > > > > Thanks, > > > Rodrigo. > > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com > > > > > > > > > --- > > > > drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++-------------- > > > > 1 file changed, 10 insertions(+), 14 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > > b/drivers/gpu/drm/i915/intel_dp.c > > > > index 09229fc66dec..ef28cc0f122c 100644 > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > @@ -5079,8 +5079,15 @@ intel_dp_long_pulse(struct intel_connector > > > > *connector, > > > > * Some external monitors do not signal loss of link > > > > synchronization > > > > * with an IRQ_HPD, so force a link status check. > > > > */ > > > > - if (!intel_dp_is_edp(intel_dp)) > > > > - intel_dp_retrain_link(encoder, ctx); > > > > + if (!intel_dp_is_edp(intel_dp)) { > > > > + int ret; > > > > + > > > > + ret = intel_dp_retrain_link(encoder, ctx); > > > > + if (ret) { > > > > + intel_display_power_put(dev_priv, intel_dp- > > > > >aux_power_domain); > > > > + return ret; > > > > + } > > > > + } > > > > > > > > /* > > > > * Clearing NACK and defer counts to get their exact values > > > > @@ -5130,19 +5137,8 @@ intel_dp_detect(struct drm_connector > > > > *connector, > > > > connector->base.id, connector->name); > > > > > > > > /* If full detect is not performed yet, do a full detect */ > > > > - if (!intel_dp->detect_done) { > > > > - struct drm_crtc *crtc; > > > > - int ret; > > > > - > > > > - crtc = connector->state->crtc; > > > > - if (crtc) { > > > > - ret = drm_modeset_lock(&crtc->mutex, ctx); > > > > - if (ret) > > > > - return ret; > > > > - } > > > > - > > > > + if (!intel_dp->detect_done) > > > > status = intel_dp_long_pulse(intel_dp- > > > > >attached_connector, ctx); > > > > - } > > > > > > > > intel_dp->detect_done = false; > > > > > > > > -- > > > > 2.14.1 > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > >
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 09229fc66dec..ef28cc0f122c 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -5079,8 +5079,15 @@ intel_dp_long_pulse(struct intel_connector *connector, * Some external monitors do not signal loss of link synchronization * with an IRQ_HPD, so force a link status check. */ - if (!intel_dp_is_edp(intel_dp)) - intel_dp_retrain_link(encoder, ctx); + if (!intel_dp_is_edp(intel_dp)) { + int ret; + + ret = intel_dp_retrain_link(encoder, ctx); + if (ret) { + intel_display_power_put(dev_priv, intel_dp->aux_power_domain); + return ret; + } + } /* * Clearing NACK and defer counts to get their exact values @@ -5130,19 +5137,8 @@ intel_dp_detect(struct drm_connector *connector, connector->base.id, connector->name); /* If full detect is not performed yet, do a full detect */ - if (!intel_dp->detect_done) { - struct drm_crtc *crtc; - int ret; - - crtc = connector->state->crtc; - if (crtc) { - ret = drm_modeset_lock(&crtc->mutex, ctx); - if (ret) - return ret; - } - + if (!intel_dp->detect_done) status = intel_dp_long_pulse(intel_dp->attached_connector, ctx); - } intel_dp->detect_done = false;
A crtc modeset lock was added for link retraining but intel_dp_retrain_link() knows to take the necessary locks since commit c85d200e8321 ("drm/i915: Move SST DP link retraining into the ->post_hotplug() hook") v2: Drop AUX power domain reference in the early return path Fixes: c85d200e8321 ("drm/i915: Move SST DP link retraining into the ->post_hotplug() hook") Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-)