This change adds the ability to read or write a "1" or a "0" to the
newly added "connected" attribute of a connector in the vkms entry in
configfs.
A write will trigger a call to drm_kms_helper_hotplug_event, causing a
hotplug uevent.
With this we can write virtualized multidisplay tests that involve
hotplugging displays (eg recompositing windows when a monitor is turned
off).
Signed-off-by: Brandon Pollack <brpol@chromium.org>
---
Documentation/gpu/vkms.rst | 2 +-
drivers/gpu/drm/vkms/vkms_configfs.c | 68 ++++++++++++++++++++++++++--
drivers/gpu/drm/vkms/vkms_drv.h | 11 +++++
drivers/gpu/drm/vkms/vkms_output.c | 47 ++++++++++++++++++-
4 files changed, 123 insertions(+), 5 deletions(-)
diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index c3875bf66dba..7f715097539c 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -145,7 +145,7 @@ We want to be able to manipulate vkms instances without having to reload the
module. Such configuration can be added as extensions to vkms's ConfigFS
support. Use-cases:
-- Hotplug/hotremove connectors on the fly (to be able to test DP MST handling
+- Hotremove connectors on the fly (to be able to test DP MST handling
of compositors).
- Change output configuration: Plug/unplug screens, change EDID, allow changing
diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
index bc35dcc47585..d231e28101ae 100644
--- a/drivers/gpu/drm/vkms/vkms_configfs.c
+++ b/drivers/gpu/drm/vkms/vkms_configfs.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
+#include "drm/drm_probe_helper.h"
#include <linux/configfs.h>
#include <linux/mutex.h>
#include <linux/platform_device.h>
@@ -40,6 +41,7 @@
* `-- vkms
* `-- test
* |-- connectors
+ * `-- connected
* |-- crtcs
* |-- encoders
* |-- planes
@@ -89,6 +91,14 @@
*
* echo 1 > /config/vkms/test/enabled
*
+ * By default no display is "connected" so to connect a connector you'll also
+ * have to write 1 to a connectors "connected" attribute::
+ *
+ * echo 1 > /config/vkms/test/connectors/connector/connected
+ *
+ * One can verify that this is worked using the `modetest` utility or the
+ * equivalent for your platform.
+ *
* When you're done with the virtual device, you can clean up the device like
* so::
*
@@ -236,7 +246,58 @@ static void add_possible_encoders(struct config_group *parent,
/* Connector item, e.g. /config/vkms/device/connectors/ID */
+static ssize_t connector_connected_show(struct config_item *item, char *buf)
+{
+ struct vkms_config_connector *connector =
+ item_to_config_connector(item);
+ struct vkms_configfs *configfs = connector_item_to_configfs(item);
+ bool connected = false;
+
+ mutex_lock(&configfs->lock);
+ connected = connector->connected;
+ mutex_unlock(&configfs->lock);
+
+ return sprintf(buf, "%d\n", connected);
+}
+
+static ssize_t connector_connected_store(struct config_item *item,
+ const char *buf, size_t len)
+{
+ struct vkms_config_connector *connector =
+ item_to_config_connector(item);
+ struct vkms_configfs *configfs = connector_item_to_configfs(item);
+ int val, ret;
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val != 1 && val != 0)
+ return -EINVAL;
+
+ mutex_lock(&configfs->lock);
+ connector->connected = val;
+ if (!connector->connector) {
+ pr_info("VKMS Device %s is not yet enabled, connector will be enabled on start",
+ configfs->device_group.cg_item.ci_name);
+ }
+ mutex_unlock(&configfs->lock);
+
+ if (connector->connector)
+ drm_kms_helper_hotplug_event(connector->connector->dev);
+
+ return len;
+}
+
+CONFIGFS_ATTR(connector_, connected);
+
+static struct configfs_attribute *connector_attrs[] = {
+ &connector_attr_connected,
+ NULL,
+};
+
static struct config_item_type connector_type = {
+ .ct_attrs = connector_attrs,
.ct_owner = THIS_MODULE,
};
@@ -264,7 +325,7 @@ static ssize_t plane_type_show(struct config_item *item, char *buf)
plane_type = plane->type;
mutex_unlock(&configfs->lock);
- return sprintf(buf, "%u", plane_type);
+ return sprintf(buf, "%u\n", plane_type);
}
static ssize_t plane_type_store(struct config_item *item, const char *buf,
@@ -319,6 +380,7 @@ static struct config_group *connectors_group_make(struct config_group *group,
&connector_type);
add_possible_encoders(&connector->config_group,
&connector->possible_encoders.group);
+ connector->connected = false;
return &connector->config_group;
}
@@ -500,7 +562,7 @@ static ssize_t device_enabled_show(struct config_item *item, char *buf)
is_enabled = configfs->vkms_device != NULL;
mutex_unlock(&configfs->lock);
- return sprintf(buf, "%d", is_enabled);
+ return sprintf(buf, "%d\n", is_enabled);
}
static ssize_t device_enabled_store(struct config_item *item, const char *buf,
@@ -557,7 +619,7 @@ static ssize_t device_id_show(struct config_item *item, char *buf)
mutex_unlock(&configfs->lock);
- return sprintf(buf, "%d", id);
+ return sprintf(buf, "%d\n", id);
}
CONFIGFS_ATTR_RO(device_, id);
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 2b9545ada9c2..5336281f397e 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -3,6 +3,7 @@
#ifndef _VKMS_DRV_H_
#define _VKMS_DRV_H_
+#include "drm/drm_connector.h"
#include <linux/configfs.h>
#include <linux/hrtimer.h>
@@ -147,7 +148,9 @@ struct vkms_config_links {
struct vkms_config_connector {
struct config_group config_group;
+ struct drm_connector *connector;
struct vkms_config_links possible_encoders;
+ bool connected;
};
struct vkms_config_crtc {
@@ -220,6 +223,10 @@ struct vkms_device {
#define item_to_configfs(item) \
container_of(to_config_group(item), struct vkms_configfs, device_group)
+#define connector_item_to_configfs(item) \
+ container_of(to_config_group(item->ci_parent), struct vkms_configfs, \
+ connectors_group)
+
#define item_to_config_connector(item) \
container_of(to_config_group(item), struct vkms_config_connector, \
config_group)
@@ -279,4 +286,8 @@ int vkms_enable_writeback_connector(struct vkms_device *vkmsdev,
int vkms_init_configfs(void);
void vkms_unregister_configfs(void);
+/* Connector hotplugging */
+enum drm_connector_status vkms_connector_detect(struct drm_connector *connector,
+ bool force);
+
#endif /* _VKMS_DRV_H_ */
diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
index 0ee1f3f4a305..1a1cd0202c5f 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
+#include <drm/drm_print.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_connector.h>
#include <drm/drm_crtc.h>
@@ -8,10 +9,12 @@
#include <drm/drm_plane.h>
#include <drm/drm_probe_helper.h>
#include <drm/drm_simple_kms_helper.h>
+#include <linux/printk.h>
#include "vkms_drv.h"
static const struct drm_connector_funcs vkms_connector_funcs = {
+ .detect = vkms_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.destroy = drm_connector_cleanup,
.reset = drm_atomic_helper_connector_reset,
@@ -19,6 +22,48 @@ static const struct drm_connector_funcs vkms_connector_funcs = {
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
+static const struct vkms_config_connector *
+find_config_for_connector(struct drm_connector *connector)
+{
+ struct vkms_device *vkms = drm_device_to_vkms_device(connector->dev);
+ struct vkms_configfs *configfs = vkms->configfs;
+ struct config_item *item;
+
+ if (!configfs) {
+ pr_info("Default connector has no configfs entry");
+ return NULL;
+ }
+
+ list_for_each_entry(item, &configfs->connectors_group.cg_children,
+ ci_entry) {
+ struct vkms_config_connector *config_connector =
+ item_to_config_connector(item);
+ if (config_connector->connector == connector)
+ return config_connector;
+ }
+
+ pr_warn("Could not find config to match connector %s, but configfs was initialized",
+ connector->name);
+
+ return NULL;
+}
+
+enum drm_connector_status vkms_connector_detect(struct drm_connector *connector,
+ bool force)
+{
+ enum drm_connector_status status = connector_status_connected;
+ const struct vkms_config_connector *config_connector =
+ find_config_for_connector(connector);
+
+ if (!config_connector)
+ return connector_status_connected;
+
+ if (!config_connector->connected)
+ status = connector_status_disconnected;
+
+ return status;
+}
+
static const struct drm_encoder_funcs vkms_encoder_funcs = {
.destroy = drm_encoder_cleanup,
};
@@ -280,12 +325,12 @@ int vkms_output_init(struct vkms_device *vkmsdev)
struct vkms_config_connector *config_connector =
item_to_config_connector(item);
struct drm_connector *connector = vkms_connector_init(vkmsdev);
-
if (IS_ERR(connector)) {
DRM_ERROR("Failed to init connector from config: %s",
item->ci_name);
return PTR_ERR(connector);
}
+ config_connector->connector = connector;
for (int j = 0; j < output->num_encoders; j++) {
struct encoder_map *encoder = &encoder_map[j];
--
2.42.0.rc2.253.gd59a3bf2b4-googOn Tue, Aug 29, 2023 at 05:30:59AM +0000, Brandon Pollack wrote:
> This change adds the ability to read or write a "1" or a "0" to the
> newly added "connected" attribute of a connector in the vkms entry in
> configfs.
>
> A write will trigger a call to drm_kms_helper_hotplug_event, causing a
> hotplug uevent.
>
> With this we can write virtualized multidisplay tests that involve
> hotplugging displays (eg recompositing windows when a monitor is turned
> off).
>
> Signed-off-by: Brandon Pollack <brpol@chromium.org>
> ---
> Documentation/gpu/vkms.rst | 2 +-
> drivers/gpu/drm/vkms/vkms_configfs.c | 68 ++++++++++++++++++++++++++--
> drivers/gpu/drm/vkms/vkms_drv.h | 11 +++++
> drivers/gpu/drm/vkms/vkms_output.c | 47 ++++++++++++++++++-
> 4 files changed, 123 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> index c3875bf66dba..7f715097539c 100644
> --- a/Documentation/gpu/vkms.rst
> +++ b/Documentation/gpu/vkms.rst
> @@ -145,7 +145,7 @@ We want to be able to manipulate vkms instances without having to reload the
> module. Such configuration can be added as extensions to vkms's ConfigFS
> support. Use-cases:
>
> -- Hotplug/hotremove connectors on the fly (to be able to test DP MST handling
> +- Hotremove connectors on the fly (to be able to test DP MST handling
> of compositors).
>
> - Change output configuration: Plug/unplug screens, change EDID, allow changing
> diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> index bc35dcc47585..d231e28101ae 100644
> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0+
>
> +#include "drm/drm_probe_helper.h"
> #include <linux/configfs.h>
> #include <linux/mutex.h>
> #include <linux/platform_device.h>
> @@ -40,6 +41,7 @@
> * `-- vkms
> * `-- test
> * |-- connectors
> + * `-- connected
> * |-- crtcs
> * |-- encoders
> * |-- planes
> @@ -89,6 +91,14 @@
> *
> * echo 1 > /config/vkms/test/enabled
> *
> + * By default no display is "connected" so to connect a connector you'll also
> + * have to write 1 to a connectors "connected" attribute::
> + *
> + * echo 1 > /config/vkms/test/connectors/connector/connected
I think it'd be really good if we allow all connector status values,
including unknown. It's not very common, which is why most compositors
utterly fail at handling it in a reasonable way.
> + *
> + * One can verify that this is worked using the `modetest` utility or the
> + * equivalent for your platform.
> + *
> * When you're done with the virtual device, you can clean up the device like
> * so::
> *
> @@ -236,7 +246,58 @@ static void add_possible_encoders(struct config_group *parent,
>
> /* Connector item, e.g. /config/vkms/device/connectors/ID */
>
> +static ssize_t connector_connected_show(struct config_item *item, char *buf)
> +{
> + struct vkms_config_connector *connector =
> + item_to_config_connector(item);
> + struct vkms_configfs *configfs = connector_item_to_configfs(item);
> + bool connected = false;
> +
> + mutex_lock(&configfs->lock);
> + connected = connector->connected;
> + mutex_unlock(&configfs->lock);
> +
> + return sprintf(buf, "%d\n", connected);
> +}
> +
> +static ssize_t connector_connected_store(struct config_item *item,
> + const char *buf, size_t len)
> +{
> + struct vkms_config_connector *connector =
> + item_to_config_connector(item);
> + struct vkms_configfs *configfs = connector_item_to_configfs(item);
> + int val, ret;
> +
> + ret = kstrtouint(buf, 10, &val);
> + if (ret)
> + return ret;
> +
> + if (val != 1 && val != 0)
> + return -EINVAL;
> +
> + mutex_lock(&configfs->lock);
> + connector->connected = val;
> + if (!connector->connector) {
> + pr_info("VKMS Device %s is not yet enabled, connector will be enabled on start",
> + configfs->device_group.cg_item.ci_name);
> + }
> + mutex_unlock(&configfs->lock);
> +
> + if (connector->connector)
> + drm_kms_helper_hotplug_event(connector->connector->dev);
Ok a few lifetime bugs here:
- Calling drm_kms_helper_hotplug_event after you unluck means all the drm
stuff might have disappeared meanwhile. Oops.
- It is worse, because switching to configfs_subsystem.su_mutex will not
prevent the race, because the vkms_device can disappear independently
(by manually unbinding the driver in sysfs) at least with the real
platform driver approach. This is another reason why I'm not sure having
a real platform driver with probe/remove hooks is a good idea.
- Furthermore the drm_connector might also disappear.
I think the way to properly fix this is:
- configfs needs to hold a reference of it's on to the drm_device in
vkms_device.
- it needs to call a vkms function to update the connector hotplug status
with only the configfs obj idx. That function then needs to find the
right drm_connector using the drm_connector_iter functions (which will
sort out any lifetime/locking issues) until is has the right one, and
then update the connector status.
No matter what, we cannot have a backpointer from any drm object to
configfs, that doesn't work correctly.
-Sima
> +
> + return len;
> +}
> +
> +CONFIGFS_ATTR(connector_, connected);
> +
> +static struct configfs_attribute *connector_attrs[] = {
> + &connector_attr_connected,
> + NULL,
> +};
> +
> static struct config_item_type connector_type = {
> + .ct_attrs = connector_attrs,
> .ct_owner = THIS_MODULE,
> };
>
> @@ -264,7 +325,7 @@ static ssize_t plane_type_show(struct config_item *item, char *buf)
> plane_type = plane->type;
> mutex_unlock(&configfs->lock);
>
> - return sprintf(buf, "%u", plane_type);
> + return sprintf(buf, "%u\n", plane_type);
> }
>
> static ssize_t plane_type_store(struct config_item *item, const char *buf,
> @@ -319,6 +380,7 @@ static struct config_group *connectors_group_make(struct config_group *group,
> &connector_type);
> add_possible_encoders(&connector->config_group,
> &connector->possible_encoders.group);
> + connector->connected = false;
>
> return &connector->config_group;
> }
> @@ -500,7 +562,7 @@ static ssize_t device_enabled_show(struct config_item *item, char *buf)
> is_enabled = configfs->vkms_device != NULL;
> mutex_unlock(&configfs->lock);
>
> - return sprintf(buf, "%d", is_enabled);
> + return sprintf(buf, "%d\n", is_enabled);
> }
>
> static ssize_t device_enabled_store(struct config_item *item, const char *buf,
> @@ -557,7 +619,7 @@ static ssize_t device_id_show(struct config_item *item, char *buf)
>
> mutex_unlock(&configfs->lock);
>
> - return sprintf(buf, "%d", id);
> + return sprintf(buf, "%d\n", id);
> }
>
> CONFIGFS_ATTR_RO(device_, id);
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 2b9545ada9c2..5336281f397e 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -3,6 +3,7 @@
> #ifndef _VKMS_DRV_H_
> #define _VKMS_DRV_H_
>
> +#include "drm/drm_connector.h"
> #include <linux/configfs.h>
> #include <linux/hrtimer.h>
>
> @@ -147,7 +148,9 @@ struct vkms_config_links {
>
> struct vkms_config_connector {
> struct config_group config_group;
> + struct drm_connector *connector;
> struct vkms_config_links possible_encoders;
> + bool connected;
> };
>
> struct vkms_config_crtc {
> @@ -220,6 +223,10 @@ struct vkms_device {
> #define item_to_configfs(item) \
> container_of(to_config_group(item), struct vkms_configfs, device_group)
>
> +#define connector_item_to_configfs(item) \
> + container_of(to_config_group(item->ci_parent), struct vkms_configfs, \
> + connectors_group)
> +
> #define item_to_config_connector(item) \
> container_of(to_config_group(item), struct vkms_config_connector, \
> config_group)
> @@ -279,4 +286,8 @@ int vkms_enable_writeback_connector(struct vkms_device *vkmsdev,
> int vkms_init_configfs(void);
> void vkms_unregister_configfs(void);
>
> +/* Connector hotplugging */
> +enum drm_connector_status vkms_connector_detect(struct drm_connector *connector,
> + bool force);
> +
> #endif /* _VKMS_DRV_H_ */
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 0ee1f3f4a305..1a1cd0202c5f 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0+
>
> +#include <drm/drm_print.h>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_connector.h>
> #include <drm/drm_crtc.h>
> @@ -8,10 +9,12 @@
> #include <drm/drm_plane.h>
> #include <drm/drm_probe_helper.h>
> #include <drm/drm_simple_kms_helper.h>
> +#include <linux/printk.h>
>
> #include "vkms_drv.h"
>
> static const struct drm_connector_funcs vkms_connector_funcs = {
> + .detect = vkms_connector_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .destroy = drm_connector_cleanup,
> .reset = drm_atomic_helper_connector_reset,
> @@ -19,6 +22,48 @@ static const struct drm_connector_funcs vkms_connector_funcs = {
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> };
>
> +static const struct vkms_config_connector *
> +find_config_for_connector(struct drm_connector *connector)
> +{
> + struct vkms_device *vkms = drm_device_to_vkms_device(connector->dev);
> + struct vkms_configfs *configfs = vkms->configfs;
> + struct config_item *item;
> +
> + if (!configfs) {
> + pr_info("Default connector has no configfs entry");
> + return NULL;
> + }
> +
> + list_for_each_entry(item, &configfs->connectors_group.cg_children,
> + ci_entry) {
> + struct vkms_config_connector *config_connector =
> + item_to_config_connector(item);
> + if (config_connector->connector == connector)
> + return config_connector;
> + }
> +
> + pr_warn("Could not find config to match connector %s, but configfs was initialized",
> + connector->name);
> +
> + return NULL;
> +}
> +
> +enum drm_connector_status vkms_connector_detect(struct drm_connector *connector,
> + bool force)
> +{
> + enum drm_connector_status status = connector_status_connected;
> + const struct vkms_config_connector *config_connector =
> + find_config_for_connector(connector);
> +
> + if (!config_connector)
> + return connector_status_connected;
> +
> + if (!config_connector->connected)
> + status = connector_status_disconnected;
> +
> + return status;
> +}
> +
> static const struct drm_encoder_funcs vkms_encoder_funcs = {
> .destroy = drm_encoder_cleanup,
> };
> @@ -280,12 +325,12 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> struct vkms_config_connector *config_connector =
> item_to_config_connector(item);
> struct drm_connector *connector = vkms_connector_init(vkmsdev);
> -
> if (IS_ERR(connector)) {
> DRM_ERROR("Failed to init connector from config: %s",
> item->ci_name);
> return PTR_ERR(connector);
> }
> + config_connector->connector = connector;
>
> for (int j = 0; j < output->num_encoders; j++) {
> struct encoder_map *encoder = &encoder_map[j];
> --
> 2.42.0.rc2.253.gd59a3bf2b4-goog
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Hello!
Thanks for the patch.
On 29/08/2023 02:30, Brandon Pollack wrote:
> This change adds the ability to read or write a "1" or a "0" to the
> newly added "connected" attribute of a connector in the vkms entry in
> configfs.
>
> A write will trigger a call to drm_kms_helper_hotplug_event, causing a
> hotplug uevent.
>
> With this we can write virtualized multidisplay tests that involve
> hotplugging displays (eg recompositing windows when a monitor is turned
> off).
Are these tests going to be added in igt?
I was just wondering if it requires any special thing for drm ci:
https://lists.freedesktop.org/archives/dri-devel/2023-September/423719.html
(btw, it would be awesome of you could test your changes with drm ci :)
Regards,
Helen
>
> Signed-off-by: Brandon Pollack <brpol@chromium.org>
> ---
> Documentation/gpu/vkms.rst | 2 +-
> drivers/gpu/drm/vkms/vkms_configfs.c | 68 ++++++++++++++++++++++++++--
> drivers/gpu/drm/vkms/vkms_drv.h | 11 +++++
> drivers/gpu/drm/vkms/vkms_output.c | 47 ++++++++++++++++++-
> 4 files changed, 123 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> index c3875bf66dba..7f715097539c 100644
> --- a/Documentation/gpu/vkms.rst
> +++ b/Documentation/gpu/vkms.rst
> @@ -145,7 +145,7 @@ We want to be able to manipulate vkms instances without having to reload the
> module. Such configuration can be added as extensions to vkms's ConfigFS
> support. Use-cases:
>
> -- Hotplug/hotremove connectors on the fly (to be able to test DP MST handling
> +- Hotremove connectors on the fly (to be able to test DP MST handling
> of compositors).
>
> - Change output configuration: Plug/unplug screens, change EDID, allow changing
> diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> index bc35dcc47585..d231e28101ae 100644
> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0+
>
> +#include "drm/drm_probe_helper.h"
> #include <linux/configfs.h>
> #include <linux/mutex.h>
> #include <linux/platform_device.h>
> @@ -40,6 +41,7 @@
> * `-- vkms
> * `-- test
> * |-- connectors
> + * `-- connected
> * |-- crtcs
> * |-- encoders
> * |-- planes
> @@ -89,6 +91,14 @@
> *
> * echo 1 > /config/vkms/test/enabled
> *
> + * By default no display is "connected" so to connect a connector you'll also
> + * have to write 1 to a connectors "connected" attribute::
> + *
> + * echo 1 > /config/vkms/test/connectors/connector/connected
> + *
> + * One can verify that this is worked using the `modetest` utility or the
> + * equivalent for your platform.
> + *
> * When you're done with the virtual device, you can clean up the device like
> * so::
> *
> @@ -236,7 +246,58 @@ static void add_possible_encoders(struct config_group *parent,
>
> /* Connector item, e.g. /config/vkms/device/connectors/ID */
>
> +static ssize_t connector_connected_show(struct config_item *item, char *buf)
> +{
> + struct vkms_config_connector *connector =
> + item_to_config_connector(item);
> + struct vkms_configfs *configfs = connector_item_to_configfs(item);
> + bool connected = false;
> +
> + mutex_lock(&configfs->lock);
> + connected = connector->connected;
> + mutex_unlock(&configfs->lock);
> +
> + return sprintf(buf, "%d\n", connected);
> +}
> +
> +static ssize_t connector_connected_store(struct config_item *item,
> + const char *buf, size_t len)
> +{
> + struct vkms_config_connector *connector =
> + item_to_config_connector(item);
> + struct vkms_configfs *configfs = connector_item_to_configfs(item);
> + int val, ret;
> +
> + ret = kstrtouint(buf, 10, &val);
> + if (ret)
> + return ret;
> +
> + if (val != 1 && val != 0)
> + return -EINVAL;
> +
> + mutex_lock(&configfs->lock);
> + connector->connected = val;
> + if (!connector->connector) {
> + pr_info("VKMS Device %s is not yet enabled, connector will be enabled on start",
> + configfs->device_group.cg_item.ci_name);
> + }
> + mutex_unlock(&configfs->lock);
> +
> + if (connector->connector)
> + drm_kms_helper_hotplug_event(connector->connector->dev);
> +
> + return len;
> +}
> +
> +CONFIGFS_ATTR(connector_, connected);
> +
> +static struct configfs_attribute *connector_attrs[] = {
> + &connector_attr_connected,
> + NULL,
> +};
> +
> static struct config_item_type connector_type = {
> + .ct_attrs = connector_attrs,
> .ct_owner = THIS_MODULE,
> };
>
> @@ -264,7 +325,7 @@ static ssize_t plane_type_show(struct config_item *item, char *buf)
> plane_type = plane->type;
> mutex_unlock(&configfs->lock);
>
> - return sprintf(buf, "%u", plane_type);
> + return sprintf(buf, "%u\n", plane_type);
> }
>
> static ssize_t plane_type_store(struct config_item *item, const char *buf,
> @@ -319,6 +380,7 @@ static struct config_group *connectors_group_make(struct config_group *group,
> &connector_type);
> add_possible_encoders(&connector->config_group,
> &connector->possible_encoders.group);
> + connector->connected = false;
>
> return &connector->config_group;
> }
> @@ -500,7 +562,7 @@ static ssize_t device_enabled_show(struct config_item *item, char *buf)
> is_enabled = configfs->vkms_device != NULL;
> mutex_unlock(&configfs->lock);
>
> - return sprintf(buf, "%d", is_enabled);
> + return sprintf(buf, "%d\n", is_enabled);
> }
>
> static ssize_t device_enabled_store(struct config_item *item, const char *buf,
> @@ -557,7 +619,7 @@ static ssize_t device_id_show(struct config_item *item, char *buf)
>
> mutex_unlock(&configfs->lock);
>
> - return sprintf(buf, "%d", id);
> + return sprintf(buf, "%d\n", id);
> }
>
> CONFIGFS_ATTR_RO(device_, id);
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 2b9545ada9c2..5336281f397e 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -3,6 +3,7 @@
> #ifndef _VKMS_DRV_H_
> #define _VKMS_DRV_H_
>
> +#include "drm/drm_connector.h"
> #include <linux/configfs.h>
> #include <linux/hrtimer.h>
>
> @@ -147,7 +148,9 @@ struct vkms_config_links {
>
> struct vkms_config_connector {
> struct config_group config_group;
> + struct drm_connector *connector;
> struct vkms_config_links possible_encoders;
> + bool connected;
> };
>
> struct vkms_config_crtc {
> @@ -220,6 +223,10 @@ struct vkms_device {
> #define item_to_configfs(item) \
> container_of(to_config_group(item), struct vkms_configfs, device_group)
>
> +#define connector_item_to_configfs(item) \
> + container_of(to_config_group(item->ci_parent), struct vkms_configfs, \
> + connectors_group)
> +
> #define item_to_config_connector(item) \
> container_of(to_config_group(item), struct vkms_config_connector, \
> config_group)
> @@ -279,4 +286,8 @@ int vkms_enable_writeback_connector(struct vkms_device *vkmsdev,
> int vkms_init_configfs(void);
> void vkms_unregister_configfs(void);
>
> +/* Connector hotplugging */
> +enum drm_connector_status vkms_connector_detect(struct drm_connector *connector,
> + bool force);
> +
> #endif /* _VKMS_DRV_H_ */
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 0ee1f3f4a305..1a1cd0202c5f 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0+
>
> +#include <drm/drm_print.h>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_connector.h>
> #include <drm/drm_crtc.h>
> @@ -8,10 +9,12 @@
> #include <drm/drm_plane.h>
> #include <drm/drm_probe_helper.h>
> #include <drm/drm_simple_kms_helper.h>
> +#include <linux/printk.h>
>
> #include "vkms_drv.h"
>
> static const struct drm_connector_funcs vkms_connector_funcs = {
> + .detect = vkms_connector_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .destroy = drm_connector_cleanup,
> .reset = drm_atomic_helper_connector_reset,
> @@ -19,6 +22,48 @@ static const struct drm_connector_funcs vkms_connector_funcs = {
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> };
>
> +static const struct vkms_config_connector *
> +find_config_for_connector(struct drm_connector *connector)
> +{
> + struct vkms_device *vkms = drm_device_to_vkms_device(connector->dev);
> + struct vkms_configfs *configfs = vkms->configfs;
> + struct config_item *item;
> +
> + if (!configfs) {
> + pr_info("Default connector has no configfs entry");
> + return NULL;
> + }
> +
> + list_for_each_entry(item, &configfs->connectors_group.cg_children,
> + ci_entry) {
> + struct vkms_config_connector *config_connector =
> + item_to_config_connector(item);
> + if (config_connector->connector == connector)
> + return config_connector;
> + }
> +
> + pr_warn("Could not find config to match connector %s, but configfs was initialized",
> + connector->name);
> +
> + return NULL;
> +}
> +
> +enum drm_connector_status vkms_connector_detect(struct drm_connector *connector,
> + bool force)
> +{
> + enum drm_connector_status status = connector_status_connected;
> + const struct vkms_config_connector *config_connector =
> + find_config_for_connector(connector);
> +
> + if (!config_connector)
> + return connector_status_connected;
> +
> + if (!config_connector->connected)
> + status = connector_status_disconnected;
> +
> + return status;
> +}
> +
> static const struct drm_encoder_funcs vkms_encoder_funcs = {
> .destroy = drm_encoder_cleanup,
> };
> @@ -280,12 +325,12 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> struct vkms_config_connector *config_connector =
> item_to_config_connector(item);
> struct drm_connector *connector = vkms_connector_init(vkmsdev);
> -
> if (IS_ERR(connector)) {
> DRM_ERROR("Failed to init connector from config: %s",
> item->ci_name);
> return PTR_ERR(connector);
> }
> + config_connector->connector = connector;
>
> for (int j = 0; j < output->num_encoders; j++) {
> struct encoder_map *encoder = &encoder_map[j];
Sorry, these tests are actually running in the chromeOS infrastructure
environment! A similar test can be written in IGT (and I think is in
the other chain that Marius published)
On Thu, Sep 21, 2023 at 3:03 AM Helen Koike <helen.koike@collabora.com> wrote:
>
> Hello!
>
> Thanks for the patch.
>
> On 29/08/2023 02:30, Brandon Pollack wrote:
> > This change adds the ability to read or write a "1" or a "0" to the
> > newly added "connected" attribute of a connector in the vkms entry in
> > configfs.
> >
> > A write will trigger a call to drm_kms_helper_hotplug_event, causing a
> > hotplug uevent.
> >
> > With this we can write virtualized multidisplay tests that involve
> > hotplugging displays (eg recompositing windows when a monitor is turned
> > off).
>
> Are these tests going to be added in igt?
>
> I was just wondering if it requires any special thing for drm ci:
>
> https://lists.freedesktop.org/archives/dri-devel/2023-September/423719.html
>
> (btw, it would be awesome of you could test your changes with drm ci :)
>
> Regards,
> Helen
>
> >
> > Signed-off-by: Brandon Pollack <brpol@chromium.org>
> > ---
> > Documentation/gpu/vkms.rst | 2 +-
> > drivers/gpu/drm/vkms/vkms_configfs.c | 68 ++++++++++++++++++++++++++--
> > drivers/gpu/drm/vkms/vkms_drv.h | 11 +++++
> > drivers/gpu/drm/vkms/vkms_output.c | 47 ++++++++++++++++++-
> > 4 files changed, 123 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> > index c3875bf66dba..7f715097539c 100644
> > --- a/Documentation/gpu/vkms.rst
> > +++ b/Documentation/gpu/vkms.rst
> > @@ -145,7 +145,7 @@ We want to be able to manipulate vkms instances without having to reload the
> > module. Such configuration can be added as extensions to vkms's ConfigFS
> > support. Use-cases:
> >
> > -- Hotplug/hotremove connectors on the fly (to be able to test DP MST handling
> > +- Hotremove connectors on the fly (to be able to test DP MST handling
> > of compositors).
> >
> > - Change output configuration: Plug/unplug screens, change EDID, allow changing
> > diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> > index bc35dcc47585..d231e28101ae 100644
> > --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> > +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> > @@ -1,5 +1,6 @@
> > // SPDX-License-Identifier: GPL-2.0+
> >
> > +#include "drm/drm_probe_helper.h"
> > #include <linux/configfs.h>
> > #include <linux/mutex.h>
> > #include <linux/platform_device.h>
> > @@ -40,6 +41,7 @@
> > * `-- vkms
> > * `-- test
> > * |-- connectors
> > + * `-- connected
> > * |-- crtcs
> > * |-- encoders
> > * |-- planes
> > @@ -89,6 +91,14 @@
> > *
> > * echo 1 > /config/vkms/test/enabled
> > *
> > + * By default no display is "connected" so to connect a connector you'll also
> > + * have to write 1 to a connectors "connected" attribute::
> > + *
> > + * echo 1 > /config/vkms/test/connectors/connector/connected
> > + *
> > + * One can verify that this is worked using the `modetest` utility or the
> > + * equivalent for your platform.
> > + *
> > * When you're done with the virtual device, you can clean up the device like
> > * so::
> > *
> > @@ -236,7 +246,58 @@ static void add_possible_encoders(struct config_group *parent,
> >
> > /* Connector item, e.g. /config/vkms/device/connectors/ID */
> >
> > +static ssize_t connector_connected_show(struct config_item *item, char *buf)
> > +{
> > + struct vkms_config_connector *connector =
> > + item_to_config_connector(item);
> > + struct vkms_configfs *configfs = connector_item_to_configfs(item);
> > + bool connected = false;
> > +
> > + mutex_lock(&configfs->lock);
> > + connected = connector->connected;
> > + mutex_unlock(&configfs->lock);
> > +
> > + return sprintf(buf, "%d\n", connected);
> > +}
> > +
> > +static ssize_t connector_connected_store(struct config_item *item,
> > + const char *buf, size_t len)
> > +{
> > + struct vkms_config_connector *connector =
> > + item_to_config_connector(item);
> > + struct vkms_configfs *configfs = connector_item_to_configfs(item);
> > + int val, ret;
> > +
> > + ret = kstrtouint(buf, 10, &val);
> > + if (ret)
> > + return ret;
> > +
> > + if (val != 1 && val != 0)
> > + return -EINVAL;
> > +
> > + mutex_lock(&configfs->lock);
> > + connector->connected = val;
> > + if (!connector->connector) {
> > + pr_info("VKMS Device %s is not yet enabled, connector will be enabled on start",
> > + configfs->device_group.cg_item.ci_name);
> > + }
> > + mutex_unlock(&configfs->lock);
> > +
> > + if (connector->connector)
> > + drm_kms_helper_hotplug_event(connector->connector->dev);
> > +
> > + return len;
> > +}
> > +
> > +CONFIGFS_ATTR(connector_, connected);
> > +
> > +static struct configfs_attribute *connector_attrs[] = {
> > + &connector_attr_connected,
> > + NULL,
> > +};
> > +
> > static struct config_item_type connector_type = {
> > + .ct_attrs = connector_attrs,
> > .ct_owner = THIS_MODULE,
> > };
> >
> > @@ -264,7 +325,7 @@ static ssize_t plane_type_show(struct config_item *item, char *buf)
> > plane_type = plane->type;
> > mutex_unlock(&configfs->lock);
> >
> > - return sprintf(buf, "%u", plane_type);
> > + return sprintf(buf, "%u\n", plane_type);
> > }
> >
> > static ssize_t plane_type_store(struct config_item *item, const char *buf,
> > @@ -319,6 +380,7 @@ static struct config_group *connectors_group_make(struct config_group *group,
> > &connector_type);
> > add_possible_encoders(&connector->config_group,
> > &connector->possible_encoders.group);
> > + connector->connected = false;
> >
> > return &connector->config_group;
> > }
> > @@ -500,7 +562,7 @@ static ssize_t device_enabled_show(struct config_item *item, char *buf)
> > is_enabled = configfs->vkms_device != NULL;
> > mutex_unlock(&configfs->lock);
> >
> > - return sprintf(buf, "%d", is_enabled);
> > + return sprintf(buf, "%d\n", is_enabled);
> > }
> >
> > static ssize_t device_enabled_store(struct config_item *item, const char *buf,
> > @@ -557,7 +619,7 @@ static ssize_t device_id_show(struct config_item *item, char *buf)
> >
> > mutex_unlock(&configfs->lock);
> >
> > - return sprintf(buf, "%d", id);
> > + return sprintf(buf, "%d\n", id);
> > }
> >
> > CONFIGFS_ATTR_RO(device_, id);
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> > index 2b9545ada9c2..5336281f397e 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -3,6 +3,7 @@
> > #ifndef _VKMS_DRV_H_
> > #define _VKMS_DRV_H_
> >
> > +#include "drm/drm_connector.h"
> > #include <linux/configfs.h>
> > #include <linux/hrtimer.h>
> >
> > @@ -147,7 +148,9 @@ struct vkms_config_links {
> >
> > struct vkms_config_connector {
> > struct config_group config_group;
> > + struct drm_connector *connector;
> > struct vkms_config_links possible_encoders;
> > + bool connected;
> > };
> >
> > struct vkms_config_crtc {
> > @@ -220,6 +223,10 @@ struct vkms_device {
> > #define item_to_configfs(item) \
> > container_of(to_config_group(item), struct vkms_configfs, device_group)
> >
> > +#define connector_item_to_configfs(item) \
> > + container_of(to_config_group(item->ci_parent), struct vkms_configfs, \
> > + connectors_group)
> > +
> > #define item_to_config_connector(item) \
> > container_of(to_config_group(item), struct vkms_config_connector, \
> > config_group)
> > @@ -279,4 +286,8 @@ int vkms_enable_writeback_connector(struct vkms_device *vkmsdev,
> > int vkms_init_configfs(void);
> > void vkms_unregister_configfs(void);
> >
> > +/* Connector hotplugging */
> > +enum drm_connector_status vkms_connector_detect(struct drm_connector *connector,
> > + bool force);
> > +
> > #endif /* _VKMS_DRV_H_ */
> > diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> > index 0ee1f3f4a305..1a1cd0202c5f 100644
> > --- a/drivers/gpu/drm/vkms/vkms_output.c
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -1,5 +1,6 @@
> > // SPDX-License-Identifier: GPL-2.0+
> >
> > +#include <drm/drm_print.h>
> > #include <drm/drm_atomic_helper.h>
> > #include <drm/drm_connector.h>
> > #include <drm/drm_crtc.h>
> > @@ -8,10 +9,12 @@
> > #include <drm/drm_plane.h>
> > #include <drm/drm_probe_helper.h>
> > #include <drm/drm_simple_kms_helper.h>
> > +#include <linux/printk.h>
> >
> > #include "vkms_drv.h"
> >
> > static const struct drm_connector_funcs vkms_connector_funcs = {
> > + .detect = vkms_connector_detect,
> > .fill_modes = drm_helper_probe_single_connector_modes,
> > .destroy = drm_connector_cleanup,
> > .reset = drm_atomic_helper_connector_reset,
> > @@ -19,6 +22,48 @@ static const struct drm_connector_funcs vkms_connector_funcs = {
> > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > };
> >
> > +static const struct vkms_config_connector *
> > +find_config_for_connector(struct drm_connector *connector)
> > +{
> > + struct vkms_device *vkms = drm_device_to_vkms_device(connector->dev);
> > + struct vkms_configfs *configfs = vkms->configfs;
> > + struct config_item *item;
> > +
> > + if (!configfs) {
> > + pr_info("Default connector has no configfs entry");
> > + return NULL;
> > + }
> > +
> > + list_for_each_entry(item, &configfs->connectors_group.cg_children,
> > + ci_entry) {
> > + struct vkms_config_connector *config_connector =
> > + item_to_config_connector(item);
> > + if (config_connector->connector == connector)
> > + return config_connector;
> > + }
> > +
> > + pr_warn("Could not find config to match connector %s, but configfs was initialized",
> > + connector->name);
> > +
> > + return NULL;
> > +}
> > +
> > +enum drm_connector_status vkms_connector_detect(struct drm_connector *connector,
> > + bool force)
> > +{
> > + enum drm_connector_status status = connector_status_connected;
> > + const struct vkms_config_connector *config_connector =
> > + find_config_for_connector(connector);
> > +
> > + if (!config_connector)
> > + return connector_status_connected;
> > +
> > + if (!config_connector->connected)
> > + status = connector_status_disconnected;
> > +
> > + return status;
> > +}
> > +
> > static const struct drm_encoder_funcs vkms_encoder_funcs = {
> > .destroy = drm_encoder_cleanup,
> > };
> > @@ -280,12 +325,12 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> > struct vkms_config_connector *config_connector =
> > item_to_config_connector(item);
> > struct drm_connector *connector = vkms_connector_init(vkmsdev);
> > -
> > if (IS_ERR(connector)) {
> > DRM_ERROR("Failed to init connector from config: %s",
> > item->ci_name);
> > return PTR_ERR(connector);
> > }
> > + config_connector->connector = connector;
> >
> > for (int j = 0; j < output->num_encoders; j++) {
> > struct encoder_map *encoder = &encoder_map[j];
© 2016 - 2025 Red Hat, Inc.