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

Wed Mar 21 15:13:02 UTC 2018

Hi Sean & Rob,

On Wed, Mar 21, 2018 at 10:01:07AM -0400, Sean Paul wrote:
> On Tue, Mar 20, 2018 at 11:35:40PM -0400, Rob Clark wrote:
> > On Tue, Mar 20, 2018 at 11:16 AM, Alexandru-Cosmin Gheorghe
> > <Alexandru-Cosmin.Gheorghe at arm.com> wrote:
> > > 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.
> > >
> > 
> > fwiw, the msm/mdp5 writeback support I implemented was using a
> > different CRTC (ie. output going either to display or to wb).. (which
> > unfortunately implies using different planes).. not sure how much this
> > case is worth supporting in drm-hwc.

Which case? With the same CRTC or different CRTCs, I think both could
prove useful.

> 
> Yeah, I wonder whether this is the method of operation we should focus on. It's
> unclear whether the restrictions Alex describes above are HW or SW limited, my
> impression was SW. Of course, this means we'll need to disable WB if we are
> using all crtcs (ie: external display connected), but that seems like a rare
> usecase of Android, and one might argue that power savings are not as important
> in this case.
> 
> Further, if we enable compositing with a dedicated WB pipe, we can also use it
> for virtual displays.
> 
> Sean

For Mali DP-650 this is a HW requirement, we don't have any way of
individually controlling just the writeback functionality without
affecting what's sent to the display.

> 
> > 
> > (I think I can do the single CRTC and multiple encoder cases, but it
> > wasn't really obvious how to get that working with a video mode
> > display and the hw I was working on didn't have a DSI cmd mode panel)
> > 
> > BR,
> > -R
> > 
> > > 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
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS

-- 
Cheers,
Alex G