[PATCH 1/4] libcamera: rkisp1: Drop base IPA headers inclusion

[Umang Jain]

Hi Laurent,

On 01/07/24 7:08 pm, Laurent Pinchart wrote:
> On Mon, Jul 01, 2024 at 10:46:45AM +0100, Kieran Bingham wrote:
>> Quoting Umang Jain (2024-07-01 08:57:17)
>>> The IPA headers ipa_interface.h and core_ipa_interface.h are
>>> included as part of the rkisp1_ipa_interface.h generated from
>>> module_ipa_interface.h.tmpl. Drop them as deemed redundant.
>> I wonder why these were put in in the first place. Are there any
>> artificats that would mean we should include them becuase of 'include
>> what you use' ? But that said - I really can't see a reason to inlcude
>> the base interface when we include a targetted/specific interface..
> libcamera/ipa/ipa_interface.h defines the interface exposed by the IPA
> module binary. That's the top-level ipaCreate() function, and the base
> IPAInterface class.
>
> The file is included in rkisp1_ipa_interface.h for the definition of the
> IPAInterface class, as IPARkISP1Interface derives from it. rkisp1.cpp
> doesn't make use of the base IPAInterface class, so there's no need to
> include ipa_interface.h for that.
>
> The ipaCreate() function declaration, however, is needed by rkisp1.cpp.
> As it's declared by ipa_interface.h, we include the header in the
> top-level .cpp file of each IPA module, and I think it should stay
> there, following the "include what you use" principle, to avoid
> depending on indirect includes.

Ah, I should have spotted that. Completely makes sense.

Hence, only the hunk of pipeline/rkisp1/rkisp1.cpp makes sense for the 
series. Rest is not applicable.

>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>>> ---
>>>   src/ipa/rkisp1/rkisp1.cpp                | 1 -
>>>   src/libcamera/pipeline/rkisp1/rkisp1.cpp | 1 -
>>>   2 files changed, 2 deletions(-)
>>>
>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
>>> index d31cdbab..a43116d7 100644
>>> --- a/src/ipa/rkisp1/rkisp1.cpp
>>> +++ b/src/ipa/rkisp1/rkisp1.cpp
>>> @@ -19,7 +19,6 @@
>>>   
>>>   #include <libcamera/control_ids.h>
>>>   #include <libcamera/framebuffer.h>
>>> -#include <libcamera/ipa/ipa_interface.h>
>>>   #include <libcamera/ipa/ipa_module_info.h>
>>>   #include <libcamera/ipa/rkisp1_ipa_interface.h>
>>>   #include <libcamera/request.h>
>>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>>> index 4cbf105d..97cd78a7 100644
>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>>> @@ -27,7 +27,6 @@
>>>   #include <libcamera/stream.h>
>>>   #include <libcamera/transform.h>
>>>   
>>> -#include <libcamera/ipa/core_ipa_interface.h>
>>>   #include <libcamera/ipa/rkisp1_ipa_interface.h>
>>>   #include <libcamera/ipa/rkisp1_ipa_proxy.h>
>>>