Re: [RFC PATCH 2/3] drm/atomic-helper: add REQUIRE_MATCHING_FB flag

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Sep 10, 2020 at 11:24:24AM +0200, Stefan Agner wrote:
> Add flag which checks that the framebuffer size matches the plane size
> exactly. This is useful for display controller which can't handle
> framebuffers other than the plane/CRTC size.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c               | 7 +++++++
>  drivers/gpu/drm/selftests/test-drm_plane_helper.c | 9 +++++++++
>  include/drm/drm_atomic_helper.h                   | 1 +
>  3 files changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 755572a37f3f..8bc7f8c2e566 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -802,6 +802,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>  	int hscale, vscale;
>  	bool can_position = flags & DRM_PLANE_CAN_POSITION;
>  	bool can_update_disabled = flags & DRM_PLANE_CAN_UPDATE_DISABLED;
> +	bool require_matching_fb = flags & DRM_PLANE_REQUIRE_MATCHING_FB;
>  
>  	WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
>  
> @@ -860,6 +861,12 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>  		return -EINVAL;
>  	}
>  
> +	if (require_matching_fb && (drm_rect_width(src) != fb->width ||
> +	    drm_rect_height(src) != fb->height)) {
> +		DRM_DEBUG_KMS("Framebuffer size must match plane size.\n");
> +		return -EINVAL;
> +	}

I think you also want to check that the x,y src are at (0, 0).

Plus needs kerneldoc.

But aside from the details, I like.

> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
> diff --git a/drivers/gpu/drm/selftests/test-drm_plane_helper.c b/drivers/gpu/drm/selftests/test-drm_plane_helper.c
> index 01e95b2d572f..2c81bbef830e 100644
> --- a/drivers/gpu/drm/selftests/test-drm_plane_helper.c
> +++ b/drivers/gpu/drm/selftests/test-drm_plane_helper.c
> @@ -139,6 +139,15 @@ int igt_check_plane_state(void *ignored)
>  	FAIL_ON(!check_src_eq(&plane_state, 0, 0, 1023 << 16, 767 << 16));
>  	FAIL_ON(!check_crtc_eq(&plane_state, 0, 0, 1023, 767));
>  
> +	/* Check whether requiring same size framebuffer works correctly. */
> +	set_src(&plane_state, 0, 0, 1024 << 16, 768 << 16);
> +	set_crtc(&plane_state, 0, 0, 1024, 768);
> +	ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_HELPER_NO_SCALING,
> +						  DRM_PLANE_REQUIRE_MATCHING_FB);
> +	FAIL(!ret, "Should not be able to use different size framebuffer with REQUIRE_MATCHING_FB\n");

I think also needs a negative test with src_x/y != 0, plus maybe one that
succeeds.

Cheers, Daniel
> +
>  	/* Simple scaling tests. */
>  	set_src(&plane_state, 0, 0, 512 << 16, 384 << 16);
>  	set_crtc(&plane_state, 0, 0, 1024, 768);
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index bb9957b4f91b..244b730e84d3 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -43,6 +43,7 @@ int drm_atomic_helper_check_modeset(struct drm_device *dev,
>  
>  #define DRM_PLANE_CAN_POSITION				BIT(0)
>  #define DRM_PLANE_CAN_UPDATE_DISABLED			BIT(1)
> +#define DRM_PLANE_REQUIRE_MATCHING_FB			BIT(2)
>  
>  int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
>  					const struct drm_crtc_state *crtc_state,
> -- 
> 2.28.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel