| Message ID | 1451998226-21433-7-git-send-email-shubhangi.shrivastava@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2016-01-05 at 18:20 +0530, Shubhangi Shrivastava wrote: > This patch checks for changes in sink count between short pulse > hpds and forces full detect when there is a change. > > This will allow both detection of hotplug and unplug of panels > through dongles that give only short pulse for such events. > > v2: changed variable type from u8 to bool (Jani) > return immediately if perform_full_detect is set(Siva) > > v3: changed method of determining full detection from using > pointer to return code (Siva) > > Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com> > Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 30 +++++++++++++++++++++++------- > 1 file changed, 23 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 0d58bfd..8a659ee 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4331,12 +4331,14 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) > * 3. Use Link Training from 2.5.3.3 and 3.5.1.3 > * 4. Check link status on receipt of hot-plug interrupt > */ > -static void > +static bool Please expand the comment above to indicate what the return value of this function is supposed to mean. > intel_dp_short_pulse(struct intel_dp *intel_dp) > { > struct drm_device *dev = intel_dp_to_dev(intel_dp); > u8 sink_irq_vector; > u8 link_status[DP_LINK_STATUS_SIZE]; > + u8 old_sink_count = intel_dp->sink_count; > + bool ret; > > /* > * Clearing compliance test variables to allow capturing > @@ -4348,12 +4350,20 @@ intel_dp_short_pulse(struct intel_dp *intel_dp) > > /* Try to read receiver status if the link appears to be up */ > if (!intel_dp_get_link_status(intel_dp, link_status)) { > - return; > + return false; > } > > - /* Now read the DPCD to see if it's actually running */ > - if (!intel_dp_get_dpcd(intel_dp)) { > - return; > + /* > + * Now read the DPCD to see if it's actually running > + * Don't return immediately if dpcd read failed, > + * if sink count was 1 and dpcd read failed we need > + * to do full detection > + */ > + ret = intel_dp_get_dpcd(intel_dp); > + > + if ((old_sink_count != intel_dp->sink_count) || !ret) { I don't see the connection of the comment above with this. If the dpcd read fails, the 'return false' will be reached regardless of the previous value of intel_dp->sink_count. Did you intend to do something different or did I miss something? > + /* No need to proceed if we are going to do full detect */ > + return false; > } > > /* Try to read the source of the interrupt */ > @@ -4373,6 +4383,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp) > drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); > intel_dp_check_link_status(intel_dp); > drm_modeset_unlock(&dev->mode_config.connection_mutex); > + > + return true; > } > > /* XXX this is probably wrong for multiple downstream ports */ > @@ -5095,8 +5107,12 @@ intel_dp_hpd_pulse(struct intel_digital_port > *intel_dig_port, bool long_hpd) > } > } > > - if (!intel_dp->is_mst) > - intel_dp_short_pulse(intel_dp); > + if (!intel_dp->is_mst) { > + if (!intel_dp_short_pulse(intel_dp)) { > + intel_dp_long_pulse(intel_dp > ->attached_connector); > + goto put_power; It could be in a follow up patch, but I think its a good moment to get rid of the goto put_power. The only thing they do is skip the 'ret = IRQ_HANDLED' assignment now. Ander > + } > + } > } > > ret = IRQ_HANDLED;
On Thursday 14 January 2016 07:20 PM, Ander Conselvan De Oliveira wrote: > On Tue, 2016-01-05 at 18:20 +0530, Shubhangi Shrivastava wrote: >> This patch checks for changes in sink count between short pulse >> hpds and forces full detect when there is a change. >> >> This will allow both detection of hotplug and unplug of panels >> through dongles that give only short pulse for such events. >> >> v2: changed variable type from u8 to bool (Jani) >> return immediately if perform_full_detect is set(Siva) >> >> v3: changed method of determining full detection from using >> pointer to return code (Siva) >> >> Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com> >> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> >> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 30 +++++++++++++++++++++++------- >> 1 file changed, 23 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 0d58bfd..8a659ee 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -4331,12 +4331,14 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) >> * 3. Use Link Training from 2.5.3.3 and 3.5.1.3 >> * 4. Check link status on receipt of hot-plug interrupt >> */ >> -static void >> +static bool > Please expand the comment above to indicate what the return value of this > function is supposed to mean. > Sure.. Will add.. >> intel_dp_short_pulse(struct intel_dp *intel_dp) >> { >> struct drm_device *dev = intel_dp_to_dev(intel_dp); >> u8 sink_irq_vector; >> u8 link_status[DP_LINK_STATUS_SIZE]; >> + u8 old_sink_count = intel_dp->sink_count; >> + bool ret; >> >> /* >> * Clearing compliance test variables to allow capturing >> @@ -4348,12 +4350,20 @@ intel_dp_short_pulse(struct intel_dp *intel_dp) >> >> /* Try to read receiver status if the link appears to be up */ >> if (!intel_dp_get_link_status(intel_dp, link_status)) { >> - return; >> + return false; >> } >> >> - /* Now read the DPCD to see if it's actually running */ >> - if (!intel_dp_get_dpcd(intel_dp)) { >> - return; >> + /* >> + * Now read the DPCD to see if it's actually running >> + * Don't return immediately if dpcd read failed, >> + * if sink count was 1 and dpcd read failed we need >> + * to do full detection >> + */ >> + ret = intel_dp_get_dpcd(intel_dp); >> + >> + if ((old_sink_count != intel_dp->sink_count) || !ret) { > I don't see the connection of the comment above with this. If the dpcd read > fails, the 'return false' will be reached regardless of the previous value of > intel_dp->sink_count. Did you intend to do something different or did I miss > something? > The code was changed but comment was not updated.. Will change the comment to explain correctly. >> + /* No need to proceed if we are going to do full detect */ >> + return false; >> } >> >> /* Try to read the source of the interrupt */ >> @@ -4373,6 +4383,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp) >> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); >> intel_dp_check_link_status(intel_dp); >> drm_modeset_unlock(&dev->mode_config.connection_mutex); >> + >> + return true; >> } >> >> /* XXX this is probably wrong for multiple downstream ports */ >> @@ -5095,8 +5107,12 @@ intel_dp_hpd_pulse(struct intel_digital_port >> *intel_dig_port, bool long_hpd) >> } >> } >> >> - if (!intel_dp->is_mst) >> - intel_dp_short_pulse(intel_dp); >> + if (!intel_dp->is_mst) { >> + if (!intel_dp_short_pulse(intel_dp)) { >> + intel_dp_long_pulse(intel_dp >> ->attached_connector); >> + goto put_power; > It could be in a follow up patch, but I think its a good moment to get rid of > the goto put_power. The only thing they do is skip the 'ret = IRQ_HANDLED' > assignment now. > > Ander Sure.. Will remove the goto put_power in follow up patch. >> + } >> + } >> } >> >> ret = IRQ_HANDLED;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 0d58bfd..8a659ee 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4331,12 +4331,14 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) * 3. Use Link Training from 2.5.3.3 and 3.5.1.3 * 4. Check link status on receipt of hot-plug interrupt */ -static void +static bool intel_dp_short_pulse(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); u8 sink_irq_vector; u8 link_status[DP_LINK_STATUS_SIZE]; + u8 old_sink_count = intel_dp->sink_count; + bool ret; /* * Clearing compliance test variables to allow capturing @@ -4348,12 +4350,20 @@ intel_dp_short_pulse(struct intel_dp *intel_dp) /* Try to read receiver status if the link appears to be up */ if (!intel_dp_get_link_status(intel_dp, link_status)) { - return; + return false; } - /* Now read the DPCD to see if it's actually running */ - if (!intel_dp_get_dpcd(intel_dp)) { - return; + /* + * Now read the DPCD to see if it's actually running + * Don't return immediately if dpcd read failed, + * if sink count was 1 and dpcd read failed we need + * to do full detection + */ + ret = intel_dp_get_dpcd(intel_dp); + + if ((old_sink_count != intel_dp->sink_count) || !ret) { + /* No need to proceed if we are going to do full detect */ + return false; } /* Try to read the source of the interrupt */ @@ -4373,6 +4383,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp) drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); intel_dp_check_link_status(intel_dp); drm_modeset_unlock(&dev->mode_config.connection_mutex); + + return true; } /* XXX this is probably wrong for multiple downstream ports */ @@ -5095,8 +5107,12 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) } } - if (!intel_dp->is_mst) - intel_dp_short_pulse(intel_dp); + if (!intel_dp->is_mst) { + if (!intel_dp_short_pulse(intel_dp)) { + intel_dp_long_pulse(intel_dp->attached_connector); + goto put_power; + } + } } ret = IRQ_HANDLED;
This patch checks for changes in sink count between short pulse hpds and forces full detect when there is a change. This will allow both detection of hotplug and unplug of panels through dongles that give only short pulse for such events. v2: changed variable type from u8 to bool (Jani) return immediately if perform_full_detect is set(Siva) v3: changed method of determining full detection from using pointer to return code (Siva) Tested-by: Nathan D Ciobanu <nathan.d.ciobanu@intel.com> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-)