Hi Niklas,
Thank you for the patch.
On Friday, 8 December 2017 03:08:23 EET Niklas Söderlund wrote:
> The logic to preserve the requested format width and height are too
> complex and come from a premature optimization for Gen3. All Gen2 SoC
> can scale and the Gen3 implementation will not use these functions at
> all so simply preserve the width and height when interacting with the
> subdevice much like the field is preserved simplifies the logic quite a
> bit.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> Reviewed-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
> drivers/media/platform/rcar-vin/rcar-dma.c | 8 --------
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 22 ++++++++++------------
> drivers/media/platform/rcar-vin/rcar-vin.h | 2 --
> 3 files changed, 10 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c index
> a7cda3922cb74baa..fd14be20a6604d7a 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -585,14 +585,6 @@ void rvin_crop_scale_comp(struct rvin_dev *vin)
> 0, 0);
> }
>
> -void rvin_scale_try(struct rvin_dev *vin, struct v4l2_pix_format *pix,
> - u32 width, u32 height)
> -{
> - /* All VIN channels on Gen2 have scalers */
> - pix->width = width;
> - pix->height = height;
> -}
> -
> /*
> ---------------------------------------------------------------------------
> -- * Hardware setup
> */
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> 19de99133f048960..1c5e7f6d5b963740 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -166,6 +166,7 @@ static int __rvin_try_format_source(struct rvin_dev
> *vin, .which = which,
> };
> enum v4l2_field field;
> + u32 width, height;
> int ret;
>
> sd = vin_to_source(vin);
> @@ -178,7 +179,10 @@ static int __rvin_try_format_source(struct rvin_dev
> *vin,
>
> format.pad = vin->digital->source_pad;
>
> + /* Allow the video device to override field and to scale */
> field = pix->field;
> + width = pix->width;
> + height = pix->height;
>
> ret = v4l2_subdev_call(sd, pad, set_fmt, pad_cfg, &format);
> if (ret < 0 && ret != -ENOIOCTLCMD)
> @@ -191,6 +195,9 @@ static int __rvin_try_format_source(struct rvin_dev
> *vin, source->width = pix->width;
> source->height = pix->height;
>
I would move the pix->field = field line not shown above to here.
Apart from that,
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> + pix->width = width;
> + pix->height = height;
> +
> vin_dbg(vin, "Source resolution: %ux%u\n", source->width,
> source->height);
>
> @@ -204,13 +211,9 @@ static int __rvin_try_format(struct rvin_dev *vin,
> struct v4l2_pix_format *pix,
> struct rvin_source_fmt *source)
> {
> - u32 rwidth, rheight, walign;
> + u32 walign;
> int ret;
>
> - /* Requested */
> - rwidth = pix->width;
> - rheight = pix->height;
> -
> /* Keep current field if no specific one is asked for */
> if (pix->field == V4L2_FIELD_ANY)
> pix->field = vin->format.field;
> @@ -248,10 +251,6 @@ static int __rvin_try_format(struct rvin_dev *vin,
> break;
> }
>
> - /* If source can't match format try if VIN can scale */
> - if (source->width != rwidth || source->height != rheight)
> - rvin_scale_try(vin, pix, rwidth, rheight);
> -
> /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */
> walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
>
> @@ -270,9 +269,8 @@ static int __rvin_try_format(struct rvin_dev *vin,
> return -EINVAL;
> }
>
> - vin_dbg(vin, "Requested %ux%u Got %ux%u bpl: %d size: %d\n",
> - rwidth, rheight, pix->width, pix->height,
> - pix->bytesperline, pix->sizeimage);
> + vin_dbg(vin, "Format %ux%u bpl: %d size: %d\n",
> + pix->width, pix->height, pix->bytesperline, pix->sizeimage);
>
> return 0;
> }
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> b/drivers/media/platform/rcar-vin/rcar-vin.h index
> 646f897f5c05ec4e..36d0f0cc4ce01a6e 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -176,8 +176,6 @@ void rvin_v4l2_unregister(struct rvin_dev *vin);
> const struct rvin_video_format *rvin_format_from_pixel(u32 pixelformat);
>
> /* Cropping, composing and scaling */
> -void rvin_scale_try(struct rvin_dev *vin, struct v4l2_pix_format *pix,
> - u32 width, u32 height);
> void rvin_crop_scale_comp(struct rvin_dev *vin);
>
> #endif
--
Regards,
Laurent Pinchart
