Hi Alex, On 10/12/2018 08:26 AM, Alexandre Courbot wrote: > On Tue, Oct 9, 2018 at 4:54 PM Malathi Gottam <mgottam@xxxxxxxxxxxxxx> wrote: >> >> When client requests for a keyframe, set the property >> to hardware to generate the sync frame. >> >> Signed-off-by: Malathi Gottam <mgottam@xxxxxxxxxxxxxx> >> --- >> 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. -- regards, Stan