[RFC][PATCH 2/2] drm_hwcomposer: Fall back to client compositon if the gl precompostior fails

[Alistair Strachan]

Hi John,

On Tue, Apr 24, 2018 at 11:38 AM John Stultz <john.stultz at linaro.org> wrote:

> On Tue, Apr 24, 2018 at 3:34 AM, Stefan Schake <stschake at gmail.com> wrote:
> > On Tue, Apr 24, 2018 at 10:09 AM, Alexandru-Cosmin Gheorghe
> > <Alexandru-Cosmin.Gheorghe at arm.com> wrote:
> >> On Mon, Apr 23, 2018 at 05:06:44PM -0700, John Stultz wrote:
> >>> @@ -695,6 +704,15 @@ HWC2::Error
> DrmHwcTwo::HwcDisplay::ValidateDisplay(uint32_t *num_types,
> >>>          layer.set_validated_type(HWC2::Composition::Client);
> >>>          ++*num_types;
> >>>          break;
> >>> +      case HWC2::Composition::Device:
> >>> +        if (!compositor_.uses_GL() && !avail_planes) {
> >>
> >> Something is not entirely correct here, you can't assume just because
> >> you have planes available they can be used.
> >> E.g Layer has rotation, but the plane doesn't support it.
> >> This part is handled by the planning part in presentDisplay which
> >> falls back on GPU.
>
> Ah. Thanks for the correction here!
>
> This is part of the disconnect I have been having with the
> drm_hwcomposer logic. The plan/validate step doesn't really do either,
> instead it defers all the checking/planning to the set/present step.
> But unfortunately it seems if we want to use the surfaceflinger gl
> composer, we need to mark the layers as client rendered in the
> validate function.
>
> >> I think the easiest and safe way is to just fallback to
> >> Composition::Client whenever the GLCompositor failed to initialize.
> >>
> >
> > I agree, this would only work out for the single plane case where you end
> > up using client composition for everything.
>
> True, but maybe as a first step we simply fall back on the
> glcompositor, just so we have something displaying in that case (right
> now nothing gets displayed).
>
> > In the spirit of DRM what we would probably want is to atomic test a
> > composition, and if that fails we can still fallback to client or GL
> > compositor depending on its availability. And then in a far far away
> > future we might even do a few iterations simplifying our composition
> until
> > it atomic tests correctly so we can still offload some "simple" parts
> > like the cursor.
>
> This sounds reasonable. I suspect it will take some heavier rework of
> the drm_hwcomposer to move such logic to the validate step.
>
> Marissa/Sean: Any objections here? Doing more planning in the validate
> step sounds like it aligns closer to the HWC2 interface goals, but I
> don't want to have to relearn-the-hard-way any lessons that resulted
> in the current mode of accept it all and sort it in present.
>

I agree. This was the design goal behind HWC_GEOMETRY_CHANGED /
HWC2_PFN_GET_CHANGED_COMPOSITION_TYPES. The validate() function should know
that the composition can be set in HWC2_PFN_PRESENT_DISPLAY so there should
be no need for an internal GL fallback. This feedback to surfaceflinger
also allows for small state optimizations that can't be done in the
fallback.

I think validating compositions w/ atomic in kernel is a valid way to do it.

I would suggest that code is also refactored a little so that the
validation/planning could also be done in userspace, maybe by linking a
static library. This would allow drivers without a way to do it in kernel
to keep working.

In the meantime I'll drop the flawed plane/layer allocation from this
> patch and resend.
>
> thanks
> -john
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20180425/d81768f7/attachment.html>