| Message ID | 1704867134-5522-1-git-send-email-quic_dikshita@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v2] media: venus: flush all buffers in output plane streamoff | expand |
On Tue, Jan 9, 2024 at 10:13 PM Dikshita Agarwal <quic_dikshita@quicinc.com> wrote: > > For scenarios, when source change is followed by VIDIOC_STREAMOFF > on output plane, driver should discard any queued OUTPUT > buffers, which are not decoded or dequeued. > Flush with HFI_FLUSH_INPUT does not have any actual impact. > So, fix it, by invoking HFI_FLUSH_ALL, which will flush all > queued buffers. > > Fixes: 85872f861d4c ("media: venus: Mark last capture buffer") > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> Tested on a SC7280 Chromebook with a custom client [0] to perform the queueing and VIDIOC_STREAMOFF sequence as described in the commit message. Before this patch, a buffer from before the VIDIOC_STREAMOFF command would be dequeued and seen by the client. With this patch, it is not seen by the client. [0]: https://crrev.com/c/5191249 Tested-by: Nathan Hebert <nhebert@chromium.org> > > --- > Changes since v1: > - Added fixes tag (Bryan) > > drivers/media/platform/qcom/venus/vdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c > index 29130a9..0d2ab95 100644 > --- a/drivers/media/platform/qcom/venus/vdec.c > +++ b/drivers/media/platform/qcom/venus/vdec.c > @@ -1255,7 +1255,7 @@ static int vdec_stop_output(struct venus_inst *inst) > break; > case VENUS_DEC_STATE_INIT: > case VENUS_DEC_STATE_CAPTURE_SETUP: > - ret = hfi_session_flush(inst, HFI_FLUSH_INPUT, true); > + ret = hfi_session_flush(inst, HFI_FLUSH_ALL, true); > break; > default: > break; > -- > 2.7.4 > >
Hi Bryan, I realized later that I missed to add your reviewed-by tag to this v2 patch, Sorry for this silly mistake, will be more careful in future. On 1/10/2024 11:42 AM, Dikshita Agarwal wrote: > For scenarios, when source change is followed by VIDIOC_STREAMOFF > on output plane, driver should discard any queued OUTPUT > buffers, which are not decoded or dequeued. > Flush with HFI_FLUSH_INPUT does not have any actual impact. > So, fix it, by invoking HFI_FLUSH_ALL, which will flush all > queued buffers. > > Fixes: 85872f861d4c ("media: venus: Mark last capture buffer") > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > Changes since v1: > - Added fixes tag (Bryan) > > drivers/media/platform/qcom/venus/vdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c > index 29130a9..0d2ab95 100644 > --- a/drivers/media/platform/qcom/venus/vdec.c > +++ b/drivers/media/platform/qcom/venus/vdec.c > @@ -1255,7 +1255,7 @@ static int vdec_stop_output(struct venus_inst *inst) > break; > case VENUS_DEC_STATE_INIT: > case VENUS_DEC_STATE_CAPTURE_SETUP: > - ret = hfi_session_flush(inst, HFI_FLUSH_INPUT, true); > + ret = hfi_session_flush(inst, HFI_FLUSH_ALL, true); > break; > default: > break;
Hello Dikshita, On 10.01.24 г. 8:12 ч., Dikshita Agarwal wrote: > For scenarios, when source change is followed by VIDIOC_STREAMOFF > on output plane, driver should discard any queued OUTPUT > buffers, which are not decoded or dequeued. > Flush with HFI_FLUSH_INPUT does not have any actual impact. > So, fix it, by invoking HFI_FLUSH_ALL, which will flush all > queued buffers. > > Fixes: 85872f861d4c ("media: venus: Mark last capture buffer") Cc: stable is missing with a kernel version. > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> > --- > Changes since v1: > - Added fixes tag (Bryan) > > drivers/media/platform/qcom/venus/vdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c > index 29130a9..0d2ab95 100644 > --- a/drivers/media/platform/qcom/venus/vdec.c > +++ b/drivers/media/platform/qcom/venus/vdec.c > @@ -1255,7 +1255,7 @@ static int vdec_stop_output(struct venus_inst *inst) > break; > case VENUS_DEC_STATE_INIT: > case VENUS_DEC_STATE_CAPTURE_SETUP: > - ret = hfi_session_flush(inst, HFI_FLUSH_INPUT, true); > + ret = hfi_session_flush(inst, HFI_FLUSH_ALL, true); Is it tested on msm8916? If I remember correctly FLUSH_ALL produces session error, could you confirm, please.
On 10/01/2024 07:12, Dikshita Agarwal wrote: > For scenarios, when source change is followed by VIDIOC_STREAMOFF > on output plane, driver should discard any queued OUTPUT > buffers, which are not decoded or dequeued. > Flush with HFI_FLUSH_INPUT does not have any actual impact. > So, fix it, by invoking HFI_FLUSH_ALL, which will flush all > queued buffers. > > Fixes: 85872f861d4c ("media: venus: Mark last capture buffer") > Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> This is a friendly reminder during the review process. It looks like you received a tag and forgot to add it. If you do not know the process, here is a short explanation: Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions, under or above your Signed-off-by tag. Tag is "received", when provided in a message replied to you on the mailing list. Tools like b4 can help here. However, there's no need to repost patches *only* to add the tags. The upstream maintainer will do that for tags received on the version they apply. https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577 If a tag was not added on purpose, please state why and what changed. Best regards, Krzysztof
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index 29130a9..0d2ab95 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -1255,7 +1255,7 @@ static int vdec_stop_output(struct venus_inst *inst) break; case VENUS_DEC_STATE_INIT: case VENUS_DEC_STATE_CAPTURE_SETUP: - ret = hfi_session_flush(inst, HFI_FLUSH_INPUT, true); + ret = hfi_session_flush(inst, HFI_FLUSH_ALL, true); break; default: break;
For scenarios, when source change is followed by VIDIOC_STREAMOFF on output plane, driver should discard any queued OUTPUT buffers, which are not decoded or dequeued. Flush with HFI_FLUSH_INPUT does not have any actual impact. So, fix it, by invoking HFI_FLUSH_ALL, which will flush all queued buffers. Fixes: 85872f861d4c ("media: venus: Mark last capture buffer") Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> --- Changes since v1: - Added fixes tag (Bryan) drivers/media/platform/qcom/venus/vdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)