[v1,2/3] egl: Call glFlush before glFinish
diff mbox series

Message ID 20260521141006.101016-3-robert.mader@collabora.com
State Superseded
Headers show
Series
  • debayer_egl: Sync output buffers after processing stats
Related show

Commit Message

Robert Mader May 21, 2026, 2:10 p.m. UTC 
Apparently it is not guaranteed that drivers will do so implicitly,
which may or may not be a driver bug. The impact on performance has been
observed to be significant and it's easy enough, so let's do it
explicitly.

Below are some benchmark results. All where done using postmarketOS edge
with updates from 21th May 2026 (Mesa 26.1.1). The mentioned pipelines
where run five times each, with the mean value included here, which should
be quite representive as the variance was rather small. All devices
where using the powersave governor.

cam -c /base/soc@0/cci@ac4a000/i2c-bus@0/camera@1a -s width=1920,height=1080 --capture=60
Before: 47986 us/frame
After: 33596 us/frame !

cam -c /base/soc@0/cci@ac4a000/i2c-bus@1/camera@1a -s width=1920,height=1080 --capture=60
Before: 30294 us/frame
After: 14922 us/frame !

cam -c /base/soc@0/cci@ac4b000/i2c-bus@1/camera@10 -s width=1920,height=1080 --capture=60
Before: 27373 us/frame
After: 26106 us/frame

cam -c /base/soc@0/cci@ac4a000/i2c-bus@1/camera@29 -s width=1920,height=1080 --capture=60
Before: 16339 us/frame
After: 15897 us/frame

cam -c /base/soc@0/cci@ac4a000/i2c-bus@1/camera@10 -s width=1920,height=1080 --capture=60
Before: 26206 us/frame
After: 25721 us/frame

cam -c /base/soc@0/cci@ac4a000/i2c-bus@0/camera@10 -s width=1920,height=1080 --capture=60
Before: 43723 us/frame
After: 34124 us/frame !

cam -c /base/soc@0/cci@ac4a000/i2c-bus@0/camera@1a -s width=1920,height=1080 --capture=60
Before: 23597 us/frame
After: 23707 us/frame

cam -c /base/soc@0/bus@30800000/i2c@30a40000/camera@20 -s width=1280,height=720 --capture=60
Before: 91200 us/frame
After: 91649 us/frame

cam -c /base/soc@0/bus@30800000/i2c@30a50000/camera@2d -s width=1280,height=720 --capture=60
Before: 76577 us/frame
After: 76956 us/frame

cam -c /base/i2c-csi/front-camera@3c -s width=1280,height=720 --capture=60
Before: 188122 us/frame
After: 188500 us/frame

cam -c /base/i2c-csi/rear-camera@4c -s width=1280,height=720 --capture=60
Before: 193712 us/frame
After: 190222 us/frame

Signed-off-by: Robert Mader <robert.mader@collabora.com>
---
 src/libcamera/egl.cpp | 1 +
 1 file changed, 1 insertion(+)

Comments

Robert Mader May 21, 2026, 4:36 p.m. UTC | #1 
Opened a Mesa issue about this: 
https://gitlab.freedesktop.org/mesa/mesa/-/work_items/15516

On 21.05.26 16:10, Robert Mader wrote:
> Apparently it is not guaranteed that drivers will do so implicitly,
> which may or may not be a driver bug. The impact on performance has been
> observed to be significant and it's easy enough, so let's do it
> explicitly.
>
> Below are some benchmark results. All where done using postmarketOS edge
> with updates from 21th May 2026 (Mesa 26.1.1). The mentioned pipelines
> where run five times each, with the mean value included here, which should
> be quite representive as the variance was rather small. All devices
> where using the powersave governor.
>
> cam -c /base/soc@0/cci@ac4a000/i2c-bus@0/camera@1a -s width=1920,height=1080 --capture=60
> Before: 47986 us/frame
> After: 33596 us/frame !
>
> cam -c /base/soc@0/cci@ac4a000/i2c-bus@1/camera@1a -s width=1920,height=1080 --capture=60
> Before: 30294 us/frame
> After: 14922 us/frame !
>
> cam -c /base/soc@0/cci@ac4b000/i2c-bus@1/camera@10 -s width=1920,height=1080 --capture=60
> Before: 27373 us/frame
> After: 26106 us/frame
>
> cam -c /base/soc@0/cci@ac4a000/i2c-bus@1/camera@29 -s width=1920,height=1080 --capture=60
> Before: 16339 us/frame
> After: 15897 us/frame
>
> cam -c /base/soc@0/cci@ac4a000/i2c-bus@1/camera@10 -s width=1920,height=1080 --capture=60
> Before: 26206 us/frame
> After: 25721 us/frame
>
> cam -c /base/soc@0/cci@ac4a000/i2c-bus@0/camera@10 -s width=1920,height=1080 --capture=60
> Before: 43723 us/frame
> After: 34124 us/frame !
>
> cam -c /base/soc@0/cci@ac4a000/i2c-bus@0/camera@1a -s width=1920,height=1080 --capture=60
> Before: 23597 us/frame
> After: 23707 us/frame
>
> cam -c /base/soc@0/bus@30800000/i2c@30a40000/camera@20 -s width=1280,height=720 --capture=60
> Before: 91200 us/frame
> After: 91649 us/frame
>
> cam -c /base/soc@0/bus@30800000/i2c@30a50000/camera@2d -s width=1280,height=720 --capture=60
> Before: 76577 us/frame
> After: 76956 us/frame
>
> cam -c /base/i2c-csi/front-camera@3c -s width=1280,height=720 --capture=60
> Before: 188122 us/frame
> After: 188500 us/frame
>
> cam -c /base/i2c-csi/rear-camera@4c -s width=1280,height=720 --capture=60
> Before: 193712 us/frame
> After: 190222 us/frame
>
> Signed-off-by: Robert Mader <robert.mader@collabora.com>
> ---
>   src/libcamera/egl.cpp | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp
> index 357918711..d9bc4f555 100644
> --- a/src/libcamera/egl.cpp
> +++ b/src/libcamera/egl.cpp
> @@ -94,6 +94,7 @@ void eGL::syncOutput()
>   {
>   	ASSERT(tid_ == Thread::currentId());
>   
> +	glFlush();
>   	glFinish();
>   }
>
Robert Mader May 25, 2026, 11:44 a.m. UTC | #2 
It turned out that this patch just works around a driver bug (apparently 
scheduling related) and that running with `GALLIUM_THREAD=0` - i.e. 
non-threaded GL driver - has the same positive impact. Thus I'll drop 
the patch from v2 (the following patch reverts it anyway) and re-run the 
benchmarks with `GALLIUM_THREAD=0`.

With that clarified I think this series should be ready to go and mostly 
a non-brainer for merging. Thus quick question: does anyone reading this 
have more remarks / suggestions? Maybe Bryan or Barnabás?

On 21.05.26 18:36, Robert Mader wrote:
> Opened a Mesa issue about this: 
> https://gitlab.freedesktop.org/mesa/mesa/-/work_items/15516
>
> On 21.05.26 16:10, Robert Mader wrote:
>> Apparently it is not guaranteed that drivers will do so implicitly,
>> which may or may not be a driver bug. The impact on performance has been
>> observed to be significant and it's easy enough, so let's do it
>> explicitly.
>>
>> Below are some benchmark results. All where done using postmarketOS edge
>> with updates from 21th May 2026 (Mesa 26.1.1). The mentioned pipelines
>> where run five times each, with the mean value included here, which 
>> should
>> be quite representive as the variance was rather small. All devices
>> where using the powersave governor.
>>
>> cam -c /base/soc@0/cci@ac4a000/i2c-bus@0/camera@1a -s 
>> width=1920,height=1080 --capture=60
>> Before: 47986 us/frame
>> After: 33596 us/frame !
>>
>> cam -c /base/soc@0/cci@ac4a000/i2c-bus@1/camera@1a -s 
>> width=1920,height=1080 --capture=60
>> Before: 30294 us/frame
>> After: 14922 us/frame !
>>
>> cam -c /base/soc@0/cci@ac4b000/i2c-bus@1/camera@10 -s 
>> width=1920,height=1080 --capture=60
>> Before: 27373 us/frame
>> After: 26106 us/frame
>>
>> cam -c /base/soc@0/cci@ac4a000/i2c-bus@1/camera@29 -s 
>> width=1920,height=1080 --capture=60
>> Before: 16339 us/frame
>> After: 15897 us/frame
>>
>> cam -c /base/soc@0/cci@ac4a000/i2c-bus@1/camera@10 -s 
>> width=1920,height=1080 --capture=60
>> Before: 26206 us/frame
>> After: 25721 us/frame
>>
>> cam -c /base/soc@0/cci@ac4a000/i2c-bus@0/camera@10 -s 
>> width=1920,height=1080 --capture=60
>> Before: 43723 us/frame
>> After: 34124 us/frame !
>>
>> cam -c /base/soc@0/cci@ac4a000/i2c-bus@0/camera@1a -s 
>> width=1920,height=1080 --capture=60
>> Before: 23597 us/frame
>> After: 23707 us/frame
>>
>> cam -c /base/soc@0/bus@30800000/i2c@30a40000/camera@20 -s 
>> width=1280,height=720 --capture=60
>> Before: 91200 us/frame
>> After: 91649 us/frame
>>
>> cam -c /base/soc@0/bus@30800000/i2c@30a50000/camera@2d -s 
>> width=1280,height=720 --capture=60
>> Before: 76577 us/frame
>> After: 76956 us/frame
>>
>> cam -c /base/i2c-csi/front-camera@3c -s width=1280,height=720 
>> --capture=60
>> Before: 188122 us/frame
>> After: 188500 us/frame
>>
>> cam -c /base/i2c-csi/rear-camera@4c -s width=1280,height=720 
>> --capture=60
>> Before: 193712 us/frame
>> After: 190222 us/frame
>>
>> Signed-off-by: Robert Mader <robert.mader@collabora.com>
>> ---
>>   src/libcamera/egl.cpp | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp
>> index 357918711..d9bc4f555 100644
>> --- a/src/libcamera/egl.cpp
>> +++ b/src/libcamera/egl.cpp
>> @@ -94,6 +94,7 @@ void eGL::syncOutput()
>>   {
>>       ASSERT(tid_ == Thread::currentId());
>>   +    glFlush();
>>       glFinish();
>>   }
>

Patch
diff mbox series 

diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp
index 357918711..d9bc4f555 100644
--- a/src/libcamera/egl.cpp
+++ b/src/libcamera/egl.cpp
@@ -94,6 +94,7 @@  void eGL::syncOutput()
 {
 	ASSERT(tid_ == Thread::currentId());
 
+	glFlush();
 	glFinish();
 }