<div dir="ltr">Hi John,<br><br><div class="gmail_quote"><div dir="ltr">On Tue, Apr 24, 2018 at 11:38 AM John Stultz <<a href="mailto:john.stultz@linaro.org">john.stultz@linaro.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Tue, Apr 24, 2018 at 3:34 AM, Stefan Schake <<a href="mailto:stschake@gmail.com" target="_blank">stschake@gmail.com</a>> wrote:<br>
> On Tue, Apr 24, 2018 at 10:09 AM, Alexandru-Cosmin Gheorghe<br>
> <<a href="mailto:Alexandru-Cosmin.Gheorghe@arm.com" target="_blank">Alexandru-Cosmin.Gheorghe@arm.com</a>> wrote:<br>
>> On Mon, Apr 23, 2018 at 05:06:44PM -0700, John Stultz wrote:<br>
>>> @@ -695,6 +704,15 @@ HWC2::Error DrmHwcTwo::HwcDisplay::ValidateDisplay(uint32_t *num_types,<br>
>>>          layer.set_validated_type(HWC2::Composition::Client);<br>
>>>          ++*num_types;<br>
>>>          break;<br>
>>> +      case HWC2::Composition::Device:<br>
>>> +        if (!compositor_.uses_GL() && !avail_planes) {<br>
>><br>
>> Something is not entirely correct here, you can't assume just because<br>
>> you have planes available they can be used.<br>
>> E.g Layer has rotation, but the plane doesn't support it.<br>
>> This part is handled by the planning part in presentDisplay which<br>
>> falls back on GPU.<br>
<br>
Ah. Thanks for the correction here!<br>
<br>
This is part of the disconnect I have been having with the<br>
drm_hwcomposer logic. The plan/validate step doesn't really do either,<br>
instead it defers all the checking/planning to the set/present step.<br>
But unfortunately it seems if we want to use the surfaceflinger gl<br>
composer, we need to mark the layers as client rendered in the<br>
validate function.<br>
<br>
>> I think the easiest and safe way is to just fallback to<br>
>> Composition::Client whenever the GLCompositor failed to initialize.<br>
>><br>
><br>
> I agree, this would only work out for the single plane case where you end<br>
> up using client composition for everything.<br>
<br>
True, but maybe as a first step we simply fall back on the<br>
glcompositor, just so we have something displaying in that case (right<br>
now nothing gets displayed).<br>
<br>
> In the spirit of DRM what we would probably want is to atomic test a<br>
> composition, and if that fails we can still fallback to client or GL<br>
> compositor depending on its availability. And then in a far far away<br>
> future we might even do a few iterations simplifying our composition until<br>
> it atomic tests correctly so we can still offload some "simple" parts<br>
> like the cursor.<br>
<br>
This sounds reasonable. I suspect it will take some heavier rework of<br>
the drm_hwcomposer to move such logic to the validate step.<br>
<br>
Marissa/Sean: Any objections here? Doing more planning in the validate<br>
step sounds like it aligns closer to the HWC2 interface goals, but I<br>
don't want to have to relearn-the-hard-way any lessons that resulted<br>
in the current mode of accept it all and sort it in present.<br></blockquote><div><br></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">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.</span></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">I think validating compositions w/ atomic in kernel is a valid way to do it.</span></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div><div><span style="color:rgb(34,34,34);font-family:sans-serif;font-size:13px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">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.</span></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
In the meantime I'll drop the flawed plane/layer allocation from this<br>
patch and resend.<br>
<br>
thanks<br>
-john<br>
</blockquote></div></div>