[Intel-gfx] [PATCH v2] drm: Pass along the hotplug connector to the uevent

Daniel Vetter daniel at ffwll.ch
Fri Oct 21 12:45:41 UTC 2016


On Fri, Oct 21, 2016 at 10:14:21AM +0100, Chris Wilson wrote:
> If we know which connector was plugged/unplugged or
> connected/disconnected, we can pass that information along to userspace
> inside the uevent to reduce the amount of work userspace has to perform
> after the event (i.e. instead of looking over all connectors, it can
> just reprobe the affected one).
> 
> v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
> need the force.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Villle Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare at intel.com>

Nothing is preventing multiple connectors to change state at the same
time. Or at least almost the same time, e.g. yank an entire dp mst chain.

I think we need something that works per-connector, so either send out
uevents for one that changed individually. Or go with the epoche counter
idea. The later has the upside that it disconnects probing from reporting:
Sometimes only deep down in the driver's probe function do we realize that
something changed (e.g. different edid, or different dpcd). With a helper
to increment the epoche there would be no need to wire the hotplug status
through the entire callchain.

To give us the same speed-up benefits like this here the only thing we'd
need to do is make sure reading a read-only (i.e. not userspace setable
prop) doesn't take any heavyweight locks. That should be easy to achieve.
Of course that leaves us with num_connector ioctl calls, but that should
be acceptable.

And it's an uabi change either way.
-Daniel

> ---
>  drivers/gpu/drm/drm_probe_helper.c     | 10 ++++++----
>  drivers/gpu/drm/drm_sysfs.c            | 19 +++++++++++++++----
>  drivers/gpu/drm/i2c/tda998x_drv.c      |  2 +-
>  drivers/gpu/drm/i915/intel_dp.c        |  3 ++-
>  drivers/gpu/drm/i915/intel_dp_mst.c    |  2 +-
>  drivers/gpu/drm/i915/intel_hotplug.c   |  2 +-
>  drivers/gpu/drm/qxl/qxl_display.c      |  2 +-
>  drivers/gpu/drm/radeon/radeon_dp_mst.c |  2 +-
>  drivers/gpu/drm/virtio/virtgpu_vq.c    |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c    |  2 +-
>  include/drm/drmP.h                     |  3 ++-
>  include/drm/drm_crtc_helper.h          |  3 ++-
>  13 files changed, 35 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index f6b64d7d3528..8790ee35a7cd 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -347,6 +347,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
>  /**
>   * drm_kms_helper_hotplug_event - fire off KMS hotplug events
>   * @dev: drm_device whose connector state changed
> + * @connector: connector on which the status changed, if any
>   *
>   * This function fires off the uevent for userspace and also calls the
>   * output_poll_changed function, which is most commonly used to inform the fbdev
> @@ -360,10 +361,11 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
>   * This function must be called from process context with no mode
>   * setting locks held.
>   */
> -void drm_kms_helper_hotplug_event(struct drm_device *dev)
> +void drm_kms_helper_hotplug_event(struct drm_device *dev,
> +				  struct drm_connector *connector)
>  {
>  	/* send a uevent + call fbdev */
> -	drm_sysfs_hotplug_event(dev);
> +	drm_sysfs_hotplug_event(dev, connector);
>  	if (dev->mode_config.funcs->output_poll_changed)
>  		dev->mode_config.funcs->output_poll_changed(dev);
>  }
> @@ -444,7 +446,7 @@ static void output_poll_execute(struct work_struct *work)
>  
>  out:
>  	if (changed)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  
>  	if (repoll)
>  		schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
> @@ -578,7 +580,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
>  	mutex_unlock(&dev->mode_config.mutex);
>  
>  	if (changed)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  
>  	return changed;
>  }
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 9a37196c1bf1..c8f46055e2d0 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -280,7 +280,7 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
>  	}
>  
>  	/* Let userspace know we have a new connector */
> -	drm_sysfs_hotplug_event(dev);
> +	drm_sysfs_hotplug_event(dev, connector);
>  
>  	return 0;
>  }
> @@ -312,19 +312,30 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
>  /**
>   * drm_sysfs_hotplug_event - generate a DRM uevent
>   * @dev: DRM device
> + * @connector: the DRM connector, if any
>   *
>   * Send a uevent for the DRM device specified by @dev.  Currently we only
>   * set HOTPLUG=1 in the uevent environment, but this could be expanded to
>   * deal with other types of events.
>   */
> -void drm_sysfs_hotplug_event(struct drm_device *dev)
> +void drm_sysfs_hotplug_event(struct drm_device *dev,
> +			     struct drm_connector *connector)
>  {
> -	char *event_string = "HOTPLUG=1";
> -	char *envp[] = { event_string, NULL };
> +	char *envp[3] = { "HOTPLUG=1" };
> +	char *connector_event = NULL;
> +
> +	if (connector)
> +		connector_event = kasprintf(GFP_KERNEL,
> +					    "CONNECTOR=%d",
> +					    connector->base.id);
> +	if (connector_event)
> +		envp[1] = connector_event;
>  
>  	DRM_DEBUG("generating hotplug event\n");
>  
>  	kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
> +
> +	kfree(connector_event);
>  }
>  EXPORT_SYMBOL(drm_sysfs_hotplug_event);
>  
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 9798d400d817..9fd873c87686 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -617,7 +617,7 @@ static void tda998x_detect_work(struct work_struct *work)
>  	struct drm_device *dev = priv->encoder.dev;
>  
>  	if (dev)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  }
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 88f3b745a326..59cd185689c0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3981,7 +3981,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>  			intel_dp->is_mst = false;
>  			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
>  			/* send a hotplug event */
> -			drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> +			drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev,
> +						     &intel_dp->attached_connector->base);
>  		}
>  	}
>  	return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 3ffbd69e4551..2bd48a987934 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -493,7 +493,7 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  
> -	drm_kms_helper_hotplug_event(dev);
> +	drm_kms_helper_hotplug_event(dev, &intel_dp->attached_connector->base);
>  }
>  
>  static const struct drm_dp_mst_topology_cbs mst_cbs = {
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 334d47b5811a..da0649aff734 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -340,7 +340,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  	mutex_unlock(&mode_config->mutex);
>  
>  	if (changed)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  }
>  
>  
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
> index 2e01c6e5d905..3c196a096c2d 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -135,7 +135,7 @@ void qxl_display_read_client_monitors_config(struct qxl_device *qdev)
>  	if (!drm_helper_hpd_irq_event(qdev->ddev)) {
>  		/* notify that the monitor configuration changed, to
>  		   adjust at the arbitrary resolution */
> -		drm_kms_helper_hotplug_event(qdev->ddev);
> +		drm_kms_helper_hotplug_event(qdev->ddev, NULL);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> index de504ea29c06..e80b1c365a2d 100644
> --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> @@ -331,7 +331,7 @@ static void radeon_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
>  	struct radeon_connector *master = container_of(mgr, struct radeon_connector, mst_mgr);
>  	struct drm_device *dev = master->base.dev;
>  
> -	drm_kms_helper_hotplug_event(dev);
> +	drm_kms_helper_hotplug_event(dev, NULL);
>  }
>  
>  const struct drm_dp_mst_topology_cbs mst_cbs = {
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 5a0f8a745b9d..72be43c9e008 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -583,7 +583,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
>  	wake_up(&vgdev->resp_wq);
>  
>  	if (!drm_helper_hpd_irq_event(vgdev->ddev))
> -		drm_kms_helper_hotplug_event(vgdev->ddev);
> +		drm_kms_helper_hotplug_event(vgdev->ddev, NULL);
>  }
>  
>  static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index e8ae3dc476d1..36aa1a0440c3 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1233,7 +1233,7 @@ static int vmw_master_set(struct drm_device *dev,
>  	}
>  
>  	dev_priv->active_master = vmaster;
> -	drm_sysfs_hotplug_event(dev);
> +	drm_sysfs_hotplug_event(dev, NULL);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index c965514b82be..26262763aa5f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1407,7 +1407,7 @@ static int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,
>  	}
>  
>  	mutex_unlock(&dev->mode_config.mutex);
> -	drm_sysfs_hotplug_event(dev);
> +	drm_sysfs_hotplug_event(dev, con);
>  
>  	return 0;
>  }
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 672644031bd5..0a4a4aa61fd3 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1038,7 +1038,8 @@ extern struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size,
>  extern void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
>  
>  			       /* sysfs support (drm_sysfs.c) */
> -extern void drm_sysfs_hotplug_event(struct drm_device *dev);
> +extern void drm_sysfs_hotplug_event(struct drm_device *dev,
> +				    struct drm_connector *connector);
>  
>  
>  struct drm_device *drm_dev_alloc(struct drm_driver *driver,
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index 982c299e435a..a8cd620e8a5c 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -69,7 +69,8 @@ extern int drm_helper_probe_single_connector_modes(struct drm_connector
>  extern void drm_kms_helper_poll_init(struct drm_device *dev);
>  extern void drm_kms_helper_poll_fini(struct drm_device *dev);
>  extern bool drm_helper_hpd_irq_event(struct drm_device *dev);
> -extern void drm_kms_helper_hotplug_event(struct drm_device *dev);
> +extern void drm_kms_helper_hotplug_event(struct drm_device *dev,
> +					 struct drm_connector *connector);
>  
>  extern void drm_kms_helper_poll_disable(struct drm_device *dev);
>  extern void drm_kms_helper_poll_enable(struct drm_device *dev);
> -- 
> 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the dri-devel mailing list