Planes enumeration with sun4i-drm driver

Planes enumeration with sun4i-drm driver

[Paul Kocialkowski]

[-- Attachment #1.1: Type: text/plain, Size: 1927 bytes --]

Hi,

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.

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.

For sun8i and onwards, there is a clear separation between UI and video
planes in both the hardware and the code, so this problem doesn't occur
(only video planes are reported to support YUV).

What do you think?

Cheers,

Paul

-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Planes enumeration with sun4i-drm driver

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2402 bytes --]

Hi,

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.

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.

Maxime

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

Re: Planes enumeration with sun4i-drm driver

[Daniel Vetter]

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?

> 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 :-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: Planes enumeration with sun4i-drm driver

[Maxime Ripard]

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

Re: Planes enumeration with sun4i-drm driver

[Daniel Vetter]

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.

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.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: Planes enumeration with sun4i-drm driver

[Maxime Ripard]

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