On Fri, 10 Jun 2022 at 10:30, Maxime Ripard <maxime@xxxxxxxxxx> wrote:
>
> vc4_plane_init() currently initialises the plane with no possible CRTCs,
> and will expect the caller to set it up by itself.
>
> Let's change that logic a bit to follow the syntax of
> drm_universal_plane_init() and pass the possible CRTCs bitmask as an
> argument to the function instead.
>
> Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
> ---
> drivers/gpu/drm/vc4/vc4_crtc.c | 2 +-
> drivers/gpu/drm/vc4/vc4_drv.h | 3 ++-
> drivers/gpu/drm/vc4/vc4_plane.c | 15 +++++++--------
> 3 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 59b20c8f132b..840a93484bb1 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -1138,7 +1138,7 @@ int vc4_crtc_init(struct drm_device *drm, struct vc4_crtc *vc4_crtc,
> * requirement of the plane configuration, and reject ones
> * that will take too much.
> */
> - primary_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_PRIMARY);
> + primary_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_PRIMARY, 0);
> if (IS_ERR(primary_plane)) {
> dev_err(drm->dev, "failed to construct primary plane\n");
> return PTR_ERR(primary_plane);
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 080deae55f64..5125ca1a8158 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -952,7 +952,8 @@ int vc4_kms_load(struct drm_device *dev);
>
> /* vc4_plane.c */
> struct drm_plane *vc4_plane_init(struct drm_device *dev,
> - enum drm_plane_type type);
> + enum drm_plane_type type,
> + unsigned int possible_crtcs);
A nit pick.
possible_crtcs in struct drm_plane is a uint32_t , not an unsigned int.
It would never matter on a Pi as unsigned int will never be 16bit, but
avoids the oddity.
Otherwise:
Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> int vc4_plane_create_additional_planes(struct drm_device *dev);
> u32 vc4_plane_write_dlist(struct drm_plane *plane, u32 __iomem *dlist);
> u32 vc4_plane_dlist_size(const struct drm_plane_state *state);
> diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c
> index b3438f4a81ce..17dab470ecdf 100644
> --- a/drivers/gpu/drm/vc4/vc4_plane.c
> +++ b/drivers/gpu/drm/vc4/vc4_plane.c
> @@ -1451,7 +1451,8 @@ static const struct drm_plane_funcs vc4_plane_funcs = {
> };
>
> struct drm_plane *vc4_plane_init(struct drm_device *dev,
> - enum drm_plane_type type)
> + enum drm_plane_type type,
> + unsigned int possible_crtcs)
> {
> struct drm_plane *plane = NULL;
> struct vc4_plane *vc4_plane;
> @@ -1483,7 +1484,7 @@ struct drm_plane *vc4_plane_init(struct drm_device *dev,
> }
>
> plane = &vc4_plane->base;
> - ret = drm_universal_plane_init(dev, plane, 0,
> + ret = drm_universal_plane_init(dev, plane, possible_crtcs,
> &vc4_plane_funcs,
> formats, num_formats,
> modifiers, type, NULL);
> @@ -1528,13 +1529,11 @@ int vc4_plane_create_additional_planes(struct drm_device *drm)
> */
> for (i = 0; i < 16; i++) {
> struct drm_plane *plane =
> - vc4_plane_init(drm, DRM_PLANE_TYPE_OVERLAY);
> + vc4_plane_init(drm, DRM_PLANE_TYPE_OVERLAY,
> + GENMASK(drm->mode_config.num_crtc - 1, 0));
>
> if (IS_ERR(plane))
> continue;
> -
> - plane->possible_crtcs =
> - GENMASK(drm->mode_config.num_crtc - 1, 0);
> }
>
> drm_for_each_crtc(crtc, drm) {
> @@ -1542,9 +1541,9 @@ int vc4_plane_create_additional_planes(struct drm_device *drm)
> * since we overlay planes on the CRTC in the order they were
> * initialized.
> */
> - cursor_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_CURSOR);
> + cursor_plane = vc4_plane_init(drm, DRM_PLANE_TYPE_CURSOR,
> + drm_crtc_mask(crtc));
> if (!IS_ERR(cursor_plane)) {
> - cursor_plane->possible_crtcs = drm_crtc_mask(crtc);
> crtc->cursor = cursor_plane;
> }
> }
> --
> 2.36.1
>
