subject:"\[PATCH v11 16\/32\] rcar\-vin\: read subdevice format for crop only when needed"

Re: [PATCH v11 16/32] rcar-vin: read subdevice format for crop only when needed

2018-03-03 Thread Niklas Söderlund

Hi Laurent,

Thanks for your feedback.

On 2018-03-02 13:06:27 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Friday, 2 March 2018 03:57:35 EET Niklas Söderlund wrote:
> > Instead of caching the subdevice format each time the video device
> > format is set read it directly when it's needed. As it turns out the
> > format is only needed when figuring out the max rectangle for cropping.
> > 
> > This simplifies the code and makes it clearer what the source format is
> > used for.
> > 
> > Signed-off-by: Niklas Söderlund 
> > ---
> >  drivers/media/platform/rcar-vin/rcar-v4l2.c | 158 +
> >  drivers/media/platform/rcar-vin/rcar-vin.h  |  12 ---
> >  2 files changed, 80 insertions(+), 90 deletions(-)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> > 3290e603b44cdf3a..55640c6b2a1200ca 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > @@ -144,67 +144,62 @@ static int rvin_format_align(struct rvin_dev *vin,
> > struct v4l2_pix_format *pix) * V4L2
> >   */
> > 
> > -static void rvin_reset_crop_compose(struct rvin_dev *vin)
> > +static int rvin_get_vin_format_from_source(struct rvin_dev *vin,
> > +  struct v4l2_pix_format *pix)
> >  {
> > +   struct v4l2_subdev_format fmt = {
> > +   .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > +   .pad = vin->digital->source_pad,
> > +   };
> > +   int ret;
> > +
> > +   ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, );
> > +   if (ret)
> > +   return ret;
> > +
> > +   v4l2_fill_pix_format(pix, );
> > +
> > +   return rvin_format_align(vin, pix);
> > +}
> > +
> > +static int rvin_reset_format(struct rvin_dev *vin)
> > +{
> > +   int ret;
> > +
> > +   ret = rvin_get_vin_format_from_source(vin, >format);
> > +   if (ret)
> > +   return ret;
> > +
> > vin->crop.top = vin->crop.left = 0;
> > -   vin->crop.width = vin->source.width;
> > -   vin->crop.height = vin->source.height;
> > +   vin->crop.width = vin->format.width;
> > +   vin->crop.height = vin->format.height;
> > 
> > vin->compose.top = vin->compose.left = 0;
> > vin->compose.width = vin->format.width;
> > vin->compose.height = vin->format.height;
> > -}
> > -
> > -static int rvin_reset_format(struct rvin_dev *vin)
> > -{
> > -   struct v4l2_subdev_format fmt = {
> > -   .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > -   };
> > -   struct v4l2_mbus_framefmt *mf = 
> > -   int ret;
> > -
> > -   fmt.pad = vin->digital->source_pad;
> > -
> > -   ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, );
> > -   if (ret)
> > -   return ret;
> > -
> > -   vin->format.width   = mf->width;
> > -   vin->format.height  = mf->height;
> > -   vin->format.colorspace  = mf->colorspace;
> > -   vin->format.field   = mf->field;
> > -
> > -   rvin_reset_crop_compose(vin);
> > -
> > -   vin->format.bytesperline = rvin_format_bytesperline(>format);
> > -   vin->format.sizeimage = rvin_format_sizeimage(>format);
> > 
> > return 0;
> >  }
> > 
> > -static int __rvin_try_format_source(struct rvin_dev *vin,
> > -   u32 which,
> > -   struct v4l2_pix_format *pix,
> > -   struct rvin_source_fmt *source)
> > +static int rvin_try_format(struct rvin_dev *vin, u32 which,
> > +  struct v4l2_pix_format *pix,
> > +  struct v4l2_rect *crop, struct v4l2_rect *compose)
> >  {
> > -   struct v4l2_subdev *sd;
> > +   struct v4l2_subdev *sd = vin_to_source(vin);
> > struct v4l2_subdev_pad_config *pad_cfg;
> > struct v4l2_subdev_format format = {
> > .which = which,
> > +   .pad = vin->digital->source_pad,
> > };
> > enum v4l2_field field;
> > u32 width, height;
> > int ret;
> > 
> > -   sd = vin_to_source(vin);
> > -
> > -   v4l2_fill_mbus_format(, pix, vin->digital->code);
> > -
> > pad_cfg = v4l2_subdev_alloc_pad_config(sd);
> > if (pad_cfg == NULL)
> > return -ENOMEM;
> > 
> > -   format.pad = vin->digital->source_pad;
> > +   v4l2_fill_mbus_format(, pix, vin->digital->code);
> > 
> > /* Allow the video device to override field and to scale */
> > field = pix->field;
> > @@ -217,34 +212,34 @@ static int __rvin_try_format_source(struct rvin_dev
> > *vin,
> > 
> > v4l2_fill_pix_format(pix, );
> > 
> > -   source->width = pix->width;
> > -   source->height = pix->height;
> > +   crop->top = crop->left = 0;
> > +   crop->width = pix->width;
> > +   crop->height = pix->height;
> > +
> > +   /*
> > +* If source is ALTERNATE the driver will use the VIN hardware
> > +* to INTERLACE it. The crop height then needs to be doubled.
> > +*/
> > +   if (pix->field == V4L2_FIELD_ALTERNATE)
> > +

Re: [PATCH v11 16/32] rcar-vin: read subdevice format for crop only when needed

2018-03-02 Thread Laurent Pinchart

Hi Niklas,

Thank you for the patch.

On Friday, 2 March 2018 03:57:35 EET Niklas Söderlund wrote:
> Instead of caching the subdevice format each time the video device
> format is set read it directly when it's needed. As it turns out the
> format is only needed when figuring out the max rectangle for cropping.
> 
> This simplifies the code and makes it clearer what the source format is
> used for.
> 
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 158 +
>  drivers/media/platform/rcar-vin/rcar-vin.h  |  12 ---
>  2 files changed, 80 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index
> 3290e603b44cdf3a..55640c6b2a1200ca 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -144,67 +144,62 @@ static int rvin_format_align(struct rvin_dev *vin,
> struct v4l2_pix_format *pix) * V4L2
>   */
> 
> -static void rvin_reset_crop_compose(struct rvin_dev *vin)
> +static int rvin_get_vin_format_from_source(struct rvin_dev *vin,
> +struct v4l2_pix_format *pix)
>  {
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + .pad = vin->digital->source_pad,
> + };
> + int ret;
> +
> + ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, );
> + if (ret)
> + return ret;
> +
> + v4l2_fill_pix_format(pix, );
> +
> + return rvin_format_align(vin, pix);
> +}
> +
> +static int rvin_reset_format(struct rvin_dev *vin)
> +{
> + int ret;
> +
> + ret = rvin_get_vin_format_from_source(vin, >format);
> + if (ret)
> + return ret;
> +
>   vin->crop.top = vin->crop.left = 0;
> - vin->crop.width = vin->source.width;
> - vin->crop.height = vin->source.height;
> + vin->crop.width = vin->format.width;
> + vin->crop.height = vin->format.height;
> 
>   vin->compose.top = vin->compose.left = 0;
>   vin->compose.width = vin->format.width;
>   vin->compose.height = vin->format.height;
> -}
> -
> -static int rvin_reset_format(struct rvin_dev *vin)
> -{
> - struct v4l2_subdev_format fmt = {
> - .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> - };
> - struct v4l2_mbus_framefmt *mf = 
> - int ret;
> -
> - fmt.pad = vin->digital->source_pad;
> -
> - ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, );
> - if (ret)
> - return ret;
> -
> - vin->format.width   = mf->width;
> - vin->format.height  = mf->height;
> - vin->format.colorspace  = mf->colorspace;
> - vin->format.field   = mf->field;
> -
> - rvin_reset_crop_compose(vin);
> -
> - vin->format.bytesperline = rvin_format_bytesperline(>format);
> - vin->format.sizeimage = rvin_format_sizeimage(>format);
> 
>   return 0;
>  }
> 
> -static int __rvin_try_format_source(struct rvin_dev *vin,
> - u32 which,
> - struct v4l2_pix_format *pix,
> - struct rvin_source_fmt *source)
> +static int rvin_try_format(struct rvin_dev *vin, u32 which,
> +struct v4l2_pix_format *pix,
> +struct v4l2_rect *crop, struct v4l2_rect *compose)
>  {
> - struct v4l2_subdev *sd;
> + struct v4l2_subdev *sd = vin_to_source(vin);
>   struct v4l2_subdev_pad_config *pad_cfg;
>   struct v4l2_subdev_format format = {
>   .which = which,
> + .pad = vin->digital->source_pad,
>   };
>   enum v4l2_field field;
>   u32 width, height;
>   int ret;
> 
> - sd = vin_to_source(vin);
> -
> - v4l2_fill_mbus_format(, pix, vin->digital->code);
> -
>   pad_cfg = v4l2_subdev_alloc_pad_config(sd);
>   if (pad_cfg == NULL)
>   return -ENOMEM;
> 
> - format.pad = vin->digital->source_pad;
> + v4l2_fill_mbus_format(, pix, vin->digital->code);
> 
>   /* Allow the video device to override field and to scale */
>   field = pix->field;
> @@ -217,34 +212,34 @@ static int __rvin_try_format_source(struct rvin_dev
> *vin,
> 
>   v4l2_fill_pix_format(pix, );
> 
> - source->width = pix->width;
> - source->height = pix->height;
> + crop->top = crop->left = 0;
> + crop->width = pix->width;
> + crop->height = pix->height;
> +
> + /*
> +  * If source is ALTERNATE the driver will use the VIN hardware
> +  * to INTERLACE it. The crop height then needs to be doubled.
> +  */
> + if (pix->field == V4L2_FIELD_ALTERNATE)
> + crop->height *= 2;
> +
> + if (field != V4L2_FIELD_ANY)
> + pix->field = field;
> 
> - pix->field = field;
>   pix->width = width;
>   pix->height = height;
> 
> - vin_dbg(vin, "Source

[PATCH v11 16/32] rcar-vin: read subdevice format for crop only when needed

2018-03-01 Thread Niklas Söderlund

Instead of caching the subdevice format each time the video device
format is set read it directly when it's needed. As it turns out the
format is only needed when figuring out the max rectangle for cropping.

This simplifies the code and makes it clearer what the source format is
used for.

Signed-off-by: Niklas Söderlund 
---
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 158 ++--
 drivers/media/platform/rcar-vin/rcar-vin.h  |  12 ---
 2 files changed, 80 insertions(+), 90 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c 
b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 3290e603b44cdf3a..55640c6b2a1200ca 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -144,67 +144,62 @@ static int rvin_format_align(struct rvin_dev *vin, struct 
v4l2_pix_format *pix)
  * V4L2
  */
 
-static void rvin_reset_crop_compose(struct rvin_dev *vin)
+static int rvin_get_vin_format_from_source(struct rvin_dev *vin,
+  struct v4l2_pix_format *pix)
 {
+   struct v4l2_subdev_format fmt = {
+   .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+   .pad = vin->digital->source_pad,
+   };
+   int ret;
+
+   ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, );
+   if (ret)
+   return ret;
+
+   v4l2_fill_pix_format(pix, );
+
+   return rvin_format_align(vin, pix);
+}
+
+static int rvin_reset_format(struct rvin_dev *vin)
+{
+   int ret;
+
+   ret = rvin_get_vin_format_from_source(vin, >format);
+   if (ret)
+   return ret;
+
vin->crop.top = vin->crop.left = 0;
-   vin->crop.width = vin->source.width;
-   vin->crop.height = vin->source.height;
+   vin->crop.width = vin->format.width;
+   vin->crop.height = vin->format.height;
 
vin->compose.top = vin->compose.left = 0;
vin->compose.width = vin->format.width;
vin->compose.height = vin->format.height;
-}
-
-static int rvin_reset_format(struct rvin_dev *vin)
-{
-   struct v4l2_subdev_format fmt = {
-   .which = V4L2_SUBDEV_FORMAT_ACTIVE,
-   };
-   struct v4l2_mbus_framefmt *mf = 
-   int ret;
-
-   fmt.pad = vin->digital->source_pad;
-
-   ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, );
-   if (ret)
-   return ret;
-
-   vin->format.width   = mf->width;
-   vin->format.height  = mf->height;
-   vin->format.colorspace  = mf->colorspace;
-   vin->format.field   = mf->field;
-
-   rvin_reset_crop_compose(vin);
-
-   vin->format.bytesperline = rvin_format_bytesperline(>format);
-   vin->format.sizeimage = rvin_format_sizeimage(>format);
 
return 0;
 }
 
-static int __rvin_try_format_source(struct rvin_dev *vin,
-   u32 which,
-   struct v4l2_pix_format *pix,
-   struct rvin_source_fmt *source)
+static int rvin_try_format(struct rvin_dev *vin, u32 which,
+  struct v4l2_pix_format *pix,
+  struct v4l2_rect *crop, struct v4l2_rect *compose)
 {
-   struct v4l2_subdev *sd;
+   struct v4l2_subdev *sd = vin_to_source(vin);
struct v4l2_subdev_pad_config *pad_cfg;
struct v4l2_subdev_format format = {
.which = which,
+   .pad = vin->digital->source_pad,
};
enum v4l2_field field;
u32 width, height;
int ret;
 
-   sd = vin_to_source(vin);
-
-   v4l2_fill_mbus_format(, pix, vin->digital->code);
-
pad_cfg = v4l2_subdev_alloc_pad_config(sd);
if (pad_cfg == NULL)
return -ENOMEM;
 
-   format.pad = vin->digital->source_pad;
+   v4l2_fill_mbus_format(, pix, vin->digital->code);
 
/* Allow the video device to override field and to scale */
field = pix->field;
@@ -217,34 +212,34 @@ static int __rvin_try_format_source(struct rvin_dev *vin,
 
v4l2_fill_pix_format(pix, );
 
-   source->width = pix->width;
-   source->height = pix->height;
+   crop->top = crop->left = 0;
+   crop->width = pix->width;
+   crop->height = pix->height;
+
+   /*
+* If source is ALTERNATE the driver will use the VIN hardware
+* to INTERLACE it. The crop height then needs to be doubled.
+*/
+   if (pix->field == V4L2_FIELD_ALTERNATE)
+   crop->height *= 2;
+
+   if (field != V4L2_FIELD_ANY)
+   pix->field = field;
 
-   pix->field = field;
pix->width = width;
pix->height = height;
 
-   vin_dbg(vin, "Source resolution: %ux%u\n", source->width,
-   source->height);
+   ret = rvin_format_align(vin, pix);
+   if (ret)
+   return ret;
 
+   compose->top = compose->left = 0;
+

Re: [PATCH v11 16/32] rcar-vin: read subdevice format for crop only when needed

Re: [PATCH v11 16/32] rcar-vin: read subdevice format for crop only when needed

[PATCH v11 16/32] rcar-vin: read subdevice format for crop only when needed

3 matches

Site Navigation

Mail list logo

Footer information