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
