[libcamera-devel,09/23] libcamera: IPAModule: Replace ipa_context with IPAInterface

Message ID 20200915142038.28757-10-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • IPA isolation implementation
Related show

Commit Message

Paul Elder Sept. 15, 2020, 2:20 p.m. UTC
With the new IPC infrastructure, we no longer need the C interface as
provided by struct ipa_context. Make ipaCreate_() and createInterface()
return IPAInterface.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 include/libcamera/internal/ipa_module.h |  4 ++--
 src/libcamera/ipa_module.cpp            | 16 +---------------
 src/libcamera/meson.build               |  1 -
 3 files changed, 3 insertions(+), 18 deletions(-)

Comments

Niklas Söderlund Sept. 19, 2020, 11:38 a.m. UTC | #1
Hi Paul,

Thanks for your work.

On 2020-09-15 23:20:24 +0900, Paul Elder wrote:
> With the new IPC infrastructure, we no longer need the C interface as
> provided by struct ipa_context. Make ipaCreate_() and createInterface()
> return IPAInterface.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  include/libcamera/internal/ipa_module.h |  4 ++--
>  src/libcamera/ipa_module.cpp            | 16 +---------------
>  src/libcamera/meson.build               |  1 -
>  3 files changed, 3 insertions(+), 18 deletions(-)
> 
> diff --git a/include/libcamera/internal/ipa_module.h b/include/libcamera/internal/ipa_module.h
> index c2df2476..19fc5827 100644
> --- a/include/libcamera/internal/ipa_module.h
> +++ b/include/libcamera/internal/ipa_module.h
> @@ -33,7 +33,7 @@ public:
>  
>  	bool load();
>  
> -	struct ipa_context *createContext();
> +	IPAInterface *createInterface();
>  
>  	bool match(PipelineHandler *pipe,
>  		   uint32_t minVersion, uint32_t maxVersion) const;
> @@ -52,7 +52,7 @@ private:
>  	bool loaded_;
>  
>  	void *dlHandle_;
> -	typedef struct ipa_context *(*IPAIntfFactory)();
> +	typedef IPAInterface *(*IPAIntfFactory)(void);
>  	IPAIntfFactory ipaCreate_;
>  };
>  
> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
> index de512a7f..008325c4 100644
> --- a/src/libcamera/ipa_module.cpp
> +++ b/src/libcamera/ipa_module.cpp
> @@ -438,21 +438,7 @@ bool IPAModule::load()
>  	return true;
>  }
>  
> -/**
> - * \brief Instantiate an IPA context
> - *
> - * After loading the IPA module with load(), this method creates an instance of
> - * the IPA module context. Ownership of the context is passed to the caller, and
> - * the context shall be destroyed by calling the \ref ipa_context_ops::destroy
> - * "ipa_context::ops::destroy()" function.
> - *
> - * Calling this function on a module that has not yet been loaded, or an
> - * invalid module (as returned by load() and isValid(), respectively) is
> - * an error.
> - *
> - * \return The IPA context on success, or nullptr on error
> - */
> -struct ipa_context *IPAModule::createContext()

With the documentation added this would get my R-b.

> +IPAInterface *IPAModule::createInterface()
>  {
>  	if (!valid_ || !loaded_)
>  		return nullptr;
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 9d1abeb0..a2a6ed04 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -21,7 +21,6 @@ libcamera_sources = files([
>      'formats.cpp',
>      'framebuffer_allocator.cpp',
>      'geometry.cpp',
> -    'ipa_context_wrapper.cpp',
>      'ipa_controls.cpp',
>      'ipa_data_serializer.cpp',
>      'ipa_ipc.cpp',
> -- 
> 2.27.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/include/libcamera/internal/ipa_module.h b/include/libcamera/internal/ipa_module.h
index c2df2476..19fc5827 100644
--- a/include/libcamera/internal/ipa_module.h
+++ b/include/libcamera/internal/ipa_module.h
@@ -33,7 +33,7 @@  public:
 
 	bool load();
 
-	struct ipa_context *createContext();
+	IPAInterface *createInterface();
 
 	bool match(PipelineHandler *pipe,
 		   uint32_t minVersion, uint32_t maxVersion) const;
@@ -52,7 +52,7 @@  private:
 	bool loaded_;
 
 	void *dlHandle_;
-	typedef struct ipa_context *(*IPAIntfFactory)();
+	typedef IPAInterface *(*IPAIntfFactory)(void);
 	IPAIntfFactory ipaCreate_;
 };
 
diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
index de512a7f..008325c4 100644
--- a/src/libcamera/ipa_module.cpp
+++ b/src/libcamera/ipa_module.cpp
@@ -438,21 +438,7 @@  bool IPAModule::load()
 	return true;
 }
 
-/**
- * \brief Instantiate an IPA context
- *
- * After loading the IPA module with load(), this method creates an instance of
- * the IPA module context. Ownership of the context is passed to the caller, and
- * the context shall be destroyed by calling the \ref ipa_context_ops::destroy
- * "ipa_context::ops::destroy()" function.
- *
- * Calling this function on a module that has not yet been loaded, or an
- * invalid module (as returned by load() and isValid(), respectively) is
- * an error.
- *
- * \return The IPA context on success, or nullptr on error
- */
-struct ipa_context *IPAModule::createContext()
+IPAInterface *IPAModule::createInterface()
 {
 	if (!valid_ || !loaded_)
 		return nullptr;
diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
index 9d1abeb0..a2a6ed04 100644
--- a/src/libcamera/meson.build
+++ b/src/libcamera/meson.build
@@ -21,7 +21,6 @@  libcamera_sources = files([
     'formats.cpp',
     'framebuffer_allocator.cpp',
     'geometry.cpp',
-    'ipa_context_wrapper.cpp',
     'ipa_controls.cpp',
     'ipa_data_serializer.cpp',
     'ipa_ipc.cpp',