| Message ID | 20171201143015.946-1-p.zabel@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Philipp, On 1 December 2017 at 14:30, Philipp Zabel <p.zabel@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@canb.auug.org.au> > Fixes: dba536cd9538 ("drm/etnaviv: add THERMAL dependency") > Signed-off-by: Philipp Zabel <p.zabel@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. 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. HTH Emil
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@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@canb.auug.org.au> > > Fixes: dba536cd9538 ("drm/etnaviv: add THERMAL dependency") > > Signed-off-by: Philipp Zabel <p.zabel@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
On 1 December 2017 at 15:35, Lucas Stach <l.stach@pengutronix.de> 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@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@canb.auug.org.au> >> > Fixes: dba536cd9538 ("drm/etnaviv: add THERMAL dependency") >> > Signed-off-by: Philipp Zabel <p.zabel@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... > 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/
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 + help + Compile in support for thermal throttling. + Say Y unless you want to risk burning your SoC. + config DRM_ETNAVIV_REGISTER_LOGGING bool "enable ETNAVIV register logging" depends on DRM_ETNAVIV diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index e19cbe05da2a3..4196f03cf1eb6 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -31,6 +31,10 @@ #include "state_hi.xml.h" #include "cmdstream.xml.h" +#if IS_ENABLED(CONFIG_THERMAL) && !IS_ENABLED(CONFIG_DRM_ETNAVIV_THERMAL) +#warning "You may want to enable CONFIG_DRM_ETNAVIV_THERMAL." +#endif + static const struct platform_device_id gpu_ids[] = { { .name = "etnaviv-gpu,2d" }, { }, @@ -1738,7 +1742,7 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master, struct etnaviv_gpu *gpu = dev_get_drvdata(dev); int ret; - if (IS_ENABLED(CONFIG_THERMAL)) { + if (IS_ENABLED(CONFIG_DRM_ETNAVIV_THERMAL)) { gpu->cooling = thermal_of_cooling_device_register(dev->of_node, (char *)dev_name(dev), gpu, &cooling_ops); if (IS_ERR(gpu->cooling)) @@ -1751,7 +1755,8 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master, ret = etnaviv_gpu_clk_enable(gpu); #endif if (ret < 0) { - thermal_cooling_device_unregister(gpu->cooling); + if (IS_ENABLED(CONFIG_DRM_ETNAVIV_THERMAL)) + thermal_cooling_device_unregister(gpu->cooling); return ret; } @@ -1808,7 +1813,8 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master, gpu->drm = NULL; - thermal_cooling_device_unregister(gpu->cooling); + if (IS_ENABLED(CONFIG_DRM_ETNAVIV_THERMAL)) + thermal_cooling_device_unregister(gpu->cooling); gpu->cooling = NULL; }
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@canb.auug.org.au> Fixes: dba536cd9538 ("drm/etnaviv: add THERMAL dependency") Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> --- drivers/gpu/drm/etnaviv/Kconfig | 11 ++++++++++- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 12 +++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-)