From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stanimir Varbanov Subject: Re: [PATCH] media: venus: add support for key frame Date: Fri, 12 Oct 2018 11:10:01 +0300 Message-ID: <40d15ea4-48e2-b2c7-1d70-68dcc1b08990@linaro.org> References: <1539071634-1644-1-git-send-email-mgottam@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Alexandre Courbot Cc: mgottam@codeaurora.org, Hans Verkuil , Mauro Carvalho Chehab , Linux Media Mailing List , LKML , linux-arm-msm@vger.kernel.org, vgarodia@codeaurora.org List-Id: linux-arm-msm@vger.kernel.org On 10/12/2018 11:06 AM, Alexandre Courbot wrote: > On Fri, Oct 12, 2018 at 4:37 PM Stanimir Varbanov > wrote: >> >> Hi Alex, >> >> On 10/12/2018 08:26 AM, Alexandre Courbot wrote: >>> On Tue, Oct 9, 2018 at 4:54 PM Malathi Gottam wrote: >>>> >>>> When client requests for a keyframe, set the property >>>> to hardware to generate the sync frame. >>>> >>>> Signed-off-by: Malathi Gottam >>>> --- >>>> drivers/media/platform/qcom/venus/venc_ctrls.c | 13 +++++++++++++ >>>> 1 file changed, 13 insertions(+) >>>> >>>> diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c >>>> index 45910172..f332c8e 100644 >>>> --- a/drivers/media/platform/qcom/venus/venc_ctrls.c >>>> +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c >>>> @@ -81,6 +81,8 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) >>>> struct venc_controls *ctr = &inst->controls.enc; >>>> u32 bframes; >>>> int ret; >>>> + void *ptr; >>>> + u32 ptype; >>>> >>>> switch (ctrl->id) { >>>> case V4L2_CID_MPEG_VIDEO_BITRATE_MODE: >>>> @@ -173,6 +175,14 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) >>>> >>>> ctr->num_b_frames = bframes; >>>> break; >>>> + case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME: >>>> + ptype = HFI_PROPERTY_CONFIG_VENC_REQUEST_SYNC_FRAME; >>>> + ret = hfi_session_set_property(inst, ptype, ptr); >>> >>> The test bot already said it, but ptr is passed to >>> hfi_session_set_property() uninitialized. And as can be expected the >>> call returns -EINVAL on my board. >>> >>> Looking at other uses of HFI_PROPERTY_CONFIG_VENC_REQUEST_SYNC_FRAME I >>> see that the packet sent to the firmware does not have room for an >>> argument, so I tried to pass NULL but got the same result. >> >> yes, because pdata cannot be NULL. I'd suggest to make a pointer to >> struct hfi_enable and pass it to the set_property function. > > FWIW I also tried doing this and got the same error, strange... > OK, when you calling the v4l control? It makes sense when you calling it, because set_property checks does the session is on START state (i.e. streamon on both queues). -- regards, Stan