Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200218184220.139656-1-jkardatzke@google.com>
In-Reply-To: <20200218184220.139656-1-jkardatzke@google.com>
From:   Jeffrey Kardatzke <jkardatzke@google.com>
Date:   Tue, 18 Feb 2020 11:14:28 -0800
Message-ID: <CA+ddPcOMpiHU=2bumCe93NzLe-uO04TnhCS+5CDDU4GyLXYCNA@mail.gmail.com>
Subject: Re: [PATCH v3] media: venus: add support for selection rectangles
To:     linux-media@vger.kernel.org
Cc:     Stanimir Varbanov <stanimir.varbanov@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
        Malathi Gottam <mgottam@codeaurora.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

(again, sorry for duplicate to some people, didn't know the plain text
email rule before)
There were a few comments made when this patch was originally posted
that were not addressed.  I left the patch as it last stood, except
for removing the unused variables.

The outstanding comments were:
1. In venc_init_session, it is only using the inst->width/height
rectangles and not the inst->out_width/out_height rectangles. So there
was a question about whether the OUTUPT rectangle should be set to
out_width/out_height. I'm looking for feedback here as I'm not
familiar enough with this driver code yet.
2. We should return EBUSY if the selection rectangles are changed
after we are initialized (I will update it to do this)
3. Support for non-zero top/left parameters. I'm suspicious that the
HFI_INDEX_EXTRADATA_INPUT_CROP property is what should be used for
that, but it's not currently used anywhere. Does anybody have details
on if that's what it's for?


On Tue, Feb 18, 2020 at 10:42 AM Jeffrey Kardatzke
<jkardatzke@google.com> wrote:
>
> From: Malathi Gottam <>
>
> Handles target type crop by setting the new active rectangle
> to hardware. The new rectangle should be within YUV size.
>
> This was taken from: https://lkml.org/lkml/2018/11/9/899
>
> Signed-off-by: Malathi Gottam <mgottam@codeaurora.org>
> Signed-off-by: Jeffrey Kardatzke <jkardatzke@google.com>
> ---
>  drivers/media/platform/qcom/venus/venc.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index 453edf966d4f..73b3181eed9a 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -479,10 +479,26 @@ venc_s_selection(struct file *file, void *fh, struct v4l2_selection *s)
>
>         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;
> --
> 2.25.0.265.gbab2e86ba0-goog
>