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