On 1 December 2017 at 15:35, Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote:
> 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@xxxxxxxxxxxxxx>
>> 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@xxxxxxxxxxxxxxxx>
>> > Fixes: dba536cd9538 ("drm/etnaviv: add THERMAL dependency")
>> > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
>> > ---
>> > 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...
>
I'm afraid so. Though that doesn't mean it's a good idea ;-)
>> 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.
>
True - having a separate toggle will allows building etnaviv
regardless of THERMAL.
My suggestion seems to align with what Arnd proposed back in April [1]
and July [2].
Just not sure what happened there .. ACPI maintainers seem happy, yet
it never got merged.
-Emil
[1] https://patchwork.freedesktop.org/patch/151317/
[2] https://patchwork.freedesktop.org/patch/169164/
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
