[v4,5/7] libcamera: egl: Add activateBindTexture
diff mbox series

Message ID 20260706110120.2509826-6-bryan.odonoghue@linaro.org
State New
Headers show
Series
  • libcamera: software_isp: gpu: Add go faster stripes
Related show

Commit Message

Bryan O'Donoghue July 6, 2026, 11:01 a.m. UTC
When operating from a texture cache on dma-buf inputs we will no longer
create new textures nor attach those textures for dma-buf handles we have
already encountered.

This means we will use the texture id associated with a given texture unit
to switch between one texture and another. The pages associated with the
texture will have been populated with new data by the CSI2 receiver. All we
will do is say to the GPU "reuse this texture id" aka zero-copy.

However we must also activate and bind that texture for each loop. This
cost is small but necessary for zero-copy.

Reviewed-by: Robert Mader <robert.mader@collabora.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 include/libcamera/internal/egl.h |  1 +
 src/libcamera/egl.cpp            | 24 +++++++++++++++++++-----
 2 files changed, 20 insertions(+), 5 deletions(-)

Comments

Robert Mader July 6, 2026, 11:13 a.m. UTC | #1
Hi Bryan, thanks for the updated series.

On 06.07.26 13:01, Bryan O'Donoghue wrote:
> When operating from a texture cache on dma-buf inputs we will no longer
> create new textures nor attach those textures for dma-buf handles we have
> already encountered.
>
> This means we will use the texture id associated with a given texture unit
> to switch between one texture and another. The pages associated with the
> texture will have been populated with new data by the CSI2 receiver. All we
> will do is say to the GPU "reuse this texture id" aka zero-copy.
>
> However we must also activate and bind that texture for each loop. This
> cost is small but necessary for zero-copy.
>
> Reviewed-by: Robert Mader <robert.mader@collabora.com>
> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   include/libcamera/internal/egl.h |  1 +
>   src/libcamera/egl.cpp            | 24 +++++++++++++++++++-----
>   2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h
> index 9b4261c55..1e31d490b 100644
> --- a/include/libcamera/internal/egl.h
> +++ b/include/libcamera/internal/egl.h
> @@ -112,6 +112,7 @@ public:
>   	void createOutputTexture2D(eGLImage &eglImage);
>   
>   	int attachTextureToFBO(eGLImage &eglImage);
> +	void activateBindTexture(eGLImage &eglImage);
>   
>   	void pushEnv(std::vector<std::string> &shaderEnv, const char *str);
>   	void makeCurrent();
> diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp
> index b91f9096a..fae907881 100644
> --- a/src/libcamera/egl.cpp
> +++ b/src/libcamera/egl.cpp
> @@ -138,6 +138,23 @@ int eGL::attachTextureToFBO(eGLImage &eglImage)
>   	return ret;
>   }
>   
> +/**
> + * \brief Activate a texture unit and bind a texture to that unit
> + * \param[in,out] eglImage EGL image containing data related to unit and texture id
> + *
> + * When we create a texture we will bind a texture unit and texture id so
> + * we can set filters. For the case where a texture already exists though
> + * we need to activate and bind an existing texture. This helper function
> + * facilitates both cases.
> + *
> + */
> +void eGL::activateBindTexture(eGLImage &eglImage)
> +{
> +	// Bind texture unit and texture
> +	glActiveTexture(eglImage.texture_unit_);
> +	glBindTexture(GL_TEXTURE_2D, eglImage.texture_);
> +}
> +
>   /**
>    * \brief Create a DMA-BUF backed 2D texture
>    * \param[in,out] eglImage EGL image to associate with the DMA-BUF
> @@ -197,9 +214,7 @@ int eGL::createDMABufTexture2D(eGLImage &eglImage, int fd, bool output)
>   		return -ENODEV;
>   	}
>   
> -	// Bind texture unit and texture
> -	glActiveTexture(eglImage.texture_unit_);
> -	glBindTexture(GL_TEXTURE_2D, eglImage.texture_);
> +	activateBindTexture(eglImage);
>   
>   	// Generate texture with filter semantics
>   	glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, image);
> @@ -270,8 +285,7 @@ void eGL::createTexture2D(eGLImage &eglImage, void *data)
>   {
>   	ASSERT(tid_ == Thread::currentId());
>   
> -	glActiveTexture(eglImage.texture_unit_);
> -	glBindTexture(GL_TEXTURE_2D, eglImage.texture_);
> +	activateBindTexture(eglImage);
>   
>   	// Generate texture, bind, associate image to texture, configure, unbind
>   	glTexImage2D(GL_TEXTURE_2D, 0, eglImage.format_, eglImage.width_, eglImage.height_, 0, eglImage.format_, GL_UNSIGNED_BYTE, data);
This still misses using `activateBindTexture()` in `updateTexture2D()` - 
which is why I again suggest to move this commit *before* "libcamera: 
egl: Add updateTexture2D" ;)
Bryan O'Donoghue July 6, 2026, 11:18 a.m. UTC | #2
On 06/07/2026 12:13, Robert Mader wrote:
> Hi Bryan, thanks for the updated series.
> 
> On 06.07.26 13:01, Bryan O'Donoghue wrote:
>> When operating from a texture cache on dma-buf inputs we will no longer
>> create new textures nor attach those textures for dma-buf handles we have
>> already encountered.
>>
>> This means we will use the texture id associated with a given texture 
>> unit
>> to switch between one texture and another. The pages associated with the
>> texture will have been populated with new data by the CSI2 receiver. 
>> All we
>> will do is say to the GPU "reuse this texture id" aka zero-copy.
>>
>> However we must also activate and bind that texture for each loop. This
>> cost is small but necessary for zero-copy.
>>
>> Reviewed-by: Robert Mader <robert.mader@collabora.com>
>> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   include/libcamera/internal/egl.h |  1 +
>>   src/libcamera/egl.cpp            | 24 +++++++++++++++++++-----
>>   2 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/libcamera/internal/egl.h b/include/libcamera/ 
>> internal/egl.h
>> index 9b4261c55..1e31d490b 100644
>> --- a/include/libcamera/internal/egl.h
>> +++ b/include/libcamera/internal/egl.h
>> @@ -112,6 +112,7 @@ public:
>>       void createOutputTexture2D(eGLImage &eglImage);
>>       int attachTextureToFBO(eGLImage &eglImage);
>> +    void activateBindTexture(eGLImage &eglImage);
>>       void pushEnv(std::vector<std::string> &shaderEnv, const char *str);
>>       void makeCurrent();
>> diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp
>> index b91f9096a..fae907881 100644
>> --- a/src/libcamera/egl.cpp
>> +++ b/src/libcamera/egl.cpp
>> @@ -138,6 +138,23 @@ int eGL::attachTextureToFBO(eGLImage &eglImage)
>>       return ret;
>>   }
>> +/**
>> + * \brief Activate a texture unit and bind a texture to that unit
>> + * \param[in,out] eglImage EGL image containing data related to unit 
>> and texture id
>> + *
>> + * When we create a texture we will bind a texture unit and texture 
>> id so
>> + * we can set filters. For the case where a texture already exists 
>> though
>> + * we need to activate and bind an existing texture. This helper 
>> function
>> + * facilitates both cases.
>> + *
>> + */
>> +void eGL::activateBindTexture(eGLImage &eglImage)
>> +{
>> +    // Bind texture unit and texture
>> +    glActiveTexture(eglImage.texture_unit_);
>> +    glBindTexture(GL_TEXTURE_2D, eglImage.texture_);
>> +}
>> +
>>   /**
>>    * \brief Create a DMA-BUF backed 2D texture
>>    * \param[in,out] eglImage EGL image to associate with the DMA-BUF
>> @@ -197,9 +214,7 @@ int eGL::createDMABufTexture2D(eGLImage &eglImage, 
>> int fd, bool output)
>>           return -ENODEV;
>>       }
>> -    // Bind texture unit and texture
>> -    glActiveTexture(eglImage.texture_unit_);
>> -    glBindTexture(GL_TEXTURE_2D, eglImage.texture_);
>> +    activateBindTexture(eglImage);
>>       // Generate texture with filter semantics
>>       glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, image);
>> @@ -270,8 +285,7 @@ void eGL::createTexture2D(eGLImage &eglImage, void 
>> *data)
>>   {
>>       ASSERT(tid_ == Thread::currentId());
>> -    glActiveTexture(eglImage.texture_unit_);
>> -    glBindTexture(GL_TEXTURE_2D, eglImage.texture_);
>> +    activateBindTexture(eglImage);
>>       // Generate texture, bind, associate image to texture, 
>> configure, unbind
>>       glTexImage2D(GL_TEXTURE_2D, 0, eglImage.format_, 
>> eglImage.width_, eglImage.height_, 0, eglImage.format_, 
>> GL_UNSIGNED_BYTE, data);
> This still misses using `activateBindTexture()` in `updateTexture2D()` - 
> which is why I again suggest to move this commit *before* "libcamera: 
> egl: Add updateTexture2D" ;)
> 

its worse than that, we are doing activateBindTexture in debayer_egl.cpp 
and then activate/Bind here again manually.

---
bod
Bryan O'Donoghue July 6, 2026, 11:37 a.m. UTC | #3
On 06/07/2026 12:18, Bryan O'Donoghue wrote:
>>> configure, unbind
>>>        glTexImage2D(GL_TEXTURE_2D, 0, eglImage.format_,
>>> eglImage.width_, eglImage.height_, 0, eglImage.format_,
>>> GL_UNSIGNED_BYTE, data);
>> This still misses using `activateBindTexture()` in `updateTexture2D()` -
>> which is why I again suggest to move this commit*before* "libcamera:
>> egl: Add updateTexture2D" 😉
>>
> its worse than that, we are doing activateBindTexture in debayer_egl.cpp
> and then activate/Bind here again manually.
> 
> ---
> bod

I'm wrong about that, you do need to activate texture on dma import mode 
each time.

---
bod

Patch
diff mbox series

diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h
index 9b4261c55..1e31d490b 100644
--- a/include/libcamera/internal/egl.h
+++ b/include/libcamera/internal/egl.h
@@ -112,6 +112,7 @@  public:
 	void createOutputTexture2D(eGLImage &eglImage);
 
 	int attachTextureToFBO(eGLImage &eglImage);
+	void activateBindTexture(eGLImage &eglImage);
 
 	void pushEnv(std::vector<std::string> &shaderEnv, const char *str);
 	void makeCurrent();
diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp
index b91f9096a..fae907881 100644
--- a/src/libcamera/egl.cpp
+++ b/src/libcamera/egl.cpp
@@ -138,6 +138,23 @@  int eGL::attachTextureToFBO(eGLImage &eglImage)
 	return ret;
 }
 
+/**
+ * \brief Activate a texture unit and bind a texture to that unit
+ * \param[in,out] eglImage EGL image containing data related to unit and texture id
+ *
+ * When we create a texture we will bind a texture unit and texture id so
+ * we can set filters. For the case where a texture already exists though
+ * we need to activate and bind an existing texture. This helper function
+ * facilitates both cases.
+ *
+ */
+void eGL::activateBindTexture(eGLImage &eglImage)
+{
+	// Bind texture unit and texture
+	glActiveTexture(eglImage.texture_unit_);
+	glBindTexture(GL_TEXTURE_2D, eglImage.texture_);
+}
+
 /**
  * \brief Create a DMA-BUF backed 2D texture
  * \param[in,out] eglImage EGL image to associate with the DMA-BUF
@@ -197,9 +214,7 @@  int eGL::createDMABufTexture2D(eGLImage &eglImage, int fd, bool output)
 		return -ENODEV;
 	}
 
-	// Bind texture unit and texture
-	glActiveTexture(eglImage.texture_unit_);
-	glBindTexture(GL_TEXTURE_2D, eglImage.texture_);
+	activateBindTexture(eglImage);
 
 	// Generate texture with filter semantics
 	glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, image);
@@ -270,8 +285,7 @@  void eGL::createTexture2D(eGLImage &eglImage, void *data)
 {
 	ASSERT(tid_ == Thread::currentId());
 
-	glActiveTexture(eglImage.texture_unit_);
-	glBindTexture(GL_TEXTURE_2D, eglImage.texture_);
+	activateBindTexture(eglImage);
 
 	// Generate texture, bind, associate image to texture, configure, unbind
 	glTexImage2D(GL_TEXTURE_2D, 0, eglImage.format_, eglImage.width_, eglImage.height_, 0, eglImage.format_, GL_UNSIGNED_BYTE, data);