Re: Planes enumeration with sun4i-drm driver

[Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org>]

On Thu, Oct 18, 2018 at 01:45:16PM +0200, Daniel Vetter wrote:
> On Thu, Oct 18, 2018 at 12:27 PM Maxime Ripard
> <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> >
> > On Mon, Oct 15, 2018 at 04:52:04PM +0200, Daniel Vetter wrote:
> > > On Mon, Oct 15, 2018 at 10:29 AM Maxime Ripard
> > > <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > > On Fri, Oct 12, 2018 at 06:47:13PM +0200, Paul Kocialkowski wrote:
> > > > > I'm looking at the sun4i DRM driver these days (especially for
> > > > > mainlining the VPU tiled format support through the frontend).
> > > > >
> > > > > The way things are done currently, all the possibly-supported plane
> > > > > formats are listed in sun4i_backend_layer_formats and exposed as-is to
> > > > > userspace for every plane. However, some of these formats cannot be
> > > > > used on all planes at the same time and will be rejected when checking
> > > > > the atomic commit.
> > > > >
> > > > > I find this a bit confusing from a userspace perspective.
> > > > >
> > > > > Not all constraints can be provided to userspace (e.g. the number of
> > > > > planes we can effectively scale), but when it comes to formats, we have
> > > > > that possibility. So perhaps it would make sense to only enumerate
> > > > > formats as many times as we can support them.
> > > > >
> > > > > For instance, it could look like:
> > > > > # plane 0: primary, RGB only
> > > > > # plane 1: overlay, RGB + backend YUV formats
> > > > > # plane 2: overlay, RGB + frontend YUV formats
> > > > > # plane 3: overlay, RGB only
> > > > >
> > > > > That's not perfect either, because e.g. requesting a scaled RGB plane
> > > > > will require the frontend and thus make it impossible to use the
> > > > > frontend YUV formats that would still be described. But it feels like
> > > > > it would be closer to representing hardware capabilities than our
> > > > > current situation.
> > > >
> > > > That's really arguable. The hardware capabilities are that you can use
> > > > any of those formats or features, on any plane, *but* on only one
> > > > plane at the same time. What you describe isn't closer to it, it's
> > > > just a way to workaround the issue we are facing (ie being able to
> > > > show those constraints to userspace), and an imperfect workaround.
> > > >
> > > > > I am really not sure about the DRM subsystem policy about this, though.
> > > > > Perhaps it was already decided that it's fine to deal with everything
> > > > > at commit checking time and report more formats than the hardware can
> > > > > really handle.
> > > >
> > > > It doesn't really matter what the DRM policy is here. Any change in
> > > > the direction you suggest would be a regression from the userspace
> > > > point of view and therefore would have to be reverted.
> > >
> > > How exactly can this cause a regression? Do you have some userspace
> > > that card-codes it's allocation of planes, which would then fail here
> > > and worked beforehand?
> >
> > Would that be considered completely unlikely that someone would have
> > used the primary plane with say, an YUV format to output video to it?
> >
> > > > IIRC Weston tries to discover those constraints by brute-forcing
> > > > atomic_check and figuring out a combination that works, and we would
> > > > surely benefit some kind of API to expose composition constraints.
> > >
> > > The current hints we have is to limit the features each plane exposes.
> > > Currently there's no proposal to do stuff like "only x of y planes can
> > > do $feature" at all. So yeah, Paul's idea doesn't seem entirely out of
> > > hand to me, and for the "bug regressions" we need a concrete example
> > > of what will break. Since we're fine-tuning the drm api all the time,
> > > within the limits of what userspace expects :-)
> >
> > It's really not just a matter of formats. The scaler is also involved,
> > or the CSC.
> >
> > There's two IP's, basically, the frontend and the backend. The backend
> > has support for 4 framebuffers, and can have in input RGB. It can also
> > support YUV through an additional data path whose output can be routed
> > to one plane. So you can have one YUV plane, on any plane.
> >
> > Then comes the tiled YUV formats. They can be supported through the
> > frontend, and the frontend output can be output on any plane of the
> > backend. Except that there is just one frontend.
> >
> > Then comes the scaler. If you have a scaling factor of 2 or 4, it can
> > be done on any of the backend plane. For any other factor, this has to
> > be supported through the frontend.
> >
> > The frontend doesn't support all the formats the backend does though,
> > so you have to make sure that combination works. And then, you have to
> > make sure that you're not using the tiled YUV format and the scaler on
> > two different planes.
> >
> > And then, the CSC is also performed by the frontend. So you have to
> > throw that into the mix too.
> >
> > Last time we discussed this, before introducing support for those, at
> > XDC 2016, every one agreed that the atomic_check was the way to go. As
> > you can see above, reducing the list of formats exposed doesn't even
> > solve the issue, it just reduces part of its scope, while not
> > adressing the main issue being that we have no way from the userspace
> > to get those constraints in the first place.
> 
> Ok, with that additional context I agree atomic_check is really the
> only option. I also don't think there's a way to describe this using
> hint properties, without essentially describing sun4i hardware.
> 
> One option that could work here (but it's very hard to implement
> correctly and fully validate) is an atomic_check failure hint: Instead
> of just -EINVAL you get back a property + the list of possible values
> userspace should try for that property instead of the one it used.
> This might need a few iterations until it convergences, but we could
> use that to convey dynamic limits on scaling, pixel formats,
> modifiers, rotations and so on. E.g. if you've run out of YUV
> converters, you could supply a new pixel format list with only rgb
> formats. Or if you've run out of scalers update the can_scale
> property.

It looks like a pretty good idea yeah.

> But that makes atomic really complex, it would still be only a hint,
> and it definitely makes drivers a nightmare to validate and fully
> test.

I guess if we go that road, we'll want to have it tested through VKMS
and IGT.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com