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