[PATCH] qxl: add a connector property to denote hotplug should rescan modes.

[PATCH] qxl: add a connector property to denote hotplug should rescan modes.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

So GNOME userspace has an issue with when it rescans for modes on hotplug
events, if the monitor has no EDID it assumes that nothing has changed on
EDID as with real hw we'd never have new modes without a new EDID, and they
kind off rely on the behaviour now, however with virtual GPUs we would
like to rescan the modes and get a new preferred mode on hotplug events
to handle dynamic guest resizing (where you resize the host window and the
guest resizes with it).

This is a simple property we can make userspace watch for to trigger new
behaviour based on it, and can be used to replaced EDID hacks in virtual
drivers.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_display.c | 18 ++++++++++++++++++
 drivers/gpu/drm/qxl/qxl_drv.h     |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 835caba..d83a0b7 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -835,8 +835,21 @@ static const struct drm_encoder_funcs qxl_enc_funcs = {
 	.destroy = qxl_enc_destroy,
 };
 
+static int qxl_mode_create_hotplug_mode_update_property(struct qxl_device *qdev)
+{
+	if (qdev->hotplug_mode_update_property)
+		return 0;
+
+	qdev->hotplug_mode_update_property =
+		drm_property_create_range(qdev->ddev, DRM_MODE_PROP_IMMUTABLE,
+					  "hotplug_mode_update", 0, 1);
+
+	return 0;
+}
+
 static int qdev_output_init(struct drm_device *dev, int num_output)
 {
+	struct qxl_device *qdev = dev->dev_private;
 	struct qxl_output *qxl_output;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
@@ -863,6 +876,8 @@ static int qdev_output_init(struct drm_device *dev, int num_output)
 	drm_encoder_helper_add(encoder, &qxl_enc_helper_funcs);
 	drm_connector_helper_add(connector, &qxl_connector_helper_funcs);
 
+	drm_object_attach_property(&connector->base,
+				   qdev->hotplug_mode_update_property, 0);
 	drm_sysfs_connector_add(connector);
 	return 0;
 }
@@ -975,6 +990,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
 	qdev->ddev->mode_config.max_height = 8192;
 
 	qdev->ddev->mode_config.fb_base = qdev->vram_base;
+
+	qxl_mode_create_hotplug_mode_update_property(qdev);
+
 	for (i = 0 ; i < qxl_num_crtc; ++i) {
 		qdev_crtc_init(qdev->ddev, i);
 		qdev_output_init(qdev->ddev, i);
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 41d22ed..7bda32f 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -323,6 +323,8 @@ struct qxl_device {
 	struct work_struct gc_work;
 
 	struct work_struct fb_work;
+
+	struct drm_property *hotplug_mode_update_property;
 };
 
 /* forward declaration for QXL_INFO_IO */
-- 
1.8.3.1

Re: [PATCH] qxl: add a connector property to denote hotplug should rescan modes.

[Daniel Vetter]

On Fri, Oct 11, 2013 at 11:05:00AM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> So GNOME userspace has an issue with when it rescans for modes on hotplug
> events, if the monitor has no EDID it assumes that nothing has changed on
> EDID as with real hw we'd never have new modes without a new EDID, and they
> kind off rely on the behaviour now, however with virtual GPUs we would
> like to rescan the modes and get a new preferred mode on hotplug events
> to handle dynamic guest resizing (where you resize the host window and the
> guest resizes with it).
> 
> This is a simple property we can make userspace watch for to trigger new
> behaviour based on it, and can be used to replaced EDID hacks in virtual
> drivers.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>

There's the related problem that we kinda don't tell userspace which
connectors exactly changed when sending out a hotplug event. We could
solve both issues with a hotplug_sequence counter that increments on any
change. Probably more work though ;-)
-Daniel

> ---
>  drivers/gpu/drm/qxl/qxl_display.c | 18 ++++++++++++++++++
>  drivers/gpu/drm/qxl/qxl_drv.h     |  2 ++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
> index 835caba..d83a0b7 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -835,8 +835,21 @@ static const struct drm_encoder_funcs qxl_enc_funcs = {
>  	.destroy = qxl_enc_destroy,
>  };
>  
> +static int qxl_mode_create_hotplug_mode_update_property(struct qxl_device *qdev)
> +{
> +	if (qdev->hotplug_mode_update_property)
> +		return 0;
> +
> +	qdev->hotplug_mode_update_property =
> +		drm_property_create_range(qdev->ddev, DRM_MODE_PROP_IMMUTABLE,
> +					  "hotplug_mode_update", 0, 1);
> +
> +	return 0;
> +}
> +
>  static int qdev_output_init(struct drm_device *dev, int num_output)
>  {
> +	struct qxl_device *qdev = dev->dev_private;
>  	struct qxl_output *qxl_output;
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
> @@ -863,6 +876,8 @@ static int qdev_output_init(struct drm_device *dev, int num_output)
>  	drm_encoder_helper_add(encoder, &qxl_enc_helper_funcs);
>  	drm_connector_helper_add(connector, &qxl_connector_helper_funcs);
>  
> +	drm_object_attach_property(&connector->base,
> +				   qdev->hotplug_mode_update_property, 0);
>  	drm_sysfs_connector_add(connector);
>  	return 0;
>  }
> @@ -975,6 +990,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
>  	qdev->ddev->mode_config.max_height = 8192;
>  
>  	qdev->ddev->mode_config.fb_base = qdev->vram_base;
> +
> +	qxl_mode_create_hotplug_mode_update_property(qdev);
> +
>  	for (i = 0 ; i < qxl_num_crtc; ++i) {
>  		qdev_crtc_init(qdev->ddev, i);
>  		qdev_output_init(qdev->ddev, i);
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> index 41d22ed..7bda32f 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.h
> +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> @@ -323,6 +323,8 @@ struct qxl_device {
>  	struct work_struct gc_work;
>  
>  	struct work_struct fb_work;
> +
> +	struct drm_property *hotplug_mode_update_property;
>  };
>  
>  /* forward declaration for QXL_INFO_IO */
> -- 
> 1.8.3.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] qxl: add a connector property to denote hotplug should rescan modes.

[Dave Airlie]

On Fri, Oct 11, 2013 at 6:21 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Oct 11, 2013 at 11:05:00AM +1000, Dave Airlie wrote:
>> From: Dave Airlie <airlied@redhat.com>
>>
>> So GNOME userspace has an issue with when it rescans for modes on hotplug
>> events, if the monitor has no EDID it assumes that nothing has changed on
>> EDID as with real hw we'd never have new modes without a new EDID, and they
>> kind off rely on the behaviour now, however with virtual GPUs we would
>> like to rescan the modes and get a new preferred mode on hotplug events
>> to handle dynamic guest resizing (where you resize the host window and the
>> guest resizes with it).
>>
>> This is a simple property we can make userspace watch for to trigger new
>> behaviour based on it, and can be used to replaced EDID hacks in virtual
>> drivers.
>>
>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>
> There's the related problem that we kinda don't tell userspace which
> connectors exactly changed when sending out a hotplug event. We could
> solve both issues with a hotplug_sequence counter that increments on any
> change. Probably more work though ;-)

Its kinda orthogonal problem though, I don't mind reprobing its more for the
case where userspace does nothing when we have no EDID I need to fix.

Dave.