Re: [PATCH] media: venus: support frame rate control
From: Jeffrey Kardatzke
Date: Wed Feb 19 2020 - 11:37:20 EST
Got it, thanks. I'll submit a new patch with that.
On Tue, Feb 18, 2020 at 11:56 PM Stanimir Varbanov
> Hi Jeff,
> On 2/18/20 9:09 PM, Jeffrey Kardatzke wrote:
> > Sorry for the duplicate, accidentally used HTML format and it got
> > bounced from the mailing lists so resending.
> > On Mon, Feb 17, 2020 at 2:15 AM Stanimir Varbanov
> > <stanimir.varbanov@xxxxxxxxxx> wrote:
> >> Hi Jeff,
> >> Thanks for the patch!
> >> On 2/13/20 11:30 PM, Jeffrey Kardatzke wrote:
> >>> Frame rate control is always enabled in this driver, so make it silently
> >>> support the V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE.
> >>> Signed-off-by: Jeffrey Kardatzke <jkardatzke@xxxxxxxxxx>
> >>> ---
> >>> drivers/media/platform/qcom/venus/venc_ctrls.c | 6 ++++++
> >>> 1 file changed, 6 insertions(+)
> >>> diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c
> >>> index 877c0b3299e9..9ede692f77c5 100644
> >>> --- a/drivers/media/platform/qcom/venus/venc_ctrls.c
> >>> +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c
> >>> @@ -199,6 +199,9 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl)
> >>> }
> >>> mutex_unlock(&inst->lock);
> >>> break;
> >>> + case V4L2_CID_MPEG_VIDEO_FRAME_RC_ENABLE:
> >>> + // Silently ignore, it's always enabled.
> >> Please, use C comments and follow the kernel coding style.
> > OK, hopefully I've got that now. I didn't see any issues aside from
> > the comment style though.
> > I'll upload a new patch shortly.
> >> I wonder shouldn't it be better to add rc_enable field in struct
> >> venc_controls and give the user choice to disable the rate control? We
> >> can keep the default to be "enabled".
> > That'd be fine. Is there a way to actually disable the rate control though?
> The rate control property values are here , and  is where we set
> the control.