[RFC] drm: add hint to userspace about whether a plane can scale

[Brian Starkey]

Hi Rob,

On Thu, Oct 20, 2016 at 04:17:14PM -0400, Rob Clark wrote:
>When you have a mix of planes that can scale and those that cannot
>scale, userspace really wants to have some hint to know which planes
>can definitely not scale so it knows to assign them to unscaled layers.
>I don't think it is fully possible to describe scaling constraints in
>a generic way, so I don't think it is even worth trying, so this is
>not a substitute for atomic TESTONLY step, but it does reduce the
>search-space for userspace.  In the common case, most layers will not
>be scaled so knowing the best planes to pick for those layers is
>useful.

Somewhat related to what Daniel mentioned on IRC about driver
consistency - how about making it "cannot_scale". This is then opt-in
for drivers, and should mean userspace can always trust it if it's
set.

i.e. if cannot_scale == false, userspace can give it a shot, but if
cannot_scale == true it should never bother.

Either way, even with a device-specific planner we would want this
hint to manage different HW versions so I'm in favour. But...

>---
> drivers/gpu/drm/drm_crtc.c                | 1 +
> drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 1 +
> include/drm/drm_crtc.h                    | 2 ++
> include/uapi/drm/drm_mode.h               | 3 +++
> 4 files changed, 7 insertions(+)
>
>diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>index b4b973f..d7e0e0d 100644
>--- a/drivers/gpu/drm/drm_crtc.c
>+++ b/drivers/gpu/drm/drm_crtc.c
>@@ -2389,6 +2389,7 @@ int drm_mode_getplane(struct drm_device *dev, void *data,
> 	plane_resp->plane_id = plane->base.id;
> 	plane_resp->possible_crtcs = plane->possible_crtcs;
> 	plane_resp->gamma_size = 0;
>+	plane_resp->can_scale = plane->can_scale;
>
> 	/*
> 	 * This ioctl is called twice, once to determine how much space is
>diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>index 692c888..2061c83 100644
>--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
>@@ -908,6 +908,7 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev,
> 	mdp5_plane->pipe = pipe;
> 	mdp5_plane->name = pipe2name(pipe);
> 	mdp5_plane->caps = caps;
>+	plane->can_scale = !!(caps & MDP_PIPE_CAP_SCALE);
>
> 	mdp5_plane->nformats = mdp_get_formats(mdp5_plane->formats,
> 		ARRAY_SIZE(mdp5_plane->formats),
>diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>index d74d47a..6e290b6 100644
>--- a/include/drm/drm_crtc.h
>+++ b/include/drm/drm_crtc.h
>@@ -1679,6 +1679,7 @@ enum drm_plane_type {
>  * @format_types: array of formats supported by this plane
>  * @format_count: number of formats supported
>  * @format_default: driver hasn't supplied supported formats for the plane
>+ * @can_scale: a hint to userspace that this plane can (or cannot) scale
>  * @crtc: currently bound CRTC
>  * @fb: currently bound fb
>  * @old_fb: Temporary tracking of the old fb while a modeset is ongoing. Used by
>@@ -1710,6 +1711,7 @@ struct drm_plane {
> 	uint32_t *format_types;
> 	unsigned int format_count;
> 	bool format_default;
>+	bool can_scale;
>
> 	struct drm_crtc *crtc;
> 	struct drm_framebuffer *fb;
>diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
>index ce71ad5..5bf9361 100644
>--- a/include/uapi/drm/drm_mode.h
>+++ b/include/uapi/drm/drm_mode.h
>@@ -191,6 +191,9 @@ struct drm_mode_get_plane {
>
> 	__u32 count_format_types;
> 	__u64 format_type_ptr;
>+
>+	__u32 can_scale;

... 32 bits for a bool does seem a bit wasteful.

Thanks,
Brian

>+	__u32 pad;
> };
>
> struct drm_mode_get_plane_res {
>-- 
>2.7.4
>
>_______________________________________________
>dri-devel mailing list
>dri-devel at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel