diff mbox series

[4/4] media: i2c: max9286: Remove cached formats

Message ID 20200817143540.247340-5-jacopo+renesas@jmondi.org (mailing list archive)
State New
Delegated to: Kieran Bingham
Headers show
Series media: i2c: max9286: Use remote endpoint image format | expand

Commit Message

Jacopo Mondi Aug. 17, 2020, 2:35 p.m. UTC 
Now that the image stream formats are retrieved from the remote
sources there's no need to cache them in the driver structure.

Remove the cached mbus frame formats and their initialization.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/max9286.c | 17 -----------------
 1 file changed, 17 deletions(-)

Comments

Hyun Kwon Aug. 17, 2020, 10:15 p.m. UTC | #1 
Hi Jacopo,

Thanks for sharing!

On Mon, Aug 17, 2020 at 07:35:40AM -0700, Jacopo Mondi wrote:
> Now that the image stream formats are retrieved from the remote
> sources there's no need to cache them in the driver structure.
> 
> Remove the cached mbus frame formats and their initialization.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/media/i2c/max9286.c | 17 -----------------
>  1 file changed, 17 deletions(-)
> 
> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> index a4e23396c4b6..97dfee767bbf 100644
> --- a/drivers/media/i2c/max9286.c
> +++ b/drivers/media/i2c/max9286.c
> @@ -160,8 +160,6 @@ struct max9286_priv {
>  	struct v4l2_ctrl_handler ctrls;
>  	struct v4l2_ctrl *pixelrate;
>  
> -	struct v4l2_mbus_framefmt fmt[MAX9286_N_SINKS];
> -
>  	/* Protects controls and fmt structures */
>  	struct mutex mutex;
>  
> @@ -758,18 +756,6 @@ static const struct v4l2_subdev_ops max9286_subdev_ops = {
>  	.pad		= &max9286_pad_ops,
>  };
>  
> -static void max9286_init_format(struct v4l2_mbus_framefmt *fmt)
> -{
> -	fmt->width		= 1280;
> -	fmt->height		= 800;
> -	fmt->code		= MEDIA_BUS_FMT_UYVY8_1X16;

There's one more hardcoded place left. This had some implication on output
format, MAX9286_DATATYPE_YUV422_8BIT, which is programmed at 0x12.
Now, this patch set can potentially result in a mismatch between bus format
and acutal programmed format.

Thanks,
-hyun

> -	fmt->colorspace		= V4L2_COLORSPACE_SRGB;
> -	fmt->field		= V4L2_FIELD_NONE;
> -	fmt->ycbcr_enc		= V4L2_YCBCR_ENC_DEFAULT;
> -	fmt->quantization	= V4L2_QUANTIZATION_DEFAULT;
> -	fmt->xfer_func		= V4L2_XFER_FUNC_DEFAULT;
> -}
> -
>  static int max9286_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh)
>  {
>  	struct max9286_priv *priv = sd_to_max9286(subdev);
> @@ -834,9 +820,6 @@ static int max9286_v4l2_register(struct max9286_priv *priv)
>  
>  	/* Configure V4L2 for the MAX9286 itself */
>  
> -	for (i = 0; i < MAX9286_N_SINKS; i++)
> -		max9286_init_format(&priv->fmt[i]);
> -
>  	v4l2_i2c_subdev_init(&priv->sd, priv->client, &max9286_subdev_ops);
>  	priv->sd.internal_ops = &max9286_subdev_internal_ops;
>  	priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> -- 
> 2.27.0
>
Jacopo Mondi Aug. 18, 2020, 12:05 p.m. UTC | #2 
Hi Hyun,

On Mon, Aug 17, 2020 at 03:15:05PM -0700, Hyun Kwon wrote:
> Hi Jacopo,
>
> Thanks for sharing!
>
> On Mon, Aug 17, 2020 at 07:35:40AM -0700, Jacopo Mondi wrote:
> > Now that the image stream formats are retrieved from the remote
> > sources there's no need to cache them in the driver structure.
> >
> > Remove the cached mbus frame formats and their initialization.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  drivers/media/i2c/max9286.c | 17 -----------------
> >  1 file changed, 17 deletions(-)
> >
> > diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> > index a4e23396c4b6..97dfee767bbf 100644
> > --- a/drivers/media/i2c/max9286.c
> > +++ b/drivers/media/i2c/max9286.c
> > @@ -160,8 +160,6 @@ struct max9286_priv {
> >  	struct v4l2_ctrl_handler ctrls;
> >  	struct v4l2_ctrl *pixelrate;
> >
> > -	struct v4l2_mbus_framefmt fmt[MAX9286_N_SINKS];
> > -
> >  	/* Protects controls and fmt structures */
> >  	struct mutex mutex;
> >
> > @@ -758,18 +756,6 @@ static const struct v4l2_subdev_ops max9286_subdev_ops = {
> >  	.pad		= &max9286_pad_ops,
> >  };
> >
> > -static void max9286_init_format(struct v4l2_mbus_framefmt *fmt)
> > -{
> > -	fmt->width		= 1280;
> > -	fmt->height		= 800;
> > -	fmt->code		= MEDIA_BUS_FMT_UYVY8_1X16;
>
> There's one more hardcoded place left. This had some implication on output
> format, MAX9286_DATATYPE_YUV422_8BIT, which is programmed at 0x12.
> Now, this patch set can potentially result in a mismatch between bus format
> and acutal programmed format.

Yup, very correct, I should adjust the DT based on the deserialized
format!

Thanks for noticing!

>
> Thanks,
> -hyun
>
> > -	fmt->colorspace		= V4L2_COLORSPACE_SRGB;
> > -	fmt->field		= V4L2_FIELD_NONE;
> > -	fmt->ycbcr_enc		= V4L2_YCBCR_ENC_DEFAULT;
> > -	fmt->quantization	= V4L2_QUANTIZATION_DEFAULT;
> > -	fmt->xfer_func		= V4L2_XFER_FUNC_DEFAULT;
> > -}
> > -
> >  static int max9286_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh)
> >  {
> >  	struct max9286_priv *priv = sd_to_max9286(subdev);
> > @@ -834,9 +820,6 @@ static int max9286_v4l2_register(struct max9286_priv *priv)
> >
> >  	/* Configure V4L2 for the MAX9286 itself */
> >
> > -	for (i = 0; i < MAX9286_N_SINKS; i++)
> > -		max9286_init_format(&priv->fmt[i]);
> > -
> >  	v4l2_i2c_subdev_init(&priv->sd, priv->client, &max9286_subdev_ops);
> >  	priv->sd.internal_ops = &max9286_subdev_internal_ops;
> >  	priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > --
> > 2.27.0
> >
diff mbox series

Patch

diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index a4e23396c4b6..97dfee767bbf 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -160,8 +160,6 @@  struct max9286_priv {
 	struct v4l2_ctrl_handler ctrls;
 	struct v4l2_ctrl *pixelrate;
 
-	struct v4l2_mbus_framefmt fmt[MAX9286_N_SINKS];
-
 	/* Protects controls and fmt structures */
 	struct mutex mutex;
 
@@ -758,18 +756,6 @@  static const struct v4l2_subdev_ops max9286_subdev_ops = {
 	.pad		= &max9286_pad_ops,
 };
 
-static void max9286_init_format(struct v4l2_mbus_framefmt *fmt)
-{
-	fmt->width		= 1280;
-	fmt->height		= 800;
-	fmt->code		= MEDIA_BUS_FMT_UYVY8_1X16;
-	fmt->colorspace		= V4L2_COLORSPACE_SRGB;
-	fmt->field		= V4L2_FIELD_NONE;
-	fmt->ycbcr_enc		= V4L2_YCBCR_ENC_DEFAULT;
-	fmt->quantization	= V4L2_QUANTIZATION_DEFAULT;
-	fmt->xfer_func		= V4L2_XFER_FUNC_DEFAULT;
-}
-
 static int max9286_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh)
 {
 	struct max9286_priv *priv = sd_to_max9286(subdev);
@@ -834,9 +820,6 @@  static int max9286_v4l2_register(struct max9286_priv *priv)
 
 	/* Configure V4L2 for the MAX9286 itself */
 
-	for (i = 0; i < MAX9286_N_SINKS; i++)
-		max9286_init_format(&priv->fmt[i]);
-
 	v4l2_i2c_subdev_init(&priv->sd, priv->client, &max9286_subdev_ops);
 	priv->sd.internal_ops = &max9286_subdev_internal_ops;
 	priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;