Re: [PATCH v5 1/5] media: qcom: camss: Add V4L2 meta format for CAMSS ISP parameters

[Gjorgji Rosikopulos (Consultant)]

On 7/30/2026 3:34 PM, Loic Poulain wrote:
> On Thu, Jul 30, 2026 at 1:57 PM Bryan O'Donoghue <bod@xxxxxxxxxx> wrote:
>>
>> On 30/07/2026 12:44, Loic Poulain wrote:
>>> On Wed, Jul 29, 2026 at 8:28 PM Gjorgji Rosikopulos (Consultant)
>>> <gjorgji.rosikopulos@xxxxxxxxxxxxxxxx> wrote:
>>>>
>>>> Hi Loic,
>>>>
>>>> On 7/24/2026 3:42 PM, Loic Poulain wrote:
>>>>> Add a V4L2 meta format code (V4L2_META_FMT_QCOM_ISP_PARAMS) for the
>>>>> Qualcomm CAMSS ISP parameter buffer. This format is used by the params
>>>>> video node exposed by CAMSS offline ISP drivers (e.g. OPE) to carry
>>>>> ISP tuning data such as white balance, color correction and chroma
>>>>> enhancement settings.
>>>>>
>>>>> Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>
>>>>> ---
>>>>>   drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
>>>>>   include/uapi/linux/videodev2.h       | 3 +++
>>>>>   2 files changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>>> index 98512ea4cc5b9d725e1851af2ed38df85bb4fa8c..7b6e9a9a514f037190d55d59409dd6cc97522943 100644
>>>>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>>>>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>>>>> @@ -1471,6 +1471,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>>>>>        case V4L2_META_FMT_C3ISP_STATS:         descr = "Amlogic C3 ISP Statistics"; break;
>>>>>        case V4L2_META_FMT_MALI_C55_PARAMS:     descr = "ARM Mali-C55 ISP Parameters"; break;
>>>>>        case V4L2_META_FMT_MALI_C55_STATS:      descr = "ARM Mali-C55 ISP 3A Statistics"; break;
>>>>> +     case V4L2_META_FMT_QCOM_ISP_PARAMS:     descr = "Qualcomm CAMSS ISP Parameters"; break;
>>>>>        case V4L2_PIX_FMT_NV12_8L128:   descr = "NV12 (8x128 Linear)"; break;
>>>>>        case V4L2_PIX_FMT_NV12M_8L128:  descr = "NV12M (8x128 Linear)"; break;
>>>>>        case V4L2_PIX_FMT_NV12_10BE_8L128:      descr = "10-bit NV12 (8x128 Linear, BE)"; break;
>>>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>>>> index add08188f06890182a5c399a223c1ab0a546cae1..011026194a20cde22a374e6fb2570cbc6bced230 100644
>>>>> --- a/include/uapi/linux/videodev2.h
>>>>> +++ b/include/uapi/linux/videodev2.h
>>>>> @@ -888,6 +888,9 @@ struct v4l2_pix_format {
>>>>>   #define V4L2_META_FMT_MALI_C55_PARAMS        v4l2_fourcc('C', '5', '5', 'P') /* ARM Mali-C55 Parameters */
>>>>>   #define V4L2_META_FMT_MALI_C55_STATS v4l2_fourcc('C', '5', '5', 'S') /* ARM Mali-C55 3A Statistics */
>>>>>
>>>>> +/* Vendor specific - used for Qualcomm CAMSS offline ISP */
>>>>
>>>> It seems that this is generic format for ISP parameters, can we update the comment and remove offline. The proposal:
>>>> /* Vendor specific - used for Qualcomm CAMSS ISP */
>>>
>>> Sure.
>>> Loic
>>
>> Since these are not the full list and as I understand them not the full
>> structures for OPE - qcom restricting the full formats ? - I'd request
>> to move these to an OPE specific header.
> 
> Yes, I think the params struct can be OPE-specific and defined in an
> OPE-specific header. However, could we keep the enum in a generic
> header so that we only have a single enum definition? We could then
> extend it later with OPE-specific or non-OPE-specific parameters as
> needed.

The format V4L2_META_FMT_QCOM_ISP_PARAMS will be common (based on its name) across all ISP versions.
This will make it confusing for userspace to know which header to use for a given ISP version.
It would be better to have at least some protection in place to reject param types that aren't supported by a specific version.
I think the simpler approach is to keep these as generic Qcom-specific params for now,
and only split them out if we actually end up with the same blocks using different parameters across versions.
But for sure that can be done latter when new params and ISP's are introduced. It was just an proposal.

~Gjorgji


> 
> Regards,
> Loic