[PATCH v3 01/23] libcamera: rkisp1: Drop base IPA headers inclusion

[Milan Zamazal]

Milan Zamazal <mzamazal at redhat.com> writes:

> Hi Laurent,
>
> Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:
>
>> On Wed, Jul 17, 2024 at 10:54:22AM +0200, Milan Zamazal wrote:
>>> From: Umang Jain <umang.jain at ideasonboard.com>
>>> 
>>
>>> 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.
>>
>> 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.
>>
>> Can you drop the first hunk of this patch and update the commit message
>> ?
>
> Oops, the first four patches by Umang were included by mistake, due to a
> wrong rebase, sorry about it.
>
> Anyway, maybe Umang would like to fix and resubmit this patch as
> explained above?  (If not, I can handle it as a penance for my
> mistake :-), separately from this series.)

After closer look, I can see the other include is also needed, so the
patch can be dropped completely.

FWIW LSP does a good job to indicate really unused headers.  I'm trying
to clean up the includes based on that.  There may be some pitfalls but
I suppose it's worth to attempt.

>>> 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 23e0826c..c5c85c8d 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>
>>>