linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled
@ 2019-07-22 13:47 YueHaibing
  2019-07-24 10:30 ` Sakari Ailus
  0 siblings, 1 reply; 11+ messages in thread
From: YueHaibing @ 2019-07-22 13:47 UTC (permalink / raw)
  To: sakari.ailus, mchehab, gregkh, yong.zhi, digetx, hverkuil-cisco
  Cc: linux-kernel, devel, linux-media, YueHaibing

If IOMMU_SUPPORT is not set, ipu3 driver may select IOMMU_IOVA to m.
But for many drivers, they use "select IOMMU_IOVA if IOMMU_SUPPORT"
in the Kconfig, for example, CONFIG_TEGRA_VDE is set to y but IOMMU_IOVA
is m, then the building fails like this:

drivers/staging/media/tegra-vde/iommu.o: In function `tegra_vde_iommu_map':
iommu.c:(.text+0x41): undefined reference to `alloc_iova'
iommu.c:(.text+0x56): undefined reference to `__free_iova'

Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device driver")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/staging/media/ipu3/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/ipu3/Kconfig b/drivers/staging/media/ipu3/Kconfig
index 4b51c67..b7df18f 100644
--- a/drivers/staging/media/ipu3/Kconfig
+++ b/drivers/staging/media/ipu3/Kconfig
@@ -4,7 +4,7 @@ config VIDEO_IPU3_IMGU
 	depends on PCI && VIDEO_V4L2
 	depends on MEDIA_CONTROLLER && VIDEO_V4L2_SUBDEV_API
 	depends on X86
-	select IOMMU_IOVA
+	select IOMMU_IOVA if IOMMU_SUPPORT
 	select VIDEOBUF2_DMA_SG
 	help
 	  This is the Video4Linux2 driver for Intel IPU3 image processing unit,
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled
  2019-07-22 13:47 [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled YueHaibing
@ 2019-07-24 10:30 ` Sakari Ailus
  2019-07-24 13:49   ` Robin Murphy
  0 siblings, 1 reply; 11+ messages in thread
From: Sakari Ailus @ 2019-07-24 10:30 UTC (permalink / raw)
  To: YueHaibing
  Cc: mchehab, gregkh, yong.zhi, digetx, hverkuil-cisco, linux-kernel,
	devel, linux-media, iommu

Hi Yue,

On Mon, Jul 22, 2019 at 09:47:49PM +0800, YueHaibing wrote:
> If IOMMU_SUPPORT is not set, ipu3 driver may select IOMMU_IOVA to m.
> But for many drivers, they use "select IOMMU_IOVA if IOMMU_SUPPORT"
> in the Kconfig, for example, CONFIG_TEGRA_VDE is set to y but IOMMU_IOVA
> is m, then the building fails like this:
> 
> drivers/staging/media/tegra-vde/iommu.o: In function `tegra_vde_iommu_map':
> iommu.c:(.text+0x41): undefined reference to `alloc_iova'
> iommu.c:(.text+0x56): undefined reference to `__free_iova'
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device driver")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  drivers/staging/media/ipu3/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/ipu3/Kconfig b/drivers/staging/media/ipu3/Kconfig
> index 4b51c67..b7df18f 100644
> --- a/drivers/staging/media/ipu3/Kconfig
> +++ b/drivers/staging/media/ipu3/Kconfig
> @@ -4,7 +4,7 @@ config VIDEO_IPU3_IMGU
>  	depends on PCI && VIDEO_V4L2
>  	depends on MEDIA_CONTROLLER && VIDEO_V4L2_SUBDEV_API
>  	depends on X86
> -	select IOMMU_IOVA
> +	select IOMMU_IOVA if IOMMU_SUPPORT

This doesn't seem right: the ipu3-cio2 driver needs IOMMU_IOVA
independently of IOMMU_SUPPORT.

Looking at tegra-vde, it seems to depend on IOMMU_SUPPORT but that's not
declared in its Kconfig entry. I wonder if adding that would be the right
way to fix this.

Cc'ing the IOMMU list.

>  	select VIDEOBUF2_DMA_SG
>  	help
>  	  This is the Video4Linux2 driver for Intel IPU3 image processing unit,

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled
  2019-07-24 10:30 ` Sakari Ailus
@ 2019-07-24 13:49   ` Robin Murphy
  2019-07-24 14:03     ` Yuehaibing
  0 siblings, 1 reply; 11+ messages in thread
From: Robin Murphy @ 2019-07-24 13:49 UTC (permalink / raw)
  To: Sakari Ailus, YueHaibing
  Cc: devel, linux-media, gregkh, linux-kernel, iommu, hverkuil-cisco,
	digetx, mchehab, yong.zhi

On 24/07/2019 11:30, Sakari Ailus wrote:
> Hi Yue,
> 
> On Mon, Jul 22, 2019 at 09:47:49PM +0800, YueHaibing wrote:
>> If IOMMU_SUPPORT is not set, ipu3 driver may select IOMMU_IOVA to m.
>> But for many drivers, they use "select IOMMU_IOVA if IOMMU_SUPPORT"
>> in the Kconfig, for example, CONFIG_TEGRA_VDE is set to y but IOMMU_IOVA
>> is m, then the building fails like this:
>>
>> drivers/staging/media/tegra-vde/iommu.o: In function `tegra_vde_iommu_map':
>> iommu.c:(.text+0x41): undefined reference to `alloc_iova'
>> iommu.c:(.text+0x56): undefined reference to `__free_iova'
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device driver")
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> ---
>>   drivers/staging/media/ipu3/Kconfig | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/media/ipu3/Kconfig b/drivers/staging/media/ipu3/Kconfig
>> index 4b51c67..b7df18f 100644
>> --- a/drivers/staging/media/ipu3/Kconfig
>> +++ b/drivers/staging/media/ipu3/Kconfig
>> @@ -4,7 +4,7 @@ config VIDEO_IPU3_IMGU
>>   	depends on PCI && VIDEO_V4L2
>>   	depends on MEDIA_CONTROLLER && VIDEO_V4L2_SUBDEV_API
>>   	depends on X86
>> -	select IOMMU_IOVA
>> +	select IOMMU_IOVA if IOMMU_SUPPORT
> 
> This doesn't seem right: the ipu3-cio2 driver needs IOMMU_IOVA
> independently of IOMMU_SUPPORT.
> 
> Looking at tegra-vde, it seems to depend on IOMMU_SUPPORT but that's not
> declared in its Kconfig entry. I wonder if adding that would be the right
> way to fix this.
> 
> Cc'ing the IOMMU list.

Right, I also had the impression that we'd made the IOVA library 
completely standalone. And what does the IPU3 driver's Kconfig have to 
do with some *other* driver failing to link anyway?

Robin.

> 
>>   	select VIDEOBUF2_DMA_SG
>>   	help
>>   	  This is the Video4Linux2 driver for Intel IPU3 image processing unit,
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled
  2019-07-24 13:49   ` Robin Murphy
@ 2019-07-24 14:03     ` Yuehaibing
  2019-07-24 14:09       ` Dmitry Osipenko
  0 siblings, 1 reply; 11+ messages in thread
From: Yuehaibing @ 2019-07-24 14:03 UTC (permalink / raw)
  To: Robin Murphy, Sakari Ailus
  Cc: devel, linux-media, gregkh, linux-kernel, iommu, hverkuil-cisco,
	digetx, mchehab, yong.zhi

On 2019/7/24 21:49, Robin Murphy wrote:
> On 24/07/2019 11:30, Sakari Ailus wrote:
>> Hi Yue,
>>
>> On Mon, Jul 22, 2019 at 09:47:49PM +0800, YueHaibing wrote:
>>> If IOMMU_SUPPORT is not set, ipu3 driver may select IOMMU_IOVA to m.
>>> But for many drivers, they use "select IOMMU_IOVA if IOMMU_SUPPORT"
>>> in the Kconfig, for example, CONFIG_TEGRA_VDE is set to y but IOMMU_IOVA
>>> is m, then the building fails like this:
>>>
>>> drivers/staging/media/tegra-vde/iommu.o: In function `tegra_vde_iommu_map':
>>> iommu.c:(.text+0x41): undefined reference to `alloc_iova'
>>> iommu.c:(.text+0x56): undefined reference to `__free_iova'
>>>
>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>> Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device driver")
>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>>> ---
>>>   drivers/staging/media/ipu3/Kconfig | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/media/ipu3/Kconfig b/drivers/staging/media/ipu3/Kconfig
>>> index 4b51c67..b7df18f 100644
>>> --- a/drivers/staging/media/ipu3/Kconfig
>>> +++ b/drivers/staging/media/ipu3/Kconfig
>>> @@ -4,7 +4,7 @@ config VIDEO_IPU3_IMGU
>>>       depends on PCI && VIDEO_V4L2
>>>       depends on MEDIA_CONTROLLER && VIDEO_V4L2_SUBDEV_API
>>>       depends on X86
>>> -    select IOMMU_IOVA
>>> +    select IOMMU_IOVA if IOMMU_SUPPORT
>>
>> This doesn't seem right: the ipu3-cio2 driver needs IOMMU_IOVA
>> independently of IOMMU_SUPPORT.
>>
>> Looking at tegra-vde, it seems to depend on IOMMU_SUPPORT but that's not
>> declared in its Kconfig entry. I wonder if adding that would be the right
>> way to fix this.
>>
>> Cc'ing the IOMMU list.
> 
> Right, I also had the impression that we'd made the IOVA library completely standalone. And what does the IPU3 driver's Kconfig have to do with some *other* driver failing to link anyway?

Oh, I misunderstand that IOMMU_IOVA is depend on IOMMU_SUPPORT, thank you for clarification.

I will try to fix this in tegra-vde.

> 
> Robin.
> 
>>
>>>       select VIDEOBUF2_DMA_SG
>>>       help
>>>         This is the Video4Linux2 driver for Intel IPU3 image processing unit,
>>
> 
> .
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled
  2019-07-24 14:03     ` Yuehaibing
@ 2019-07-24 14:09       ` Dmitry Osipenko
  2019-07-24 14:23         ` Robin Murphy
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2019-07-24 14:09 UTC (permalink / raw)
  To: Yuehaibing, Robin Murphy, Sakari Ailus
  Cc: devel, linux-media, gregkh, linux-kernel, iommu, hverkuil-cisco,
	mchehab, yong.zhi

24.07.2019 17:03, Yuehaibing пишет:
> On 2019/7/24 21:49, Robin Murphy wrote:
>> On 24/07/2019 11:30, Sakari Ailus wrote:
>>> Hi Yue,
>>>
>>> On Mon, Jul 22, 2019 at 09:47:49PM +0800, YueHaibing wrote:
>>>> If IOMMU_SUPPORT is not set, ipu3 driver may select IOMMU_IOVA to m.
>>>> But for many drivers, they use "select IOMMU_IOVA if IOMMU_SUPPORT"
>>>> in the Kconfig, for example, CONFIG_TEGRA_VDE is set to y but IOMMU_IOVA
>>>> is m, then the building fails like this:
>>>>
>>>> drivers/staging/media/tegra-vde/iommu.o: In function `tegra_vde_iommu_map':
>>>> iommu.c:(.text+0x41): undefined reference to `alloc_iova'
>>>> iommu.c:(.text+0x56): undefined reference to `__free_iova'
>>>>
>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>> Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device driver")
>>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>>>> ---
>>>>   drivers/staging/media/ipu3/Kconfig | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/staging/media/ipu3/Kconfig b/drivers/staging/media/ipu3/Kconfig
>>>> index 4b51c67..b7df18f 100644
>>>> --- a/drivers/staging/media/ipu3/Kconfig
>>>> +++ b/drivers/staging/media/ipu3/Kconfig
>>>> @@ -4,7 +4,7 @@ config VIDEO_IPU3_IMGU
>>>>       depends on PCI && VIDEO_V4L2
>>>>       depends on MEDIA_CONTROLLER && VIDEO_V4L2_SUBDEV_API
>>>>       depends on X86
>>>> -    select IOMMU_IOVA
>>>> +    select IOMMU_IOVA if IOMMU_SUPPORT
>>>
>>> This doesn't seem right: the ipu3-cio2 driver needs IOMMU_IOVA
>>> independently of IOMMU_SUPPORT.
>>>
>>> Looking at tegra-vde, it seems to depend on IOMMU_SUPPORT but that's not
>>> declared in its Kconfig entry. I wonder if adding that would be the right
>>> way to fix this.
>>>
>>> Cc'ing the IOMMU list.
IOMMU_SUPPORT is optional for the Tegra-VDE driver.

>> Right, I also had the impression that we'd made the IOVA library completely standalone. And what does the IPU3 driver's Kconfig have to do with some *other* driver failing to link anyway?

I can see it failing if IPU3 is compiled as a loadable module, while
Tegra-VDE is a built-in driver. Hence IOVA lib should be also a kernel
module and thus the IOVA symbols will be missing during of linkage of
the VDE driver.

> Oh, I misunderstand that IOMMU_IOVA is depend on IOMMU_SUPPORT, thank you for clarification.
> 
> I will try to fix this in tegra-vde.

Probably IOVA could be selected independently of IOMMU_SUPPORT, but IOVA
library isn't needed for the VDE driver if IOMMU_SUPPORT is disabled.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled
  2019-07-24 14:09       ` Dmitry Osipenko
@ 2019-07-24 14:23         ` Robin Murphy
  2019-07-24 14:28           ` Dmitry Osipenko
  0 siblings, 1 reply; 11+ messages in thread
From: Robin Murphy @ 2019-07-24 14:23 UTC (permalink / raw)
  To: Dmitry Osipenko, Yuehaibing, Sakari Ailus
  Cc: devel, linux-media, gregkh, linux-kernel, iommu, hverkuil-cisco,
	mchehab, yong.zhi

On 24/07/2019 15:09, Dmitry Osipenko wrote:
> 24.07.2019 17:03, Yuehaibing пишет:
>> On 2019/7/24 21:49, Robin Murphy wrote:
>>> On 24/07/2019 11:30, Sakari Ailus wrote:
>>>> Hi Yue,
>>>>
>>>> On Mon, Jul 22, 2019 at 09:47:49PM +0800, YueHaibing wrote:
>>>>> If IOMMU_SUPPORT is not set, ipu3 driver may select IOMMU_IOVA to m.
>>>>> But for many drivers, they use "select IOMMU_IOVA if IOMMU_SUPPORT"
>>>>> in the Kconfig, for example, CONFIG_TEGRA_VDE is set to y but IOMMU_IOVA
>>>>> is m, then the building fails like this:
>>>>>
>>>>> drivers/staging/media/tegra-vde/iommu.o: In function `tegra_vde_iommu_map':
>>>>> iommu.c:(.text+0x41): undefined reference to `alloc_iova'
>>>>> iommu.c:(.text+0x56): undefined reference to `__free_iova'
>>>>>
>>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>>> Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device driver")
>>>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>>>>> ---
>>>>>    drivers/staging/media/ipu3/Kconfig | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/staging/media/ipu3/Kconfig b/drivers/staging/media/ipu3/Kconfig
>>>>> index 4b51c67..b7df18f 100644
>>>>> --- a/drivers/staging/media/ipu3/Kconfig
>>>>> +++ b/drivers/staging/media/ipu3/Kconfig
>>>>> @@ -4,7 +4,7 @@ config VIDEO_IPU3_IMGU
>>>>>        depends on PCI && VIDEO_V4L2
>>>>>        depends on MEDIA_CONTROLLER && VIDEO_V4L2_SUBDEV_API
>>>>>        depends on X86
>>>>> -    select IOMMU_IOVA
>>>>> +    select IOMMU_IOVA if IOMMU_SUPPORT
>>>>
>>>> This doesn't seem right: the ipu3-cio2 driver needs IOMMU_IOVA
>>>> independently of IOMMU_SUPPORT.
>>>>
>>>> Looking at tegra-vde, it seems to depend on IOMMU_SUPPORT but that's not
>>>> declared in its Kconfig entry. I wonder if adding that would be the right
>>>> way to fix this.
>>>>
>>>> Cc'ing the IOMMU list.
> IOMMU_SUPPORT is optional for the Tegra-VDE driver.
> 
>>> Right, I also had the impression that we'd made the IOVA library completely standalone. And what does the IPU3 driver's Kconfig have to do with some *other* driver failing to link anyway?
> 
> I can see it failing if IPU3 is compiled as a loadable module, while
> Tegra-VDE is a built-in driver. Hence IOVA lib should be also a kernel
> module and thus the IOVA symbols will be missing during of linkage of
> the VDE driver.
> 
>> Oh, I misunderstand that IOMMU_IOVA is depend on IOMMU_SUPPORT, thank you for clarification.
>>
>> I will try to fix this in tegra-vde.
> 
> Probably IOVA could be selected independently of IOMMU_SUPPORT, but IOVA
> library isn't needed for the VDE driver if IOMMU_SUPPORT is disabled.

Oh, I think I get the problem now - tegra-vde/iommu.c is built 
unconditionally and relies on the static inline stubs for IOMMU and IOVA 
calls if !IOMMU_SUPPORT, but in a compile-test config where IOVA=m for 
other reasons, it then picks up the real declarations from linux/iova.h 
instead of the stubs, and things go downhill from there. So there is a 
real issue, but indeed it's Tegra-VDE which needs to be restructured to 
cope with such configurations, and not IPU3's (or anyone else who may 
select IOVA=m in future) job to work around it.

Robin.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled
  2019-07-24 14:23         ` Robin Murphy
@ 2019-07-24 14:28           ` Dmitry Osipenko
  2019-07-25  2:41             ` [PATCH] media: staging: tegra-vde: Fix build error YueHaibing
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2019-07-24 14:28 UTC (permalink / raw)
  To: Robin Murphy, Yuehaibing, Sakari Ailus
  Cc: devel, linux-media, gregkh, linux-kernel, iommu, hverkuil-cisco,
	mchehab, yong.zhi

24.07.2019 17:23, Robin Murphy пишет:
> On 24/07/2019 15:09, Dmitry Osipenko wrote:
>> 24.07.2019 17:03, Yuehaibing пишет:
>>> On 2019/7/24 21:49, Robin Murphy wrote:
>>>> On 24/07/2019 11:30, Sakari Ailus wrote:
>>>>> Hi Yue,
>>>>>
>>>>> On Mon, Jul 22, 2019 at 09:47:49PM +0800, YueHaibing wrote:
>>>>>> If IOMMU_SUPPORT is not set, ipu3 driver may select IOMMU_IOVA to m.
>>>>>> But for many drivers, they use "select IOMMU_IOVA if IOMMU_SUPPORT"
>>>>>> in the Kconfig, for example, CONFIG_TEGRA_VDE is set to y but
>>>>>> IOMMU_IOVA
>>>>>> is m, then the building fails like this:
>>>>>>
>>>>>> drivers/staging/media/tegra-vde/iommu.o: In function
>>>>>> `tegra_vde_iommu_map':
>>>>>> iommu.c:(.text+0x41): undefined reference to `alloc_iova'
>>>>>> iommu.c:(.text+0x56): undefined reference to `__free_iova'
>>>>>>
>>>>>> Reported-by: Hulk Robot <hulkci@huawei.com>
>>>>>> Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top
>>>>>> level pci device driver")
>>>>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>>>>>> ---
>>>>>>    drivers/staging/media/ipu3/Kconfig | 2 +-
>>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/staging/media/ipu3/Kconfig
>>>>>> b/drivers/staging/media/ipu3/Kconfig
>>>>>> index 4b51c67..b7df18f 100644
>>>>>> --- a/drivers/staging/media/ipu3/Kconfig
>>>>>> +++ b/drivers/staging/media/ipu3/Kconfig
>>>>>> @@ -4,7 +4,7 @@ config VIDEO_IPU3_IMGU
>>>>>>        depends on PCI && VIDEO_V4L2
>>>>>>        depends on MEDIA_CONTROLLER && VIDEO_V4L2_SUBDEV_API
>>>>>>        depends on X86
>>>>>> -    select IOMMU_IOVA
>>>>>> +    select IOMMU_IOVA if IOMMU_SUPPORT
>>>>>
>>>>> This doesn't seem right: the ipu3-cio2 driver needs IOMMU_IOVA
>>>>> independently of IOMMU_SUPPORT.
>>>>>
>>>>> Looking at tegra-vde, it seems to depend on IOMMU_SUPPORT but
>>>>> that's not
>>>>> declared in its Kconfig entry. I wonder if adding that would be the
>>>>> right
>>>>> way to fix this.
>>>>>
>>>>> Cc'ing the IOMMU list.
>> IOMMU_SUPPORT is optional for the Tegra-VDE driver.
>>
>>>> Right, I also had the impression that we'd made the IOVA library
>>>> completely standalone. And what does the IPU3 driver's Kconfig have
>>>> to do with some *other* driver failing to link anyway?
>>
>> I can see it failing if IPU3 is compiled as a loadable module, while
>> Tegra-VDE is a built-in driver. Hence IOVA lib should be also a kernel
>> module and thus the IOVA symbols will be missing during of linkage of
>> the VDE driver.
>>
>>> Oh, I misunderstand that IOMMU_IOVA is depend on IOMMU_SUPPORT, thank
>>> you for clarification.
>>>
>>> I will try to fix this in tegra-vde.
>>
>> Probably IOVA could be selected independently of IOMMU_SUPPORT, but IOVA
>> library isn't needed for the VDE driver if IOMMU_SUPPORT is disabled.
> 
> Oh, I think I get the problem now - tegra-vde/iommu.c is built
> unconditionally and relies on the static inline stubs for IOMMU and IOVA
> calls if !IOMMU_SUPPORT, but in a compile-test config where IOVA=m for
> other reasons, it then picks up the real declarations from linux/iova.h
> instead of the stubs, and things go downhill from there. So there is a
> real issue, but indeed it's Tegra-VDE which needs to be restructured to
> cope with such configurations, and not IPU3's (or anyone else who may
> select IOVA=m in future) job to work around it.

I guess it could be:

	select IOMMU_IOVA if (IOMMU_SUPPORT || COMPILE_TEST)

as a workaround.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] media: staging: tegra-vde: Fix build error
  2019-07-24 14:28           ` Dmitry Osipenko
@ 2019-07-25  2:41             ` YueHaibing
  2019-07-25  7:19               ` Dmitry Osipenko
  0 siblings, 1 reply; 11+ messages in thread
From: YueHaibing @ 2019-07-25  2:41 UTC (permalink / raw)
  To: digetx, mchehab, gregkh, thierry.reding, jonathanh, robin.murphy,
	sakari.ailus
  Cc: linux-kernel, devel, linux-media, linux-tegra, iommu, YueHaibing

If IOMMU_SUPPORT is not set, and COMPILE_TEST is y,
IOMMU_IOVA may be set to m. So building will fails:

drivers/staging/media/tegra-vde/iommu.o: In function `tegra_vde_iommu_map':
iommu.c:(.text+0x41): undefined reference to `alloc_iova'
iommu.c:(.text+0x56): undefined reference to `__free_iova'

Select IOMMU_IOVA while COMPILE_TEST is set to fix this.

Reported-by: Hulk Robot <hulkci@huawei.com>
Suggested-by: Dmitry Osipenko <digetx@gmail.com>
Fixes: b301f8de1925 ("media: staging: media: tegra-vde: Add IOMMU support")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/staging/media/tegra-vde/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/tegra-vde/Kconfig b/drivers/staging/media/tegra-vde/Kconfig
index 2e7f644..ba49ea5 100644
--- a/drivers/staging/media/tegra-vde/Kconfig
+++ b/drivers/staging/media/tegra-vde/Kconfig
@@ -3,7 +3,7 @@ config TEGRA_VDE
 	tristate "NVIDIA Tegra Video Decoder Engine driver"
 	depends on ARCH_TEGRA || COMPILE_TEST
 	select DMA_SHARED_BUFFER
-	select IOMMU_IOVA if IOMMU_SUPPORT
+	select IOMMU_IOVA if (IOMMU_SUPPORT || COMPILE_TEST)
 	select SRAM
 	help
 	    Say Y here to enable support for the NVIDIA Tegra video decoder
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] media: staging: tegra-vde: Fix build error
  2019-07-25  2:41             ` [PATCH] media: staging: tegra-vde: Fix build error YueHaibing
@ 2019-07-25  7:19               ` Dmitry Osipenko
  2019-09-20 19:32                 ` Arnd Bergmann
  0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2019-07-25  7:19 UTC (permalink / raw)
  To: YueHaibing, mchehab, gregkh, thierry.reding, jonathanh,
	robin.murphy, sakari.ailus
  Cc: linux-kernel, devel, linux-media, linux-tegra, iommu

25.07.2019 5:41, YueHaibing пишет:
> If IOMMU_SUPPORT is not set, and COMPILE_TEST is y,
> IOMMU_IOVA may be set to m. So building will fails:
> 
> drivers/staging/media/tegra-vde/iommu.o: In function `tegra_vde_iommu_map':
> iommu.c:(.text+0x41): undefined reference to `alloc_iova'
> iommu.c:(.text+0x56): undefined reference to `__free_iova'
> 
> Select IOMMU_IOVA while COMPILE_TEST is set to fix this.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Suggested-by: Dmitry Osipenko <digetx@gmail.com>
> Fixes: b301f8de1925 ("media: staging: media: tegra-vde: Add IOMMU support")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  drivers/staging/media/tegra-vde/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/tegra-vde/Kconfig b/drivers/staging/media/tegra-vde/Kconfig
> index 2e7f644..ba49ea5 100644
> --- a/drivers/staging/media/tegra-vde/Kconfig
> +++ b/drivers/staging/media/tegra-vde/Kconfig
> @@ -3,7 +3,7 @@ config TEGRA_VDE
>  	tristate "NVIDIA Tegra Video Decoder Engine driver"
>  	depends on ARCH_TEGRA || COMPILE_TEST
>  	select DMA_SHARED_BUFFER
> -	select IOMMU_IOVA if IOMMU_SUPPORT
> +	select IOMMU_IOVA if (IOMMU_SUPPORT || COMPILE_TEST)
>  	select SRAM
>  	help
>  	    Say Y here to enable support for the NVIDIA Tegra video decoder
> 

This results in missing the case of compile-testing !IOMMU_IOVA for the
driver, but probably that's not a big deal.

Acked-by: Dmitry Osipenko <digetx@gmail.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] media: staging: tegra-vde: Fix build error
  2019-07-25  7:19               ` Dmitry Osipenko
@ 2019-09-20 19:32                 ` Arnd Bergmann
  2019-09-20 22:26                   ` Dmitry Osipenko
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2019-09-20 19:32 UTC (permalink / raw)
  To: Dmitry Osipenko
  Cc: YueHaibing, Mauro Carvalho Chehab, gregkh, Thierry Reding,
	Jonathan Hunter, Robin Murphy, Sakari Ailus, linux-kernel,
	driverdevel, Linux Media Mailing List,
	open list:TEGRA ARCHITECTURE SUPPORT, open list:IOMMU DRIVERS

On Thu, Jul 25, 2019 at 2:24 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>
> 25.07.2019 5:41, YueHaibing пишет:
> > If IOMMU_SUPPORT is not set, and COMPILE_TEST is y,
> > IOMMU_IOVA may be set to m. So building will fails:
> >
> > drivers/staging/media/tegra-vde/iommu.o: In function `tegra_vde_iommu_map':
> > iommu.c:(.text+0x41): undefined reference to `alloc_iova'
> > iommu.c:(.text+0x56): undefined reference to `__free_iova'
> >
> > Select IOMMU_IOVA while COMPILE_TEST is set to fix this.

> > @@ -3,7 +3,7 @@ config TEGRA_VDE
> >       tristate "NVIDIA Tegra Video Decoder Engine driver"
> >       depends on ARCH_TEGRA || COMPILE_TEST
> >       select DMA_SHARED_BUFFER
> > -     select IOMMU_IOVA if IOMMU_SUPPORT
> > +     select IOMMU_IOVA if (IOMMU_SUPPORT || COMPILE_TEST)
> >       select SRAM
> >       help
> >           Say Y here to enable support for the NVIDIA Tegra video decoder
> >
>
> This results in missing the case of compile-testing !IOMMU_IOVA for the
> driver, but probably that's not a big deal.
>
> Acked-by: Dmitry Osipenko <digetx@gmail.com>

I don't know what happened here, but the patch from YueHaibing caused this
error for me, which is very much like the problem it was meant to fix:

drivers/gpu/host1x/dev.o: In function `host1x_probe':
dev.c:(.text+0x1734): undefined reference to `put_iova_domain'
dev.c:(.text+0x1744): undefined reference to `iova_cache_put'
drivers/gpu/host1x/dev.o: In function `host1x_remove':
dev.c:(.text+0x1894): undefined reference to `put_iova_domain'
dev.c:(.text+0x1898): undefined reference to `iova_cache_put'
drivers/gpu/host1x/cdma.o: In function `host1x_cdma_init':
cdma.c:(.text+0x5d0): undefined reference to `alloc_iova'
cdma.c:(.text+0x61c): undefined reference to `__free_iova'
drivers/gpu/host1x/cdma.o: In function `host1x_cdma_deinit':
cdma.c:(.text+0x6c8): undefined reference to `free_iova'
drivers/gpu/host1x/job.o: In function `host1x_job_pin':
job.c:(.text+0x514): undefined reference to `alloc_iova'
job.c:(.text+0x528): undefined reference to `__free_iova'
drivers/gpu/host1x/job.o: In function `host1x_job_unpin':
job.c:(.text+0x5bc): undefined reference to `free_iova'

After reverthing commit 6b2265975239 ("media: staging:
tegra-vde: Fix build error"), I can no longer reproduce the
issue.

       Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] media: staging: tegra-vde: Fix build error
  2019-09-20 19:32                 ` Arnd Bergmann
@ 2019-09-20 22:26                   ` Dmitry Osipenko
  0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2019-09-20 22:26 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: YueHaibing, Mauro Carvalho Chehab, gregkh, Thierry Reding,
	Jonathan Hunter, Robin Murphy, Sakari Ailus, linux-kernel,
	driverdevel, Linux Media Mailing List,
	open list:TEGRA ARCHITECTURE SUPPORT, open list:IOMMU DRIVERS

20.09.2019 22:32, Arnd Bergmann пишет:
> On Thu, Jul 25, 2019 at 2:24 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> 25.07.2019 5:41, YueHaibing пишет:
>>> If IOMMU_SUPPORT is not set, and COMPILE_TEST is y,
>>> IOMMU_IOVA may be set to m. So building will fails:
>>>
>>> drivers/staging/media/tegra-vde/iommu.o: In function `tegra_vde_iommu_map':
>>> iommu.c:(.text+0x41): undefined reference to `alloc_iova'
>>> iommu.c:(.text+0x56): undefined reference to `__free_iova'
>>>
>>> Select IOMMU_IOVA while COMPILE_TEST is set to fix this.
> 
>>> @@ -3,7 +3,7 @@ config TEGRA_VDE
>>>       tristate "NVIDIA Tegra Video Decoder Engine driver"
>>>       depends on ARCH_TEGRA || COMPILE_TEST
>>>       select DMA_SHARED_BUFFER
>>> -     select IOMMU_IOVA if IOMMU_SUPPORT
>>> +     select IOMMU_IOVA if (IOMMU_SUPPORT || COMPILE_TEST)
>>>       select SRAM
>>>       help
>>>           Say Y here to enable support for the NVIDIA Tegra video decoder
>>>
>>
>> This results in missing the case of compile-testing !IOMMU_IOVA for the
>> driver, but probably that's not a big deal.
>>
>> Acked-by: Dmitry Osipenko <digetx@gmail.com>
> 
> I don't know what happened here, but the patch from YueHaibing caused this
> error for me, which is very much like the problem it was meant to fix:
> 
> drivers/gpu/host1x/dev.o: In function `host1x_probe':
> dev.c:(.text+0x1734): undefined reference to `put_iova_domain'
> dev.c:(.text+0x1744): undefined reference to `iova_cache_put'
> drivers/gpu/host1x/dev.o: In function `host1x_remove':
> dev.c:(.text+0x1894): undefined reference to `put_iova_domain'
> dev.c:(.text+0x1898): undefined reference to `iova_cache_put'
> drivers/gpu/host1x/cdma.o: In function `host1x_cdma_init':
> cdma.c:(.text+0x5d0): undefined reference to `alloc_iova'
> cdma.c:(.text+0x61c): undefined reference to `__free_iova'
> drivers/gpu/host1x/cdma.o: In function `host1x_cdma_deinit':
> cdma.c:(.text+0x6c8): undefined reference to `free_iova'
> drivers/gpu/host1x/job.o: In function `host1x_job_pin':
> job.c:(.text+0x514): undefined reference to `alloc_iova'
> job.c:(.text+0x528): undefined reference to `__free_iova'
> drivers/gpu/host1x/job.o: In function `host1x_job_unpin':
> job.c:(.text+0x5bc): undefined reference to `free_iova'
> 
> After reverthing commit 6b2265975239 ("media: staging:
> tegra-vde: Fix build error"), I can no longer reproduce the
> issue.

There is a follow up here: https://patchwork.ozlabs.org/patch/1153176/

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-09-20 22:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-22 13:47 [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled YueHaibing
2019-07-24 10:30 ` Sakari Ailus
2019-07-24 13:49   ` Robin Murphy
2019-07-24 14:03     ` Yuehaibing
2019-07-24 14:09       ` Dmitry Osipenko
2019-07-24 14:23         ` Robin Murphy
2019-07-24 14:28           ` Dmitry Osipenko
2019-07-25  2:41             ` [PATCH] media: staging: tegra-vde: Fix build error YueHaibing
2019-07-25  7:19               ` Dmitry Osipenko
2019-09-20 19:32                 ` Arnd Bergmann
2019-09-20 22:26                   ` Dmitry Osipenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).