Message-ID: <Z-p2ii-N2-dd_HJ6@phenom.ffwll.local>
Date: Mon, 31 Mar 2025 13:03:38 +0200
From: Simona Vetter <simona.vetter@...ll.ch>
To: Jani Nikula <jani.nikula@...ux.intel.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
	Dave Airlie <airlied@...il.com>, simona.vetter@...ll.ch,
	dri-devel <dri-devel@...ts.freedesktop.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [git pull] drm for 6.15-rc1

Hi Linus,

On Mon, Mar 31, 2025 at 01:17:28PM +0300, Jani Nikula wrote:
> On Fri, 28 Mar 2025, Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> > If you want to do that hdrtest thing, do it as part of your *own*
> > checks. Don't make everybody else see that disgusting thing and have
> > those turds in their trees.
> >
> > I'll just disable it by marking it BROKEN for now. You guys can figure
> > out what you want to do, but no, forcing others to see those things is
> > not the answer.
> 
> Fair. I hear you.

Mea culpa also from the drm maintainer side. As Jani explains below, we
really want to make this and similar things happen in drm, and we thought
this much more limited version would address your concerns. Evidently not.
We also got a heads-up from the Kbuild maintainer that you really don't
like this, and I guess that wasn't clear enough in the pr cover letter.
Plus at that point it was all already in drm-next, and we figured it's not
worth the trouble of pulling it out and doing a separate topic branch pr
so you can check it out separately.

Hindsight and all that.

Cheers, Sima

> 
> > I would suggest you *not* make this part of the Kconfig setup and
> > normal build at all, but be something where *you* can run it as part
> > of your tests (ie do it as a "make drm-hdrtest" kind of thing, not as
> > part of regular builds).
> 
> I would very much prefer for this to be part of the build, just hidden
> behind Kconfig. We're doing build-time checks, and kbuild gives us all
> the machinery to make it happen. Without the dependency tracking you'd
> have to check everything every time, and that's just going to mean
> people won't run it.
> 
> I suggest a Kconfig knob to truly make this opt-in, only for developers
> who actually want it. Not enabled by allmodconfig or allyesconfig or
> even allnoconfig. Only if you manually enable it. And yes, that's how it
> should've been from the start. My bad.
> 
> Below's a patch to make it happen. We'll probably want to add more
> checks like this in the future. We want to catch a whole bunch of build
> issues up front. We want to be clean of e.g. W=1 and kernel-doc issues
> pre-merge instead of doing extra rounds of fixes afterwards.
> 
> BR,
> Jani.
> 
> 
> 
> From 8c709510caab4b4ad6aa73cbcd972f32b58cad8d Mon Sep 17 00:00:00 2001
> From: Jani Nikula <jani.nikula@...el.com>
> Date: Mon, 31 Mar 2025 12:25:45 +0300
> Subject: [PATCH] drm: add config option for extra build-time checks
> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
> Cc: Jani Nikula <jani.nikula@...el.com>
> 
> The DRM subsystem contains additional build-time checks, primarily aimed
> at DRM developers and CI systems. The checks may be overzealous. They
> may slow down or fail the build altogether. They may create excessive
> dependency files in the build tree. They should not be enabled for
> regular builds, and certainly not forced on unsuspecting developers
> running an allyesconfig or allmodconfig build.
> 
> Add config DRM_DISABLE_EXTRA_BUILD_CHECKS, enabled by default as well as
> by allyesconfig/allmodconfig, hiding the extra checks from anyone but
> people who intentionally opt-in for the checks.
> 
> For example, to enable header tests:
> 
> $ scripts/config --disable CONFIG_DRM_DISABLE_EXTRA_BUILD_CHECKS --enable CONFIG_DRM_HEADER_TEST
> $ make olddefconfig
> 
> Reported-by: Linus Torvalds <torvalds@...ux-foundation.org>
> Closes: https://lore.kernel.org/r/CAHk-=wjcdfrDTjzm6J6T-3fxtVyBG7a_0BXc2=mgOuM6KPFnCg@mail.gmail.com
> Fixes: 62ae45687e43 ("drm: ensure drm headers are self-contained and pass kernel-doc")
> Signed-off-by: Jani Nikula <jani.nikula@...el.com>
> ---
>  drivers/gpu/drm/Kconfig | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 2cba2b6ebe1c..5a3fce9ef998 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -489,9 +489,26 @@ config DRM_PRIVACY_SCREEN
>  	bool
>  	default n
>  
> +# Reversed option to disable on allyesconfig/allmodconfig builds
> +config DRM_DISABLE_EXTRA_BUILD_CHECKS
> +	bool "Disable DRM subsystem extra build-time checks"
> +	default y
> +	help
> +	  The DRM subsystem contains additional build-time checks, primarily
> +	  aimed at DRM developers and CI systems. The checks may be
> +	  overzealous. They may slow down or fail the build altogether. They may
> +	  create excessive dependency files in the tree. They should not be
> +	  enabled for regular builds, and thus they are disabled by default.
> +
> +# Proxy config to allow simple "depends on DRM_EXTRA_BUILD_CHECKS"
> +config DRM_EXTRA_BUILD_CHECKS
> +	bool
> +	depends on DRM && EXPERT && DRM_DISABLE_EXTRA_BUILD_CHECKS=n
> +	default !DRM_DISABLE_EXTRA_BUILD_CHECKS
> +
>  config DRM_WERROR
>  	bool "Compile the drm subsystem with warnings as errors"
> -	depends on DRM && EXPERT
> +	depends on DRM_EXTRA_BUILD_CHECKS
>  	depends on !WERROR
>  	default n
>  	help
> @@ -505,7 +522,7 @@ config DRM_WERROR
>  
>  config DRM_HEADER_TEST
>  	bool "Ensure DRM headers are self-contained and pass kernel-doc"
> -	depends on DRM && EXPERT && BROKEN
> +	depends on DRM_EXTRA_BUILD_CHECKS
>  	default n
>  	help
>  	  Ensure the DRM subsystem headers both under drivers/gpu/drm and
> -- 
> 2.39.5
> 
> 
> -- 
> Jani Nikula, Intel

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Powered by blists - more mailing lists