* [PATCH v2] media: venus: add support for selection rectangles @ 2018-11-09 7:39 Malathi Gottam 2018-11-09 7:56 ` Tomasz Figa 0 siblings, 1 reply; 5+ messages in thread From: Malathi Gottam @ 2018-11-09 7:39 UTC (permalink / raw) To: stanimir.varbanov, hverkuil, mchehab Cc: linux-media, linux-kernel, linux-arm-msm, acourbot, vgarodia, mgottam Handles target type crop by setting the new active rectangle to hardware. The new rectangle should be within YUV size. Signed-off-by: Malathi Gottam <mgottam@codeaurora.org> --- drivers/media/platform/qcom/venus/venc.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c index ce85962..d26c129 100644 --- a/drivers/media/platform/qcom/venus/venc.c +++ b/drivers/media/platform/qcom/venus/venc.c @@ -478,16 +478,34 @@ static int venc_g_fmt(struct file *file, void *fh, struct v4l2_format *f) venc_s_selection(struct file *file, void *fh, struct v4l2_selection *s) { struct venus_inst *inst = to_inst(file); + int ret; + u32 buftype; if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) return -EINVAL; switch (s->target) { case V4L2_SEL_TGT_CROP: - if (s->r.width != inst->out_width || - s->r.height != inst->out_height || - s->r.top != 0 || s->r.left != 0) - return -EINVAL; + if (s->r.left != 0) { + s->r.width += s->r.left; + s->r.left = 0; + } + + if (s->r.top != 0) { + s->r.height += s->r.top; + s->r.top = 0; + } + + if (s->r.width > inst->width) + s->r.width = inst->width; + else + inst->width = s->r.width; + + if (s->r.height > inst->height) + s->r.height = inst->height; + else + inst->height = s->r.height; + break; default: return -EINVAL; -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] media: venus: add support for selection rectangles 2018-11-09 7:39 [PATCH v2] media: venus: add support for selection rectangles Malathi Gottam @ 2018-11-09 7:56 ` Tomasz Figa 2018-11-09 9:20 ` mgottam 0 siblings, 1 reply; 5+ messages in thread From: Tomasz Figa @ 2018-11-09 7:56 UTC (permalink / raw) To: mgottam Cc: Stanimir Varbanov, Hans Verkuil, Mauro Carvalho Chehab, Linux Media Mailing List, Linux Kernel Mailing List, linux-arm-msm, Alexandre Courbot, vgarodia Hi Malathi, On Fri, Nov 9, 2018 at 4:39 PM Malathi Gottam <mgottam@codeaurora.org> wrote: > > Handles target type crop by setting the new active rectangle > to hardware. The new rectangle should be within YUV size. > > Signed-off-by: Malathi Gottam <mgottam@codeaurora.org> > --- > drivers/media/platform/qcom/venus/venc.c | 26 ++++++++++++++++++++++---- > 1 file changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c > index ce85962..d26c129 100644 > --- a/drivers/media/platform/qcom/venus/venc.c > +++ b/drivers/media/platform/qcom/venus/venc.c > @@ -478,16 +478,34 @@ static int venc_g_fmt(struct file *file, void *fh, struct v4l2_format *f) > venc_s_selection(struct file *file, void *fh, struct v4l2_selection *s) > { > struct venus_inst *inst = to_inst(file); > + int ret; > + u32 buftype; > > if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) > return -EINVAL; > > switch (s->target) { > case V4L2_SEL_TGT_CROP: > - if (s->r.width != inst->out_width || > - s->r.height != inst->out_height || > - s->r.top != 0 || s->r.left != 0) > - return -EINVAL; > + if (s->r.left != 0) { > + s->r.width += s->r.left; > + s->r.left = 0; > + } > + > + if (s->r.top != 0) { > + s->r.height += s->r.top; > + s->r.top = 0; > + } > + > + if (s->r.width > inst->width) > + s->r.width = inst->width; > + else > + inst->width = s->r.width; > + > + if (s->r.height > inst->height) > + s->r.height = inst->height; > + else > + inst->height = s->r.height; > + From semantic point of view, it looks fine, but where is the rectangle actually set to the hardware? Best regards, Tomasz ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] media: venus: add support for selection rectangles 2018-11-09 7:56 ` Tomasz Figa @ 2018-11-09 9:20 ` mgottam 2018-11-09 10:31 ` Tomasz Figa 0 siblings, 1 reply; 5+ messages in thread From: mgottam @ 2018-11-09 9:20 UTC (permalink / raw) To: Tomasz Figa Cc: Stanimir Varbanov, Hans Verkuil, Mauro Carvalho Chehab, Linux Media Mailing List, Linux Kernel Mailing List, linux-arm-msm, Alexandre Courbot, vgarodia On 2018-11-09 07:56, Tomasz Figa wrote: > Hi Malathi, > > On Fri, Nov 9, 2018 at 4:39 PM Malathi Gottam <mgottam@codeaurora.org> > wrote: >> >> Handles target type crop by setting the new active rectangle >> to hardware. The new rectangle should be within YUV size. >> >> Signed-off-by: Malathi Gottam <mgottam@codeaurora.org> >> --- >> drivers/media/platform/qcom/venus/venc.c | 26 >> ++++++++++++++++++++++---- >> 1 file changed, 22 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/venus/venc.c >> b/drivers/media/platform/qcom/venus/venc.c >> index ce85962..d26c129 100644 >> --- a/drivers/media/platform/qcom/venus/venc.c >> +++ b/drivers/media/platform/qcom/venus/venc.c >> @@ -478,16 +478,34 @@ static int venc_g_fmt(struct file *file, void >> *fh, struct v4l2_format *f) >> venc_s_selection(struct file *file, void *fh, struct v4l2_selection >> *s) >> { >> struct venus_inst *inst = to_inst(file); >> + int ret; >> + u32 buftype; >> >> if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) >> return -EINVAL; >> >> switch (s->target) { >> case V4L2_SEL_TGT_CROP: >> - if (s->r.width != inst->out_width || >> - s->r.height != inst->out_height || >> - s->r.top != 0 || s->r.left != 0) >> - return -EINVAL; >> + if (s->r.left != 0) { >> + s->r.width += s->r.left; >> + s->r.left = 0; >> + } >> + >> + if (s->r.top != 0) { >> + s->r.height += s->r.top; >> + s->r.top = 0; >> + } >> + >> + if (s->r.width > inst->width) >> + s->r.width = inst->width; >> + else >> + inst->width = s->r.width; >> + >> + if (s->r.height > inst->height) >> + s->r.height = inst->height; >> + else >> + inst->height = s->r.height; >> + > > From semantic point of view, it looks fine, but where is the rectangle > actually set to the hardware? > > Best regards, > Tomasz As this set selection call occurs before the hfi session initialization, for now we are holding these values in driver. As this call is followed by VIDIOC_REQBUFS(), as a part of this we have venc_init_session static int venc_init_session(struct venus_inst *inst) { int ret; ret = hfi_session_init(inst, inst->fmt_cap->pixfmt); if (ret) return ret; ret = venus_helper_set_input_resolution(inst, inst->width, inst->height); if (ret) goto deinit; ret = venus_helper_set_output_resolution(inst, inst->width, inst->height, HFI_BUFFER_OUTPUT); if (ret) goto deinit; ret = venus_helper_set_color_format(inst, inst->fmt_out->pixfmt); if (ret) goto deinit; ret = venc_set_properties(inst); From here we set these values to hardware. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] media: venus: add support for selection rectangles 2018-11-09 9:20 ` mgottam @ 2018-11-09 10:31 ` Tomasz Figa 2018-11-12 8:35 ` Alexandre Courbot 0 siblings, 1 reply; 5+ messages in thread From: Tomasz Figa @ 2018-11-09 10:31 UTC (permalink / raw) To: mgottam Cc: Stanimir Varbanov, Hans Verkuil, Mauro Carvalho Chehab, Linux Media Mailing List, Linux Kernel Mailing List, linux-arm-msm, Alexandre Courbot, vgarodia Hi Malathi, On Fri, Nov 9, 2018 at 6:20 PM <mgottam@codeaurora.org> wrote: > > On 2018-11-09 07:56, Tomasz Figa wrote: > > Hi Malathi, > > > > On Fri, Nov 9, 2018 at 4:39 PM Malathi Gottam <mgottam@codeaurora.org> > > wrote: > >> > >> Handles target type crop by setting the new active rectangle > >> to hardware. The new rectangle should be within YUV size. > >> > >> Signed-off-by: Malathi Gottam <mgottam@codeaurora.org> > >> --- > >> drivers/media/platform/qcom/venus/venc.c | 26 > >> ++++++++++++++++++++++---- > >> 1 file changed, 22 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/media/platform/qcom/venus/venc.c > >> b/drivers/media/platform/qcom/venus/venc.c > >> index ce85962..d26c129 100644 > >> --- a/drivers/media/platform/qcom/venus/venc.c > >> +++ b/drivers/media/platform/qcom/venus/venc.c > >> @@ -478,16 +478,34 @@ static int venc_g_fmt(struct file *file, void > >> *fh, struct v4l2_format *f) > >> venc_s_selection(struct file *file, void *fh, struct v4l2_selection > >> *s) > >> { > >> struct venus_inst *inst = to_inst(file); > >> + int ret; > >> + u32 buftype; > >> > >> if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) > >> return -EINVAL; > >> > >> switch (s->target) { > >> case V4L2_SEL_TGT_CROP: > >> - if (s->r.width != inst->out_width || > >> - s->r.height != inst->out_height || > >> - s->r.top != 0 || s->r.left != 0) > >> - return -EINVAL; > >> + if (s->r.left != 0) { > >> + s->r.width += s->r.left; > >> + s->r.left = 0; > >> + } > >> + > >> + if (s->r.top != 0) { > >> + s->r.height += s->r.top; > >> + s->r.top = 0; > >> + } > >> + > >> + if (s->r.width > inst->width) > >> + s->r.width = inst->width; > >> + else > >> + inst->width = s->r.width; > >> + > >> + if (s->r.height > inst->height) > >> + s->r.height = inst->height; > >> + else > >> + inst->height = s->r.height; > >> + > > > > From semantic point of view, it looks fine, but where is the rectangle > > actually set to the hardware? > > > > Best regards, > > Tomasz > > As this set selection call occurs before the hfi session initialization, > for now we are holding these values in driver. > > As this call is followed by VIDIOC_REQBUFS(), as a part of this > we have venc_init_session > > static int venc_init_session(struct venus_inst *inst) > { > int ret; > > ret = hfi_session_init(inst, inst->fmt_cap->pixfmt); > if (ret) > return ret; > > ret = venus_helper_set_input_resolution(inst, inst->width, > inst->height); > if (ret) > goto deinit; > > ret = venus_helper_set_output_resolution(inst, inst->width, > inst->height, > HFI_BUFFER_OUTPUT); Something sounds not right here. Shouldn't one of the width/height be the OUPUT format and the other the selection CROP target rectangle? > if (ret) > goto deinit; > > ret = venus_helper_set_color_format(inst, inst->fmt_out->pixfmt); > if (ret) > goto deinit; > > ret = venc_set_properties(inst); > > > From here we set these values to hardware. Okay, thanks for the explanation. In this case, we must return -EBUSY if selection is attempted to be set after the session is initialized. Best regards, Tomasz ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] media: venus: add support for selection rectangles 2018-11-09 10:31 ` Tomasz Figa @ 2018-11-12 8:35 ` Alexandre Courbot 0 siblings, 0 replies; 5+ messages in thread From: Alexandre Courbot @ 2018-11-12 8:35 UTC (permalink / raw) To: Tomasz Figa Cc: mgottam, Stanimir Varbanov, Hans Verkuil, Mauro Carvalho Chehab, Linux Media Mailing List, LKML, linux-arm-msm, vgarodia On Fri, Nov 9, 2018 at 7:31 PM Tomasz Figa <tfiga@chromium.org> wrote: > > Hi Malathi, > > On Fri, Nov 9, 2018 at 6:20 PM <mgottam@codeaurora.org> wrote: > > > > On 2018-11-09 07:56, Tomasz Figa wrote: > > > Hi Malathi, > > > > > > On Fri, Nov 9, 2018 at 4:39 PM Malathi Gottam <mgottam@codeaurora.org> > > > wrote: > > >> > > >> Handles target type crop by setting the new active rectangle > > >> to hardware. The new rectangle should be within YUV size. > > >> > > >> Signed-off-by: Malathi Gottam <mgottam@codeaurora.org> > > >> --- > > >> drivers/media/platform/qcom/venus/venc.c | 26 > > >> ++++++++++++++++++++++---- > > >> 1 file changed, 22 insertions(+), 4 deletions(-) > > >> > > >> diff --git a/drivers/media/platform/qcom/venus/venc.c > > >> b/drivers/media/platform/qcom/venus/venc.c > > >> index ce85962..d26c129 100644 > > >> --- a/drivers/media/platform/qcom/venus/venc.c > > >> +++ b/drivers/media/platform/qcom/venus/venc.c > > >> @@ -478,16 +478,34 @@ static int venc_g_fmt(struct file *file, void > > >> *fh, struct v4l2_format *f) > > >> venc_s_selection(struct file *file, void *fh, struct v4l2_selection > > >> *s) > > >> { > > >> struct venus_inst *inst = to_inst(file); > > >> + int ret; > > >> + u32 buftype; Looks like these two variables are not used anymore? > > >> > > >> if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) > > >> return -EINVAL; > > >> > > >> switch (s->target) { > > >> case V4L2_SEL_TGT_CROP: > > >> - if (s->r.width != inst->out_width || > > >> - s->r.height != inst->out_height || > > >> - s->r.top != 0 || s->r.left != 0) > > >> - return -EINVAL; > > >> + if (s->r.left != 0) { > > >> + s->r.width += s->r.left; > > >> + s->r.left = 0; > > >> + } > > >> + > > >> + if (s->r.top != 0) { > > >> + s->r.height += s->r.top; > > >> + s->r.top = 0; > > >> + } > > >> + > > >> + if (s->r.width > inst->width) > > >> + s->r.width = inst->width; > > >> + else > > >> + inst->width = s->r.width; > > >> + > > >> + if (s->r.height > inst->height) > > >> + s->r.height = inst->height; > > >> + else > > >> + inst->height = s->r.height; > > >> + > > > > > > From semantic point of view, it looks fine, but where is the rectangle > > > actually set to the hardware? > > > > > > Best regards, > > > Tomasz > > > > As this set selection call occurs before the hfi session initialization, > > for now we are holding these values in driver. > > > > As this call is followed by VIDIOC_REQBUFS(), as a part of this > > we have venc_init_session > > > > static int venc_init_session(struct venus_inst *inst) > > { > > int ret; > > > > ret = hfi_session_init(inst, inst->fmt_cap->pixfmt); > > if (ret) > > return ret; > > > > ret = venus_helper_set_input_resolution(inst, inst->width, > > inst->height); > > if (ret) > > goto deinit; > > > > ret = venus_helper_set_output_resolution(inst, inst->width, > > inst->height, > > HFI_BUFFER_OUTPUT); > > Something sounds not right here. Shouldn't one of the width/height be > the OUPUT format and the other the selection CROP target rectangle? Yeah, I don't see where the stride of the input (from HFI perspective) buffer is set, so unless I missed something that would not capture the correct part of the buffer. Also doesn't Venus support non-zero left and top parameters for the selection rectangle? ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-12 8:35 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-09 7:39 [PATCH v2] media: venus: add support for selection rectangles Malathi Gottam 2018-11-09 7:56 ` Tomasz Figa 2018-11-09 9:20 ` mgottam 2018-11-09 10:31 ` Tomasz Figa 2018-11-12 8:35 ` Alexandre Courbot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).