[RFC,5/5] drm_hwcomposer: HACK: Fix tearing on hikey/hikey960

When using drm_hwcomposer with the hikey/hikey960 boards, the
resulting display shows lots of tearing.

I'm not much of an expert in how this code should work, but it
seems that we never call sync_wait(), and thus don't seem to be
handling the fences properly? I'm not sure.

Anyway, in a daze, I started cutting out code trying to make
sure we call the CreateNextTimelineFence() and
fb.set_release_fence_fd().

After doing so the tearing went away. I'm really not sure
what is wrong that requires these hacks. It may be the
hikey/hikey960 drm driver is incorrectly reporting or
handling something?

We do only have a single plane and no hardware compositing on
the boards, so we are having to force the gpu to do all the
compositing.

Any ideas for what a proper fix here would be? Or even just
hints on why this might make things work?

Change-Id: Ifba58f6f1cb00e5271892c0241e4891abe211f22
Cc: Marissa Wall <marissaw@google.com>
Cc: Sean Paul <seanpaul@google.com>
Cc: Dmitry Shmidt <dimitrysh@google.com>
Cc: Robert Foss <robert.foss@collabora.com>
Cc: Matt Szczesiak <matt.szczesiak@arm.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: David Hanna <david.hanna11@gmail.com>
Cc: Rob Herring <rob.herring@linaro.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>

---
 drmdisplaycompositor.cpp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

-- 
2.7.4

On Tue, Jan 9, 2018 at 10:05 PM, John Stultz <john.stultz@linaro.org> wrote:
> When using drm_hwcomposer with the hikey/hikey960 boards, the
> resulting display shows lots of tearing.
>
> I'm not much of an expert in how this code should work, but it
> seems that we never call sync_wait(), and thus don't seem to be
> handling the fences properly? I'm not sure.
>
> Anyway, in a daze, I started cutting out code trying to make
> sure we call the CreateNextTimelineFence() and
> fb.set_release_fence_fd().
>
> After doing so the tearing went away. I'm really not sure
> what is wrong that requires these hacks. It may be the
> hikey/hikey960 drm driver is incorrectly reporting or
> handling something?
>
> We do only have a single plane and no hardware compositing on
> the boards, so we are having to force the gpu to do all the
> compositing.
>
> Any ideas for what a proper fix here would be? Or even just
> hints on why this might make things work?

So I've maybe started to understand things a bit (though probably not
much) more here. Again, my apologies for being very new to all this.

This patch actually isn't necessary on the HiKey960, as in the mix of
testing both boards, I didn't get it working until after the HiKey
board so I incorrectly assumed this would be needed.

This patch is really only useful to avoid the tearing I see with the
HiKey board.

The core issue on HiKey is that in
DrmDisplayCompositor::PrepareFrame(), when we initialize the
precompositor_ to a new GLWorkerCompositor, the eglCreateContext()
call fails because we're asking for version 3, which the HiKey's
utgard mali implementation doesn't support (thus why the
pre_compositor_->Composite() call always fails in ApplySquash() -
though still need to figure out why squash_regions are empty).

Asking for version 2 gets us along a bit further, but then I hit other
trouble with the eglMakeCurrent() call in BeginContext, as passing
EGL_NO_SURFACE, EGL_NO_SURFACE without EGL_NO_CONTEXT seems to trigger
an error in the mali GL implementation. However, using EGL_NO_CONTEXT
causes surfaceflinger to crash, so I just hacked it up to skip the
eglMakeCurrent call to keep things going.

The next issue that I run into is maybe the more problematic one: The
vertex and fragment shaders in the glworker.c are marked as "#version
300 es", which utgard can't handle (apparently it maxes out at
"#version 100", but can't seemingly handle other syntax like "in").

So the bigger question I guess I have is: Is the drm_hwcomposer as a
project targeting a certain minimal GLES version, or is it open to
supporting older/limited hardware like the utgard level mali?  I can
look into trying to figure out if what needs to be done can be done in
a simpler shader language, or alternatively we can add fallback code
to still allow for fence handling for tear-free functionality w/o the
GL compositor (which is basically what this hack patch in-effect
allows).

Are either of these preferred? Or none?

Thoughts?

thanks
-john

On Thu, Jan 11, 2018 at 11:20 PM, John Stultz <john.stultz@linaro.org> wrote:
> On Tue, Jan 9, 2018 at 10:05 PM, John Stultz <john.stultz@linaro.org> wrote:
>> When using drm_hwcomposer with the hikey/hikey960 boards, the
>> resulting display shows lots of tearing.
>>
>> I'm not much of an expert in how this code should work, but it
>> seems that we never call sync_wait(), and thus don't seem to be
>> handling the fences properly? I'm not sure.
>>
>> Anyway, in a daze, I started cutting out code trying to make
>> sure we call the CreateNextTimelineFence() and
>> fb.set_release_fence_fd().
>>
>> After doing so the tearing went away. I'm really not sure
>> what is wrong that requires these hacks. It may be the
>> hikey/hikey960 drm driver is incorrectly reporting or
>> handling something?
>>
>> We do only have a single plane and no hardware compositing on
>> the boards, so we are having to force the gpu to do all the
>> compositing.
>>
>> Any ideas for what a proper fix here would be? Or even just
>> hints on why this might make things work?
>
> So I've maybe started to understand things a bit (though probably not
> much) more here. Again, my apologies for being very new to all this.
>
> This patch actually isn't necessary on the HiKey960, as in the mix of
> testing both boards, I didn't get it working until after the HiKey
> board so I incorrectly assumed this would be needed.
>
> This patch is really only useful to avoid the tearing I see with the
> HiKey board.
>
> The core issue on HiKey is that in
> DrmDisplayCompositor::PrepareFrame(), when we initialize the
> precompositor_ to a new GLWorkerCompositor, the eglCreateContext()
> call fails because we're asking for version 3, which the HiKey's
> utgard mali implementation doesn't support (thus why the
> pre_compositor_->Composite() call always fails in ApplySquash() -
> though still need to figure out why squash_regions are empty).
>
> Asking for version 2 gets us along a bit further, but then I hit other
> trouble with the eglMakeCurrent() call in BeginContext, as passing
> EGL_NO_SURFACE, EGL_NO_SURFACE without EGL_NO_CONTEXT seems to trigger
> an error in the mali GL implementation. However, using EGL_NO_CONTEXT
> causes surfaceflinger to crash, so I just hacked it up to skip the
> eglMakeCurrent call to keep things going.
>
> The next issue that I run into is maybe the more problematic one: The
> vertex and fragment shaders in the glworker.c are marked as "#version
> 300 es", which utgard can't handle (apparently it maxes out at
> "#version 100", but can't seemingly handle other syntax like "in").
>
> So the bigger question I guess I have is: Is the drm_hwcomposer as a
> project targeting a certain minimal GLES version, or is it open to
> supporting older/limited hardware like the utgard level mali?  I can
> look into trying to figure out if what needs to be done can be done in
> a simpler shader language, or alternatively we can add fallback code
> to still allow for fence handling for tear-free functionality w/o the
> GL compositor (which is basically what this hack patch in-effect
> allows).

It requires ES 3.0 'cause no one has implemented anything else...
Doesn't passing CTS since N at least require ES 3.0?

I think having a fallback path that requires no GL is needed. The
fence handling there should be simple pass thru to/from the kernel
display driver.

Rob