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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id AE16EC433EF for ; Tue, 28 Jun 2022 14:32:32 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1EBF110E73D; Tue, 28 Jun 2022 14:32:32 +0000 (UTC) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5E55010E73D for ; Tue, 28 Jun 2022 14:32:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1656426751; x=1687962751; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=YXDLiPRwMjRrEraaUuxs6lSyjUWqeHOEiNIpor3pc88=; b=boImtMj7A+Jx0X35hY+BkMaW1qscv/nCz/BjmSsRLE6BtXbCVH/v5+WA ZYSmQsZ3UUz1TOxqSzg8FYDnalbAncuEHGMSEBQ2BYRM5JL1LOP7xNGJL yx0X8qWgcMD56utVcqGkwOO+I7WrL5N/EglLFNfUqd9zevluNYUayENj3 HJKeT/NmydW3n1xU2Ao1XYTTLqJnxdY87TTk/0gxaz+VARiaDsAksgWMN SD6LopabEd2f8+dcSABcIVFmPUw+WKOKCebuEnPAGsRoMhoZLIaxwveeC yOCpeZSJOZ08BxPNLNltZWckFJxGkNl8wcn4hTlEqwP6XkY2gF+1iv9dy w==; X-IronPort-AV: E=McAfee;i="6400,9594,10391"; a="307239789" X-IronPort-AV: E=Sophos;i="5.92,229,1650956400"; d="scan'208";a="307239789" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jun 2022 07:32:30 -0700 X-IronPort-AV: E=Sophos;i="5.92,229,1650956400"; d="scan'208";a="646935490" Received: from mwardyn-mobl2.ger.corp.intel.com (HELO localhost) ([10.252.49.11]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jun 2022 07:32:29 -0700 From: Jani Nikula To: Kevin Brace , dri-devel@lists.freedesktop.org Subject: Re: [PATCH 04/28] drm/via: Add via_drv.h In-Reply-To: <20220624202633.3978-5-kevinbrace@gmx.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20220624202633.3978-1-kevinbrace@gmx.com> <20220624202633.3978-5-kevinbrace@gmx.com> Date: Tue, 28 Jun 2022 17:32:26 +0300 Message-ID: <87bkucrh8l.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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: Kevin Brace Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Fri, 24 Jun 2022, Kevin Brace wrote: > From: Kevin Brace > > Main header file for the module. I sincerely suggest you reconsider this "main header file" approach. It seems like a nice idea to only have to include one header in each source file. Kind of feels cleaner in the C source. However, you'll end up recompiling your entire driver every time anything changes in this header, or in any of the headers it includes. Initially, it doesn't seem like much, for a small driver, etc. but it'll end up hurting you more than you'll know. For example, imagine a git bisect that's supposed to funnel towards the regressing commit, but every build for every commit takes just as long because everything gets rebuilt. And the bigger the driver grows, the harder it is to untangle this. Trust me, I know. I've spent a lot of time and effort trying to untangle a similar mess in i915. It's really, really painful. I suggest: * Split the header, add corresponding .h for each .c for the stuff it exposes. * Split the types and the macros to separate headers too. * Avoid including any headers from headers. Use forward declarations as much as possible. * Include minimal headers from headers. And from .c files too, but it's of lesser importance. * Avoid static inlines in headers, if they require you to include other headers. At the end of the day, it's your driver, and your call. But please do give it some thought. BR, Jani. > > Signed-off-by: Kevin Brace > --- > drivers/gpu/drm/via/via_drv.h | 437 ++++++++++++++++++++++++++++++++++ > 1 file changed, 437 insertions(+) > create mode 100644 drivers/gpu/drm/via/via_drv.h > > diff --git a/drivers/gpu/drm/via/via_drv.h b/drivers/gpu/drm/via/via_drv.h > new file mode 100644 > index 000000000000..330ab8905417 > --- /dev/null > +++ b/drivers/gpu/drm/via/via_drv.h > @@ -0,0 +1,437 @@ > +/* > + * Copyright =C2=A9 2019 Kevin Brace. > + * Copyright 2012 James Simmons. All Rights Reserved. > + * > + * Permission is hereby granted, free of charge, to any person obtaining= a > + * copy of this software and associated documentation files (the "Softwa= re"), > + * to deal in the Software without restriction, including without limita= tion > + * the rights to use, copy, modify, merge, publish, distribute, sub lice= nse, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the > + * next paragraph) shall be included in all copies or substantial portio= ns > + * of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRE= SS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILI= TY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SH= ALL > + * THE AUTHOR(S) OR COPYRIGHT HOLDER(S) BE LIABLE FOR ANY CLAIM, DAMAGES= OR > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR= OTHER > + * DEALINGS IN THE SOFTWARE. > + * > + * Author(s): > + * Kevin Brace > + * James Simmons > + */ > + > +#ifndef _VIA_DRV_H > +#define _VIA_DRV_H > + > + > +#include > + > +#include