| Message ID | 20250428-qcom-iris-hevc-vp9-v2-0-3a6013ecb8a5@quicinc.com |
|---|---|
| Headers | show |
| Series | Add support for HEVC and VP9 codecs in decoder | expand |
On 4/29/2025 2:54 PM, Vikash Garodia wrote: > > On 4/28/2025 2:58 PM, Dikshita Agarwal wrote: >> Firmware might hold the DPB buffers for reference in case of sequence >> change, so skip destroying buffers for which QUEUED flag is not removed. >> Also, make sure that all buffers are released during streamoff. >> >> Cc: stable@vger.kernel.org >> Fixes: 73702f45db81 ("media: iris: allocate, initialize and queue internal buffers") >> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> >> --- >> drivers/media/platform/qcom/iris/iris_buffer.c | 37 +++++++++++++++++++++++++- >> drivers/media/platform/qcom/iris/iris_buffer.h | 3 ++- >> drivers/media/platform/qcom/iris/iris_vdec.c | 4 +-- >> drivers/media/platform/qcom/iris/iris_vidc.c | 6 +++-- >> 4 files changed, 44 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c >> index e5c5a564fcb8..606d76b10be2 100644 >> --- a/drivers/media/platform/qcom/iris/iris_buffer.c >> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c >> @@ -376,7 +376,7 @@ int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buf >> return 0; >> } >> >> -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) >> +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force) >> { >> const struct iris_platform_data *platform_data = inst->core->iris_platform_data; >> struct iris_buffer *buf, *next; >> @@ -396,6 +396,14 @@ int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) >> for (i = 0; i < len; i++) { >> buffers = &inst->buffers[internal_buf_type[i]]; >> list_for_each_entry_safe(buf, next, &buffers->list, list) { >> + /* >> + * during stream on, skip destroying internal(DPB) buffer >> + * if firmware did not return it. >> + * during close, destroy all buffers irrespectively. >> + */ >> + if (!force && buf->attr & BUF_ATTR_QUEUED) >> + continue; >> + >> ret = iris_destroy_internal_buffer(inst, buf); >> if (ret) >> return ret; >> @@ -446,6 +454,33 @@ static int iris_release_input_internal_buffers(struct iris_inst *inst) >> return 0; >> } >> >> +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane) > name this iris_check_num_queued_internal_buffers.. > >> +{ >> + const struct iris_platform_data *platform_data = inst->core->iris_platform_data; >> + struct iris_buffer *buf, *next; >> + struct iris_buffers *buffers; >> + const u32 *internal_buf_type; >> + u32 internal_buffer_count, i; >> + u32 count = 0; >> + >> + if (V4L2_TYPE_IS_OUTPUT(plane)) { >> + internal_buf_type = platform_data->dec_ip_int_buf_tbl; >> + internal_buffer_count = platform_data->dec_ip_int_buf_tbl_size; >> + } else { >> + internal_buf_type = platform_data->dec_op_int_buf_tbl; >> + internal_buffer_count = platform_data->dec_op_int_buf_tbl_size; >> + } >> + >> + for (i = 0; i < internal_buffer_count; i++) { >> + buffers = &inst->buffers[internal_buf_type[i]]; >> + list_for_each_entry_safe(buf, next, &buffers->list, list) >> + count++; >> + if (count) >> + dev_err(inst->core->dev, "%d buffer of type %d not released", >> + count, internal_buf_type[i]); >> + } >> +} >> + >> int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst) >> { >> struct iris_buffers *buffers = &inst->buffers[BUF_PERSIST]; >> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h >> index c36b6347b077..03a32b91cf21 100644 >> --- a/drivers/media/platform/qcom/iris/iris_buffer.h >> +++ b/drivers/media/platform/qcom/iris/iris_buffer.h >> @@ -106,7 +106,8 @@ void iris_get_internal_buffers(struct iris_inst *inst, u32 plane); >> int iris_create_internal_buffers(struct iris_inst *inst, u32 plane); >> int iris_queue_internal_buffers(struct iris_inst *inst, u32 plane); >> int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buffer); >> -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane); >> +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force); >> +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane); > make this static With the above changes, you can mark it Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com> > > Regards, > Vikash >> int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst); >> int iris_alloc_and_queue_input_int_bufs(struct iris_inst *inst); >> int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf); >> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c >> index 4143acedfc57..2c1a7162d2da 100644 >> --- a/drivers/media/platform/qcom/iris/iris_vdec.c >> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c >> @@ -408,7 +408,7 @@ int iris_vdec_streamon_input(struct iris_inst *inst) >> >> iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); >> >> - ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); >> + ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, false); >> if (ret) >> return ret; >> >> @@ -496,7 +496,7 @@ int iris_vdec_streamon_output(struct iris_inst *inst) >> >> iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); >> >> - ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); >> + ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, false); >> if (ret) >> return ret; >> >> diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c >> index ca0f4e310f77..56531a7f0dfe 100644 >> --- a/drivers/media/platform/qcom/iris/iris_vidc.c >> +++ b/drivers/media/platform/qcom/iris/iris_vidc.c >> @@ -233,8 +233,10 @@ int iris_close(struct file *filp) >> iris_session_close(inst); >> iris_inst_change_state(inst, IRIS_INST_DEINIT); >> iris_v4l2_fh_deinit(inst); >> - iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); >> - iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); >> + iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, true); >> + iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, true); >> + iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); >> + iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); >> iris_remove_session(inst); >> mutex_unlock(&inst->lock); >> mutex_destroy(&inst->ctx_q_lock); >>
On 4/28/2025 2:58 PM, Dikshita Agarwal wrote: > Firmware sends the picture type as NO_SHOW for frames which are not > supposed to be displayed, add handling for the same in driver to drop > them. > > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- > drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h | 1 + > drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 4 +++- > 2 files changed, 4 insertions(+), 1 deletion(-) > > 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 806f8bb7f505..666061a612c3 100644 > --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h > +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_defines.h > @@ -113,6 +113,7 @@ enum hfi_picture_type { > HFI_PICTURE_I = 0x00000008, > HFI_PICTURE_CRA = 0x00000010, > HFI_PICTURE_BLA = 0x00000020, > + HFI_PICTURE_NOSHOW = 0x00000040, > }; > > enum hfi_buffer_type { > diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c > index 5bb20ec0d67f..1ed798d31a3f 100644 > --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c > +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c > @@ -91,7 +91,9 @@ static int iris_hfi_gen2_get_driver_buffer_flags(struct iris_inst *inst, u32 hfi > struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst); > u32 driver_flags = 0; > > - if (inst_hfi_gen2->hfi_frame_info.picture_type & keyframe) > + if (inst_hfi_gen2->hfi_frame_info.picture_type & HFI_PICTURE_NOSHOW) > + driver_flags |= V4L2_BUF_FLAG_ERROR; > + else if (inst_hfi_gen2->hfi_frame_info.picture_type & keyframe) > driver_flags |= V4L2_BUF_FLAG_KEYFRAME; > else if (inst_hfi_gen2->hfi_frame_info.picture_type & HFI_PICTURE_P) > driver_flags |= V4L2_BUF_FLAG_PFRAME; > > Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
On 4/28/2025 2:59 PM, Dikshita Agarwal wrote: > HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER is deprecated and no longer > supported on current firmware, remove setting the same to firmware. > > At the same time, remove the check for non-zero number of v4l2 controls > as some SOC might not expose any capability which requires v4l2 control. > > Cc: stable@vger.kernel.org > Fixes: 79865252acb6 ("media: iris: enable video driver probe of SM8250 SoC") > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- > drivers/media/platform/qcom/iris/iris_ctrls.c | 6 ------ > drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c | 8 -------- > drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h | 1 - > drivers/media/platform/qcom/iris/iris_platform_common.h | 2 +- > drivers/media/platform/qcom/iris/iris_platform_sm8250.c | 9 --------- > 5 files changed, 1 insertion(+), 25 deletions(-) > > diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.c b/drivers/media/platform/qcom/iris/iris_ctrls.c > index b690578256d5..915de101fcba 100644 > --- a/drivers/media/platform/qcom/iris/iris_ctrls.c > +++ b/drivers/media/platform/qcom/iris/iris_ctrls.c > @@ -17,8 +17,6 @@ static inline bool iris_valid_cap_id(enum platform_inst_fw_cap_type cap_id) > static enum platform_inst_fw_cap_type iris_get_cap_id(u32 id) > { > switch (id) { > - case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER: > - return DEBLOCK; > case V4L2_CID_MPEG_VIDEO_H264_PROFILE: > return PROFILE; > case V4L2_CID_MPEG_VIDEO_H264_LEVEL: > @@ -34,8 +32,6 @@ static u32 iris_get_v4l2_id(enum platform_inst_fw_cap_type cap_id) > return 0; > > switch (cap_id) { > - case DEBLOCK: > - return V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER; > case PROFILE: > return V4L2_CID_MPEG_VIDEO_H264_PROFILE; > case LEVEL: > @@ -84,8 +80,6 @@ int iris_ctrls_init(struct iris_inst *inst) > if (iris_get_v4l2_id(cap[idx].cap_id)) > num_ctrls++; > } > - if (!num_ctrls) > - return -EINVAL; > > /* Adding 1 to num_ctrls to include V4L2_CID_MIN_BUFFERS_FOR_CAPTURE */ > > diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c > index 2239708d2d7e..f9f3e2d2ce29 100644 > --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c > +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c > @@ -490,14 +490,6 @@ iris_hfi_gen1_packet_session_set_property(struct hfi_session_set_property_pkt *p > packet->shdr.hdr.size += sizeof(u32) + sizeof(*wm); > break; > } > - case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: { > - struct hfi_enable *en = prop_data; > - u32 *in = pdata; > - > - en->enable = *in; > - packet->shdr.hdr.size += sizeof(u32) + sizeof(*en); > - break; > - } > default: > return -EINVAL; > } > diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h > index 93b5f838c290..adffcead58ea 100644 > --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h > +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h > @@ -65,7 +65,6 @@ > > #define HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS 0x202001 > > -#define HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER 0x1200001 > #define HFI_PROPERTY_PARAM_VDEC_DPB_COUNTS 0x120300e > #define HFI_PROPERTY_CONFIG_VDEC_ENTROPY 0x1204004 > > diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h > index ac76d9e1ef9c..1dab276431c7 100644 > --- a/drivers/media/platform/qcom/iris/iris_platform_common.h > +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h > @@ -89,7 +89,7 @@ enum platform_inst_fw_cap_type { > CODED_FRAMES, > BIT_DEPTH, > RAP_FRAME, > - DEBLOCK, > + TIER, > INST_FW_CAP_MAX, > }; > > diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8250.c b/drivers/media/platform/qcom/iris/iris_platform_sm8250.c > index 5c86fd7b7b6f..543fa2661539 100644 > --- a/drivers/media/platform/qcom/iris/iris_platform_sm8250.c > +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8250.c > @@ -30,15 +30,6 @@ static struct platform_inst_fw_cap inst_fw_cap_sm8250[] = { > .hfi_id = HFI_PROPERTY_PARAM_WORK_MODE, > .set = iris_set_stage, > }, > - { > - .cap_id = DEBLOCK, > - .min = 0, > - .max = 1, > - .step_or_mask = 1, > - .value = 0, > - .hfi_id = HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER, > - .set = iris_set_u32, > - }, > }; > > static struct platform_inst_caps platform_inst_cap_sm8250 = { > Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com>
On 4/29/2025 3:13 PM, Bryan O'Donoghue wrote: > On 28/04/2025 10:28, Dikshita Agarwal wrote: >> + /* >> + * during stream on, skip destroying internal(DPB) buffer >> + * if firmware did not return it. >> + * during close, destroy all buffers irrespectively. >> + */ >> + if (!force && buf->attr & BUF_ATTR_QUEUED) >> + continue; >> + > > What's the effect of the firmware not having dequeued the buffer though ? > > My main concern here is APSS and firmware have a different view of DMA memory. > > We release on the APSS side but firmware has not. > > Surely failure to release buffers by the time we get to Linux::close() is a > failure of the software contract sufficient to require resetting the > firmware ? > > i.e. we release memory on the APSS side but firmware writes into it anyway ... > As I mentioned earlier as well, during STOP issued before close, firmware is expected to release all these internal buffers and the check I added now in this patch will also ensures that. Firmware also has a check to make sure all buffers queued are released as part of STOP and then only firmware sends the STOP_DONE response to driver. STOP is a synchronous call and driver doesn't proceed without receiving the response for the same from firmware. So we will never run into this issue where driver release buffers which are still being accessed by firmware. Please note, these are internal buffers which are allocated and managed by driver only. Thanks, Dikshita > ? > > --- > bod
On 4/29/2025 2:54 PM, Vikash Garodia wrote: > > On 4/28/2025 2:58 PM, Dikshita Agarwal wrote: >> Firmware might hold the DPB buffers for reference in case of sequence >> change, so skip destroying buffers for which QUEUED flag is not removed. >> Also, make sure that all buffers are released during streamoff. >> >> Cc: stable@vger.kernel.org >> Fixes: 73702f45db81 ("media: iris: allocate, initialize and queue internal buffers") >> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> >> --- >> drivers/media/platform/qcom/iris/iris_buffer.c | 37 +++++++++++++++++++++++++- >> drivers/media/platform/qcom/iris/iris_buffer.h | 3 ++- >> drivers/media/platform/qcom/iris/iris_vdec.c | 4 +-- >> drivers/media/platform/qcom/iris/iris_vidc.c | 6 +++-- >> 4 files changed, 44 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c >> index e5c5a564fcb8..606d76b10be2 100644 >> --- a/drivers/media/platform/qcom/iris/iris_buffer.c >> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c >> @@ -376,7 +376,7 @@ int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buf >> return 0; >> } >> >> -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) >> +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force) >> { >> const struct iris_platform_data *platform_data = inst->core->iris_platform_data; >> struct iris_buffer *buf, *next; >> @@ -396,6 +396,14 @@ int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) >> for (i = 0; i < len; i++) { >> buffers = &inst->buffers[internal_buf_type[i]]; >> list_for_each_entry_safe(buf, next, &buffers->list, list) { >> + /* >> + * during stream on, skip destroying internal(DPB) buffer >> + * if firmware did not return it. >> + * during close, destroy all buffers irrespectively. >> + */ >> + if (!force && buf->attr & BUF_ATTR_QUEUED) >> + continue; >> + >> ret = iris_destroy_internal_buffer(inst, buf); >> if (ret) >> return ret; >> @@ -446,6 +454,33 @@ static int iris_release_input_internal_buffers(struct iris_inst *inst) >> return 0; >> } >> >> +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane) > name this iris_check_num_queued_internal_buffers.. > Ack. Thanks, Dikshita >> +{ >> + const struct iris_platform_data *platform_data = inst->core->iris_platform_data; >> + struct iris_buffer *buf, *next; >> + struct iris_buffers *buffers; >> + const u32 *internal_buf_type; >> + u32 internal_buffer_count, i; >> + u32 count = 0; >> + >> + if (V4L2_TYPE_IS_OUTPUT(plane)) { >> + internal_buf_type = platform_data->dec_ip_int_buf_tbl; >> + internal_buffer_count = platform_data->dec_ip_int_buf_tbl_size; >> + } else { >> + internal_buf_type = platform_data->dec_op_int_buf_tbl; >> + internal_buffer_count = platform_data->dec_op_int_buf_tbl_size; >> + } >> + >> + for (i = 0; i < internal_buffer_count; i++) { >> + buffers = &inst->buffers[internal_buf_type[i]]; >> + list_for_each_entry_safe(buf, next, &buffers->list, list) >> + count++; >> + if (count) >> + dev_err(inst->core->dev, "%d buffer of type %d not released", >> + count, internal_buf_type[i]); >> + } >> +} >> + >> int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst) >> { >> struct iris_buffers *buffers = &inst->buffers[BUF_PERSIST]; >> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h >> index c36b6347b077..03a32b91cf21 100644 >> --- a/drivers/media/platform/qcom/iris/iris_buffer.h >> +++ b/drivers/media/platform/qcom/iris/iris_buffer.h >> @@ -106,7 +106,8 @@ void iris_get_internal_buffers(struct iris_inst *inst, u32 plane); >> int iris_create_internal_buffers(struct iris_inst *inst, u32 plane); >> int iris_queue_internal_buffers(struct iris_inst *inst, u32 plane); >> int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buffer); >> -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane); >> +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force); >> +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane); > make this static > > Regards, > Vikash >> int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst); >> int iris_alloc_and_queue_input_int_bufs(struct iris_inst *inst); >> int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf); >> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c >> index 4143acedfc57..2c1a7162d2da 100644 >> --- a/drivers/media/platform/qcom/iris/iris_vdec.c >> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c >> @@ -408,7 +408,7 @@ int iris_vdec_streamon_input(struct iris_inst *inst) >> >> iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); >> >> - ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); >> + ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, false); >> if (ret) >> return ret; >> >> @@ -496,7 +496,7 @@ int iris_vdec_streamon_output(struct iris_inst *inst) >> >> iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); >> >> - ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); >> + ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, false); >> if (ret) >> return ret; >> >> diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c >> index ca0f4e310f77..56531a7f0dfe 100644 >> --- a/drivers/media/platform/qcom/iris/iris_vidc.c >> +++ b/drivers/media/platform/qcom/iris/iris_vidc.c >> @@ -233,8 +233,10 @@ int iris_close(struct file *filp) >> iris_session_close(inst); >> iris_inst_change_state(inst, IRIS_INST_DEINIT); >> iris_v4l2_fh_deinit(inst); >> - iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); >> - iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); >> + iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, true); >> + iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, true); >> + iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); >> + iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); >> iris_remove_session(inst); >> mutex_unlock(&inst->lock); >> mutex_destroy(&inst->ctx_q_lock); >>
On 4/28/2025 2:59 PM, Dikshita Agarwal wrote: > Currently, two types of flush commands are queued to the firmware, > the first flush queued as part of sequence change, does not wait for a > response, while the second flush queued as part of stop, expects a > completion response before proceeding further. > > Due to timing issue, the flush response corresponding to the first > command could arrive after the second flush is issued. This casuses the > driver to incorrectly assume that the second flush has completed, > leading to the premature signaling of flush_completion. > > To address this, introduce a counter to track the number of pending > flush responses and signal flush completion only when all expected > responses are received. > > Cc: stable@vger.kernel.org > Fixes: 11712ce70f8e ("media: iris: implement vb2 streaming ops") > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- > .../media/platform/qcom/iris/iris_hfi_gen1_command.c | 4 +++- > .../media/platform/qcom/iris/iris_hfi_gen1_response.c | 17 +++++++++++------ > drivers/media/platform/qcom/iris/iris_instance.h | 2 ++ > 3 files changed, 16 insertions(+), 7 deletions(-) > > diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c > index f9f3e2d2ce29..ef3ca676d2ea 100644 > --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c > +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c > @@ -208,8 +208,10 @@ static int iris_hfi_gen1_session_stop(struct iris_inst *inst, u32 plane) > flush_pkt.flush_type = flush_type; > > ret = iris_hfi_queue_cmd_write(core, &flush_pkt, flush_pkt.shdr.hdr.size); > - if (!ret) > + if (!ret) { > + inst->flush_responses_pending++; > ret = iris_wait_for_session_response(inst, true); > + } > } > > return ret; > diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c > index dfca45d85759..01338baf3788 100644 > --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c > +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c > @@ -207,7 +207,8 @@ static void iris_hfi_gen1_event_seq_changed(struct iris_inst *inst, > flush_pkt.shdr.hdr.pkt_type = HFI_CMD_SESSION_FLUSH; > flush_pkt.shdr.session_id = inst->session_id; > flush_pkt.flush_type = HFI_FLUSH_OUTPUT; > - iris_hfi_queue_cmd_write(inst->core, &flush_pkt, flush_pkt.shdr.hdr.size); > + if (!iris_hfi_queue_cmd_write(inst->core, &flush_pkt, flush_pkt.shdr.hdr.size)) > + inst->flush_responses_pending++; > } > > iris_vdec_src_change(inst); > @@ -408,7 +409,9 @@ static void iris_hfi_gen1_session_ftb_done(struct iris_inst *inst, void *packet) > flush_pkt.shdr.hdr.pkt_type = HFI_CMD_SESSION_FLUSH; > flush_pkt.shdr.session_id = inst->session_id; > flush_pkt.flush_type = HFI_FLUSH_OUTPUT; > - iris_hfi_queue_cmd_write(core, &flush_pkt, flush_pkt.shdr.hdr.size); > + if (!iris_hfi_queue_cmd_write(core, &flush_pkt, flush_pkt.shdr.hdr.size)) > + inst->flush_responses_pending++; > + > iris_inst_sub_state_change_drain_last(inst); > > return; > @@ -570,7 +573,6 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response > const struct iris_hfi_gen1_response_pkt_info *pkt_info; > struct device *dev = core->dev; > struct hfi_session_pkt *pkt; > - struct completion *done; > struct iris_inst *inst; > bool found = false; > u32 i; > @@ -631,9 +633,12 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response > if (shdr->error_type != HFI_ERR_NONE) > iris_inst_change_state(inst, IRIS_INST_ERROR); > > - done = pkt_info->pkt == HFI_MSG_SESSION_FLUSH ? > - &inst->flush_completion : &inst->completion; > - complete(done); > + if (pkt_info->pkt == HFI_MSG_SESSION_FLUSH) { > + if (--inst->flush_responses_pending <= 0) No need to check for < 0 condition as its an unsigned int. Signal when equals 0. With above change, mark Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com> > + complete(&inst->flush_completion); > + } else { > + complete(&inst->completion); > + } > } > mutex_unlock(&inst->lock); > > diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h > index 5150237f0020..9ed197799ee7 100644 > --- a/drivers/media/platform/qcom/iris/iris_instance.h > +++ b/drivers/media/platform/qcom/iris/iris_instance.h > @@ -27,6 +27,7 @@ > * @crop: structure of crop info > * @completion: structure of signal completions > * @flush_completion: structure of signal completions for flush cmd > + * @flush_responses_pending: counter to track number of pending flush responses > * @fw_caps: array of supported instance firmware capabilities > * @buffers: array of different iris buffers > * @fw_min_count: minimnum count of buffers needed by fw > @@ -59,6 +60,7 @@ struct iris_inst { > struct iris_hfi_rect_desc crop; > struct completion completion; > struct completion flush_completion; > + u32 flush_responses_pending; > struct platform_inst_fw_cap fw_caps[INST_FW_CAP_MAX]; > struct iris_buffers buffers[BUF_TYPE_MAX]; > u32 fw_min_count; >
Hi All, This patch series adds initial support for the HEVC(H.265) and VP9 codecs in iris decoder. The objective of this work is to extend the decoder's capabilities to handle HEVC and VP9 codec streams, including necessary format handling and buffer management. In addition, the series also includes a set of fixes to address issues identified during testing of these additional codecs. These patches also address the comments and feedback received from the RFC patches previously sent. I have made the necessary improvements based on the community's suggestions. Changes in v2: - Added Changes to make sure all buffers are released in session close (bryna) - Added tracking for flush responses to fix a timing issue. - Added a handling to fix timing issue in reconfig - Splitted patch 06/20 in two patches (Bryan) - Added missing fixes tag (bryan) - Updated fluster report (Nicolas) - Link to v1: https://lore.kernel.org/r/20250408-iris-dec-hevc-vp9-v1-0-acd258778bd6@quicinc.com Changes sinces RFC: - Added additional fixes to address issues identified during further testing. - Moved typo fix to a seperate patch [Neil] - Reordered the patches for better logical flow and clarity [Neil, Dmitry] - Added fixes tag wherever applicable [Neil, Dmitry] - Removed the default case in the switch statement for codecs [Bryan] - Replaced if-else statements with switch-case [Bryan] - Added comments for mbpf [Bryan] - RFC: https://lore.kernel.org/linux-media/20250305104335.3629945-1-quic_dikshita@quicinc.com/ This patch series depends on [1] & [2] [1] https://lore.kernel.org/linux-media/20250417-topic-sm8x50-iris-v10-v7-0-f020cb1d0e98@linaro.org/ [2] https://lore.kernel.org/linux-media/20250424-qcs8300_iris-v5-0-f118f505c300@quicinc.com/ These patches are tested on SM8250 and SM8550 with v4l2-ctl and Gstreamer for HEVC and VP9 decoders, at the same time ensured that the existing H264 decoder functionality remains uneffected. Note: 1 of the fluster compliance test is fixed with firmware [3] [3]: https://lore.kernel.org/linux-firmware/1a511921-446d-cdc4-0203-084c88a5dc1e@quicinc.com/T/#u The result of fluster test on SM8550: 131/147 testcases passed while testing JCT-VC-HEVC_V1 with GStreamer-H.265-V4L2-Gst1.0. The failing test case: - 10 testcases failed due to unsupported 10 bit format. - DBLK_A_MAIN10_VIXS_4 - INITQP_B_Main10_Sony_1 - TSUNEQBD_A_MAIN10_Technicolor_2 - WP_A_MAIN10_Toshiba_3 - WP_MAIN10_B_Toshiba_3 - WPP_A_ericsson_MAIN10_2 - WPP_B_ericsson_MAIN10_2 - WPP_C_ericsson_MAIN10_2 - WPP_E_ericsson_MAIN10_2 - WPP_F_ericsson_MAIN10_2 - 4 testcase failed due to unsupported resolution - PICSIZE_A_Bossen_1 - PICSIZE_B_Bossen_1 - WPP_D_ericsson_MAIN10_2 - WPP_D_ericsson_MAIN_2 - 2 testcase failed due to CRC mismatch - RAP_A_docomo_6 - RAP_B_Bossen_2 - BUG reported: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/4392 Analysis - First few frames in this discarded by firmware and are sent to driver with 0 filled length. Driver send such buffers to client with timestamp 0 and payload set to 0 and make buf state to VB2_BUF_STATE_ERROR. Such buffers should be dropped by GST. But instead, the first frame displayed as green frame and when a valid buffer is sent to client later with same 0 timestamp, its dropped, leading to CRC mismatch for first frame. 235/305 testcases passed while testing VP9-TEST-VECTORS with GStreamer-VP9-V4L2-Gst1.0. The failing test case: - 64 testcases failed due to unsupported resolution - vp90-2-02-size-08x08.webm - vp90-2-02-size-08x10.webm - vp90-2-02-size-08x16.webm - vp90-2-02-size-08x18.webm - vp90-2-02-size-08x32.webm - vp90-2-02-size-08x34.webm - vp90-2-02-size-08x64.webm - vp90-2-02-size-08x66.webm - vp90-2-02-size-10x08.webm - vp90-2-02-size-10x10.webm - vp90-2-02-size-10x16.webm - vp90-2-02-size-10x18.webm - vp90-2-02-size-10x32.webm - vp90-2-02-size-10x34.webm - vp90-2-02-size-10x64.webm - vp90-2-02-size-10x66.webm - vp90-2-02-size-16x08.webm - vp90-2-02-size-16x10.webm - vp90-2-02-size-16x16.webm - vp90-2-02-size-16x18.webm - vp90-2-02-size-16x32.webm - vp90-2-02-size-16x34.webm - vp90-2-02-size-16x64.webm - vp90-2-02-size-16x66.webm - vp90-2-02-size-18x08.webm - vp90-2-02-size-18x10.webm - vp90-2-02-size-18x16.webm - vp90-2-02-size-18x18.webm - vp90-2-02-size-18x32.webm - vp90-2-02-size-18x34.webm - vp90-2-02-size-18x64.webm - vp90-2-02-size-18x66.webm - vp90-2-02-size-32x08.webm - vp90-2-02-size-32x10.webm - vp90-2-02-size-32x16.webm - vp90-2-02-size-32x18.webm - vp90-2-02-size-32x32.webm - vp90-2-02-size-32x34.webm - vp90-2-02-size-32x64.webm - vp90-2-02-size-32x66.webm - vp90-2-02-size-34x08.webm - vp90-2-02-size-34x10.webm - vp90-2-02-size-34x16.webm - vp90-2-02-size-34x18.webm - vp90-2-02-size-34x32.webm - vp90-2-02-size-34x34.webm - vp90-2-02-size-34x64.webm - vp90-2-02-size-34x66.webm - vp90-2-02-size-64x08.webm - vp90-2-02-size-64x10.webm - vp90-2-02-size-64x16.webm - vp90-2-02-size-64x18.webm - vp90-2-02-size-64x32.webm - vp90-2-02-size-64x34.webm - vp90-2-02-size-64x64.webm - vp90-2-02-size-64x66.webm - vp90-2-02-size-66x08.webm - vp90-2-02-size-66x10.webm - vp90-2-02-size-66x16.webm - vp90-2-02-size-66x18.webm - vp90-2-02-size-66x32.webm - vp90-2-02-size-66x34.webm - vp90-2-02-size-66x64.webm - vp90-2-02-size-66x66.webm - 2 testcases failed due to unsupported format - vp91-2-04-yuv422.webm - vp91-2-04-yuv444.webm - 1 testcase failed with CRC mismatch - vp90-2-22-svc_1280x720_3.ivf - Bug reported: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/4371 - 2 testcase failed due to unsupported resolution after sequence change - vp90-2-21-resize_inter_320x180_5_1-2.webm - vp90-2-21-resize_inter_320x180_7_1-2.webm - 1 testcase failed due to unsupported stream - vp90-2-16-intra-only.webm The result of fluster test on SM8250: 133/147 testcases passed while testing JCT-VC-HEVC_V1 with GStreamer-H.265-V4L2-Gst1.0. The failing test case: - 10 testcases failed due to unsupported 10 bit format. - DBLK_A_MAIN10_VIXS_4 - INITQP_B_Main10_Sony_1 - TSUNEQBD_A_MAIN10_Technicolor_2 - WP_A_MAIN10_Toshiba_3 - WP_MAIN10_B_Toshiba_3 - WPP_A_ericsson_MAIN10_2 - WPP_B_ericsson_MAIN10_2 - WPP_C_ericsson_MAIN10_2 - WPP_E_ericsson_MAIN10_2 - WPP_F_ericsson_MAIN10_2 - 4 testcase failed due to unsupported resolution - PICSIZE_A_Bossen_1 - PICSIZE_B_Bossen_1 - WPP_D_ericsson_MAIN10_2 - WPP_D_ericsson_MAIN_2 232/305 testcases passed while testing VP9-TEST-VECTORS with GStreamer-VP9-V4L2-Gst1.0. The failing test case: - 64 testcases failed due to unsupported resolution - vp90-2-02-size-08x08.webm - vp90-2-02-size-08x10.webm - vp90-2-02-size-08x16.webm - vp90-2-02-size-08x18.webm - vp90-2-02-size-08x32.webm - vp90-2-02-size-08x34.webm - vp90-2-02-size-08x64.webm - vp90-2-02-size-08x66.webm - vp90-2-02-size-10x08.webm - vp90-2-02-size-10x10.webm - vp90-2-02-size-10x16.webm - vp90-2-02-size-10x18.webm - vp90-2-02-size-10x32.webm - vp90-2-02-size-10x34.webm - vp90-2-02-size-10x64.webm - vp90-2-02-size-10x66.webm - vp90-2-02-size-16x08.webm - vp90-2-02-size-16x10.webm - vp90-2-02-size-16x16.webm - vp90-2-02-size-16x18.webm - vp90-2-02-size-16x32.webm - vp90-2-02-size-16x34.webm - vp90-2-02-size-16x64.webm - vp90-2-02-size-16x66.webm - vp90-2-02-size-18x08.webm - vp90-2-02-size-18x10.webm - vp90-2-02-size-18x16.webm - vp90-2-02-size-18x18.webm - vp90-2-02-size-18x32.webm - vp90-2-02-size-18x34.webm - vp90-2-02-size-18x64.webm - vp90-2-02-size-18x66.webm - vp90-2-02-size-32x08.webm - vp90-2-02-size-32x10.webm - vp90-2-02-size-32x16.webm - vp90-2-02-size-32x18.webm - vp90-2-02-size-32x32.webm - vp90-2-02-size-32x34.webm - vp90-2-02-size-32x64.webm - vp90-2-02-size-32x66.webm - vp90-2-02-size-34x08.webm - vp90-2-02-size-34x10.webm - vp90-2-02-size-34x16.webm - vp90-2-02-size-34x18.webm - vp90-2-02-size-34x32.webm - vp90-2-02-size-34x34.webm - vp90-2-02-size-34x64.webm - vp90-2-02-size-34x66.webm - vp90-2-02-size-64x08.webm - vp90-2-02-size-64x10.webm - vp90-2-02-size-64x16.webm - vp90-2-02-size-64x18.webm - vp90-2-02-size-64x32.webm - vp90-2-02-size-64x34.webm - vp90-2-02-size-64x64.webm - vp90-2-02-size-64x66.webm - vp90-2-02-size-66x08.webm - vp90-2-02-size-66x10.webm - vp90-2-02-size-66x16.webm - vp90-2-02-size-66x18.webm - vp90-2-02-size-66x32.webm - vp90-2-02-size-66x34.webm - vp90-2-02-size-66x64.webm - vp90-2-02-size-66x66.webm - 2 testcases failed due to unsupported format - vp91-2-04-yuv422.webm - vp91-2-04-yuv444.webm - 1 testcase failed with CRC mismatch - vp90-2-22-svc_1280x720_3.ivf - Bug raised: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/4371 - 5 testcase failed due to unsupported resolution after sequence change - vp90-2-21-resize_inter_320x180_5_1-2.webm - vp90-2-21-resize_inter_320x180_7_1-2.webm - vp90-2-21-resize_inter_320x240_5_1-2.webm - vp90-2-21-resize_inter_320x240_7_1-2.webm - vp90-2-18-resize.ivf - 1 testcase failed with CRC mismatch - vp90-2-16-intra-only.webm Analysis: First few frames are marked by firmware as NO_SHOW frame. Driver make buf state to VB2_BUF_STATE_ERROR for such frames. Such buffers should be dropped by GST. But instead, the first frame is being displayed and when a valid buffer is sent to client later with same timestamp, its dropped, leading to CRC mismatch for first frame. Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> --- Dikshita Agarwal (23): media: iris: Skip destroying internal buffer if not dequeued media: iris: Update CAPTURE format info based on OUTPUT format media: iris: Add handling for corrupt and drop frames media: iris: Avoid updating frame size to firmware during reconfig media: iris: Send V4L2_BUF_FLAG_ERROR for buffers with 0 filled length media: iris: Drop port check for session property response media: iris: Add handling for no show frames media: iris: Improve last flag handling media: iris: Skip flush on first sequence change media: iris: Prevent HFI queue writes when core is in deinit state media: iris: Remove redundant buffer count check in stream off media: iris: Remove deprecated property setting to firmware media: iris: Fix missing function pointer initialization media: iris: Fix NULL pointer dereference media: iris: Fix typo in depth variable media: iris: Add a comment to explain usage of MBPS media: iris: Track flush responses to prevent premature completion media: iris: Fix buffer preparation failure during resolution change media: iris: Add HEVC and VP9 formats for decoder media: iris: Add platform capabilities for HEVC and VP9 decoders media: iris: Set mandatory properties for HEVC and VP9 decoders. media: iris: Add internal buffer calculation for HEVC and VP9 decoders media: iris: Add codec specific check for VP9 decoder drain handling drivers/media/platform/qcom/iris/iris_buffer.c | 52 ++- drivers/media/platform/qcom/iris/iris_buffer.h | 3 +- drivers/media/platform/qcom/iris/iris_ctrls.c | 35 +- drivers/media/platform/qcom/iris/iris_hfi_common.h | 1 + .../platform/qcom/iris/iris_hfi_gen1_command.c | 48 ++- .../platform/qcom/iris/iris_hfi_gen1_defines.h | 5 +- .../platform/qcom/iris/iris_hfi_gen1_response.c | 39 +- .../platform/qcom/iris/iris_hfi_gen2_command.c | 143 +++++++- .../platform/qcom/iris/iris_hfi_gen2_defines.h | 5 + .../platform/qcom/iris/iris_hfi_gen2_response.c | 58 ++- drivers/media/platform/qcom/iris/iris_hfi_queue.c | 2 +- drivers/media/platform/qcom/iris/iris_instance.h | 8 + .../platform/qcom/iris/iris_platform_common.h | 28 +- .../media/platform/qcom/iris/iris_platform_gen2.c | 198 ++++++++-- .../platform/qcom/iris/iris_platform_qcs8300.h | 126 +++++-- .../platform/qcom/iris/iris_platform_sm8250.c | 15 +- drivers/media/platform/qcom/iris/iris_vb2.c | 18 +- drivers/media/platform/qcom/iris/iris_vdec.c | 117 +++--- drivers/media/platform/qcom/iris/iris_vdec.h | 11 + drivers/media/platform/qcom/iris/iris_vidc.c | 9 +- drivers/media/platform/qcom/iris/iris_vpu_buffer.c | 397 ++++++++++++++++++++- drivers/media/platform/qcom/iris/iris_vpu_buffer.h | 46 ++- 22 files changed, 1154 insertions(+), 210 deletions(-) --- base-commit: 398a1b33f1479af35ca915c5efc9b00d6204f8fa change-id: 20250428-qcom-iris-hevc-vp9-eb31f30c3390 prerequisite-message-id: <20250417-topic-sm8x50-iris-v10-v7-0-f020cb1d0e98@linaro.org> prerequisite-patch-id: 35f8dae1416977e88c2db7c767800c01822e266e prerequisite-patch-id: 2bba98151ca103aa62a513a0fbd0df7ae64d9868 prerequisite-patch-id: 0e43a6d758b5fa5ab921c6aa3c19859e312b47d0 prerequisite-patch-id: b7b50aa1657be59fd51c3e53d73382a1ee75a08e prerequisite-patch-id: 30960743105a36f20b3ec4a9ff19e7bca04d6add prerequisite-patch-id: b93c37dc7e09d1631b75387dc1ca90e3066dce17 prerequisite-patch-id: afffe7096c8e110a8da08c987983bc4441d39578 prerequisite-message-id: <20250424-qcs8300_iris-v5-0-f118f505c300@quicinc.com> prerequisite-patch-id: 2e72fe4d11d264db3d42fa450427d30171303c6f prerequisite-patch-id: 3398937a7fabb45934bb98a530eef73252231132 prerequisite-patch-id: feda620f147ca14a958c92afdc85a1dc507701ac prerequisite-patch-id: 07ba0745c7d72796567e0a57f5c8e5355a8d2046 prerequisite-patch-id: e35b05c527217206ae871aef0d7b0261af0319ea Best regards,