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@xxxxxxxxxxxxxx> > --- > 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); > > -- regards, Stan
