Re: [PATCH v3 3/6] media: qcom: iris: gen2: add support for 10bit decoding

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

On 5/13/26 20:50, Vikash Garodia wrote:
> 
> On 5/11/2026 2:50 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     | 75 +++++++++++++++++++++-
>>   .../platform/qcom/iris/iris_hfi_gen2_defines.h     |  1 +
>>   drivers/media/platform/qcom/iris/iris_utils.c      |  4 +-
>>   3 files changed, 76 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..89de8c366836 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;
>> +        };
> 
> semicolon ?

Ack

> 
>>       } 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,61 @@ 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];
>> +
>> +    if (inst->domain != DECODER ||
>> +        inst->fmt_src->fmt.pix_mp.pixelformat != V4L2_PIX_FMT_AV1)
> 
> whats the restriction with AV1 here ?

This restriction is used in downstream driver and Dikshita reported we shouldn't
call HFI_PROP_UBWC_STRIDE_SCANLINE except for AV1.

Could you please check if it's necessary ?

> 
>> +        return 0;
>> +
>> +    pixelformat = inst->fmt_dst->fmt.pix_mp.pixelformat;
>> +    width = inst->fmt_dst->fmt.pix_mp.width;
>> +    height = inst->fmt_dst->fmt.pix_mp.height;
>> +
>> +    switch (pixelformat) {
>> +    case V4L2_PIX_FMT_QC08C:
>> +        stride_y = ALIGN(width, 128);
>> +        scanline_y = ALIGN(height, 32);
>> +        stride_uv = ALIGN(width, 128);
>> +        scanline_uv = ALIGN((height + 1) >> 1, 32);
>> +        meta_stride_y = ALIGN(DIV_ROUND_UP(width, 32), 64);
>> +        meta_scanline_y = ALIGN(DIV_ROUND_UP(height, 8), 16);
>> +        meta_stride_uv = ALIGN(DIV_ROUND_UP((width + 1) >> 1, 16), 64);
>> +        meta_scanline_uv = ALIGN(DIV_ROUND_UP((height + 1) >> 1, 8), 16);
>> +        break;
>> +    case V4L2_PIX_FMT_QC10C:
>> +        stride_y = ALIGN(width * 4 / 3, 256);
>> +        scanline_y = ALIGN(height, 16);
>> +        stride_uv = ALIGN(width * 4 / 3, 256);
>> +        scanline_uv = ALIGN((height + 1) >> 1, 16);
>> +        meta_stride_y = ALIGN(DIV_ROUND_UP(width, 48), 64);
>> +        meta_scanline_y = ALIGN(DIV_ROUND_UP(height, 4), 16);
>> +        meta_stride_uv = ALIGN(DIV_ROUND_UP((width + 1) >> 1, 24), 64);
>> +        meta_scanline_uv = ALIGN(DIV_ROUND_UP((height + 1) >> 1, 4), 16);
>> +        break;
>> +    default:
>> +        return 0;
>> +    }
>> +
>> +    payload[0] = stride_y << 16 | scanline_y;
>> +    payload[1] = stride_uv << 16 | scanline_uv;
>> +    payload[2] = meta_stride_y << 16 | meta_scanline_y;
>> +    payload[3] = meta_stride_uv << 16 | meta_scanline_uv;
>> +
>> +    return iris_hfi_gen2_session_set_property(inst,
>> +                          HFI_PROP_UBWC_STRIDE_SCANLINE,
>> +                          HFI_HOST_FLAGS_NONE,
>> +                          port,
>> +                          HFI_PAYLOAD_U32_ARRAY,
>> +                          &payload[0],
>> +                          sizeof(u32) * 4);
>> +}
>> +
>>   static int iris_hfi_gen2_set_tier(struct iris_inst *inst, u32 plane)
>>   {
>>       u32 port = iris_hfi_gen2_get_port(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
>> @@ -620,6 +688,7 @@ static int iris_hfi_gen2_session_set_config_params(struct iris_inst *inst, u32 p
>>           {HFI_PROP_OPB_ENABLE,                 iris_hfi_gen2_set_opb_enable             },
>>           {HFI_PROP_COLOR_FORMAT,               iris_hfi_gen2_set_colorformat            },
>>           {HFI_PROP_LINEAR_STRIDE_SCANLINE,     iris_hfi_gen2_set_linear_stride_scanline },
>> +        {HFI_PROP_UBWC_STRIDE_SCANLINE,       iris_hfi_gen2_set_ubwc_stride_scanline },
> 
> alignment
> 
>>           {HFI_PROP_TIER,                       iris_hfi_gen2_set_tier                   },
>>           {HFI_PROP_FRAME_RATE,                 iris_hfi_gen2_set_frame_rate             },
>>           {HFI_PROP_AV1_FILM_GRAIN_PRESENT,     iris_hfi_gen2_set_film_grain             },
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> index cecf771c55dd..68f849232906 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h
>> @@ -118,6 +118,7 @@ enum hfi_flip {
>>   #define HFI_PROP_OPB_ENABLE            0x03000184
>>   #define HFI_PROP_AV1_TILE_ROWS_COLUMNS        0x03000187
>>   #define HFI_PROP_AV1_DRAP_CONFIG        0x03000189
>> +#define HFI_PROP_UBWC_STRIDE_SCANLINE        0x03000190
>>   #define HFI_PROP_COMV_BUFFER_COUNT        0x03000193
>>   #define HFI_PROP_AV1_UNIFORM_TILE_SPACING    0x03000197
>>   #define HFI_PROP_END                0x03FFFFFF
>> diff --git a/drivers/media/platform/qcom/iris/iris_utils.c b/drivers/media/platform/qcom/iris/iris_utils.c
>> index bdedd6bfa87a..c75dcb8e671e 100644
>> --- a/drivers/media/platform/qcom/iris/iris_utils.c
>> +++ b/drivers/media/platform/qcom/iris/iris_utils.c
>> @@ -35,7 +35,9 @@ int iris_get_mbpf(struct iris_inst *inst)
>>   bool iris_split_mode_enabled(struct iris_inst *inst)
>>   {
>>       return inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_NV12 ||
>> -        inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C;
>> +        inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC08C ||
>> +        inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_P010 ||
>> +        inst->fmt_dst->fmt.pix_mp.pixelformat == V4L2_PIX_FMT_QC10C;
>>   }
>>   bool iris_fmt_is_8bit(__u32 pixelformat)
>>
> 

Thanks,
Neil