Re: [PATCH v3 32/33] drm/vkms: Allow to hot-add connectors

José Expósito
2025-12-29T17:09:39Z

On Mon, Dec 22, 2025 at 11:11:34AM +0100, Louis Chauvet wrote:
> In order to allow creating dynamic connector, add the required
> infrastructure in vkms_connector.
>
> [Louis Chauvet: use drm_atomic_helper_connector_reset instead of
> drm_mode_config_reset because connector is not yet registered]
>
> Co-developed-by: José Expósito <jose.exposito89@gmail.com>
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
> drivers/gpu/drm/vkms/vkms_connector.c | 81 +++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/vkms/vkms_connector.h | 33 ++++++++++++++
> drivers/gpu/drm/vkms/vkms_output.c | 9 ++++
> 3 files changed, 123 insertions(+)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_connector.c b/drivers/gpu/drm/vkms/vkms_connector.c
> index 3ad614642355..616036494937 100644
> --- a/drivers/gpu/drm/vkms/vkms_connector.c
> +++ b/drivers/gpu/drm/vkms/vkms_connector.c
> @@ -163,9 +163,90 @@ struct vkms_connector *vkms_connector_init_static(struct vkms_device *vkmsdev,
> return connector;
> }
>
> +static void vkms_connector_dynamic_destroy(struct drm_connector *connector)
> +{
> + struct drm_device *dev = connector->dev;
> + struct vkms_connector *vkms_connector;
> +
> + drm_connector_cleanup(connector);
> +
> + vkms_connector = drm_connector_to_vkms_connector(connector);
> + drmm_kfree(dev, vkms_connector);
> +}
> +
> +static const struct drm_connector_funcs vkms_dynamic_connector_funcs = {
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .reset = drm_atomic_helper_connector_reset,
> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> + .destroy = vkms_connector_dynamic_destroy,
> + .detect = vkms_connector_detect,
> +};
> +
> void vkms_trigger_connector_hotplug(struct vkms_device *vkmsdev)
> {
> struct drm_device *dev = &vkmsdev->drm;
>
> drm_kms_helper_hotplug_event(dev);
> }
> +
> +struct vkms_connector *vkms_connector_hot_add(struct vkms_device *vkmsdev,
> + struct vkms_config_connector *connector_cfg)
> +{
> + struct vkms_config_encoder *encoder_cfg;
> + struct vkms_connector *connector;
> + int ret;
> + unsigned long idx = 0;
> +
> + connector = drmm_kzalloc(&vkmsdev->drm, sizeof(*connector), GFP_KERNEL);
> + if (IS_ERR(connector))

I need to test this patch carefully, but in a first review I noticed that this
"if" should check for NULL, not for error.

Jose

> + return connector;
> + ret = drm_connector_dynamic_init(&vkmsdev->drm,
> + &connector->base,
> + &vkms_dynamic_connector_funcs,
> + connector_cfg->type,
> + NULL);
> + if (ret)
> + return ERR_PTR(ret);
> + drm_connector_helper_add(&connector->base, &vkms_conn_helper_funcs);
> +
> + vkms_config_connector_for_each_possible_encoder(connector_cfg, idx, encoder_cfg) {
> + ret = drm_connector_attach_encoder(&connector->base,
> + encoder_cfg->encoder);
> + if (ret)
> + return ERR_PTR(ret);
> + }
> +
> + drm_atomic_helper_connector_reset(&connector->base);
> +
> + vkms_connector_init(vkmsdev, connector, connector_cfg);
> +
> + ret = drm_connector_dynamic_register(&connector->base);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return connector;
> +}
> +
> +void vkms_connector_hot_remove(struct vkms_device *vkmsdev,
> + struct vkms_connector *connector)
> +{
> + drm_connector_unregister(&connector->base);
> + drm_mode_config_reset(&vkmsdev->drm);
> + drm_connector_put(&connector->base);
> +}
> +
> +int vkms_connector_hot_attach_encoder(struct vkms_device *vkmsdev,
> + struct vkms_connector *connector,
> + struct drm_encoder *encoder)
> +{
> + int ret;
> +
> + ret = drm_connector_attach_encoder(&connector->base, encoder);
> + if (ret)
> + return ret;
> +
> + drm_mode_config_reset(&vkmsdev->drm);
> +
> + return ret;
> +}
> diff --git a/drivers/gpu/drm/vkms/vkms_connector.h b/drivers/gpu/drm/vkms/vkms_connector.h
> index 85f9082c710e..a235a518d5a0 100644
> --- a/drivers/gpu/drm/vkms/vkms_connector.h
> +++ b/drivers/gpu/drm/vkms/vkms_connector.h
> @@ -34,4 +34,37 @@ struct vkms_connector *vkms_connector_init_static(struct vkms_device *vkmsdev,
> */
> void vkms_trigger_connector_hotplug(struct vkms_device *vkmsdev);
>
> +/**
> + * vkms_connector_hot_add() - Create a connector after the device is created
> + * @vkmsdev: Device to hot-add the connector to
> + * @connector_cfg: Connector's configuration
> + *
> + * Returns:
> + * A pointer to the newly created connector or a PTR_ERR on failure.
> + */
> +struct vkms_connector *vkms_connector_hot_add(struct vkms_device *vkmsdev,
> + struct vkms_config_connector *connector_cfg);
> +
> +/**
> + * vkms_connector_hot_remove() - Remove a connector after a device is created
> + * @vkmsdev: Device to containing the connector to be removed
> + * @connector: The connector to hot-remove
> + */
> +void vkms_connector_hot_remove(struct vkms_device *vkmsdev,
> + struct vkms_connector *connector);
> +
> +/**
> + * vkms_connector_hot_attach_encoder() - Attach a connector to a encoder after
> + * the device is created.
> + * @vkmsdev: Device containing the connector and the encoder
> + * @connector: Connector to attach to @encoder
> + * @encoder: Target encoder
> + *
> + * Returns:
> + * 0 on success or an error on failure.
> + */
> +int vkms_connector_hot_attach_encoder(struct vkms_device *vkmsdev,
> + struct vkms_connector *connector,
> + struct drm_encoder *encoder);
> +
> #endif /* _VKMS_CONNECTOR_H_ */
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 13c4ca5fd39d..50f7d88dee8b 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -117,5 +117,14 @@ int vkms_output_init(struct vkms_device *vkmsdev)
>
> drm_mode_config_reset(dev);
>
> + vkms_config_for_each_connector_dynamic(vkmsdev->config, connector_cfg) {
> + if (connector_cfg->enabled) {
> + connector_cfg->connector = vkms_connector_hot_add(vkmsdev, connector_cfg);
> +
> + if (IS_ERR(connector_cfg->connector))
> + return PTR_ERR(connector_cfg->connector);
> + }
> + }
> +
> return 0;
> }
>
> --
> 2.51.2
>