diff mbox series

drm/i915: Restrict DRM_I915_DEBUG to developer builds

Message ID 20210122091058.5145-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series drm/i915: Restrict DRM_I915_DEBUG to developer builds | expand

Commit Message

Chris Wilson Jan. 22, 2021, 9:10 a.m. UTC
Let's not encourage everybody to build i915's debug code, and certainly
not the build robots who need to scrutinise the production build. Since
CI will complain if the debug build is broken, having the other build
bots focus on the builds we don't cover ourselves should improve the
build coverage.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Fixes: 4f86975f539d ("drm/i915: Add DEBUG_GEM to the recommended CI config")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/Kconfig.debug | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jani Nikula Jan. 22, 2021, 9:35 a.m. UTC | #1
On Fri, 22 Jan 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Let's not encourage everybody to build i915's debug code, and certainly
> not the build robots who need to scrutinise the production build. Since
> CI will complain if the debug build is broken, having the other build
> bots focus on the builds we don't cover ourselves should improve the
> build coverage.

I don't disagree with this, although I wrote in another mail that I'm
not sure DRM_I915_DEBUG should select DRM_I915_WERROR. I think they
should be two separate things.

Even so, for this change,

Acked-by: Jani Nikula <jani.nikula@intel.com>

>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Fixes: 4f86975f539d ("drm/i915: Add DEBUG_GEM to the recommended CI config")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/Kconfig.debug | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> index e2d33f1abb3d..5b457fb2d268 100644
> --- a/drivers/gpu/drm/i915/Kconfig.debug
> +++ b/drivers/gpu/drm/i915/Kconfig.debug
> @@ -19,6 +19,8 @@ config DRM_I915_WERROR
>  config DRM_I915_DEBUG
>  	bool "Enable additional driver debugging"
>  	depends on DRM_I915
> +	depends on EXPERT # only for developers
> +	depends on !COMPILE_TEST # never built by robots
>  	select PCI_MSI # ... for iommu enabled by default
>  	select IOMMU_API
>  	select IOMMU_IOVA
Chris Wilson Jan. 22, 2021, 9:39 a.m. UTC | #2
Quoting Jani Nikula (2021-01-22 09:35:42)
> On Fri, 22 Jan 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Let's not encourage everybody to build i915's debug code, and certainly
> > not the build robots who need to scrutinise the production build. Since
> > CI will complain if the debug build is broken, having the other build
> > bots focus on the builds we don't cover ourselves should improve the
> > build coverage.
> 
> I don't disagree with this, although I wrote in another mail that I'm
> not sure DRM_I915_DEBUG should select DRM_I915_WERROR. I think they
> should be two separate things.

DRM_I915_DEBUG is the CI catch-all, and so the build we recommend
developers try at least once. But the original purpose of DRM_I915_DEBUG
was to be able to switch on features for CI without having to bug Tomi.
-Chris
Jani Nikula Jan. 22, 2021, 9:43 a.m. UTC | #3
On Fri, 22 Jan 2021, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Fri, 22 Jan 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Let's not encourage everybody to build i915's debug code, and certainly
>> not the build robots who need to scrutinise the production build. Since
>> CI will complain if the debug build is broken, having the other build
>> bots focus on the builds we don't cover ourselves should improve the
>> build coverage.
>
> I don't disagree with this, although I wrote in another mail that I'm
> not sure DRM_I915_DEBUG should select DRM_I915_WERROR. I think they
> should be two separate things.
>
> Even so, for this change,
>
> Acked-by: Jani Nikula <jani.nikula@intel.com>

Musing, a compile test could still enable all the individual knobs in
Kconfig.debug, right?

How would this work in Kconfig?

+if DRM_I915 && EXPERT && !COMPILE_TEST
 menu "drm/i915 Debugging"
-depends on DRM_I915
-depends on EXPERT
 source "drivers/gpu/drm/i915/Kconfig.debug"
 endmenu
+endif


BR,
Jani.

>
>>
>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> Fixes: 4f86975f539d ("drm/i915: Add DEBUG_GEM to the recommended CI config")
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>  drivers/gpu/drm/i915/Kconfig.debug | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
>> index e2d33f1abb3d..5b457fb2d268 100644
>> --- a/drivers/gpu/drm/i915/Kconfig.debug
>> +++ b/drivers/gpu/drm/i915/Kconfig.debug
>> @@ -19,6 +19,8 @@ config DRM_I915_WERROR
>>  config DRM_I915_DEBUG
>>  	bool "Enable additional driver debugging"
>>  	depends on DRM_I915
>> +	depends on EXPERT # only for developers
>> +	depends on !COMPILE_TEST # never built by robots
>>  	select PCI_MSI # ... for iommu enabled by default
>>  	select IOMMU_API
>>  	select IOMMU_IOVA
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index e2d33f1abb3d..5b457fb2d268 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -19,6 +19,8 @@  config DRM_I915_WERROR
 config DRM_I915_DEBUG
 	bool "Enable additional driver debugging"
 	depends on DRM_I915
+	depends on EXPERT # only for developers
+	depends on !COMPILE_TEST # never built by robots
 	select PCI_MSI # ... for iommu enabled by default
 	select IOMMU_API
 	select IOMMU_IOVA