[PATCH] drm/etnaviv: make THERMAL selectable

[Lucas Stach]

Hi Emil,

Am Freitag, den 01.12.2017, 14:56 +0000 schrieb Emil Velikov:
> Hi Philipp,
> 
> On 1 December 2017 at 14:30, Philipp Zabel <p.zabel at pengutronix.de>
> wrote:
> > Depending on THERMAL || !THERMAL caused a Kconfig dependency loop
> > on
> > x86_64:
> > 
> >   drivers/gpu/drm/tve200/Kconfig:1:error: recursive dependency
> > detected!
> >   For a resolution refer to Documentation/kbuild/kconfig-
> > language.txt
> >   subsection "Kconfig recursive dependency limitations"
> >   drivers/gpu/drm/tve200/Kconfig:1:       symbol DRM_TVE200 depends
> > on CMA
> >   For a resolution refer to Documentation/kbuild/kconfig-
> > language.txt
> >   subsection "Kconfig recursive dependency limitations"
> >   mm/Kconfig:489: symbol CMA is selected by DRM_ETNAVIV
> >   For a resolution refer to Documentation/kbuild/kconfig-
> > language.txt
> >   subsection "Kconfig recursive dependency limitations"
> >   drivers/gpu/drm/etnaviv/Kconfig:2:      symbol DRM_ETNAVIV
> > depends on THERMAL
> >   For a resolution refer to Documentation/kbuild/kconfig-
> > language.txt
> >   subsection "Kconfig recursive dependency limitations"
> >   drivers/thermal/Kconfig:5:      symbol THERMAL is selected by
> > ACPI_VIDEO
> >   For a resolution refer to Documentation/kbuild/kconfig-
> > language.txt
> >   subsection "Kconfig recursive dependency limitations"
> >   drivers/acpi/Kconfig:189:       symbol ACPI_VIDEO is selected by
> > BACKLIGHT_CLASS_DEVICE
> >   For a resolution refer to Documentation/kbuild/kconfig-
> > language.txt
> >   subsection "Kconfig recursive dependency limitations"
> >   drivers/video/backlight/Kconfig:158:    symbol
> > BACKLIGHT_CLASS_DEVICE is selected by DRM_PARADE_PS8622
> >   For a resolution refer to Documentation/kbuild/kconfig-
> > language.txt
> >   subsection "Kconfig recursive dependency limitations"
> >   drivers/gpu/drm/bridge/Kconfig:62:      symbol DRM_PARADE_PS8622
> > depends on DRM_BRIDGE
> >   For a resolution refer to Documentation/kbuild/kconfig-
> > language.txt
> >   subsection "Kconfig recursive dependency limitations"
> >   drivers/gpu/drm/bridge/Kconfig:1:       symbol DRM_BRIDGE is
> > selected by DRM_TVE200
> > 
> > To work around this, add a new option to make DRM_ETNAVIV select
> > THERMAL
> > instead.
> > 
> > Reported-by: Stephen Rothwell <sfr at canb.auug.org.au>
> > Fixes: dba536cd9538 ("drm/etnaviv: add THERMAL dependency")
> > Signed-off-by: Philipp Zabel <p.zabel at pengutronix.de>
> > ---
> >  drivers/gpu/drm/etnaviv/Kconfig       | 11 ++++++++++-
> >  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 12 +++++++++---
> >  2 files changed, 19 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/etnaviv/Kconfig
> > b/drivers/gpu/drm/etnaviv/Kconfig
> > index bbf2828ca5768..c446e867b3a24 100644
> > --- a/drivers/gpu/drm/etnaviv/Kconfig
> > +++ b/drivers/gpu/drm/etnaviv/Kconfig
> > @@ -4,9 +4,9 @@ config DRM_ETNAVIV
> >         depends on DRM
> >         depends on ARCH_MXC || ARCH_DOVE || (ARM && COMPILE_TEST)
> >         depends on MMU
> > -       depends on THERMAL || !THERMAL # if THERMAL=m, this can't
> > be 'y'
> >         select SHMEM
> >         select SYNC_FILE
> > +       select THERMAL if DRM_ETNAVIV_THERMAL
> >         select TMPFS
> >         select WANT_DEV_COREDUMP
> >         select CMA if HAVE_DMA_CONTIGUOUS
> > @@ -14,6 +14,15 @@ config DRM_ETNAVIV
> >         help
> >           DRM driver for Vivante GPUs.
> > 
> > +config DRM_ETNAVIV_THERMAL
> > +       bool "enable ETNAVIV thermal throttling"
> > +       depends on DRM_ETNAVIV
> > +       default y
> > +       select THERMAL
> 
> Just a quick braindump, while waiting for coffee to kick-in:
> 
> Having a separate toggle sounds cool, but yet select(ing) THERMAL, a
> user visible option, seems like an abuse.

It's not like this is the first occasion of such a thing in the kernel
Kconfig...

> One should swap the select with the original depends line.
>   depends on THERMAL || !THERMAL # if THERMAL=m, this can't be 'y'
> 
> That doesn't help with the original circular dependency, yet it seems
> that ACPI is also abusing THERMAL.
> If one swaps the "select THERMAL" with depends... while guarding the
> respective code behind IS_ENABLED things should work like a charm.

The intention of the separate symbol is exactly to avoid the dependency
chain while still being able to de-select THERMAL without having to
disable all of etnaviv, but only the thermal throttling support. And if
the user wants thermal throttling support, we better make sure it's
actually available.

Regards,
Lucas