| Message ID | 20240304090812.3941084-4-boris.brezillon@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/panthor: Fix 3 issues reported by the kernel test bot | expand |
On Mon, Mar 04, 2024 at 10:08:12AM +0100, Boris Brezillon wrote: > panthor_device_resume/suspend() are only compiled when CONFIG_PM is > enabled but panthro_drv.c doesn't use the pm_ptr() macro to conditionally > discard resume/suspend assignments, which causes undefined symbol > errors at link time when !PM. > > We could fix that by using pm_ptr(), but supporting the !PM case makes > little sense (the whole point of these embedded GPUs is to be low power, > so proper PM is a basic requirement in that case). So let's just enforce > the presence of CONFIG_PM with a Kconfig dependency instead. > > If someone needs to relax this dependency, it can be done in a follow-up. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202403031944.EOimQ8WK-lkp@intel.com/ > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com> Agree on this, there is no point on running on kernels without CONFIG_PM enabled. Best regards, Liviu > --- > drivers/gpu/drm/panthor/Kconfig | 1 + > drivers/gpu/drm/panthor/panthor_device.c | 2 -- > 2 files changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig > index 55b40ad07f3b..fdce7c1b2310 100644 > --- a/drivers/gpu/drm/panthor/Kconfig > +++ b/drivers/gpu/drm/panthor/Kconfig > @@ -6,6 +6,7 @@ config DRM_PANTHOR > depends on ARM || ARM64 || COMPILE_TEST > depends on !GENERIC_ATOMIC64 # for IOMMU_IO_PGTABLE_LPAE > depends on MMU > + depends on PM > select DEVFREQ_GOV_SIMPLE_ONDEMAND > select DRM_EXEC > select DRM_GEM_SHMEM_HELPER > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c > index 68e467ee458a..efea29143a54 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.c > +++ b/drivers/gpu/drm/panthor/panthor_device.c > @@ -403,7 +403,6 @@ int panthor_device_mmap_io(struct panthor_device *ptdev, struct vm_area_struct * > return 0; > } > > -#ifdef CONFIG_PM > int panthor_device_resume(struct device *dev) > { > struct panthor_device *ptdev = dev_get_drvdata(dev); > @@ -548,4 +547,3 @@ int panthor_device_suspend(struct device *dev) > mutex_unlock(&ptdev->pm.mmio_lock); > return ret; > } > -#endif > -- > 2.43.0 >
On 04/03/2024 09:08, Boris Brezillon wrote: > panthor_device_resume/suspend() are only compiled when CONFIG_PM is > enabled but panthro_drv.c doesn't use the pm_ptr() macro to conditionally > discard resume/suspend assignments, which causes undefined symbol > errors at link time when !PM. > > We could fix that by using pm_ptr(), but supporting the !PM case makes > little sense (the whole point of these embedded GPUs is to be low power, > so proper PM is a basic requirement in that case). So let's just enforce > the presence of CONFIG_PM with a Kconfig dependency instead. > > If someone needs to relax this dependency, it can be done in a follow-up. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202403031944.EOimQ8WK-lkp@intel.com/ > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> Seems reasonable, it should be easy to relax the CONFIG_PM condition in the future if anyone has an actual need. Reviewed-by: Steven Price <steven.price@arm.com> > --- > drivers/gpu/drm/panthor/Kconfig | 1 + > drivers/gpu/drm/panthor/panthor_device.c | 2 -- > 2 files changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig > index 55b40ad07f3b..fdce7c1b2310 100644 > --- a/drivers/gpu/drm/panthor/Kconfig > +++ b/drivers/gpu/drm/panthor/Kconfig > @@ -6,6 +6,7 @@ config DRM_PANTHOR > depends on ARM || ARM64 || COMPILE_TEST > depends on !GENERIC_ATOMIC64 # for IOMMU_IO_PGTABLE_LPAE > depends on MMU > + depends on PM > select DEVFREQ_GOV_SIMPLE_ONDEMAND > select DRM_EXEC > select DRM_GEM_SHMEM_HELPER > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c > index 68e467ee458a..efea29143a54 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.c > +++ b/drivers/gpu/drm/panthor/panthor_device.c > @@ -403,7 +403,6 @@ int panthor_device_mmap_io(struct panthor_device *ptdev, struct vm_area_struct * > return 0; > } > > -#ifdef CONFIG_PM > int panthor_device_resume(struct device *dev) > { > struct panthor_device *ptdev = dev_get_drvdata(dev); > @@ -548,4 +547,3 @@ int panthor_device_suspend(struct device *dev) > mutex_unlock(&ptdev->pm.mmio_lock); > return ret; > } > -#endif
On Mon, 04 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote: > panthor_device_resume/suspend() are only compiled when CONFIG_PM is > enabled but panthro_drv.c doesn't use the pm_ptr() macro to conditionally > discard resume/suspend assignments, which causes undefined symbol > errors at link time when !PM. > > We could fix that by using pm_ptr(), but supporting the !PM case makes > little sense (the whole point of these embedded GPUs is to be low power, > so proper PM is a basic requirement in that case). So let's just enforce > the presence of CONFIG_PM with a Kconfig dependency instead. > > If someone needs to relax this dependency, it can be done in a follow-up. 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 For a resolution refer to Documentation/kbuild/kconfig-language.rst subsection "Kconfig recursive dependency limitations" BR, Jani. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202403031944.EOimQ8WK-lkp@intel.com/ > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > --- > drivers/gpu/drm/panthor/Kconfig | 1 + > drivers/gpu/drm/panthor/panthor_device.c | 2 -- > 2 files changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig > index 55b40ad07f3b..fdce7c1b2310 100644 > --- a/drivers/gpu/drm/panthor/Kconfig > +++ b/drivers/gpu/drm/panthor/Kconfig > @@ -6,6 +6,7 @@ config DRM_PANTHOR > depends on ARM || ARM64 || COMPILE_TEST > depends on !GENERIC_ATOMIC64 # for IOMMU_IO_PGTABLE_LPAE > depends on MMU > + depends on PM > select DEVFREQ_GOV_SIMPLE_ONDEMAND > select DRM_EXEC > select DRM_GEM_SHMEM_HELPER > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c > index 68e467ee458a..efea29143a54 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.c > +++ b/drivers/gpu/drm/panthor/panthor_device.c > @@ -403,7 +403,6 @@ int panthor_device_mmap_io(struct panthor_device *ptdev, struct vm_area_struct * > return 0; > } > > -#ifdef CONFIG_PM > int panthor_device_resume(struct device *dev) > { > struct panthor_device *ptdev = dev_get_drvdata(dev); > @@ -548,4 +547,3 @@ int panthor_device_suspend(struct device *dev) > mutex_unlock(&ptdev->pm.mmio_lock); > return ret; > } > -#endif
On Mon, 11 Mar 2024 13:05:01 +0200 Jani Nikula <jani.nikula@linux.intel.com> 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. > For a resolution refer to Documentation/kbuild/kconfig-language.rst > subsection "Kconfig recursive dependency limitations"
On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote: > On Mon, 11 Mar 2024 13:05:01 +0200 > Jani Nikula <jani.nikula@linux.intel.com> 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. > >> For a resolution refer to Documentation/kbuild/kconfig-language.rst >> subsection "Kconfig recursive dependency limitations"
On Mon, 11 Mar 2024 13:49:56 +0200 Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote: > > On Mon, 11 Mar 2024 13:05:01 +0200 > > Jani Nikula <jani.nikula@linux.intel.com> 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. > > > > >> For a resolution refer to Documentation/kbuild/kconfig-language.rst > >> subsection "Kconfig recursive dependency limitations" >
On 2024-03-11 11:52 am, Boris Brezillon wrote: > On Mon, 11 Mar 2024 13:49:56 +0200 > Jani Nikula <jani.nikula@linux.intel.com> wrote: > >> On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote: >>> On Mon, 11 Mar 2024 13:05:01 +0200 >>> Jani Nikula <jani.nikula@linux.intel.com> 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. Thanks, Robin.
On Mon, 11 Mar 2024 13:11:28 +0000 Robin Murphy <robin.murphy@arm.com> wrote: > On 2024-03-11 11:52 am, Boris Brezillon wrote: > > On Mon, 11 Mar 2024 13:49:56 +0200 > > Jani Nikula <jani.nikula@linux.intel.com> wrote: > > > >> On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote: > >>> On Mon, 11 Mar 2024 13:05:01 +0200 > >>> Jani Nikula <jani.nikula@linux.intel.com> 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... > > Thanks, > Robin.
On 2024-03-11 1:22 pm, Boris Brezillon wrote: > On Mon, 11 Mar 2024 13:11:28 +0000 > Robin Murphy <robin.murphy@arm.com> wrote: > >> On 2024-03-11 11:52 am, Boris Brezillon wrote: >>> On Mon, 11 Mar 2024 13:49:56 +0200 >>> Jani Nikula <jani.nikula@linux.intel.com> wrote: >>> >>>> On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote: >>>>> On Mon, 11 Mar 2024 13:05:01 +0200 >>>>> Jani Nikula <jani.nikula@linux.intel.com> 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. Thanks, Robin.
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@arm.com> wrote: >> >>> On 2024-03-11 11:52 am, Boris Brezillon wrote: >>>> On Mon, 11 Mar 2024 13:49:56 +0200 >>>> Jani Nikula <jani.nikula@linux.intel.com> wrote: >>>> >>>>> On Mon, 11 Mar 2024, Boris Brezillon >>>>> <boris.brezillon@collabora.com> wrote: >>>>>> On Mon, 11 Mar 2024 13:05:01 +0200 >>>>>> Jani Nikula <jani.nikula@linux.intel.com> 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
On Mon, 11 Mar 2024 13:46:23 +0000 Steven Price <steven.price@arm.com> 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@arm.com> wrote: > >> > >>> On 2024-03-11 11:52 am, Boris Brezillon wrote: > >>>> On Mon, 11 Mar 2024 13:49:56 +0200 > >>>> Jani Nikula <jani.nikula@linux.intel.com> wrote: > >>>> > >>>>> On Mon, 11 Mar 2024, Boris Brezillon > >>>>> <boris.brezillon@collabora.com> wrote: > >>>>>> On Mon, 11 Mar 2024 13:05:01 +0200 > >>>>>> Jani Nikula <jani.nikula@linux.intel.com> 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---
On Mon, 11 Mar 2024 14:58:37 +0100 Boris Brezillon <boris.brezillon@collabora.com> wrote: > On Mon, 11 Mar 2024 13:46:23 +0000 > Steven Price <steven.price@arm.com> 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@arm.com> wrote: > > >> > > >>> On 2024-03-11 11:52 am, Boris Brezillon wrote: > > >>>> On Mon, 11 Mar 2024 13:49:56 +0200 > > >>>> Jani Nikula <jani.nikula@linux.intel.com> wrote: > > >>>> > > >>>>> On Mon, 11 Mar 2024, Boris Brezillon > > >>>>> <boris.brezillon@collabora.com> wrote: > > >>>>>> On Mon, 11 Mar 2024 13:05:01 +0200 > > >>>>>> Jani Nikula <jani.nikula@linux.intel.com> 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--- This time with the diff :-) --->8--- diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c index 69deb8e17778..3d05e7358f0e 100644 --- a/drivers/gpu/drm/panthor/panthor_device.c +++ b/drivers/gpu/drm/panthor/panthor_device.c @@ -87,6 +87,10 @@ void panthor_device_unplug(struct panthor_device *ptdev) pm_runtime_dont_use_autosuspend(ptdev->base.dev); pm_runtime_put_sync_suspend(ptdev->base.dev); + /* If PM is disabled, we need to call the suspend handler manually. */ + if (!IS_ENABLED(CONFIG_PM)) + panthor_device_suspend(ptdev->base.dev); + /* Report the unplug operation as done to unblock concurrent * panthor_device_unplug() callers. */ @@ -218,6 +222,13 @@ int panthor_device_init(struct panthor_device *ptdev) if (ret) return ret; + /* If PM is disabled, we need to call panthor_device_resume() manually. */ + if (!IS_ENABLED(CONFIG_PM)) { + ret = panthor_device_resume(ptdev->base.dev); + if (ret) + return ret; + } + ret = panthor_gpu_init(ptdev); if (ret) goto err_rpm_put; diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c index ff484506229f..2ea6a9f436db 100644 --- a/drivers/gpu/drm/panthor/panthor_drv.c +++ b/drivers/gpu/drm/panthor/panthor_drv.c @@ -1407,17 +1407,19 @@ static const struct of_device_id dt_match[] = { }; MODULE_DEVICE_TABLE(of, dt_match); +#ifdef CONFIG_PM static DEFINE_RUNTIME_DEV_PM_OPS(panthor_pm_ops, panthor_device_suspend, panthor_device_resume, NULL); +#endif static struct platform_driver panthor_driver = { .probe = panthor_probe, .remove_new = panthor_remove, .driver = { .name = "panthor", - .pm = &panthor_pm_ops, + .pm = pm_ptr(&panthor_pm_ops), .of_match_table = dt_match, }, };
On Mon, 11 Mar 2024 14:59:39 +0100 Boris Brezillon <boris.brezillon@collabora.com> wrote: > On Mon, 11 Mar 2024 14:58:37 +0100 > Boris Brezillon <boris.brezillon@collabora.com> wrote: > > > On Mon, 11 Mar 2024 13:46:23 +0000 > > Steven Price <steven.price@arm.com> 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@arm.com> wrote: > > > >> > > > >>> On 2024-03-11 11:52 am, Boris Brezillon wrote: > > > >>>> On Mon, 11 Mar 2024 13:49:56 +0200 > > > >>>> Jani Nikula <jani.nikula@linux.intel.com> wrote: > > > >>>> > > > >>>>> On Mon, 11 Mar 2024, Boris Brezillon > > > >>>>> <boris.brezillon@collabora.com> wrote: > > > >>>>>> On Mon, 11 Mar 2024 13:05:01 +0200 > > > >>>>>> Jani Nikula <jani.nikula@linux.intel.com> 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--- > > This time with the diff :-) > > --->8--- > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c > index 69deb8e17778..3d05e7358f0e 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.c > +++ b/drivers/gpu/drm/panthor/panthor_device.c > @@ -87,6 +87,10 @@ void panthor_device_unplug(struct panthor_device *ptdev) > pm_runtime_dont_use_autosuspend(ptdev->base.dev); > pm_runtime_put_sync_suspend(ptdev->base.dev); > > + /* If PM is disabled, we need to call the suspend handler manually. */ > + if (!IS_ENABLED(CONFIG_PM)) > + panthor_device_suspend(ptdev->base.dev); > + > /* Report the unplug operation as done to unblock concurrent > * panthor_device_unplug() callers. > */ > @@ -218,6 +222,13 @@ int panthor_device_init(struct panthor_device *ptdev) > if (ret) > return ret; > > + /* If PM is disabled, we need to call panthor_device_resume() manually. */ > + if (!IS_ENABLED(CONFIG_PM)) { > + ret = panthor_device_resume(ptdev->base.dev); > + if (ret) > + return ret; > + } > + > ret = panthor_gpu_init(ptdev); > if (ret) > goto err_rpm_put; And I forgot to remove the #ifdef CONFIG_PM in panthor_device.c. > diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c > index ff484506229f..2ea6a9f436db 100644 > --- a/drivers/gpu/drm/panthor/panthor_drv.c > +++ b/drivers/gpu/drm/panthor/panthor_drv.c > @@ -1407,17 +1407,19 @@ static const struct of_device_id dt_match[] = { > }; > MODULE_DEVICE_TABLE(of, dt_match); > > +#ifdef CONFIG_PM > static DEFINE_RUNTIME_DEV_PM_OPS(panthor_pm_ops, > panthor_device_suspend, > panthor_device_resume, > NULL); > +#endif > > static struct platform_driver panthor_driver = { > .probe = panthor_probe, > .remove_new = panthor_remove, > .driver = { > .name = "panthor", > - .pm = &panthor_pm_ops, > + .pm = pm_ptr(&panthor_pm_ops), > .of_match_table = dt_match, > }, > };
diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig index 55b40ad07f3b..fdce7c1b2310 100644 --- a/drivers/gpu/drm/panthor/Kconfig +++ b/drivers/gpu/drm/panthor/Kconfig @@ -6,6 +6,7 @@ config DRM_PANTHOR depends on ARM || ARM64 || COMPILE_TEST depends on !GENERIC_ATOMIC64 # for IOMMU_IO_PGTABLE_LPAE depends on MMU + depends on PM select DEVFREQ_GOV_SIMPLE_ONDEMAND select DRM_EXEC select DRM_GEM_SHMEM_HELPER diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c index 68e467ee458a..efea29143a54 100644 --- a/drivers/gpu/drm/panthor/panthor_device.c +++ b/drivers/gpu/drm/panthor/panthor_device.c @@ -403,7 +403,6 @@ int panthor_device_mmap_io(struct panthor_device *ptdev, struct vm_area_struct * return 0; } -#ifdef CONFIG_PM int panthor_device_resume(struct device *dev) { struct panthor_device *ptdev = dev_get_drvdata(dev); @@ -548,4 +547,3 @@ int panthor_device_suspend(struct device *dev) mutex_unlock(&ptdev->pm.mmio_lock); return ret; } -#endif
panthor_device_resume/suspend() are only compiled when CONFIG_PM is enabled but panthro_drv.c doesn't use the pm_ptr() macro to conditionally discard resume/suspend assignments, which causes undefined symbol errors at link time when !PM. We could fix that by using pm_ptr(), but supporting the !PM case makes little sense (the whole point of these embedded GPUs is to be low power, so proper PM is a basic requirement in that case). So let's just enforce the presence of CONFIG_PM with a Kconfig dependency instead. If someone needs to relax this dependency, it can be done in a follow-up. Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202403031944.EOimQ8WK-lkp@intel.com/ Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> --- drivers/gpu/drm/panthor/Kconfig | 1 + drivers/gpu/drm/panthor/panthor_device.c | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-)