From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Subject: Re: [PATCH 1/6] drm: Add writeback connector type Date: Wed, 19 Apr 2017 13:34:34 +0200 Message-ID: <20170419133434.229593a1@bbrezillon> References: <1480092544-1725-1-git-send-email-brian.starkey@arm.com> <1480092544-1725-2-git-send-email-brian.starkey@arm.com> <20170414120823.2cafc748@bbrezillon> <20170418173443.GA325@e106950-lin.cambridge.arm.com> <20170418215717.4381dd6e@bbrezillon> <20170419095122.GA6314@e106950-lin.cambridge.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail.free-electrons.com (mail.free-electrons.com [62.4.15.54]) by gabe.freedesktop.org (Postfix) with ESMTP id CD9B9891F9 for ; Wed, 19 Apr 2017 11:34:40 +0000 (UTC) In-Reply-To: <20170419095122.GA6314@e106950-lin.cambridge.arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Brian Starkey Cc: Daniel Vetter , liviu.dudau@arm.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, laurent.pinchart@ideasonboard.com, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org T24gV2VkLCAxOSBBcHIgMjAxNyAxMDo1MToyMyArMDEwMApCcmlhbiBTdGFya2V5IDxicmlhbi5z dGFya2V5QGFybS5jb20+IHdyb3RlOgoKPiBPbiBUdWUsIEFwciAxOCwgMjAxNyBhdCAwOTo1Nzox N1BNICswMjAwLCBCb3JpcyBCcmV6aWxsb24gd3JvdGU6Cj4gPkhpIEJyaWFuLAo+ID4KPiA+T24g VHVlLCAxOCBBcHIgMjAxNyAxODozNDo0MyArMDEwMAo+ID5CcmlhbiBTdGFya2V5IDxicmlhbi5z dGFya2V5QGFybS5jb20+IHdyb3RlOgo+ID4gIAo+ID4+ID4+IEBAIC0yMTQsNiArMjE0LDE5IEBA IHN0cnVjdCBkcm1fY29ubmVjdG9yX3N0YXRlIHsKPiA+PiA+PiAgCXN0cnVjdCBkcm1fZW5jb2Rl ciAqYmVzdF9lbmNvZGVyOwo+ID4+ID4+Cj4gPj4gPj4gIAlzdHJ1Y3QgZHJtX2F0b21pY19zdGF0 ZSAqc3RhdGU7Cj4gPj4gPj4gKwo+ID4+ID4+ICsJLyoqCj4gPj4gPj4gKwkgKiBAd3JpdGViYWNr X2pvYjogV3JpdGViYWNrIGpvYiBmb3Igd3JpdGViYWNrIGNvbm5lY3RvcnMKPiA+PiA+PiArCSAq Cj4gPj4gPj4gKwkgKiBIb2xkcyB0aGUgZnJhbWVidWZmZXIgZm9yIGEgd3JpdGViYWNrIGNvbm5l Y3Rvci4gQXMgdGhlIHdyaXRlYmFjawo+ID4+ID4+ICsJICogY29tcGxldGlvbiBtYXkgYmUgYXN5 bmNocm9ub3VzIHRvIHRoZSBub3JtYWwgY29tbWl0IGN5Y2xlLCB0aGUKPiA+PiA+PiArCSAqIHdy aXRlYmFjayBqb2IgbGlmZXRpbWUgaXMgbWFuYWdlZCBzZXBhcmF0ZWx5IGZyb20gdGhlIG5vcm1h bCBhdG9taWMKPiA+PiA+PiArCSAqIHN0YXRlIGJ5IHRoaXMgb2JqZWN0Lgo+ID4+ID4+ICsJICoK PiA+PiA+PiArCSAqIFNlZSBhbHNvOiBkcm1fd3JpdGViYWNrX3F1ZXVlX2pvYigpIGFuZAo+ID4+ ID4+ICsJICogZHJtX3dyaXRlYmFja19zaWduYWxfY29tcGxldGlvbigpCj4gPj4gPj4gKwkgKi8K PiA+PiA+PiArCXN0cnVjdCBkcm1fd3JpdGViYWNrX2pvYiAqd3JpdGViYWNrX2pvYjsgIAo+ID4+ ID4KPiA+PiA+TWF5YmUgSSdtIHdyb25nLCBidXQgaXMgZmVlbHMgd2VpcmQgdG8gaGF2ZSB0aGUg d3JpdGViYWNrX2pvYiBmaWVsZAo+ID4+ID5kaXJlY3RseSBlbWJlZGRlZCBpbiBkcm1fY29ubmVj dG9yX3N0YXRlLCB3aGlsZSBkcm1fd3JpdGViYWNrX2Nvbm5lY3Rvcgo+ID4+ID5pbmhlcml0cyBm cm9tIGRybV9jb25uZWN0b3IuCj4gPj4gPgo+ID4+ID5JTU8sIGVpdGhlciB5b3UgZGVjaWRlIHRv IGRpcmVjdGx5IHB1dCB0aGUgZHJtX3dyaXRlYmFja19jb25uZWN0b3Incwo+ID4+ID5qb2JfeHh4 IGZpZWxkcyBpbiBkcm1fY29ubmVjdG9yIGFuZCBrZWVwIHRoZSBkcm1fY29ubmVjdG9yX3N0YXRl IGFzIGlzLAo+ID4+ID5vciB5b3UgY3JlYXRlIGEgZHJtX3dyaXRlYmFja19jb25uZWN0b3Jfc3Rh dGUgd2hpY2ggaW5oZXJpdHMgZnJvbQo+ID4+ID5kcm1fY29ubmVjdG9yX3N0YXRlIGFuZCBlbWJl ZHMgdGhlIHdyaXRlYmFja19qb2IgZmllbGQuICAKPiA+Pgo+ID4+IEkgZGlkIHNwZW5kIGEgZGVj ZW50IGFtb3VudCBvZiB0aW1lIGxvb2tpbmcgYXQgdHJhY2tpbmcgdGhlIHdyaXRlYmFjawo+ID4+ IHN0YXRlIGFsb25nIHdpdGggdGhlIG5vcm1hbCBjb25uZWN0b3Igc3RhdGUuIEkgY291bGRuJ3Qg Y29tZSB1cCB3aXRoCj4gPj4gYW55dGhpbmcgSSBsaWtlZC4KPiA+Pgo+ID4+IEFzIHRoZSBjb21t ZW50IG1lbnRpb25zLCBvbmUgb2YgdGhlIHByb2JsZW1zIGlzIHRoYXQgeW91IGhhdmUgdG8gbWFr ZQo+ID4+IHN1cmUgdGhlIHJlbGV2YW50IHBhcnRzIG9mIHRoZSBjb25uZWN0b3Jfc3RhdGUgc3Rh eSBhcm91bmQgdW50aWwgdGhlCj4gPj4gd3JpdGViYWNrIGlzIGZpbmlzaGVkLiBUaGF0IG1lYW5z IHlvdSd2ZSBnb3QgdG8gYmxvY2sgYmVmb3JlCj4gPj4gInN3YXBfc3RhdGUoKSIgdW50aWwgdGhl IHByZXZpb3VzIHdyaXRlYmFjayBpcyBkb25lLCBhbmQgdGhhdAo+ID4+IGVmZmVjdGl2ZWx5IGxp bWl0cyB5b3VyIGZyYW1lIHJhdGUgdG8gcmVmcmVzaC8yLgo+ID4+Cj4gPj4gVGhlIE1hbGktRFAg SFcgZG9lc24ndCBoYXZlIHRoYXQgbGltaXRhdGlvbiAtIHdlIGNhbiBxdWV1ZSB1cCBhIG5ldwo+ ID4+IGNvbW1pdCB3aGlsZSB0aGUgY3VycmVudCB3cml0ZWJhY2sgaXMgb25nb2luZy4gRm9yIHRo YXQgcmVhc29uIEkKPiA+PiBkaWRuJ3Qgd2FudCB0byBpbXBvc2Ugc3VjaCBhIGxpbWl0YXRpb24g aW4gdGhlIGZyYW1ld29yay4KPiA+Pgo+ID4+IEluIHYxIEkgYWxsb3dlZCB0aGF0IGJ5IG1ha2lu ZyB0aGUgTWFsaS1EUCBkcml2ZXIgaG9sZCBpdHMgb3duCj4gPj4gcmVmZXJlbmNlcyB0byB0aGUg cmVsZXZhbnQgYml0cyBvZiB0aGUgc3RhdGUgZm9yIGFzIGxvbmcgYXMgaXQgbmVlZGVkCj4gPj4g dGhlbS4gSW4gdjMgSSBtb3ZlZCBtb3N0IG9mIHRoYXQgY29kZSBiYWNrIHRvIHRoZSBjb3JlIChp biBwYXJ0Cj4gPj4gYmVjYXVzZSBHdXN0YXZvIGRpZG4ndCBsaWtlIG1lIHNpZ25hbGxpbmcgdGhl IERSTS0ib3duZWQiIGZlbmNlIGZyb20KPiA+PiBteSBkcml2ZXIgY29kZSBkaXJlY3RseSkuIEkg dGhpbmsgdGhlIG5ldyBhcHByb2FjaCBvZiAicXVldWVfam9iKCkiCj4gPj4gYW5kICJzaWduYWxf am9iKCkiIHJlZHVjZXMgdGhlIGFtb3VudCBvZiB0cmlja3kgY29kZSBpbiBkcml2ZXJzLCBhbmQK PiA+PiBpcyBnZW5lcmFsbHkgbW9yZSBjbGVhciAoYWxzbyBmYW1pbGlhciwgd2hlbiBjb21wYXJl ZCB0byB2c3luYwo+ID4+IGV2ZW50cykuCj4gPj4KPiA+PiBJJ20gY2VydGFpbiB0aGVyZSdzIG90 aGVyIHdheXMgdG8gZG8gaXQgKHJlZmNvdW50IGF0b21pYyBzdGF0ZXM/KSwgYnV0Cj4gPj4gaXQg c2VlbWVkIGxpa2UgYSBiaWdnaXNoIG92ZXJoYXVsIHRvIGFjaGlldmUgd2hhdCB3b3VsZCBiYXNp Y2FsbHkgYmUKPiA+PiB0aGUgc2FtZSB0aGluZy4KPiA+Pgo+ID4+IEkgd2FzIGV4cGVjdGluZyBl YWNoIGRyaXZlciBzdXBwb3J0aW5nIHdyaXRlYmFjayB0byBoYXZlIGl0cyBvd24KPiA+PiBkaWZm ZXJlbnQgcmVxdWlyZW1lbnRzIGFyb3VuZCB3cml0ZWJhY2sgbGlmZXRpbWUvZHVyYXRpb24uIEZv ciBleGFtcGxlCj4gPj4gSSB0aGluayBWQzQgc3BlY2lmaWNhbGx5IGNhbWUgdXAsIGluIHRoYXQg aXRzIHdyaXRlYmFjayBjb3VsZCB0YWtlCj4gPj4gc2V2ZXJhbCBmcmFtZXMsIHdoZXJlYXMgb24g TWFsaS1EUCB3ZSBlaXRoZXIgZmluaXNoIHdpdGhpbiB0aGUgZnJhbWUKPiA+PiBvciB3ZSBmYWls Lgo+ID4+Cj4gPj4gTGV0dGluZyB0aGUgZHJpdmVyIG1hbmFnZSBpdHMgd3JpdGViYWNrX2pvYiBs aWZldGltZSBzZWVtZWQgbGlrZSBhCj4gPj4gcmVhc29uYWJsZSB3YXkgdG8gaGFuZGxlIGFsbCB0 aGF0LCB3aXRoIHRoZSBkb2N1bWVudGF0aW9uIHN0YXRpbmcgdGhlCj4gPj4gb25seSBiZWhhdmlv dXIgd2hpY2ggaXMgZ3VhcmFudGVlZCB0byB3b3JrIG9uIGFsbCBkcml2ZXJzOgo+ID4+Cj4gPj4g ICAgKiAgICAgVXNlcnNwYWNlIHNob3VsZCB3YWl0IGZvciB0aGlzIGZlbmNlIHRvIHNpZ25hbCBi ZWZvcmUgbWFraW5nIGFub3RoZXIKPiA+PiAgICAqICAgICBjb21taXQgYWZmZWN0aW5nIGFueSBv ZiB0aGUgc2FtZSBDUlRDcywgUGxhbmVzIG9yIENvbm5lY3RvcnMuCj4gPj4gICAgKiAgICAgKipG YWlsdXJlIHRvIGRvIHNvIHdpbGwgcmVzdWx0IGluIHVuZGVmaW5lZCBiZWhhdmlvdXIuKioKPiA+ PiAgICAqICAgICBGb3IgdGhpcyByZWFzb24gaXQgaXMgc3Ryb25nbHkgcmVjb21tZW5kZWQgdGhh dCBhbGwgdXNlcnNwYWNlCj4gPj4gICAgKiAgICAgYXBwbGljYXRpb25zIG1ha2luZyB1c2Ugb2Yg d3JpdGViYWNrIGNvbm5lY3RvcnMgKmFsd2F5cyogcmV0cmlldmUgYW4KPiA+PiAgICAqICAgICBv dXQtZmVuY2UgZm9yIHRoZSBjb21taXQgYW5kIHVzZSBpdCBhcHByb3ByaWF0ZWx5Lgo+ID4+Cj4g Pj4KPiA+Pgo+ID4+IC4uLiBzbyBhbGwgb2YgdGhhdCBpcyB3aHkgdGhlIF9qb2IgZmllbGRzIGRv bid0IGxpdmUgaW4gYSAqX3N0YXRlCj4gPj4gc3RydWN0dXJlIGRpcmVjdGx5LCBhbmQgaW5zdGVh ZCBoYXZlIHRvIGxpdmUgaW4gdGhlIHNlcGFyYXRlbHktbWFuYWdlZAo+ID4+IHN0cnVjdHVyZSBw b2ludGVkIHRvIGJ5IC0+d3JpdGViYWNrX2pvYi4KPiA+Pgo+ID4+IE5vdywgSSBkaWQgbG9vayBh dCBjcmVhdGluZyBkcm1fd3JpdGViYWNrX2Nvbm5lY3Rvcl9zdGF0ZSwgYnV0IGFzIGl0Cj4gPj4g d291bGQgb25seSBiZSBob2xkaW5nIHRoZSBqb2IgcG9pbnRlciAoc2VlIGFib3ZlKSBpdCBkaWRu J3Qgc2VlbSB3b3J0aAo+ID4+IHNjYXR0ZXJpbmcgYXJvdW5kIHRoZQo+ID4+Cj4gPj4gICAgIGlm IChjb25uX3N0YXRlLT5jb25uZWN0b3ItPmNvbm5lY3Rvcl90eXBlID09Cj4gPj4gICAgICAgICBE Uk1fTU9ERV9DT05ORUNUT1JfV1JJVEVCQUNLKQo+ID4+Cj4gPj4gY2hlY2tzIGV2ZXJ5d2hlcmUg YmVmb3JlIHVwLWNhc3RpbmcgLSB7Y2xlYXIscmVzZXQsZHVwbGljYXRlfV9zdGF0ZSgpLAo+ID4+ IHByZXBhcmVfc2lnbmFsbGluZygpLCBjb21wbGV0ZV9zaWduYWxsaW5nKCksIGV0Yy4gSXQganVz dCB0b3VjaGVkIGEKPiA+PiBsb3Qgb2YgY29kZSBmb3IgdGhlIHNha2Ugb2YgYW4gZXh0cmEgcG9p bnRlciBmaWVsZCBpbiBlYWNoIGNvbm5lY3Rvcgo+ID4+IHN0YXRlLgo+ID4+Cj4gPj4gSSBjYW4g ZWFzaWx5IHJldmlzaXQgdGhhdCBwYXJ0IGlmIHlvdSBsaWtlLiAgCj4gPgo+ID5JIHRoaW5rIHRo ZXJlJ3MgYSBtaXN1bmRlcnN0YW5kaW5nLiBJIHdhcyBqdXN0IHN1Z2dlc3RpbmcgdG8gYmUKPiA+ Y29uc2lzdGVudCBpbiB0aGUgaW5oZXJpdGFuY2UgdnMgJ29uZSBvYmplY3QgdG8gaGFuZGxlIGV2 ZXJ5dGhpbmcnCj4gPmFwcHJvYWNoLiAgCj4gCj4gZG9oLi4gcmlnaHQgeWVhaCBJIG1pc3JlYWQu IFNvcnJ5IGZvciB0aGUgdGFuZ2VudC4gOi0pCj4gCj4gPgo+ID5JJ20gcGVyZmVjdGx5IGZpbmUg d2l0aCBlbWJlZGRpbmcgdGhlIHdyaXRlYmFja19qb2IgcG9pbnRlciBkaXJlY3RseSBpbgo+ID5k cm1fY29ubmVjdG9yX3N0YXRlLCBidXQgdGhlbiBpdCB3b3VsZCBJTU8gbWFrZSBtb3JlIHNlbnNl IHRvIGRvIHRoZQo+ID5zYW1lIGZvciB0aGUgZHJtX2Nvbm5lY3RvciBvYmplY3QgKGVtYmVkIGRy bV93cml0ZWJhY2tfY29ubmVjdG9yIGZpZWxkcwo+ID5pbnRvIGRybV9jb25uZWN0b3IgaW5zdGVh ZCBvZiBtYWtpbmcgZHJtX3dyaXRlYmFja19jb25uZWN0b3IgaW5oZXJpdAo+ID5mcm9tIGRybV9j b25uZWN0b3IpLgo+ID4gIAo+IAo+IEkgYWdyZWUgdGhhdCBpdCdzIGluY29uc2lzdGVudC4gSSBn dWVzcyBJIGRpZCBpdCBvdXQgb2YgcHJhZ21hdGlzbSAtCj4gdGhlcmUncyBxdWl0ZSBhIGxvdCBv ZiBuZXcgZmllbGRzIGluIGRybV93cml0ZWJhY2tfY29ubmVjdG9yLCBhbmQgdGhlCj4gY29kZSBu ZWVkZWQgdG8gc3VwcG9ydCBpdCB3YXMgY29tcGFyYXRpdmVseSBzbWFsbC4gT24gdGhlIG90aGVy IGhhbmQKPiB0aGVyZSdzIG9ubHkgb25lIGFkZGl0aW9uYWwgZmllbGQgaW4gZHJtX2Nvbm5lY3Rv cl9zdGF0ZSBhbmQgdGhlIGNvZGUKPiByZXF1aXJlZCB0byBzdWJjbGFzcyBpdCBsb29rZWQgY29t cGFyYXRpdmVseSBsYXJnZS4KPiAKPiA+QW55d2F5LCB0aGF0J3MganVzdCBhIGRldGFpbC4KPiA+ ICAKPiA+PiAgCj4gPj4gPgo+ID4+ID5Bbnl3YXksIHdhaXQgZm9yIERhbmllbCdzIGZlZWRiYWNr IGJlZm9yZSBkb2luZyB0aGlzIGNoYW5nZS4KPiA+PiA+ICAKPiA+Pgo+ID4+IEFtIEkgZXhwZWN0 aW5nIHNvbWUgbW9yZSBmZWVkYmFjayBmcm9tIERhbmllbD8gIAo+ID4KPiA+Tm8sIEkgd2FzIGp1 c3Qgc2F5aW5nIHRoYXQgYmVmb3JlIGRvaW5nIHRoZSBjaGFuZ2VzIEkgd2FzIHN1Z2dlc3Rpbmcs Cj4gPnlvdSBzaG91bGQgd2FpdCBmb3IgRGFuaWVsJ3Mgb3BpbmlvbiwgYmVjYXVzZSBJIG1pZ2h0 IGJlIHdyb25nLgo+ID4gIAo+ID4+ICAKPiA+PiA+PiAgfTsKPiA+PiA+Pgo+ID4+ID4+ICAvKioK PiA+PiA+PiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9kcm0vZHJtX21vZGVfY29uZmlnLmggYi9pbmNs dWRlL2RybS9kcm1fbW9kZV9jb25maWcuaAo+ID4+ID4+IGluZGV4IGJmOTk5MWIyLi4zZDNkMDdm IDEwMDY0NAo+ID4+ID4+IC0tLSBhL2luY2x1ZGUvZHJtL2RybV9tb2RlX2NvbmZpZy5oCj4gPj4g Pj4gKysrIGIvaW5jbHVkZS9kcm0vZHJtX21vZGVfY29uZmlnLmgKPiA+PiA+PiBAQCAtNjM0LDYg KzYzNCwyMCBAQCBzdHJ1Y3QgZHJtX21vZGVfY29uZmlnIHsKPiA+PiA+PiAgCSAqLwo+ID4+ID4+ ICAJc3RydWN0IGRybV9wcm9wZXJ0eSAqc3VnZ2VzdGVkX3lfcHJvcGVydHk7Cj4gPj4gPj4KPiA+ PiA+PiArCS8qKgo+ID4+ID4+ICsJICogQHdyaXRlYmFja19mYl9pZF9wcm9wZXJ0eTogUHJvcGVy dHkgZm9yIHdyaXRlYmFjayBjb25uZWN0b3JzLCBzdG9yaW5nCj4gPj4gPj4gKwkgKiB0aGUgSUQg b2YgdGhlIG91dHB1dCBmcmFtZWJ1ZmZlci4KPiA+PiA+PiArCSAqIFNlZSBhbHNvOiBkcm1fd3Jp dGViYWNrX2Nvbm5lY3Rvcl9pbml0KCkKPiA+PiA+PiArCSAqLwo+ID4+ID4+ICsJc3RydWN0IGRy bV9wcm9wZXJ0eSAqd3JpdGViYWNrX2ZiX2lkX3Byb3BlcnR5Owo+ID4+ID4+ICsJLyoqCj4gPj4g Pj4gKwkgKiBAd3JpdGViYWNrX3BpeGVsX2Zvcm1hdHNfcHJvcGVydHk6IFByb3BlcnR5IGZvciB3 cml0ZWJhY2sgY29ubmVjdG9ycywKPiA+PiA+PiArCSAqIHN0b3JpbmcgYW4gYXJyYXkgb2YgdGhl IHN1cHBvcnRlZCBwaXhlbCBmb3JtYXRzIGZvciB0aGUgd3JpdGViYWNrCj4gPj4gPj4gKwkgKiBl bmdpbmUgKHJlYWQtb25seSkuCj4gPj4gPj4gKwkgKiBTZWUgYWxzbzogZHJtX3dyaXRlYmFja19j b25uZWN0b3JfaW5pdCgpCj4gPj4gPj4gKwkgKi8KPiA+PiA+PiArCXN0cnVjdCBkcm1fcHJvcGVy dHkgKndyaXRlYmFja19waXhlbF9mb3JtYXRzX3Byb3BlcnR5Owo+ID4+ID4+ICsKPiA+PiA+PiAg CS8qIGR1bWIgaW9jdGwgcGFyYW1ldGVycyAqLwo+ID4+ID4+ICAJdWludDMyX3QgcHJlZmVycmVk X2RlcHRoLCBwcmVmZXJfc2hhZG93Owo+ID4+ID4+Cj4gPj4gPj4gZGlmZiAtLWdpdCBhL2luY2x1 ZGUvZHJtL2RybV93cml0ZWJhY2suaCBiL2luY2x1ZGUvZHJtL2RybV93cml0ZWJhY2suaAo+ID4+ ID4+IG5ldyBmaWxlIG1vZGUgMTAwNjQ0Cj4gPj4gPj4gaW5kZXggMDAwMDAwMC4uNmIyYWM0NQo+ ID4+ID4+IC0tLSAvZGV2L251bGwKPiA+PiA+PiArKysgYi9pbmNsdWRlL2RybS9kcm1fd3JpdGVi YWNrLmgKPiA+PiA+PiBAQCAtMCwwICsxLDc4IEBACj4gPj4gPj4gKy8qCj4gPj4gPj4gKyAqIChD KSBDT1BZUklHSFQgMjAxNiBBUk0gTGltaXRlZC4gQWxsIHJpZ2h0cyByZXNlcnZlZC4KPiA+PiA+ PiArICogQXV0aG9yOiBCcmlhbiBTdGFya2V5IDxicmlhbi5zdGFya2V5QGFybS5jb20+Cj4gPj4g Pj4gKyAqCj4gPj4gPj4gKyAqIFRoaXMgcHJvZ3JhbSBpcyBmcmVlIHNvZnR3YXJlIGFuZCBpcyBw cm92aWRlZCB0byB5b3UgdW5kZXIgdGhlIHRlcm1zIG9mIHRoZQo+ID4+ID4+ICsgKiBHTlUgR2Vu ZXJhbCBQdWJsaWMgTGljZW5zZSB2ZXJzaW9uIDIgYXMgcHVibGlzaGVkIGJ5IHRoZSBGcmVlIFNv ZnR3YXJlCj4gPj4gPj4gKyAqIEZvdW5kYXRpb24sIGFuZCBhbnkgdXNlIGJ5IHlvdSBvZiB0aGlz IHByb2dyYW0gaXMgc3ViamVjdCB0byB0aGUgdGVybXMKPiA+PiA+PiArICogb2Ygc3VjaCBHTlUg bGljZW5jZS4KPiA+PiA+PiArICovCj4gPj4gPj4gKwo+ID4+ID4+ICsjaWZuZGVmIF9fRFJNX1dS SVRFQkFDS19IX18KPiA+PiA+PiArI2RlZmluZSBfX0RSTV9XUklURUJBQ0tfSF9fCj4gPj4gPj4g KyNpbmNsdWRlIDxkcm0vZHJtX2Nvbm5lY3Rvci5oPgo+ID4+ID4+ICsjaW5jbHVkZSA8bGludXgv d29ya3F1ZXVlLmg+Cj4gPj4gPj4gKwo+ID4+ID4+ICtzdHJ1Y3QgZHJtX3dyaXRlYmFja19jb25u ZWN0b3Igewo+ID4+ID4+ICsJc3RydWN0IGRybV9jb25uZWN0b3IgYmFzZTsgIAo+ID4+ID4KPiA+ PiA+QUZBSVUsIGEgd3JpdGViYWNrIGNvbm5lY3RvciB3aWxsIGFsd2F5cyByZXF1aXJlIGFuICdk dW1teScgZW5jb2RlciB0bwo+ID4+ID5tYWtlIHRoZSBEUk0gZnJhbWV3b3JrIGhhcHB5IChBRkFJ SywgYSBjb25uZWN0b3IgaXMgYWx3YXlzIGNvbm5lY3RlZCB0bwo+ID4+ID5hIENSVEMgdGhyb3Vn aCBhbiBlbmNvZGVyKS4KPiA+PiA+Cj4gPj4gPldvdWxkbid0IGl0IG1ha2UgbW9yZSBzZW5zZSB0 byBoYXZlIGEgZHJtX2VuY29kZXIgb2JqZWN0IGVtYmVkZGVkIGluCj4gPj4gPmRybV93cml0ZWJh Y2tfY29ubmVjdG9yIHNvIHRoYXQgcGVvcGxlIGRvbid0IGhhdmUgdG8gZGVjbGFyZSBhbiBleHRy YQo+ID4+ID5zdHJ1Y3R1cmUgY29udGFpbmluZyBib3RoIHRoZSBkcm1fd3JpdGViYWNrX2Nvbm5l Y3RvciBjb25uZWN0b3IgYW5kIGEKPiA+PiA+ZHJtX2VuY29kZXI/IElzIHRoZXJlIGEgZ29vZCBy ZWFzb24gdG8ga2VlcCB0aGVtIHNlcGFyYXRlPwo+ID4+ID4gIAo+ID4+Cj4gPj4gWWVhaCB0aGF0 J3Mgbm90IGEgYmFkIGlkZWEuIFRoZSBlbmNvZGVyIGZ1bmNzIGNvdWxkIGJlIHBhc3NlZCBpbiB0 bwo+ID4+IGRybV93cml0ZWJhY2tfY29ubmVjdG9yX2luaXQoKSAoaW4gd2hpY2ggY2FzZSBhZGRp bmcgYSB3cml0ZWJhY2sKPiA+PiBlbmNvZGVyIHR5cGUgd291bGQgYWxzbyBtYWtlIHNlbnNlKS4g IAo+ID4KPiA+V2VsbCwgdGhlIG1vcmUgSSBsb29rIGF0IGl0IHRoZSBtb3JlIEkgZmluZCBpdCB3 ZWlyZCB0byByZXByZXNlbnQgdGhpcwo+ID53cml0ZWJhY2sgZmVhdHVyZSBhcyBhIGNvbm5lY3Rv ci4gVG8gbWUsIGl0IHNlZW1zIHRvIGJlIGNsb3NlciB0byBhCj4gPnBsYW5lIG9iamVjdCAoRFJN X1BMQU5FX1RZUEVfV1JJVEVCQUNLPykgdGhhbiBhIHJlYWwgY29ubmVjdG9yLgo+ID5BY3R1YWxs eSwgdGhlIEF0bWVsIEhMQ0RDIElQIHVzZSB0aGUgc2FtZSByZWdpc3RlciBpbnRlcmZhY2UgdG8g ZXhwb3NlCj4gPnRoZSBvdmVybGF5IHBsYW5lcyBhbmQgd3JpdGViYWNrIGZlYXR1cmVzLgo+ID5P ZiBjb3Vyc2UsIHJlcHJlc2VudGluZyBpdCBhcyBhIHBsYW5lIHJlcXVpcmVzIHBhdGNoaW5nIHRo ZSBjb3JlIHRvCj4gPmFsbG93IGVuYWJsaW5nIGEgQ1JUQyB0aGF0IGhhcyBubyBhY3RpdmUgY29u bmVjdG9ycyBjb25uZWN0ZWQgdG8gaXQgaWYKPiA+YXQgbGVhc3Qgb25lIHdyaXRlYmFjayBwbGFu ZSBpcyBlbmFibGVkIGFuZCBjb25uZWN0ZWQgdG8gdGhlIENSVEMsIGJ1dAo+ID5pdCBzaG91bGQg YmUgZG9hYmxlLiAgCj4gCj4gQ291bGQgeW91IGV4cGFuZCBhIGJpdCBvbiBob3cgeW91IHRoaW5r IHBsYW5lcyBmaXQgYmV0dGVyPwoKSnVzdCBoYWQgdGhlIGltcHJlc3Npb24gdGhhdCB0aGUgd3Jp dGViYWNrIGZlYXR1cmUgd2FzIGNvbmNlcHR1YWxseQpjbG9zZXIgdG8gYSBwbGFuZSBvYmplY3Qg KHdoaWNoIGlzIGF0dGFjaGVkIGJ1ZmZlcnMgYW5kIGV4cG9zZSB3YXlzIHRvCnNwZWNpZnkgd2hp Y2ggcG9ydGlvbiBvZiB0aGUgYnVmZmVyIHNob3VsZCBiZSB1c2VkLCBwcm92aWRlcyB3YXkgdG8K YXRvbWljYWxseSBzd2l0Y2ggMiBidWZmZXJzLCAuLi4pLgoKPiBJdCBpcwo+IHNvbWV0aGluZyB3 ZSd2ZSBwcmV2aW91c2x5IHRhbGtlZCBhYm91dCBpbnRlcm5hbGx5LCBidXQgc28gZmFyIEknbSBu b3QKPiBjb252aW5jZWQgOi0pCgpPa2F5LCBhcyBJIHNhaWQsIEkgZG9uJ3Qga25vdyBhbGwgdGhl IGhpc3RvcnksIGhlbmNlIG15IHF1ZXN0aW9ucyA7LSkuCgo+IAo+ID5CeSBkb2luZyB0aGF0LCB3 ZSB3b3VsZCBhbHNvIGdldCByaWQgb2YgdGhlc2UgZmFrZSBjb25uZWN0b3IvZW5jb2Rlcgo+ID5v YmplY3RzIGFzIHdlbGwgYXMgdGhlIGZha2UgbW9kZXMgd2UgYXJlIHJldHVybmluZyBpbgo+ID5j b25uZWN0b3ItPmdldF9tb2RlcygpLiAgCj4gCj4gV2hhdCBtYWtlcyB0aGUgY29ubmVjdG9yL2Vu Y29kZXIgZmFrZT8gVGhleSByZXByZXNlbnQgYSByZWFsIHBpZWNlIG9mCj4gaGFyZHdhcmUganVz dCB0aGUgc2FtZSBhcyBhIGRybV9wbGFuZSB3b3VsZC4KCldlbGwsIHRoYXQncyBwcm9iYWJseSBz dWJqZWN0IHRvIGludGVycHJldGF0aW9uLCBidXQgSSBkb24ndCBjb25zaWRlcgp0aGVzZSB3cml0 ZWJhY2sgZW5jb2RlcnMvY29ubmVjdG9ycyBhcyByZWFsIGVuY29kZXJzL2Nvbm5lY3RvcnMuIFRo ZXkKYXJlIGRlZmluaXRlbHkgcmVhbCBIVyBibG9ja3MsIGJ1dCBub3Qgd2hhdCB3ZSB1c3VhbGx5 IGNvbnNpZGVyIGFzIGFuCmVuY29kZXIvY29ubmVjdG9yLgoKPiAKPiBJIGRvbid0IG1pbmQgZHJv cHBpbmcgdGhlIG1vZGUgbGlzdCBhbmQgbGV0dGluZyB1c2Vyc3BhY2UganVzdCBtYWtlCj4gdXAg d2hhdGV2ZXIgdGltaW5nIGl0IHdhbnRzIC0gaXQnZCBuZWVkIHRvIGRvIHRoZSBzYW1lIGlmIHdy aXRlYmFjawo+IHdhcyBhIHBsYW5lIC0gYnV0IGluIHNvbWUgcmVzcGVjdHMgZ2l2aW5nIGl0IGEg bGlzdCBvZiBtb2RlcyB0aGUgc2FtZQo+IHdheSB3ZSBub3JtYWxseSBkbyBzZWVtcyBuaWNlciBm b3IgdXNlcnNwYWNlLgo+IAo+ID4KPiA+QXMgZmFyIGFzIEkgY2FuIHRlbGwsIHRoZSBWQzQgYW5k IEF0bWVsIEhMQ0RDIElQIHNob3VsZCBmaXQgcHJldHR5IHdlbGwKPiA+aW4gdGhpcyBtb2RlbCwg bm90IHN1cmUgYWJvdXQgdGhlIG1hbGktZHAgdGhvdWdoLgo+ID4KPiA+QnJpYW4sIGRpZCB5b3Ug Y29uc2lkZXIgdGhpcyBhcHByb2FjaCwgYW5kIGlmIHlvdSBkaWQsIGNhbiB5b3UgZGV0YWlsCj4g PndoeSB5b3UgZGVjaWRlZCB0byBleHBvc2UgdGhpcyBmZWF0dXJlIGFzIGEgY29ubmVjdG9yPwo+ ID4KPiA+RGFuaWVsIChvciBhbnlvbmUgZWxzZSksIHBsZWFzZSBzdGVwIGluIGlmIHlvdSB0aGlu ayB0aGlzIGlzIGEgc3R1cGlkCj4gPmlkZWEgOi0pLiAgCj4gCj4gVmlsbGUgb3JpZ2luYWxseSBz dWdnZXN0ZWQgdXNpbmcgYSBjb25uZWN0b3IsIHdoaWNoIEVyaWMgZm9sbG93ZWQgdXAKPiBieSBz YXlpbmcgdGhhdCdzIHdoYXQgaGUgd2FzIHRoaW5raW5nIG9mIGZvciBWQzQgd3JpdGViYWNrIHRv b1sxXS4KClRoYW5rcyBmb3IgdGhlIHBvaW50ZXIuCgo+IFRoYXQgd2FzIG15IGluaXRpYWwgcmVh c29uIGZvciBmb2N1c3Npbmcgb24gYSBjb25uZWN0b3ItYmFzZWQKPiBhcHByb2FjaC4KPiAKPiBJ IHByZWZlciBjb25uZWN0b3Igb3ZlciBwbGFuZSBjb25jZXB0dWFsbHkgYmVjYXVzZSBpdCBrZWVw cyB3aXRoIHRoZQo+IGVzdGFibGlzaGVkIGRhdGEgZmxvdzogcGxhbmVzIGFyZSBzb3VyY2VzLCBj b25uZWN0b3JzIGFyZSBzaW5rcy4KCk9rYXksIGl0J3MgYSB2YWxpZCBwb2ludC4KCj4gCj4gSW4g c29tZSByZXNwZWN0cyB0aGUgcGxhbmUgX29iamVjdF8gbG9va3MgbGlrZSBpdCB3b3VsZCBmaXQg LSBpdCBoYXMgYQo+IHBpeGVsIGZvcm1hdCBsaXN0IGFuZCBhbiBGQl9JRC4gQnV0IGV2ZXJ5dGhp bmcgZWxzZSB3b3VsZCBiZSBhY3RpbmcKPiB0aGUgZXhhY3Qgb3Bwb3NpdGUgdG8gYSBub3JtYWwg cGxhbmUsIGFuZCBJIHRoaW5rIHRoZXJlJ3MgYSBidW5jaCBvZgo+IGJha2VkLWluIGFzc3VtcHRp b25zIGluIHRoZSBrZXJuZWwgYW5kIHVzZXJzcGFjZSBhcm91bmQgQ1JUQ3MgYWx3YXlzCj4gaGF2 aW5nIGF0IGxlYXN0IG9uZSBjb25uZWN0b3IuCgpZZXAsIGJ1dCB3cml0ZWJhY2sgY29ubmVjdG9y cyBhcmUgYWxyZWFkeSBkaWZmZXJlbnQgZW5vdWdoIHRvIG5vdCBiZQpjb25zaWRlcmVkIGFzIHJl Z3VsYXIgY29ubmVjdG9ycywgc28gdXNlcnNwYWNlIHByb2dyYW1zIHdpbGwgaGF2ZSB0bwpoYW5k bGUgdGhlbSBkaWZmZXJlbnRseSBhbnl3YXkgKHBhc3MgYSBmcmFtZWJ1ZmZlciBhbmQgcGl4ZWwg Zm9ybWF0IHRvCml0IGJlZm9yZSBhZGRpbmcgdGhlbSB0byB0aGUgZGlzcGxheSBwaXBlbGluZSku CgpBbnl3YXksIEkgc2VlIHRoaXMgYXBwcm9hY2ggaGFzIGFscmVhZHkgYmVlbiBzdWdnZXN0ZWQg aW4gWzFdLCBhbmQgeW91CmFsbCBhZ3JlZWQgdGhhdCB0aGUgd3JpdGViYWNrIGZlYXR1cmUgc2hv dWxkIGJlIGV4cG9zZWQgYXMgYSBjb25uZWN0b3IsCnNvIEknbGwganVzdCBzdG9wIGhlcmUgOi0p LgoKVGhhbmtzIGZvciB0YWtpbmcgdGhlIHRpbWUgdG8gZXhwbGFpbiB0aGUgcmF0aW9uYWxlIGJl aGluZCB0aGlzCmRlY2lzaW9uLgoKPiAKPiBPbiB0aGUgb3RoZXIgaGFuZCwgYSB3cml0ZWJhY2sg Y29ubmVjdG9yIGdhaW5zIGEgZmV3IGV4dHJhIHByb3BlcnRpZXMKPiBvdmVyIGEgbm9ybWFsIGNv bm5lY3RvciwgYnV0IG1vc3Qgc3R1ZmYgc3RheXMgdGhlIHNhbWUuIFRoZSBwaXBlbGluZQo+IHNl dHVwIGxvb2tzIHRoZSBzYW1lIGFzIG5vcm1hbCB0byB1c2Vyc3BhY2UsIHlvdSBkb24ndCBuZWVk IGEgQ1JUQyB0bwo+IGJlIGFjdGl2ZSB3aXRoIG5vIGNvbm5lY3RvcnMsIG91dHB1dCBjbG9uaW5n IGlzIHRoZSBzYW1lIGV0Yy4KPiAKPiBUaGFua3MsCj4gLUJyaWFuCj4gCj4gWzFdIGh0dHBzOi8v bGlzdHMuZnJlZWRlc2t0b3Aub3JnL2FyY2hpdmVzL2RyaS1kZXZlbC8yMDE2LUp1bHkvMTEzMzI5 Lmh0bWwKPiAKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f CmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpo dHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail.free-electrons.com ([62.4.15.54]:39373 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761087AbdDSLek (ORCPT ); Wed, 19 Apr 2017 07:34:40 -0400 Date: Wed, 19 Apr 2017 13:34:34 +0200 From: Boris Brezillon To: Brian Starkey Cc: Daniel Vetter , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, liviu.dudau@arm.com, laurent.pinchart@ideasonboard.com, linux-media@vger.kernel.org Subject: Re: [PATCH 1/6] drm: Add writeback connector type Message-ID: <20170419133434.229593a1@bbrezillon> In-Reply-To: <20170419095122.GA6314@e106950-lin.cambridge.arm.com> References: <1480092544-1725-1-git-send-email-brian.starkey@arm.com> <1480092544-1725-2-git-send-email-brian.starkey@arm.com> <20170414120823.2cafc748@bbrezillon> <20170418173443.GA325@e106950-lin.cambridge.arm.com> <20170418215717.4381dd6e@bbrezillon> <20170419095122.GA6314@e106950-lin.cambridge.arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: On Wed, 19 Apr 2017 10:51:23 +0100 Brian Starkey wrote: > On Tue, Apr 18, 2017 at 09:57:17PM +0200, Boris Brezillon wrote: > >Hi Brian, > > > >On Tue, 18 Apr 2017 18:34:43 +0100 > >Brian Starkey wrote: > > > >> >> @@ -214,6 +214,19 @@ struct drm_connector_state { > >> >> struct drm_encoder *best_encoder; > >> >> > >> >> struct drm_atomic_state *state; > >> >> + > >> >> + /** > >> >> + * @writeback_job: Writeback job for writeback connectors > >> >> + * > >> >> + * Holds the framebuffer for a writeback connector. As the writeback > >> >> + * completion may be asynchronous to the normal commit cycle, the > >> >> + * writeback job lifetime is managed separately from the normal atomic > >> >> + * state by this object. > >> >> + * > >> >> + * See also: drm_writeback_queue_job() and > >> >> + * drm_writeback_signal_completion() > >> >> + */ > >> >> + struct drm_writeback_job *writeback_job; > >> > > >> >Maybe I'm wrong, but is feels weird to have the writeback_job field > >> >directly embedded in drm_connector_state, while drm_writeback_connector > >> >inherits from drm_connector. > >> > > >> >IMO, either you decide to directly put the drm_writeback_connector's > >> >job_xxx fields in drm_connector and keep the drm_connector_state as is, > >> >or you create a drm_writeback_connector_state which inherits from > >> >drm_connector_state and embeds the writeback_job field. > >> > >> I did spend a decent amount of time looking at tracking the writeback > >> state along with the normal connector state. I couldn't come up with > >> anything I liked. > >> > >> As the comment mentions, one of the problems is that you have to make > >> sure the relevant parts of the connector_state stay around until the > >> writeback is finished. That means you've got to block before > >> "swap_state()" until the previous writeback is done, and that > >> effectively limits your frame rate to refresh/2. > >> > >> The Mali-DP HW doesn't have that limitation - we can queue up a new > >> commit while the current writeback is ongoing. For that reason I > >> didn't want to impose such a limitation in the framework. > >> > >> In v1 I allowed that by making the Mali-DP driver hold its own > >> references to the relevant bits of the state for as long as it needed > >> them. In v3 I moved most of that code back to the core (in part > >> because Gustavo didn't like me signalling the DRM-"owned" fence from > >> my driver code directly). I think the new approach of "queue_job()" > >> and "signal_job()" reduces the amount of tricky code in drivers, and > >> is generally more clear (also familiar, when compared to vsync > >> events). > >> > >> I'm certain there's other ways to do it (refcount atomic states?), but > >> it seemed like a biggish overhaul to achieve what would basically be > >> the same thing. > >> > >> I was expecting each driver supporting writeback to have its own > >> different requirements around writeback lifetime/duration. For example > >> I think VC4 specifically came up, in that its writeback could take > >> several frames, whereas on Mali-DP we either finish within the frame > >> or we fail. > >> > >> Letting the driver manage its writeback_job lifetime seemed like a > >> reasonable way to handle all that, with the documentation stating the > >> only behaviour which is guaranteed to work on all drivers: > >> > >> * Userspace should wait for this fence to signal before making another > >> * commit affecting any of the same CRTCs, Planes or Connectors. > >> * **Failure to do so will result in undefined behaviour.** > >> * For this reason it is strongly recommended that all userspace > >> * applications making use of writeback connectors *always* retrieve an > >> * out-fence for the commit and use it appropriately. > >> > >> > >> > >> ... so all of that is why the _job fields don't live in a *_state > >> structure directly, and instead have to live in the separately-managed > >> structure pointed to by ->writeback_job. > >> > >> Now, I did look at creating drm_writeback_connector_state, but as it > >> would only be holding the job pointer (see above) it didn't seem worth > >> scattering around the > >> > >> if (conn_state->connector->connector_type == > >> DRM_MODE_CONNECTOR_WRITEBACK) > >> > >> checks everywhere before up-casting - {clear,reset,duplicate}_state(), > >> prepare_signalling(), complete_signalling(), etc. It just touched a > >> lot of code for the sake of an extra pointer field in each connector > >> state. > >> > >> I can easily revisit that part if you like. > > > >I think there's a misunderstanding. I was just suggesting to be > >consistent in the inheritance vs 'one object to handle everything' > >approach. > > doh.. right yeah I misread. Sorry for the tangent. :-) > > > > >I'm perfectly fine with embedding the writeback_job pointer directly in > >drm_connector_state, but then it would IMO make more sense to do the > >same for the drm_connector object (embed drm_writeback_connector fields > >into drm_connector instead of making drm_writeback_connector inherit > >from drm_connector). > > > > I agree that it's inconsistent. I guess I did it out of pragmatism - > there's quite a lot of new fields in drm_writeback_connector, and the > code needed to support it was comparatively small. On the other hand > there's only one additional field in drm_connector_state and the code > required to subclass it looked comparatively large. > > >Anyway, that's just a detail. > > > >> > >> > > >> >Anyway, wait for Daniel's feedback before doing this change. > >> > > >> > >> Am I expecting some more feedback from Daniel? > > > >No, I was just saying that before doing the changes I was suggesting, > >you should wait for Daniel's opinion, because I might be wrong. > > > >> > >> >> }; > >> >> > >> >> /** > >> >> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > >> >> index bf9991b2..3d3d07f 100644 > >> >> --- a/include/drm/drm_mode_config.h > >> >> +++ b/include/drm/drm_mode_config.h > >> >> @@ -634,6 +634,20 @@ struct drm_mode_config { > >> >> */ > >> >> struct drm_property *suggested_y_property; > >> >> > >> >> + /** > >> >> + * @writeback_fb_id_property: Property for writeback connectors, storing > >> >> + * the ID of the output framebuffer. > >> >> + * See also: drm_writeback_connector_init() > >> >> + */ > >> >> + struct drm_property *writeback_fb_id_property; > >> >> + /** > >> >> + * @writeback_pixel_formats_property: Property for writeback connectors, > >> >> + * storing an array of the supported pixel formats for the writeback > >> >> + * engine (read-only). > >> >> + * See also: drm_writeback_connector_init() > >> >> + */ > >> >> + struct drm_property *writeback_pixel_formats_property; > >> >> + > >> >> /* dumb ioctl parameters */ > >> >> uint32_t preferred_depth, prefer_shadow; > >> >> > >> >> diff --git a/include/drm/drm_writeback.h b/include/drm/drm_writeback.h > >> >> new file mode 100644 > >> >> index 0000000..6b2ac45 > >> >> --- /dev/null > >> >> +++ b/include/drm/drm_writeback.h > >> >> @@ -0,0 +1,78 @@ > >> >> +/* > >> >> + * (C) COPYRIGHT 2016 ARM Limited. All rights reserved. > >> >> + * Author: Brian Starkey > >> >> + * > >> >> + * This program is free software and is provided to you under the terms of the > >> >> + * GNU General Public License version 2 as published by the Free Software > >> >> + * Foundation, and any use by you of this program is subject to the terms > >> >> + * of such GNU licence. > >> >> + */ > >> >> + > >> >> +#ifndef __DRM_WRITEBACK_H__ > >> >> +#define __DRM_WRITEBACK_H__ > >> >> +#include > >> >> +#include > >> >> + > >> >> +struct drm_writeback_connector { > >> >> + struct drm_connector base; > >> > > >> >AFAIU, a writeback connector will always require an 'dummy' encoder to > >> >make the DRM framework happy (AFAIK, a connector is always connected to > >> >a CRTC through an encoder). > >> > > >> >Wouldn't it make more sense to have a drm_encoder object embedded in > >> >drm_writeback_connector so that people don't have to declare an extra > >> >structure containing both the drm_writeback_connector connector and a > >> >drm_encoder? Is there a good reason to keep them separate? > >> > > >> > >> Yeah that's not a bad idea. The encoder funcs could be passed in to > >> drm_writeback_connector_init() (in which case adding a writeback > >> encoder type would also make sense). > > > >Well, the more I look at it the more I find it weird to represent this > >writeback feature as a connector. To me, it seems to be closer to a > >plane object (DRM_PLANE_TYPE_WRITEBACK?) than a real connector. > >Actually, the Atmel HLCDC IP use the same register interface to expose > >the overlay planes and writeback features. > >Of course, representing it as a plane requires patching the core to > >allow enabling a CRTC that has no active connectors connected to it if > >at least one writeback plane is enabled and connected to the CRTC, but > >it should be doable. > > Could you expand a bit on how you think planes fit better? Just had the impression that the writeback feature was conceptually closer to a plane object (which is attached buffers and expose ways to specify which portion of the buffer should be used, provides way to atomically switch 2 buffers, ...). > It is > something we've previously talked about internally, but so far I'm not > convinced :-) Okay, as I said, I don't know all the history, hence my questions ;-). > > >By doing that, we would also get rid of these fake connector/encoder > >objects as well as the fake modes we are returning in > >connector->get_modes(). > > What makes the connector/encoder fake? They represent a real piece of > hardware just the same as a drm_plane would. Well, that's probably subject to interpretation, but I don't consider these writeback encoders/connectors as real encoders/connectors. They are definitely real HW blocks, but not what we usually consider as an encoder/connector. > > I don't mind dropping the mode list and letting userspace just make > up whatever timing it wants - it'd need to do the same if writeback > was a plane - but in some respects giving it a list of modes the same > way we normally do seems nicer for userspace. > > > > >As far as I can tell, the VC4 and Atmel HLCDC IP should fit pretty well > >in this model, not sure about the mali-dp though. > > > >Brian, did you consider this approach, and if you did, can you detail > >why you decided to expose this feature as a connector? > > > >Daniel (or anyone else), please step in if you think this is a stupid > >idea :-). > > Ville originally suggested using a connector, which Eric followed up > by saying that's what he was thinking of for VC4 writeback too[1]. Thanks for the pointer. > That was my initial reason for focussing on a connector-based > approach. > > I prefer connector over plane conceptually because it keeps with the > established data flow: planes are sources, connectors are sinks. Okay, it's a valid point. > > In some respects the plane _object_ looks like it would fit - it has a > pixel format list and an FB_ID. But everything else would be acting > the exact opposite to a normal plane, and I think there's a bunch of > baked-in assumptions in the kernel and userspace around CRTCs always > having at least one connector. Yep, but writeback connectors are already different enough to not be considered as regular connectors, so userspace programs will have to handle them differently anyway (pass a framebuffer and pixel format to it before adding them to the display pipeline). Anyway, I see this approach has already been suggested in [1], and you all agreed that the writeback feature should be exposed as a connector, so I'll just stop here :-). Thanks for taking the time to explain the rationale behind this decision. > > On the other hand, a writeback connector gains a few extra properties > over a normal connector, but most stuff stays the same. The pipeline > setup looks the same as normal to userspace, you don't need a CRTC to > be active with no connectors, output cloning is the same etc. > > Thanks, > -Brian > > [1] https://lists.freedesktop.org/archives/dri-devel/2016-July/113329.html >