[PATCH hwc v1] [RFC] drm_hwcomposer: Flatten composition using writeback connector

Tue Mar 20 15:16:08 UTC 2018

On Tue, Mar 20, 2018 at 10:00:17AM -0400, Sean Paul wrote:
> On Tue, Mar 13, 2018 at 04:21:20PM +0000, Alexandru Gheorghe wrote:
> > This patchset tries to add support for using writeback connector to
> > flatten a scene when it doesn't change for a while. This idea had
> > been floated around on IRC here [1] and here [2].
> > 
> > Developed on top of the latest writeback series, sent by Liviu here
> > [3].
> > 
> > Probably the patch should/could be broken in more patches, but since I
> > want to put this out there to get feedback, I kept them as a single
> > patch for now.
> > 
> > This change could be summarize as follow:
> > - Attach a writeback connector to the CRTC that's controlling a
> > display.
> > - Detect the scene did not change for a while(60 vblanks).
> > - Re-commit scene and get the composited scene through the writeback
> > connector.
> > - Commit the whole scene as a single plane.
> > 
> > Some areas that I consider important and I could use some
> > feedback/ideas:
> > 
> > 1. Building the pipeline.
> > Currently, drm_hwcomposer allows to connect just a single connector
> > to a crtc. For now, I decided to treat the writeback connector as a
> > separate field inside DrmCrtc. I'm not sure if it's a good idea to try
> > to handle this in a unified way, since the writeback connector is such
> > a special connector. Regarding the allocation of writeback connectors,
> > my idea was to allocate writeback connector to the primary display
> > first and then continue allocating while respecting the display number. 0
> > gets a writeback before 1 and so on.
> > 
> > 2. Heuristic for triggering the flattening.
> > I just created a VSyncWorker which will trigger the flattening of the
> > scene if it doesn't change for 60 consecutive vsyncs. The countdown
> > gets reset every time ValidateDisplay is called. This is a relatively
> > basic heuristic, so I'm open to suggestions.
> > 
> > 3. Locking scheme corner cases.
> > The Vsynworker is a separate thread which will contend with
> > SurfaceFlinger for showing things on the screen. I tried to limit the
> > race window by resetting the countdown on ValidateDisplay and
> > explicitely checking that we still need to use the flatten scene before
> > commiting to get the writeback result or before applying the flattened
> > scene.
> 
> What are the consequences of the race? It seems like it might be possible that
> stale content will be shown over fresh? 
> I think it'd be fine to serialize
> vsyncworker and "normal" commits such that the race window is closed instead of
> reduced. I think you could do the writeback operation asynchronously and then
> commit the result once it's ready, that shouldn't block things up too much.
> 

Just, to make sure we are on the same page, for Mali DP650 the pipeline
looks like this, I didn't see how it looks for the other hardware.

CRTC ---- encoder ------------ display connector
 |------- writeback enc ------ writeback connector.

There is no asynchronously writeback operation, the scene that you
do writeback for will be shown on the display as well.

Flattening thread:                             
1) Commit original scene + writeback buffer
2) Wait for writeback fence.
3) Commit flattened scene.

Before 1) and 3) I'm doing a locked check where I verify if flattened
scene is still needed and then I release the lock and commit.

Now, I can see a crazy race where immediately after I decided that we
need the flattened scene the flattening thread doesn't get any CPU and
the SurfaceFlinger comes ahead and commits it's new scene followed by
a commit of the old scene.
That's the limitted race window I'm talking about.

The alternative would be to serialize the commits 1) & 3) with
SurfaceFlinger commits, but while 1) or 3) happens surfaceflinger
cannot commit, therefore could potentially miss a VBlank. I suppose
this option is more desireable than the side effect of previously
explained race.

> > 
> > 4. Building the DrmDisplayComposition for the flattened scene.
> > I kind of lost myself  in all types of layers/planes and compositions,
> > so I'm not sure if I'm correctly building the DrmDisplayComposition
> > object for the FlattenScene, it works and shows what I expect on the
> > screen. So, any feedback here is appreciated.
> 
> Yeah, this code needs some love. I had envisioned some Planner-esque interface
> where platforms could plug their 2D blocks in and they'd be used by core to
> flatten things. This scheme would at least separate the squashing complexity
> from compositing. Any interest in taking this on?
> 

I could imagine how that would work with a totally independent 2D
block, not sure if it's doable in my current setup with the writeback
linked to same CRTC as the display, don't you think?

> 
> > 
> > 5. I see there is a drmcompositorworker.cpp which implemented the same
> > idea using the GPU, however that seems to not be used anymore, does
> > anyone know the rationale behind it?
> 
> Let's delete it if it's not used.
> 
> Sean
> 
> <snip />
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS

-- 
Cheers,
Alex G