From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Subject: Re: [PATCH v4 1/3] drm: add bus_formats and num_bus_formats fields to drm_display_info Date: Mon, 1 Dec 2014 22:22:38 +0100 Message-ID: <20141201222238.40e9ff86@bbrezillon> References: <1417422061-10384-1-git-send-email-boris.brezillon@free-electrons.com> <1417422061-10384-6-git-send-email-boris.brezillon@free-electrons.com> <20141201154257.GD11943@ulmo.nvidia.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail.free-electrons.com (down.free-electrons.com [37.187.137.238]) by gabe.freedesktop.org (Postfix) with ESMTP id 3D6226E264 for ; Mon, 1 Dec 2014 13:22:57 -0800 (PST) In-Reply-To: <20141201154257.GD11943@ulmo.nvidia.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Thierry Reding Cc: Nicolas Ferre , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Alexandre Belloni , Laurent Pinchart , Jean-Christophe Plagniol-Villard , Andrew Victor List-Id: dri-devel@lists.freedesktop.org SGkgVGhpZXJyeSwKCk9uIE1vbiwgMSBEZWMgMjAxNCAxNjo0Mjo1OCArMDEwMApUaGllcnJ5IFJl ZGluZyA8dGhpZXJyeS5yZWRpbmdAZ21haWwuY29tPiB3cm90ZToKCj4gT24gTW9uLCBEZWMgMDEs IDIwMTQgYXQgMDk6MjA6NTlBTSArMDEwMCwgQm9yaXMgQnJlemlsbG9uIHdyb3RlOgo+ID4gQWRk IGJ1c19mb3JtYXRzIGFuZCBudW1fYnVzX2Zvcm1hdHMgZmllbGRzIGFuZAo+ID4gZHJtX2Rpc3Bs YXlfaW5mb19zZXRfYnVzX2Zvcm1hdHMgaGVscGVyIGZ1bmN0aW9uIHRvIHNwZWNpZnkgdGhlIGJ1 cwo+ID4gZm9ybWF0cyBzdXBwb3J0ZWQgYnkgYSBnaXZlbiBkaXNwbGF5Lgo+ID4gCj4gPiBUaGlz IGluZm9ybWF0aW9uIGNhbiBiZSB1c2VkIGJ5IGRpc3BsYXkgY29udHJvbGxlciBkcml2ZXJzIHRv IGNvbmZpZ3VyZQo+ID4gdGhlIG91dHB1dCBpbnRlcmZhY2UgYXBwcm9wcmlhdGVseSAoaS5lLiBS R0I1NjUsIFJHQjY2NiBvciBSR0I4ODggb24gcmF3Cj4gPiBSR0Igb3IgTFZEUyBidXNzZXMpLgo+ ID4gCj4gPiBTaWduZWQtb2ZmLWJ5OiBCb3JpcyBCcmV6aWxsb24gPGJvcmlzLmJyZXppbGxvbkBm cmVlLWVsZWN0cm9ucy5jb20+Cj4gPiAtLS0KPiA+ICBkcml2ZXJzL2dwdS9kcm0vZHJtX2NydGMu YyB8IDMyICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrCj4gPiAgaW5jbHVkZS9kcm0v ZHJtX2NydGMuaCAgICAgfCAgNyArKysrKysrCj4gPiAgMiBmaWxlcyBjaGFuZ2VkLCAzOSBpbnNl cnRpb25zKCspCj4gPiAKPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vZHJtX2NydGMu YyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fY3J0Yy5jCj4gPiBpbmRleCBlNzljOGQzLi5kM2I3ZWQw IDEwMDY0NAo+ID4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2RybV9jcnRjLmMKPiA+ICsrKyBiL2Ry aXZlcnMvZ3B1L2RybS9kcm1fY3J0Yy5jCj4gPiBAQCAtNzYzLDYgKzc2MywzOCBAQCBzdGF0aWMg dm9pZCBkcm1fbW9kZV9yZW1vdmUoc3RydWN0IGRybV9jb25uZWN0b3IgKmNvbm5lY3RvciwKPiA+ ICAJZHJtX21vZGVfZGVzdHJveShjb25uZWN0b3ItPmRldiwgbW9kZSk7Cj4gPiAgfQo+ID4gIAo+ ID4gKy8qCj4gCj4gVGhpcyBuZWVkcyBhIC8qKiBtYXJrZXIgdG8gbWFrZSBpdCBhIHZhbGlkIGtl cm5lbGRvYyBjb21tZW50LgoKWWVwLCBJJ2xsIGZpeCB0aGF0LgoKPiAKPiA+ICsgKiBkcm1fZGlz cGxheV9pbmZvX3NldF9idXNfZm9ybWF0cyAtIHNldCB0aGUgc3VwcG9ydGVkIGJ1cyBmb3JtYXRz Cj4gPiArICogQGluZm86IGRpc3BsYXkgaW5mbyB0byBzdG9yZSBidXMgZm9ybWF0cyBpbgo+ID4g KyAqIEBmbXRzOiBhcnJheSBjb250YWluaW5nIHRoZSBzdXBwb3J0ZWQgYnVzIGZvcm1hdHMKPiA+ ICsgKiBAbmZtdHM6IHRoZSBudW1iZXIgb2YgZW50cmllcyBpbiB0aGUgZm10cyBhcnJheQo+ID4g KyAqCj4gPiArICogU3RvcmUgdGhlIHN1cHBwb3J0ZWQgYnVzIGZvcm1hdHMgaW4gZGlzcGxheSBp bmZvIHN0cnVjdHVyZS4KPiA+ICsgKiBTZWUgTUVESUFfQlVTX0ZNVF8qIGRlZmluaXRpb25zIGlu IGluY2x1ZGUvdWFwaS9saW51eC9tZWRpYS1idXMtZm9ybWF0LmggZm9yCj4gPiArICogYSBmdWxs IGxpc3Qgb2YgYXZhaWxhYmxlIGZvcm1hdHMuCj4gPiArICovCj4gPiAraW50IGRybV9kaXNwbGF5 X2luZm9fc2V0X2J1c19mb3JtYXRzKHN0cnVjdCBkcm1fZGlzcGxheV9pbmZvICppbmZvLCBjb25z dCB1MzIgKmZtdHMsCj4gCj4gVGhpcyBvbmUgc3RpbGwgZXhjZWVkcyA4MC1jaGFyYWN0ZXJzIHBl ciBsaW5lLCBidXQgcGVyaGFwcyBJJ20gcmV2aWV3aW5nCj4gdGhlIHdyb25nIHBhdGNoPyBBbHNv IEkgdGhpbmsgZm10cyBzaG91bGQgYmUgZm9ybWF0cyBoZXJlLgo+IAo+ID4gKwkJCQkgICAgIHVu c2lnbmVkIGludCBudW1fZm10cykKPiAKPiBBbmQgdGhpcyBzaG91bGQgYmUgbnVtX2Zvcm1hdHMs IGZvciBjb25zaXN0ZW5jeS4gQWxzbyBtYWtlIHN1cmUgdG8ga2VlcAo+IHRoaXMgY29uc2lzdGVu dCB3aXRoIHRoZSBrZXJuZWxkb2MuCj4gCj4gPiArewo+ID4gKwl1MzIgKmZvcm1hdHMgPSBOVUxM Owo+IAo+IElmIHlvdSBuYW1lIHRoZSBwYXJhbWV0ZXIgZm9ybWF0cywgbWF5YmUgYnVzX2Zvcm1h dHMgd291bGQgYmUgYSBnb29kCj4gYWx0ZXJuYXRpdmUgZm9yIHRoaXMgbG9jYWwgdmFyaWFibGUu IE9yIGZtdHMgaGVyZS4gSSB0aGluayBpdCdzIG1vc3QKPiBpbXBvcnRhbnQgdGhhdCB0aGUgcHVi bGljIEFQSSBnZXRzIHRoZSBtb3JlIGlkaW9tYXRpYyBuYW1lLgoKT2theSwgSSdsbCByZXdvcmsg dGhlIGFyZ3VtZW50IGFuZCB2YXJpYWJsZSBuYW1lcy4KCj4gCj4gPiArCj4gPiArCWlmICghZm10 cyAmJiBudW1fZm10cykKPiA+ICsJCXJldHVybiAtRUlOVkFMOwo+ID4gKwo+ID4gKwlpZiAoZm10 cyAmJiBudW1fZm10cykgewo+ID4gKwkJZm9ybWF0cyA9IGttZW1kdXAoZm10cywgc2l6ZW9mKCpm bXRzKSAqIG51bV9mbXRzLCBHRlBfS0VSTkVMKTsKPiA+ICsJCWlmICghZm9ybWF0cykKPiA+ICsJ CQlyZXR1cm4gLUVOT01FTTsKPiA+ICsJfQo+ID4gKwo+ID4gKwlrZnJlZShpbmZvLT5idXNfZm9y bWF0cyk7Cj4gPiArCWluZm8tPmJ1c19mb3JtYXRzID0gZm9ybWF0czsKPiA+ICsJaW5mby0+bnVt X2J1c19mb3JtYXRzID0gbnVtX2ZtdHM7Cj4gPiArCj4gPiArCXJldHVybiAwOwo+ID4gK30KPiA+ ICtFWFBPUlRfU1lNQk9MKGRybV9kaXNwbGF5X2luZm9fc2V0X2J1c19mb3JtYXRzKTsKPiAKPiBJ IHRoaW5rIHlvdSdsbCB3YW50IHRvIGNhbGwga2ZyZWUoKSBvbiB0aGUgYnVzX2Zvcm1hdHMgYXJy YXkgZnJvbQo+IGRybV9jb25uZWN0b3JfY2xlYW51cCgpIGFzIHdlbGwgdG8gbWFrZSBzdXJlIHlv dSBkb24ndCBsZWFrIHRoaXMgb24KPiBkcml2ZXIgdW5sb2FkLgoKSW5kZWVkLCBJJ2xsIGZpeCB0 aGF0IG1lbW9yeSBsZWFrLgoKVGhhbmtzLAoKQm9yaXMKCi0tIApCb3JpcyBCcmV6aWxsb24sIEZy ZWUgRWxlY3Ryb25zCkVtYmVkZGVkIExpbnV4IGFuZCBLZXJuZWwgZW5naW5lZXJpbmcKaHR0cDov L2ZyZWUtZWxlY3Ryb25zLmNvbQpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVz a3RvcC5vcmcKaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2Ry aS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932513AbaLAVW5 (ORCPT ); Mon, 1 Dec 2014 16:22:57 -0500 Received: from down.free-electrons.com ([37.187.137.238]:55618 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932080AbaLAVW4 (ORCPT ); Mon, 1 Dec 2014 16:22:56 -0500 Date: Mon, 1 Dec 2014 22:22:38 +0100 From: Boris Brezillon To: Thierry Reding Cc: David Airlie , dri-devel@lists.freedesktop.org, Laurent Pinchart , linux-kernel@vger.kernel.org, Nicolas Ferre , Jean-Christophe Plagniol-Villard , Alexandre Belloni , Andrew Victor Subject: Re: [PATCH v4 1/3] drm: add bus_formats and num_bus_formats fields to drm_display_info Message-ID: <20141201222238.40e9ff86@bbrezillon> In-Reply-To: <20141201154257.GD11943@ulmo.nvidia.com> References: <1417422061-10384-1-git-send-email-boris.brezillon@free-electrons.com> <1417422061-10384-6-git-send-email-boris.brezillon@free-electrons.com> <20141201154257.GD11943@ulmo.nvidia.com> X-Mailer: Claws Mail 3.9.3 (GTK+ 2.24.23; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Thierry, On Mon, 1 Dec 2014 16:42:58 +0100 Thierry Reding wrote: > On Mon, Dec 01, 2014 at 09:20:59AM +0100, Boris Brezillon wrote: > > Add bus_formats and num_bus_formats fields and > > drm_display_info_set_bus_formats helper function to specify the bus > > formats supported by a given display. > > > > This information can be used by display controller drivers to configure > > the output interface appropriately (i.e. RGB565, RGB666 or RGB888 on raw > > RGB or LVDS busses). > > > > Signed-off-by: Boris Brezillon > > --- > > drivers/gpu/drm/drm_crtc.c | 32 ++++++++++++++++++++++++++++++++ > > include/drm/drm_crtc.h | 7 +++++++ > > 2 files changed, 39 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > index e79c8d3..d3b7ed0 100644 > > --- a/drivers/gpu/drm/drm_crtc.c > > +++ b/drivers/gpu/drm/drm_crtc.c > > @@ -763,6 +763,38 @@ static void drm_mode_remove(struct drm_connector *connector, > > drm_mode_destroy(connector->dev, mode); > > } > > > > +/* > > This needs a /** marker to make it a valid kerneldoc comment. Yep, I'll fix that. > > > + * drm_display_info_set_bus_formats - set the supported bus formats > > + * @info: display info to store bus formats in > > + * @fmts: array containing the supported bus formats > > + * @nfmts: the number of entries in the fmts array > > + * > > + * Store the suppported bus formats in display info structure. > > + * See MEDIA_BUS_FMT_* definitions in include/uapi/linux/media-bus-format.h for > > + * a full list of available formats. > > + */ > > +int drm_display_info_set_bus_formats(struct drm_display_info *info, const u32 *fmts, > > This one still exceeds 80-characters per line, but perhaps I'm reviewing > the wrong patch? Also I think fmts should be formats here. > > > + unsigned int num_fmts) > > And this should be num_formats, for consistency. Also make sure to keep > this consistent with the kerneldoc. > > > +{ > > + u32 *formats = NULL; > > If you name the parameter formats, maybe bus_formats would be a good > alternative for this local variable. Or fmts here. I think it's most > important that the public API gets the more idiomatic name. Okay, I'll rework the argument and variable names. > > > + > > + if (!fmts && num_fmts) > > + return -EINVAL; > > + > > + if (fmts && num_fmts) { > > + formats = kmemdup(fmts, sizeof(*fmts) * num_fmts, GFP_KERNEL); > > + if (!formats) > > + return -ENOMEM; > > + } > > + > > + kfree(info->bus_formats); > > + info->bus_formats = formats; > > + info->num_bus_formats = num_fmts; > > + > > + return 0; > > +} > > +EXPORT_SYMBOL(drm_display_info_set_bus_formats); > > I think you'll want to call kfree() on the bus_formats array from > drm_connector_cleanup() as well to make sure you don't leak this on > driver unload. Indeed, I'll fix that memory leak. Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com