From mboxrd@z Thu Jan 1 00:00:00 1970 From: jilaiw@codeaurora.org Subject: Re: [PATCH 2/3] drm:msm: Initial Add Writeback Support Date: Thu, 2 Apr 2015 18:07:22 -0000 Message-ID: <3b757ab0549f700dafb87abb2cda3ac5.squirrel@www.codeaurora.org> References: <1427922770-13721-1-git-send-email-jilaiw@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Emil Velikov Cc: linux-arm-msm , "Linux-Kernel@Vger. Kernel. Org" , ML dri-devel List-Id: linux-arm-msm@vger.kernel.org VGhhbmtzIEVtaWwuIFBsZWFzZSBjaGVjayB0aGUgY29tbWVudHMgZW1iZWRkZWQgYW5kIGZvciB0 aGUgcmVzdCBJIHdpbGwKdXBkYXRlIHRoZSBjb2RlLgoKPiBIaSBKaWxhaSwKPgo+IEp1c3QgYSBm ZXcgcXVlc3Rpb25zLCBub3QgcmVhbGx5IGEgcmV2aWV3IGFzIEknbSBub3QgdGhhdCBmYW1pbGlh cgo+IHdpdGggdGhlIGNvZGUuCj4KPgo+PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vbXNtL0tjb25m aWcKPj4gQEAgLTI3LDYgKzI3LDE2IEBAIGNvbmZpZyBEUk1fTVNNX0ZCREVWCj4+ICAgICAgICAg ICBzdXBwb3J0LiBOb3RlIHRoYXQgdGhpcyBzdXBwb3J0IGFsc28gcHJvdmlkZSB0aGUgbGludXgg Y29uc29sZQo+PiAgICAgICAgICAgc3VwcG9ydCBvbiB0b3Agb2YgdGhlIE1TTSBtb2Rlc2V0dGlu ZyBkcml2ZXIuCj4+Cj4+ICtjb25maWcgRFJNX01TTV9XQgo+PiArICAgICAgIGJvb2wgIkVuYWJs ZSB3cml0ZWJhY2sgc3VwcG9ydCBmb3IgTVNNIG1vZGVzZXR0aW5nIGRyaXZlciIKPj4gKyAgICAg ICBkZXBlbmRzIG9uIERSTV9NU00KPj4gKyAgICAgICBkZXBlbmRzIG9uIFZJREVPX1Y0TDIKPj4g KyAgICAgICBzZWxlY3QgVklERU9CVUYyX0NPUkUKPj4gKyAgICAgICBkZWZhdWx0IHkKPj4gKyAg ICAgICBoZWxwCj4+ICsgICAgICAgICBDaG9vc2UgdGhpcyBvcHRpb24gaWYgeW91IGhhdmUgYSBu ZWVkIHRvIHN1cHBvcnQgd3JpdGViYWNrCj4+ICsgICAgICAgICBjb25uZWN0b3IuCj4+ICsKPiBJ cyBpdCB3b3J0aCBtZW50aW9uaW5nIHdoaWNoIGRldmljZXMvU29DcyBoYXZlIHN1Y2ggY29ubmVj dG9yID8KSWYgdGhlIGRldmljZXMgaGF2ZSBXQiBjb25uZWN0b3IsIGl0IHdpbGwgYmUgYWRkZWQg aW4gdGhlIGRldmljZSB0cmVlLgoKPgo+Cj4+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9tc20vTWFr ZWZpbGUKPj4gQEAgLTEsNCArMSw1IEBACj4+IC1jY2ZsYWdzLXkgOj0gLUlpbmNsdWRlL2RybSAt SWRyaXZlcnMvZ3B1L2RybS9tc20KPj4gK2NjZmxhZ3MteSA6PSAtSWluY2x1ZGUvZHJtIC1JZHJp dmVycy9ncHUvZHJtL21zbQo+PiAtSWRyaXZlcnMvZ3B1L2RybS9tc20vbWRwX3diCj4+ICtjY2Zs YWdzLSQoQ09ORklHX0RSTV9NU01fV0IpICs9IC1JZHJpdmVycy9ncHUvZHJtL21zbS9tZHAvbWRw X3diCj4+Cj4gSSB0aGluayB5b3Ugb25seSB3YW50IHRoZSBzZWNvbmQgbGluZSBoZXJlLgo+Cj4K Pj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL21zbS9tZHAvbWRwNS9tZHA1X3diX2VuY29kZXIuYwo+ Cj4+ICtzdGF0aWMgc3RydWN0IG1zbV9idXNfcGF0aHMgbWRwX2J1c191c2VjYXNlc1tdID0geyB7 Cj4+ICsgICAgICAgICAgICAgICAubnVtX3BhdGhzID0gMSwKPj4gKyAgICAgICAgICAgICAgIC52 ZWN0b3JzID0gJm1kcF9idXNfdmVjdG9yc1swXSwKPj4gK30sIHsKPj4gKyAgICAgICAgICAgICAg IC5udW1fcGF0aHMgPSAxLAo+PiArICAgICAgICAgICAgICAgLnZlY3RvcnMgPSAmbWRwX2J1c192 ZWN0b3JzWzFdLAo+PiArfSB9Owo+IFRoZSBmb2xsb3dpbmcgZm9ybWF0dGluZyBzZWVtcyBtb3Jl IGNvbW1vbjoKPgo+IHN0YXRpYyBzdHJ1Y3QgZm9vIGZvbzFbXSA9IHsKPiAgICAgewo+ICAgICAg ICAgYmFyCj4gICAgICB9Cj4gfTsKPgo+Cj4+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9tc20vbWRw L21kcF93Yi9tZHBfd2IuYwo+Cj4+ICtpbnQgbXNtX3diX21vZGVzZXRfaW5pdChzdHJ1Y3QgbXNt X3diICp3YiwKPj4gKyAgICAgICBzdHJ1Y3QgZHJtX2RldmljZSAqZGV2LCBzdHJ1Y3QgZHJtX2Vu Y29kZXIgKmVuY29kZXIpCj4+ICt7Cj4+ICsgICAgICAgc3RydWN0IG1zbV9kcm1fcHJpdmF0ZSAq cHJpdiA9IGRldi0+ZGV2X3ByaXZhdGU7Cj4+ICsgICAgICAgaW50IHJldDsKPj4gKwo+PiArICAg ICAgIHdiLT5kZXYgPSBkZXY7Cj4+ICsgICAgICAgd2ItPmVuY29kZXIgPSBlbmNvZGVyOwo+PiAr Cj4+ICsgICAgICAgd2ItPmNvbm5lY3RvciA9IG1zbV93Yl9jb25uZWN0b3JfaW5pdCh3Yik7Cj4+ ICsgICAgICAgaWYgKElTX0VSUih3Yi0+Y29ubmVjdG9yKSkgewo+PiArICAgICAgICAgICAgICAg cmV0ID0gUFRSX0VSUih3Yi0+Y29ubmVjdG9yKTsKPj4gKyAgICAgICAgICAgICAgIGRldl9lcnIo ZGV2LT5kZXYsICJmYWlsZWQgdG8gY3JlYXRlIFdCIGNvbm5lY3RvcjogJWRcbiIsCj4+IHJldCk7 Cj4+ICsgICAgICAgICAgICAgICB3Yi0+Y29ubmVjdG9yID0gTlVMTDsKPj4gKyAgICAgICAgICAg ICAgIGdvdG8gZmFpbDsKPj4gKyAgICAgICB9Cj4+ICsKPj4gKyAgICAgICBwcml2LT5jb25uZWN0 b3JzW3ByaXYtPm51bV9jb25uZWN0b3JzKytdID0gd2ItPmNvbm5lY3RvcjsKPj4gKwo+PiArICAg ICAgIHJldHVybiAwOwo+PiArCj4+ICtmYWlsOgo+PiArICAgICAgIGlmICh3Yi0+Y29ubmVjdG9y KSB7Cj4+ICsgICAgICAgICAgICAgICB3Yi0+Y29ubmVjdG9yLT5mdW5jcy0+ZGVzdHJveSh3Yi0+ Y29ubmVjdG9yKTsKPj4gKyAgICAgICAgICAgICAgIHdiLT5jb25uZWN0b3IgPSBOVUxMOwo+PiAr ICAgICAgIH0KPj4gKwo+IERyb3AgdGhlIHVudXNlZCBpZiBibG9jayA/Cj4KPgo+PiArc3RhdGlj IHN0cnVjdCBtc21fd2IgKm1zbV93Yl9pbml0KHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYp Cj4+ICt7Cj4+ICsgICAgICAgc3RydWN0IG1zbV93YiAqd2IgPSBOVUxMOwo+PiArCj4+ICsgICAg ICAgd2IgPSBkZXZtX2t6YWxsb2MoJnBkZXYtPmRldiwgc2l6ZW9mKCp3YiksIEdGUF9LRVJORUwp Owo+PiArICAgICAgIGlmICghd2IpCj4+ICsgICAgICAgICAgICAgICByZXR1cm4gRVJSX1BUUigt RU5PTUVNKTsKPj4gKwo+PiArICAgICAgIHdiLT5wZGV2ID0gcGRldjsKPj4gKyAgICAgICB3Yi0+ cHJpdl9kYXRhID0gZGV2bV9remFsbG9jKCZwZGV2LT5kZXYsIHNpemVvZigqd2ItPnByaXZfZGF0 YSksCj4+ICsgICAgICAgICAgICAgICBHRlBfS0VSTkVMKTsKPj4gKyAgICAgICBpZiAoIXdiLT5w cml2X2RhdGEpCj4+ICsgICAgICAgICAgICAgICByZXR1cm4gRVJSX1BUUigtRU5PTUVNKTsKPj4g Kwo+PiArICAgICAgIGlmIChtc21fd2JfdjRsMl9pbml0KHdiKSkgewo+PiArICAgICAgICAgICAg ICAgcHJfZXJyKCIlczogd2IgdjRsMiBpbml0IGZhaWxlZFxuIiwgX19mdW5jX18pOwo+PiArICAg ICAgICAgICAgICAgcmV0dXJuIEVSUl9QVFIoLUVOT0RFVik7Cj4+ICsgICAgICAgfQo+PiArCj4g U2VlbXMgbGlrZSB3ZSdyZSBsZWFraW5nIHdiIGFuZC9vciB3Yi0+cHJpdl9kYXRhLiBBZGQgYSBs YWJlbCBhbmQKPiBjb25zb2xpZGF0ZSBlcnJvciBoYW5kbGluZyBpbiB0aGVyZSA/CgpTaW5jZSB0 aGUgZGV2bV9remFsbG9jIGZ1bmN0aW9uIGlzIHVzZWQgaGVyZSwgdGhlIHN5c3RlbSBzaG91bGQg dGFrZSBjYXJlCmZyZWVpbmcgdGhlIG1lbW9yeS4KPgo+Cj4+ICsrKyBiL2RyaXZlcnMvZ3B1L2Ry bS9tc20vbWRwL21kcF93Yi9tZHBfd2IuaAo+Cj4+ICsjaWZuZGVmIF9fV0JfQ09OTkVDVE9SX0hf Xwo+PiArI2RlZmluZSBfX1dCX0NPTk5FQ1RPUl9IX18KPj4gKwo+IFRoZSBmaWxlIGlzIGNhbGxl ZCBtZHBfd2IuaCwgc28gb25lIG1pZ2h0IHdhbnQgdG8gY2hhbmdlIHRoaXMgdG8KPiBfX01EUF9X Ql9IX18KPgo+Cj4+IC0tLSAvZGV2L251bGwKPj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL21zbS9t ZHAvbWRwX3diL21kcF93Yl92NGwyLmMKPj4gQEAgLTAsMCArMSw1MjIgQEAKPgo+PiArc3RhdGlj IGNvbnN0IHN0cnVjdCBtc21fd2JfZm10ICpnZXRfZm9ybWF0KHUzMiBmb3VyY2MpCj4+ICt7Cj4+ ICsgICAgICAgY29uc3Qgc3RydWN0IG1zbV93Yl9mbXQgKmZtdDsKPj4gKyAgICAgICB1bnNpZ25l ZCBpbnQgazsKPj4gKwo+PiArICAgICAgIGZvciAoayA9IDA7IGsgPCBBUlJBWV9TSVpFKGZvcm1h dHMpOyBrKyspIHsKPj4gKyAgICAgICAgICAgICAgIGZtdCA9ICZmb3JtYXRzW2tdOwo+PiArICAg ICAgICAgICAgICAgaWYgKGZtdC0+Zm91cmNjID09IGZvdXJjYykKPj4gKyAgICAgICAgICAgICAg ICAgICAgICAgYnJlYWs7Cj4+ICsgICAgICAgfQo+PiArCj4+ICsgICAgICAgaWYgKGsgPT0gQVJS QVlfU0laRShmb3JtYXRzKSkKPj4gKyAgICAgICAgICAgICAgIHJldHVybiBOVUxMOwo+PiArCj4+ ICsgICAgICAgcmV0dXJuICZmb3JtYXRzW2tdOwo+IFlvdSBjb3VsZCBtb3ZlIHRoZSByZXR1cm4g d2l0aGluIHRoZSBsb29wLCBhbmQgZHJvcCB0aGUgZm9sbG93IHVwCj4gY29uZGl0aW9uYWwuCj4K Pgo+PiArc3RhdGljIHZvaWQgKm1zbV93Yl92YjJfYXR0YWNoX2RtYWJ1Zih2b2lkICphbGxvY19j dHgsIHN0cnVjdCBkbWFfYnVmCj4+ICpkYnVmLAo+PiArICAgICAgIHVuc2lnbmVkIGxvbmcgc2l6 ZSwgaW50IHdyaXRlKQo+PiArewo+PiArICAgICAgIHN0cnVjdCBtc21fd2JfdjRsMl9kZXYgKmRl diA9IGFsbG9jX2N0eDsKPj4gKyAgICAgICBzdHJ1Y3QgZHJtX2RldmljZSAqZHJtX2RldiA9IGRl di0+d2ItPmRldjsKPj4gKyAgICAgICBzdHJ1Y3QgZHJtX2dlbV9vYmplY3QgKm9iajsKPj4gKwo+ PiArICAgICAgIG11dGV4X2xvY2soJmRybV9kZXYtPm9iamVjdF9uYW1lX2xvY2spOwo+PiArICAg ICAgIG9iaiA9IGRybV9kZXYtPmRyaXZlci0+Z2VtX3ByaW1lX2ltcG9ydChkcm1fZGV2LCBkYnVm KTsKPj4gKyAgICAgICBpZiAoV0FSTl9PTighb2JqKSkgewo+PiArICAgICAgICAgICAgICAgbXV0 ZXhfdW5sb2NrKCZkcm1fZGV2LT5vYmplY3RfbmFtZV9sb2NrKTsKPj4gKyAgICAgICAgICAgICAg IHY0bDJfZXJyKCZkZXYtPnY0bDJfZGV2LCAiQ2FuJ3QgY29udmVydCBkbWFidWYgdG8gZ2VtCj4+ IG9iai5cbiIpOwo+PiArICAgICAgICAgICAgICAgcmV0dXJuIE5VTEw7Cj4gU2hvdWxkbid0IG9u ZSByZXR1cm4gRVJSX1BUUiBoZXJlID8gQ29uc29saWRhdGluZyB0aGUgZXJyb3IgcGF0aHMgdG8g YQo+IHNpbmdsZSBsYWJlbCB3aWxsIGJlIGNsZWFuZXIgaW1oby4KPgo+Cj4+IC0tLSBhL2RyaXZl cnMvZ3B1L2RybS9tc20vbXNtX2Rydi5oCj4+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9tc20vbXNt X2Rydi5oCj4KPj4gQEAgLTIyNCwxOCArMjI5LDI4IEBAIHN0cnVjdCBkcm1fZnJhbWVidWZmZXIK Pj4gKm1zbV9mcmFtZWJ1ZmZlcl9jcmVhdGUoc3RydWN0IGRybV9kZXZpY2UgKmRldiwKPj4KPj4g IHN0cnVjdCBkcm1fZmJfaGVscGVyICptc21fZmJkZXZfaW5pdChzdHJ1Y3QgZHJtX2RldmljZSAq ZGV2KTsKPj4KPj4gLXN0cnVjdCBoZG1pOwo+PiAgaW50IGhkbWlfbW9kZXNldF9pbml0KHN0cnVj dCBoZG1pICpoZG1pLCBzdHJ1Y3QgZHJtX2RldmljZSAqZGV2LAo+PiAgICAgICAgICAgICAgICAg c3RydWN0IGRybV9lbmNvZGVyICplbmNvZGVyKTsKPj4gIHZvaWQgX19pbml0IGhkbWlfcmVnaXN0 ZXIodm9pZCk7Cj4+ICB2b2lkIF9fZXhpdCBoZG1pX3VucmVnaXN0ZXIodm9pZCk7Cj4+Cj4+IC1z dHJ1Y3QgbXNtX2VkcDsKPiBVbnJlbGF0ZWQgY29zbWV0aWMgY2hhbmdlcyA/Cj4KPgo+PiAtLS0g YS9kcml2ZXJzL2dwdS9kcm0vbXNtL21zbV9nZW0uYwo+PiArKysgYi9kcml2ZXJzL2dwdS9kcm0v bXNtL21zbV9nZW0uYwo+PiBAQCAtNTM0LDYgKzUzNCw3IEBAIHZvaWQgbXNtX2dlbV9mcmVlX29i amVjdChzdHJ1Y3QgZHJtX2dlbV9vYmplY3QgKm9iaikKPj4gICAgICAgICAgICAgICAgIGlmICht c21fb2JqLT5wYWdlcykKPj4gICAgICAgICAgICAgICAgICAgICAgICAgZHJtX2ZyZWVfbGFyZ2Uo bXNtX29iai0+cGFnZXMpOwo+Pgo+PiArICAgICAgICAgICAgICAgZHJtX3ByaW1lX2dlbV9kZXN0 cm95KG9iaiwgbXNtX29iai0+c2d0KTsKPiBTaG91bGQgdGhpcyBmaXggYmUgYSBzZXBhcmF0ZSBw YXRjaCA/Cj4KPgo+IENoZWVycywKPiBFbWlsCj4KCgpfX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBs aXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFu L2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753354AbbDBSHZ (ORCPT ); Thu, 2 Apr 2015 14:07:25 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:53704 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752555AbbDBSHX (ORCPT ); Thu, 2 Apr 2015 14:07:23 -0400 Message-ID: <3b757ab0549f700dafb87abb2cda3ac5.squirrel@www.codeaurora.org> In-Reply-To: References: <1427922770-13721-1-git-send-email-jilaiw@codeaurora.org> Date: Thu, 2 Apr 2015 18:07:22 -0000 Subject: Re: [PATCH 2/3] drm:msm: Initial Add Writeback Support From: jilaiw@codeaurora.org To: "Emil Velikov" Cc: "Jilai Wang" , "ML dri-devel" , "linux-arm-msm" , "Linux-Kernel@Vger. Kernel. Org" User-Agent: SquirrelMail/1.4.22-4.el6 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7BIT X-Priority: 3 (Normal) Importance: Normal Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Thanks Emil. Please check the comments embedded and for the rest I will update the code. > Hi Jilai, > > Just a few questions, not really a review as I'm not that familiar > with the code. > > >> +++ b/drivers/gpu/drm/msm/Kconfig >> @@ -27,6 +27,16 @@ config DRM_MSM_FBDEV >> support. Note that this support also provide the linux console >> support on top of the MSM modesetting driver. >> >> +config DRM_MSM_WB >> + bool "Enable writeback support for MSM modesetting driver" >> + depends on DRM_MSM >> + depends on VIDEO_V4L2 >> + select VIDEOBUF2_CORE >> + default y >> + help >> + Choose this option if you have a need to support writeback >> + connector. >> + > Is it worth mentioning which devices/SoCs have such connector ? If the devices have WB connector, it will be added in the device tree. > > >> +++ b/drivers/gpu/drm/msm/Makefile >> @@ -1,4 +1,5 @@ >> -ccflags-y := -Iinclude/drm -Idrivers/gpu/drm/msm >> +ccflags-y := -Iinclude/drm -Idrivers/gpu/drm/msm >> -Idrivers/gpu/drm/msm/mdp_wb >> +ccflags-$(CONFIG_DRM_MSM_WB) += -Idrivers/gpu/drm/msm/mdp/mdp_wb >> > I think you only want the second line here. > > >> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_wb_encoder.c > >> +static struct msm_bus_paths mdp_bus_usecases[] = { { >> + .num_paths = 1, >> + .vectors = &mdp_bus_vectors[0], >> +}, { >> + .num_paths = 1, >> + .vectors = &mdp_bus_vectors[1], >> +} }; > The following formatting seems more common: > > static struct foo foo1[] = { > { > bar > } > }; > > >> +++ b/drivers/gpu/drm/msm/mdp/mdp_wb/mdp_wb.c > >> +int msm_wb_modeset_init(struct msm_wb *wb, >> + struct drm_device *dev, struct drm_encoder *encoder) >> +{ >> + struct msm_drm_private *priv = dev->dev_private; >> + int ret; >> + >> + wb->dev = dev; >> + wb->encoder = encoder; >> + >> + wb->connector = msm_wb_connector_init(wb); >> + if (IS_ERR(wb->connector)) { >> + ret = PTR_ERR(wb->connector); >> + dev_err(dev->dev, "failed to create WB connector: %d\n", >> ret); >> + wb->connector = NULL; >> + goto fail; >> + } >> + >> + priv->connectors[priv->num_connectors++] = wb->connector; >> + >> + return 0; >> + >> +fail: >> + if (wb->connector) { >> + wb->connector->funcs->destroy(wb->connector); >> + wb->connector = NULL; >> + } >> + > Drop the unused if block ? > > >> +static struct msm_wb *msm_wb_init(struct platform_device *pdev) >> +{ >> + struct msm_wb *wb = NULL; >> + >> + wb = devm_kzalloc(&pdev->dev, sizeof(*wb), GFP_KERNEL); >> + if (!wb) >> + return ERR_PTR(-ENOMEM); >> + >> + wb->pdev = pdev; >> + wb->priv_data = devm_kzalloc(&pdev->dev, sizeof(*wb->priv_data), >> + GFP_KERNEL); >> + if (!wb->priv_data) >> + return ERR_PTR(-ENOMEM); >> + >> + if (msm_wb_v4l2_init(wb)) { >> + pr_err("%s: wb v4l2 init failed\n", __func__); >> + return ERR_PTR(-ENODEV); >> + } >> + > Seems like we're leaking wb and/or wb->priv_data. Add a label and > consolidate error handling in there ? Since the devm_kzalloc function is used here, the system should take care freeing the memory. > > >> +++ b/drivers/gpu/drm/msm/mdp/mdp_wb/mdp_wb.h > >> +#ifndef __WB_CONNECTOR_H__ >> +#define __WB_CONNECTOR_H__ >> + > The file is called mdp_wb.h, so one might want to change this to > __MDP_WB_H__ > > >> --- /dev/null >> +++ b/drivers/gpu/drm/msm/mdp/mdp_wb/mdp_wb_v4l2.c >> @@ -0,0 +1,522 @@ > >> +static const struct msm_wb_fmt *get_format(u32 fourcc) >> +{ >> + const struct msm_wb_fmt *fmt; >> + unsigned int k; >> + >> + for (k = 0; k < ARRAY_SIZE(formats); k++) { >> + fmt = &formats[k]; >> + if (fmt->fourcc == fourcc) >> + break; >> + } >> + >> + if (k == ARRAY_SIZE(formats)) >> + return NULL; >> + >> + return &formats[k]; > You could move the return within the loop, and drop the follow up > conditional. > > >> +static void *msm_wb_vb2_attach_dmabuf(void *alloc_ctx, struct dma_buf >> *dbuf, >> + unsigned long size, int write) >> +{ >> + struct msm_wb_v4l2_dev *dev = alloc_ctx; >> + struct drm_device *drm_dev = dev->wb->dev; >> + struct drm_gem_object *obj; >> + >> + mutex_lock(&drm_dev->object_name_lock); >> + obj = drm_dev->driver->gem_prime_import(drm_dev, dbuf); >> + if (WARN_ON(!obj)) { >> + mutex_unlock(&drm_dev->object_name_lock); >> + v4l2_err(&dev->v4l2_dev, "Can't convert dmabuf to gem >> obj.\n"); >> + return NULL; > Shouldn't one return ERR_PTR here ? Consolidating the error paths to a > single label will be cleaner imho. > > >> --- a/drivers/gpu/drm/msm/msm_drv.h >> +++ b/drivers/gpu/drm/msm/msm_drv.h > >> @@ -224,18 +229,28 @@ struct drm_framebuffer >> *msm_framebuffer_create(struct drm_device *dev, >> >> struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev); >> >> -struct hdmi; >> int hdmi_modeset_init(struct hdmi *hdmi, struct drm_device *dev, >> struct drm_encoder *encoder); >> void __init hdmi_register(void); >> void __exit hdmi_unregister(void); >> >> -struct msm_edp; > Unrelated cosmetic changes ? > > >> --- a/drivers/gpu/drm/msm/msm_gem.c >> +++ b/drivers/gpu/drm/msm/msm_gem.c >> @@ -534,6 +534,7 @@ void msm_gem_free_object(struct drm_gem_object *obj) >> if (msm_obj->pages) >> drm_free_large(msm_obj->pages); >> >> + drm_prime_gem_destroy(obj, msm_obj->sgt); > Should this fix be a separate patch ? > > > Cheers, > Emil >