Re: [PATCH 00/18] media: microchip-isc: Color correction and histogram stats

[<Balamanikandan.Gunasundar@microchip.com>]

Hi Kieran,

On 10/10/25 1:51 pm, Kieran Bingham wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Quoting Eugen Hristev (2025-10-09 21:11:17)
>> Hi Bala,
>>
>> On 10/9/25 18:52, Balamanikandan Gunasundar wrote:
>>> Hi,
>>>
>>> This patch series has a set of enhancements to the Microchip Image Sensor
>>> Controller driver. The objective is to expand its image processing
>>> capabilities and to improve the colors.
>>>
>>> This series also introduces a new stats driver that exposes the histogram
>>> data to userspace via v4l2 controls. This allows applications such as


My apologies. I made a mistake clarifying this earlier. The new stats 
driver does not use V4L2 controls, it exports the buffer through the 
v4l2 meta data| (|V4L2_BUF_TYPE_META_CAPTURE)||instead. So I guess this 
is the approach you are suggesting.

The controls like brightness, contrast, hue and saturation are exported 
as V4L2 controls.

root@sam9x75-curiosity-sd:~# v4l2-ctl -l

User Controls

                      brightness 0x00980900 (int)    : min=-1024 
max=1023 step=1 default=0 value=0 flags=slider
                        contrast 0x00980901 (int)    : min=-2048 
max=2047 step=1 default=16 value=16 flags=slider
                      saturation 0x00980902 (int)    : min=0 max=100 
step=1 default=16 value=16 flags=slider
                             hue 0x00980903 (int)    : min=-180 max=180 
step=1 default=0 value=0 flags=slider

<snip>


Thanks,

Bala.


>>> libcamera to access real time image statistics for advanced image
>>> processing like automatic exposure, white balance adjustments etc.
> How much data do you anticipate to be passing through controls? (What
> can the hardware provide in total if we look at the bigger picture to
> support the full device?)
>
> For all other ISPs we've been working towards using structured parameter
> buffers to pass data - and we've been making that format extensible,
> which I think could also be a design that can apply to statistics.
>
> This would greatly reduce the overhead of managing 'one control per
> value' ... or things like passing large tables (like a lens shading
> table perhaps) through controls.
>
> --
> Kieran
>
>>> Balakrishnan Sambath (11):
>>>    media: microchip-isc: Enable GDC and CBC module flags for RGB formats
>>>    media: microchip-isc: Improve histogram calculation with outlier
>>>      rejection
>>>    media: microchip-isc: Use channel averages for Grey World AWB
>>>    media: microchip-isc: Add range based black level correction
>>>    media: platform: microchip: Extend gamma table and control range
>>>    media: platform: microchip: Add new histogram submodule
>>>    media: microchip-isc: Register and unregister statistics device
>>>    media: microchip-isc: Always enable histogram for all RAW formats
>>>    media: microchip-isc: fix histogram state initialization order
>>>    media: microchip-isc: decouple histogram cycling from AWB mode
>>>    media: microchip-isc: enable userspace histogram statistics export
>>>
>>> Balamanikandan Gunasundar (7):
>>>    media: platform: microchip: set maximum resolution for sam9x7
>>>    media: platform: microchip: Include DPC modules flags in pipeline
>>>    media: microchip-isc: expose hue and saturation as v4l2 controls
>>>    media: microchip-isc: Rename CBC to CBHS
>>>    media: microchip-isc: Store histogram data of all channels
>>>    media: videodev2.h, v4l2-ioctl: Add microchip statistics format
>>>    media: microchip-isc: expose color correction registers as v4l2
>>>      controls
>>>
>>>   drivers/media/platform/microchip/Kconfig      |   2 +
>>>   drivers/media/platform/microchip/Makefile     |   2 +-
>>>   .../platform/microchip/microchip-isc-base.c   | 373 ++++++++++--
>>>   .../platform/microchip/microchip-isc-regs.h   |   3 +
>>>   .../platform/microchip/microchip-isc-stats.c  | 549 ++++++++++++++++++
>>>   .../media/platform/microchip/microchip-isc.h  |  44 +-
>>>   .../microchip/microchip-sama5d2-isc.c         |   2 +-
>>>   .../microchip/microchip-sama7g5-isc.c         |  73 ++-
>>>   drivers/media/v4l2-core/v4l2-ioctl.c          |   1 +
>>>   include/linux/atmel-isc-media.h               |  13 +
>>>   include/uapi/linux/videodev2.h                |   3 +
>>>   11 files changed, 1001 insertions(+), 64 deletions(-)
>>>   create mode 100644 drivers/media/platform/microchip/microchip-isc-stats.c
>>>
>> This looks interesting.
>> I would like to see the compliance tool output for more platforms
>> (sama7g5, sama5d2, and the new sam9x7), also the media-ctl -p , to see
>> the topology with your new driver.
>>
>> Thanks,
>> Eugen