drivers/media/platform/amphion/vdec.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
Start the decoding job when both queue are on, except the for the
initialization sequence.
Especially when seeking, the capture streamon may be called after output
streamon, driver will start to decode job immediately after output
streamo, if seek to a new resolution, then the source change flow may be
mixed with the seek, it will cause confusion, then may led to pipeline
hang.
When both output and capture queue are on, it's ready to start the
decoding job, and it can avoid the above potential problem.
Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
drivers/media/platform/amphion/vdec.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/amphion/vdec.c b/drivers/media/platform/amphion/vdec.c
index 6a38a0fa0e2d..ca8f7319503a 100644
--- a/drivers/media/platform/amphion/vdec.c
+++ b/drivers/media/platform/amphion/vdec.c
@@ -1363,6 +1363,12 @@ static int vdec_process_output(struct vpu_inst *inst, struct vb2_buffer *vb)
if (inst->state == VPU_CODEC_STATE_STARTED)
vdec_update_state(inst, VPU_CODEC_STATE_ACTIVE, 0);
+ if (vdec->seq_hdr_found &&
+ !vb2_start_streaming_called((v4l2_m2m_get_dst_vq(inst->fh.m2m_ctx)))) {
+ vpu_trace(inst->dev, "[%d] capture is not ready, pend input frame\n", inst->id);
+ return -EINVAL;
+ }
+
ret = vpu_iface_get_stream_buffer_desc(inst, &desc);
if (ret)
return ret;
@@ -1555,6 +1561,16 @@ static int vdec_start(struct vpu_inst *inst)
return ret;
}
+static void vdec_enqueue_pending_frames(struct vpu_inst *inst)
+{
+ int i;
+
+ for (i = 0; i < v4l2_m2m_num_src_bufs_ready(inst->fh.m2m_ctx); i++) {
+ if (vpu_process_output_buffer(inst))
+ break;
+ }
+}
+
static int vdec_start_session(struct vpu_inst *inst, u32 type)
{
struct vdec_t *vdec = inst->priv;
@@ -1573,10 +1589,10 @@ static int vdec_start_session(struct vpu_inst *inst, u32 type)
if (V4L2_TYPE_IS_OUTPUT(type)) {
vdec_update_state(inst, vdec->state, 1);
vdec->eos_received = 0;
- vpu_process_output_buffer(inst);
} else {
vdec_cmd_start(inst);
}
+ vdec_enqueue_pending_frames(inst);
if (inst->state == VPU_CODEC_STATE_ACTIVE)
vdec_response_fs_request(inst, false);
--
2.43.0-rc1Hi,
Le vendredi 19 juillet 2024 à 16:50 +0900, Ming Qian a écrit :
> Start the decoding job when both queue are on, except the for the
> initialization sequence.
>
> Especially when seeking, the capture streamon may be called after output
> streamon, driver will start to decode job immediately after output
> streamo, if seek to a new resolution, then the source change flow may be
> mixed with the seek, it will cause confusion, then may led to pipeline
> hang.
>
> When both output and capture queue are on, it's ready to start the
> decoding job, and it can avoid the above potential problem.
This commit message needs some work and I'm unsure I understand its
meaning. After reading the change, I am under the impression that you
simply say that once the seq_hdr is found, the driver should keep
delaying the processing of output buffer until the capture queue is
ready ?
>
> Signed-off-by: Ming Qian <ming.qian@nxp.com>
> ---
> drivers/media/platform/amphion/vdec.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/amphion/vdec.c b/drivers/media/platform/amphion/vdec.c
> index 6a38a0fa0e2d..ca8f7319503a 100644
> --- a/drivers/media/platform/amphion/vdec.c
> +++ b/drivers/media/platform/amphion/vdec.c
> @@ -1363,6 +1363,12 @@ static int vdec_process_output(struct vpu_inst *inst, struct vb2_buffer *vb)
> if (inst->state == VPU_CODEC_STATE_STARTED)
> vdec_update_state(inst, VPU_CODEC_STATE_ACTIVE, 0);
>
> + if (vdec->seq_hdr_found &&
> + !vb2_start_streaming_called((v4l2_m2m_get_dst_vq(inst->fh.m2m_ctx)))) {
> + vpu_trace(inst->dev, "[%d] capture is not ready, pend input frame\n", inst->id);
> + return -EINVAL;
I got really confused by this error return value. Its clearly not an
error, but just a delay. I think before we add more on top, you should
turn all these function for which the return value is unused to void.
And use a plain "return;" here. That applies to all similar ops.
> + }
> +
> ret = vpu_iface_get_stream_buffer_desc(inst, &desc);
> if (ret)
> return ret;
> @@ -1555,6 +1561,16 @@ static int vdec_start(struct vpu_inst *inst)
> return ret;
> }
>
> +static void vdec_enqueue_pending_frames(struct vpu_inst *inst)
> +{
> + int i;
> +
> + for (i = 0; i < v4l2_m2m_num_src_bufs_ready(inst->fh.m2m_ctx); i++) {
> + if (vpu_process_output_buffer(inst))
> + break;
How well does this work ? Previous you made good care of interleaving
the processing output and capture, which I could imaging prevents the
hardware from starving ?
> + }
> +}
> +
> static int vdec_start_session(struct vpu_inst *inst, u32 type)
> {
> struct vdec_t *vdec = inst->priv;
> @@ -1573,10 +1589,10 @@ static int vdec_start_session(struct vpu_inst *inst, u32 type)
> if (V4L2_TYPE_IS_OUTPUT(type)) {
> vdec_update_state(inst, vdec->state, 1);
> vdec->eos_received = 0;
> - vpu_process_output_buffer(inst);
> } else {
> vdec_cmd_start(inst);
> }
> + vdec_enqueue_pending_frames(inst);
Was this intentional to reverse the STREAMON(CAPTURE) call flow to:
- process_capture() (inside vdec_cmd_start())
- proces_output() x n
Nicolas
> if (inst->state == VPU_CODEC_STATE_ACTIVE)
> vdec_response_fs_request(inst, false);
>
Hi Nicolas,
On 2025/4/23 3:46, Nicolas Dufresne wrote:
> Hi,
>
> Le vendredi 19 juillet 2024 à 16:50 +0900, Ming Qian a écrit :
>> Start the decoding job when both queue are on, except the for the
>> initialization sequence.
>>
>> Especially when seeking, the capture streamon may be called after output
>> streamon, driver will start to decode job immediately after output
>> streamo, if seek to a new resolution, then the source change flow may be
>> mixed with the seek, it will cause confusion, then may led to pipeline
>> hang.
>>
>> When both output and capture queue are on, it's ready to start the
>> decoding job, and it can avoid the above potential problem.
>
> This commit message needs some work and I'm unsure I understand its
> meaning. After reading the change, I am under the impression that you
> simply say that once the seq_hdr is found, the driver should keep
> delaying the processing of output buffer until the capture queue is
> ready ?
>
I just want to do something like v4l2_m2m, that device_run() is only
scheduled when both queues are ready.
I admit that this patch does not have any substantial functional
changes.
I think this patch can be discarded, as it is confusing.
Thanks a lot for the review and comments.
Regards,
Ming
>>
>> Signed-off-by: Ming Qian <ming.qian@nxp.com>
>> ---
>> drivers/media/platform/amphion/vdec.c | 18 +++++++++++++++++-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/amphion/vdec.c b/drivers/media/platform/amphion/vdec.c
>> index 6a38a0fa0e2d..ca8f7319503a 100644
>> --- a/drivers/media/platform/amphion/vdec.c
>> +++ b/drivers/media/platform/amphion/vdec.c
>> @@ -1363,6 +1363,12 @@ static int vdec_process_output(struct vpu_inst *inst, struct vb2_buffer *vb)
>> if (inst->state == VPU_CODEC_STATE_STARTED)
>> vdec_update_state(inst, VPU_CODEC_STATE_ACTIVE, 0);
>>
>> + if (vdec->seq_hdr_found &&
>> + !vb2_start_streaming_called((v4l2_m2m_get_dst_vq(inst->fh.m2m_ctx)))) {
>> + vpu_trace(inst->dev, "[%d] capture is not ready, pend input frame\n", inst->id);
>> + return -EINVAL;
>
> I got really confused by this error return value. Its clearly not an
> error, but just a delay. I think before we add more on top, you should
> turn all these function for which the return value is unused to void.
> And use a plain "return;" here. That applies to all similar ops.
>
>> + }
>> +
>> ret = vpu_iface_get_stream_buffer_desc(inst, &desc);
>> if (ret)
>> return ret;
>> @@ -1555,6 +1561,16 @@ static int vdec_start(struct vpu_inst *inst)
>> return ret;
>> }
>>
>> +static void vdec_enqueue_pending_frames(struct vpu_inst *inst)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < v4l2_m2m_num_src_bufs_ready(inst->fh.m2m_ctx); i++) {
>> + if (vpu_process_output_buffer(inst))
>> + break;
>
> How well does this work ? Previous you made good care of interleaving
> the processing output and capture, which I could imaging prevents the
> hardware from starving ?
>
>> + }
>> +}
>> +
>> static int vdec_start_session(struct vpu_inst *inst, u32 type)
>> {
>> struct vdec_t *vdec = inst->priv;
>> @@ -1573,10 +1589,10 @@ static int vdec_start_session(struct vpu_inst *inst, u32 type)
>> if (V4L2_TYPE_IS_OUTPUT(type)) {
>> vdec_update_state(inst, vdec->state, 1);
>> vdec->eos_received = 0;
>> - vpu_process_output_buffer(inst);
>> } else {
>> vdec_cmd_start(inst);
>> }
>> + vdec_enqueue_pending_frames(inst);
>
> Was this intentional to reverse the STREAMON(CAPTURE) call flow to:
>
> - process_capture() (inside vdec_cmd_start())
> - proces_output() x n
>
> Nicolas
>
>> if (inst->state == VPU_CODEC_STATE_ACTIVE)
>> vdec_response_fs_request(inst, false);
>>
>
For format H264 and HEVC, the firmware can report the parsed profile idc
and level idc to driver, there information may be useful.
Implement the H264 and HEVC profile and level control to report them.
Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
drivers/media/platform/amphion/vdec.c | 60 +++++++++++++
drivers/media/platform/amphion/vpu_defs.h | 1 +
drivers/media/platform/amphion/vpu_helpers.c | 93 ++++++++++++++++++++
drivers/media/platform/amphion/vpu_helpers.h | 5 ++
drivers/media/platform/amphion/vpu_malone.c | 3 +-
5 files changed, 161 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/amphion/vdec.c b/drivers/media/platform/amphion/vdec.c
index ca8f7319503a..e414d1b56133 100644
--- a/drivers/media/platform/amphion/vdec.c
+++ b/drivers/media/platform/amphion/vdec.c
@@ -232,6 +232,36 @@ static int vdec_ctrl_init(struct vpu_inst *inst)
V4L2_CID_MPEG_VIDEO_DEC_DISPLAY_DELAY_ENABLE,
0, 1, 1, 0);
+ v4l2_ctrl_new_std_menu(&inst->ctrl_handler, NULL,
+ V4L2_CID_MPEG_VIDEO_H264_PROFILE,
+ V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH,
+ ~((1 << V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
+ (1 << V4L2_MPEG_VIDEO_H264_PROFILE_MAIN) |
+ (1 << V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED) |
+ (1 << V4L2_MPEG_VIDEO_H264_PROFILE_HIGH) |
+ (1 << V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH) |
+ (1 << V4L2_MPEG_VIDEO_H264_PROFILE_STEREO_HIGH)),
+ V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE);
+
+ v4l2_ctrl_new_std_menu(&inst->ctrl_handler, NULL,
+ V4L2_CID_MPEG_VIDEO_H264_LEVEL,
+ V4L2_MPEG_VIDEO_H264_LEVEL_6_2,
+ 0,
+ V4L2_MPEG_VIDEO_H264_LEVEL_4_0);
+
+ v4l2_ctrl_new_std_menu(&inst->ctrl_handler, NULL,
+ V4L2_CID_MPEG_VIDEO_HEVC_PROFILE,
+ V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10,
+ ~((1 << V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN) |
+ (1 << V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10)),
+ V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN);
+
+ v4l2_ctrl_new_std_menu(&inst->ctrl_handler, NULL,
+ V4L2_CID_MPEG_VIDEO_HEVC_LEVEL,
+ V4L2_MPEG_VIDEO_HEVC_LEVEL_6_2,
+ 0,
+ V4L2_MPEG_VIDEO_HEVC_LEVEL_4);
+
ctrl = v4l2_ctrl_new_std(&inst->ctrl_handler, &vdec_ctrl_ops,
V4L2_CID_MIN_BUFFERS_FOR_CAPTURE, 1, 32, 1, 2);
if (ctrl)
@@ -1166,6 +1196,35 @@ static void vdec_clear_slots(struct vpu_inst *inst)
}
}
+static void vdec_update_v4l2_ctrl(struct vpu_inst *inst, u32 id, u32 val)
+{
+ struct v4l2_ctrl *ctrl = v4l2_ctrl_find(&inst->ctrl_handler, id);
+
+ if (ctrl)
+ v4l2_ctrl_s_ctrl(ctrl, val);
+}
+
+static void vdec_update_v4l2_profile_level(struct vpu_inst *inst, struct vpu_dec_codec_info *hdr)
+{
+ switch (inst->out_format.pixfmt) {
+ case V4L2_PIX_FMT_H264:
+ case V4L2_PIX_FMT_H264_MVC:
+ vdec_update_v4l2_ctrl(inst, V4L2_CID_MPEG_VIDEO_H264_PROFILE,
+ vpu_get_h264_v4l2_profile(hdr->profile_idc));
+ vdec_update_v4l2_ctrl(inst, V4L2_CID_MPEG_VIDEO_H264_LEVEL,
+ vpu_get_h264_v4l2_level(hdr->level_idc));
+ break;
+ case V4L2_PIX_FMT_HEVC:
+ vdec_update_v4l2_ctrl(inst, V4L2_CID_MPEG_VIDEO_HEVC_PROFILE,
+ vpu_get_hevc_v4l2_profile(hdr->profile_idc));
+ vdec_update_v4l2_ctrl(inst, V4L2_CID_MPEG_VIDEO_HEVC_LEVEL,
+ vpu_get_hevc_v4l2_level(hdr->level_idc));
+ break;
+ default:
+ return;
+ }
+}
+
static void vdec_event_seq_hdr(struct vpu_inst *inst, struct vpu_dec_codec_info *hdr)
{
struct vdec_t *vdec = inst->priv;
@@ -1189,6 +1248,7 @@ static void vdec_event_seq_hdr(struct vpu_inst *inst, struct vpu_dec_codec_info
vdec_init_crop(inst);
vdec_init_mbi(inst);
vdec_init_dcp(inst);
+ vdec_update_v4l2_profile_level(inst, hdr);
if (!vdec->seq_hdr_found) {
vdec->seq_tag = vdec->codec_info.tag;
if (vdec->is_source_changed) {
diff --git a/drivers/media/platform/amphion/vpu_defs.h b/drivers/media/platform/amphion/vpu_defs.h
index 428d988cf2f7..606f9d61a265 100644
--- a/drivers/media/platform/amphion/vpu_defs.h
+++ b/drivers/media/platform/amphion/vpu_defs.h
@@ -134,6 +134,7 @@ struct vpu_dec_codec_info {
u32 decoded_height;
struct v4l2_fract frame_rate;
u32 dsp_asp_ratio;
+ u32 profile_idc;
u32 level_idc;
u32 bit_depth_luma;
u32 bit_depth_chroma;
diff --git a/drivers/media/platform/amphion/vpu_helpers.c b/drivers/media/platform/amphion/vpu_helpers.c
index d12310af9ebc..108b75ceb4ae 100644
--- a/drivers/media/platform/amphion/vpu_helpers.c
+++ b/drivers/media/platform/amphion/vpu_helpers.c
@@ -509,3 +509,96 @@ const char *vpu_codec_state_name(enum vpu_codec_state state)
}
return "<unknown>";
}
+
+struct codec_id_mapping {
+ u32 id;
+ u32 v4l2_id;
+};
+
+static struct codec_id_mapping h264_profiles[] = {
+ {66, V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE},
+ {77, V4L2_MPEG_VIDEO_H264_PROFILE_MAIN},
+ {88, V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED},
+ {100, V4L2_MPEG_VIDEO_H264_PROFILE_HIGH},
+ {118, V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH},
+ {128, V4L2_MPEG_VIDEO_H264_PROFILE_STEREO_HIGH}
+};
+
+static struct codec_id_mapping h264_levels[] = {
+ {10, V4L2_MPEG_VIDEO_H264_LEVEL_1_0},
+ {9, V4L2_MPEG_VIDEO_H264_LEVEL_1B},
+ {11, V4L2_MPEG_VIDEO_H264_LEVEL_1_1},
+ {12, V4L2_MPEG_VIDEO_H264_LEVEL_1_2},
+ {13, V4L2_MPEG_VIDEO_H264_LEVEL_1_3},
+ {20, V4L2_MPEG_VIDEO_H264_LEVEL_2_0},
+ {21, V4L2_MPEG_VIDEO_H264_LEVEL_2_1},
+ {22, V4L2_MPEG_VIDEO_H264_LEVEL_2_2},
+ {30, V4L2_MPEG_VIDEO_H264_LEVEL_3_0},
+ {31, V4L2_MPEG_VIDEO_H264_LEVEL_3_1},
+ {32, V4L2_MPEG_VIDEO_H264_LEVEL_3_2},
+ {40, V4L2_MPEG_VIDEO_H264_LEVEL_4_0},
+ {41, V4L2_MPEG_VIDEO_H264_LEVEL_4_1},
+ {42, V4L2_MPEG_VIDEO_H264_LEVEL_4_2},
+ {50, V4L2_MPEG_VIDEO_H264_LEVEL_5_0},
+ {51, V4L2_MPEG_VIDEO_H264_LEVEL_5_1},
+ {52, V4L2_MPEG_VIDEO_H264_LEVEL_5_2},
+ {60, V4L2_MPEG_VIDEO_H264_LEVEL_6_0},
+ {61, V4L2_MPEG_VIDEO_H264_LEVEL_6_1},
+ {62, V4L2_MPEG_VIDEO_H264_LEVEL_6_2}
+};
+
+static struct codec_id_mapping hevc_profiles[] = {
+ {1, V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN},
+ {2, V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN_10}
+};
+
+static struct codec_id_mapping hevc_levels[] = {
+ {30, V4L2_MPEG_VIDEO_HEVC_LEVEL_1},
+ {60, V4L2_MPEG_VIDEO_HEVC_LEVEL_2},
+ {63, V4L2_MPEG_VIDEO_HEVC_LEVEL_2_1},
+ {90, V4L2_MPEG_VIDEO_HEVC_LEVEL_3},
+ {93, V4L2_MPEG_VIDEO_HEVC_LEVEL_3_1},
+ {120, V4L2_MPEG_VIDEO_HEVC_LEVEL_4},
+ {123, V4L2_MPEG_VIDEO_HEVC_LEVEL_4_1},
+ {150, V4L2_MPEG_VIDEO_HEVC_LEVEL_5},
+ {153, V4L2_MPEG_VIDEO_HEVC_LEVEL_5_1},
+ {156, V4L2_MPEG_VIDEO_HEVC_LEVEL_5_2},
+ {180, V4L2_MPEG_VIDEO_HEVC_LEVEL_6},
+ {183, V4L2_MPEG_VIDEO_HEVC_LEVEL_6_1},
+ {186, V4L2_MPEG_VIDEO_HEVC_LEVEL_6_2}
+};
+
+static u32 vpu_find_v4l2_id(u32 id, struct codec_id_mapping *array, u32 array_sz)
+{
+ u32 i;
+
+ if (!array || !array_sz)
+ return 0;
+
+ for (i = 0; i < array_sz; i++) {
+ if (id == array[i].id)
+ return array[i].v4l2_id;
+ }
+
+ return 0;
+}
+
+u32 vpu_get_h264_v4l2_profile(u32 idc)
+{
+ return vpu_find_v4l2_id(idc, h264_profiles, ARRAY_SIZE(h264_profiles));
+}
+
+u32 vpu_get_h264_v4l2_level(u32 idc)
+{
+ return vpu_find_v4l2_id(idc, h264_levels, ARRAY_SIZE(h264_levels));
+}
+
+u32 vpu_get_hevc_v4l2_profile(u32 idc)
+{
+ return vpu_find_v4l2_id(idc, hevc_profiles, ARRAY_SIZE(hevc_profiles));
+}
+
+u32 vpu_get_hevc_v4l2_level(u32 idc)
+{
+ return vpu_find_v4l2_id(idc, hevc_levels, ARRAY_SIZE(hevc_levels));
+}
diff --git a/drivers/media/platform/amphion/vpu_helpers.h b/drivers/media/platform/amphion/vpu_helpers.h
index 0eaddb07190d..dc5fb1ca2d33 100644
--- a/drivers/media/platform/amphion/vpu_helpers.h
+++ b/drivers/media/platform/amphion/vpu_helpers.h
@@ -70,4 +70,9 @@ int vpu_color_get_default(u32 primaries, u32 *ptransfers, u32 *pmatrix, u32 *pfu
int vpu_find_dst_by_src(struct vpu_pair *pairs, u32 cnt, u32 src);
int vpu_find_src_by_dst(struct vpu_pair *pairs, u32 cnt, u32 dst);
+
+u32 vpu_get_h264_v4l2_profile(u32 idc);
+u32 vpu_get_h264_v4l2_level(u32 idc);
+u32 vpu_get_hevc_v4l2_profile(u32 idc);
+u32 vpu_get_hevc_v4l2_level(u32 idc);
#endif
diff --git a/drivers/media/platform/amphion/vpu_malone.c b/drivers/media/platform/amphion/vpu_malone.c
index 4769c053c6c2..5c6b2a841b6f 100644
--- a/drivers/media/platform/amphion/vpu_malone.c
+++ b/drivers/media/platform/amphion/vpu_malone.c
@@ -889,7 +889,8 @@ static void vpu_malone_unpack_seq_hdr(struct vpu_rpc_event *pkt,
info->frame_rate.numerator = 1000;
info->frame_rate.denominator = pkt->data[8];
info->dsp_asp_ratio = pkt->data[9];
- info->level_idc = pkt->data[10];
+ info->profile_idc = (pkt->data[10] >> 8) & 0xff;
+ info->level_idc = pkt->data[10] & 0xff;
info->bit_depth_luma = pkt->data[13];
info->bit_depth_chroma = pkt->data[14];
info->chroma_fmt = pkt->data[15];
--
2.43.0-rc1© 2016 - 2025 Red Hat, Inc.