| Message ID | 20250502-qcom-iris-hevc-vp9-v3-1-552158a10a7d@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,01/23] media: iris: Skip destroying internal buffer if not dequeued | expand |
On 01/05/2025 20:13, 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") > Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com> > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- > drivers/media/platform/qcom/iris/iris_buffer.c | 20 +++++++++++++++- > 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 | 33 ++++++++++++++++++++++++-- > 4 files changed, 54 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..981fedb000ed 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) > +static 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; > @@ -405,6 +413,16 @@ int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) > return 0; > } > > +int iris_destroy_all_internal_buffers(struct iris_inst *inst, u32 plane) > +{ > + return iris_destroy_internal_buffers(inst, plane, true); > +} > + > +int iris_destroy_dequeued_internal_buffers(struct iris_inst *inst, u32 plane) > +{ > + return iris_destroy_internal_buffers(inst, plane, false); > +} > + > static int iris_release_internal_buffers(struct iris_inst *inst, > enum iris_buffer_type buffer_type) > { > diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h > index c36b6347b077..00825ad2dc3a 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_all_internal_buffers(struct iris_inst *inst, u32 plane); > +int iris_destroy_dequeued_internal_buffers(struct iris_inst *inst, u32 plane); > 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..9c049b9671cc 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_dequeued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); > 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_dequeued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > 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..a8144595cc78 100644 > --- a/drivers/media/platform/qcom/iris/iris_vidc.c > +++ b/drivers/media/platform/qcom/iris/iris_vidc.c > @@ -221,6 +221,33 @@ static void iris_session_close(struct iris_inst *inst) > iris_wait_for_session_response(inst, false); > } > > +static void iris_check_num_queued_internal_buffers(struct iris_inst *inst, u32 plane) > +{ > + 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_close(struct file *filp) > { > struct iris_inst *inst = iris_get_inst(filp, NULL); > @@ -233,8 +260,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_all_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); > + iris_destroy_all_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); > + iris_check_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); > + iris_check_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); > I left some comments in the previous cycle https://lore.kernel.org/linux-arm-msm/a056266e-612d-4abf-916f-3db49b00dbde@linaro.org I don't see those addressed in this cycle. Can you give some feedback from your POV. --- bod
diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c index e5c5a564fcb8..981fedb000ed 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) +static 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; @@ -405,6 +413,16 @@ int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) return 0; } +int iris_destroy_all_internal_buffers(struct iris_inst *inst, u32 plane) +{ + return iris_destroy_internal_buffers(inst, plane, true); +} + +int iris_destroy_dequeued_internal_buffers(struct iris_inst *inst, u32 plane) +{ + return iris_destroy_internal_buffers(inst, plane, false); +} + static int iris_release_internal_buffers(struct iris_inst *inst, enum iris_buffer_type buffer_type) { diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h index c36b6347b077..00825ad2dc3a 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_all_internal_buffers(struct iris_inst *inst, u32 plane); +int iris_destroy_dequeued_internal_buffers(struct iris_inst *inst, u32 plane); 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..9c049b9671cc 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_dequeued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); 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_dequeued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); 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..a8144595cc78 100644 --- a/drivers/media/platform/qcom/iris/iris_vidc.c +++ b/drivers/media/platform/qcom/iris/iris_vidc.c @@ -221,6 +221,33 @@ static void iris_session_close(struct iris_inst *inst) iris_wait_for_session_response(inst, false); } +static void iris_check_num_queued_internal_buffers(struct iris_inst *inst, u32 plane) +{ + 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_close(struct file *filp) { struct iris_inst *inst = iris_get_inst(filp, NULL); @@ -233,8 +260,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_all_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); + iris_destroy_all_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); + iris_check_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); + iris_check_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);
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") Reviewed-by: Vikash Garodia <quic_vgarodia@quicinc.com> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> --- drivers/media/platform/qcom/iris/iris_buffer.c | 20 +++++++++++++++- 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 | 33 ++++++++++++++++++++++++-- 4 files changed, 54 insertions(+), 6 deletions(-)