Re: [PATCH v4 22/45] media: sun6i-csi: Split stream sequences and irq code in capture

From: "Jernej Škrabec" <jernej.skrabec@gmail.com>
To: linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-sunxi@lists.linux.dev,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cc: Yong Deng <yong.deng@magewell.com>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Chen-Yu Tsai <wens@csie.org>,
	Samuel Holland <samuel@sholland.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH v4 22/45] media: sun6i-csi: Split stream sequences and irq code in capture
Date: Wed, 27 Apr 2022 21:19:15 +0200	[thread overview]
Message-ID: <3661760.kQq0lBPeGt@jernej-laptop> (raw)
In-Reply-To: <20220415152811.636419-23-paul.kocialkowski@bootlin.com>

Dne petek, 15. april 2022 ob 17:27:48 CEST je Paul Kocialkowski napisal(a):
> Create minimal helpers that split the enable/disable flow, which will
> make it easier to move control over to the bridge later on.
> 
> Generally speaking the goal is to move register configuration to
> the capture code and later split it with the bridge code.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

> ---
>  .../platform/sunxi/sun6i-csi/sun6i_csi.c      | 26 ---------
>  .../platform/sunxi/sun6i-csi/sun6i_csi.h      |  7 ---
>  .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 58 ++++++++++++++++++-
>  3 files changed, 55 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c index
> 98133c1dbf68..29892e8c2b9d 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> @@ -535,32 +535,6 @@ int sun6i_csi_update_config(struct sun6i_csi_device
> *csi_dev, return 0;
>  }
> 
> -void sun6i_csi_set_stream(struct sun6i_csi_device *csi_dev, bool enable)
> -{
> -	struct regmap *regmap = csi_dev->regmap;
> -
> -	if (!enable) {
> -		regmap_update_bits(regmap, SUN6I_CSI_CAP_REG,
> -				   SUN6I_CSI_CAP_VCAP_ON, 0);
> -		regmap_write(regmap, SUN6I_CSI_CH_INT_EN_REG, 0);
> -		return;
> -	}
> -
> -	regmap_write(regmap, SUN6I_CSI_CH_INT_STA_REG,
> -		     SUN6I_CSI_CH_INT_STA_CLEAR);
> -	regmap_write(regmap, SUN6I_CSI_CH_INT_EN_REG,
> -		     SUN6I_CSI_CH_INT_EN_VS |
> -		     SUN6I_CSI_CH_INT_EN_HB_OF |
> -		     SUN6I_CSI_CH_INT_EN_FIFO2_OF |
> -		     SUN6I_CSI_CH_INT_EN_FIFO1_OF |
> -		     SUN6I_CSI_CH_INT_EN_FIFO0_OF |
> -		     SUN6I_CSI_CH_INT_EN_FD |
> -		     SUN6I_CSI_CH_INT_EN_CD);
> -
> -	regmap_update_bits(regmap, SUN6I_CSI_CAP_REG, 
SUN6I_CSI_CAP_VCAP_ON,
> -			   SUN6I_CSI_CAP_VCAP_ON);
> -}
> -
>  /* Media */
> 
>  static const struct media_device_ops sun6i_csi_media_ops = {
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h index
> 44fc4d486877..155527961280 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> @@ -92,13 +92,6 @@ int sun6i_csi_set_power(struct sun6i_csi_device *csi_dev,
> bool enable); int sun6i_csi_update_config(struct sun6i_csi_device *csi_dev,
>  			    struct sun6i_csi_config *config);
> 
> -/**
> - * sun6i_csi_set_stream() - start/stop csi streaming
> - * @csi:	pointer to the csi
> - * @enable:	start/stop
> - */
> -void sun6i_csi_set_stream(struct sun6i_csi_device *csi_dev, bool enable);
> -
>  /* get bpp form v4l2 pixformat */
>  static inline int sun6i_csi_get_bpp(unsigned int pixformat)
>  {
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c index
> 7788cbab7810..72fcc60d2695 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> @@ -107,6 +107,51 @@ static bool sun6i_csi_capture_format_check(u32 format)
> 
>  /* Capture */
> 
> +static void sun6i_csi_capture_irq_enable(struct sun6i_csi_device *csi_dev)
> +{
> +	struct regmap *regmap = csi_dev->regmap;
> +
> +	regmap_write(regmap, SUN6I_CSI_CH_INT_EN_REG,
> +		     SUN6I_CSI_CH_INT_EN_VS |
> +		     SUN6I_CSI_CH_INT_EN_HB_OF |
> +		     SUN6I_CSI_CH_INT_EN_FIFO2_OF |
> +		     SUN6I_CSI_CH_INT_EN_FIFO1_OF |
> +		     SUN6I_CSI_CH_INT_EN_FIFO0_OF |
> +		     SUN6I_CSI_CH_INT_EN_FD |
> +		     SUN6I_CSI_CH_INT_EN_CD);
> +}
> +
> +static void sun6i_csi_capture_irq_disable(struct sun6i_csi_device *csi_dev)
> +{
> +	struct regmap *regmap = csi_dev->regmap;
> +
> +	regmap_write(regmap, SUN6I_CSI_CH_INT_EN_REG, 0);
> +}
> +
> +static void sun6i_csi_capture_irq_clear(struct sun6i_csi_device *csi_dev)
> +{
> +	struct regmap *regmap = csi_dev->regmap;
> +
> +	regmap_write(regmap, SUN6I_CSI_CH_INT_EN_REG, 0);
> +	regmap_write(regmap, SUN6I_CSI_CH_INT_STA_REG,
> +		     SUN6I_CSI_CH_INT_STA_CLEAR);
> +}
> +
> +static void sun6i_csi_capture_enable(struct sun6i_csi_device *csi_dev)
> +{
> +	struct regmap *regmap = csi_dev->regmap;
> +
> +	regmap_update_bits(regmap, SUN6I_CSI_CAP_REG, 
SUN6I_CSI_CAP_VCAP_ON,
> +			   SUN6I_CSI_CAP_VCAP_ON);
> +}
> +
> +static void sun6i_csi_capture_disable(struct sun6i_csi_device *csi_dev)
> +{
> +	struct regmap *regmap = csi_dev->regmap;
> +
> +	regmap_update_bits(regmap, SUN6I_CSI_CAP_REG, 
SUN6I_CSI_CAP_VCAP_ON, 0);
> +}
> +
>  static void
>  sun6i_csi_capture_buffer_configure(struct sun6i_csi_device *csi_dev,
>  				   struct sun6i_csi_buffer 
*csi_buffer)
> @@ -357,6 +402,10 @@ static int sun6i_csi_capture_start_streaming(struct
> vb2_queue *queue, goto error_media_pipeline;
>  	}
> 
> +	/* Clear */
> +
> +	sun6i_csi_capture_irq_clear(csi_dev);
> +
>  	/* Configure */
> 
>  	sun6i_csi_capture_configure(csi_dev);
> @@ -367,7 +416,8 @@ static int sun6i_csi_capture_start_streaming(struct
> vb2_queue *queue,
> 
>  	/* Enable */
> 
> -	sun6i_csi_set_stream(csi_dev, true);
> +	sun6i_csi_capture_irq_enable(csi_dev);
> +	sun6i_csi_capture_enable(csi_dev);
> 
>  	ret = v4l2_subdev_call(subdev, video, s_stream, 1);
>  	if (ret && ret != -ENOIOCTLCMD)
> @@ -376,7 +426,8 @@ static int sun6i_csi_capture_start_streaming(struct
> vb2_queue *queue, return 0;
> 
>  error_stream:
> -	sun6i_csi_set_stream(csi_dev, false);
> +	sun6i_csi_capture_disable(csi_dev);
> +	sun6i_csi_capture_irq_disable(csi_dev);
> 
>  error_media_pipeline:
>  	media_pipeline_stop(&video_dev->entity);
> @@ -397,7 +448,8 @@ static void sun6i_csi_capture_stop_streaming(struct
> vb2_queue *queue) if (subdev)
>  		v4l2_subdev_call(subdev, video, s_stream, 0);
> 
> -	sun6i_csi_set_stream(csi_dev, false);
> +	sun6i_csi_capture_disable(csi_dev);
> +	sun6i_csi_capture_irq_disable(csi_dev);
> 
>  	media_pipeline_stop(&capture->video_dev.entity);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel