From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [Intel-gfx] [PATCH v3] drm: move allocation out of drm_get_format_name() Date: Thu, 10 Nov 2016 12:30:09 +0200 Message-ID: <871syjfwzy.fsf@intel.com> References: <20161108101558.ihvrprbbdqjwu5wg@phenom.ffwll.local> <20161109165931.GR25290@imgtec.com> <2360827.8WFanMYCQ1@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <2360827.8WFanMYCQ1@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Laurent Pinchart , Eric Engestrom Cc: dri-devel , Wei Yongjun , Daniel Vetter , Flora Cui , Gustavo Padovan , Tom St Denis , Thomas Hellstrom , Laurent Pinchart , Xinliang Liu , VMware Graphics , Vitaly Prosyak , Alexandre Demers , intel-gfx , Eric Engestrom , Emily Deng , Junwei Zhang , Michel =?utf-8?Q?D=C3=A4nzer?= , Linux Kernel Mailing List , Alex Deucher , Colin Ian King List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCAxMCBOb3YgMjAxNiwgTGF1cmVudCBQaW5jaGFydCA8bGF1cmVudC5waW5jaGFydEBp ZGVhc29uYm9hcmQuY29tPiB3cm90ZToKPiBIaSBFcmljLAo+Cj4gT24gV2VkbmVzZGF5IDA5IE5v diAyMDE2IDE2OjU5OjMxIEVyaWMgRW5nZXN0cm9tIHdyb3RlOgo+PiBPbiBXZWRuZXNkYXksIDIw MTYtMTEtMDkgMTQ6MTM6NDAgKzAxMDAsIERhbmllbCBWZXR0ZXIgd3JvdGU6Cj4+ID4gT24gV2Vk LCBOb3YgOSwgMjAxNiBhdCAxMjo0MiBQTSwgRXJpYyBFbmdlc3Ryb20gd3JvdGU6Cj4+ID4gPj4g V2VsbCwgaGFkIHRvIGRyb3AgaXQgYWdhaW4gc2luY2UgaXQgZGlkbid0IGNvbXBpbGU6Cj4+ID4g Pj4gICBDQyBbTV0gIGRyaXZlcnMvZ3B1L2RybS9kcm1fYmxlbmQubwo+PiA+ID4+IAo+PiA+ID4+ IGRyaXZlcnMvZ3B1L2RybS9kcm1fYXRvbWljLmM6IEluIGZ1bmN0aW9uCj4+ID4gPj4g4oCYZHJt X2F0b21pY19wbGFuZV9wcmludF9zdGF0ZeKAmToKPj4gPiA+PiBkcml2ZXJzL2dwdS9kcm0vZHJt X2F0b21pYy5jOjkyMDo1OiBlcnJvcjogdG9vIGZldyBhcmd1bWVudHMgdG8KPj4gPiA+PiBmdW5j dGlvbiDigJhkcm1fZ2V0X2Zvcm1hdF9uYW1l4oCZPiA+PiAKPj4gPiA+PiAgICAgIGRybV9nZXRf Zm9ybWF0X25hbWUoZmItPnBpeGVsX2Zvcm1hdCkpOwo+PiA+ID4+ICAgICAgXn5+fn5+fn5+fn5+ fn5+fn5+fgo+PiA+ID4+IAo+PiA+ID4+IEluIGZpbGUgaW5jbHVkZWQgZnJvbSAuL2luY2x1ZGUv ZHJtL2RybVAuaDo3MTowLAo+PiA+ID4+IAo+PiA+ID4+ICAgICAgICAgICAgICAgICAgZnJvbSBk cml2ZXJzL2dwdS9kcm0vZHJtX2F0b21pYy5jOjI5Ogo+PiA+ID4+IC4vaW5jbHVkZS9kcm0vZHJt X2ZvdXJjYy5oOjY1Ojc6IG5vdGU6IGRlY2xhcmVkIGhlcmUKPj4gPiA+PiAKPj4gPiA+PiAgY2hh ciAqZHJtX2dldF9mb3JtYXRfbmFtZSh1aW50MzJfdCBmb3JtYXQsIHN0cnVjdCBkcm1fZm9ybWF0 X25hbWVfYnVmCj4+ID4gPj4gICpidWYpOz4gPj4gIAo+PiA+ID4+ICAgICAgICBefn5+fn5+fn5+ fn5+fn5+fn5+Cj4+ID4gPj4gCj4+ID4gPj4gQ2FuIHlvdSBwbHMgcmViYXNlIG9udG8gZHJtLW1p c2Mgb3IgbGludXgtbmV4dCBvciBzb21ldGhpbmc/Cj4+ID4gPiAKPj4gPiA+IFRoYXQgd2FzIGJh c2VkIG9uIGFpcmxpZWQvZHJtLW5leHQgKGxhc3QgZmV0Y2hlZCBvbiBTdW5kYXkgSSB0aGluayks Cj4+ID4gPiBJIGNhbiByZWJhc2UgaXQgb24gZHJtLW1pc2MgaWYgaXQgaGVscHMsIGJ1dCBpdCBz ZWVtcyBvbGRlciB0aGFuCj4+ID4gPiBkcm0tbmV4dC4gU2hvdWxkIEkganVzdCByZWJhc2Ugb24g dG9wIG9mIGN1cnJlbnQgaGVhZCBvZiBkcm0tbmV4dD8KPj4gPiAKPj4gPiBJdCBuZWVkcyB0byBi ZSBkcm0tbWlzYyAobGludXgtbmV4dCBkb2Vzbid0IGhhdmUgaXQgeWV0KSBkdWUgdG8gdGhlCj4+ ID4gbmV3IGF0b21pYyBkZWJ1ZyB3b3JrIHRoYXQgd2UganVzdCBsYW5kZWQuIEknbSB3b3JraW5n IG9uIGRybS10aXAgYXMgYQo+PiA+IGRybSBsb2NhbCBpbnRlZ3JhdGlvbiB0cmVlIHRvIGVhc2Ug cGFpbnMgbGlrZSB0aGVzZSBhIGJpdCwgYnV0IHRoYXQKPj4gPiBkb2Vzbid0IHJlYWxseSBleGlz dCB5ZXQuCj4+IAo+PiBJJ20gY29uZnVzZWQgYXMgdG8gaG93IHRoZSBkaWZmZXJlbnQgdHJlZXMg YW5kIGJyYW5jaGVzIG1lcmdlIGJhY2sgdG8KPj4gVG9ydmFsZHMnIHRyZWUgKEknbSBpbnRlcmVz dGVkIGluIHBhcnRpY3VsYXIgaW4gZHJtKSwgYW5kIEknbSBub3Qgc3VyZQo+PiB3aGljaCBicmFu Y2ggeW91IHdhbnQgbWUgdG8gcmViYXNlIG9uIGluIHRoZSBkcm0tbWlzYyB0cmVlIFsxXSwKPj4g ZXNwZWNpYWxseSBzaW5jZSBhbGwgb2YgdGhlbSBhcmUgb2xkZXIgdGhhbiBkcm0tbmV4dCBbMl0u Cj4+IAo+PiBJJ2xsIHRyeSB0byByZWJhc2Ugb24gZHJtLW1pc2MtZml4ZXMgKGN1cnJlbnRseSBh dCA0ZGE1Y2FhNmE2ZjgyY2RhMzE5MykKPj4gYXMgaXQgc291bmRzIGFib3V0IHJpZ2h0LCBidXQg aXQgZG9lc24ndCBhcHBseSBhdCBhbGwsIHNvIGl0J2xsIHRha2UKPj4gYSBsaXR0bGUgd2hpbGUu Cj4KPiBXaGlsZSBhdCBpdCwgY291bGQgeW91IG1ha2UgdGhlIGZ1bmN0aW9uIHJldHVybiBhIGNv bnN0IGNoYXIgKiA/CgpJIHRob3VnaHQgSSBtZW50aW9uZWQgdGhhdCB0b28sIHRob3VnaCBJIGRp ZG4ndCBpbnNpc3QuCgo+IEJ5IHRoZSB3YXksIHdoaWxlIHRoaXMgaXMgYW4gaW1wcm92ZW1lbnQg b3ZlciB0aGUgY3VycmVudCBzaXR1YXRpb24gaW4gdGhhdCBpdCAKPiBmaXhlcyB0aGUgbWlzc2lu ZyBrZnJlZSgpIGlzc3VlLCBJIHdvbmRlciB3aGV0aGVyIHRoZSBwcm9ibGVtIHdlJ3JlIHRyeWlu ZyB0byAKPiBzb2x2ZSBzaG91bGQgYmUgYWRkcmVzc2VkIGF0IGEgbW9yZSBnbG9iYWwgbGV2ZWwu CgpNYXliZSwgYnV0IGxldCdzIG5vdCBibG9jayB0aGlzIG9uZSEKCj4gVGhlIGlzc3VlIGhlcmUg aXMgdGhhdCBwcmludGsgY2FuJ3QgZm9ybWF0IHRoZSBmb3VyY2MgYXMgYSBzdHJpbmcgYnkgaXRz ZWxmLiAKPiBUaGVyZSdzIGEgYnVuY2ggb2YgcGxhY2VzIGluIHRoZSBrZXJuZWwgd2hlcmUgYSBz aW1pbGFyIGZvcm1hdHRpbmcgcHJvYmxlbSAKPiBvY2N1cnMuIEluIGEgZmV3IG9jY2FzaW9ucyBp dCBoYXMgYmVlbiBzb2x2ZWQgYnkgZXh0ZW5kaW5nIHByaW50ayB3aXRoIAo+IGFkZGl0aW9uYWwg Zm9ybWF0IHNwZWNpZmllcnMgKHN1Y2ggYXMgZm9yIE1BQy9JUCBhZGRyZXNzZXMsIEdVSURzLCB2 YXJpb3VzIAo+IGtpbmQgb2YgZGV2aWNlIG5hbWVzLCAuLi4pLiBEUk0gZm91cmNjcyBhcmUgcHJv YmFibHkgdG9vIERSTSBzcGVjaWZpYyB0byBiZSAKPiB3b3J0aCBhIGZvcm1hdCBzcGVjaWZpZXIs IGJ1dCBJIHdvbmRlciB3aGV0aGVyIHdlIGNvdWxkIGludHJvZHVjZSBhIG5ldyAKPiBzcGVjaWZp ZXIgdGhhdCB0YWtlcyBhIGZ1bmN0aW9uIHBvaW50ZXIgYXMgYSBmb3JtYXR0aW5nIGhlbHBlci4g QW5vdGhlciAKPiBzaW1pbGFybHkgY3Jhenkgb3B0aW9uIHdvdWxkIGJlIGEgZm9ybWF0IHNwZWNp ZmllciBmb3Igc3RyaW5ncyB0aGF0IHdvdWxkIGZyZWUgCj4gdGhlIHBhc3NlZCBwb2ludGVyIGFm dGVyIHByaW50aW5nIGl0LgoKSSB0aGluayB0aGVyZSBhcmUgdG9vIG1hbnkgbm9uLXN0YW5kYXJk IGZvcm1hdCBzcGVjaWZpZXJzIGFscmVhZHkuIEkKY2FuJ3QgcmV2aWV3IHRoZSBub24tc3RhbmRh cmQgZm9ybWF0IHN0cmluZ3Mgd2l0aG91dCBsb29raW5nIGF0CkRvY3VtZW50YXRpb24vcHJpbmst Zm9ybWF0cy50eHQgZmlyc3QuIFRoZSBmb3JtYXR0aW5nIGhvb2sgd291bGQgYmUgYQpnZW5lcmlj IGFsdGVybmF0aXZlLCBidXQgdGhhdCdzIG1vcmUgdGhhbiBhIGxpdHRsZSBzY2FyeSBmcm9tIHRo ZQpzZWN1cml0eSBzdGFuZHBvaW50LiBBbmQgd2hhdCBpZiB0aGUgaG9vayBoYXMgdG8gYWxsb2Nh dGUgbWVtb3J5PyBDYW4ndApkbyB0aGF0IGluIGF0b21pYyBjb250ZXh0cy4KCkJSLApKYW5pLgoK Cj4KPj4gQ291bGQgeW91IGdpdmUgbWUgYSBxdWljayBleHBsYW5hdGlvbiBvciBwb2ludCBtZSB0 byBhIGRvYy9wYWdlIHRoYXQKPj4gZXhwbGFpbnMgaG93IHRoZSB2YXJpb3VzIHRyZWVzIGFuZCBi cmFuY2hlcyBnZXQgbWVyZ2VkPwo+PiBJIGdvb2dsZWQgYSBiaXQgYW5kIGZvdW5kIHRoaXMgZG9j IFs0XSBieSBKYW5pLCBidXQgaXQgZG9lc24ndCBtZW50aW9uCj4+IGRybS1taXNjIGZvciBpbnN0 YW5jZSwgc28gSSdtIG5vdCBzdXJlIGhvdyB1cC10by1kYXRlIGFuZAo+PiBub24taW50ZWwtc3Bl Y2lmaWMgaXQgaXMuCj4+IAo+PiBMb29raW5nIGF0IHRoaXMgcGFnZSwgc29tZXRoaW5nIGp1c3Qg b2NjdXJyZWQgdG8gbWU6IGRpZCB5b3UgbWVhbgo+PiBkcm0tZml4ZXMgWzNdLCBpbnN0ZWFkIG9m IG9uZSBvZiB0aGUgYnJhbmNoZXMgb24gZHJtLW1pc2M/Cj4+IAo+PiBDaGVlcnMsCj4+ICAgRXJp Ywo+PiAKPj4gWzFdIGdpdDovL2Fub25naXQuZnJlZWRlc2t0b3Aub3JnL2RybS9kcm0tbWlzYwo+ PiBbMl0gZ2l0Oi8vcGVvcGxlLmZyZWVkZXNrdG9wLm9yZy9+YWlybGllZC9saW51eCBkcm0tbmV4 dAo+PiBbMl0gZ2l0Oi8vcGVvcGxlLmZyZWVkZXNrdG9wLm9yZy9+YWlybGllZC9saW51eCBkcm0t Zml4ZXMKPj4gWzNdIGh0dHBzOi8vMDEub3JnL2xpbnV4Z3JhcGhpY3MvZ2Z4LWRvY3MvbWFpbnRh aW5lci10b29scy9kcm0taW50ZWwuaHRtbAoKLS0gCkphbmkgTmlrdWxhLCBJbnRlbCBPcGVuIFNv dXJjZSBUZWNobm9sb2d5IENlbnRlcgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVl ZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5m by9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754932AbcKJKaS convert rfc822-to-8bit (ORCPT ); Thu, 10 Nov 2016 05:30:18 -0500 Received: from mga07.intel.com ([134.134.136.100]:32140 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752186AbcKJKaR (ORCPT ); Thu, 10 Nov 2016 05:30:17 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,618,1473145200"; d="scan'208";a="784804110" From: Jani Nikula To: Laurent Pinchart , 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 , intel-gfx , Emily Deng , Colin Ian King , Junwei Zhang , Michel =?utf-8?Q?D=C3=A4nzer?= , Alex Deucher , Christian =?utf-8?Q?K=C3=B6nig?= Subject: Re: [Intel-gfx] [PATCH v3] drm: move allocation out of drm_get_format_name() In-Reply-To: <2360827.8WFanMYCQ1@avalon> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20161108101558.ihvrprbbdqjwu5wg@phenom.ffwll.local> <20161109165931.GR25290@imgtec.com> <2360827.8WFanMYCQ1@avalon> Date: Thu, 10 Nov 2016 12:30:09 +0200 Message-ID: <871syjfwzy.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 10 Nov 2016, Laurent Pinchart wrote: > 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 * ? I thought I mentioned that too, though I didn't insist. > 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. Maybe, but let's not block this one! > 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. I think there are too many non-standard format specifiers already. I can't review the non-standard format strings without looking at Documentation/prink-formats.txt first. The formatting hook would be a generic alternative, but that's more than a little scary from the security standpoint. And what if the hook has to allocate memory? Can't do that in atomic contexts. BR, Jani. > >> 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 -- Jani Nikula, Intel Open Source Technology Center