[libcamera-devel,01/13] libcamera: pipeline: vimc: Increase version of dummy IPA

Message ID 20190828011710.32128-2-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: ipa: Add basic IPA support
Related show

Commit Message

Niklas Söderlund Aug. 28, 2019, 1:16 a.m. UTC
An IPA version number of 0 will be redefined as No IPA support. Increase
the VIMC dummy IPA version to 1 to allow it to keep loading.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/ipa/ipa_dummy.cpp           | 2 +-
 src/libcamera/pipeline/vimc.cpp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Kieran Bingham Aug. 29, 2019, 10:03 a.m. UTC | #1
Hi Niklas,

On 28/08/2019 02:16, Niklas Söderlund wrote:
> An IPA version number of 0 will be redefined as No IPA support. Increase
> the VIMC dummy IPA version to 1 to allow it to keep loading.
> 

This seems reasonably sane, but I'm not sure I fully understand the
rationale.

If a pipeline handler doesn't want any IPA support, then wouldn't it ...
simply not create an IPA?

Who will ever set a version of 0 ?

--
Kieran


> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/ipa/ipa_dummy.cpp           | 2 +-
>  src/libcamera/pipeline/vimc.cpp | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp
> index 4c8b6657689d0c9f..b0e944a17fc5cffb 100644
> --- a/src/ipa/ipa_dummy.cpp
> +++ b/src/ipa/ipa_dummy.cpp
> @@ -31,7 +31,7 @@ int IPADummy::init()
>  extern "C" {
>  const struct IPAModuleInfo ipaModuleInfo = {
>  	IPA_MODULE_API_VERSION,
> -	0,
> +	1,
>  	"PipelineHandlerVimc",
>  	"Dummy IPA for Vimc",
>  	"LGPL-2.1-or-later",
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index f8f91d6219b1aee4..e5c4890501db71c8 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -361,7 +361,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>  	if (!media)
>  		return false;
>  
> -	ipa_ = IPAManager::instance()->createIPA(this, 0, 0);
> +	ipa_ = IPAManager::instance()->createIPA(this, 1, 1);
>  	if (ipa_ == nullptr)
>  		LOG(VIMC, Warning) << "no matching IPA found";
>  	else
>
Kieran Bingham Aug. 29, 2019, 10:06 a.m. UTC | #2
Hi Niklas,

On 29/08/2019 11:03, Kieran Bingham wrote:
> Hi Niklas,
> 
> On 28/08/2019 02:16, Niklas Söderlund wrote:
>> An IPA version number of 0 will be redefined as No IPA support. Increase
>> the VIMC dummy IPA version to 1 to allow it to keep loading.
>>
> 
> This seems reasonably sane, but I'm not sure I fully understand the
> rationale.
> 
> If a pipeline handler doesn't want any IPA support, then wouldn't it ...
> simply not create an IPA?
> 
> Who will ever set a version of 0 ?

Aha - never mind - I've now fully read the /next/ patch.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> --
> Kieran
> 
> 
>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
>> ---
>>  src/ipa/ipa_dummy.cpp           | 2 +-
>>  src/libcamera/pipeline/vimc.cpp | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp
>> index 4c8b6657689d0c9f..b0e944a17fc5cffb 100644
>> --- a/src/ipa/ipa_dummy.cpp
>> +++ b/src/ipa/ipa_dummy.cpp
>> @@ -31,7 +31,7 @@ int IPADummy::init()
>>  extern "C" {
>>  const struct IPAModuleInfo ipaModuleInfo = {
>>  	IPA_MODULE_API_VERSION,
>> -	0,
>> +	1,
>>  	"PipelineHandlerVimc",
>>  	"Dummy IPA for Vimc",
>>  	"LGPL-2.1-or-later",
>> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
>> index f8f91d6219b1aee4..e5c4890501db71c8 100644
>> --- a/src/libcamera/pipeline/vimc.cpp
>> +++ b/src/libcamera/pipeline/vimc.cpp
>> @@ -361,7 +361,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>>  	if (!media)
>>  		return false;
>>  
>> -	ipa_ = IPAManager::instance()->createIPA(this, 0, 0);
>> +	ipa_ = IPAManager::instance()->createIPA(this, 1, 1);
>>  	if (ipa_ == nullptr)
>>  		LOG(VIMC, Warning) << "no matching IPA found";
>>  	else
>>
>

Patch

diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp
index 4c8b6657689d0c9f..b0e944a17fc5cffb 100644
--- a/src/ipa/ipa_dummy.cpp
+++ b/src/ipa/ipa_dummy.cpp
@@ -31,7 +31,7 @@  int IPADummy::init()
 extern "C" {
 const struct IPAModuleInfo ipaModuleInfo = {
 	IPA_MODULE_API_VERSION,
-	0,
+	1,
 	"PipelineHandlerVimc",
 	"Dummy IPA for Vimc",
 	"LGPL-2.1-or-later",
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index f8f91d6219b1aee4..e5c4890501db71c8 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -361,7 +361,7 @@  bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
 	if (!media)
 		return false;
 
-	ipa_ = IPAManager::instance()->createIPA(this, 0, 0);
+	ipa_ = IPAManager::instance()->createIPA(this, 1, 1);
 	if (ipa_ == nullptr)
 		LOG(VIMC, Warning) << "no matching IPA found";
 	else