Re: [RFC PATCH 04/12] media: iris: Add internal buffer calculation for HEVC and VP9 decoders

[Dikshita Agarwal <quic_dikshita@quicinc.com>]

On 3/6/2025 6:35 AM, Bryan O'Donoghue wrote:
> On 05/03/2025 10:43, Dikshita Agarwal wrote:
>> Add internal buffer count and size calculations for HEVC and VP9
>> decoders.
>>
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
>> ---
>>   .../media/platform/qcom/iris/iris_buffer.c    |   3 +
>>   .../platform/qcom/iris/iris_vpu_buffer.c      | 397 +++++++++++++++++-
>>   .../platform/qcom/iris/iris_vpu_buffer.h      |  46 +-
>>   3 files changed, 432 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c
>> b/drivers/media/platform/qcom/iris/iris_buffer.c
>> index e5c5a564fcb8..8c9d5b7fe75c 100644
>> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
>> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
>> @@ -205,6 +205,9 @@ static u32 iris_bitstream_buffer_size(struct
>> iris_inst *inst)
>>       if (num_mbs > NUM_MBS_4K) {
>>           div_factor = 4;
>>           base_res_mbs = caps->max_mbpf;
>> +    } else {
>> +        if (inst->codec == V4L2_PIX_FMT_VP9)
>> +            div_factor = 1;
>>       }
>>         /*
>> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
>> b/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
>> index dce25e410d80..13ee93356bcb 100644
>> --- a/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
>> +++ b/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
>> @@ -31,6 +31,42 @@ static u32 hfi_buffer_bin_h264d(u32 frame_width, u32
>> frame_height, u32 num_vpp_p
>>       return size_h264d_hw_bin_buffer(n_aligned_w, n_aligned_h,
>> num_vpp_pipes);
>>   }
>>   +static u32 size_h265d_hw_bin_buffer(u32 frame_width, u32 frame_height,
>> u32 num_vpp_pipes)
>> +{
>> +    u32 product = frame_width * frame_height;
>> +    u32 size_yuv, size_bin_hdr, size_bin_res;
>> +
>> +    size_yuv = (product <= BIN_BUFFER_THRESHOLD) ?
>> +        ((BIN_BUFFER_THRESHOLD * 3) >> 1) : ((product * 3) >> 1);
> 
> When I read this code I have no way of knowing if it makes sense.
> 
> #define BIN_BUFFER_THRESHOLD        (1280 * 736)
> 
> ((BIN_BUFFER_THRESHOLD * 3) >> 1)
> 
> How/why is that correct ?
> 
Bin buffers are intermediate buffers which are used by different sub
hardware blocks within video IP. The calculation of these buffers are
hardware mandated. While we can't explain/justify every factor, these are
based on hardware constraints and validated with firmware requirements,
Software is just coding it up the way hardware specification defines it.
>> +    size_bin_hdr = size_yuv * H265_CABAC_HDR_RATIO_HD_TOT;
>> +    size_bin_res = size_yuv * H265_CABAC_RES_RATIO_HD_TOT;
>> +    size_bin_hdr = ALIGN(size_bin_hdr / num_vpp_pipes, DMA_ALIGNMENT) *
>> num_vpp_pipes;
>> +    size_bin_res = ALIGN(size_bin_res / num_vpp_pipes, DMA_ALIGNMENT) *
>> num_vpp_pipes;
>> +
>> +    return size_bin_hdr + size_bin_res;
>> +}
>> +
>> +static u32 hfi_buffer_bin_vp9d(u32 frame_width, u32 frame_height, u32
>> num_vpp_pipes)
>> +{
>> +    u32 _size_yuv = ALIGN(frame_width, 16) * ALIGN(frame_height, 16) * 3
>> / 2;
>> +    u32 _size = ALIGN(((max_t(u32, _size_yuv, ((BIN_BUFFER_THRESHOLD *
>> 3) >> 1)) *
>> +            VPX_DECODER_FRAME_BIN_HDR_BUDGET /
>> VPX_DECODER_FRAME_BIN_DENOMINATOR *
>> +            VPX_DECODER_FRAME_CONCURENCY_LVL) / num_vpp_pipes),
>> DMA_ALIGNMENT) +
>> +            ALIGN(((max_t(u32, _size_yuv, ((BIN_BUFFER_THRESHOLD * 3) >>
>> 1)) *
>> +            VPX_DECODER_FRAME_BIN_RES_BUDGET /
>> VPX_DECODER_FRAME_BIN_DENOMINATOR *
>> +            VPX_DECODER_FRAME_CONCURENCY_LVL) / num_vpp_pipes),
>> DMA_ALIGNMENT);
> 
> The size_yuv I guess just about makes sense but the _size component here is
> pretty hard to say whether or not this adds up.
> 
> Could you please add some comments to describe the calculations in these
> complex size/alignment clauses.
I believe the MACROS here defines the parameters used in calculation,
beyond this, its again how internal buffers (used by hardware) are
calculated by hardware and defined in the hardware specification.

Thanks,
Dikshita
> 
> ---
> bod