Le 13/08/24 - 12:44, José Expósito a écrit :
> Add a list of overlay planes to vkms_config and add as many overlay
> planes as configured during output initialization.
>
> For backwards compatibility, add NUM_OVERLAY_PLANES to the default
> configuration.
>
> Note that a primary plane is created for each CRTC and it is not
> possible to configure it.
> A cursor plane is added conditionally if it is enabled in the CRTC
> configuration.
>
> Signed-off-by: José Expósito <jose.exposito89@xxxxxxxxx>
> ---
> drivers/gpu/drm/vkms/vkms_config.c | 53 +++++++++++++++++++++++++++---
> drivers/gpu/drm/vkms/vkms_config.h | 16 +++++++--
> drivers/gpu/drm/vkms/vkms_drv.c | 2 +-
> drivers/gpu/drm/vkms/vkms_drv.h | 5 +--
> drivers/gpu/drm/vkms/vkms_output.c | 43 ++++++++++++------------
> drivers/gpu/drm/vkms/vkms_plane.c | 6 ++--
> 6 files changed, 91 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
> index a456f9db3c66..d95a42a6745a 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.c
> +++ b/drivers/gpu/drm/vkms/vkms_config.c
> @@ -17,6 +17,7 @@ struct vkms_config *vkms_config_create(char *dev_name)
> return ERR_PTR(-ENOMEM);
>
> config->dev_name = dev_name;
> + config->planes = (struct list_head)LIST_HEAD_INIT(config->planes);
> config->crtcs = (struct list_head)LIST_HEAD_INIT(config->crtcs);
> config->encoders = (struct list_head)LIST_HEAD_INIT(config->encoders);
> config->connectors = (struct list_head)LIST_HEAD_INIT(config->connectors);
> @@ -32,15 +33,22 @@ struct vkms_config *vkms_config_default_create(bool enable_cursor,
> struct vkms_config_crtc *crtc_cfg;
> struct vkms_config_encoder *encoder_cfg;
> struct vkms_config_connector *connector_cfg;
> + struct vkms_config_plane *plane_cfg;
> + int n;
>
> config = vkms_config_create(DEFAULT_DEVICE_NAME);
> if (IS_ERR(config))
> return config;
>
> - config->cursor = enable_cursor;
> - config->overlay = enable_overlay;
> + if (enable_overlay) {
> + for (n = 0; n < NUM_OVERLAY_PLANES; n++) {
> + plane_cfg = vkms_config_add_overlay_plane(config, BIT(0));
> + if (IS_ERR(plane_cfg))
> + return ERR_CAST(plane_cfg);
The config pointer is leaked here.
> + }
> + }
>
> - crtc_cfg = vkms_config_add_crtc(config, enable_writeback);
> + crtc_cfg = vkms_config_add_crtc(config, enable_cursor, enable_writeback);
> if (IS_ERR(crtc_cfg))
> return ERR_CAST(crtc_cfg);
>
> @@ -57,10 +65,14 @@ struct vkms_config *vkms_config_default_create(bool enable_cursor,
>
> void vkms_config_destroy(struct vkms_config *config)
> {
> + struct vkms_config_plane *plane_cfg, *plane_tmp;
> struct vkms_config_crtc *crtc_cfg, *crtc_tmp;
> struct vkms_config_encoder *encoder_cfg, *encoder_tmp;
> struct vkms_config_connector *connector_cfg, *connector_tmp;
>
> + list_for_each_entry_safe(plane_cfg, plane_tmp, &config->planes, list)
> + vkms_config_destroy_overlay_plane(config, plane_cfg);
> +
> list_for_each_entry_safe(crtc_cfg, crtc_tmp, &config->crtcs, list)
> vkms_config_destroy_crtc(config, crtc_cfg);
>
> @@ -78,17 +90,24 @@ static int vkms_config_show(struct seq_file *m, void *data)
> struct drm_debugfs_entry *entry = m->private;
> struct drm_device *dev = entry->dev;
> struct vkms_device *vkmsdev = drm_device_to_vkms_device(dev);
> + struct vkms_config_plane *plane_cfg;
> struct vkms_config_crtc *crtc_cfg;
> struct vkms_config_encoder *encoder_cfg;
> struct vkms_config_connector *connector_cfg;
> int n;
>
> seq_printf(m, "dev_name=%s\n", vkmsdev->config->dev_name);
> - seq_printf(m, "cursor=%d\n", vkmsdev->config->cursor);
> - seq_printf(m, "overlay=%d\n", vkmsdev->config->overlay);
> +
> + n = 0;
> + list_for_each_entry(plane_cfg, &vkmsdev->config->planes, list) {
> + seq_printf(m, "plane(%d).possible_crtcs=%d\n", n,
> + plane_cfg->possible_crtcs);
> + n++;
> + }
>
> n = 0;
> list_for_each_entry(crtc_cfg, &vkmsdev->config->crtcs, list) {
> + seq_printf(m, "crtc(%d).cursor=%d\n", n, crtc_cfg->cursor);
> seq_printf(m, "crtc(%d).writeback=%d\n", n,
> crtc_cfg->writeback);
> n++;
> @@ -121,7 +140,30 @@ void vkms_config_debugfs_init(struct vkms_device *vkms_device)
> ARRAY_SIZE(vkms_config_debugfs_list));
> }
>
> +struct vkms_config_plane *vkms_config_add_overlay_plane(struct vkms_config *config,
> + uint32_t possible_crtcs)
> +{
> + struct vkms_config_plane *plane_cfg;
> +
> + plane_cfg = kzalloc(sizeof(*plane_cfg), GFP_KERNEL);
> + if (!plane_cfg)
> + return ERR_PTR(-ENOMEM);
> +
> + plane_cfg->possible_crtcs = possible_crtcs;
> + list_add_tail(&plane_cfg->list, &config->planes);
> +
> + return plane_cfg;
> +}
> +
> +void vkms_config_destroy_overlay_plane(struct vkms_config *config,
> + struct vkms_config_plane *plane_cfg)
> +{
> + list_del(&plane_cfg->list);
> + kfree(plane_cfg);
> +}
> +
> struct vkms_config_crtc *vkms_config_add_crtc(struct vkms_config *config,
> + bool enable_cursor,
> bool enable_writeback)
> {
> struct vkms_config_crtc *crtc_cfg;
> @@ -130,6 +172,7 @@ struct vkms_config_crtc *vkms_config_add_crtc(struct vkms_config *config,
> if (!crtc_cfg)
> return ERR_PTR(-ENOMEM);
>
> + crtc_cfg->cursor = enable_cursor;
> crtc_cfg->writeback = enable_writeback;
>
> crtc_cfg->index = 0;
> diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
> index f1dd59fc6300..25dab63e7ae7 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.h
> +++ b/drivers/gpu/drm/vkms/vkms_config.h
> @@ -8,9 +8,15 @@
>
> struct vkms_device;
>
> +struct vkms_config_plane {
> + struct list_head list;
> + uint32_t possible_crtcs;
As for encoder and connector, the crtc cleanup does not change
possible_crtcs value, so you may end up with an invalid configuration.
> +};
> +
> struct vkms_config_crtc {
> struct list_head list;
> unsigned int index;
> + bool cursor;
> bool writeback;
> };
>
> @@ -27,8 +33,7 @@ struct vkms_config_connector {
>
> struct vkms_config {
> char *dev_name;
> - bool cursor;
> - bool overlay;
> + struct list_head planes;
> struct list_head crtcs;
> struct list_head encoders;
> struct list_head connectors;
> @@ -46,8 +51,15 @@ void vkms_config_destroy(struct vkms_config *config);
> /* DebugFS */
> void vkms_config_debugfs_init(struct vkms_device *vkms_device);
>
> +/* Planes */
> +struct vkms_config_plane *vkms_config_add_overlay_plane(struct vkms_config *config,
> + uint32_t possible_crtcs);
> +void vkms_config_destroy_overlay_plane(struct vkms_config *config,
> + struct vkms_config_plane *plane_cfg);
> +
> /* CRTCs */
> struct vkms_config_crtc *vkms_config_add_crtc(struct vkms_config *config,
> + bool enable_cursor,
> bool enable_writeback);
> void vkms_config_destroy_crtc(struct vkms_config *config,
> struct vkms_config_crtc *crtc_cfg);
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 15a2ba26d190..b0a079eb4598 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -154,7 +154,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
> dev->mode_config.preferred_depth = 0;
> dev->mode_config.helper_private = &vkms_mode_config_helpers;
>
> - return vkms_output_init(vkmsdev, 0);
> + return vkms_output_init(vkmsdev);
> }
>
> static int vkms_create(struct vkms_config *config)
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index cac37d21654a..76394285dc68 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -149,10 +149,11 @@ struct vkms_crtc *vkms_crtc_init(struct drm_device *dev,
> struct drm_plane *cursor,
> unsigned int index);
>
> -int vkms_output_init(struct vkms_device *vkmsdev, int index);
> +int vkms_output_init(struct vkms_device *vkmsdev);
>
> struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> - enum drm_plane_type type, int index);
> + enum drm_plane_type type,
> + uint32_t possible_crtcs);
>
> /* CRC Support */
> const char *const *vkms_get_crc_sources(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 021a491de817..a5b1ab326cdd 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -82,21 +82,22 @@ static struct drm_encoder *vkms_encoder_init(struct vkms_device *vkms_device,
> return encoder;
> }
>
> -static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int index)
> +static int vkms_add_overlay_plane(struct vkms_device *vkmsdev,
> + uint32_t possible_crtcs)
> {
> struct vkms_plane *overlay;
>
> - overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY, index);
> + overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY, possible_crtcs);
> if (IS_ERR(overlay))
> return PTR_ERR(overlay);
>
> if (!overlay->base.possible_crtcs)
> - overlay->base.possible_crtcs = BIT(index);
> + overlay->base.possible_crtcs = possible_crtcs;
>
> return 0;
> }
>
> -int vkms_output_init(struct vkms_device *vkmsdev, int index)
> +int vkms_output_init(struct vkms_device *vkmsdev)
> {
> struct drm_device *dev = &vkmsdev->drm;
> struct drm_connector *connector;
> @@ -106,29 +107,27 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
> struct vkms_crtc *vkms_crtc;
> struct vkms_config_crtc *crtc_cfg;
> struct vkms_plane *primary, *cursor = NULL;
> + struct vkms_config_plane *plane_cfg;
> int ret;
> int writeback;
> - unsigned int n;
>
> - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index);
> - if (IS_ERR(primary))
> - return PTR_ERR(primary);
> -
> - if (vkmsdev->config->overlay) {
> - for (n = 0; n < NUM_OVERLAY_PLANES; n++) {
> - ret = vkms_add_overlay_plane(vkmsdev, index);
> - if (ret)
> - return ret;
> - }
> - }
> -
> - if (vkmsdev->config->cursor) {
> - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index);
> - if (IS_ERR(cursor))
> - return PTR_ERR(cursor);
> + list_for_each_entry(plane_cfg, &vkmsdev->config->planes, list) {
> + ret = vkms_add_overlay_plane(vkmsdev, plane_cfg->possible_crtcs);
> + if (ret)
> + return ret;
> }
>
> list_for_each_entry(crtc_cfg, &vkmsdev->config->crtcs, list) {
> + primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, 0);
> + if (IS_ERR(primary))
> + return PTR_ERR(primary);
> +
> + if (crtc_cfg->cursor) {
> + cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, 0);
Why are you hardcoding the primary and cursor plane here? I think it is
easier to manage them as "normal" planes with vkms_config_plane, and use
the enum drm_plane_type to find the correct one here.
Otherwise we will need either to:
- duplicate all the plane configuration for those primary and cursor
planes
- rework this part of the code to use vkms_config_plane.
> + if (IS_ERR(cursor))
> + return PTR_ERR(cursor);
> + }
> +
> vkms_crtc = vkms_crtc_init(dev, &primary->base, &cursor->base,
> crtc_cfg->index);
> if (IS_ERR(vkms_crtc))
> @@ -141,6 +140,8 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
> if (writeback)
> DRM_ERROR("Failed to init writeback connector\n");
> }
> +
> + cursor = NULL;
> }
>
> list_for_each_entry(encoder_cfg, &vkmsdev->config->encoders, list) {
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index e5c625ab8e3e..eb3edf4cb928 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -198,13 +198,13 @@ static const struct drm_plane_helper_funcs vkms_plane_helper_funcs = {
> };
>
> struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> - enum drm_plane_type type, int index)
> + enum drm_plane_type type, uint32_t possible_crtcs)
> {
> struct drm_device *dev = &vkmsdev->drm;
> struct vkms_plane *plane;
>
> - plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 1 << index,
> - &vkms_plane_funcs,
> + plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base,
> + possible_crtcs, &vkms_plane_funcs,
> vkms_formats, ARRAY_SIZE(vkms_formats),
> NULL, type, NULL);
> if (IS_ERR(plane))
> --
> 2.46.0
>
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
