Re: [PATCH v4 14/16] drm/vkms: Allow to configure connector status

[Louis Chauvet <louis.chauvet@bootlin.com>]

Le 07/04/2025 à 10:14, José Expósito a écrit :
> Allow to store the connector status in vkms_config_connector and add a
> getter and a setter functions as well a KUnit test.
> 
> This change only adds the configuration, the connector status is not
> used yet.
> 
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> ---
>   drivers/gpu/drm/vkms/tests/vkms_config_test.c | 24 +++++++++++++++++
>   drivers/gpu/drm/vkms/vkms_config.c            |  8 ++++--
>   drivers/gpu/drm/vkms/vkms_config.h            | 26 +++++++++++++++++++
>   3 files changed, 56 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/tests/vkms_config_test.c b/drivers/gpu/drm/vkms/tests/vkms_config_test.c
> index ff4566cf9925..3574a829a6ed 100644
> --- a/drivers/gpu/drm/vkms/tests/vkms_config_test.c
> +++ b/drivers/gpu/drm/vkms/tests/vkms_config_test.c
> @@ -916,6 +916,29 @@ static void vkms_config_test_connector_get_possible_encoders(struct kunit *test)
>   	vkms_config_destroy(config);
>   }
>   
> +static void vkms_config_test_connector_status(struct kunit *test)
> +{
> +	struct vkms_config *config;
> +	struct vkms_config_connector *connector_cfg;
> +	enum drm_connector_status status;
> +
> +	config = vkms_config_create("test");
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, config);
> +
> +	connector_cfg = vkms_config_create_connector(config);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, connector_cfg);
> +
> +	status = vkms_config_connector_get_status(connector_cfg);
> +	KUNIT_EXPECT_EQ(test, status, connector_status_connected);
> +
> +	vkms_config_connector_set_status(connector_cfg,
> +					 connector_status_disconnected);
> +	status = vkms_config_connector_get_status(connector_cfg);
> +	KUNIT_EXPECT_EQ(test, status, connector_status_disconnected);
> +
> +	vkms_config_destroy(config);
> +}
> +
>   static struct kunit_case vkms_config_test_cases[] = {
>   	KUNIT_CASE(vkms_config_test_empty_config),
>   	KUNIT_CASE_PARAM(vkms_config_test_default_config,
> @@ -937,6 +960,7 @@ static struct kunit_case vkms_config_test_cases[] = {
>   	KUNIT_CASE(vkms_config_test_plane_get_possible_crtcs),
>   	KUNIT_CASE(vkms_config_test_encoder_get_possible_crtcs),
>   	KUNIT_CASE(vkms_config_test_connector_get_possible_encoders),
> +	KUNIT_CASE(vkms_config_test_connector_status),
>   	{}
>   };
>   
> diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
> index a1df5659b0fb..f8394a063ecf 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.c
> +++ b/drivers/gpu/drm/vkms/vkms_config.c
> @@ -361,8 +361,11 @@ static int vkms_config_show(struct seq_file *m, void *data)
>   	vkms_config_for_each_encoder(vkmsdev->config, encoder_cfg)
>   		seq_puts(m, "encoder\n");
>   
> -	vkms_config_for_each_connector(vkmsdev->config, connector_cfg)
> -		seq_puts(m, "connector\n");
> +	vkms_config_for_each_connector(vkmsdev->config, connector_cfg) {
> +		seq_puts(m, "connector:\n");
> +		seq_printf(m, "\tstatus=%d\n",
> +			   vkms_config_connector_get_status(connector_cfg));
> +	}
>   
>   	return 0;
>   }
> @@ -588,6 +591,7 @@ struct vkms_config_connector *vkms_config_create_connector(struct vkms_config *c
>   		return ERR_PTR(-ENOMEM);
>   
>   	connector_cfg->config = config;
> +	connector_cfg->status = connector_status_connected;
>   	xa_init_flags(&connector_cfg->possible_encoders, XA_FLAGS_ALLOC);
>   
>   	list_add_tail(&connector_cfg->link, &config->connectors);
> diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
> index 0118e3f99706..e202b5a84ddd 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.h
> +++ b/drivers/gpu/drm/vkms/vkms_config.h
> @@ -7,6 +7,8 @@
>   #include <linux/types.h>
>   #include <linux/xarray.h>
>   
> +#include <drm/drm_connector.h>
> +
>   #include "vkms_drv.h"
>   
>   /**
> @@ -99,6 +101,7 @@ struct vkms_config_encoder {
>    *
>    * @link: Link to the others connector in vkms_config
>    * @config: The vkms_config this connector belongs to
> + * @status: Status (connected, disconnected...) of the connector
>    * @possible_encoders: Array of encoders that can be used with this connector
>    * @connector: Internal usage. This pointer should never be considered as valid.
>    *             It can be used to store a temporary reference to a VKMS connector
> @@ -109,6 +112,7 @@ struct vkms_config_connector {
>   	struct list_head link;
>   	struct vkms_config *config;
>   
> +	enum drm_connector_status status;
>   	struct xarray possible_encoders;
>   
>   	/* Internal usage */
> @@ -434,4 +438,26 @@ int __must_check vkms_config_connector_attach_encoder(struct vkms_config_connect
>   void vkms_config_connector_detach_encoder(struct vkms_config_connector *connector_cfg,
>   					  struct vkms_config_encoder *encoder_cfg);
>   
> +/**
> + * vkms_config_connector_get_status() - Return the status of the connector
> + * @connector_cfg: Connector to get the status from
> + */
> +static inline enum drm_connector_status
> +vkms_config_connector_get_status(struct vkms_config_connector *connector_cfg)
> +{
> +	return connector_cfg->status;
> +}
> +
> +/**
> + * vkms_config_crtc_set_writeback() - If a writeback connector will be created
> + * @crtc_cfg: Target CRTC
> + * @writeback: Enable or disable the writeback connector
> + */
> +static inline void
> +vkms_config_connector_set_status(struct vkms_config_connector *connector_cfg,
> +				 enum drm_connector_status status)
> +{
> +	connector_cfg->status = status;
> +}
> +
>   #endif /* _VKMS_CONFIG_H_ */

I think we should have a way to fire a hot

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com