drm/i915: Don't select BROKEN
diff mbox series

Message ID 20191105193829.11599-1-daniel.vetter@ffwll.ch
State New
Headers show
Series
  • drm/i915: Don't select BROKEN
Related show

Commit Message

Daniel Vetter Nov. 5, 2019, 7:38 p.m. UTC
It's broken.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
References: https://lists.freedesktop.org/archives/dri-devel/2019-November/242625.html
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
Note: Probably best to apply this directly onto drm-next to avoid
having drm-next dropped from linux-next until the next pull request.
-Daniel
---
 drivers/gpu/drm/i915/Kconfig.debug | 1 -
 1 file changed, 1 deletion(-)

Comments

Chris Wilson Nov. 5, 2019, 8:34 p.m. UTC | #1
Quoting Daniel Vetter (2019-11-05 19:38:29)
> It's broken.

So is the code that depends on it. Which is the entire point.

Don't select it then.
-Chris
Daniel Vetter Nov. 5, 2019, 8:37 p.m. UTC | #2
On Tue, Nov 5, 2019 at 9:34 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Daniel Vetter (2019-11-05 19:38:29)
> > It's broken.
>
> So is the code that depends on it. Which is the entire point.
>
> Don't select it then.

See the mail from Stephen, it breaks autobuilders. So we need to do
this explicitly, i.e. if you want to set this, you need to explicitly
enable CONFIG_BROKEN first. Auto-enabling CONFIG_BROKEN also somewhat
defeats the point of this debug option, since it doesn't force users
to do the multi-step thing anymore.
-Daniel
Chris Wilson Nov. 5, 2019, 8:38 p.m. UTC | #3
Quoting Daniel Vetter (2019-11-05 19:38:29)
> It's broken.
> 
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> References: https://lists.freedesktop.org/archives/dri-devel/2019-November/242625.html
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> Note: Probably best to apply this directly onto drm-next to avoid
> having drm-next dropped from linux-next until the next pull request.
> -Daniel
> ---
>  drivers/gpu/drm/i915/Kconfig.debug | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> index ef123eb29168..d2ba8f7e5e50 100644
> --- a/drivers/gpu/drm/i915/Kconfig.debug
> +++ b/drivers/gpu/drm/i915/Kconfig.debug
> @@ -44,7 +44,6 @@ config DRM_I915_DEBUG
>         select DRM_I915_SELFTEST
>         select DRM_I915_DEBUG_RUNTIME_PM
>         select DRM_I915_DEBUG_MMIO
> -       select BROKEN # for prototype uAPI

You have to replace it with another secret bool as you cannot otherwise
enable CONFIG_BROKEN in .config.
-Chris
Daniel Vetter Nov. 5, 2019, 8:58 p.m. UTC | #4
On Tue, Nov 5, 2019 at 9:38 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Quoting Daniel Vetter (2019-11-05 19:38:29)
> > It's broken.
> >
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > References: https://lists.freedesktop.org/archives/dri-devel/2019-November/242625.html
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > Note: Probably best to apply this directly onto drm-next to avoid
> > having drm-next dropped from linux-next until the next pull request.
> > -Daniel
> > ---
> >  drivers/gpu/drm/i915/Kconfig.debug | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> > index ef123eb29168..d2ba8f7e5e50 100644
> > --- a/drivers/gpu/drm/i915/Kconfig.debug
> > +++ b/drivers/gpu/drm/i915/Kconfig.debug
> > @@ -44,7 +44,6 @@ config DRM_I915_DEBUG
> >         select DRM_I915_SELFTEST
> >         select DRM_I915_DEBUG_RUNTIME_PM
> >         select DRM_I915_DEBUG_MMIO
> > -       select BROKEN # for prototype uAPI
>
> You have to replace it with another secret bool as you cannot otherwise
> enable CONFIG_BROKEN in .config.

Or this:

diff --git a/init/Kconfig b/init/Kconfig
index b4daad2bac23..4dbea1b9e6bb 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -75,6 +75,7 @@ menu "General setup"

 config BROKEN
        bool
+       default y

 config BROKEN_ON_SMP
        bool

Either way it needs to be in topic/core-for-CI, not in any official
tree. Because if you allow autobuilders to enable CONFIG_BROKEN, no
matter how well hidden, they'll all break. You can also just revert my
patch that Dave pushed to drm-next (to get us back into the linux-next
club).
-Daniel
Chris Wilson Nov. 5, 2019, 9:03 p.m. UTC | #5
Quoting Daniel Vetter (2019-11-05 20:58:25)
> On Tue, Nov 5, 2019 at 9:38 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Quoting Daniel Vetter (2019-11-05 19:38:29)
> > > It's broken.
> > >
> > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > References: https://lists.freedesktop.org/archives/dri-devel/2019-November/242625.html
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > > Note: Probably best to apply this directly onto drm-next to avoid
> > > having drm-next dropped from linux-next until the next pull request.
> > > -Daniel
> > > ---
> > >  drivers/gpu/drm/i915/Kconfig.debug | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> > > index ef123eb29168..d2ba8f7e5e50 100644
> > > --- a/drivers/gpu/drm/i915/Kconfig.debug
> > > +++ b/drivers/gpu/drm/i915/Kconfig.debug
> > > @@ -44,7 +44,6 @@ config DRM_I915_DEBUG
> > >         select DRM_I915_SELFTEST
> > >         select DRM_I915_DEBUG_RUNTIME_PM
> > >         select DRM_I915_DEBUG_MMIO
> > > -       select BROKEN # for prototype uAPI
> >
> > You have to replace it with another secret bool as you cannot otherwise
> > enable CONFIG_BROKEN in .config.
> 
> Or this:
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index b4daad2bac23..4dbea1b9e6bb 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -75,6 +75,7 @@ menu "General setup"
> 
>  config BROKEN
>         bool
> +       default y
> 
>  config BROKEN_ON_SMP
>         bool
> 
> Either way it needs to be in topic/core-for-CI, not in any official
> tree. Because if you allow autobuilders to enable CONFIG_BROKEN, no
> matter how well hidden, they'll all break. You can also just revert my
> patch that Dave pushed to drm-next (to get us back into the linux-next
> club).

But we've explicitly tried to stop autobuilders from selecting it in the
first place. That I think is a problem all of its own.
-Chris
Chris Wilson Nov. 5, 2019, 9:13 p.m. UTC | #6
Quoting Daniel Vetter (2019-11-05 20:58:25)
> On Tue, Nov 5, 2019 at 9:38 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > Quoting Daniel Vetter (2019-11-05 19:38:29)
> > > It's broken.
> > >
> > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > References: https://lists.freedesktop.org/archives/dri-devel/2019-November/242625.html
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > > Note: Probably best to apply this directly onto drm-next to avoid
> > > having drm-next dropped from linux-next until the next pull request.
> > > -Daniel
> > > ---
> > >  drivers/gpu/drm/i915/Kconfig.debug | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> > > index ef123eb29168..d2ba8f7e5e50 100644
> > > --- a/drivers/gpu/drm/i915/Kconfig.debug
> > > +++ b/drivers/gpu/drm/i915/Kconfig.debug
> > > @@ -44,7 +44,6 @@ config DRM_I915_DEBUG
> > >         select DRM_I915_SELFTEST
> > >         select DRM_I915_DEBUG_RUNTIME_PM
> > >         select DRM_I915_DEBUG_MMIO
> > > -       select BROKEN # for prototype uAPI
> >
> > You have to replace it with another secret bool as you cannot otherwise
> > enable CONFIG_BROKEN in .config.
> 
> Or this:
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index b4daad2bac23..4dbea1b9e6bb 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -75,6 +75,7 @@ menu "General setup"
> 
>  config BROKEN
>         bool
> +       default y
> 
>  config BROKEN_ON_SMP
>         bool
> 
> Either way it needs to be in topic/core-for-CI, not in any official
> tree. Because if you allow autobuilders to enable CONFIG_BROKEN, no
> matter how well hidden, they'll all break. You can also just revert my
> patch that Dave pushed to drm-next (to get us back into the linux-next
> club).

Fwiw, I think the revert in core-for-CI is reasonable, as that gives
devs the ability to toggle on the hidden menus, while at the same time
requiring them to have the minimal debug setup.
-Chris
Joonas Lahtinen Nov. 6, 2019, 10:24 a.m. UTC | #7
Quoting Chris Wilson (2019-11-05 23:13:36)
> Quoting Daniel Vetter (2019-11-05 20:58:25)
> > On Tue, Nov 5, 2019 at 9:38 PM Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > >
> > > Quoting Daniel Vetter (2019-11-05 19:38:29)
> > > > It's broken.
> > > >
> > > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > References: https://lists.freedesktop.org/archives/dri-devel/2019-November/242625.html
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > ---
> > > > Note: Probably best to apply this directly onto drm-next to avoid
> > > > having drm-next dropped from linux-next until the next pull request.
> > > > -Daniel
> > > > ---
> > > >  drivers/gpu/drm/i915/Kconfig.debug | 1 -
> > > >  1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
> > > > index ef123eb29168..d2ba8f7e5e50 100644
> > > > --- a/drivers/gpu/drm/i915/Kconfig.debug
> > > > +++ b/drivers/gpu/drm/i915/Kconfig.debug
> > > > @@ -44,7 +44,6 @@ config DRM_I915_DEBUG
> > > >         select DRM_I915_SELFTEST
> > > >         select DRM_I915_DEBUG_RUNTIME_PM
> > > >         select DRM_I915_DEBUG_MMIO
> > > > -       select BROKEN # for prototype uAPI
> > >
> > > You have to replace it with another secret bool as you cannot otherwise
> > > enable CONFIG_BROKEN in .config.
> > 
> > Or this:
> > 
> > diff --git a/init/Kconfig b/init/Kconfig
> > index b4daad2bac23..4dbea1b9e6bb 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -75,6 +75,7 @@ menu "General setup"
> > 
> >  config BROKEN
> >         bool
> > +       default y
> > 
> >  config BROKEN_ON_SMP
> >         bool
> > 
> > Either way it needs to be in topic/core-for-CI, not in any official
> > tree. Because if you allow autobuilders to enable CONFIG_BROKEN, no
> > matter how well hidden, they'll all break. You can also just revert my
> > patch that Dave pushed to drm-next (to get us back into the linux-next
> > club).
> 
> Fwiw, I think the revert in core-for-CI is reasonable, as that gives
> devs the ability to toggle on the hidden menus, while at the same time
> requiring them to have the minimal debug setup.

I've now reverted this in core-for-CI.

Regards, Joonas

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index ef123eb29168..d2ba8f7e5e50 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -44,7 +44,6 @@  config DRM_I915_DEBUG
 	select DRM_I915_SELFTEST
 	select DRM_I915_DEBUG_RUNTIME_PM
 	select DRM_I915_DEBUG_MMIO
-	select BROKEN # for prototype uAPI
 	default n
 	help
 	  Choose this option to turn on extra driver debugging that may affect