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

[Rob Clark]

On Fri, Oct 21, 2016 at 4:58 AM, Brian Starkey <brian.starkey at arm.com> wrote:
> 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.

yeah, I thought about it.. but a negative-cap sounded funny.  I could
just initialize plane->can_scale to true in drm_universal_plane_init()
if needed.

> 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.

alternative is to make it a bitmask of caps, although not sure how
much else we would add.  And isn't like this ioctl is critical path,
or anything like that.

BR,
-R

> 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