Re: [PATCH 2/2] drm/i915: Lane count change detection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 13, 2016 at 11:53:43AM +0530, Shubhangi Shrivastava wrote:
> DP panel can issue short pulse interrupts during which it can
> modify the number of lanes supported by it. This will result in
> change in capabilities of the display and limit the max
> resoultion possible on it. This should be informed to the
> user mode as well since HWC will issue the modeset assuming
> old capabilities.
> 
> This patch detects lane count change and simulates a detatch
> and attach, so to the user mode or anyone else tracking the
> display it will appear as unplug and replug of different
> display.
> 
> v2: moved queuing of delayed thread to intel_dp where we
> set simulate_disconnect_connect flag. There is a chance
> for short pulse to be hit even before long pulse detection
> is complete, in such a scenario the delayed thread won't
> be queued resulting in flag never getting cleared.
> Since we set the flag and queue it immediately we can be
> sure that the flag will be cleared.
> 
> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@xxxxxxxxx>
> Signed-off-by: Shubhangi Shrivastava <shubhangi.shrivastava@xxxxxxxxx>

This is nonsense, just send a uevent to userspace, and make sure that we
re-validate all the modes. Userspace is supposed to ask for updated mode
lists after every uevent, and then reset a suitable mode.

If hwc can't do that, then we're not going to add a horrible hack to the
kernel of temporarily unplugging something, since fundamentally that's
just duct-tape and still racy.

Note that right now we don't have support to generate uevents for when the
connector status hasn't changed. But we need that in a lot of other cases
too. I think the simplest option is to add a sink_lifetime counter
to drm_connector, and a helper to increment that, e.g.
drm_connector_sink_change().

Then we could call that from our probe/hotplug hooks every time something
relevant changes (different sink dongle, different number of lanes,
different edid). And the top-level probe helpers could also check for any
changes in the sink_lifetime instead of just connect/disconnect. I think
it'd be good if we also wire this up for edid changes, in the core
function which updates the edid blob.

Cheers, Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/i915_irq.c      |  2 ++
>  drivers/gpu/drm/i915/intel_dp.c      | 47 ++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_drv.h     |  4 +++
>  drivers/gpu/drm/i915/intel_hotplug.c | 50 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 100 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7a0b513..30d3636 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -280,6 +280,7 @@ struct i915_hotplug {
>  	u32 long_port_mask;
>  	u32 short_port_mask;
>  	struct work_struct dig_port_work;
> +	struct delayed_work simulate_work;
>  
>  	/*
>  	 * if we get a HPD irq from DP and a HPD irq from non-DP
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index f0d9414..74fe755 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4572,6 +4572,8 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>  
>  	INIT_WORK(&dev_priv->rps.work, gen6_pm_rps_work);
>  	INIT_WORK(&dev_priv->l3_parity.error_work, ivybridge_parity_work);
> +	INIT_DELAYED_WORK(&dev_priv->hotplug.simulate_work,
> +			  intel_hpd_simulate_reconnect_work);
>  
>  	/* Let's track the enabled rps events */
>  	if (IS_VALLEYVIEW(dev_priv))
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d5ed84f..ebd525e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3783,6 +3783,26 @@ update_status:
>  	}
>  }
>  
> +static void intel_dp_update_simulate_detach_info(struct intel_dp *intel_dp)
> +{
> +	struct intel_connector *intel_connector = intel_dp->attached_connector;
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	/*
> +	 * Queue thread only if it is not already done.
> +	 * The flag is cleared inside the callback function
> +	 * hence we can be sure that there is no race condition
> +	 * under which it may remain set.
> +	 */
> +	if (!intel_connector->simulate_disconnect_connect) {
> +		intel_connector->simulate_disconnect_connect = true;
> +		DRM_DEBUG_KMS("Queue simulate work func\n");
> +		mod_delayed_work(system_wq, &dev_priv->hotplug.simulate_work,
> +				 msecs_to_jiffies(100));
> +	}
> +}
> +
>  static int
>  intel_dp_check_mst_status(struct intel_dp *intel_dp)
>  {
> @@ -3893,6 +3913,7 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>  	u8 sink_irq_vector;
>  	u8 old_sink_count = intel_dp->sink_count;
> +	u8 old_lane_count = intel_dp->dpcd[DP_MAX_LANE_COUNT];
>  	bool ret;
>  
>  	/*
> @@ -3924,6 +3945,20 @@ intel_dp_short_pulse(struct intel_dp *intel_dp)
>  			sink_irq_vector &= ~DP_AUTOMATED_TEST_REQUEST;
>  		}
>  
> +		/*
> +		 * If lane count has changed, we need to inform
> +		 * user mode of new capablities, this is done by setting
> +		 * our flag to do a fake disconnect and connect so it
> +		 * will appear to the user mode that a new panel is
> +		 * connected and will use the new capabilties of the
> +		 * panel
> +		 */
> +		if (old_lane_count != intel_dp->dpcd[DP_MAX_LANE_COUNT]) {
> +			DRM_DEBUG_KMS("Lane count changed\n");
> +			intel_dp_update_simulate_detach_info(intel_dp);
> +			return false;
> +		}
> +
>  		if (sink_irq_vector & (DP_CP_IRQ | DP_SINK_SPECIFIC_IRQ))
>  			DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n");
>  
> @@ -4213,13 +4248,17 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  	intel_display_power_get(to_i915(dev), power_domain);
>  
>  	/* Can't disconnect eDP, but you can close the lid... */
> -	if (is_edp(intel_dp))
> +	if (is_edp(intel_dp)) {
>  		status = edp_detect(intel_dp);
> -	else if (intel_digital_port_connected(to_i915(dev),
> -					      dp_to_dig_port(intel_dp)))
> +	} else if (intel_connector->simulate_disconnect_connect) {
> +		DRM_DEBUG_KMS("Simulating disconnect\n");
> +		status = connector_status_disconnected;
> +	} else if (intel_digital_port_connected(to_i915(dev),
> +					      dp_to_dig_port(intel_dp))) {
>  		status = intel_dp_detect_dpcd(intel_dp);
> -	else
> +	} else {
>  		status = connector_status_disconnected;
> +	}
>  
>  	if (status != connector_status_connected) {
>  		intel_dp->compliance_test_active = 0;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8daadc5..995f0da 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -257,6 +257,8 @@ struct intel_connector {
>  	struct edid *edid;
>  	struct edid *detect_edid;
>  
> +	bool simulate_disconnect_connect;
> +
>  	/* since POLL and HPD connectors may use the same HPD line keep the native
>  	   state of connector->polled in case hotplug storm detection changes it */
>  	u8 polled;
> @@ -1420,6 +1422,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  			       struct intel_crtc_state *pipe_config);
>  void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable);
>  
> +/* intel_hotplug.c */
> +void intel_hpd_simulate_reconnect_work(struct work_struct *work);
>  
>  /* intel_lvds.c */
>  void intel_lvds_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 38eeca7..7e346d7 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -247,6 +247,55 @@ static bool intel_hpd_irq_event(struct drm_device *dev,
>  	return true;
>  }
>  
> +/*
> + * This function is the second half of logic to perform fake
> + * disconnect-connect. The first half was the connector setting
> + * a flag, returning status as disconnected and queing this function.
> + *
> + * This function uses simulate_disconnect_connect flag to identify the
> + * connector that should be detected again. Since this is executed after
> + * a delay if the panel is still plugged in it will be reported as
> + * connected to user mode
> + */
> +void intel_hpd_simulate_reconnect_work(struct work_struct *work)
> +{
> +	struct drm_i915_private *dev_priv =
> +		container_of(work, struct drm_i915_private,
> +			     hotplug.simulate_work.work);
> +	struct drm_device *dev = dev_priv->dev;
> +	struct intel_encoder *intel_encoder;
> +	struct intel_connector *intel_connector;
> +	struct drm_connector *connector;
> +	struct drm_mode_config *mode_config = &dev->mode_config;
> +	enum port port;
> +
> +	DRM_DEBUG_KMS("\n");
> +	mutex_lock(&mode_config->mutex);
> +
> +	list_for_each_entry(connector, &mode_config->connector_list, head) {
> +		intel_connector = to_intel_connector(connector);
> +		if (!intel_connector->simulate_disconnect_connect)
> +			continue;
> +
> +		intel_connector->simulate_disconnect_connect = false;
> +
> +		if (!intel_connector->encoder)
> +			continue;
> +
> +		intel_encoder = intel_connector->encoder;
> +
> +		port = intel_ddi_get_encoder_port(intel_encoder);
> +
> +		spin_lock_irq(&dev_priv->irq_lock);
> +		dev_priv->hotplug.long_port_mask |= (1<<port);
> +		spin_unlock_irq(&dev_priv->irq_lock);
> +	}
> +
> +	mutex_unlock(&mode_config->mutex);
> +
> +	queue_work(dev_priv->hotplug.dp_wq, &dev_priv->hotplug.dig_port_work);
> +}
> +
>  static void i915_digport_work_func(struct work_struct *work)
>  {
>  	struct drm_i915_private *dev_priv =
> @@ -509,4 +558,5 @@ void intel_hpd_cancel_work(struct drm_i915_private *dev_priv)
>  	cancel_work_sync(&dev_priv->hotplug.dig_port_work);
>  	cancel_work_sync(&dev_priv->hotplug.hotplug_work);
>  	cancel_delayed_work_sync(&dev_priv->hotplug.reenable_work);
> +	cancel_delayed_work_sync(&dev_priv->hotplug.simulate_work);
>  }
> -- 
> 2.6.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux