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