On 11/03/2024 13:36, Robin Murphy wrote: > On 2024-03-11 1:22 pm, Boris Brezillon wrote: >> On Mon, 11 Mar 2024 13:11:28 +0000 >> Robin Murphy <robin.murphy@xxxxxxx> wrote: >> >>> On 2024-03-11 11:52 am, Boris Brezillon wrote: >>>> On Mon, 11 Mar 2024 13:49:56 +0200 >>>> Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote: >>>> >>>>> On Mon, 11 Mar 2024, Boris Brezillon >>>>> <boris.brezillon@xxxxxxxxxxxxx> wrote: >>>>>> On Mon, 11 Mar 2024 13:05:01 +0200 >>>>>> Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote: >>>>>> >>>>>>> This breaks the config for me: >>>>>>> >>>>>>> SYNC include/config/auto.conf.cmd >>>>>>> GEN Makefile >>>>>>> drivers/iommu/Kconfig:14:error: recursive dependency detected! >>>>>>> drivers/iommu/Kconfig:14: symbol IOMMU_SUPPORT is selected by >>>>>>> DRM_PANTHOR >>>>>>> drivers/gpu/drm/panthor/Kconfig:3: symbol DRM_PANTHOR depends >>>>>>> on PM >>>>>>> kernel/power/Kconfig:183: symbol PM is selected by PM_SLEEP >>>>>>> kernel/power/Kconfig:117: symbol PM_SLEEP depends on >>>>>>> HIBERNATE_CALLBACKS >>>>>>> kernel/power/Kconfig:35: symbol HIBERNATE_CALLBACKS is >>>>>>> selected by XEN_SAVE_RESTORE >>>>>>> arch/x86/xen/Kconfig:67: symbol XEN_SAVE_RESTORE depends on XEN >>>>>>> arch/x86/xen/Kconfig:6: symbol XEN depends on PARAVIRT >>>>>>> arch/x86/Kconfig:781: symbol PARAVIRT is selected by HYPERV >>>>>>> drivers/hv/Kconfig:5: symbol HYPERV depends on X86_LOCAL_APIC >>>>>>> arch/x86/Kconfig:1106: symbol X86_LOCAL_APIC depends on >>>>>>> X86_UP_APIC >>>>>>> arch/x86/Kconfig:1081: symbol X86_UP_APIC prompt is visible >>>>>>> depending on PCI_MSI >>>>>>> drivers/pci/Kconfig:39: symbol PCI_MSI is selected by AMD_IOMMU >>>>>>> drivers/iommu/amd/Kconfig:3: symbol AMD_IOMMU depends on >>>>>>> IOMMU_SUPPORT >>>>>> >>>>>> Uh, I guess we want a "depends on IOMMU_SUPPORT" instead of "select >>>>>> IOMMU_SUPPORT" in panthor then. >>>>> >>>>> That works for me. >>>> >>>> Let's revert the faulty commit first. We'll see if Steve has a >>>> different solution for the original issue. >>> >>> FWIW, the reasoning in the offending commit seems incredibly tenuous. >>> There are far more practical reasons for building an arm/arm64 kernel >>> without PM - for debugging or whatever, and where one may even still >>> want a usable GPU, let alone just a non-broken build - than there are >>> for building this driver for x86. Using pm_ptr() is trivial, and if you >>> want to support COMPILE_TEST then there's really no justifiable excuse >>> not to. >> >> The problem is not just about using pm_ptr(), but also making sure >> panthor_device_resume/suspend() are called called in the init/unplug >> path when !PM, as I don't think the PM helpers automate that for us. I >> was just aiming for a simple fix that wouldn't force me to test the !PM >> case... > Fair enough, at worst we could always have a runtime check and refuse to > probe in conditions we don't think are worth the bother of implementing > fully-functional support for. However if we want to make an argument for > only supporting "realistic" configs at build time then that is an > argument for dropping COMPILE_TEST as well. Can we just replace the "depends on PM" with "select PM"? In my (admittedly very limited) testing this works. Otherwise I think we need to bite the bullet and support !PM in some way (maybe just as Robin suggests with a runtime bail out). Steve