> The current vkms driver only allows the usage of one primary, eight
> overlays and one cursor plane. This new configuration structure aims to
> make the configuration more flexible.
>
> Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
> ---
> drivers/gpu/drm/vkms/vkms_config.c | 90 ++++++++++++++++++++++++++++++++++++--
> drivers/gpu/drm/vkms/vkms_config.h | 44 +++++++++++++++++--
> drivers/gpu/drm/vkms/vkms_drv.c | 33 +++++---------
> drivers/gpu/drm/vkms/vkms_plane.c | 4 +-
> drivers/gpu/drm/vkms/vkms_plane.h | 3 +-
> 5 files changed, 141 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
> index 27b49a2ad9c8..b1c6160d350f 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.c
> +++ b/drivers/gpu/drm/vkms/vkms_config.c
> @@ -13,32 +13,111 @@ struct vkms_config *vkms_config_create(void)
> if (!config)
> return ERR_PTR(-ENOMEM);
>
> + INIT_LIST_HEAD(&config->planes);
> +
> return config;
> }
>
> struct vkms_config *vkms_config_alloc_default(bool enable_writeback, bool enable_overlay,
> bool enable_cursor)
> {
> + struct vkms_config_plane *plane;
> struct vkms_config *vkms_config = vkms_config_create();
>
> if (IS_ERR(vkms_config))
> return vkms_config;
>
> vkms_config->writeback = enable_writeback;
> - vkms_config->overlay = enable_overlay;
> - vkms_config->cursor = enable_cursor;
> +
> + plane = vkms_config_create_plane(vkms_config);
> + if (!plane)
> + goto err_alloc;
> +
> + plane->type = DRM_PLANE_TYPE_PRIMARY;
Since all planes need to have a type, I think it'd make sense
to make "type" a param of vkms_config_create_plane().
> +
> + if (enable_overlay) {
> + for (int i = 0; i < NUM_OVERLAY_PLANES; i++) {
> + plane = vkms_config_create_plane(vkms_config);
> + if (!plane)
> + goto err_alloc;
> + plane->type = DRM_PLANE_TYPE_OVERLAY;
> + }
> + }
> + if (enable_cursor) {
> + plane = vkms_config_create_plane(vkms_config);
> + if (!plane)
> + goto err_alloc;
> + plane->type = DRM_PLANE_TYPE_CURSOR;
> + }
>
> return vkms_config;
> +err_alloc:
> + vkms_config_destroy(vkms_config);
> + return ERR_PTR(-ENOMEM);
> }
>
> +struct vkms_config_plane *vkms_config_create_plane(struct vkms_config *vkms_config)
> +{
> + if (!vkms_config)
> + return NULL;
> +
> + struct vkms_config_plane *vkms_config_overlay = kzalloc(sizeof(*vkms_config_overlay),
> + GFP_KERNEL);
Nit: I guess the variable is named "vkms_config_overlay" due to a rebase.
As "struct vkms_config_plane" is used to configure any plane, it'd be
great to rename the variable to reflect it.
> +
> + if (!vkms_config_overlay)
> + return NULL;
> +
> + vkms_config_overlay->type = DRM_PLANE_TYPE_OVERLAY;
> +
> + list_add(&vkms_config_overlay->link, &vkms_config->planes);
> +
> + return vkms_config_overlay;
> +}
> +
> +void vkms_config_delete_plane(struct vkms_config_plane *vkms_config_overlay)
> +{
> + if (!vkms_config_overlay)
> + return;
> + list_del(&vkms_config_overlay->link);
> + kfree(vkms_config_overlay);
> +}
>
> void vkms_config_destroy(struct vkms_config *config)
> {
> + struct vkms_config_plane *vkms_config_plane, *tmp_plane;
> +
> + list_for_each_entry_safe(vkms_config_plane, tmp_plane, &config->planes, link) {
> + vkms_config_delete_plane(vkms_config_plane);
> + }
You can drop the extra curly brackets here.
> +
> kfree(config);
> }
>
> bool vkms_config_is_valid(struct vkms_config *config)
> {
> + struct vkms_config_plane *config_plane;
> +
> + bool has_cursor = false;
> + bool has_primary = false;
> +
> + list_for_each_entry(config_plane, &config->planes, link) {
> + if (config_plane->type == DRM_PLANE_TYPE_PRIMARY) {
> + // Multiple primary planes for only one CRTC
> + if (has_primary)
> + return false;
> + has_primary = true;
> + }
> + if (config_plane->type == DRM_PLANE_TYPE_CURSOR) {
> + // Multiple cursor planes for only one CRTC
> + if (has_cursor)
> + return false;
> + has_cursor = true;
> + }
> + }
> +
> + if (!has_primary)
> + return false;
> +
> return true;
> }
>
> @@ -47,10 +126,13 @@ 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 *config_plane;
>
> seq_printf(m, "writeback=%d\n", vkmsdev->config->writeback);
> - seq_printf(m, "cursor=%d\n", vkmsdev->config->cursor);
> - seq_printf(m, "overlay=%d\n", vkmsdev->config->overlay);
> + list_for_each_entry(config_plane, &vkmsdev->config->planes, link) {
> + seq_puts(m, "plane:\n");
> + seq_printf(m, "\ttype: %d\n", config_plane->type);
> + }
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
> index 50c3b021a66b..77c1c3934189 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.h
> +++ b/drivers/gpu/drm/vkms/vkms_config.h
> @@ -10,15 +10,34 @@
> * struct vkms_config - General configuration for VKMS driver
> *
> * @writeback: If true, a writeback buffer can be attached to the CRTC
> - * @cursor: If true, a cursor plane is created in the VKMS device
> - * @overlay: If true, NUM_OVERLAY_PLANES will be created for the VKMS device
> + * @planes: List of planes configured for this device. They are created by the function
> + * vkms_config_create_plane().
> * @dev: Used to store the current vkms device. Only set when the device is instancied.
> */
> struct vkms_config {
> bool writeback;
> - bool cursor;
> - bool overlay;
> struct vkms_device *dev;
> +
> + struct list_head planes;
> +};
> +
> +/**
> + * struct vkms_config_plane
> + *
> + * @link: Link to the others planes
> + * @type: Type of the plane. The creator of configuration needs to ensures that at least one
> + * plane is primary.
> + * @plane: Internal usage. This pointer should never be considered as valid. It can be used to
> + * store a temporary reference to a vkms plane during device creation. This pointer is
> + * not managed by the configuration and must be managed by other means.
> + */
> +struct vkms_config_plane {
> + struct list_head link;
> +
> + enum drm_plane_type type;
> +
> + /* Internal usage */
> + struct vkms_plane *plane;
> };
>
> /**
> @@ -42,6 +61,23 @@ void vkms_config_destroy(struct vkms_config *config);
> */
> bool vkms_config_is_valid(struct vkms_config *vkms_config);
>
> +/**
> + * vkms_config_create_plane() - Create a plane configuration
> + *
> + * This will allocate and add a new plane to @vkms_config. This plane will have by default the
> + * maximum supported values.
> + * @vkms_config: Configuration where to insert new plane
> + */
> +struct vkms_config_plane *vkms_config_create_plane(struct vkms_config *vkms_config);
> +
> +/**
> + * vkms_config_delete_plane() - Remove a plane configuration and frees its memory
> + *
> + * This will delete a plane configuration from the parent configuration. This will NOT
> + * cleanup and frees the vkms_plane that can be stored in @vkms_config_plane.
> + * @vkms_config_plane: Plane configuration to cleanup
> + */
> +void vkms_config_delete_plane(struct vkms_config_plane *vkms_config_plane);
>
> /**
> * vkms_config_alloc_default() - Allocate the configuration for the default device
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index af237daa3c5c..403006a0bb61 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -153,38 +153,27 @@ static const struct drm_connector_helper_funcs vkms_conn_helper_funcs = {
>
> static int vkms_output_init(struct vkms_device *vkmsdev)
> {
> + struct vkms_config_plane *config_plane;
> struct drm_device *dev = &vkmsdev->drm;
> struct drm_connector *connector;
> struct drm_encoder *encoder;
> struct vkms_crtc *crtc;
> struct drm_plane *plane;
> - struct vkms_plane *primary, *cursor, *overlay = NULL;
> + struct vkms_plane *primary, *cursor = NULL;
> int ret;
> int writeback;
> - unsigned int n;
>
> - /*
> - * Initialize used plane. One primary plane is required to perform the composition.
> - *
> - * The overlay and cursor planes are not mandatory, but can be used to perform complex
> - * composition.
> - */
> - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY);
> - if (IS_ERR(primary))
> - return PTR_ERR(primary);
> -
> - if (vkmsdev->config->overlay) {
> - for (n = 0; n < NUM_OVERLAY_PLANES; n++) {
> - overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY);
> - if (IS_ERR(overlay))
> - return PTR_ERR(overlay);
> + list_for_each_entry(config_plane, &vkmsdev->config->planes, link) {
> + config_plane->plane = vkms_plane_init(vkmsdev, config_plane);
> + if (IS_ERR(config_plane->plane)) {
> + ret = PTR_ERR(config_plane->plane);
> + return ret;
> }
> - }
>
> - if (vkmsdev->config->cursor) {
> - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR);
> - if (IS_ERR(cursor))
> - return PTR_ERR(cursor);
> + if (config_plane->type == DRM_PLANE_TYPE_PRIMARY)
> + primary = config_plane->plane;
> + else if (config_plane->type == DRM_PLANE_TYPE_CURSOR)
> + cursor = config_plane->plane;
> }
>
> /* [1]: Initialize the crtc component */
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index b5740c27180b..dc9bccf60071 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -223,7 +223,7 @@ 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)
> + struct vkms_config_plane *config)
> {
> struct drm_device *dev = &vkmsdev->drm;
> struct vkms_plane *plane;
> @@ -231,7 +231,7 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 0,
> &vkms_plane_funcs,
> vkms_formats, ARRAY_SIZE(vkms_formats),
> - NULL, type, NULL);
> + NULL, config->type, NULL);
> if (IS_ERR(plane))
> return plane;
>
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.h b/drivers/gpu/drm/vkms/vkms_plane.h
> index 95b2428331b8..82049966ee7e 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.h
> +++ b/drivers/gpu/drm/vkms/vkms_plane.h
> @@ -9,6 +9,7 @@
> #include <linux/iosys-map.h>
>
> #include "vkms_formats.h"
> +#include "vkms_config.h"
>
> struct vkms_plane {
> struct drm_plane base;
> @@ -54,7 +55,7 @@ struct vkms_frame_info {
> * @type: type of plane to initialize
> */
> struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> - enum drm_plane_type type);
> + struct vkms_config_plane *config);
>
> #define drm_plane_state_to_vkms_plane_state(target) \
> container_of(target, struct vkms_plane_state, base.base)
>
