On Mon, 11 Mar 2024 13:46:23 +0000 Steven Price <steven.price@xxxxxxx> wrote: > 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). I won't have time to test it this week, but if someone is interested, here's a diff implementing manual resume/suspend in the init/unplug path: --->8---
