| Message ID | 20250305062630.2329032-1-ming.qian@oss.nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,1/2] media: amphion: Reduce decoding latency for HEVC decoder | expand |
Hi, Le mercredi 05 mars 2025 à 14:26 +0800, ming.qian@oss.nxp.com a écrit : > From: Ming Qian <ming.qian@oss.nxp.com> > > 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. Just curiosity, does it stay spec compliant or not ? Perhaps share some compliance (fluster) results ? > > Signed-off-by: Ming Qian <ming.qian@oss.nxp.com> > --- > v3 > - Improve commit message as recommended > 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) Just a suggestion, could also use an inline function. #define MALONE_VERSION(maj, min, inc) (((maj) << 16) | ((min) << 16) | (inc)) & MALONE_VERSION_MASK) #define CHECK_VERSION(iface, maj, min) ((iface->fw_version & MALONE_VERSION_MASK) >= MALONE_VERSION(maj, min, 0)) > + > 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) So if could be: !CHECK_VERSION(iface, 1, 9) There might be even better ways, this is not a hard request from me though. > + 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) With or without the adjustments. Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com
Hi Nicolas, On 2025/3/27 4:48, Nicolas Dufresne wrote: > Hi, > > Le mercredi 05 mars 2025 à 14:26 +0800, ming.qian@oss.nxp.com a écrit : >> From: Ming Qian <ming.qian@oss.nxp.com> >> >> 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. > > Just curiosity, does it stay spec compliant or not ? Perhaps share some > compliance (fluster) results ? > I don't think this will affect spec compliant, and the v4l2-compliance results are all pass: Total for amphion-vpu device /dev/video0: 48, Succeeded: 48, Failed: 0, Warnings: 0 Total for amphion-vpu device /dev/video1: 48, Succeeded: 48, Failed: 0, Warnings: 0 And the result of fluster is same as previous. The number of passes is the same as before. >> >> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com> >> --- >> v3 >> - Improve commit message as recommended >> 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) > > Just a suggestion, could also use an inline function. > > #define MALONE_VERSION(maj, min, inc) (((maj) << 16) | ((min) << 16) | (inc)) & MALONE_VERSION_MASK) > #define CHECK_VERSION(iface, maj, min) ((iface->fw_version & MALONE_VERSION_MASK) >= MALONE_VERSION(maj, min, 0)) > >> + >> 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) > > So if could be: > !CHECK_VERSION(iface, 1, 9) > > There might be even better ways, this is not a hard request from me > though. > Good suggestion, I'll apply it. >> + 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) > > With or without the adjustments. > > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com > Thanks, Ming
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)