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

[Daniel Vetter]

On Fri, Oct 21, 2016 at 09:26:22AM -0400, Rob Clark wrote:
> On Fri, Oct 21, 2016 at 8:35 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> > On Fri, Oct 21, 2016 at 09:58:45AM +0100, Brian Starkey 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.
> >>
> >> 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...
> >
> > I think first thing we should do is add some backoff heuristics to
> > drm_hwcomposer to try the same plane also with a non-scaled surface (after
> > having tried it with a scaled one). That would probably be as effective as
> > adding "can_scale", but with the upshot that we're not at the mercy of
> > drivers exposing it correctly.
> 
> well, ignoring the fact that drm-hwc2 just tries one thing then falls
> back to gl (which should be fixed but it is a big pile of c++11 code
> so that will be someone elses job ;-))...
> 
> I did think about this approach, but two many changing variables so
> making userspace guess about this sort of thing just seems evil.

Imo drm-hwc2 needs to be fixed. Adding new uabi because the current
userspace makes a few too many assumption about how hw works (the only
thing which is supposed to always work is one full-screen unscaled primary
buffer) feels wrong. Imo the proper fix is to fix userspace to not scale
in the absolute last-resort fallback path. Because that won't work on many
i915 platforms either (or on many other chips fwiw).

> > I'm always vary when we have the same limit checks 2 (in this case once in
> > atomic_check, and once in the code that registers the property). And I'd
> > like to avoid that as much as possible. We could avoid this by making the
> > can_scale property mandatory, and enforcing it in the drm core. I.e. if
> > it's not set, we reject scaled planes. That should give some decent
> > motivation for driver writers to update them correctly. But without such a
> > self-consistency check I don't really like this. It would be akin to
> > adding "can_rotate" besides the "rotation" prop, and allowing drivers to
> > botch things up and not register stuff consistently.
> 
> Fair enough, I'll add a check in drm core.  That is simple enough.  I
> guess remaining question should can_scale default to true?

What's the point in making it true by default, i.e. lying by default?

> I guess the first time someone tries to bring up drm-hwc or similar
> (ie. something more complex than single fullscreen layer) on their hw,
> they are going to run into a few bugs in their driver, so I guess I'd
> be ok for it to default to false and let people fix it when the time
> comes.

Still not convinced that can_scale is worth it. I'd like to fix userspace
first, before we roll out the duct-tape on the kernel ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch