[libcamera-devel] [PATCH 00/11] libcamera: introduce Software ISP and Software IPA

[Hans de Goede]

Hi,

On 12/14/23 17:02, Hans de Goede wrote:
> Hi Bryan,
> 
> On 12/14/23 16:16, Bryan O'Donoghue wrote:
>> On 14/12/2023 14:47, Hans de Goede wrote:
> 
> <snip>
> 
>>> BTW I'm currently working on adding support for 10bpp unpacked
>>> bayer and while working on this I noticed a small bug in the swstats
>>> code, you may want to squash in this fix:
>>
>> Applied that change.
>>
>> I'm comparing to your earlier branch
>>
>> commit ae92fa44991ded151c63d5d202efdacc9d640aff (HEAD -> SoftwareISP-v01-hans1, softisp/SoftwareISP-v01-hans1)
>> Author: Hans de Goede <hdegoede at redhat.com>
>> Date:   Thu Nov 30 20:13:29 2023 +0100
>>
>>
>> On the earlier branch I get 30fps @ 68% CPU usage. On this branch I'm getting 18fps @ 100%.
>> Seems to be eating alot more cycles
> 
> Ok, that is no good.
> 
> Can you try:
> 
> https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v02-hans3
> 
> And then start with checking out:
> 
> 64c9bc0f55f459b9722fe78dfd35a07fb35d2a7b ("libcamera: software_isp: Add debayer-line callbacks for bayer patterns repeating every 4 lines")
> 
> that is basically softisp/SoftwareISP-v01-hans1 rebased on top of SoftwareISP-v02,
> so that should give you to same performance as before.
> 
> First please check this indeed restores performance ?
> 
> And then after that try newer commits from that branch in this order
> (I'm skipping commits which should not have a performance impact here):
> 
> 7136d4d59aafb2564a24a2d4be773ca220257fdc
> e3c2a5931dd825c58f626da8c12429b70d20219b (not really expecting a performance impact from this one but maybe)
> b3aa4e4f781e881746953177dacbce8c943cb5a3 (bugfix but one which is expected to have some performance impact)
> ec75339a5bff8b8e9a0031b9408bd47020905a4f
> f62ed5df54850a24406bf3a762271a5fa3c0303d
> 
> The reason why I'm asking you to test these instead of do a bisect
> is since there might be a number of smaller performance regressions
> all adding up ...
> 
> If you can let me know the results of this then I'll see if I can
> restore the old performance for you.
> 
> Note that as mentioned above b3aa4e4f781e is a bug fix which
> some potentially big performance implications but the old code
> really was wrong there ...
> 
>> and producing a more pinkish result on my hw.
> 
> As part of my work I more or less rewrote white-balancing, but I thought I fixed
> the pinkish thing. It seems the new white-balancing is somewhat sensitive to
> over-exposure and your image does look a bit overexposed.

p.s.

It would also be good to compare:

https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v02-hans2
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v02-hans3

(just the last commit of each)

hans2 -> hans3 is mostly prep work for merging things with SoftwareISP-v03,
but one significant change in hans3 is that I added Pavel's code changes
to avoid macro-use (in swstats) resp reduce code duplication (in debayer).

Pavel's changes rely on the compiler being smart enough to inline *all*
the helpers and then replace the ctxt struct with using registers for each
of the struct members. Maybe that is not working for you and we need
to undo Pavel's changes.

Regards,

Hans