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

Balamanikandan.Gunasundar
2025-10-15T06:33:33Z

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