[RFC,v2,8/8] libcamera: software_isp: debayer_egl: Remove EGL context switch

27730 diff mbox series

`eGL::initEGLContext()`, which is called from the worker thread already
includes a call to `eGL::makeCurrent()`, so calling it for each
`process()` is not necessary, so skip it.

Instead, add a function that asserts that the execution is in the correct
thread, and call that.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 include/libcamera/internal/egl.h           | 9 ++++++++-
 src/libcamera/egl.cpp                      | 8 ++++++++
 src/libcamera/software_isp/debayer_egl.cpp | 4 +---
 3 files changed, 17 insertions(+), 4 deletions(-)

Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes:

> `eGL::initEGLContext()`, which is called from the worker thread already
> includes a call to `eGL::makeCurrent()`, so calling it for each
> `process()` is not necessary, so skip it.
>
> Instead, add a function that asserts that the execution is in the correct
> thread, and call that.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  include/libcamera/internal/egl.h           | 9 ++++++++-
>  src/libcamera/egl.cpp                      | 8 ++++++++
>  src/libcamera/software_isp/debayer_egl.cpp | 4 +---
>  3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h
> index e18bb91f4d..8f2ef296c3 100644
> --- a/include/libcamera/internal/egl.h
> +++ b/include/libcamera/internal/egl.h
> @@ -14,6 +14,7 @@
>  
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/span.h>
> +#include <libcamera/base/thread.h>
>  #include <libcamera/base/utils.h>
>  
>  /*
> @@ -116,7 +117,6 @@ public:
>  	void activateBindTexture(eGLImage &eglImage);
>  
>  	void pushEnv(std::vector<std::string> &shaderEnv, const char *str);
> -	void makeCurrent();
>  
>  	int compileVertexShader(GLuint &shaderId, Span<const unsigned char> shaderData,
>  				Span<const std::string> shaderEnv);
> @@ -127,6 +127,11 @@ public:
>  	void syncOutput();
>  	void flushOutput();
>  
> +	void assertThread() const
> +	{
> +		ASSERT(tid_ == Thread::currentId());
> +	}

Should all these assertions be replaced by this method?

> +
>  private:
>  	LIBCAMERA_DISABLE_COPY_AND_MOVE(eGL)
>  
> @@ -140,6 +145,8 @@ private:
>  
>  	int createDMABufTexture2D(eGLImage &eglImage, int fd, bool output);
>  
> +	void makeCurrent();
> +
>  	struct VTable {
>  		PFNGLEGLIMAGETARGETTEXTURE2DOESPROC glEGLImageTargetTexture2DOES;
>  		PFNEGLCREATEIMAGEKHRPROC eglCreateImageKHR;
> diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp
> index edb00ef997..5e5535f2be 100644
> --- a/src/libcamera/egl.cpp
> +++ b/src/libcamera/egl.cpp
> @@ -104,6 +104,14 @@ void eGL::flushOutput()
>  	glFlush();
>  }
>  
> +/**
> + * \fn eGL::assertThread()
> + * \brief Assert that the execution is in the right thread
> + *
> + * Asserts that the execution is in the thread for which
> + * the created EGL context has been made current.
> + */
> +
>  /**
>   * \brief Attach a texture to a frame-buffer-object
>   *
> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
> index 35a5c56321..97aa037935 100644
> --- a/src/libcamera/software_isp/debayer_egl.cpp
> +++ b/src/libcamera/software_isp/debayer_egl.cpp
> @@ -578,9 +578,6 @@ int DebayerEGL::debayerGPU(FrameBuffer *input, FrameBuffer *output, const Debaye
>  	eGLImage *eglImageIn;
>  	eGLImage *eglImageOut;
>  
> -	/* eGL context switch */
> -	egl_.makeCurrent();
> -
>  	eglImageIn = getCachedInputFrameBuffer(input, inMapped, inDmaSyncer);
>  	if (!eglImageIn)
>  		return -ENOMEM;
> @@ -608,6 +605,7 @@ int DebayerEGL::debayerGPU(FrameBuffer *input, FrameBuffer *output, const Debaye
>  
>  void DebayerEGL::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, const DebayerParams &params)
>  {
> +	egl_.assertThread();
>  	bench_.startFrame();
>  
>  	/* Copy metadata from the input buffer */