Re: [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled

2019-07-24 Thread Dmitry Osipenko 
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 
>> Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top
>> level pci device driver")
>> Signed-off-by: YueHaibing 
>> ---
>>    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.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled

2019-07-24 Thread 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 
Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device 
driver")
Signed-off-by: YueHaibing 
---
   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.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled

2019-07-24 Thread Dmitry Osipenko 
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 
 Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci 
 device driver")
 Signed-off-by: YueHaibing 
 ---
   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.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled

2019-07-24 Thread 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 
>>> Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci 
>>> device driver")
>>> Signed-off-by: YueHaibing 
>>> ---
>>>   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,
>>
> 
> .
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled

2019-07-24 Thread Robin Murphy 

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 
Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device 
driver")
Signed-off-by: YueHaibing 
---
  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,



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled

2019-07-24 Thread Sakari Ailus 
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 
> Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci 
> device driver")
> Signed-off-by: YueHaibing 
> ---
>  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.ai...@linux.intel.com
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] media: staging: ipu3: Enable IOVA API only when IOMMU support is enabled

2019-07-22 Thread 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 
Fixes: 7fc7af649ca7 ("media: staging/intel-ipu3: Add imgu top level pci device 
driver")
Signed-off-by: YueHaibing 
---
 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


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel