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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
