From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brian Starkey Subject: Re: [RFC PATCH 00/11] Introduce writeback connectors Date: Wed, 12 Oct 2016 08:30:37 +0100 Message-ID: <20161012072643.GA17390@localhost> References: <1476197648-24918-1-git-send-email-brian.starkey@arm.com> <87d1j6emmd.fsf@eliezer.anholt.net> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: Received: from foss.arm.com (foss.arm.com [217.140.101.70]) by gabe.freedesktop.org (Postfix) with ESMTP id 31A4C6E7CE for ; Wed, 12 Oct 2016 07:31:34 +0000 (UTC) Content-Disposition: inline In-Reply-To: <87d1j6emmd.fsf@eliezer.anholt.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Eric Anholt Cc: liviu.dudau@arm.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, hverkuil@xs4all.nl, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org SGkgRXJpYywKCk9uIFR1ZSwgT2N0IDExLCAyMDE2IGF0IDEyOjAxOjE0UE0gLTA3MDAsIEVyaWMg QW5ob2x0IHdyb3RlOgo+QnJpYW4gU3RhcmtleSA8YnJpYW4uc3RhcmtleUBhcm0uY29tPiB3cml0 ZXM6Cj4KPj4gSGksCj4+Cj4+IFRoaXMgUkZDIHNlcmllcyBpbnRyb2R1Y2VzIGEgbmV3IGNvbm5l Y3RvciB0eXBlOgo+PiAgRFJNX01PREVfQ09OTkVDVE9SX1dSSVRFQkFDSwo+PiBJdCBpcyBhIGZv bGxvdy1vbiBmcm9tIGEgcHJldmlvdXMgZGlzY3Vzc2lvbjogWzFdCj4+Cj4+IFdyaXRlYmFjayBj b25uZWN0b3JzIGFyZSB1c2VkIHRvIGV4cG9zZSB0aGUgbWVtb3J5IHdyaXRlYmFjayBlbmdpbmVz Cj4+IGZvdW5kIGluIHNvbWUgZGlzcGxheSBjb250cm9sbGVycywgd2hpY2ggY2FuIHdyaXRlIGEg Q1JUQydzCj4+IGNvbXBvc2l0aW9uIHJlc3VsdCB0byBhIG1lbW9yeSBidWZmZXIuCj4+IFRoaXMg aXMgdXNlZnVsIGUuZy4gZm9yIHRlc3RpbmcsIHNjcmVlbi1yZWNvcmRpbmcsIHNjcmVlbnNob3Rz LAo+PiB3aXJlbGVzcyBkaXNwbGF5LCBkaXNwbGF5IGNsb25pbmcsIG1lbW9yeS10by1tZW1vcnkg Y29tcG9zaXRpb24uCj4+Cj4+IFBhdGNoZXMgMS03IGluY2x1ZGUgdGhlIGNvcmUgZnJhbWV3b3Jr IGNoYW5nZXMgcmVxdWlyZWQsIGFuZCBwYXRjaGVzCj4+IDgtMTEgaW1wbGVtZW50IGEgd3JpdGVi YWNrIGNvbm5lY3RvciBmb3IgdGhlIE1hbGktRFAgd3JpdGViYWNrIGVuZ2luZS4KPj4gVGhlIE1h bGktRFAgcGF0Y2hlcyBkZXBlbmQgb24gdGhpcyBvdGhlciBzZXJpZXM6IFsyXS4KPj4KPj4gVGhl IGNvbm5lY3RvciBpcyBnaXZlbiB0aGUgRkJfSUQgcHJvcGVydHkgZm9yIHRoZSBvdXRwdXQgZnJh bWVidWZmZXIsCj4+IGFuZCB0d28gbmV3IHJlYWQtb25seSBwcm9wZXJ0aWVzOiBQSVhFTF9GT1JN QVRTIGFuZAo+PiBQSVhFTF9GT1JNQVRTX1NJWkUsIHdoaWNoIGV4cG9zZSB0aGUgc3VwcG9ydGVk IGZyYW1lYnVmZmVyIHBpeGVsCj4+IGZvcm1hdHMgb2YgdGhlIGVuZ2luZS4KPj4KPj4gVGhlIEVE SUQgcHJvcGVydHkgaXMgbm90IGV4cG9zZWQgZm9yIHdyaXRlYmFjayBjb25uZWN0b3JzLgo+Pgo+ PiBXcml0ZWJhY2sgY29ubmVjdG9yIHVzYWdlOgo+PiAtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LQo+PiBEdWUgdG8gY29ubmVjdG9yIHJvdXRpbmcgY2hhbmdlcyBiZWluZyB0cmVhdGVkIGFzICJm dWxsIG1vZGVzZXQiCj4+IG9wZXJhdGlvbnMsIGFueSBjbGllbnQgd2hpY2ggd2lzaGVzIHRvIHVz ZSBhIHdyaXRlYmFjayBjb25uZWN0b3IKPj4gc2hvdWxkIGluY2x1ZGUgdGhlIGNvbm5lY3RvciBp biBldmVyeSBtb2Rlc2V0LiBUaGUgd3JpdGViYWNrIHdpbGwgbm90Cj4+IGFjdHVhbGx5IGJlY29t ZSBhY3RpdmUgdW50aWwgYSBmcmFtZWJ1ZmZlciBpcyBhdHRhY2hlZC4KPj4KPj4gVGhlIHdyaXRl YmFjayBpdHNlbGYgaXMgZW5hYmxlZCBieSBhdHRhY2hpbmcgYSBmcmFtZWJ1ZmZlciB0byB0aGUK Pj4gRkJfSUQgcHJvcGVydHkgb2YgdGhlIGNvbm5lY3Rvci4gVGhlIGRyaXZlciBtdXN0IHRoZW4g ZW5zdXJlIHRoYXQgdGhlCj4+IENSVEMgY29udGVudCBvZiB0aGF0IGF0b21pYyBjb21taXQgaXMg d3JpdHRlbiBpbnRvIHRoZSBmcmFtZWJ1ZmZlci4KPj4KPj4gVGhlIHdyaXRlYmFjayB3b3JrcyBp biBhIG9uZS1zaG90IG1vZGUgd2l0aCBlYWNoIGF0b21pYyBjb21taXQuIFRoaXMKPj4gcHJldmVu dHMgdGhlIHNhbWUgY29udGVudCBmcm9tIGJlaW5nIHdyaXR0ZW4gbXVsdGlwbGUgdGltZXMuCj4+ IEluIHNvbWUgY2FzZXMgKGZyb250LWJ1ZmZlciByZW5kZXJpbmcpIHRoZXJlIG1pZ2h0IGJlIGEg ZGVzaXJlIGZvcgo+PiBjb250aW51b3VzIG9wZXJhdGlvbiAtIEkgdGhpbmsgYSBwcm9wZXJ0eSBj b3VsZCBiZSBhZGRlZCBsYXRlciBmb3IKPj4gdGhpcyBraW5kIG9mIGNvbnRyb2wuCj4+Cj4+IFdy aXRlYmFjayBjYW4gYmUgZGlzYWJsZWQgYnkgc2V0dGluZyBGQl9JRCB0byB6ZXJvLgo+Cj5JIHRo aW5rIHRoaXMgc291bmRzIGdyZWF0LCBhbmQgdGhlIGludGVyZmFjZSBpcyBqdXN0IHJpZ2h0IElN Ty4KPgoKVGhhbmtzLCBnbGFkIHlvdSBsaWtlIGl0ISBIb3BlZnVsbHkgeW91J3JlIGVxdWFsbHkg YWdyZWVhYmxlIHdpdGggdGhlCmNoYW5nZXMgRGFuaWVsIGhhcyBiZWVuIHN1Z2dlc3RpbmcuCgo+ SSBkb24ndCByZWFsbHkgc2VlIGEgdXNlIGZvciBjb250aW51b3VzIG1vZGUgLS0gYSBzZXF1ZW5j ZSBvZiBvbmUtc2hvdHMKPm1ha2VzIGEgbG90IG1vcmUgc2Vuc2UgYmVjYXVzZSB0aGVuIHlvdSBj YW4ga25vdyB3aGF0IGRhdGEgaGFzIGNoYW5nZWQsCj53aGljaCBhbnlvbmUgdHJ5aW5nIHRvIHVz ZSB0aGUgd3JpdGViYWNrIGJ1ZmZlciB3b3VsZCBuZWVkIHRvIGtub3cuCj4KCkFncmVlZCAtIHdl J3ZlIG5ldmVyIGZvdW5kIGEgdXNlIGZvciBpdC4KCj4+IEtub3duIGlzc3VlczoKPj4gLS0tLS0t LS0tLS0tLQo+PiAgKiBJJ20gbm90IHN1cmUgd2hhdCAiRFBNUyIgc2hvdWxkIG1lYW4gZm9yIHdy aXRlYmFjayBjb25uZWN0b3JzLgo+PiAgICBJdCBjb3VsZCBiZSB1c2VkIHRvIGRpc2FibGUgd3Jp dGViYWNrIChldmVuIHdoZW4gYSBmcmFtZWJ1ZmZlciBpcwo+PiAgICBhdHRhY2hlZCksIG9yIGl0 IGNvdWxkIGJlIGhpZGRlbiBlbnRpcmVseSAod2hpY2ggd291bGQgYnJlYWsgdGhlCj4+ICAgIGxl Z2FjeSBEUE1TIGNhbGwgZm9yIHdyaXRlYmFjayBjb25uZWN0b3JzKS4KPj4gICogV2l0aCBEYW5p ZWwncyByZWNlbnQgcmUtaXRlcmF0aW9uIG9mIHRoZSB1c2Vyc3BhY2UgQVBJIHJ1bGVzLCBJCj4+ ICAgIGZ1bGx5IGV4cGVjdCB0byBwcm92aWRlIHNvbWUgdXNlcnNwYWNlIGNvZGUgdG8gc3VwcG9y dCB0aGlzLiBUaGUKPj4gICAgcXVlc3Rpb24gaXMgd2hhdCwgYW5kIHdoZXJlPyBXZSB3YW50IHRv IHVzZSB3cml0ZWJhY2sgZm9yIHRlc3RpbmcsCj4+ICAgIHNvIHBlcmhhcHMgc29tZSB0ZXN0cyBp biBpZ3QgaXMgc3VpdGFibGUuCj4+ICAqIERvY3VtZW50YXRpb24uIFByb2JhYmx5IHNvbWUgcG9y dGlvbiBvZiB0aGlzIGNvdmVyIGxldHRlciBuZWVkcyB0bwo+PiAgICBtYWtlIGl0IGludG8gRG9j dW1lbnRhdGlvbi8KPj4gICogU3luY2hyb25pc2F0aW9uLiBPdXIgaGFyZHdhcmUgd2lsbCBmaW5p c2ggdGhlIHdyaXRlYmFjayBieSB0aGUgbmV4dAo+PiAgICB2c3luYy4gSSd2ZSBub3QgaW1wbGVt ZW50ZWQgZmVuY2Ugc3VwcG9ydCBoZXJlLCBidXQgaXQgd291bGQgYmUgYW4KPj4gICAgb2J2aW91 cyBhZGRpdGlvbi4KPgo+TXkgaGFyZHdhcmUgd29uJ3QgbmVjZXNzYXJpbHkgZmluaXNoIGJ5IHRo ZSBuZXh0IHZzeW5jIC0tIGl0IHRyaWNrbGVzCj5vdXQgYXQgd2hhdGV2ZXIgcmF0ZSBpdCBjYW4g ZmluZCBtZW1vcnkgYmFuZHdpZHRoIHRvIGdldCB0aGUgam9iIGRvbmUsCj5hbmQgZmlyZXMgYW4g aW50ZXJydXB0IHdoZW4gaXQncyBmaW5pc2hlZC4KPgoKSXMgaXQgYm91bmRlZD8gWW91IHByZXN1 bWFibHkgaGF2ZSB0byBmaW5pc2ggdGhlIHdyaXRlLW91dCBiZWZvcmUgeW91CmNhbiBjaGFuZ2Ug YW55IGlucHV0IGJ1ZmZlcnM/Cgo+U28gSSB3b3VsZCBsaWtlIHNvbWUgZGVmaW5pdGlvbiBmb3Ig aG93IHN5bmNpbmcgd29ya3MuICBPbmUgYW5zd2VyIHdvdWxkCj5iZSB0aGF0IHRoZXNlIGZsaXBz IGRvbid0IHRyaWdnZXIgdGhlaXIgcGFnZWZsaXAgZXZlbnRzIHVudGlsIHRoZQo+d3JpdGViYWNr IGlzIGRvbmUgKHNvIEkgbmVlZCB0byBjb2xsZWN0IGJvdGggdGhlIHZzeW5jIGlycSBhbmQgdGhl Cj53cml0ZWJhY2sgaXJxIGJlZm9yZSBzZW5kaW5nKS4gIEFub3RoZXIgd291bGQgYmUgdGhhdCBt YW5hZ2UgYW4KPmluZGVwZW5kZW50IGZlbmNlIGZvciB0aGUgd3JpdGViYWNrIGZiLCBzbyB0aGF0 IHlvdSBzdGlsbCBpbW1lZGlhdGVseQo+a25vdyB3aGVuIGZyYW1lYnVmZmVycyBmcm9tIHRoZSBw cmV2aW91cyBzY2Fub3V0LW9ubHkgZnJhbWUgYXJlIGlkbGUuCj4KCkkgbXVjaCBwcmVmZXIgdGhl IHNvdW5kIG9mIHRoZSBleHBsaWNpdCBmZW5jZSBhcHByb2FjaC4KCkhvcGVmdWxseSB3ZSBjYW4g YWdyZWUgdGhhdCBhIG5ldyBhdG9taWMgY29tbWl0IGNhbid0IGJlIGNvbXBsZXRlZAp3aGlsc3Qg dGhlcmUncyBhIHdyaXRlYmFjayBvbmdvaW5nLCBvdGhlcndpc2UgbWFuYWdpbmcgdGhlIGZlbmNl IGFuZApmcmFtZWJ1ZmZlciBsaWZldGltZSBzb3VuZHMgcmVhbGx5IHRyaWNreSAtIHRoZXknZCBu ZWVkIHRvIGJlIGRlY291cGxlZApmcm9tIHRoZSBhdG9taWNfc3RhdGUgYW5kIG91dGxpdmUgdGhl IGNvbW1pdCB0aGF0IHNwYXduZWQgdGhlbS4KCkNoZWVycywKLUJyaWFuCgo+QWxzbywgdGVzdHMg Zm9yIHRoaXMgaW4gaWd0LCBwbGVhc2UuICBXcml0ZWJhY2sgaW4gaWd0IHdpbGwgZ2l2ZSB1cyBz bwo+bXVjaCBtb3JlIGFiaWxpdHkgdG8gY292ZXIgS01TIGZ1bmN0aW9uYWxpdHkgb24gbm9uLUlu dGVsIGhhcmR3YXJlLgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5v cmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2 ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from foss.arm.com ([217.140.101.70]:54054 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754073AbcJLHlk (ORCPT ); Wed, 12 Oct 2016 03:41:40 -0400 Date: Wed, 12 Oct 2016 08:30:37 +0100 From: Brian Starkey To: Eric Anholt Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, liviu.dudau@arm.com, robdclark@gmail.com, hverkuil@xs4all.nl, ville.syrjala@linux.intel.com, daniel@ffwll.ch Subject: Re: [RFC PATCH 00/11] Introduce writeback connectors Message-ID: <20161012072643.GA17390@localhost> References: <1476197648-24918-1-git-send-email-brian.starkey@arm.com> <87d1j6emmd.fsf@eliezer.anholt.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <87d1j6emmd.fsf@eliezer.anholt.net> Sender: linux-media-owner@vger.kernel.org List-ID: Hi Eric, On Tue, Oct 11, 2016 at 12:01:14PM -0700, Eric Anholt wrote: >Brian Starkey writes: > >> Hi, >> >> This RFC series introduces a new connector type: >> DRM_MODE_CONNECTOR_WRITEBACK >> It is a follow-on from a previous discussion: [1] >> >> Writeback connectors are used to expose the memory writeback engines >> found in some display controllers, which can write a CRTC's >> composition result to a memory buffer. >> This is useful e.g. for testing, screen-recording, screenshots, >> wireless display, display cloning, memory-to-memory composition. >> >> Patches 1-7 include the core framework changes required, and patches >> 8-11 implement a writeback connector for the Mali-DP writeback engine. >> The Mali-DP patches depend on this other series: [2]. >> >> The connector is given the FB_ID property for the output framebuffer, >> and two new read-only properties: PIXEL_FORMATS and >> PIXEL_FORMATS_SIZE, which expose the supported framebuffer pixel >> formats of the engine. >> >> The EDID property is not exposed for writeback connectors. >> >> Writeback connector usage: >> -------------------------- >> Due to connector routing changes being treated as "full modeset" >> operations, any client which wishes to use a writeback connector >> should include the connector in every modeset. The writeback will not >> actually become active until a framebuffer is attached. >> >> The writeback itself is enabled by attaching a framebuffer to the >> FB_ID property of the connector. The driver must then ensure that the >> CRTC content of that atomic commit is written into the framebuffer. >> >> The writeback works in a one-shot mode with each atomic commit. This >> prevents the same content from being written multiple times. >> In some cases (front-buffer rendering) there might be a desire for >> continuous operation - I think a property could be added later for >> this kind of control. >> >> Writeback can be disabled by setting FB_ID to zero. > >I think this sounds great, and the interface is just right IMO. > Thanks, glad you like it! Hopefully you're equally agreeable with the changes Daniel has been suggesting. >I don't really see a use for continuous mode -- a sequence of one-shots >makes a lot more sense because then you can know what data has changed, >which anyone trying to use the writeback buffer would need to know. > Agreed - we've never found a use for it. >> Known issues: >> ------------- >> * I'm not sure what "DPMS" should mean for writeback connectors. >> It could be used to disable writeback (even when a framebuffer is >> attached), or it could be hidden entirely (which would break the >> legacy DPMS call for writeback connectors). >> * With Daniel's recent re-iteration of the userspace API rules, I >> fully expect to provide some userspace code to support this. The >> question is what, and where? We want to use writeback for testing, >> so perhaps some tests in igt is suitable. >> * Documentation. Probably some portion of this cover letter needs to >> make it into Documentation/ >> * Synchronisation. Our hardware will finish the writeback by the next >> vsync. I've not implemented fence support here, but it would be an >> obvious addition. > >My hardware won't necessarily finish by the next vsync -- it trickles >out at whatever rate it can find memory bandwidth to get the job done, >and fires an interrupt when it's finished. > Is it bounded? You presumably have to finish the write-out before you can change any input buffers? >So I would like some definition for how syncing works. One answer would >be that these flips don't trigger their pageflip events until the >writeback is done (so I need to collect both the vsync irq and the >writeback irq before sending). Another would be that manage an >independent fence for the writeback fb, so that you still immediately >know when framebuffers from the previous scanout-only frame are idle. > I much prefer the sound of the explicit fence approach. Hopefully we can agree that a new atomic commit can't be completed whilst there's a writeback ongoing, otherwise managing the fence and framebuffer lifetime sounds really tricky - they'd need to be decoupled from the atomic_state and outlive the commit that spawned them. Cheers, -Brian >Also, tests for this in igt, please. Writeback in igt will give us so >much more ability to cover KMS functionality on non-Intel hardware.