Re: [PATCH v2 1/2] media: amphion: Reduce decoding latency for HEVC decoder

["Ming Qian(OSS)" <ming.qian@oss.nxp.com>]

Hi Sebastian,

On 2025/2/26 17:45, Sebastian Fricke wrote:
> [You don't often get email from sebastian.fricke@collabora.com. Learn 
> why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Hey Ming,
> 
> thank you for the patch!
> 
> On 17.01.2025 16:57, Ming Qian wrote:
>> The amphion decoder firmware supports low latency flush mode for
>> HEVC format since v1.9.0, it can help reduce the decoding latency by
>> appending some padding data after every frame, then driver can enable
>> this feature when the display delay 0 is enabled.
> 
> I see that you already changed the commit message for this version, but
> I still have a few recommendations for the description:
> 
>      The amphion decoder firmware supports a low latency flush mode for
>      the HEVC format since v1.9.0. This feature, which is enabled when
>      the display delay is set to 0, can help to reduce the decoding
>      latency by appending some padding data to every frame.
> 
> The rest looks good to me.

Thanks for your feedback, I'm preparing the v3 patch, and it will follow
your advice.

Thanks,
Ming

> 
> Regards,
> Sebastian
> 
>>
>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>> ---
>> v2
>> - Improve commit message
>> - Add firmware version check
>>
>> drivers/media/platform/amphion/vpu_malone.c | 22 ++++++++++++++++++---
>> 1 file changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/amphion/vpu_malone.c 
>> b/drivers/media/platform/amphion/vpu_malone.c
>> index 5c6b2a841b6f..1d9e10d9bec1 100644
>> --- a/drivers/media/platform/amphion/vpu_malone.c
>> +++ b/drivers/media/platform/amphion/vpu_malone.c
>> @@ -68,6 +68,9 @@
>>
>> #define MALONE_DEC_FMT_RV_MASK                        BIT(21)
>>
>> +#define MALONE_VERSION_MASK                   0xFFFFF
>> +#define MALONE_MIN_VERSION_HEVC_BUFFLUSH      (((1 << 16) | (9 << 8) 
>> | 0) & MALONE_VERSION_MASK)
>> +
>> enum vpu_malone_stream_input_mode {
>>       INVALID_MODE = 0,
>>       FRAME_LVL,
>> @@ -332,6 +335,8 @@ struct vpu_dec_ctrl {
>>       u32 buf_addr[VID_API_NUM_STREAMS];
>> };
>>
>> +static const struct malone_padding_scode *get_padding_scode(u32 type, 
>> u32 fmt);
>> +
>> u32 vpu_malone_get_data_size(void)
>> {
>>       return sizeof(struct vpu_dec_ctrl);
>> @@ -654,9 +659,16 @@ static int vpu_malone_set_params(struct 
>> vpu_shared_addr *shared,
>>               hc->jpg[instance].jpg_mjpeg_interlaced = 0;
>>       }
>>
>> -      hc->codec_param[instance].disp_imm = 
>> params->display_delay_enable ? 1 : 0;
>> -      if (malone_format != MALONE_FMT_AVC)
>> +      if (params->display_delay_enable &&
>> +          get_padding_scode(SCODE_PADDING_BUFFLUSH, 
>> params->codec_format))
>> +              hc->codec_param[instance].disp_imm = 1;
>> +      else
>>               hc->codec_param[instance].disp_imm = 0;
>> +
>> +      if (params->codec_format == V4L2_PIX_FMT_HEVC &&
>> +          (iface->fw_version & MALONE_VERSION_MASK) < 
>> MALONE_MIN_VERSION_HEVC_BUFFLUSH)
>> +              hc->codec_param[instance].disp_imm = 0;
>> +
>>       hc->codec_param[instance].dbglog_enable = 0;
>>       iface->dbglog_desc.level = 0;
>>
>> @@ -1024,6 +1036,7 @@ static const struct malone_padding_scode 
>> padding_scodes[] = {
>>       {SCODE_PADDING_EOS,      V4L2_PIX_FMT_JPEG,        {0x0, 0x0}},
>>       {SCODE_PADDING_BUFFLUSH, V4L2_PIX_FMT_H264,        {0x15010000, 
>> 0x0}},
>>       {SCODE_PADDING_BUFFLUSH, V4L2_PIX_FMT_H264_MVC,    {0x15010000, 
>> 0x0}},
>> +      {SCODE_PADDING_BUFFLUSH, V4L2_PIX_FMT_HEVC,        {0x3e010000, 
>> 0x20}},
>> };
>>
>> static const struct malone_padding_scode padding_scode_dft = {0x0, 0x0};
>> @@ -1058,8 +1071,11 @@ static int vpu_malone_add_padding_scode(struct 
>> vpu_buffer *stream_buffer,
>>       int ret;
>>
>>       ps = get_padding_scode(scode_type, pixelformat);
>> -      if (!ps)
>> +      if (!ps) {
>> +              if (scode_type == SCODE_PADDING_BUFFLUSH)
>> +                      return 0;
>>               return -EINVAL;
>> +      }
>>
>>       wptr = readl(&str_buf->wptr);
>>       if (wptr < stream_buffer->phys || wptr > stream_buffer->phys + 
>> stream_buffer->length)
>> -- 
>> 2.43.0-rc1
>>
>>