Re: [PATCH RFC v5 02/10] drm: Introduce solid fill DRM plane property

[Pekka Paalanen <ppaalanen@gmail.com>]

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

On Mon, 21 Aug 2023 17:30:21 +0300
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:

> On Fri, 18 Aug 2023 at 16:55, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> >
> > On Fri, 18 Aug 2023 14:03:14 +0300
> > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:
> >  
> > > On 18/08/2023 13:51, Pekka Paalanen wrote:  
> > > > On Fri, 4 Aug 2023 16:59:00 +0300
> > > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:
> > > >  
> > > >> On Fri, 4 Aug 2023 at 16:44, Sebastian Wick <sebastian.wick@redhat.com> wrote:  
> > > >>>
> > > >>> On Fri, Aug 4, 2023 at 3:27 PM Dmitry Baryshkov
> > > >>> <dmitry.baryshkov@linaro.org> wrote:  
> > > >>>>
> > > >>>> On Fri, 28 Jul 2023 at 20:03, Jessica Zhang <quic_jesszhan@quicinc.com> wrote:  
> > > >>>>>
> > > >>>>> Document and add support for solid_fill property to drm_plane. In
> > > >>>>> addition, add support for setting and getting the values for solid_fill.
> > > >>>>>
> > > >>>>> To enable solid fill planes, userspace must assign a property blob to
> > > >>>>> the "solid_fill" plane property containing the following information:
> > > >>>>>
> > > >>>>> struct drm_mode_solid_fill {
> > > >>>>>          u32 version;
> > > >>>>>          u32 r, g, b;
> > > >>>>> };
> > > >>>>>
> > > >>>>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> > > >>>>> ---
> > > >>>>>   drivers/gpu/drm/drm_atomic_state_helper.c |  9 +++++
> > > >>>>>   drivers/gpu/drm/drm_atomic_uapi.c         | 55 +++++++++++++++++++++++++++++++
> > > >>>>>   drivers/gpu/drm/drm_blend.c               | 30 +++++++++++++++++
> > > >>>>>   include/drm/drm_blend.h                   |  1 +
> > > >>>>>   include/drm/drm_plane.h                   | 35 ++++++++++++++++++++
> > > >>>>>   include/uapi/drm/drm_mode.h               | 24 ++++++++++++++
> > > >>>>>   6 files changed, 154 insertions(+)
> > > >>>>>  
> > > >>>>
> > > >>>> [skipped most of the patch]  
> >
> > ...
> >  
> > > >>> Maybe another COLOR_FILL enum value
> > > >>> with alpha might be better? Maybe just doing the alpha via the alpha
> > > >>> property is good enough.  
> > > >>
> > > >> One of our customers has a use case for setting the opaque solid fill,
> > > >> while keeping the plane's alpha intact.  
> > > >
> > > > Could you explain more about why they must keep plane alpha intact
> > > > instead of reprogramming everything with atomic? Is there some
> > > > combination that just cannot reach the same end result via userspace
> > > > manipulation of the solid fill values with plane alpha?
> > > >
> > > > Or is it a matter of userspace architecture where you have independent
> > > > components responsible for different KMS property values?  
> >  
> > > The latter one. The goal is to be able to switch between pixel sources
> > > without touching any additional properties (including plane's alpha value).  
> >
> > Sorry, but that does not seem like a good justification for KMS UAPI
> > design.
> >
> > It is even in conflict with how atomic KMS UAPI was designed to work:
> > collect all your changes into a single commit, and push it at once.
> > Here we are talking about separate components changing the different
> > properties of the same KMS plane even. If you want to change both plane
> > opacity and contents, does it mean you need two refresh cycles, one at
> > a time? Could the two components be even racing with each other,
> > stalling each other randomly?  
> 
> Most likely I was not verbose enough.
> 
> We want to setup the blending scene, including the FB and the solid
> fill properties for the plane. FB is set up in the RGBA format, each
> pixel having its own alpha value in addition to the plane's alpha
> value. Then under certain circumstances, the plane gets hidden by the
> solid fill (think of a curtain). We do not want to touch the global
> scene setup (including plane alpha value), just switch the curtain on
> and off.
> I think this plays good enough with the defined plane blending rules,
> where one can use pre-multiplied blending mode or use coverage
> blending mode.

Right, that's what I understood. But this does complicate the KMS UAPI
for something that is well possible and feasible without the added
complication as well.

Is there a hardware or driver reason to avoid touching the global scene
setup? Does something in the hardware or driver work more optimally
that way?

Personally I'd favour simpler UAPI with more complex userspace for
maintainability and testing reasons. I'd also favour UAPI that exposes
common hardware features instead of design driven by userspace
process-internal architecture. There does not seem to be any
functionality or performance reasons to justify adding alpha channel to
the solid fill color.

OTOH, do we know of hardware that does not have separate alpha for the
fill color?

Do we know of hardware that can only do opaque solid fills, meaning no
alpha in the fill color nor for the plane?

What about hardware that has no plane alpha, but does have fill color
alpha?

If the plane has an alpha property, then drivers could implement fill
color alpha by combining the two alpha values before programming the
hardware. If there is no plane alpha, but there is fill color alpha, it
would be really awkward to expose a fake plane alpha because it would
only work with fill color.

Assuming that all those combinations exist in hardware, then separate
fill colors without and with alpha make sense, advertised independently.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]