Re: [PATCH 3/4] rcar-vin: make rvin_{start,stop}_streaming() available for internal use

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Niklas,

Thank you for the patch.

On Friday, 14 December 2018 08:18:23 EET Niklas Söderlund wrote:
> To support suspend/resume rvin_{start,stop}_streaming() needs to be
> accessible from the suspend and resume callbacks. Up until now the only
> users of these functions have been the callbacks in struct vb2_ops so
> the arguments to the functions are not suitable for use by the driver it
> self.
> 
> Fix this by adding wrappers for the struct vb2_ops callbacks which calls
> the new rvin_{start,stop}_streaming() using more friendly arguments.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c | 20 ++++++++++++++------
>  drivers/media/platform/rcar-vin/rcar-vin.h |  3 +++
>  2 files changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c index
> 64f7636f94d6a0a3..d11d4df1906a8962 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1143,9 +1143,8 @@ static int rvin_set_stream(struct rvin_dev *vin, int
> on) return ret;
>  }
> 
> -static int rvin_start_streaming(struct vb2_queue *vq, unsigned int count)
> +int rvin_start_streaming(struct rvin_dev *vin)
>  {
> -	struct rvin_dev *vin = vb2_get_drv_priv(vq);
>  	unsigned long flags;
>  	int ret;
> 
> @@ -1187,9 +1186,13 @@ static int rvin_start_streaming(struct vb2_queue *vq,
> unsigned int count) return ret;
>  }
> 
> -static void rvin_stop_streaming(struct vb2_queue *vq)
> +static int rvin_start_streaming_vq(struct vb2_queue *vq, unsigned int
> count)
> +{
> +	return rvin_start_streaming(vb2_get_drv_priv(vq));
> +}
> +
> +void rvin_stop_streaming(struct rvin_dev *vin)
>  {
> -	struct rvin_dev *vin = vb2_get_drv_priv(vq);
>  	unsigned long flags;
>  	int retries = 0;
> 
> @@ -1238,12 +1241,17 @@ static void rvin_stop_streaming(struct vb2_queue
> *vq) vin->scratch_phys);
>  }
> 
> +static void rvin_stop_streaming_vq(struct vb2_queue *vq)
> +{
> +	rvin_stop_streaming(vb2_get_drv_priv(vq));
> +}

You'll need a bit more than this. rvin_stop_streaming() calls 
return_all_buffers() and dma_free_coherent() which you don't want at suspend 
time. Buffers should not be returned to userspace but kept for reuse, and it's 
pointless to free the scratch buffer at suspend time to reallocate it at 
resume time.

>  static const struct vb2_ops rvin_qops = {
>  	.queue_setup		= rvin_queue_setup,
>  	.buf_prepare		= rvin_buffer_prepare,
>  	.buf_queue		= rvin_buffer_queue,
> -	.start_streaming	= rvin_start_streaming,
> -	.stop_streaming		= rvin_stop_streaming,
> +	.start_streaming	= rvin_start_streaming_vq,
> +	.stop_streaming		= rvin_stop_streaming_vq,
>  	.wait_prepare		= vb2_ops_wait_prepare,
>  	.wait_finish		= vb2_ops_wait_finish,
>  };
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> b/drivers/media/platform/rcar-vin/rcar-vin.h index
> d21fc991b7a9da36..700fae1c1225a2f3 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -269,4 +269,7 @@ void rvin_crop_scale_comp(struct rvin_dev *vin);
> 
>  int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel);
> 
> +int rvin_start_streaming(struct rvin_dev *vin);
> +void rvin_stop_streaming(struct rvin_dev *vin);
> +
>  #endif


-- 
Regards,

Laurent Pinchart