Re: [PATCH v4 22/45] media: sun6i-csi: Split stream sequences and irq code in capture
From: Jernej Škrabec
Date: Wed Apr 27 2022 - 15:21:22 EST
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@xxxxxxxxxxx>
Acked-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
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);