Hi All, Since 6.0-rc1 it is possible for ACPI_VIDEO to be enabled on non X86 platforms. I already send an email about this being a possible problem for nouveau, when nouveau is builtin and apci_video is a module: https://lore.kernel.org/linux-acpi/a385b626-217f-25be-f076-7478da3d1147@xxxxxxxxxx/ """ I just noticed this change: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=038275d227841d4978ceceb397b584b4b39f2b50 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -210,7 +210,7 @@ config ACPI_TINY_POWER_BUTTON_SIGNAL config ACPI_VIDEO tristate "Video" - depends on X86 && BACKLIGHT_CLASS_DEVICE + depends on BACKLIGHT_CLASS_DEVICE depends on INPUT select THERMAL help I think this is going to cause random-config build errors because at the nouveau driver calls acpi_video functions and it expects those functions to either be there or to get the stubs. This is an issue when acpi_video is build as a module and the i915 / nouveau code is builtin. To avoid this issue nouveau does a select on ACPI_VIDEO, here is the code from drivers/gpu/drm/nouveau/Kconfig # Similar to i915, we need to select ACPI_VIDEO and it's dependencies select BACKLIGHT_CLASS_DEVICE if ACPI && X86 select INPUT if ACPI && X86 select THERMAL if ACPI && X86 select ACPI_VIDEO if ACPI && X86 notice the if ACPI && X86, since ACPI_VIDEO can now be selected on ARM, this can lead to ACPI_VIDEO being enabled as module (so no stubs) while nouveau can be builtin which will lead to unresolved symbol errors in nouveau when building the zImage. I believe that to fix this the conditions after the select must be changed from "if ACPI && X86" to just "if ACPI" """ I was looking at a lkp warning in my backlight refactor series which is explained by the above today: https://lore.kernel.org/lkml/202208231625.icHjRXDI-lkp@xxxxxxxxx/ and while working on fixing this I noticed that nouveau not only assumes ACPI[_VIDEO] is only available on X86 in its Kconfig but also in other places: drivers/gpu/drm/nouveau/Kbuild: ifdef CONFIG_X86 nouveau-$(CONFIG_ACPI) += nouveau_acpi.o endif drivers/gpu/drm/nouveau/nouveau_acpi.h: #if defined(CONFIG_ACPI) && defined(CONFIG_X86) bool nouveau_is_optimus(void); ... #else static inline bool nouveau_is_optimus(void) { return false; }; ... #endif Since currently the only nouveau dep on acpi_video is inside nouveau_acpi.c the not building + stubbing of nouveau_acpi.c is saving us from the problem which I expected in my mail quoted above. The backlight refactor series breaks this though, because it adds calls to: acpi_video_backlight_use_native() acpi_video_register_backlight() to nouveau outside of the drivers/gpu/drm/nouveau/nouveau_acpi.c file. To fix this for the next version of the backlight refactor series I have added wrappers for: acpi_video_backlight_use_native() acpi_video_register_backlight() to nouveau_acpi.? After thinking about this a bit this seemed by far the cleanest and consistent with how the current nouveau code is abstracting other ACPI use, so this makes the backlight changes consistent with existing nouveau practices. Lyude, because of these changes I've dropped your Reviewed-by. I'll send out a new version 4 series sometime today, can you please re-review the 2 nouveau patches? ### My first instinct to fix this, was to adjust the nouveau Kconfig bits selecting ACPI_VIDEO to take into account that it now is available on non X86 too: diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig index 03d12caf9e26..26b895a49b9c 100644 --- a/drivers/gpu/drm/nouveau/Kconfig +++ b/drivers/gpu/drm/nouveau/Kconfig @@ -16,10 +16,10 @@ config DRM_NOUVEAU select MXM_WMI if ACPI && X86 select POWER_SUPPLY # Similar to i915, we need to select ACPI_VIDEO and it's dependencies - select BACKLIGHT_CLASS_DEVICE if ACPI && X86 - select INPUT if ACPI && X86 - select THERMAL if ACPI && X86 - select ACPI_VIDEO if ACPI && X86 + select BACKLIGHT_CLASS_DEVICE if ACPI + select INPUT if ACPI + select THERMAL if ACPI + select ACPI_VIDEO if ACPI select SND_HDA_COMPONENT if SND_HDA_CORE help Choose this option for open-source NVIDIA support. We could still make this change together with Kbuild + nouveau_acpi.h to build on non x86 too. This might be good to prepare for some aarch64 devices which may use some of the ACPI bits, they may need e.g. acpi_video_get_edid(). OTOH this might cause unexpected issue so it might be better to wait with making such a change until it is actually necessary. Regards, Hans
