| Message ID | 1649747356-3207-2-git-send-email-quic_vgarodia@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Venus fixes | expand |
Hi Vikash, Thanks for the patch! On 4/12/22 10:09, Vikash Garodia wrote: > During reconfig (DRC) event from firmware, it is not guaranteed that > all the DPB(internal) buffers would be released by the firmware. Some > buffers might be released gradually while processing frames from the > new sequence. These buffers now stay idle in the dpblist. > In subsequent call to queue the DPBs to firmware, these idle buffers > should not be queued. The fix identifies those buffers and free them. > --- > drivers/media/platform/qcom/venus/helpers.c | 37 ++++++++++++++++++++++------- > 1 file changed, 28 insertions(+), 9 deletions(-) > > diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c > index 0bca95d..0602f03 100644 > --- a/drivers/media/platform/qcom/venus/helpers.c > +++ b/drivers/media/platform/qcom/venus/helpers.c > @@ -90,12 +90,31 @@ bool venus_helper_check_codec(struct venus_inst *inst, u32 v4l2_pixfmt) > } > EXPORT_SYMBOL_GPL(venus_helper_check_codec); > > +static int venus_helper_free_dpb_buf(struct venus_inst *inst, struct intbuf *buf) This function should return void. Also, venus_helper_ prefix is not needed since if you follow the helpers.c style the prefix is only used for exported helper functions. I guess it could be: static void free_dpb_buf(struct venus_inst *inst, struct intbuf *buf) With this fixed: Acked-by: Stanimir Varbanov <stanimir.varbanov@linaro.org> > +{ > + ida_free(&inst->dpb_ids, buf->dpb_out_tag); > + > + list_del_init(&buf->list); > + dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da, > + buf->attrs); > + kfree(buf); > + > + return 0; > +} > + > int venus_helper_queue_dpb_bufs(struct venus_inst *inst) > { > - struct intbuf *buf; > + struct intbuf *buf, *next; > + unsigned int dpb_size = 0; > int ret = 0; > > - list_for_each_entry(buf, &inst->dpbbufs, list) { > + if (inst->dpb_buftype == HFI_BUFFER_OUTPUT) > + dpb_size = inst->output_buf_size; > + else if (inst->dpb_buftype == HFI_BUFFER_OUTPUT2) > + dpb_size = inst->output2_buf_size; > + > + > + list_for_each_entry_safe(buf, next, &inst->dpbbufs, list) { > struct hfi_frame_data fdata; > > memset(&fdata, 0, sizeof(fdata)); > @@ -106,6 +125,12 @@ int venus_helper_queue_dpb_bufs(struct venus_inst *inst) > if (buf->owned_by == FIRMWARE) > continue; > > + /* free buffer from previous sequence which was released later */ > + if (dpb_size > buf->size) { > + venus_helper_free_dpb_buf(inst, buf); > + continue; > + } > + > fdata.clnt_data = buf->dpb_out_tag; > > ret = hfi_session_process_buf(inst, &fdata); > @@ -127,13 +152,7 @@ int venus_helper_free_dpb_bufs(struct venus_inst *inst) > list_for_each_entry_safe(buf, n, &inst->dpbbufs, list) { > if (buf->owned_by == FIRMWARE) > continue; > - > - ida_free(&inst->dpb_ids, buf->dpb_out_tag); > - > - list_del_init(&buf->list); > - dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da, > - buf->attrs); > - kfree(buf); > + venus_helper_free_dpb_buf(inst, buf); > } > > if (list_empty(&inst->dpbbufs))
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c index 0bca95d..0602f03 100644 --- a/drivers/media/platform/qcom/venus/helpers.c +++ b/drivers/media/platform/qcom/venus/helpers.c @@ -90,12 +90,31 @@ bool venus_helper_check_codec(struct venus_inst *inst, u32 v4l2_pixfmt) } EXPORT_SYMBOL_GPL(venus_helper_check_codec); +static int venus_helper_free_dpb_buf(struct venus_inst *inst, struct intbuf *buf) +{ + ida_free(&inst->dpb_ids, buf->dpb_out_tag); + + list_del_init(&buf->list); + dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da, + buf->attrs); + kfree(buf); + + return 0; +} + int venus_helper_queue_dpb_bufs(struct venus_inst *inst) { - struct intbuf *buf; + struct intbuf *buf, *next; + unsigned int dpb_size = 0; int ret = 0; - list_for_each_entry(buf, &inst->dpbbufs, list) { + if (inst->dpb_buftype == HFI_BUFFER_OUTPUT) + dpb_size = inst->output_buf_size; + else if (inst->dpb_buftype == HFI_BUFFER_OUTPUT2) + dpb_size = inst->output2_buf_size; + + + list_for_each_entry_safe(buf, next, &inst->dpbbufs, list) { struct hfi_frame_data fdata; memset(&fdata, 0, sizeof(fdata)); @@ -106,6 +125,12 @@ int venus_helper_queue_dpb_bufs(struct venus_inst *inst) if (buf->owned_by == FIRMWARE) continue; + /* free buffer from previous sequence which was released later */ + if (dpb_size > buf->size) { + venus_helper_free_dpb_buf(inst, buf); + continue; + } + fdata.clnt_data = buf->dpb_out_tag; ret = hfi_session_process_buf(inst, &fdata); @@ -127,13 +152,7 @@ int venus_helper_free_dpb_bufs(struct venus_inst *inst) list_for_each_entry_safe(buf, n, &inst->dpbbufs, list) { if (buf->owned_by == FIRMWARE) continue; - - ida_free(&inst->dpb_ids, buf->dpb_out_tag); - - list_del_init(&buf->list); - dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da, - buf->attrs); - kfree(buf); + venus_helper_free_dpb_buf(inst, buf); } if (list_empty(&inst->dpbbufs))