On Sat, Oct 18, 2025 at 04:01:22AM +0200, Louis Chauvet wrote:
> DRM allows the connector to be created after the device. To allows
> emulating this, add two configfs attributes to connector to allows this.
> 
> Using the dynamic attribute you can set if a connector will be dynamic or
> not.
> Using the enabled attribute, you can set at runtime if a dynamic connector
> is present or not.
> 
> Co-developed-by: José Expósito <[email protected]>
> Signed-off-by: José Expósito <[email protected]>
> Signed-off-by: Louis Chauvet <[email protected]>
> ---
>  Documentation/gpu/vkms.rst           |   6 +-
>  drivers/gpu/drm/vkms/vkms_configfs.c | 146 
> ++++++++++++++++++++++++++++++++---
>  2 files changed, 139 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> index bbd03f61e61c..8b17aaa28eeb 100644
> --- a/Documentation/gpu/vkms.rst
> +++ b/Documentation/gpu/vkms.rst
> @@ -135,7 +135,7 @@ Last but not least, create one or more connectors::
>  
>    sudo mkdir /config/vkms/my-vkms/connectors/connector0
>  
> -Connectors have 5 configurable attribute:
> +Connectors have 7 configurable attribute:
>  
>  - status: Connection status: 1 connected, 2 disconnected, 3 unknown (same 
> values
>    as those exposed by the "status" property of a connector)
> @@ -147,7 +147,9 @@ Connectors have 5 configurable attribute:
>  - edid_enabled: Enable or not EDID for this connector. Some connectors may 
> not have an
>    EDID but just a list of modes, this attribute allows to disable EDID 
> property.
>  - edid: Content of the EDID. Ignored if edid_enabled is not set
> -
> +- dynamic: Set to 1 while configuring the device to create a dynamic 
> connector. A dynamic
> +  connector can be used to emulate DP MST connectors.
> +- enabled: For dynamic connector, set it to 1 to create the connector, 0 to 
> remove it.
>  
>  To finish the configuration, link the different pipeline items::
>  
> diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c 
> b/drivers/gpu/drm/vkms/vkms_configfs.c
> index a977c0842cd6..937b749142ad 100644
> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> @@ -1115,8 +1115,12 @@ static ssize_t connector_status_store(struct 
> config_item *item,
>       scoped_guard(mutex, &connector->dev->lock) {
>               vkms_config_connector_set_status(connector->config, status);
>  
> -             if (connector->dev->enabled)
> -                     
> vkms_trigger_connector_hotplug(connector->dev->config->dev);
> +             if (connector->dev->enabled) {
> +                     if (connector->config->dynamic && 
> connector->config->enabled)
> +                             
> vkms_trigger_connector_hotplug(connector->dev->config->dev);
> +                     if (!connector->config->dynamic)
> +                             
> vkms_trigger_connector_hotplug(connector->dev->config->dev);

To avoid duplicating code, we could do something like:

        bool hotplug = !connector->config->dynamic ||
                       (connector->config->dynamic && 
connector->config->enabled);
        if (hotplug)
                vkms_trigger_connector_hotplug(connector->dev->config->dev);


> +             }
>       }
>  
>       return (ssize_t)count;
> @@ -1176,8 +1180,12 @@ static ssize_t connector_type_store(struct config_item 
> *item,
>       }
>  
>       scoped_guard(mutex, &connector->dev->lock) {
> -             if (connector->dev->enabled)
> -                     return -EINVAL;
> +             if (connector->dev->enabled) {
> +                     if (connector->config->dynamic && 
> connector->config->enabled)
> +                             return -EBUSY;
> +                     if (!connector->config->dynamic)
> +                             return -EBUSY;
> +             }

And something similar here. Also, instead of returning -EINVAL
in "drm/vkms: Introduce configfs for connector type", you could return
-EBUSY and avoid changing the return type here.

>               vkms_config_connector_set_type(connector->config, val);
>       }
>  
> @@ -1293,6 +1301,102 @@ static ssize_t connector_edid_store(struct 
> config_item *item,
>                   connector_status_disconnected)
>                       
> vkms_trigger_connector_hotplug(connector->dev->config->dev);
>       }
> +     return count;
> +}
> +
> +static ssize_t connector_enabled_show(struct config_item *item, char *page)
> +{
> +     struct vkms_configfs_connector *connector;
> +     bool enabled;
> +
> +     connector = connector_item_to_vkms_configfs_connector(item);
> +
> +     scoped_guard(mutex, &connector->dev->lock)
> +             enabled = vkms_config_connector_is_enabled(connector->config);
> +
> +     return sprintf(page, "%d\n", enabled);
> +}
> +
> +static ssize_t connector_enabled_store(struct config_item *item,
> +                                    const char *page, size_t count)
> +{
> +     struct vkms_configfs_connector *connector;
> +     struct vkms_config_connector *connector_cfg;
> +     bool enabled, was_enabled;
> +
> +     connector = connector_item_to_vkms_configfs_connector(item);
> +     connector_cfg = connector->config;
> +
> +     if (kstrtobool(page, &enabled))
> +             return -EINVAL;
> +
> +     if (!connector->dev->enabled) {
> +             vkms_config_connector_set_enabled(connector_cfg, enabled);
> +     } else {
> +             // Only dynamic connector can be enabled/disabled at runtime
> +             if (!connector_cfg->dynamic)
> +                     return -EBUSY;
> +
> +             was_enabled = vkms_config_connector_is_enabled(connector_cfg);
> +             vkms_config_connector_set_enabled(connector_cfg, enabled);
> +
> +             // Resulting configuration is invalid (missing encoder for 
> example)
> +             // Early return to avoid drm core issue
> +             if (!vkms_config_is_valid(connector->dev->config)) {
> +                     vkms_config_connector_set_enabled(connector_cfg, 
> was_enabled);
> +                     return -EINVAL;
> +             }
> +
> +             if (!was_enabled && enabled) {
> +                     // Adding the connector
> +                     connector_cfg->connector = 
> vkms_connector_hot_add(connector->dev->config->dev,
> +                                                                       
> connector_cfg);
> +                     if (IS_ERR(connector_cfg->connector)) {
> +                             
> vkms_config_connector_set_enabled(connector_cfg, was_enabled);
> +                             return PTR_ERR(connector_cfg->connector);
> +                     }
> +             } else if (was_enabled && !enabled) {
> +                     vkms_connector_hot_remove(connector->dev->config->dev,
> +                                               connector_cfg->connector);
> +             }
> +     }
> +     return count;

As a suggestion, we could add a "goto rollback;" and avoid duplicating error
handling:

        rollback:
                vkms_config_connector_set_enabled(connector_cfg, was_enabled);
                return ret;

> +}
> +
> +static ssize_t connector_dynamic_show(struct config_item *item, char *page)
> +{
> +     struct vkms_configfs_connector *connector;
> +     bool enabled;
> +
> +     connector = connector_item_to_vkms_configfs_connector(item);
> +
> +     scoped_guard(mutex, &connector->dev->lock) {
> +             enabled = vkms_config_connector_is_dynamic(connector->config);
> +     }
> +
> +     return sprintf(page, "%d\n", enabled);
> +}
> +
> +static ssize_t connector_dynamic_store(struct config_item *item,
> +                                    const char *page, size_t count)
> +{
> +     struct vkms_configfs_connector *connector;
> +     struct vkms_config_connector *connector_cfg;
> +     bool dynamic;
> +
> +     connector = connector_item_to_vkms_configfs_connector(item);
> +     connector_cfg = connector->config;
> +
> +     if (kstrtobool(page, &dynamic))
> +             return -EINVAL;
> +
> +     scoped_guard(mutex, &connector->dev->lock) {
> +             // Can't change the dynamic status when the device is activated
> +             if (connector->dev->enabled)
> +                     return -EBUSY;
> +
> +             vkms_config_connector_set_dynamic(connector_cfg, dynamic);
> +     }
>  
>       return count;
>  }
> @@ -1302,6 +1406,8 @@ CONFIGFS_ATTR(connector_, type);
>  CONFIGFS_ATTR(connector_, supported_colorspaces);
>  CONFIGFS_ATTR(connector_, edid_enabled);
>  CONFIGFS_ATTR(connector_, edid);
> +CONFIGFS_ATTR(connector_, dynamic);
> +CONFIGFS_ATTR(connector_, enabled);
>  
>  static struct configfs_attribute *connector_item_attrs[] = {
>       &connector_attr_status,
> @@ -1309,19 +1415,28 @@ static struct configfs_attribute 
> *connector_item_attrs[] = {
>       &connector_attr_supported_colorspaces,
>       &connector_attr_edid_enabled,
>       &connector_attr_edid,
> +     &connector_attr_dynamic,
> +     &connector_attr_enabled,
>       NULL,
>  };
>  
>  static void connector_release(struct config_item *item)
>  {
>       struct vkms_configfs_connector *connector;
> +     struct vkms_config_connector *connector_cfg;
>       struct mutex *lock;
>  
>       connector = connector_item_to_vkms_configfs_connector(item);
> +     connector_cfg = connector->config;
>       lock = &connector->dev->lock;
>  
>       scoped_guard(mutex, lock) {
> +             if (connector->dev->enabled && connector_cfg->dynamic && 
> connector_cfg->enabled)
> +                     vkms_connector_hot_remove(connector->dev->config->dev,
> +                                               connector_cfg->connector);
> +
>               vkms_config_destroy_connector(connector->config);
> +
>               kfree(connector);
>       }
>  }
> @@ -1340,6 +1455,7 @@ static int 
> connector_possible_encoders_allow_link(struct config_item *src,
>                                                 struct config_item *target)
>  {
>       struct vkms_configfs_connector *connector;
> +     struct vkms_config_connector *connector_cfg;
>       struct vkms_configfs_encoder *encoder;
>       int ret;
>  
> @@ -1347,16 +1463,25 @@ static int 
> connector_possible_encoders_allow_link(struct config_item *src,
>               return -EINVAL;
>  
>       connector = 
> connector_possible_encoders_item_to_vkms_configfs_connector(src);
> +     connector_cfg = connector->config;
>       encoder = encoder_item_to_vkms_configfs_encoder(target);
>  
>       scoped_guard(mutex, &connector->dev->lock) {
> -             if (connector->dev->enabled)
> -                     return -EBUSY;
> +             if (connector->dev->enabled && connector_cfg->enabled) {
> +                     if (!connector_cfg->dynamic)
> +                             return -EBUSY;
> +                     ret = 
> vkms_connector_hot_attach_encoder(connector->dev->config->dev,
> +                                                             
> connector->config->connector,
> +                                                             
> encoder->config->encoder);
> +                     if (ret)
> +                             return ret;
> +             }
>  
>               ret = vkms_config_connector_attach_encoder(connector->config,
>                                                          encoder->config);
> +             if (ret)
> +                     return ret;
>       }
> -
>       return ret;
>  }
>  
> @@ -1394,9 +1519,6 @@ static struct config_group *make_connector_group(struct 
> config_group *group,
>       dev = child_group_to_vkms_configfs_device(group);
>  
>       scoped_guard(mutex, &dev->lock) {
> -             if (dev->enabled)
> -                     return ERR_PTR(-EBUSY);
> -
>               connector = kzalloc(sizeof(*connector), GFP_KERNEL);
>               if (!connector)
>                       return ERR_PTR(-ENOMEM);
> @@ -1409,9 +1531,11 @@ static struct config_group 
> *make_connector_group(struct config_group *group,
>                       return ERR_CAST(connector->config);
>               }
>  
> +             vkms_config_connector_set_dynamic(connector->config, 
> connector->dev->enabled);
> +             vkms_config_connector_set_enabled(connector->config, 
> !connector->dev->enabled);
> +
>               config_group_init_type_name(&connector->group, name,
>                                           &connector_item_type);
> -
>               config_group_init_type_name(&connector->possible_encoders_group,
>                                           "possible_encoders",
>                                           
> &connector_possible_encoders_group_type);
> 
> -- 
> 2.51.0
> 

Reply via email to