Re: Planes enumeration with sun4i-drm driver

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

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.

Maxime

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