[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zrue4jb9ikn-_n-j@louis-chauvet-laptop>
Date: Tue, 13 Aug 2024 19:58:58 +0200
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: José Expósito <jose.exposito89@...il.com>
Cc: rodrigosiqueiramelo@...il.com, melissa.srw@...il.com,
mairacanal@...eup.net, hamohammed.sa@...il.com, daniel@...ll.ch,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
tzimmermann@...e.de, airlied@...il.com,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 10/17] drm/vkms: Allow to change connector status
Le 13/08/24 - 12:44, José Expósito a écrit :
> Store the connector status in vkms_config_connector and use the stored
> value to update the connector status in the drm_connector_funcs.detect()
> function.
I did not work on this part, so my comments may be not relevant.
> Signed-off-by: José Expósito <jose.exposito89@...il.com>
> ---
> drivers/gpu/drm/vkms/vkms_config.c | 18 ++++++++++++++++--
> drivers/gpu/drm/vkms/vkms_config.h | 12 +++++++++++-
> drivers/gpu/drm/vkms/vkms_output.c | 20 ++++++++++++++++++++
> 3 files changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
> index d95a42a6745a..e8e5c02c9d43 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.c
> +++ b/drivers/gpu/drm/vkms/vkms_config.c
> @@ -3,6 +3,7 @@
> #include <linux/slab.h>
>
> #include <drm/drm_print.h>
> +#include <drm/drm_probe_helper.h>
> #include <drm/drm_debugfs.h>
>
> #include "vkms_config.h"
> @@ -56,7 +57,7 @@ struct vkms_config *vkms_config_default_create(bool enable_cursor,
> if (IS_ERR(encoder_cfg))
> return ERR_CAST(encoder_cfg);
>
> - connector_cfg = vkms_config_add_connector(config, BIT(0));
> + connector_cfg = vkms_config_add_connector(config, BIT(0), connector_status_connected);
> if (IS_ERR(connector_cfg))
> return ERR_CAST(connector_cfg);
>
> @@ -124,6 +125,8 @@ static int vkms_config_show(struct seq_file *m, void *data)
> list_for_each_entry(connector_cfg, &vkmsdev->config->connectors, list) {
> seq_printf(m, "connector(%d).possible_encoders=%d\n", n,
> connector_cfg->possible_encoders);
> + seq_printf(m, "connector(%d).status=%d\n", n,
> + connector_cfg->status);
> n++;
> }
>
> @@ -229,7 +232,8 @@ void vkms_config_destroy_encoder(struct vkms_config *config,
> }
>
> struct vkms_config_connector *vkms_config_add_connector(struct vkms_config *config,
> - uint32_t possible_encoders)
> + uint32_t possible_encoders,
> + enum drm_connector_status status)
> {
> struct vkms_config_connector *connector_cfg;
>
> @@ -238,6 +242,7 @@ struct vkms_config_connector *vkms_config_add_connector(struct vkms_config *conf
> return ERR_PTR(-ENOMEM);
>
> connector_cfg->possible_encoders = possible_encoders;
> + connector_cfg->status = status;
> list_add_tail(&connector_cfg->list, &config->connectors);
>
> return connector_cfg;
> @@ -249,3 +254,12 @@ void vkms_config_destroy_connector(struct vkms_config *config,
> list_del(&connector_cfg->list);
> kfree(connector_cfg);
> }
> +
> +void vkms_update_connector_status(struct vkms_config *config,
> + struct vkms_config_connector *connector_cfg,
> + enum drm_connector_status status)
> +{
> + connector_cfg->status = status;
> + if (config->dev)
> + drm_kms_helper_hotplug_event(&config->dev->drm);
> +}
> diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
> index 25dab63e7ae7..3237406fa3a3 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.h
> +++ b/drivers/gpu/drm/vkms/vkms_config.h
> @@ -6,6 +6,8 @@
> #include <linux/list.h>
> #include <linux/types.h>
>
> +#include <drm/drm_connector.h>
> +
> struct vkms_device;
>
> struct vkms_config_plane {
> @@ -29,6 +31,9 @@ struct vkms_config_encoder {
> struct vkms_config_connector {
> struct list_head list;
> uint32_t possible_encoders;
> + enum drm_connector_status status;
> + /* only set when instantiated */
> + struct drm_connector *connector;
It seems very strange to have a "dynamic" field (status) in a
configuration structure.
> };
>
> struct vkms_config {
> @@ -72,8 +77,13 @@ void vkms_config_destroy_encoder(struct vkms_config *config,
>
> /* Connectors */
> struct vkms_config_connector *vkms_config_add_connector(struct vkms_config *config,
> - uint32_t possible_encoders);
> + uint32_t possible_encoders,
> + enum drm_connector_status status);
> void vkms_config_destroy_connector(struct vkms_config *config,
> struct vkms_config_connector *connector_cfg);
>
> +void vkms_update_connector_status(struct vkms_config *config,
> + struct vkms_config_connector *connector_cfg,
> + enum drm_connector_status status);
> +
> #endif /* _VKMS_CONFIG_H_ */
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index a5b1ab326cdd..511cc2c14c44 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -7,7 +7,25 @@
> #include <drm/drm_managed.h>
> #include <drm/drm_probe_helper.h>
>
> +static enum drm_connector_status vkms_connector_detect(struct drm_connector *connector,
> + bool force)
> +{
> + struct vkms_device *vkmsdev = drm_device_to_vkms_device(connector->dev);
> + enum drm_connector_status status = connector->status;
> + struct vkms_config_connector *connector_cfg;
> +
> + list_for_each_entry(connector_cfg, &vkmsdev->config->connectors, list) {
> + if (connector_cfg->connector == connector) {
> + status = connector_cfg->status;
> + break;
> + }
> + }
To avoid this, maybe we can create a vkms_connector:
struct vkms_connector {
int status;
struct drm_connector base;
};
and use container_of?
> +
> + return status;
> +}
> +
> static const struct drm_connector_funcs vkms_connector_funcs = {
> + .detect = vkms_connector_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
> .reset = drm_atomic_helper_connector_reset,
> .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> @@ -155,6 +173,8 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> connector = vkms_connector_init(vkmsdev, connector_cfg->possible_encoders);
> if (IS_ERR(connector))
> return PTR_ERR(connector);
> +
> + connector_cfg->connector = connector;
> }
>
> drm_mode_config_reset(dev);
> --
> 2.46.0
>
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Powered by blists - more mailing lists