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@xxxxxxxxxxxxxxxxxx>
> Cc: Villle Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx>
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@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
