From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brian Starkey Subject: Re: [PATCH 1/6] drm: Add writeback connector type Date: Tue, 18 Apr 2017 18:34:43 +0100 Message-ID: <20170418173443.GA325@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> 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 6330B6E208 for ; Tue, 18 Apr 2017 17:34:51 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20170414120823.2cafc748@bbrezillon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Boris Brezillon Cc: linux-media@vger.kernel.org, liviu.dudau@arm.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, laurent.pinchart@ideasonboard.com List-Id: dri-devel@lists.freedesktop.org SGkgQm9yaXMsCgpPbiBGcmksIEFwciAxNCwgMjAxNyBhdCAxMjowODoyM1BNICswMjAwLCBCb3Jp cyBCcmV6aWxsb24gd3JvdGU6Cj5PbiBGcmksIDI1IE5vdiAyMDE2IDE2OjQ4OjU5ICswMDAwCj5C cmlhbiBTdGFya2V5IDxicmlhbi5zdGFya2V5QGFybS5jb20+IHdyb3RlOgo+Cj4KPj4KPj4gZGlm ZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fY29ubmVjdG9yLmMgYi9kcml2ZXJzL2dwdS9k cm0vZHJtX2Nvbm5lY3Rvci5jCj4+IGluZGV4IGI1YzZhOGUuLjZiYmQ5M2YgMTAwNjQ0Cj4+IC0t LSBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fY29ubmVjdG9yLmMKPj4gKysrIGIvZHJpdmVycy9ncHUv ZHJtL2RybV9jb25uZWN0b3IuYwo+PiBAQCAtODYsNiArODYsNyBAQCBzdHJ1Y3QgZHJtX2Nvbm5f cHJvcF9lbnVtX2xpc3Qgewo+PiAgCXsgRFJNX01PREVfQ09OTkVDVE9SX1ZJUlRVQUwsICJWaXJ0 dWFsIiB9LAo+PiAgCXsgRFJNX01PREVfQ09OTkVDVE9SX0RTSSwgIkRTSSIgfSwKPj4gIAl7IERS TV9NT0RFX0NPTk5FQ1RPUl9EUEksICJEUEkiIH0sCj4+ICsJeyBEUk1fTU9ERV9DT05ORUNUT1Jf V1JJVEVCQUNLLCAiV3JpdGViYWNrIiB9LAo+Cj5JcyB0aGVyZSBhIHJlYXNvbiB3ZSBoYXZlIGEg V3JpdGViYWNrIGNvbm5lY3RvciwgYnV0IGtlZXAgdXNpbmcgYQo+VmlydHVhbCBlbmNvZGVyIHRv IGNvbm5lY3QgaXQgdG8gdGhlIENSVEM/IFdvdWxkbid0IGl0IG1ha2UgbW9yZSBzZW5zZQo+dG8g YWxzbyBhZGQgYSBXcml0ZWJhY2sgZW5jb2Rlcj8KPgoKT25seSB0aGF0IGEgd3JpdGViYWNrIGNv bm5lY3RvciBpcyBmdW5jdGlvbmFsbHkgYW5kIGNvbmNlcHR1YWxseSBxdWl0ZQpkaWZmZXJlbnQg ZnJvbSB0aGUgZXhpc3RpbmcgY29ubmVjdG9yIHR5cGVzLCB3aGVyZWFzIHRoZSAiZW5jb2RlciIK KHdoaWNoIHJlYWxpc3RpY2FsbHkgb25seSBleGlzdHMgYmVjYXVzZSB0aGUgZnJhbWV3b3JrIGZv cmNlcyBpdCB0bykKYWN0cyBwcmV0dHkgbXVjaCBsaWtlIGFueSBvdGhlci4KCj4+ICB9Owo+Pgo+ PiAgdm9pZCBkcm1fY29ubmVjdG9yX2lkYV9pbml0KHZvaWQpCj4+IEBAIC0yMzUsNyArMjM2LDgg QEAgaW50IGRybV9jb25uZWN0b3JfaW5pdChzdHJ1Y3QgZHJtX2RldmljZSAqZGV2LAo+PiAgCWxp c3RfYWRkX3RhaWwoJmNvbm5lY3Rvci0+aGVhZCwgJmNvbmZpZy0+Y29ubmVjdG9yX2xpc3QpOwo+ PiAgCWNvbmZpZy0+bnVtX2Nvbm5lY3RvcisrOwo+Pgo+PiAtCWlmIChjb25uZWN0b3JfdHlwZSAh PSBEUk1fTU9ERV9DT05ORUNUT1JfVklSVFVBTCkKPj4gKwlpZiAoKGNvbm5lY3Rvcl90eXBlICE9 IERSTV9NT0RFX0NPTk5FQ1RPUl9WSVJUVUFMKSAmJgo+PiArCSAgICAoY29ubmVjdG9yX3R5cGUg IT0gRFJNX01PREVfQ09OTkVDVE9SX1dSSVRFQkFDSykpCj4KPk5pdHBpY2s6IHlvdSBkb24ndCBu ZWVkIHRoZSBleHRyYSBwYXJlbnRoZXNpczoKPgo+CWlmIChjb25uZWN0b3JfdHlwZSAhPSBEUk1f TU9ERV9DT05ORUNUT1JfVklSVFVBTCAmJgo+CSAgICBjb25uZWN0b3JfdHlwZSAhPSBEUk1fTU9E RV9DT05ORUNUT1JfV1JJVEVCQUNLKQo+CgpZZWFoIGZhaXIgZW5vdWdoLCBJIGNhbiBkcm9wIHRo ZW0uCgo+PiAgCQlkcm1fb2JqZWN0X2F0dGFjaF9wcm9wZXJ0eSgmY29ubmVjdG9yLT5iYXNlLAo+ PiAgCQkJCQkgICAgICBjb25maWctPmVkaWRfcHJvcGVydHksCj4+ICAJCQkJCSAgICAgIDApOwo+ Cj4KPgo+PiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9kcm0vZHJtX2Nvbm5lY3Rvci5oIGIvaW5jbHVk ZS9kcm0vZHJtX2Nvbm5lY3Rvci5oCj4+IGluZGV4IDM0Zjk3NDEuLmRjNDkxMGQ2IDEwMDY0NAo+ PiAtLS0gYS9pbmNsdWRlL2RybS9kcm1fY29ubmVjdG9yLmgKPj4gKysrIGIvaW5jbHVkZS9kcm0v ZHJtX2Nvbm5lY3Rvci5oCj4+IEBAIC0yMTQsNiArMjE0LDE5IEBAIHN0cnVjdCBkcm1fY29ubmVj dG9yX3N0YXRlIHsKPj4gIAlzdHJ1Y3QgZHJtX2VuY29kZXIgKmJlc3RfZW5jb2RlcjsKPj4KPj4g IAlzdHJ1Y3QgZHJtX2F0b21pY19zdGF0ZSAqc3RhdGU7Cj4+ICsKPj4gKwkvKioKPj4gKwkgKiBA d3JpdGViYWNrX2pvYjogV3JpdGViYWNrIGpvYiBmb3Igd3JpdGViYWNrIGNvbm5lY3RvcnMKPj4g KwkgKgo+PiArCSAqIEhvbGRzIHRoZSBmcmFtZWJ1ZmZlciBmb3IgYSB3cml0ZWJhY2sgY29ubmVj dG9yLiBBcyB0aGUgd3JpdGViYWNrCj4+ICsJICogY29tcGxldGlvbiBtYXkgYmUgYXN5bmNocm9u b3VzIHRvIHRoZSBub3JtYWwgY29tbWl0IGN5Y2xlLCB0aGUKPj4gKwkgKiB3cml0ZWJhY2sgam9i IGxpZmV0aW1lIGlzIG1hbmFnZWQgc2VwYXJhdGVseSBmcm9tIHRoZSBub3JtYWwgYXRvbWljCj4+ ICsJICogc3RhdGUgYnkgdGhpcyBvYmplY3QuCj4+ICsJICoKPj4gKwkgKiBTZWUgYWxzbzogZHJt X3dyaXRlYmFja19xdWV1ZV9qb2IoKSBhbmQKPj4gKwkgKiBkcm1fd3JpdGViYWNrX3NpZ25hbF9j b21wbGV0aW9uKCkKPj4gKwkgKi8KPj4gKwlzdHJ1Y3QgZHJtX3dyaXRlYmFja19qb2IgKndyaXRl YmFja19qb2I7Cj4KPk1heWJlIEknbSB3cm9uZywgYnV0IGlzIGZlZWxzIHdlaXJkIHRvIGhhdmUg dGhlIHdyaXRlYmFja19qb2IgZmllbGQKPmRpcmVjdGx5IGVtYmVkZGVkIGluIGRybV9jb25uZWN0 b3Jfc3RhdGUsIHdoaWxlIGRybV93cml0ZWJhY2tfY29ubmVjdG9yCj5pbmhlcml0cyBmcm9tIGRy bV9jb25uZWN0b3IuCj4KPklNTywgZWl0aGVyIHlvdSBkZWNpZGUgdG8gZGlyZWN0bHkgcHV0IHRo ZSBkcm1fd3JpdGViYWNrX2Nvbm5lY3RvcidzCj5qb2JfeHh4IGZpZWxkcyBpbiBkcm1fY29ubmVj dG9yIGFuZCBrZWVwIHRoZSBkcm1fY29ubmVjdG9yX3N0YXRlIGFzIGlzLAo+b3IgeW91IGNyZWF0 ZSBhIGRybV93cml0ZWJhY2tfY29ubmVjdG9yX3N0YXRlIHdoaWNoIGluaGVyaXRzIGZyb20KPmRy bV9jb25uZWN0b3Jfc3RhdGUgYW5kIGVtYmVkcyB0aGUgd3JpdGViYWNrX2pvYiBmaWVsZC4KCkkg ZGlkIHNwZW5kIGEgZGVjZW50IGFtb3VudCBvZiB0aW1lIGxvb2tpbmcgYXQgdHJhY2tpbmcgdGhl IHdyaXRlYmFjawpzdGF0ZSBhbG9uZyB3aXRoIHRoZSBub3JtYWwgY29ubmVjdG9yIHN0YXRlLiBJ IGNvdWxkbid0IGNvbWUgdXAgd2l0aAphbnl0aGluZyBJIGxpa2VkLgoKQXMgdGhlIGNvbW1lbnQg bWVudGlvbnMsIG9uZSBvZiB0aGUgcHJvYmxlbXMgaXMgdGhhdCB5b3UgaGF2ZSB0byBtYWtlCnN1 cmUgdGhlIHJlbGV2YW50IHBhcnRzIG9mIHRoZSBjb25uZWN0b3Jfc3RhdGUgc3RheSBhcm91bmQg dW50aWwgdGhlCndyaXRlYmFjayBpcyBmaW5pc2hlZC4gVGhhdCBtZWFucyB5b3UndmUgZ290IHRv IGJsb2NrIGJlZm9yZQoic3dhcF9zdGF0ZSgpIiB1bnRpbCB0aGUgcHJldmlvdXMgd3JpdGViYWNr IGlzIGRvbmUsIGFuZCB0aGF0CmVmZmVjdGl2ZWx5IGxpbWl0cyB5b3VyIGZyYW1lIHJhdGUgdG8g cmVmcmVzaC8yLgoKVGhlIE1hbGktRFAgSFcgZG9lc24ndCBoYXZlIHRoYXQgbGltaXRhdGlvbiAt IHdlIGNhbiBxdWV1ZSB1cCBhIG5ldwpjb21taXQgd2hpbGUgdGhlIGN1cnJlbnQgd3JpdGViYWNr IGlzIG9uZ29pbmcuIEZvciB0aGF0IHJlYXNvbiBJCmRpZG4ndCB3YW50IHRvIGltcG9zZSBzdWNo IGEgbGltaXRhdGlvbiBpbiB0aGUgZnJhbWV3b3JrLgoKSW4gdjEgSSBhbGxvd2VkIHRoYXQgYnkg bWFraW5nIHRoZSBNYWxpLURQIGRyaXZlciBob2xkIGl0cyBvd24KcmVmZXJlbmNlcyB0byB0aGUg cmVsZXZhbnQgYml0cyBvZiB0aGUgc3RhdGUgZm9yIGFzIGxvbmcgYXMgaXQgbmVlZGVkCnRoZW0u IEluIHYzIEkgbW92ZWQgbW9zdCBvZiB0aGF0IGNvZGUgYmFjayB0byB0aGUgY29yZSAoaW4gcGFy dApiZWNhdXNlIEd1c3Rhdm8gZGlkbid0IGxpa2UgbWUgc2lnbmFsbGluZyB0aGUgRFJNLSJvd25l ZCIgZmVuY2UgZnJvbQpteSBkcml2ZXIgY29kZSBkaXJlY3RseSkuIEkgdGhpbmsgdGhlIG5ldyBh cHByb2FjaCBvZiAicXVldWVfam9iKCkiCmFuZCAic2lnbmFsX2pvYigpIiByZWR1Y2VzIHRoZSBh bW91bnQgb2YgdHJpY2t5IGNvZGUgaW4gZHJpdmVycywgYW5kCmlzIGdlbmVyYWxseSBtb3JlIGNs ZWFyIChhbHNvIGZhbWlsaWFyLCB3aGVuIGNvbXBhcmVkIHRvIHZzeW5jCmV2ZW50cykuCgpJJ20g Y2VydGFpbiB0aGVyZSdzIG90aGVyIHdheXMgdG8gZG8gaXQgKHJlZmNvdW50IGF0b21pYyBzdGF0 ZXM/KSwgYnV0Cml0IHNlZW1lZCBsaWtlIGEgYmlnZ2lzaCBvdmVyaGF1bCB0byBhY2hpZXZlIHdo YXQgd291bGQgYmFzaWNhbGx5IGJlCnRoZSBzYW1lIHRoaW5nLgoKSSB3YXMgZXhwZWN0aW5nIGVh Y2ggZHJpdmVyIHN1cHBvcnRpbmcgd3JpdGViYWNrIHRvIGhhdmUgaXRzIG93bgpkaWZmZXJlbnQg cmVxdWlyZW1lbnRzIGFyb3VuZCB3cml0ZWJhY2sgbGlmZXRpbWUvZHVyYXRpb24uIEZvciBleGFt cGxlCkkgdGhpbmsgVkM0IHNwZWNpZmljYWxseSBjYW1lIHVwLCBpbiB0aGF0IGl0cyB3cml0ZWJh Y2sgY291bGQgdGFrZQpzZXZlcmFsIGZyYW1lcywgd2hlcmVhcyBvbiBNYWxpLURQIHdlIGVpdGhl ciBmaW5pc2ggd2l0aGluIHRoZSBmcmFtZQpvciB3ZSBmYWlsLgoKTGV0dGluZyB0aGUgZHJpdmVy IG1hbmFnZSBpdHMgd3JpdGViYWNrX2pvYiBsaWZldGltZSBzZWVtZWQgbGlrZSBhCnJlYXNvbmFi bGUgd2F5IHRvIGhhbmRsZSBhbGwgdGhhdCwgd2l0aCB0aGUgZG9jdW1lbnRhdGlvbiBzdGF0aW5n IHRoZQpvbmx5IGJlaGF2aW91ciB3aGljaCBpcyBndWFyYW50ZWVkIHRvIHdvcmsgb24gYWxsIGRy aXZlcnM6CgogICAqICAgICBVc2Vyc3BhY2Ugc2hvdWxkIHdhaXQgZm9yIHRoaXMgZmVuY2UgdG8g c2lnbmFsIGJlZm9yZSBtYWtpbmcgYW5vdGhlcgogICAqICAgICBjb21taXQgYWZmZWN0aW5nIGFu eSBvZiB0aGUgc2FtZSBDUlRDcywgUGxhbmVzIG9yIENvbm5lY3RvcnMuCiAgICogICAgICoqRmFp bHVyZSB0byBkbyBzbyB3aWxsIHJlc3VsdCBpbiB1bmRlZmluZWQgYmVoYXZpb3VyLioqCiAgICog ICAgIEZvciB0aGlzIHJlYXNvbiBpdCBpcyBzdHJvbmdseSByZWNvbW1lbmRlZCB0aGF0IGFsbCB1 c2Vyc3BhY2UKICAgKiAgICAgYXBwbGljYXRpb25zIG1ha2luZyB1c2Ugb2Ygd3JpdGViYWNrIGNv bm5lY3RvcnMgKmFsd2F5cyogcmV0cmlldmUgYW4KICAgKiAgICAgb3V0LWZlbmNlIGZvciB0aGUg Y29tbWl0IGFuZCB1c2UgaXQgYXBwcm9wcmlhdGVseS4KCgoKLi4uIHNvIGFsbCBvZiB0aGF0IGlz IHdoeSB0aGUgX2pvYiBmaWVsZHMgZG9uJ3QgbGl2ZSBpbiBhICpfc3RhdGUKc3RydWN0dXJlIGRp cmVjdGx5LCBhbmQgaW5zdGVhZCBoYXZlIHRvIGxpdmUgaW4gdGhlIHNlcGFyYXRlbHktbWFuYWdl ZApzdHJ1Y3R1cmUgcG9pbnRlZCB0byBieSAtPndyaXRlYmFja19qb2IuCgpOb3csIEkgZGlkIGxv b2sgYXQgY3JlYXRpbmcgZHJtX3dyaXRlYmFja19jb25uZWN0b3Jfc3RhdGUsIGJ1dCBhcyBpdAp3 b3VsZCBvbmx5IGJlIGhvbGRpbmcgdGhlIGpvYiBwb2ludGVyIChzZWUgYWJvdmUpIGl0IGRpZG4n dCBzZWVtIHdvcnRoCnNjYXR0ZXJpbmcgYXJvdW5kIHRoZQoKICAgIGlmIChjb25uX3N0YXRlLT5j b25uZWN0b3ItPmNvbm5lY3Rvcl90eXBlID09CiAgICAgICAgRFJNX01PREVfQ09OTkVDVE9SX1dS SVRFQkFDSykKCmNoZWNrcyBldmVyeXdoZXJlIGJlZm9yZSB1cC1jYXN0aW5nIC0ge2NsZWFyLHJl c2V0LGR1cGxpY2F0ZX1fc3RhdGUoKSwKcHJlcGFyZV9zaWduYWxsaW5nKCksIGNvbXBsZXRlX3Np Z25hbGxpbmcoKSwgZXRjLiBJdCBqdXN0IHRvdWNoZWQgYQpsb3Qgb2YgY29kZSBmb3IgdGhlIHNh a2Ugb2YgYW4gZXh0cmEgcG9pbnRlciBmaWVsZCBpbiBlYWNoIGNvbm5lY3RvcgpzdGF0ZS4KCkkg Y2FuIGVhc2lseSByZXZpc2l0IHRoYXQgcGFydCBpZiB5b3UgbGlrZS4KCj4KPkFueXdheSwgd2Fp dCBmb3IgRGFuaWVsJ3MgZmVlZGJhY2sgYmVmb3JlIGRvaW5nIHRoaXMgY2hhbmdlLgo+CgpBbSBJ IGV4cGVjdGluZyBzb21lIG1vcmUgZmVlZGJhY2sgZnJvbSBEYW5pZWw/Cgo+PiAgfTsKPj4KPj4g IC8qKgo+PiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9kcm0vZHJtX21vZGVfY29uZmlnLmggYi9pbmNs dWRlL2RybS9kcm1fbW9kZV9jb25maWcuaAo+PiBpbmRleCBiZjk5OTFiMi4uM2QzZDA3ZiAxMDA2 NDQKPj4gLS0tIGEvaW5jbHVkZS9kcm0vZHJtX21vZGVfY29uZmlnLmgKPj4gKysrIGIvaW5jbHVk ZS9kcm0vZHJtX21vZGVfY29uZmlnLmgKPj4gQEAgLTYzNCw2ICs2MzQsMjAgQEAgc3RydWN0IGRy bV9tb2RlX2NvbmZpZyB7Cj4+ICAJICovCj4+ICAJc3RydWN0IGRybV9wcm9wZXJ0eSAqc3VnZ2Vz dGVkX3lfcHJvcGVydHk7Cj4+Cj4+ICsJLyoqCj4+ICsJICogQHdyaXRlYmFja19mYl9pZF9wcm9w ZXJ0eTogUHJvcGVydHkgZm9yIHdyaXRlYmFjayBjb25uZWN0b3JzLCBzdG9yaW5nCj4+ICsJICog dGhlIElEIG9mIHRoZSBvdXRwdXQgZnJhbWVidWZmZXIuCj4+ICsJICogU2VlIGFsc286IGRybV93 cml0ZWJhY2tfY29ubmVjdG9yX2luaXQoKQo+PiArCSAqLwo+PiArCXN0cnVjdCBkcm1fcHJvcGVy dHkgKndyaXRlYmFja19mYl9pZF9wcm9wZXJ0eTsKPj4gKwkvKioKPj4gKwkgKiBAd3JpdGViYWNr X3BpeGVsX2Zvcm1hdHNfcHJvcGVydHk6IFByb3BlcnR5IGZvciB3cml0ZWJhY2sgY29ubmVjdG9y cywKPj4gKwkgKiBzdG9yaW5nIGFuIGFycmF5IG9mIHRoZSBzdXBwb3J0ZWQgcGl4ZWwgZm9ybWF0 cyBmb3IgdGhlIHdyaXRlYmFjawo+PiArCSAqIGVuZ2luZSAocmVhZC1vbmx5KS4KPj4gKwkgKiBT ZWUgYWxzbzogZHJtX3dyaXRlYmFja19jb25uZWN0b3JfaW5pdCgpCj4+ICsJICovCj4+ICsJc3Ry dWN0IGRybV9wcm9wZXJ0eSAqd3JpdGViYWNrX3BpeGVsX2Zvcm1hdHNfcHJvcGVydHk7Cj4+ICsK Pj4gIAkvKiBkdW1iIGlvY3RsIHBhcmFtZXRlcnMgKi8KPj4gIAl1aW50MzJfdCBwcmVmZXJyZWRf ZGVwdGgsIHByZWZlcl9zaGFkb3c7Cj4+Cj4+IGRpZmYgLS1naXQgYS9pbmNsdWRlL2RybS9kcm1f d3JpdGViYWNrLmggYi9pbmNsdWRlL2RybS9kcm1fd3JpdGViYWNrLmgKPj4gbmV3IGZpbGUgbW9k ZSAxMDA2NDQKPj4gaW5kZXggMDAwMDAwMC4uNmIyYWM0NQo+PiAtLS0gL2Rldi9udWxsCj4+ICsr KyBiL2luY2x1ZGUvZHJtL2RybV93cml0ZWJhY2suaAo+PiBAQCAtMCwwICsxLDc4IEBACj4+ICsv Kgo+PiArICogKEMpIENPUFlSSUdIVCAyMDE2IEFSTSBMaW1pdGVkLiBBbGwgcmlnaHRzIHJlc2Vy dmVkLgo+PiArICogQXV0aG9yOiBCcmlhbiBTdGFya2V5IDxicmlhbi5zdGFya2V5QGFybS5jb20+ Cj4+ICsgKgo+PiArICogVGhpcyBwcm9ncmFtIGlzIGZyZWUgc29mdHdhcmUgYW5kIGlzIHByb3Zp ZGVkIHRvIHlvdSB1bmRlciB0aGUgdGVybXMgb2YgdGhlCj4+ICsgKiBHTlUgR2VuZXJhbCBQdWJs aWMgTGljZW5zZSB2ZXJzaW9uIDIgYXMgcHVibGlzaGVkIGJ5IHRoZSBGcmVlIFNvZnR3YXJlCj4+ ICsgKiBGb3VuZGF0aW9uLCBhbmQgYW55IHVzZSBieSB5b3Ugb2YgdGhpcyBwcm9ncmFtIGlzIHN1 YmplY3QgdG8gdGhlIHRlcm1zCj4+ICsgKiBvZiBzdWNoIEdOVSBsaWNlbmNlLgo+PiArICovCj4+ ICsKPj4gKyNpZm5kZWYgX19EUk1fV1JJVEVCQUNLX0hfXwo+PiArI2RlZmluZSBfX0RSTV9XUklU RUJBQ0tfSF9fCj4+ICsjaW5jbHVkZSA8ZHJtL2RybV9jb25uZWN0b3IuaD4KPj4gKyNpbmNsdWRl IDxsaW51eC93b3JrcXVldWUuaD4KPj4gKwo+PiArc3RydWN0IGRybV93cml0ZWJhY2tfY29ubmVj dG9yIHsKPj4gKwlzdHJ1Y3QgZHJtX2Nvbm5lY3RvciBiYXNlOwo+Cj5BRkFJVSwgYSB3cml0ZWJh Y2sgY29ubmVjdG9yIHdpbGwgYWx3YXlzIHJlcXVpcmUgYW4gJ2R1bW15JyBlbmNvZGVyIHRvCj5t YWtlIHRoZSBEUk0gZnJhbWV3b3JrIGhhcHB5IChBRkFJSywgYSBjb25uZWN0b3IgaXMgYWx3YXlz IGNvbm5lY3RlZCB0bwo+YSBDUlRDIHRocm91Z2ggYW4gZW5jb2RlcikuCj4KPldvdWxkbid0IGl0 IG1ha2UgbW9yZSBzZW5zZSB0byBoYXZlIGEgZHJtX2VuY29kZXIgb2JqZWN0IGVtYmVkZGVkIGlu Cj5kcm1fd3JpdGViYWNrX2Nvbm5lY3RvciBzbyB0aGF0IHBlb3BsZSBkb24ndCBoYXZlIHRvIGRl Y2xhcmUgYW4gZXh0cmEKPnN0cnVjdHVyZSBjb250YWluaW5nIGJvdGggdGhlIGRybV93cml0ZWJh Y2tfY29ubmVjdG9yIGNvbm5lY3RvciBhbmQgYQo+ZHJtX2VuY29kZXI/IElzIHRoZXJlIGEgZ29v ZCByZWFzb24gdG8ga2VlcCB0aGVtIHNlcGFyYXRlPwo+CgpZZWFoIHRoYXQncyBub3QgYSBiYWQg aWRlYS4gVGhlIGVuY29kZXIgZnVuY3MgY291bGQgYmUgcGFzc2VkIGluIHRvCmRybV93cml0ZWJh Y2tfY29ubmVjdG9yX2luaXQoKSAoaW4gd2hpY2ggY2FzZSBhZGRpbmcgYSB3cml0ZWJhY2sKZW5j b2RlciB0eXBlIHdvdWxkIGFsc28gbWFrZSBzZW5zZSkuCgpUaGFua3MgZm9yIHRoZSByZXZpZXcs CgotQnJpYW4KCj4+ICsKPj4gKwkvKioKPj4gKwkgKiBAcGl4ZWxfZm9ybWF0c19ibG9iX3B0cjoK Pj4gKwkgKgo+PiArCSAqIERSTSBibG9iIHByb3BlcnR5IGRhdGEgZm9yIHRoZSBwaXhlbCBmb3Jt YXRzIGxpc3Qgb24gd3JpdGViYWNrCj4+ICsJICogY29ubmVjdG9ycwo+PiArCSAqIFNlZSBhbHNv IGRybV93cml0ZWJhY2tfY29ubmVjdG9yX2luaXQoKQo+PiArCSAqLwo+PiArCXN0cnVjdCBkcm1f cHJvcGVydHlfYmxvYiAqcGl4ZWxfZm9ybWF0c19ibG9iX3B0cjsKPj4gKwo+PiArCS8qKiBAam9i X2xvY2s6IFByb3RlY3RzIGpvYl9xdWV1ZSAqLwo+PiArCXNwaW5sb2NrX3Qgam9iX2xvY2s7Cj4+ ICsJLyoqCj4+ICsJICogQGpvYl9xdWV1ZToKPj4gKwkgKgo+PiArCSAqIEhvbGRzIGEgbGlzdCBv ZiBhIGNvbm5lY3RvcidzIHdyaXRlYmFjayBqb2JzOyB0aGUgbGFzdCBpdGVtIGlzIHRoZQo+PiAr CSAqIG1vc3QgcmVjZW50LiBUaGUgZmlyc3QgaXRlbSBtYXkgYmUgZWl0aGVyIHdhaXRpbmcgZm9y IHRoZSBoYXJkd2FyZQo+PiArCSAqIHRvIGJlZ2luIHdyaXRpbmcsIG9yIGN1cnJlbnRseSBiZWlu ZyB3cml0dGVuLgo+PiArCSAqCj4+ICsJICogU2VlIGFsc286IGRybV93cml0ZWJhY2tfcXVldWVf am9iKCkgYW5kCj4+ICsJICogZHJtX3dyaXRlYmFja19zaWduYWxfY29tcGxldGlvbigpCj4+ICsJ ICovCj4+ICsJc3RydWN0IGxpc3RfaGVhZCBqb2JfcXVldWU7Cj4+ICt9OwpfX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0 CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3Rv cC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from foss.arm.com ([217.140.101.70]:59258 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752817AbdDRRew (ORCPT ); Tue, 18 Apr 2017 13:34:52 -0400 Date: Tue, 18 Apr 2017 18:34:43 +0100 From: Brian Starkey To: Boris Brezillon Cc: 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: <20170418173443.GA325@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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20170414120823.2cafc748@bbrezillon> Sender: linux-media-owner@vger.kernel.org List-ID: Hi Boris, On Fri, Apr 14, 2017 at 12:08:23PM +0200, Boris Brezillon wrote: >On Fri, 25 Nov 2016 16:48:59 +0000 >Brian Starkey wrote: > > >> >> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c >> index b5c6a8e..6bbd93f 100644 >> --- a/drivers/gpu/drm/drm_connector.c >> +++ b/drivers/gpu/drm/drm_connector.c >> @@ -86,6 +86,7 @@ struct drm_conn_prop_enum_list { >> { DRM_MODE_CONNECTOR_VIRTUAL, "Virtual" }, >> { DRM_MODE_CONNECTOR_DSI, "DSI" }, >> { DRM_MODE_CONNECTOR_DPI, "DPI" }, >> + { DRM_MODE_CONNECTOR_WRITEBACK, "Writeback" }, > >Is there a reason we have a Writeback connector, but keep using a >Virtual encoder to connect it to the CRTC? Wouldn't it make more sense >to also add a Writeback encoder? > Only that a writeback connector is functionally and conceptually quite different from the existing connector types, whereas the "encoder" (which realistically only exists because the framework forces it to) acts pretty much like any other. >> }; >> >> void drm_connector_ida_init(void) >> @@ -235,7 +236,8 @@ int drm_connector_init(struct drm_device *dev, >> list_add_tail(&connector->head, &config->connector_list); >> config->num_connector++; >> >> - if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL) >> + if ((connector_type != DRM_MODE_CONNECTOR_VIRTUAL) && >> + (connector_type != DRM_MODE_CONNECTOR_WRITEBACK)) > >Nitpick: you don't need the extra parenthesis: > > if (connector_type != DRM_MODE_CONNECTOR_VIRTUAL && > connector_type != DRM_MODE_CONNECTOR_WRITEBACK) > Yeah fair enough, I can drop them. >> drm_object_attach_property(&connector->base, >> config->edid_property, >> 0); > > > >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h >> index 34f9741..dc4910d6 100644 >> --- a/include/drm/drm_connector.h >> +++ b/include/drm/drm_connector.h >> @@ -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. > >Anyway, wait for Daniel's feedback before doing this change. > Am I expecting some more feedback from Daniel? >> }; >> >> /** >> 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). Thanks for the review, -Brian >> + >> + /** >> + * @pixel_formats_blob_ptr: >> + * >> + * DRM blob property data for the pixel formats list on writeback >> + * connectors >> + * See also drm_writeback_connector_init() >> + */ >> + struct drm_property_blob *pixel_formats_blob_ptr; >> + >> + /** @job_lock: Protects job_queue */ >> + spinlock_t job_lock; >> + /** >> + * @job_queue: >> + * >> + * Holds a list of a connector's writeback jobs; the last item is the >> + * most recent. The first item may be either waiting for the hardware >> + * to begin writing, or currently being written. >> + * >> + * See also: drm_writeback_queue_job() and >> + * drm_writeback_signal_completion() >> + */ >> + struct list_head job_queue; >> +};