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
