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 Content-Length: 4578 Lines: 102 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