From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 2/3] drm:msm: Initial Add Writeback Support Date: Wed, 8 Apr 2015 10:07:07 +0200 Message-ID: <20150408080707.GN6354@phenom.ffwll.local> References: <1427922770-13721-1-git-send-email-jilaiw@codeaurora.org> <20150407055949.GI6354@phenom.ffwll.local> <93d3802305fa348738dae7f458c3fb56.squirrel@www.codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <93d3802305fa348738dae7f458c3fb56.squirrel@www.codeaurora.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: jilaiw@codeaurora.org Cc: linux-arm-msm , Linux Kernel Mailing List , "dri-devel@lists.freedesktop.org" List-Id: linux-arm-msm@vger.kernel.org T24gVHVlLCBBcHIgMDcsIDIwMTUgYXQgMDM6NTU6NDVQTSAtMDAwMCwgamlsYWl3QGNvZGVhdXJv cmEub3JnIHdyb3RlOgo+ID4gT24gVGh1LCBBcHIgMDIsIDIwMTUgYXQgMTA6Mjk6NTJBTSAtMDQw MCwgUm9iIENsYXJrIHdyb3RlOgo+ID4+IFNvLCBmcm9tIGEgcXVpY2sgbG9vaywgaXQgc2VlbXMg bGlrZSB0aGVyZSBpcyBhIGxvdCBvZiBwb3RlbnRpYWwgdG8KPiA+PiBzcGxpdCB0aGUgdjRsIHBh cnQgb3V0IGludG8gc29tZSBkcm0gaGVscGVycy4uIGl0IGxvb2tzIHByZXR0eQo+ID4+IGdlbmVy aWMoaXNoKSwgb3IgYXQgbGVhc3QgaXQgY291bGQgYmUgd2l0aCBzb21lIHN0cmF0ZWdpY2FsbHkg cGxhY2VkCj4gPj4gdmZ1bmNzIGluIGRybV92NGwyX2hlbHBlcl9mdW5jcy4KPiA+Pgo+ID4+IEkg ZG8gdGhpbmsgd2UgbmVlZCB0byBmaWd1cmUgb3V0IHRoZSBhdXRoL3NlY3VyaXR5IHNpdHVhdGlv bi4gIFdlCj4gPj4gcHJvYmFibHkgZG9uJ3Qgd2FudCB0byBsZXQgYXJiaXRyYXJ5IHByb2Nlc3Nl cyBvcGVuIGEgdjRsIGRldmljZSBhbmQKPiA+PiBzbm9vcCBvbiB0aGUgc2NyZWVuIGNvbnRlbnRz LiAgV2UgcGVyaGFwcyBjb3VsZCByZS11c2UgdGhlIGRyaTIgZHJtCj4gPj4gYXV0aCBzdHVmZiAo djRsMl9kcm1fZ2V0X21hZ2ljIGlvY3RsPykuICBPciwgd2VsbCwgaXQgd291bGQgYmUgbmljZSBp Zgo+ID4+IHRoZSB3YiBkZXZpY2UgY291bGQgYmUgbWFkZSB0byBub3QgZXhpc3QgaW4gL2RldiBh dCBhbGwsIGFuZAo+ID4+IHByZS1vcGVuJ2QgZmQgcmV0dXJuZWQgZnJvbSBhbiBpb2N0bCBvbiB0 aGUgZHJtIGRldmljZSwgYnV0IG5vdCByZWFsbHkKPiA+PiBzdXJlIGlmIHRoYXQgaXMgcG9zc2li bGUgKG9yIHRvbyB3ZWlyZCkuICBPbmNlIHRoZSBjb21wb3NpdG9yIHByb2Nlc3MKPiA+PiBoYXMg dGhlIHY0bCBkZXZpY2Ugb3BlbiBhbmQgYXV0aGVudGljYXRlZCBzb21laG93LCBJIGV4cGVjdCBp dCB3b3VsZAo+ID4+IHVzZSBmZCBwYXNzaW5nIHRvIHBhc3MgdGhlIGZkIG9mZiB0byBhIHRydXN0 ZWQgaGVscGVyIHByb2Nlc3MuCj4gPgo+ID4gUGxlYXNlIGRvbid0IHJlc3VycmVjdCB0aGUgbWFn aWMgc3R1ZmYgOy0pCj4gPgo+ID4gQW55d2F5IEkgZGlzY3Vzc2VkIHRoaXMgYSBiaXQgd2l0aCBM YXVyZW50IGFuZCB3ZSBmaWd1cmVkIHRoZSBiZXN0IHdheSB0bwo+ID4gd2lyZSB1cCB3cml0ZWJh Y2sgc3VwcG9ydCBpcyBieSB1c2luZyBkcm0gZnJhbWVidWZmZXJzLiBUaGVuIHlvdSBjYW4gdXNl Cj4gPiBhdG9taWMgZmxpcHMgdG8gY3JlYXRlIGEgbmV3IHNuYXBzaG90LiBPZiBjb3Vyc2UgdGhh dCB3b24ndCB3b3JrIHdpdGggaHcKPiA+IHdoZXJlIHdyaXRlYmFjayBpcyBjb250aW51b3VzLCB0 aGVyZSB2NGwgaXMgYSBtdWNoIGJldHRlciBmaXQuIEFuZCB3ZSBhbHNvCj4gPiBoYXZlIGhhcmR3 YXJlIHdoZXJlIHNvbWUgdjRsIHBpcGVsaW5lIGNvdWxkIGRpcmVjdGx5IGZlZWQgaW50byBhIGRy bQo+ID4gb3V0cHV0IHBpcGVsaW5lLCBzbyB3ZSBuZWVkIGEgZ2VuZXJpYyB3YXkgdG8gY29ubmVj dCB2NGwgYW5kIGRybSBhbnl3YXkuCj4gPiBGb3IgdGhhdCBJIHRoaW5rIHdlIHNob3VsZCBhZGQg YSBuZXcgZmxhZyB0byBhZGRmYjIgKG9yIGEgbmV3IGFkZGZidjRsKQo+ID4gd2hpY2ggY3JlYXRl cyBhIG1hZ2ljIGZyYW1lYnVmZmVyIGZyb20gYSB2NGwgaW5wdXQvb3V0cHV0LiBTb21lIHZhbHVl cwo+ID4gbGlrZSBzdHJpZGUgZG9uJ3QgbWFrZSBzZW5zZSBpbiBzdWNoIGEgdmlydHVhbCBmcmFt ZWJ1ZmZlciwgYnV0IHBpeGVsCj4gPiBmb3JtYXQgYW5kIHNpemUgYXJlIGFsbCBuZWVkZWQuCj4g Pgo+ID4gVGhpcyB3YXkgd2UgZG9uJ3QgbmVlZCBwYXJhbGxlbCBhYmlzIGZvciBzaW5nbGUtc2hv dCB3cml0ZWJhY2sgZGlyZWN0bHkKPiA+IGludG8gZnJhbWVidWZmZXJzIGFuZCBmb3IgY29udGlu dW91cyB3cml0ZWJhY2sgdGhyb3VnaCB2NGwsIHdlIGNhbiByZXVzZQo+ID4gdGhlIHNhbWUgZHJt IGZyYW1lYnVmZmVyIG9uZXMuIEFuZCB0aGlzIGFsc28gc29sdmVzIHRoZSBzZWN1cml0eSBpc3N1 ZXMKPiA+IHNpbmNlIG5vIG9uZSBjYW4gc3RhcnQgd3JpdGViYWNrIHdpdGhvdXQgdGhlIGRybSBk ZXZpY2Ugb3duZXIncyBjb25zZW50LAo+ID4gc28gbm8gbmVlZCB0byByZWludmVudCBhbnl0aGlu ZyB0aGVyZS4gQW5kIHdpdGggYXRvbWljIHdlIGFscmVhZHkgaGF2ZQo+ID4gYWxtb3N0IGV2ZXJ5 dGhpbmcgdGhlcmU6IEZvciB0aGUgd3JpdGViYWNrIGZyYW1lYnVmZmVyIHdlIG9ubHkgbmVlZCBh IG5ldwo+ID4gIldSSVRFQkFDSyIgcHJvcGVydHkgKHdoaWNoIHRha2VzIGFuIGZiIGlkKSBhbmQg dGhlIHNtYWxsIGV4dGVuc2lvbiB0bwo+ID4gY3JlYXRlIHY0bC1iYWNrZWQgZnJhbWVidWZmZXJz Lgo+ID4KPiA+IENoZWVycywgRGFuaWVsCj4gPiAtLQo+ID4gRGFuaWVsIFZldHRlcgo+ID4gU29m dHdhcmUgRW5naW5lZXIsIEludGVsIENvcnBvcmF0aW9uCj4gPiBodHRwOi8vYmxvZy5mZndsbC5j aAo+ID4KPiBIaSBEYW5pZWwsCj4gCj4gMS4gVGhpcyBjaGFuZ2UgaXMgdG8gaW1wbGVtZW50IGEg Y29udGludW91cyB3cml0ZWJhY2suCj4gMi4gQXMgeW91IHNhaWQsIHdlIG5lZWQgImEgZ2VuZXJp YyB3YXkgdG8gY29ubmVjdCB2NGwgYW5kIGRybSIuCj4gRXNwZWNpYWxseSBob3cgdG8gc2hhcmUg dGhlIGJ1ZmZlciBpbmZvcm1hdGlvbiBiZXR3ZWVuIHY0bCBhbmQgZHJtIGZvcgo+IHdyaXRlYmFj ayBvdXRwdXQuCj4gCj4gQmVsb3cgYXJlIGp1c3Qgc29tZSBkZXRhaWxzIG9mIHRoaXMgY2hhbmdl Ogo+IAo+IEluIGN1cnJlbnQgaW1wbGVtZW50YXRpb24sIEkgZXhwZWN0IHRoZSBvdXRwdXQgYnVm ZmVyIGlzIGRtYSBidWZmZXIKPiB3aGljaCBjb3VsZCBiZSBmcm9tIEdFTSBvYmplY3QgKGRybSkg b3IgZnJvbSB2aWRlbyBlbmNvZGVyIChWNGwpLiBPbmNlCj4gdGhlIGJ1ZmZlciBpcyBxdWV1ZWQg aW50byBWNGwgZHJpdmVyLCBpdCB3aWxsIGJlIGNvbnZlcnRlZCBpbnRvIGEgR0VNCj4gb2JqZWN0 IGFuZCB0aGVuIHBhc3MgaXQgdG8gZHJtIGFzIHdyaXRlYmFjayBvdXRwdXQgYnVmZmVyLiBPbmNl IHRoZQo+IGJ1ZmZlciBpcyBjYXB0dXJlZCwgaXQgd2lsbCBub3RpZnkgVjRsIGRyaXZlciB0byBs ZXQgdXNlciBkZXF1ZXVlCj4gYnVmZmVyLgo+IAo+IERybSB3aWxsIG5vdGljZSB0aGVyZSBpcyBh IFZpcnR1YWwgQ29ubmVjdG9yIChtYXliZSBhIG5ldyB0eXBlIFdSSVRFQkFDSwo+IGNhbiBiZSBh ZGRlZCksIGJ1dCBpdCB3aWxsIG9ubHkgYmUgImNvbm5lY3RlZCIgdW50aWwgVjRsCj4gc3RhcnRz IHN0cmVhbWluZy4KClllcyB3ZSBkZWZpbml0ZWx5IHNob3VsZCBhZGQgYSBuZXcgY29ubmVjdG9y IHR5cGUgV1JJVEVCQUNLLiBBbmQganVzdCB0aGUKY29ubmVjdG9yIGtpbmRhIHdvcmtzIGZvciB5 b3VyIGh3IGRlc2lnbiB3aGVyZSB3cml0ZWJhY2sgd29ya3MgbGlrZSBhCnNlcGFyYXRlIGVuY29k ZXIuIEJ1dCB0aGVyZSdzIGFsc28gaHcgb3V0IHRoZXJlIHdoZXJlIGFueSBjcnRjIGNhbiBiZQp3 cml0dGVuIGJhY2ssIGFuZCBmb3IgdGhvc2UgY2FzZXMgd2UgbmVlZCBleHBsaWNpdCBwcm9wZXJ0 aWVzLiBUaGVuCnRoZXJlJ3MgYWxzbyB0aGUgb25lLXNob3QgdnMuIGNvbnRpbnVvdXMgaXNzdWVz LgoKR2l2ZW4gYWxsIHRoYXQgSSBzdGlsbCB0aGluayB5b3Ugd2FudCBhbiBleHBsaWNpdCBkcm0g ZnJhbWVidWZmZXIgdG8KY29ubmVjdCB0aGUga21zIGFuZCB0aGUgdjRsIHNpZGUgb2YgdGhpbmdz LiBUaGF0IHdvdWxkIGFsc28gaGVscCBhIGJpdAp3aXRoIG1ha2luZyBpdCBjbGVhciB3aGljaCB2 NGwgY29ubmVjdHMgdG8gd2hpY2ggZHJtIGRldmljZS4KLURhbmllbAotLSAKRGFuaWVsIFZldHRl cgpTb2Z0d2FyZSBFbmdpbmVlciwgSW50ZWwgQ29ycG9yYXRpb24KaHR0cDovL2Jsb2cuZmZ3bGwu Y2gKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRl dmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9s aXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753342AbbDHIFT (ORCPT ); Wed, 8 Apr 2015 04:05:19 -0400 Received: from mail-wi0-f181.google.com ([209.85.212.181]:36117 "EHLO mail-wi0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751287AbbDHIFM (ORCPT ); Wed, 8 Apr 2015 04:05:12 -0400 Date: Wed, 8 Apr 2015 10:07:07 +0200 From: Daniel Vetter To: jilaiw@codeaurora.org Cc: Rob Clark , linux-arm-msm , Linux Kernel Mailing List , "dri-devel@lists.freedesktop.org" Subject: Re: [PATCH 2/3] drm:msm: Initial Add Writeback Support Message-ID: <20150408080707.GN6354@phenom.ffwll.local> Mail-Followup-To: jilaiw@codeaurora.org, Rob Clark , linux-arm-msm , Linux Kernel Mailing List , "dri-devel@lists.freedesktop.org" References: <1427922770-13721-1-git-send-email-jilaiw@codeaurora.org> <20150407055949.GI6354@phenom.ffwll.local> <93d3802305fa348738dae7f458c3fb56.squirrel@www.codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <93d3802305fa348738dae7f458c3fb56.squirrel@www.codeaurora.org> X-Operating-System: Linux phenom 4.0.0-rc3+ User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 07, 2015 at 03:55:45PM -0000, jilaiw@codeaurora.org wrote: > > On Thu, Apr 02, 2015 at 10:29:52AM -0400, Rob Clark wrote: > >> So, from a quick look, it seems like there is a lot of potential to > >> split the v4l part out into some drm helpers.. it looks pretty > >> generic(ish), or at least it could be with some strategically placed > >> vfuncs in drm_v4l2_helper_funcs. > >> > >> I do think we need to figure out the auth/security situation. We > >> probably don't want to let arbitrary processes open a v4l device and > >> snoop on the screen contents. We perhaps could re-use the dri2 drm > >> auth stuff (v4l2_drm_get_magic ioctl?). Or, well, it would be nice if > >> the wb device could be made to not exist in /dev at all, and > >> pre-open'd fd returned from an ioctl on the drm device, but not really > >> sure if that is possible (or too weird). Once the compositor process > >> has the v4l device open and authenticated somehow, I expect it would > >> use fd passing to pass the fd off to a trusted helper process. > > > > Please don't resurrect the magic stuff ;-) > > > > Anyway I discussed this a bit with Laurent and we figured the best way to > > wire up writeback support is by using drm framebuffers. Then you can use > > atomic flips to create a new snapshot. Of course that won't work with hw > > where writeback is continuous, there v4l is a much better fit. And we also > > have hardware where some v4l pipeline could directly feed into a drm > > output pipeline, so we need a generic way to connect v4l and drm anyway. > > For that I think we should add a new flag to addfb2 (or a new addfbv4l) > > which creates a magic framebuffer from a v4l input/output. Some values > > like stride don't make sense in such a virtual framebuffer, but pixel > > format and size are all needed. > > > > This way we don't need parallel abis for single-shot writeback directly > > into framebuffers and for continuous writeback through v4l, we can reuse > > the same drm framebuffer ones. And this also solves the security issues > > since no one can start writeback without the drm device owner's consent, > > so no need to reinvent anything there. And with atomic we already have > > almost everything there: For the writeback framebuffer we only need a new > > "WRITEBACK" property (which takes an fb id) and the small extension to > > create v4l-backed framebuffers. > > > > Cheers, Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > > Hi Daniel, > > 1. This change is to implement a continuous writeback. > 2. As you said, we need "a generic way to connect v4l and drm". > Especially how to share the buffer information between v4l and drm for > writeback output. > > Below are just some details of this change: > > In current implementation, I expect the output buffer is dma buffer > which could be from GEM object (drm) or from video encoder (V4l). Once > the buffer is queued into V4l driver, it will be converted into a GEM > object and then pass it to drm as writeback output buffer. Once the > buffer is captured, it will notify V4l driver to let user dequeue > buffer. > > Drm will notice there is a Virtual Connector (maybe a new type WRITEBACK > can be added), but it will only be "connected" until V4l > starts streaming. Yes we definitely should add a new connector type WRITEBACK. And just the connector kinda works for your hw design where writeback works like a separate encoder. But there's also hw out there where any crtc can be written back, and for those cases we need explicit properties. Then there's also the one-shot vs. continuous issues. Given all that I still think you want an explicit drm framebuffer to connect the kms and the v4l side of things. That would also help a bit with making it clear which v4l connects to which drm device. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch