Re: [PATCH RFC 2/7] media: qcom: iris: gen2: add support for 10bit decoding

[Neil Armstrong <neil.armstrong@linaro.org>]

On 4/21/26 07:24, Dikshita Agarwal wrote:
> 
> 
> On 4/17/2026 1:33 PM, Neil Armstrong wrote:
>> On 4/17/26 09:22, Dikshita Agarwal wrote:
>>>
>>>
>>> On 4/8/2026 10:13 PM, Neil Armstrong wrote:
>>>> Add the necessary plumbing into the HFi Gen2 to signal the decoder
>>>> the right 10bit pixel format and stride when in compressed mode.
>>>>
>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>>> ---
>>>>    .../platform/qcom/iris/iris_hfi_gen2_command.c     | 71
>>>> +++++++++++++++++++++-
>>>>    .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
>>>>    drivers/media/platform/qcom/iris/iris_utils.c      |  4 +-
>>>>    3 files changed, 72 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>>>> b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>>>> index 30bfd90d423b..8e547e390fa3 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>>>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
>>>> @@ -481,8 +481,20 @@ static int iris_hfi_gen2_set_colorformat(struct
>>>> iris_inst *inst, u32 plane)
>>>>          if (inst->domain == DECODER) {
>>>>            pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
>>>> -        hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
>>>> -            HFI_COLOR_FMT_NV12 : HFI_COLOR_FMT_NV12_UBWC;
>>>> +        switch (pixelformat) {
>>>> +        case V4L2_PIX_FMT_NV12:
>>>> +            hfi_colorformat = HFI_COLOR_FMT_NV12;
>>>> +            break;
>>>> +        case V4L2_PIX_FMT_QC08C:
>>>> +            hfi_colorformat = HFI_COLOR_FMT_NV12_UBWC;
>>>> +            break;
>>>> +        case V4L2_PIX_FMT_P010:
>>>> +            hfi_colorformat = HFI_COLOR_FMT_P010;
>>>> +            break;
>>>> +        case V4L2_PIX_FMT_QC10C:
>>>> +            hfi_colorformat = HFI_COLOR_FMT_TP10_UBWC;
>>>> +            break;
>>>> +        };
>>>>        } else {
>>>>            pixelformat = inst->fmt_src->fmt.pix_mp.pixelformat;
>>>>            hfi_colorformat = pixelformat == V4L2_PIX_FMT_NV12 ?
>>>> @@ -517,7 +529,8 @@ static int
>>>> iris_hfi_gen2_set_linear_stride_scanline(struct iris_inst *inst, u32
>>>>        stride_uv = stride_y;
>>>>        scanline_uv = scanline_y / 2;
>>>>    -    if (pixelformat != V4L2_PIX_FMT_NV12)
>>>> +    if (pixelformat != V4L2_PIX_FMT_NV12 &&
>>>> +        pixelformat != V4L2_PIX_FMT_P010)
>>>>            return 0;
>>>>          payload[0] = stride_y << 16 | scanline_y;
>>>> @@ -532,6 +545,57 @@ static int
>>>> iris_hfi_gen2_set_linear_stride_scanline(struct iris_inst *inst, u32
>>>>                              sizeof(u64));
>>>>    }
>>>>    +static int iris_hfi_gen2_set_ubwc_stride_scanline(struct iris_inst
>>>> *inst, u32 plane)
>>>> +{
>>>> +    u32 meta_stride_y, meta_scanline_y, meta_stride_uv, meta_scanline_uv;
>>>> +    u32 stride_y, scanline_y, stride_uv, scanline_uv;
>>>> +    u32 port = iris_hfi_gen2_get_port(inst, plane);
>>>> +    u32 pixelformat, width, height;
>>>> +    u32 payload[4];
>>>> +
>>>> +    pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
>>>> +    width = inst->fmt_dst->fmt.pix_mp.width;
>>>> +    height = inst->fmt_dst->fmt.pix_mp.height;
>>>
>>> This HFI is only applicable to AV1, you might see some corruption due to
>>> this. Please check.
>>
>> This is what I saw looking at donwstream, but I had not implemented this
>> initially
>> but I got some corruption with some different width/height, which was
>> solved adding
>> this command.
> 
> Even with your implementation, this API would only ever be triggered for
> QC08C and QC10C formats, which I don’t believe are currently being tested
> with your series. Since HFI defines this as AV1D‑specific, invoking it for
> other codecs is unlikely to help and would be incorrect.
> 
> Pls re-check.

I actually did test QC08C and QC10C with dma-buf drm import, and it works fine
with this API.

Neil

> 
> Thanks,
> Dikshita
>> I guess this in-firmware calculations are not exactly the same the DRM
>> driver expects.
>>
>> Honestly I think it's preferable to sync the stride/scanlines calculations
>> between the
>> driver and the firmware.
>> For example I used the same "ALIGN(width, 192)" for the v2 iris patchset
>> which was wrong
>> because ALIGN only works on Power Of Two number but it still worked because
>> this HFI
>> HFI_PROP_UBWC_STRIDE_SCANLINE command was called with the wrong aligned width.
>>
>> Neil
>>
>>>
>>> Thanks,
>>> Dikshita
>>