| Message ID | 20180924224528.4744-5-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/6] drm/i915/dp: Fix link retraining comment in intel_dp_long_pulse() | expand |
On Mon, Sep 24, 2018 at 03:45:27PM -0700, Dhinakaran Pandiyan wrote: > The intel_dp->detect_done flag is no more useful. Pull > intel_dp_long_pulse() into the lone caller, > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > 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 | 43 ++++++++------------------------ > drivers/gpu/drm/i915/intel_drv.h | 1 - > 2 files changed, 11 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 87a631098a6d..d6ea93e453a2 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -5012,15 +5012,18 @@ intel_dp_unset_edid(struct intel_dp *intel_dp) > } > > static int > -intel_dp_long_pulse(struct intel_connector *connector, > - struct drm_modeset_acquire_ctx *ctx) > +intel_dp_detect(struct drm_connector *connector, > + struct drm_modeset_acquire_ctx *ctx, > + bool force) > { > - struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > - struct intel_dp *intel_dp = intel_attached_dp(&connector->base); > + struct drm_i915_private *dev_priv = to_i915(connector->dev); > + struct intel_dp *intel_dp = intel_attached_dp(connector); > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; > - enum drm_connector_status status; > + int status; I guess we don't need this change anymore? Series is Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Not sure if ci actually found some issue with the series, or whether it was just having a bad day. > u8 sink_irq_vector = 0; > > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > + connector->base.id, connector->name); > WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex)); > > intel_display_power_get(dev_priv, intel_dp->aux_power_domain); > @@ -5096,9 +5099,9 @@ intel_dp_long_pulse(struct intel_connector *connector, > intel_dp->aux.i2c_defer_count = 0; > > intel_dp_set_edid(intel_dp); > - if (intel_dp_is_edp(intel_dp) || connector->detect_edid) > + if (intel_dp_is_edp(intel_dp) || > + to_intel_connector(connector)->detect_edid) > status = connector_status_connected; > - intel_dp->detect_done = true; > > /* Try to read the source of the interrupt */ > if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && > @@ -5123,26 +5126,6 @@ intel_dp_long_pulse(struct intel_connector *connector, > return status; > } > > -static int > -intel_dp_detect(struct drm_connector *connector, > - struct drm_modeset_acquire_ctx *ctx, > - bool force) > -{ > - struct intel_dp *intel_dp = intel_attached_dp(connector); > - int status = connector->status; > - > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > - connector->base.id, connector->name); > - > - /* If full detect is not performed yet, do a full detect */ > - if (!intel_dp->detect_done) > - status = intel_dp_long_pulse(intel_dp->attached_connector, ctx); > - > - intel_dp->detect_done = false; > - > - return status; > -} > - > static void > intel_dp_force(struct drm_connector *connector) > { > @@ -5635,7 +5618,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) > > if (long_hpd) { > intel_dp->reset_link_params = true; > - intel_dp->detect_done = false; > return IRQ_NONE; > } > > @@ -5652,7 +5634,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) > intel_dp->is_mst = false; > drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, > intel_dp->is_mst); > - intel_dp->detect_done = false; > goto put_power; > } > } > @@ -5665,10 +5646,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) > /* Short pulse can signify loss of hdcp authentication */ > intel_hdcp_check_link(intel_dp->attached_connector); > > - if (!handled) { > - intel_dp->detect_done = false; > + if (!handled) > goto put_power; > - } > } > > ret = IRQ_HANDLED; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 8073a85d7178..a7c151c40e7d 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1070,7 +1070,6 @@ struct intel_dp { > bool link_mst; > bool link_trained; > bool has_audio; > - bool detect_done; > bool reset_link_params; > enum aux_ch aux_ch; > uint8_t dpcd[DP_RECEIVER_CAP_SIZE]; > -- > 2.17.1
On Tue, 2018-09-25 at 20:53 +0300, Ville Syrjälä wrote: > On Mon, Sep 24, 2018 at 03:45:27PM -0700, Dhinakaran Pandiyan wrote: > > The intel_dp->detect_done flag is no more useful. Pull > > intel_dp_long_pulse() into the lone caller, > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > 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 | 43 ++++++++------------------ > > ------ > > drivers/gpu/drm/i915/intel_drv.h | 1 - > > 2 files changed, 11 insertions(+), 33 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > b/drivers/gpu/drm/i915/intel_dp.c > > index 87a631098a6d..d6ea93e453a2 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -5012,15 +5012,18 @@ intel_dp_unset_edid(struct intel_dp > > *intel_dp) > > } > > > > static int > > -intel_dp_long_pulse(struct intel_connector *connector, > > - struct drm_modeset_acquire_ctx *ctx) > > +intel_dp_detect(struct drm_connector *connector, > > + struct drm_modeset_acquire_ctx *ctx, > > + bool force) > > { > > - struct drm_i915_private *dev_priv = to_i915(connector- > > >base.dev); > > - struct intel_dp *intel_dp = intel_attached_dp(&connector- > > >base); > > + struct drm_i915_private *dev_priv = to_i915(connector- > > >dev); > > + struct intel_dp *intel_dp = intel_attached_dp(connector); > > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)- > > >base; > > - enum drm_connector_status status; > > + int status; > > I guess we don't need this change anymore? Yeah, I'll remove it. > > Series is > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Not sure if ci actually found some issue with the series, or > whether it was just having a bad day. It did find an issue, patch 4/6 wasn't dropping the aux domain reference in the error path. I've sent a new version with the fix. -DK > > > u8 sink_irq_vector = 0; > > > > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > > + connector->base.id, connector->name); > > WARN_ON(!drm_modeset_is_locked(&dev_priv- > > >drm.mode_config.connection_mutex)); > > > > intel_display_power_get(dev_priv, intel_dp- > > >aux_power_domain); > > @@ -5096,9 +5099,9 @@ intel_dp_long_pulse(struct intel_connector > > *connector, > > intel_dp->aux.i2c_defer_count = 0; > > > > intel_dp_set_edid(intel_dp); > > - if (intel_dp_is_edp(intel_dp) || connector->detect_edid) > > + if (intel_dp_is_edp(intel_dp) || > > + to_intel_connector(connector)->detect_edid) > > status = connector_status_connected; > > - intel_dp->detect_done = true; > > > > /* Try to read the source of the interrupt */ > > if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && > > @@ -5123,26 +5126,6 @@ intel_dp_long_pulse(struct intel_connector > > *connector, > > return status; > > } > > > > -static int > > -intel_dp_detect(struct drm_connector *connector, > > - struct drm_modeset_acquire_ctx *ctx, > > - bool force) > > -{ > > - struct intel_dp *intel_dp = intel_attached_dp(connector); > > - int status = connector->status; > > - > > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > > - connector->base.id, connector->name); > > - > > - /* If full detect is not performed yet, do a full detect > > */ > > - if (!intel_dp->detect_done) > > - status = intel_dp_long_pulse(intel_dp- > > >attached_connector, ctx); > > - > > - intel_dp->detect_done = false; > > - > > - return status; > > -} > > - > > static void > > intel_dp_force(struct drm_connector *connector) > > { > > @@ -5635,7 +5618,6 @@ intel_dp_hpd_pulse(struct intel_digital_port > > *intel_dig_port, bool long_hpd) > > > > if (long_hpd) { > > intel_dp->reset_link_params = true; > > - intel_dp->detect_done = false; > > return IRQ_NONE; > > } > > > > @@ -5652,7 +5634,6 @@ intel_dp_hpd_pulse(struct intel_digital_port > > *intel_dig_port, bool long_hpd) > > intel_dp->is_mst = false; > > drm_dp_mst_topology_mgr_set_mst(&intel_dp- > > >mst_mgr, > > intel_dp- > > >is_mst); > > - intel_dp->detect_done = false; > > goto put_power; > > } > > } > > @@ -5665,10 +5646,8 @@ intel_dp_hpd_pulse(struct intel_digital_port > > *intel_dig_port, bool long_hpd) > > /* Short pulse can signify loss of hdcp > > authentication */ > > intel_hdcp_check_link(intel_dp- > > >attached_connector); > > > > - if (!handled) { > > - intel_dp->detect_done = false; > > + if (!handled) > > goto put_power; > > - } > > } > > > > ret = IRQ_HANDLED; > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index 8073a85d7178..a7c151c40e7d 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1070,7 +1070,6 @@ struct intel_dp { > > bool link_mst; > > bool link_trained; > > bool has_audio; > > - bool detect_done; > > bool reset_link_params; > > enum aux_ch aux_ch; > > uint8_t dpcd[DP_RECEIVER_CAP_SIZE]; > > -- > > 2.17.1 > >
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 87a631098a6d..d6ea93e453a2 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -5012,15 +5012,18 @@ intel_dp_unset_edid(struct intel_dp *intel_dp) } static int -intel_dp_long_pulse(struct intel_connector *connector, - struct drm_modeset_acquire_ctx *ctx) +intel_dp_detect(struct drm_connector *connector, + struct drm_modeset_acquire_ctx *ctx, + bool force) { - struct drm_i915_private *dev_priv = to_i915(connector->base.dev); - struct intel_dp *intel_dp = intel_attached_dp(&connector->base); + struct drm_i915_private *dev_priv = to_i915(connector->dev); + struct intel_dp *intel_dp = intel_attached_dp(connector); struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; - enum drm_connector_status status; + int status; u8 sink_irq_vector = 0; + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", + connector->base.id, connector->name); WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex)); intel_display_power_get(dev_priv, intel_dp->aux_power_domain); @@ -5096,9 +5099,9 @@ intel_dp_long_pulse(struct intel_connector *connector, intel_dp->aux.i2c_defer_count = 0; intel_dp_set_edid(intel_dp); - if (intel_dp_is_edp(intel_dp) || connector->detect_edid) + if (intel_dp_is_edp(intel_dp) || + to_intel_connector(connector)->detect_edid) status = connector_status_connected; - intel_dp->detect_done = true; /* Try to read the source of the interrupt */ if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && @@ -5123,26 +5126,6 @@ intel_dp_long_pulse(struct intel_connector *connector, return status; } -static int -intel_dp_detect(struct drm_connector *connector, - struct drm_modeset_acquire_ctx *ctx, - bool force) -{ - struct intel_dp *intel_dp = intel_attached_dp(connector); - int status = connector->status; - - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", - connector->base.id, connector->name); - - /* If full detect is not performed yet, do a full detect */ - if (!intel_dp->detect_done) - status = intel_dp_long_pulse(intel_dp->attached_connector, ctx); - - intel_dp->detect_done = false; - - return status; -} - static void intel_dp_force(struct drm_connector *connector) { @@ -5635,7 +5618,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) if (long_hpd) { intel_dp->reset_link_params = true; - intel_dp->detect_done = false; return IRQ_NONE; } @@ -5652,7 +5634,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) intel_dp->is_mst = false; drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst); - intel_dp->detect_done = false; goto put_power; } } @@ -5665,10 +5646,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) /* Short pulse can signify loss of hdcp authentication */ intel_hdcp_check_link(intel_dp->attached_connector); - if (!handled) { - intel_dp->detect_done = false; + if (!handled) goto put_power; - } } ret = IRQ_HANDLED; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8073a85d7178..a7c151c40e7d 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1070,7 +1070,6 @@ struct intel_dp { bool link_mst; bool link_trained; bool has_audio; - bool detect_done; bool reset_link_params; enum aux_ch aux_ch; uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
The intel_dp->detect_done flag is no more useful. Pull intel_dp_long_pulse() into the lone caller, Cc: Jani Nikula <jani.nikula@linux.intel.com> 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 | 43 ++++++++------------------------ drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 11 insertions(+), 33 deletions(-)