| Message ID | 1538222432-25894-4-git-send-email-sgorle@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Venus - Decode reconfig sequence | expand |
Hi Srinu, On 9/29/18 3:00 PM, Srinu Gorle wrote: > - open and close video sessions for plane properties is incorrect. Could you rephrase this statement? I really don't understand what you mean. > - add check to ensure, same instance persist from driver open to close. This assumption is wrong. The v4l client can change the codec by SFMT without close the device node, in that case we have to destroy and create a new session with new codec. > > Signed-off-by: Srinu Gorle <sgorle@codeaurora.org> > --- > drivers/media/platform/qcom/venus/hfi.c | 3 +++ > drivers/media/platform/qcom/venus/vdec.c | 2 ++ > 2 files changed, 5 insertions(+) > > diff --git a/drivers/media/platform/qcom/venus/hfi.c b/drivers/media/platform/qcom/venus/hfi.c > index 36a4784..59c34ba 100644 > --- a/drivers/media/platform/qcom/venus/hfi.c > +++ b/drivers/media/platform/qcom/venus/hfi.c > @@ -207,6 +207,9 @@ int hfi_session_init(struct venus_inst *inst, u32 pixfmt) > const struct hfi_ops *ops = core->ops; > int ret; > > + if (inst->state >= INST_INIT && inst->state < INST_STOP) > + return 0; In fact you want to be able to call session_init multiple times but deinit the session only once? The hfi.c layer is designed to follow the states as they are expected by the firmware side, if you want to call session_init multiple times just make a wrapper in the vdec.c with reference counting. > + > inst->hfi_codec = to_codec_type(pixfmt); > reinit_completion(&inst->done); > > diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c > index afe3b36..0035cf2 100644 > --- a/drivers/media/platform/qcom/venus/vdec.c > +++ b/drivers/media/platform/qcom/venus/vdec.c > @@ -700,6 +700,8 @@ static int vdec_num_buffers(struct venus_inst *inst, unsigned int *in_num, > > *out_num = HFI_BUFREQ_COUNT_MIN(&bufreq, ver); > > + return 0; OK in present implementation I decided that the codec is settled when streamon on both queues is called (i.e. the final session_init is made in streamon). IMO the correct one is to init the session in reqbuf(output) and deinit session in reqbuf(output, count=0)? The problem I see when you skip session_deinit is that the codec cannot be changed without closing the video node. > + > deinit: > hfi_session_deinit(inst); > >
On Fri, Nov 9, 2018 at 7:00 PM Stanimir Varbanov <stanimir.varbanov@linaro.org> wrote: > > Hi Srinu, > > On 9/29/18 3:00 PM, Srinu Gorle wrote: > > - open and close video sessions for plane properties is incorrect. > > Could you rephrase this statement? I really don't understand what you mean. > > > - add check to ensure, same instance persist from driver open to close. > > This assumption is wrong. The v4l client can change the codec by SFMT > without close the device node, in that case we have to destroy and > create a new session with new codec. Right. [snip] > > + > > inst->hfi_codec = to_codec_type(pixfmt); > > reinit_completion(&inst->done); > > > > diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c > > index afe3b36..0035cf2 100644 > > --- a/drivers/media/platform/qcom/venus/vdec.c > > +++ b/drivers/media/platform/qcom/venus/vdec.c > > @@ -700,6 +700,8 @@ static int vdec_num_buffers(struct venus_inst *inst, unsigned int *in_num, > > > > *out_num = HFI_BUFREQ_COUNT_MIN(&bufreq, ver); > > > > + return 0; > > OK in present implementation I decided that the codec is settled when > streamon on both queues is called (i.e. the final session_init is made > in streamon). IMO the correct one is to init the session in > reqbuf(output) and deinit session in reqbuf(output, count=0)? > Depending on what you mean by "settled" that could be fine. Generally for initialization, one would typically use REQBUFS(OUTPUT, >0) (since it's not possible to change the format after the allocation) or STREAMON(OUTPUT) (to defer the hardware operations until really necessary). Since STREAMOFF(OUTPUT) is expected to just trigger a seek, termination should be done in REQBUFS(OUTPUT, 0) indeed and after that one should be able to switch to another codec (OUTPUT format). Best regards, Tomasz
diff --git a/drivers/media/platform/qcom/venus/hfi.c b/drivers/media/platform/qcom/venus/hfi.c index 36a4784..59c34ba 100644 --- a/drivers/media/platform/qcom/venus/hfi.c +++ b/drivers/media/platform/qcom/venus/hfi.c @@ -207,6 +207,9 @@ int hfi_session_init(struct venus_inst *inst, u32 pixfmt) const struct hfi_ops *ops = core->ops; int ret; + if (inst->state >= INST_INIT && inst->state < INST_STOP) + return 0; + inst->hfi_codec = to_codec_type(pixfmt); reinit_completion(&inst->done); diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index afe3b36..0035cf2 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -700,6 +700,8 @@ static int vdec_num_buffers(struct venus_inst *inst, unsigned int *in_num, *out_num = HFI_BUFREQ_COUNT_MIN(&bufreq, ver); + return 0; + deinit: hfi_session_deinit(inst);
- open and close video sessions for plane properties is incorrect. - add check to ensure, same instance persist from driver open to close. Signed-off-by: Srinu Gorle <sgorle@codeaurora.org> --- drivers/media/platform/qcom/venus/hfi.c | 3 +++ drivers/media/platform/qcom/venus/vdec.c | 2 ++ 2 files changed, 5 insertions(+)