From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomasz Figa Subject: Re: [PATCH v1 3/5] media: venus: do not destroy video session during queue setup Date: Wed, 14 Nov 2018 20:02:10 +0900 Message-ID: References: <1538222432-25894-1-git-send-email-sgorle@codeaurora.org> <1538222432-25894-4-git-send-email-sgorle@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Stanimir Varbanov Cc: sgorle@codeaurora.org, Hans Verkuil , Mauro Carvalho Chehab , Linux Media Mailing List , Linux Kernel Mailing List , linux-arm-msm , Alexandre Courbot , vgarodia@codeaurora.org List-Id: linux-arm-msm@vger.kernel.org On Fri, Nov 9, 2018 at 7:00 PM Stanimir Varbanov 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