From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [Intel-gfx] [PATCH v3] drm: move allocation out of drm_get_format_name() Date: Thu, 10 Nov 2016 12:59:27 +0200 Message-ID: <1577179.SKedfdZVPT@avalon> References: <20161108101558.ihvrprbbdqjwu5wg@phenom.ffwll.local> <2360827.8WFanMYCQ1@avalon> <871syjfwzy.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <871syjfwzy.fsf@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jani Nikula 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 =?ISO-8859-1?Q?D=E4nzer?= , Linux Kernel Mailing List , Alex Deucher , Colin Ian King List-Id: dri-devel@lists.freedesktop.org SGkgSmFuaSwKCk9uIFRodXJzZGF5IDEwIE5vdiAyMDE2IDEyOjMwOjA5IEphbmkgTmlrdWxhIHdy b3RlOgo+IE9uIFRodSwgMTAgTm92IDIwMTYsIExhdXJlbnQgUGluY2hhcnQgd3JvdGU6Cj4gPiBP biBXZWRuZXNkYXkgMDkgTm92IDIwMTYgMTY6NTk6MzEgRXJpYyBFbmdlc3Ryb20gd3JvdGU6Cj4g Pj4gT24gV2VkbmVzZGF5LCAyMDE2LTExLTA5IDE0OjEzOjQwICswMTAwLCBEYW5pZWwgVmV0dGVy IHdyb3RlOgo+ID4+PiBPbiBXZWQsIE5vdiA5LCAyMDE2IGF0IDEyOjQyIFBNLCBFcmljIEVuZ2Vz dHJvbSB3cm90ZToKPiA+Pj4+PiBXZWxsLCBoYWQgdG8gZHJvcCBpdCBhZ2FpbiBzaW5jZSBpdCBk aWRuJ3QgY29tcGlsZToKPiA+Pj4+PiAgIENDIFtNXSAgZHJpdmVycy9ncHUvZHJtL2RybV9ibGVu ZC5vCj4gPj4+Pj4gCj4gPj4+Pj4gZHJpdmVycy9ncHUvZHJtL2RybV9hdG9taWMuYzogSW4gZnVu Y3Rpb24KPiA+Pj4+PiDigJhkcm1fYXRvbWljX3BsYW5lX3ByaW50X3N0YXRl4oCZOgo+ID4+Pj4+ IGRyaXZlcnMvZ3B1L2RybS9kcm1fYXRvbWljLmM6OTIwOjU6IGVycm9yOiB0b28gZmV3IGFyZ3Vt ZW50cyB0bwo+ID4+Pj4+IGZ1bmN0aW9uIOKAmGRybV9nZXRfZm9ybWF0X25hbWXigJk+ID4+Cj4g Pj4+Pj4gICAgICBkcm1fZ2V0X2Zvcm1hdF9uYW1lKGZiLT5waXhlbF9mb3JtYXQpKTsKPiA+Pj4+ PiAgICAgIF5+fn5+fn5+fn5+fn5+fn5+fn4KPiA+Pj4+PiBJbiBmaWxlIGluY2x1ZGVkIGZyb20g Li9pbmNsdWRlL2RybS9kcm1QLmg6NzE6MCwKPiA+Pj4+PiAgICAgICAgICAgICAgICAgIGZyb20g ZHJpdmVycy9ncHUvZHJtL2RybV9hdG9taWMuYzoyOToKPiA+Pj4+PiAuL2luY2x1ZGUvZHJtL2Ry bV9mb3VyY2MuaDo2NTo3OiBub3RlOiBkZWNsYXJlZCBoZXJlCj4gPj4+Pj4gIGNoYXIgKmRybV9n ZXRfZm9ybWF0X25hbWUodWludDMyX3QgZm9ybWF0LCBzdHJ1Y3QKPiA+Pj4+PiAgZHJtX2Zvcm1h dF9uYW1lX2J1ZiAqYnVmKTsKPiA+Pj4+PiAgICAgICAgXn5+fn5+fn5+fn5+fn5+fn5+fgo+ID4+ Pj4+IAo+ID4+Pj4+IENhbiB5b3UgcGxzIHJlYmFzZSBvbnRvIGRybS1taXNjIG9yIGxpbnV4LW5l eHQgb3Igc29tZXRoaW5nPwo+ID4+Pj4gCj4gPj4+PiBUaGF0IHdhcyBiYXNlZCBvbiBhaXJsaWVk L2RybS1uZXh0IChsYXN0IGZldGNoZWQgb24gU3VuZGF5IEkgdGhpbmspLAo+ID4+Pj4gSSBjYW4g cmViYXNlIGl0IG9uIGRybS1taXNjIGlmIGl0IGhlbHBzLCBidXQgaXQgc2VlbXMgb2xkZXIgdGhh bgo+ID4+Pj4gZHJtLW5leHQuIFNob3VsZCBJIGp1c3QgcmViYXNlIG9uIHRvcCBvZiBjdXJyZW50 IGhlYWQgb2YgZHJtLW5leHQ/Cj4gPj4+IAo+ID4+PiBJdCBuZWVkcyB0byBiZSBkcm0tbWlzYyAo bGludXgtbmV4dCBkb2Vzbid0IGhhdmUgaXQgeWV0KSBkdWUgdG8gdGhlCj4gPj4+IG5ldyBhdG9t aWMgZGVidWcgd29yayB0aGF0IHdlIGp1c3QgbGFuZGVkLiBJJ20gd29ya2luZyBvbiBkcm0tdGlw IGFzIGEKPiA+Pj4gZHJtIGxvY2FsIGludGVncmF0aW9uIHRyZWUgdG8gZWFzZSBwYWlucyBsaWtl IHRoZXNlIGEgYml0LCBidXQgdGhhdAo+ID4+PiBkb2Vzbid0IHJlYWxseSBleGlzdCB5ZXQuCj4g Pj4gCj4gPj4gSSdtIGNvbmZ1c2VkIGFzIHRvIGhvdyB0aGUgZGlmZmVyZW50IHRyZWVzIGFuZCBi cmFuY2hlcyBtZXJnZSBiYWNrIHRvCj4gPj4gVG9ydmFsZHMnIHRyZWUgKEknbSBpbnRlcmVzdGVk IGluIHBhcnRpY3VsYXIgaW4gZHJtKSwgYW5kIEknbSBub3Qgc3VyZQo+ID4+IHdoaWNoIGJyYW5j aCB5b3Ugd2FudCBtZSB0byByZWJhc2Ugb24gaW4gdGhlIGRybS1taXNjIHRyZWUgWzFdLAo+ID4+ IGVzcGVjaWFsbHkgc2luY2UgYWxsIG9mIHRoZW0gYXJlIG9sZGVyIHRoYW4gZHJtLW5leHQgWzJd Lgo+ID4+IAo+ID4+IEknbGwgdHJ5IHRvIHJlYmFzZSBvbiBkcm0tbWlzYy1maXhlcyAoY3VycmVu dGx5IGF0IDRkYTVjYWE2YTZmODJjZGEzMTkzKQo+ID4+IGFzIGl0IHNvdW5kcyBhYm91dCByaWdo dCwgYnV0IGl0IGRvZXNuJ3QgYXBwbHkgYXQgYWxsLCBzbyBpdCdsbCB0YWtlCj4gPj4gYSBsaXR0 bGUgd2hpbGUuCj4gPiAKPiA+IFdoaWxlIGF0IGl0LCBjb3VsZCB5b3UgbWFrZSB0aGUgZnVuY3Rp b24gcmV0dXJuIGEgY29uc3QgY2hhciAqID8KPiAKPiBJIHRob3VnaHQgSSBtZW50aW9uZWQgdGhh dCB0b28sIHRob3VnaCBJIGRpZG4ndCBpbnNpc3QuCgpZb3UgZGlkLCBhbmQgSSB0aGluayBFcmlj IGFncmVlZCB0byBjaGFuZ2UgdGhhdC4KCj4gPiBCeSB0aGUgd2F5LCB3aGlsZSB0aGlzIGlzIGFu IGltcHJvdmVtZW50IG92ZXIgdGhlIGN1cnJlbnQgc2l0dWF0aW9uIGluCj4gPiB0aGF0IGl0IGZp eGVzIHRoZSBtaXNzaW5nIGtmcmVlKCkgaXNzdWUsIEkgd29uZGVyIHdoZXRoZXIgdGhlIHByb2Js ZW0KPiA+IHdlJ3JlIHRyeWluZyB0byBzb2x2ZSBzaG91bGQgYmUgYWRkcmVzc2VkIGF0IGEgbW9y ZSBnbG9iYWwgbGV2ZWwuCj4gCj4gTWF5YmUsIGJ1dCBsZXQncyBub3QgYmxvY2sgdGhpcyBvbmUh CgpTdXJlLCB0aGF0IHdhc24ndCB0aGUgaW50ZW50IG9mIG15IGUtbWFpbC4KCj4gPiBUaGUgaXNz dWUgaGVyZSBpcyB0aGF0IHByaW50ayBjYW4ndCBmb3JtYXQgdGhlIGZvdXJjYyBhcyBhIHN0cmlu ZyBieQo+ID4gaXRzZWxmLiBUaGVyZSdzIGEgYnVuY2ggb2YgcGxhY2VzIGluIHRoZSBrZXJuZWwg d2hlcmUgYSBzaW1pbGFyCj4gPiBmb3JtYXR0aW5nIHByb2JsZW0gb2NjdXJzLiBJbiBhIGZldyBv Y2Nhc2lvbnMgaXQgaGFzIGJlZW4gc29sdmVkIGJ5Cj4gPiBleHRlbmRpbmcgcHJpbnRrIHdpdGgg YWRkaXRpb25hbCBmb3JtYXQgc3BlY2lmaWVycyAoc3VjaCBhcyBmb3IgTUFDL0lQCj4gPiBhZGRy ZXNzZXMsIEdVSURzLCB2YXJpb3VzIGtpbmQgb2YgZGV2aWNlIG5hbWVzLCAuLi4pLiBEUk0gZm91 cmNjcyBhcmUKPiA+IHByb2JhYmx5IHRvbyBEUk0gc3BlY2lmaWMgdG8gYmUgd29ydGggYSBmb3Jt YXQgc3BlY2lmaWVyLCBidXQgSSB3b25kZXIKPiA+IHdoZXRoZXIgd2UgY291bGQgaW50cm9kdWNl IGEgbmV3IHNwZWNpZmllciB0aGF0IHRha2VzIGEgZnVuY3Rpb24gcG9pbnRlcgo+ID4gYXMgYSBm b3JtYXR0aW5nIGhlbHBlci4gQW5vdGhlciBzaW1pbGFybHkgY3Jhenkgb3B0aW9uIHdvdWxkIGJl IGEgZm9ybWF0Cj4gPiBzcGVjaWZpZXIgZm9yIHN0cmluZ3MgdGhhdCB3b3VsZCBmcmVlIHRoZSBw YXNzZWQgcG9pbnRlciBhZnRlciBwcmludGluZwo+ID4gaXQuCj4gCj4gSSB0aGluayB0aGVyZSBh cmUgdG9vIG1hbnkgbm9uLXN0YW5kYXJkIGZvcm1hdCBzcGVjaWZpZXJzIGFscmVhZHkuIEkKPiBj YW4ndCByZXZpZXcgdGhlIG5vbi1zdGFuZGFyZCBmb3JtYXQgc3RyaW5ncyB3aXRob3V0IGxvb2tp bmcgYXQKPiBEb2N1bWVudGF0aW9uL3ByaW5rLWZvcm1hdHMudHh0IGZpcnN0LiBUaGUgZm9ybWF0 dGluZyBob29rIHdvdWxkIGJlIGEKPiBnZW5lcmljIGFsdGVybmF0aXZlLCBidXQgdGhhdCdzIG1v cmUgdGhhbiBhIGxpdHRsZSBzY2FyeSBmcm9tIHRoZQo+IHNlY3VyaXR5IHN0YW5kcG9pbnQuIEFu ZCB3aGF0IGlmIHRoZSBob29rIGhhcyB0byBhbGxvY2F0ZSBtZW1vcnk/IENhbid0Cj4gZG8gdGhh dCBpbiBhdG9taWMgY29udGV4dHMuCgpUaGVyZSBhcmUgbG90cyBvZiBkZXRhaWxzIHRvIHNvcnQg b3V0IG9idmlvdXNseSBhbmQgSSBkb24ndCBoYXZlIGFuIGFuc3dlciB0byAKYWxsIHF1ZXN0aW9u cyB5ZXQuIEkgdGhpbmsgaXQgd291bGQgYmUgd29ydGggcmVzZWFyY2hpbmcgdGhpcywgYXMgdGhl IHByb2JsZW0gCmlzbid0IHNwZWNpZmljIHRvIERSTS9LTVMuCgo+ID4+IENvdWxkIHlvdSBnaXZl IG1lIGEgcXVpY2sgZXhwbGFuYXRpb24gb3IgcG9pbnQgbWUgdG8gYSBkb2MvcGFnZSB0aGF0Cj4g Pj4gZXhwbGFpbnMgaG93IHRoZSB2YXJpb3VzIHRyZWVzIGFuZCBicmFuY2hlcyBnZXQgbWVyZ2Vk Pwo+ID4+IEkgZ29vZ2xlZCBhIGJpdCBhbmQgZm91bmQgdGhpcyBkb2MgWzRdIGJ5IEphbmksIGJ1 dCBpdCBkb2Vzbid0IG1lbnRpb24KPiA+PiBkcm0tbWlzYyBmb3IgaW5zdGFuY2UsIHNvIEknbSBu b3Qgc3VyZSBob3cgdXAtdG8tZGF0ZSBhbmQKPiA+PiBub24taW50ZWwtc3BlY2lmaWMgaXQgaXMu Cj4gPj4gCj4gPj4gTG9va2luZyBhdCB0aGlzIHBhZ2UsIHNvbWV0aGluZyBqdXN0IG9jY3VycmVk IHRvIG1lOiBkaWQgeW91IG1lYW4KPiA+PiBkcm0tZml4ZXMgWzNdLCBpbnN0ZWFkIG9mIG9uZSBv ZiB0aGUgYnJhbmNoZXMgb24gZHJtLW1pc2M/Cj4gPj4gCj4gPj4gQ2hlZXJzLAo+ID4+IAo+ID4+ ICAgRXJpYwo+ID4+IAo+ID4+IFsxXSBnaXQ6Ly9hbm9uZ2l0LmZyZWVkZXNrdG9wLm9yZy9kcm0v ZHJtLW1pc2MKPiA+PiBbMl0gZ2l0Oi8vcGVvcGxlLmZyZWVkZXNrdG9wLm9yZy9+YWlybGllZC9s aW51eCBkcm0tbmV4dAo+ID4+IFsyXSBnaXQ6Ly9wZW9wbGUuZnJlZWRlc2t0b3Aub3JnL35haXJs aWVkL2xpbnV4IGRybS1maXhlcwo+ID4+IFszXSBodHRwczovLzAxLm9yZy9saW51eGdyYXBoaWNz L2dmeC1kb2NzL21haW50YWluZXItdG9vbHMvZHJtLWludGVsLmh0bWwKCi0tIApSZWdhcmRzLAoK TGF1cmVudCBQaW5jaGFydAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0 b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJp LWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755259AbcKJK7f (ORCPT ); Thu, 10 Nov 2016 05:59:35 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:52197 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754745AbcKJK70 (ORCPT ); Thu, 10 Nov 2016 05:59:26 -0500 From: Laurent Pinchart To: Jani Nikula Cc: Eric Engestrom , 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 =?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:59:27 +0200 Message-ID: <1577179.SKedfdZVPT@avalon> User-Agent: KMail/4.14.10 (Linux/4.8.6-gentoo; KDE/4.14.24; x86_64; ; ) In-Reply-To: <871syjfwzy.fsf@intel.com> References: <20161108101558.ihvrprbbdqjwu5wg@phenom.ffwll.local> <2360827.8WFanMYCQ1@avalon> <871syjfwzy.fsf@intel.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 uAAAxhSc022368 Hi Jani, On Thursday 10 Nov 2016 12:30:09 Jani Nikula wrote: > On Thu, 10 Nov 2016, Laurent Pinchart wrote: > > 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. You did, and I think Eric agreed to change that. > > 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! Sure, that wasn't the intent of my e-mail. > > 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. There are lots of details to sort out obviously and I don't have an answer to all questions yet. I think it would be worth researching this, as the problem isn't specific to DRM/KMS. > >> 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