From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v3] drm: move allocation out of drm_get_format_name() Date: Thu, 10 Nov 2016 12:03:13 +0200 Message-ID: <2360827.8WFanMYCQ1@avalon> References: <20161108101558.ihvrprbbdqjwu5wg@phenom.ffwll.local> <20161109165931.GR25290@imgtec.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20161109165931.GR25290@imgtec.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Eric Engestrom Cc: David Airlie , dri-devel , Wei Yongjun , Daniel Vetter , Flora Cui , Gustavo Padovan , Tom St Denis , Chunming Zhou , Thomas Hellstrom , Laurent Pinchart , Sinclair Yeh , Xinliang Liu , Xinwei Kong , VMware Graphics , Vitaly Prosyak , Alexandre Demers , Jani Nikula , intel-gfx , Eric Engestrom , Emily Deng , Junwei Zhang , Mi List-Id: dri-devel@lists.freedesktop.org SGkgRXJpYywKCk9uIFdlZG5lc2RheSAwOSBOb3YgMjAxNiAxNjo1OTozMSBFcmljIEVuZ2VzdHJv bSB3cm90ZToKPiBPbiBXZWRuZXNkYXksIDIwMTYtMTEtMDkgMTQ6MTM6NDAgKzAxMDAsIERhbmll bCBWZXR0ZXIgd3JvdGU6Cj4gPiBPbiBXZWQsIE5vdiA5LCAyMDE2IGF0IDEyOjQyIFBNLCBFcmlj IEVuZ2VzdHJvbSB3cm90ZToKPiA+ID4+IFdlbGwsIGhhZCB0byBkcm9wIGl0IGFnYWluIHNpbmNl IGl0IGRpZG4ndCBjb21waWxlOgo+ID4gPj4gICBDQyBbTV0gIGRyaXZlcnMvZ3B1L2RybS9kcm1f YmxlbmQubwo+ID4gPj4gCj4gPiA+PiBkcml2ZXJzL2dwdS9kcm0vZHJtX2F0b21pYy5jOiBJbiBm dW5jdGlvbgo+ID4gPj4g4oCYZHJtX2F0b21pY19wbGFuZV9wcmludF9zdGF0ZeKAmToKPiA+ID4+ IGRyaXZlcnMvZ3B1L2RybS9kcm1fYXRvbWljLmM6OTIwOjU6IGVycm9yOiB0b28gZmV3IGFyZ3Vt ZW50cyB0bwo+ID4gPj4gZnVuY3Rpb24g4oCYZHJtX2dldF9mb3JtYXRfbmFtZeKAmT4gPj4gCj4g PiA+PiAgICAgIGRybV9nZXRfZm9ybWF0X25hbWUoZmItPnBpeGVsX2Zvcm1hdCkpOwo+ID4gPj4g ICAgICBefn5+fn5+fn5+fn5+fn5+fn5+Cj4gPiA+PiAKPiA+ID4+IEluIGZpbGUgaW5jbHVkZWQg ZnJvbSAuL2luY2x1ZGUvZHJtL2RybVAuaDo3MTowLAo+ID4gPj4gCj4gPiA+PiAgICAgICAgICAg ICAgICAgIGZyb20gZHJpdmVycy9ncHUvZHJtL2RybV9hdG9taWMuYzoyOToKPiA+ID4+IC4vaW5j bHVkZS9kcm0vZHJtX2ZvdXJjYy5oOjY1Ojc6IG5vdGU6IGRlY2xhcmVkIGhlcmUKPiA+ID4+IAo+ ID4gPj4gIGNoYXIgKmRybV9nZXRfZm9ybWF0X25hbWUodWludDMyX3QgZm9ybWF0LCBzdHJ1Y3Qg ZHJtX2Zvcm1hdF9uYW1lX2J1Zgo+ID4gPj4gICpidWYpOz4gPj4gIAo+ID4gPj4gICAgICAgIF5+ fn5+fn5+fn5+fn5+fn5+fn4KPiA+ID4+IAo+ID4gPj4gQ2FuIHlvdSBwbHMgcmViYXNlIG9udG8g ZHJtLW1pc2Mgb3IgbGludXgtbmV4dCBvciBzb21ldGhpbmc/Cj4gPiA+IAo+ID4gPiBUaGF0IHdh cyBiYXNlZCBvbiBhaXJsaWVkL2RybS1uZXh0IChsYXN0IGZldGNoZWQgb24gU3VuZGF5IEkgdGhp bmspLAo+ID4gPiBJIGNhbiByZWJhc2UgaXQgb24gZHJtLW1pc2MgaWYgaXQgaGVscHMsIGJ1dCBp dCBzZWVtcyBvbGRlciB0aGFuCj4gPiA+IGRybS1uZXh0LiBTaG91bGQgSSBqdXN0IHJlYmFzZSBv biB0b3Agb2YgY3VycmVudCBoZWFkIG9mIGRybS1uZXh0Pwo+ID4gCj4gPiBJdCBuZWVkcyB0byBi ZSBkcm0tbWlzYyAobGludXgtbmV4dCBkb2Vzbid0IGhhdmUgaXQgeWV0KSBkdWUgdG8gdGhlCj4g PiBuZXcgYXRvbWljIGRlYnVnIHdvcmsgdGhhdCB3ZSBqdXN0IGxhbmRlZC4gSSdtIHdvcmtpbmcg b24gZHJtLXRpcCBhcyBhCj4gPiBkcm0gbG9jYWwgaW50ZWdyYXRpb24gdHJlZSB0byBlYXNlIHBh aW5zIGxpa2UgdGhlc2UgYSBiaXQsIGJ1dCB0aGF0Cj4gPiBkb2Vzbid0IHJlYWxseSBleGlzdCB5 ZXQuCj4gCj4gSSdtIGNvbmZ1c2VkIGFzIHRvIGhvdyB0aGUgZGlmZmVyZW50IHRyZWVzIGFuZCBi cmFuY2hlcyBtZXJnZSBiYWNrIHRvCj4gVG9ydmFsZHMnIHRyZWUgKEknbSBpbnRlcmVzdGVkIGlu IHBhcnRpY3VsYXIgaW4gZHJtKSwgYW5kIEknbSBub3Qgc3VyZQo+IHdoaWNoIGJyYW5jaCB5b3Ug d2FudCBtZSB0byByZWJhc2Ugb24gaW4gdGhlIGRybS1taXNjIHRyZWUgWzFdLAo+IGVzcGVjaWFs bHkgc2luY2UgYWxsIG9mIHRoZW0gYXJlIG9sZGVyIHRoYW4gZHJtLW5leHQgWzJdLgo+IAo+IEkn bGwgdHJ5IHRvIHJlYmFzZSBvbiBkcm0tbWlzYy1maXhlcyAoY3VycmVudGx5IGF0IDRkYTVjYWE2 YTZmODJjZGEzMTkzKQo+IGFzIGl0IHNvdW5kcyBhYm91dCByaWdodCwgYnV0IGl0IGRvZXNuJ3Qg YXBwbHkgYXQgYWxsLCBzbyBpdCdsbCB0YWtlCj4gYSBsaXR0bGUgd2hpbGUuCgpXaGlsZSBhdCBp dCwgY291bGQgeW91IG1ha2UgdGhlIGZ1bmN0aW9uIHJldHVybiBhIGNvbnN0IGNoYXIgKiA/CgpC eSB0aGUgd2F5LCB3aGlsZSB0aGlzIGlzIGFuIGltcHJvdmVtZW50IG92ZXIgdGhlIGN1cnJlbnQg c2l0dWF0aW9uIGluIHRoYXQgaXQgCmZpeGVzIHRoZSBtaXNzaW5nIGtmcmVlKCkgaXNzdWUsIEkg d29uZGVyIHdoZXRoZXIgdGhlIHByb2JsZW0gd2UncmUgdHJ5aW5nIHRvIApzb2x2ZSBzaG91bGQg YmUgYWRkcmVzc2VkIGF0IGEgbW9yZSBnbG9iYWwgbGV2ZWwuCgpUaGUgaXNzdWUgaGVyZSBpcyB0 aGF0IHByaW50ayBjYW4ndCBmb3JtYXQgdGhlIGZvdXJjYyBhcyBhIHN0cmluZyBieSBpdHNlbGYu IApUaGVyZSdzIGEgYnVuY2ggb2YgcGxhY2VzIGluIHRoZSBrZXJuZWwgd2hlcmUgYSBzaW1pbGFy IGZvcm1hdHRpbmcgcHJvYmxlbSAKb2NjdXJzLiBJbiBhIGZldyBvY2Nhc2lvbnMgaXQgaGFzIGJl ZW4gc29sdmVkIGJ5IGV4dGVuZGluZyBwcmludGsgd2l0aCAKYWRkaXRpb25hbCBmb3JtYXQgc3Bl Y2lmaWVycyAoc3VjaCBhcyBmb3IgTUFDL0lQIGFkZHJlc3NlcywgR1VJRHMsIHZhcmlvdXMgCmtp bmQgb2YgZGV2aWNlIG5hbWVzLCAuLi4pLiBEUk0gZm91cmNjcyBhcmUgcHJvYmFibHkgdG9vIERS TSBzcGVjaWZpYyB0byBiZSAKd29ydGggYSBmb3JtYXQgc3BlY2lmaWVyLCBidXQgSSB3b25kZXIg d2hldGhlciB3ZSBjb3VsZCBpbnRyb2R1Y2UgYSBuZXcgCnNwZWNpZmllciB0aGF0IHRha2VzIGEg ZnVuY3Rpb24gcG9pbnRlciBhcyBhIGZvcm1hdHRpbmcgaGVscGVyLiBBbm90aGVyIApzaW1pbGFy bHkgY3Jhenkgb3B0aW9uIHdvdWxkIGJlIGEgZm9ybWF0IHNwZWNpZmllciBmb3Igc3RyaW5ncyB0 aGF0IHdvdWxkIGZyZWUgCnRoZSBwYXNzZWQgcG9pbnRlciBhZnRlciBwcmludGluZyBpdC4KCj4g Q291bGQgeW91IGdpdmUgbWUgYSBxdWljayBleHBsYW5hdGlvbiBvciBwb2ludCBtZSB0byBhIGRv Yy9wYWdlIHRoYXQKPiBleHBsYWlucyBob3cgdGhlIHZhcmlvdXMgdHJlZXMgYW5kIGJyYW5jaGVz IGdldCBtZXJnZWQ/Cj4gSSBnb29nbGVkIGEgYml0IGFuZCBmb3VuZCB0aGlzIGRvYyBbNF0gYnkg SmFuaSwgYnV0IGl0IGRvZXNuJ3QgbWVudGlvbgo+IGRybS1taXNjIGZvciBpbnN0YW5jZSwgc28g SSdtIG5vdCBzdXJlIGhvdyB1cC10by1kYXRlIGFuZAo+IG5vbi1pbnRlbC1zcGVjaWZpYyBpdCBp cy4KPiAKPiBMb29raW5nIGF0IHRoaXMgcGFnZSwgc29tZXRoaW5nIGp1c3Qgb2NjdXJyZWQgdG8g bWU6IGRpZCB5b3UgbWVhbgo+IGRybS1maXhlcyBbM10sIGluc3RlYWQgb2Ygb25lIG9mIHRoZSBi cmFuY2hlcyBvbiBkcm0tbWlzYz8KPiAKPiBDaGVlcnMsCj4gICBFcmljCj4gCj4gWzFdIGdpdDov L2Fub25naXQuZnJlZWRlc2t0b3Aub3JnL2RybS9kcm0tbWlzYwo+IFsyXSBnaXQ6Ly9wZW9wbGUu ZnJlZWRlc2t0b3Aub3JnL35haXJsaWVkL2xpbnV4IGRybS1uZXh0Cj4gWzJdIGdpdDovL3Blb3Bs ZS5mcmVlZGVza3RvcC5vcmcvfmFpcmxpZWQvbGludXggZHJtLWZpeGVzCj4gWzNdIGh0dHBzOi8v MDEub3JnL2xpbnV4Z3JhcGhpY3MvZ2Z4LWRvY3MvbWFpbnRhaW5lci10b29scy9kcm0taW50ZWwu aHRtbAoKLS0gClJlZ2FyZHMsCgpMYXVyZW50IFBpbmNoYXJ0CgpfX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fXwpJbnRlbC1nZnggbWFpbGluZyBsaXN0CkludGVs LWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcv bWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755111AbcKJKDO (ORCPT ); Thu, 10 Nov 2016 05:03:14 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:51996 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754183AbcKJKDM (ORCPT ); Thu, 10 Nov 2016 05:03:12 -0500 From: Laurent Pinchart To: Eric Engestrom Cc: Daniel Vetter , Eric Engestrom , Linux Kernel Mailing List , David Airlie , dri-devel , Wei Yongjun , Daniel Vetter , Flora Cui , Gustavo Padovan , Tom St Denis , Chunming Zhou , Thomas Hellstrom , Laurent Pinchart , Sinclair Yeh , Xinliang Liu , Xinwei Kong , VMware Graphics , Vitaly Prosyak , Alexandre Demers , Jani Nikula , intel-gfx , Emily Deng , Colin Ian King , Junwei Zhang , Michel =?ISO-8859-1?Q?D=E4nzer?= , Alex Deucher , Christian =?ISO-8859-1?Q?K=F6nig?= Subject: Re: [Intel-gfx] [PATCH v3] drm: move allocation out of drm_get_format_name() Date: Thu, 10 Nov 2016 12:03:13 +0200 Message-ID: <2360827.8WFanMYCQ1@avalon> User-Agent: KMail/4.14.10 (Linux/4.8.6-gentoo; KDE/4.14.24; x86_64; ; ) In-Reply-To: <20161109165931.GR25290@imgtec.com> References: <20161108101558.ihvrprbbdqjwu5wg@phenom.ffwll.local> <20161109165931.GR25290@imgtec.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id uAAA3Wqg022150 Hi Eric, On Wednesday 09 Nov 2016 16:59:31 Eric Engestrom wrote: > On Wednesday, 2016-11-09 14:13:40 +0100, Daniel Vetter wrote: > > On Wed, Nov 9, 2016 at 12:42 PM, Eric Engestrom wrote: > > >> Well, had to drop it again since it didn't compile: > > >> CC [M] drivers/gpu/drm/drm_blend.o > > >> > > >> drivers/gpu/drm/drm_atomic.c: In function > > >> ‘drm_atomic_plane_print_state’: > > >> drivers/gpu/drm/drm_atomic.c:920:5: error: too few arguments to > > >> function ‘drm_get_format_name’> >> > > >> drm_get_format_name(fb->pixel_format)); > > >> ^~~~~~~~~~~~~~~~~~~ > > >> > > >> In file included from ./include/drm/drmP.h:71:0, > > >> > > >> from drivers/gpu/drm/drm_atomic.c:29: > > >> ./include/drm/drm_fourcc.h:65:7: note: declared here > > >> > > >> char *drm_get_format_name(uint32_t format, struct drm_format_name_buf > > >> *buf);> >> > > >> ^~~~~~~~~~~~~~~~~~~ > > >> > > >> Can you pls rebase onto drm-misc or linux-next or something? > > > > > > That was based on airlied/drm-next (last fetched on Sunday I think), > > > I can rebase it on drm-misc if it helps, but it seems older than > > > drm-next. Should I just rebase on top of current head of drm-next? > > > > It needs to be drm-misc (linux-next doesn't have it yet) due to the > > new atomic debug work that we just landed. I'm working on drm-tip as a > > drm local integration tree to ease pains like these a bit, but that > > doesn't really exist yet. > > I'm confused as to how the different trees and branches merge back to > Torvalds' tree (I'm interested in particular in drm), and I'm not sure > which branch you want me to rebase on in the drm-misc tree [1], > especially since all of them are older than drm-next [2]. > > I'll try to rebase on drm-misc-fixes (currently at 4da5caa6a6f82cda3193) > as it sounds about right, but it doesn't apply at all, so it'll take > a little while. While at it, could you make the function return a const char * ? By the way, while this is an improvement over the current situation in that it fixes the missing kfree() issue, I wonder whether the problem we're trying to solve should be addressed at a more global level. The issue here is that printk can't format the fourcc as a string by itself. There's a bunch of places in the kernel where a similar formatting problem occurs. In a few occasions it has been solved by extending printk with additional format specifiers (such as for MAC/IP addresses, GUIDs, various kind of device names, ...). DRM fourccs are probably too DRM specific to be worth a format specifier, but I wonder whether we could introduce a new specifier that takes a function pointer as a formatting helper. Another similarly crazy option would be a format specifier for strings that would free the passed pointer after printing it. > Could you give me a quick explanation or point me to a doc/page that > explains how the various trees and branches get merged? > I googled a bit and found this doc [4] by Jani, but it doesn't mention > drm-misc for instance, so I'm not sure how up-to-date and > non-intel-specific it is. > > Looking at this page, something just occurred to me: did you mean > drm-fixes [3], instead of one of the branches on drm-misc? > > Cheers, > Eric > > [1] git://anongit.freedesktop.org/drm/drm-misc > [2] git://people.freedesktop.org/~airlied/linux drm-next > [2] git://people.freedesktop.org/~airlied/linux drm-fixes > [3] https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-intel.html -- Regards, Laurent Pinchart