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@xxxxxxxx> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel