From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A121BC433DB for ; Mon, 15 Feb 2021 13:52:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 68ABC64D9D for ; Mon, 15 Feb 2021 13:52:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229910AbhBONwC (ORCPT ); Mon, 15 Feb 2021 08:52:02 -0500 Received: from mga04.intel.com ([192.55.52.120]:3452 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229805AbhBONui (ORCPT ); Mon, 15 Feb 2021 08:50:38 -0500 IronPort-SDR: BaL9/nUIttOaIv7FOT7OpJSxM2jDEBrk7Qm+TQTycle6UniRyHE5JmPJz9gjEYMKfv0nABD0/Y GiUS+YpbIUpg== X-IronPort-AV: E=McAfee;i="6000,8403,9895"; a="180123256" X-IronPort-AV: E=Sophos;i="5.81,180,1610438400"; d="scan'208";a="180123256" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Feb 2021 05:48:37 -0800 IronPort-SDR: R+vTNA8KsI7v/v3QB9ncTDcyBiWLW0KQZIWz8nfziTZynxrchxmy56k75BSox1RrD4HxmfOCSd esSfNbQ2d5sA== X-IronPort-AV: E=Sophos;i="5.81,180,1610438400"; d="scan'208";a="400656303" Received: from paasikivi.fi.intel.com ([10.237.72.42]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Feb 2021 05:48:33 -0800 Received: from paasikivi.fi.intel.com (localhost [127.0.0.1]) by paasikivi.fi.intel.com (Postfix) with SMTP id D1CBE20345; Mon, 15 Feb 2021 15:48:30 +0200 (EET) Date: Mon, 15 Feb 2021 15:48:30 +0200 From: Sakari Ailus To: Andy Shevchenko Cc: linux-kernel@vger.kernel.org, linux-media@vger.kernel.org, Petr Mladek , Dave Stevenson , dri-devel@lists.freedesktop.org, hverkuil@xs4all.nl, laurent.pinchart@ideasonboard.com, mchehab@kernel.org, Sergey Senozhatsky , Steven Rostedt , Joe Perches , Jani Nikula , Rasmus Villemoes Subject: Re: [PATCH v7 3/3] drm: Switch to %p4cc format modifier Message-ID: <20210215134830.GG3@paasikivi.fi.intel.com> References: <20210215114030.11862-1-sakari.ailus@linux.intel.com> <20210215114030.11862-4-sakari.ailus@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andy, On Mon, Feb 15, 2021 at 03:41:14PM +0200, Andy Shevchenko wrote: > On Mon, Feb 15, 2021 at 01:40:30PM +0200, Sakari Ailus wrote: > > Switch DRM drivers from drm_get_format_name() to %p4cc. This gets rid of a > > large number of temporary variables at the same time. > > What a nice clean up! > Reviewed-by: Andy Shevchenko Thanks! > after addressing nit-picks below. > > Side note (no need to implement esp. right now): it seems often it's coupled > with modifier, would be nice to have them together that %p4ccM or so can do it > in one go. ... > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c > > index 03262472059c..5cf45aa6eedc 100644 > > --- a/drivers/gpu/drm/drm_fourcc.c > > +++ b/drivers/gpu/drm/drm_fourcc.c > > @@ -30,11 +30,6 @@ > > #include > > #include > > > > -static char printable_char(int c) > > -{ > > - return isascii(c) && isprint(c) ? c : '?'; > > -} > > If it goes as an ABI than your dot is incompatible with this and '?' should be > used instead in the patch 1. And I bend towards suggested '?' rather than '.'. > > Also it means that you probably would need different specifiers for full and > short formats. I thought of that, but then the resulting string would be indeed different and comparing short and long formats would be harder. Remember this is for debug prints. If the format is too long, then it should be made shorter, but during the earlier review rounds people have expressed interest in having this information there. "?" can be expanded by the shell whereas "." is not. If DRM folks think we should go back to "?" I'm fine with that. Also note that there's something wrong with the fourcc code to begin with if it's got either "." or "?". -- Regards, Sakari Ailus From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC7EFC433DB for ; Mon, 15 Feb 2021 13:48:39 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7C47D64E2C for ; Mon, 15 Feb 2021 13:48:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7C47D64E2C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E10646E3DF; Mon, 15 Feb 2021 13:48:38 +0000 (UTC) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id 148346E1A4 for ; Mon, 15 Feb 2021 13:48:38 +0000 (UTC) IronPort-SDR: euQSByoYX23/Ny030A1G6ceIyJe9KrjIeTIts76wBZuUDmOTddMayZkACvHk7qgun/Q9Bm62Sd ir2/5ZhdfMHA== X-IronPort-AV: E=McAfee;i="6000,8403,9895"; a="246746882" X-IronPort-AV: E=Sophos;i="5.81,180,1610438400"; d="scan'208";a="246746882" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Feb 2021 05:48:35 -0800 IronPort-SDR: R+vTNA8KsI7v/v3QB9ncTDcyBiWLW0KQZIWz8nfziTZynxrchxmy56k75BSox1RrD4HxmfOCSd esSfNbQ2d5sA== X-IronPort-AV: E=Sophos;i="5.81,180,1610438400"; d="scan'208";a="400656303" Received: from paasikivi.fi.intel.com ([10.237.72.42]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Feb 2021 05:48:33 -0800 Received: from paasikivi.fi.intel.com (localhost [127.0.0.1]) by paasikivi.fi.intel.com (Postfix) with SMTP id D1CBE20345; Mon, 15 Feb 2021 15:48:30 +0200 (EET) Date: Mon, 15 Feb 2021 15:48:30 +0200 From: Sakari Ailus To: Andy Shevchenko Subject: Re: [PATCH v7 3/3] drm: Switch to %p4cc format modifier Message-ID: <20210215134830.GG3@paasikivi.fi.intel.com> References: <20210215114030.11862-1-sakari.ailus@linux.intel.com> <20210215114030.11862-4-sakari.ailus@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Petr Mladek , Dave Stevenson , Rasmus Villemoes , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, hverkuil@xs4all.nl, Sergey Senozhatsky , Steven Rostedt , laurent.pinchart@ideasonboard.com, Joe Perches , mchehab@kernel.org, linux-media@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Andy, On Mon, Feb 15, 2021 at 03:41:14PM +0200, Andy Shevchenko wrote: > On Mon, Feb 15, 2021 at 01:40:30PM +0200, Sakari Ailus wrote: > > Switch DRM drivers from drm_get_format_name() to %p4cc. This gets rid of a > > large number of temporary variables at the same time. > > What a nice clean up! > Reviewed-by: Andy Shevchenko Thanks! > after addressing nit-picks below. > > Side note (no need to implement esp. right now): it seems often it's coupled > with modifier, would be nice to have them together that %p4ccM or so can do it > in one go. ... > > diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c > > index 03262472059c..5cf45aa6eedc 100644 > > --- a/drivers/gpu/drm/drm_fourcc.c > > +++ b/drivers/gpu/drm/drm_fourcc.c > > @@ -30,11 +30,6 @@ > > #include > > #include > > > > -static char printable_char(int c) > > -{ > > - return isascii(c) && isprint(c) ? c : '?'; > > -} > > If it goes as an ABI than your dot is incompatible with this and '?' should be > used instead in the patch 1. And I bend towards suggested '?' rather than '.'. > > Also it means that you probably would need different specifiers for full and > short formats. I thought of that, but then the resulting string would be indeed different and comparing short and long formats would be harder. Remember this is for debug prints. If the format is too long, then it should be made shorter, but during the earlier review rounds people have expressed interest in having this information there. "?" can be expanded by the shell whereas "." is not. If DRM folks think we should go back to "?" I'm fine with that. Also note that there's something wrong with the fourcc code to begin with if it's got either "." or "?". -- Regards, Sakari Ailus _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel