diff mbox series

[RFC,14/17] drm/vkms: Allow to configure multiple encoders

Message ID 20240813105134.17439-15-jose.exposito89@gmail.com (mailing list archive)
State New, archived
Headers show
Series VKMS: Add configfs support | expand

Commit Message

José Expósito Aug. 13, 2024, 10:44 a.m. UTC
Add a list of encoder configurations to vkms_config and add as many
encoders as configured during output initialization.

For backwards compatibility, create a single encoder in the default
configuration.

Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 Documentation/gpu/vkms.rst           |  16 +++
 drivers/gpu/drm/vkms/vkms_config.h   |   3 +
 drivers/gpu/drm/vkms/vkms_configfs.c | 184 ++++++++++++++++++++++++---
 3 files changed, 188 insertions(+), 15 deletions(-)

Comments

Louis Chauvet Aug. 13, 2024, 5:59 p.m. UTC | #1
Le 13/08/24 - 12:44, José Expósito a écrit :
> Add a list of encoder configurations to vkms_config and add as many
> encoders as configured during output initialization.

Small mistake, this commit is about connector with configfs :)

> 
> For backwards compatibility, create a single encoder in the default
> configuration.
> 
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> ---
>  Documentation/gpu/vkms.rst           |  16 +++
>  drivers/gpu/drm/vkms/vkms_config.h   |   3 +
>  drivers/gpu/drm/vkms/vkms_configfs.c | 184 ++++++++++++++++++++++++---
>  3 files changed, 188 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> index c69be063d3b4..b6ceb8c48310 100644
> --- a/Documentation/gpu/vkms.rst
> +++ b/Documentation/gpu/vkms.rst
> @@ -75,6 +75,7 @@ And directories are created for each configurable item of the display pipeline::
>  
>    tree /config/vkms/my-vkms
>      /config/vkms/my-vkms
> +    ├── connectors
>      ├── crtcs
>      ├── enabled
>      └── encoders
> @@ -100,16 +101,31 @@ Encoders can be linked to CRTCs by creating a symbolic link under
>  
>    sudo ln -s /config/vkms/my-vkms/crtcs/crtc0 /config/vkms/my-vkms/encoders/encoder0/possible_crtcs
>  
> +Next, create one or more connectors::
> +
> +  sudo mkdir /config/vkms/my-vkms/connectors/connector0
> +
> +The status of the connector can be changed writing ``1`` (connected), ``2``
> +(disconnected) or ``3`` (unknown) to the ``status`` attribute and they can be
> +linked to encoders by creating a symbolic link under ``possible_encoders``::
> +
> +  sudo ln -s /config/vkms/my-vkms/encoders/encoder0 /config/vkms/my-vkms/connectors/connector0/possible_encoders
> +
>  Once you are done configuring the VKMS instance, enable it::
>  
>    echo "1" | sudo tee /config/vkms/my-vkms/enabled
>  
> +Note that the connector ``status`` can be changed once the VKMS instance is
> +enabled to emulate hot-plug/unplug.
> +
>  Finally, you can remove the VKMS instance disabling it::
>  
>    echo "0" | sudo tee /config/vkms/my-vkms/enabled
>  
>  Or removing the top level directory and its subdirectories::
>  
> +  sudo rm /config/vkms/my-vkms/connectors/*/possible_encoders/*
> +  sudo rmdir /config/vkms/my-vkms/connectors/*
>    sudo rm /config/vkms/my-vkms/encoders/*/possible_crtcs/*
>    sudo rmdir /config/vkms/my-vkms/encoders/*
>    sudo rmdir /config/vkms/my-vkms/crtcs/*
> diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
> index f9423533a7f3..cf45e3f6ae92 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.h
> +++ b/drivers/gpu/drm/vkms/vkms_config.h
> @@ -40,6 +40,9 @@ struct vkms_config_connector {
>  	enum drm_connector_status status;
>  	/* only set when instantiated */
>  	struct drm_connector *connector;
> +	/* only used if created from configfs */
> +	struct config_group connector_group;
> +	struct config_group possible_encoders_group;
>  };
>  
>  struct vkms_config {
> diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> index c0b62fb93dc0..d2369f53e2d8 100644
> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> @@ -21,6 +21,8 @@ static bool is_configfs_registered;
>   * Each of its items represent a CRTC
>   * @encoders_group: Default subgroup of @device_group at
>   * "/config/vkms/encoders". Each of its items represent a encoder
> + * @connectors_group: Default subgroup of @device_group at
> + * "/config/vkms/connectors". Each of its items represent a connector
>   * @lock: Lock used to project concurrent access to the configuration attributes
>   * @enabled: Protected by @lock. The device is created or destroyed when this
>   * option changes
> @@ -30,6 +32,7 @@ struct vkms_configfs {
>  	struct config_group device_group;
>  	struct config_group crtcs_group;
>  	struct config_group encoders_group;
> +	struct config_group connectors_group;
>  
>  	/* protected by @lock */
>  	struct mutex lock;
> @@ -57,6 +60,18 @@ struct vkms_configfs {
>  #define encoder_possible_crtcs_item_to_vkms_config_encoder(item) \
>  	container_of(to_config_group(item), struct vkms_config_encoder, possible_crtcs_group)
>  
> +#define connectors_group_to_vkms_configfs(group) \
> +	container_of(group, struct vkms_configfs, connectors_group)
> +
> +#define connectors_item_to_vkms_configfs(item) \
> +	container_of(to_config_group(item), struct vkms_configfs, connectors_group)
> +
> +#define connector_item_to_vkms_config_connector(item) \
> +	container_of(to_config_group(item), struct vkms_config_connector, connector_group)
> +
> +#define connector_possible_encoders_item_to_vkms_config_connector(item) \
> +	container_of(to_config_group(item), struct vkms_config_connector, possible_encoders_group)
> +
>  static ssize_t crtc_cursor_show(struct config_item *item, char *page)
>  {
>  	struct vkms_config_crtc *crtc_cfg = crtcs_item_to_vkms_config_crtc(item);
> @@ -287,6 +302,154 @@ static struct config_item_type encoders_group_type = {
>  	.ct_owner = THIS_MODULE,
>  };
>  
> +static int connector_possible_encoders_allow_link(struct config_item *src,
> +						  struct config_item *target)
> +{
> +	struct vkms_config_connector *connector_cfg;
> +	struct vkms_config_encoder *encoder_cfg;

I think I forgit it before, but once the device is enabled, no 
configuration should be possible (except connector status).

> +	if (target->ci_type != &encoder_group_type)
> +		return -EINVAL;
> +

Same as before, the type and the device must be checked.

> +	connector_cfg = connector_possible_encoders_item_to_vkms_config_connector(src);
> +	encoder_cfg = encoders_item_to_vkms_config_encoder(target);
> +
> +	if (connector_cfg->possible_encoders & BIT(encoder_cfg->index))
> +		return -EINVAL;
> +
> +	connector_cfg->possible_encoders |= BIT(encoder_cfg->index);
> +
> +	return 0;
> +}
> +
> +static void connector_possible_encoders_drop_link(struct config_item *src,
> +						  struct config_item *target)
> +{
> +	struct vkms_config_connector *connector_cfg;
> +	struct vkms_config_encoder *encoder_cfg;
> +
> +	connector_cfg = connector_possible_encoders_item_to_vkms_config_connector(src);
> +	encoder_cfg = encoders_item_to_vkms_config_encoder(target);
> +
> +	connector_cfg->possible_encoders &= ~BIT(encoder_cfg->index);
> +}
> +
> +static struct configfs_item_operations connector_possible_encoders_item_ops = {
> +	.allow_link = &connector_possible_encoders_allow_link,
> +	.drop_link = &connector_possible_encoders_drop_link,
> +};
> +
> +static struct config_item_type connector_possible_encoders_group_type = {
> +	.ct_item_ops = &connector_possible_encoders_item_ops,
> +	.ct_owner = THIS_MODULE,
> +};
> +
> +static ssize_t connector_status_show(struct config_item *item, char *page)
> +{
> +	struct vkms_config_connector *connector_cfg =
> +		connector_item_to_vkms_config_connector(item);
> +
> +	return sprintf(page, "%d\n", connector_cfg->status);
> +}
> +
> +static ssize_t connector_status_store(struct config_item *item, const char *page,
> +				      size_t count)
> +{
> +	struct vkms_configfs *configfs;
> +	struct vkms_config_connector *connector_cfg;
> +	int status;
> +
> +	if (kstrtoint(page, 10, &status))
> +		return -EINVAL;
> +
> +	switch (status) {
> +	case connector_status_connected:
> +	case connector_status_disconnected:
> +	case connector_status_unknown:
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	configfs = connectors_item_to_vkms_configfs(item->ci_parent);
> +	connector_cfg = connector_item_to_vkms_config_connector(item);
> +
> +	vkms_update_connector_status(configfs->vkms_config, connector_cfg, status);
> +
> +	return (ssize_t)count;
> +}
> +
> +CONFIGFS_ATTR(connector_, status);
> +
> +static struct configfs_attribute *connector_group_attrs[] = {
> +	&connector_attr_status,
> +	NULL,
> +};
> +
> +static const struct config_item_type connector_group_type = {
> +	.ct_attrs = connector_group_attrs,
> +	.ct_owner = THIS_MODULE,
> +};
> +
> +static struct config_group *make_connectors_group(struct config_group *group,
> +						  const char *name)
> +{
> +	struct vkms_configfs *configfs = connectors_group_to_vkms_configfs(group);
> +	struct vkms_config_connector *connector_cfg;
> +	int ret;
> +
> +	mutex_lock(&configfs->lock);
> +
> +	if (configfs->enabled) {
> +		ret = -EINVAL;
> +		goto err_unlock;
> +	}
> +
> +	connector_cfg = vkms_config_add_connector(configfs->vkms_config, 0,
> +						  connector_status_connected);
> +	if (IS_ERR(connector_cfg)) {
> +		ret = PTR_ERR(connector_cfg);
> +		goto err_unlock;
> +	}
> +
> +	config_group_init_type_name(&connector_cfg->connector_group, name,
> +				    &connector_group_type);
> +
> +	config_group_init_type_name(&connector_cfg->possible_encoders_group,
> +				    "possible_encoders",
> +				    &connector_possible_encoders_group_type);
> +	configfs_add_default_group(&connector_cfg->possible_encoders_group,
> +				   &connector_cfg->connector_group);
> +
> +	mutex_unlock(&configfs->lock);
> +
> +	return &connector_cfg->connector_group;
> +
> +err_unlock:
> +	mutex_unlock(&configfs->lock);
> +	return ERR_PTR(ret);
> +}
> +
> +static void drop_connectors_group(struct config_group *group,
> +				  struct config_item *item)
> +{
> +	struct vkms_configfs *configfs = connectors_group_to_vkms_configfs(group);
> +	struct vkms_config_connector *connector_cfg =
> +		connector_item_to_vkms_config_connector(item);
> +
> +	vkms_config_destroy_connector(configfs->vkms_config, connector_cfg);
> +}

Again, I think drop should not free memory.

> +
> +static struct configfs_group_operations connectors_group_ops = {
> +	.make_group = &make_connectors_group,
> +	.drop_item = &drop_connectors_group,
> +};
> +
> +static struct config_item_type connectors_group_type = {
> +	.ct_group_ops = &connectors_group_ops,
> +	.ct_owner = THIS_MODULE,
> +};
> +
>  static ssize_t device_enabled_show(struct config_item *item, char *page)
>  {
>  	struct vkms_configfs *configfs = config_item_to_vkms_configfs(item);
> @@ -341,9 +504,7 @@ static struct config_group *make_device_group(struct config_group *group,
>  					      const char *name)
>  {
>  	struct vkms_configfs *configfs;
> -	struct vkms_config_connector *connector_cfg = NULL;
>  	char *config_name;
> -	int ret;
>  
>  	if (strcmp(name, DEFAULT_DEVICE_NAME) == 0)
>  		return ERR_PTR(-EINVAL);
> @@ -358,8 +519,8 @@ static struct config_group *make_device_group(struct config_group *group,
>  	config_name = config_item_name(&configfs->device_group.cg_item);
>  	configfs->vkms_config = vkms_config_create(config_name);
>  	if (IS_ERR(configfs->vkms_config)) {
> -		ret = PTR_ERR(configfs->vkms_config);
> -		goto err_kfree;
> +		kfree(configfs);
> +		return ERR_CAST(configfs->vkms_config);
>  	}
>  
>  	config_group_init_type_name(&configfs->crtcs_group, "crtcs",
> @@ -372,19 +533,12 @@ static struct config_group *make_device_group(struct config_group *group,
>  	configfs_add_default_group(&configfs->encoders_group,
>  				   &configfs->device_group);
>  
> -	connector_cfg = vkms_config_add_connector(configfs->vkms_config, BIT(0),
> -						  connector_status_connected);
> -	if (IS_ERR(connector_cfg)) {
> -		ret = PTR_ERR(connector_cfg);
> -		goto err_kfree;
> -	}
> +	config_group_init_type_name(&configfs->connectors_group, "connectors",
> +				    &connectors_group_type);
> +	configfs_add_default_group(&configfs->connectors_group,
> +				   &configfs->device_group);
>  
>  	return &configfs->device_group;
> -
> -err_kfree:
> -	kfree(configfs);
> -	kfree(connector_cfg);
> -	return ERR_PTR(ret);
>  }
>  
>  static void drop_device_group(struct config_group *group,
> -- 
> 2.46.0
>
diff mbox series

Patch

diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index c69be063d3b4..b6ceb8c48310 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -75,6 +75,7 @@  And directories are created for each configurable item of the display pipeline::
 
   tree /config/vkms/my-vkms
     /config/vkms/my-vkms
+    ├── connectors
     ├── crtcs
     ├── enabled
     └── encoders
@@ -100,16 +101,31 @@  Encoders can be linked to CRTCs by creating a symbolic link under
 
   sudo ln -s /config/vkms/my-vkms/crtcs/crtc0 /config/vkms/my-vkms/encoders/encoder0/possible_crtcs
 
+Next, create one or more connectors::
+
+  sudo mkdir /config/vkms/my-vkms/connectors/connector0
+
+The status of the connector can be changed writing ``1`` (connected), ``2``
+(disconnected) or ``3`` (unknown) to the ``status`` attribute and they can be
+linked to encoders by creating a symbolic link under ``possible_encoders``::
+
+  sudo ln -s /config/vkms/my-vkms/encoders/encoder0 /config/vkms/my-vkms/connectors/connector0/possible_encoders
+
 Once you are done configuring the VKMS instance, enable it::
 
   echo "1" | sudo tee /config/vkms/my-vkms/enabled
 
+Note that the connector ``status`` can be changed once the VKMS instance is
+enabled to emulate hot-plug/unplug.
+
 Finally, you can remove the VKMS instance disabling it::
 
   echo "0" | sudo tee /config/vkms/my-vkms/enabled
 
 Or removing the top level directory and its subdirectories::
 
+  sudo rm /config/vkms/my-vkms/connectors/*/possible_encoders/*
+  sudo rmdir /config/vkms/my-vkms/connectors/*
   sudo rm /config/vkms/my-vkms/encoders/*/possible_crtcs/*
   sudo rmdir /config/vkms/my-vkms/encoders/*
   sudo rmdir /config/vkms/my-vkms/crtcs/*
diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
index f9423533a7f3..cf45e3f6ae92 100644
--- a/drivers/gpu/drm/vkms/vkms_config.h
+++ b/drivers/gpu/drm/vkms/vkms_config.h
@@ -40,6 +40,9 @@  struct vkms_config_connector {
 	enum drm_connector_status status;
 	/* only set when instantiated */
 	struct drm_connector *connector;
+	/* only used if created from configfs */
+	struct config_group connector_group;
+	struct config_group possible_encoders_group;
 };
 
 struct vkms_config {
diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
index c0b62fb93dc0..d2369f53e2d8 100644
--- a/drivers/gpu/drm/vkms/vkms_configfs.c
+++ b/drivers/gpu/drm/vkms/vkms_configfs.c
@@ -21,6 +21,8 @@  static bool is_configfs_registered;
  * Each of its items represent a CRTC
  * @encoders_group: Default subgroup of @device_group at
  * "/config/vkms/encoders". Each of its items represent a encoder
+ * @connectors_group: Default subgroup of @device_group at
+ * "/config/vkms/connectors". Each of its items represent a connector
  * @lock: Lock used to project concurrent access to the configuration attributes
  * @enabled: Protected by @lock. The device is created or destroyed when this
  * option changes
@@ -30,6 +32,7 @@  struct vkms_configfs {
 	struct config_group device_group;
 	struct config_group crtcs_group;
 	struct config_group encoders_group;
+	struct config_group connectors_group;
 
 	/* protected by @lock */
 	struct mutex lock;
@@ -57,6 +60,18 @@  struct vkms_configfs {
 #define encoder_possible_crtcs_item_to_vkms_config_encoder(item) \
 	container_of(to_config_group(item), struct vkms_config_encoder, possible_crtcs_group)
 
+#define connectors_group_to_vkms_configfs(group) \
+	container_of(group, struct vkms_configfs, connectors_group)
+
+#define connectors_item_to_vkms_configfs(item) \
+	container_of(to_config_group(item), struct vkms_configfs, connectors_group)
+
+#define connector_item_to_vkms_config_connector(item) \
+	container_of(to_config_group(item), struct vkms_config_connector, connector_group)
+
+#define connector_possible_encoders_item_to_vkms_config_connector(item) \
+	container_of(to_config_group(item), struct vkms_config_connector, possible_encoders_group)
+
 static ssize_t crtc_cursor_show(struct config_item *item, char *page)
 {
 	struct vkms_config_crtc *crtc_cfg = crtcs_item_to_vkms_config_crtc(item);
@@ -287,6 +302,154 @@  static struct config_item_type encoders_group_type = {
 	.ct_owner = THIS_MODULE,
 };
 
+static int connector_possible_encoders_allow_link(struct config_item *src,
+						  struct config_item *target)
+{
+	struct vkms_config_connector *connector_cfg;
+	struct vkms_config_encoder *encoder_cfg;
+
+	if (target->ci_type != &encoder_group_type)
+		return -EINVAL;
+
+	connector_cfg = connector_possible_encoders_item_to_vkms_config_connector(src);
+	encoder_cfg = encoders_item_to_vkms_config_encoder(target);
+
+	if (connector_cfg->possible_encoders & BIT(encoder_cfg->index))
+		return -EINVAL;
+
+	connector_cfg->possible_encoders |= BIT(encoder_cfg->index);
+
+	return 0;
+}
+
+static void connector_possible_encoders_drop_link(struct config_item *src,
+						  struct config_item *target)
+{
+	struct vkms_config_connector *connector_cfg;
+	struct vkms_config_encoder *encoder_cfg;
+
+	connector_cfg = connector_possible_encoders_item_to_vkms_config_connector(src);
+	encoder_cfg = encoders_item_to_vkms_config_encoder(target);
+
+	connector_cfg->possible_encoders &= ~BIT(encoder_cfg->index);
+}
+
+static struct configfs_item_operations connector_possible_encoders_item_ops = {
+	.allow_link = &connector_possible_encoders_allow_link,
+	.drop_link = &connector_possible_encoders_drop_link,
+};
+
+static struct config_item_type connector_possible_encoders_group_type = {
+	.ct_item_ops = &connector_possible_encoders_item_ops,
+	.ct_owner = THIS_MODULE,
+};
+
+static ssize_t connector_status_show(struct config_item *item, char *page)
+{
+	struct vkms_config_connector *connector_cfg =
+		connector_item_to_vkms_config_connector(item);
+
+	return sprintf(page, "%d\n", connector_cfg->status);
+}
+
+static ssize_t connector_status_store(struct config_item *item, const char *page,
+				      size_t count)
+{
+	struct vkms_configfs *configfs;
+	struct vkms_config_connector *connector_cfg;
+	int status;
+
+	if (kstrtoint(page, 10, &status))
+		return -EINVAL;
+
+	switch (status) {
+	case connector_status_connected:
+	case connector_status_disconnected:
+	case connector_status_unknown:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	configfs = connectors_item_to_vkms_configfs(item->ci_parent);
+	connector_cfg = connector_item_to_vkms_config_connector(item);
+
+	vkms_update_connector_status(configfs->vkms_config, connector_cfg, status);
+
+	return (ssize_t)count;
+}
+
+CONFIGFS_ATTR(connector_, status);
+
+static struct configfs_attribute *connector_group_attrs[] = {
+	&connector_attr_status,
+	NULL,
+};
+
+static const struct config_item_type connector_group_type = {
+	.ct_attrs = connector_group_attrs,
+	.ct_owner = THIS_MODULE,
+};
+
+static struct config_group *make_connectors_group(struct config_group *group,
+						  const char *name)
+{
+	struct vkms_configfs *configfs = connectors_group_to_vkms_configfs(group);
+	struct vkms_config_connector *connector_cfg;
+	int ret;
+
+	mutex_lock(&configfs->lock);
+
+	if (configfs->enabled) {
+		ret = -EINVAL;
+		goto err_unlock;
+	}
+
+	connector_cfg = vkms_config_add_connector(configfs->vkms_config, 0,
+						  connector_status_connected);
+	if (IS_ERR(connector_cfg)) {
+		ret = PTR_ERR(connector_cfg);
+		goto err_unlock;
+	}
+
+	config_group_init_type_name(&connector_cfg->connector_group, name,
+				    &connector_group_type);
+
+	config_group_init_type_name(&connector_cfg->possible_encoders_group,
+				    "possible_encoders",
+				    &connector_possible_encoders_group_type);
+	configfs_add_default_group(&connector_cfg->possible_encoders_group,
+				   &connector_cfg->connector_group);
+
+	mutex_unlock(&configfs->lock);
+
+	return &connector_cfg->connector_group;
+
+err_unlock:
+	mutex_unlock(&configfs->lock);
+	return ERR_PTR(ret);
+}
+
+static void drop_connectors_group(struct config_group *group,
+				  struct config_item *item)
+{
+	struct vkms_configfs *configfs = connectors_group_to_vkms_configfs(group);
+	struct vkms_config_connector *connector_cfg =
+		connector_item_to_vkms_config_connector(item);
+
+	vkms_config_destroy_connector(configfs->vkms_config, connector_cfg);
+}
+
+static struct configfs_group_operations connectors_group_ops = {
+	.make_group = &make_connectors_group,
+	.drop_item = &drop_connectors_group,
+};
+
+static struct config_item_type connectors_group_type = {
+	.ct_group_ops = &connectors_group_ops,
+	.ct_owner = THIS_MODULE,
+};
+
 static ssize_t device_enabled_show(struct config_item *item, char *page)
 {
 	struct vkms_configfs *configfs = config_item_to_vkms_configfs(item);
@@ -341,9 +504,7 @@  static struct config_group *make_device_group(struct config_group *group,
 					      const char *name)
 {
 	struct vkms_configfs *configfs;
-	struct vkms_config_connector *connector_cfg = NULL;
 	char *config_name;
-	int ret;
 
 	if (strcmp(name, DEFAULT_DEVICE_NAME) == 0)
 		return ERR_PTR(-EINVAL);
@@ -358,8 +519,8 @@  static struct config_group *make_device_group(struct config_group *group,
 	config_name = config_item_name(&configfs->device_group.cg_item);
 	configfs->vkms_config = vkms_config_create(config_name);
 	if (IS_ERR(configfs->vkms_config)) {
-		ret = PTR_ERR(configfs->vkms_config);
-		goto err_kfree;
+		kfree(configfs);
+		return ERR_CAST(configfs->vkms_config);
 	}
 
 	config_group_init_type_name(&configfs->crtcs_group, "crtcs",
@@ -372,19 +533,12 @@  static struct config_group *make_device_group(struct config_group *group,
 	configfs_add_default_group(&configfs->encoders_group,
 				   &configfs->device_group);
 
-	connector_cfg = vkms_config_add_connector(configfs->vkms_config, BIT(0),
-						  connector_status_connected);
-	if (IS_ERR(connector_cfg)) {
-		ret = PTR_ERR(connector_cfg);
-		goto err_kfree;
-	}
+	config_group_init_type_name(&configfs->connectors_group, "connectors",
+				    &connectors_group_type);
+	configfs_add_default_group(&configfs->connectors_group,
+				   &configfs->device_group);
 
 	return &configfs->device_group;
-
-err_kfree:
-	kfree(configfs);
-	kfree(connector_cfg);
-	return ERR_PTR(ret);
 }
 
 static void drop_device_group(struct config_group *group,