[PATCH] Revert "drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue"

[PATCH] Revert "drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue"

[Jani Nikula]

This reverts commit 674dc7f61aefea81901c21402946074927e63f1a.

The commit causes a recursive dependency in kconfig:

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"

Fixes: 674dc7f61aef ("drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue")
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Steven Price <steven.price@arm.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/panthor/Kconfig          | 1 -
 drivers/gpu/drm/panthor/panthor_device.c | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig
index fdce7c1b2310..55b40ad07f3b 100644
--- a/drivers/gpu/drm/panthor/Kconfig
+++ b/drivers/gpu/drm/panthor/Kconfig
@@ -6,7 +6,6 @@ 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 083888b48591..69deb8e17778 100644
--- a/drivers/gpu/drm/panthor/panthor_device.c
+++ b/drivers/gpu/drm/panthor/panthor_device.c
@@ -402,6 +402,7 @@ 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);
@@ -546,3 +547,4 @@ int panthor_device_suspend(struct device *dev)
 	mutex_unlock(&ptdev->pm.mmio_lock);
 	return ret;
 }
+#endif
-- 
2.39.2

Re: [PATCH] Revert "drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue"

[Boris Brezillon]

On Mon, 11 Mar 2024 13:16:19 +0200
Jani Nikula <jani.nikula@intel.com> wrote:

> This reverts commit 674dc7f61aefea81901c21402946074927e63f1a.
> 
> The commit causes a recursive dependency in kconfig:
> 
> 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"
> 
> Fixes: 674dc7f61aef ("drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue")
> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Steven Price <steven.price@arm.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Acked-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/gpu/drm/panthor/Kconfig          | 1 -
>  drivers/gpu/drm/panthor/panthor_device.c | 2 ++
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig
> index fdce7c1b2310..55b40ad07f3b 100644
> --- a/drivers/gpu/drm/panthor/Kconfig
> +++ b/drivers/gpu/drm/panthor/Kconfig
> @@ -6,7 +6,6 @@ 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 083888b48591..69deb8e17778 100644
> --- a/drivers/gpu/drm/panthor/panthor_device.c
> +++ b/drivers/gpu/drm/panthor/panthor_device.c
> @@ -402,6 +402,7 @@ 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);
> @@ -546,3 +547,4 @@ int panthor_device_suspend(struct device *dev)
>  	mutex_unlock(&ptdev->pm.mmio_lock);
>  	return ret;
>  }
> +#endif

Re: [PATCH] Revert "drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue"

[Jani Nikula]

On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote:
> On Mon, 11 Mar 2024 13:16:19 +0200
> Jani Nikula <jani.nikula@intel.com> wrote:
>
>> This reverts commit 674dc7f61aefea81901c21402946074927e63f1a.
>> 
>> The commit causes a recursive dependency in kconfig:
>> 
>> 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"
>> 
>> Fixes: 674dc7f61aef ("drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue")
>> Cc: Boris Brezillon <boris.brezillon@collabora.com>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> Cc: Steven Price <steven.price@arm.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Acked-by: Boris Brezillon <boris.brezillon@collabora.com>

Your suggestion select -> depends on IOMMU_SUPPORT seems to also work,
at least for me. Want to send a patch for that instead of me merging the
revert?

BR,
Jani.

>
>> ---
>>  drivers/gpu/drm/panthor/Kconfig          | 1 -
>>  drivers/gpu/drm/panthor/panthor_device.c | 2 ++
>>  2 files changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/panthor/Kconfig b/drivers/gpu/drm/panthor/Kconfig
>> index fdce7c1b2310..55b40ad07f3b 100644
>> --- a/drivers/gpu/drm/panthor/Kconfig
>> +++ b/drivers/gpu/drm/panthor/Kconfig
>> @@ -6,7 +6,6 @@ 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 083888b48591..69deb8e17778 100644
>> --- a/drivers/gpu/drm/panthor/panthor_device.c
>> +++ b/drivers/gpu/drm/panthor/panthor_device.c
>> @@ -402,6 +402,7 @@ 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);
>> @@ -546,3 +547,4 @@ int panthor_device_suspend(struct device *dev)
>>  	mutex_unlock(&ptdev->pm.mmio_lock);
>>  	return ret;
>>  }
>> +#endif
>

-- 
Jani Nikula, Intel

Re: [PATCH] Revert "drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue"

[Boris Brezillon]

On Mon, 11 Mar 2024 13:51:46 +0200
Jani Nikula <jani.nikula@intel.com> wrote:

> On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote:
> > On Mon, 11 Mar 2024 13:16:19 +0200
> > Jani Nikula <jani.nikula@intel.com> wrote:
> >  
> >> This reverts commit 674dc7f61aefea81901c21402946074927e63f1a.
> >> 
> >> The commit causes a recursive dependency in kconfig:
> >> 
> >> 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"
> >> 
> >> Fixes: 674dc7f61aef ("drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue")
> >> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> >> Cc: Liviu Dudau <liviu.dudau@arm.com>
> >> Cc: Steven Price <steven.price@arm.com>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>  
> >
> > Acked-by: Boris Brezillon <boris.brezillon@collabora.com>  
> 
> Your suggestion select -> depends on IOMMU_SUPPORT seems to also work,
> at least for me. Want to send a patch for that instead of me merging the
> revert?

I replied on the other thread :-). I think we're better off reverting
the faulty commit, so we can discuss how to fix the original issue
properly without blocking the build.

Thanks;

Boris

Re: [PATCH] Revert "drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue"

[Jani Nikula]

On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote:
> On Mon, 11 Mar 2024 13:51:46 +0200
> Jani Nikula <jani.nikula@intel.com> wrote:
>
>> On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote:
>> > On Mon, 11 Mar 2024 13:16:19 +0200
>> > Jani Nikula <jani.nikula@intel.com> wrote:
>> >  
>> >> This reverts commit 674dc7f61aefea81901c21402946074927e63f1a.
>> >> 
>> >> The commit causes a recursive dependency in kconfig:
>> >> 
>> >> 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"
>> >> 
>> >> Fixes: 674dc7f61aef ("drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue")
>> >> Cc: Boris Brezillon <boris.brezillon@collabora.com>
>> >> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> >> Cc: Steven Price <steven.price@arm.com>
>> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>  
>> >
>> > Acked-by: Boris Brezillon <boris.brezillon@collabora.com>  
>> 
>> Your suggestion select -> depends on IOMMU_SUPPORT seems to also work,
>> at least for me. Want to send a patch for that instead of me merging the
>> revert?
>
> I replied on the other thread :-). I think we're better off reverting
> the faulty commit, so we can discuss how to fix the original issue
> properly without blocking the build.

Thanks, pushed to drm-misc-next.

BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH] Revert "drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue"

[Liviu Dudau]

On Mon, Mar 11, 2024 at 02:26:50PM +0200, Jani Nikula wrote:
> On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote:
> > On Mon, 11 Mar 2024 13:51:46 +0200
> > Jani Nikula <jani.nikula@intel.com> wrote:
> >
> >> On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote:
> >> > On Mon, 11 Mar 2024 13:16:19 +0200
> >> > Jani Nikula <jani.nikula@intel.com> wrote:
> >> >  
> >> >> This reverts commit 674dc7f61aefea81901c21402946074927e63f1a.
> >> >> 
> >> >> The commit causes a recursive dependency in kconfig:
> >> >> 
> >> >> 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"
> >> >> 
> >> >> Fixes: 674dc7f61aef ("drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue")
> >> >> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> >> >> Cc: Liviu Dudau <liviu.dudau@arm.com>
> >> >> Cc: Steven Price <steven.price@arm.com>
> >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>  
> >> >
> >> > Acked-by: Boris Brezillon <boris.brezillon@collabora.com>  
> >> 
> >> Your suggestion select -> depends on IOMMU_SUPPORT seems to also work,
> >> at least for me. Want to send a patch for that instead of me merging the
> >> revert?
> >
> > I replied on the other thread :-). I think we're better off reverting
> > the faulty commit, so we can discuss how to fix the original issue
> > properly without blocking the build.
> 
> Thanks, pushed to drm-misc-next.

So with this revert we're OK with an undefined symbol if !CONFIG_PM, but we're not happy
with a recursive dependency that is only triggered for COMPILE_TEST? I would've thought
IOMMU_SUPPORT options is a better one.

Best regards,
Liviu

> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH] Revert "drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue"

[Jani Nikula]

On Mon, 11 Mar 2024, Liviu Dudau <liviu.dudau@arm.com> wrote:
> On Mon, Mar 11, 2024 at 02:26:50PM +0200, Jani Nikula wrote:
>> On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote:
>> > On Mon, 11 Mar 2024 13:51:46 +0200
>> > Jani Nikula <jani.nikula@intel.com> wrote:
>> >
>> >> On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote:
>> >> > On Mon, 11 Mar 2024 13:16:19 +0200
>> >> > Jani Nikula <jani.nikula@intel.com> wrote:
>> >> >  
>> >> >> This reverts commit 674dc7f61aefea81901c21402946074927e63f1a.
>> >> >> 
>> >> >> The commit causes a recursive dependency in kconfig:
>> >> >> 
>> >> >> 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"
>> >> >> 
>> >> >> Fixes: 674dc7f61aef ("drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue")
>> >> >> Cc: Boris Brezillon <boris.brezillon@collabora.com>
>> >> >> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> >> >> Cc: Steven Price <steven.price@arm.com>
>> >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>  
>> >> >
>> >> > Acked-by: Boris Brezillon <boris.brezillon@collabora.com>  
>> >> 
>> >> Your suggestion select -> depends on IOMMU_SUPPORT seems to also work,
>> >> at least for me. Want to send a patch for that instead of me merging the
>> >> revert?
>> >
>> > I replied on the other thread :-). I think we're better off reverting
>> > the faulty commit, so we can discuss how to fix the original issue
>> > properly without blocking the build.
>> 
>> Thanks, pushed to drm-misc-next.
>
> So with this revert we're OK with an undefined symbol if !CONFIG_PM, but we're not happy
> with a recursive dependency that is only triggered for COMPILE_TEST? I would've thought
> IOMMU_SUPPORT options is a better one.

It's a real config.

# CONFIG_COMPILE_TEST is not set

BR,
Jani.

>
> Best regards,
> Liviu
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> -- 
>> Jani Nikula, Intel

-- 
Jani Nikula, Intel

Re: [PATCH] Revert "drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue"

[Liviu Dudau]

On Mon, Mar 11, 2024 at 04:49:30PM +0200, Jani Nikula wrote:
> On Mon, 11 Mar 2024, Liviu Dudau <liviu.dudau@arm.com> wrote:
> > On Mon, Mar 11, 2024 at 02:26:50PM +0200, Jani Nikula wrote:
> >> On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote:
> >> > On Mon, 11 Mar 2024 13:51:46 +0200
> >> > Jani Nikula <jani.nikula@intel.com> wrote:
> >> >
> >> >> On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote:
> >> >> > On Mon, 11 Mar 2024 13:16:19 +0200
> >> >> > Jani Nikula <jani.nikula@intel.com> wrote:
> >> >> >  
> >> >> >> This reverts commit 674dc7f61aefea81901c21402946074927e63f1a.
> >> >> >> 
> >> >> >> The commit causes a recursive dependency in kconfig:
> >> >> >> 
> >> >> >> 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"
> >> >> >> 
> >> >> >> Fixes: 674dc7f61aef ("drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue")
> >> >> >> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> >> >> >> Cc: Liviu Dudau <liviu.dudau@arm.com>
> >> >> >> Cc: Steven Price <steven.price@arm.com>
> >> >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>  
> >> >> >
> >> >> > Acked-by: Boris Brezillon <boris.brezillon@collabora.com>  
> >> >> 
> >> >> Your suggestion select -> depends on IOMMU_SUPPORT seems to also work,
> >> >> at least for me. Want to send a patch for that instead of me merging the
> >> >> revert?
> >> >
> >> > I replied on the other thread :-). I think we're better off reverting
> >> > the faulty commit, so we can discuss how to fix the original issue
> >> > properly without blocking the build.
> >> 
> >> Thanks, pushed to drm-misc-next.
> >
> > So with this revert we're OK with an undefined symbol if !CONFIG_PM, but we're not happy
> > with a recursive dependency that is only triggered for COMPILE_TEST? I would've thought
> > IOMMU_SUPPORT options is a better one.
> 
> It's a real config.
> 
> # CONFIG_COMPILE_TEST is not set

So I can select CONFIG_ARM64 and CONFIG_X86_LOCAL_APIC at the same time? DRM_PANTHOR depends on ARM || ARM64
and X86_LOCAL_APIC depends on X86_64. At some moment the recursive dependency detector should've stopped as
there are no common dependencies between DRM_PANTHOR and X86_LOCAL_APIC and going further just triggers false
positives. I'm curious how you've created your config now.

Best regards,
Liviu

> 
> BR,
> Jani.
> 
> >
> > Best regards,
> > Liviu
> >
> >> 
> >> BR,
> >> Jani.
> >> 
> >> 
> >> -- 
> >> Jani Nikula, Intel
> 
> -- 
> Jani Nikula, Intel

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH] Revert "drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue"

[Jani Nikula]

On Mon, 11 Mar 2024, Liviu Dudau <liviu.dudau@arm.com> wrote:
> On Mon, Mar 11, 2024 at 04:49:30PM +0200, Jani Nikula wrote:
>> On Mon, 11 Mar 2024, Liviu Dudau <liviu.dudau@arm.com> wrote:
>> > So with this revert we're OK with an undefined symbol if !CONFIG_PM, but we're not happy
>> > with a recursive dependency that is only triggered for COMPILE_TEST? I would've thought
>> > IOMMU_SUPPORT options is a better one.
>> 
>> It's a real config.
>> 
>> # CONFIG_COMPILE_TEST is not set
>
> So I can select CONFIG_ARM64 and CONFIG_X86_LOCAL_APIC at the same time? DRM_PANTHOR depends on ARM || ARM64
> and X86_LOCAL_APIC depends on X86_64. At some moment the recursive dependency detector should've stopped as
> there are no common dependencies between DRM_PANTHOR and X86_LOCAL_APIC and going further just triggers false
> positives. I'm curious how you've created your config now.

The thing is, I don't have *any* of the dependencies ARM || ARM64 ||
COMPILE_TEST set in the config that triggered this. I don't have
DRM_PANTHOR set. But make olddefconfig detects a circular dependency
nonetheless.

It's possible the issue is in kconfig. I don't know. But not being able
to even dodge the warning makes it a show stopper. I wouldn't even know
what to change in the config.


BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH] Revert "drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue"

[Liviu Dudau]

On Mon, Mar 11, 2024 at 05:52:59PM +0200, Jani Nikula wrote:
> On Mon, 11 Mar 2024, Liviu Dudau <liviu.dudau@arm.com> wrote:
> > On Mon, Mar 11, 2024 at 04:49:30PM +0200, Jani Nikula wrote:
> >> On Mon, 11 Mar 2024, Liviu Dudau <liviu.dudau@arm.com> wrote:
> >> > So with this revert we're OK with an undefined symbol if !CONFIG_PM, but we're not happy
> >> > with a recursive dependency that is only triggered for COMPILE_TEST? I would've thought
> >> > IOMMU_SUPPORT options is a better one.
> >> 
> >> It's a real config.
> >> 
> >> # CONFIG_COMPILE_TEST is not set
> >
> > So I can select CONFIG_ARM64 and CONFIG_X86_LOCAL_APIC at the same time? DRM_PANTHOR depends on ARM || ARM64
> > and X86_LOCAL_APIC depends on X86_64. At some moment the recursive dependency detector should've stopped as
> > there are no common dependencies between DRM_PANTHOR and X86_LOCAL_APIC and going further just triggers false
> > positives. I'm curious how you've created your config now.
> 
> The thing is, I don't have *any* of the dependencies ARM || ARM64 ||
> COMPILE_TEST set in the config that triggered this. I don't have
> DRM_PANTHOR set. But make olddefconfig detects a circular dependency
> nonetheless.
> 
> It's possible the issue is in kconfig. I don't know. But not being able
> to even dodge the warning makes it a show stopper. I wouldn't even know
> what to change in the config.

Understood, thanks for clarifying the condition under which you've discovered
the issue. I assumed (appologies!) that you were generating or using a crafted
.config for testing new drivers.

Reverting is then a better option for olddefconfig case, thanks for the patch
and the quick merge.

Best regards,
Liviu

> 
> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH] Revert "drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue"

[Boris Brezillon]

On Mon, 11 Mar 2024 14:41:42 +0000
Liviu Dudau <liviu.dudau@arm.com> wrote:

> On Mon, Mar 11, 2024 at 02:26:50PM +0200, Jani Nikula wrote:
> > On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote:  
> > > On Mon, 11 Mar 2024 13:51:46 +0200
> > > Jani Nikula <jani.nikula@intel.com> wrote:
> > >  
> > >> On Mon, 11 Mar 2024, Boris Brezillon <boris.brezillon@collabora.com> wrote:  
> > >> > On Mon, 11 Mar 2024 13:16:19 +0200
> > >> > Jani Nikula <jani.nikula@intel.com> wrote:
> > >> >    
> > >> >> This reverts commit 674dc7f61aefea81901c21402946074927e63f1a.
> > >> >> 
> > >> >> The commit causes a recursive dependency in kconfig:
> > >> >> 
> > >> >> 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"
> > >> >> 
> > >> >> Fixes: 674dc7f61aef ("drm/panthor: Fix undefined panthor_device_suspend/resume symbol issue")
> > >> >> Cc: Boris Brezillon <boris.brezillon@collabora.com>
> > >> >> Cc: Liviu Dudau <liviu.dudau@arm.com>
> > >> >> Cc: Steven Price <steven.price@arm.com>
> > >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>    
> > >> >
> > >> > Acked-by: Boris Brezillon <boris.brezillon@collabora.com>    
> > >> 
> > >> Your suggestion select -> depends on IOMMU_SUPPORT seems to also work,
> > >> at least for me. Want to send a patch for that instead of me merging the
> > >> revert?  
> > >
> > > I replied on the other thread :-). I think we're better off reverting
> > > the faulty commit, so we can discuss how to fix the original issue
> > > properly without blocking the build.  
> > 
> > Thanks, pushed to drm-misc-next.  
> 
> So with this revert we're OK with an undefined symbol if !CONFIG_PM, but we're not happy
> with a recursive dependency that is only triggered for COMPILE_TEST? I would've thought
> IOMMU_SUPPORT options is a better one.

No worries, we have plenty of time to fix this. Just didn't want to
rush a fix before discussing it with you and Steve.