Re: [PATCH 4/5] rcar-vin: Break out hardware start and stop to new methods

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Niklas,

On Fri, Oct 16, 2020 at 01:14:07AM +0200, Niklas Söderlund wrote:
> To support suspend and resume the ability to start and stop the hardware
> needs to be available internally in the driver. Currently this code is
> in the start and stop callbacks of the vb2_ops struct. In these
> callbacks the code is intertwined with buffer allocation and freeing.
>
> Prepare for suspend and resume support by braking out the hardware

breaking

> start/stop code into new methods.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>

I very much like this, it really simplifies the code
Reviewed-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>

Thanks
   j

> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c | 78 +++++++++++++---------
>  drivers/media/platform/rcar-vin/rcar-vin.h |  3 +
>  2 files changed, 48 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index f65deac4c2dbed54..5a5f0e5007478c8d 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1050,16 +1050,20 @@ static irqreturn_t rvin_irq(int irq, void *data)
>  	return IRQ_RETVAL(handled);
>  }
>
> -/* Need to hold qlock before calling */
>  static void return_unused_buffers(struct rvin_dev *vin,
>  				  enum vb2_buffer_state state)
>  {
>  	struct rvin_buffer *buf, *node;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&vin->qlock, flags);
>
>  	list_for_each_entry_safe(buf, node, &vin->buf_list, list) {
>  		vb2_buffer_done(&buf->vb.vb2_buf, state);
>  		list_del(&buf->list);
>  	}
> +
> +	spin_unlock_irqrestore(&vin->qlock, flags);
>  }
>
>  static int rvin_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
> @@ -1243,53 +1247,55 @@ 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;
>
> -	/* Allocate scratch buffer. */
> -	vin->scratch = dma_alloc_coherent(vin->dev, vin->format.sizeimage,
> -					  &vin->scratch_phys, GFP_KERNEL);
> -	if (!vin->scratch) {
> -		spin_lock_irqsave(&vin->qlock, flags);
> -		return_unused_buffers(vin, VB2_BUF_STATE_QUEUED);
> -		spin_unlock_irqrestore(&vin->qlock, flags);
> -		vin_err(vin, "Failed to allocate scratch buffer\n");
> -		return -ENOMEM;
> -	}
> -
>  	ret = rvin_set_stream(vin, 1);
> -	if (ret) {
> -		spin_lock_irqsave(&vin->qlock, flags);
> -		return_unused_buffers(vin, VB2_BUF_STATE_QUEUED);
> -		spin_unlock_irqrestore(&vin->qlock, flags);
> -		goto out;
> -	}
> +	if (ret)
> +		return ret;
>
>  	spin_lock_irqsave(&vin->qlock, flags);
>
>  	vin->sequence = 0;
>
>  	ret = rvin_capture_start(vin);
> -	if (ret) {
> -		return_unused_buffers(vin, VB2_BUF_STATE_QUEUED);
> +	if (ret)
>  		rvin_set_stream(vin, 0);
> -	}
>
>  	spin_unlock_irqrestore(&vin->qlock, flags);
> -out:
> -	if (ret)
> -		dma_free_coherent(vin->dev, vin->format.sizeimage, vin->scratch,
> -				  vin->scratch_phys);
>
>  	return ret;
>  }
>
> -static void rvin_stop_streaming(struct vb2_queue *vq)
> +static int rvin_start_streaming_vq(struct vb2_queue *vq, unsigned int count)
>  {
>  	struct rvin_dev *vin = vb2_get_drv_priv(vq);
> +	int ret = -ENOMEM;
> +
> +	/* Allocate scratch buffer. */
> +	vin->scratch = dma_alloc_coherent(vin->dev, vin->format.sizeimage,
> +					  &vin->scratch_phys, GFP_KERNEL);
> +	if (!vin->scratch)
> +		goto err_scratch;
> +
> +	ret = rvin_start_streaming(vin);
> +	if (ret)
> +		goto err_start;
> +
> +	return 0;
> +err_start:
> +	dma_free_coherent(vin->dev, vin->format.sizeimage, vin->scratch,
> +			  vin->scratch_phys);
> +err_scratch:
> +	return_unused_buffers(vin, VB2_BUF_STATE_QUEUED);
> +
> +	return ret;
> +}
> +
> +void rvin_stop_streaming(struct rvin_dev *vin)
> +{
>  	unsigned int i, retries;
>  	unsigned long flags;
>  	bool buffersFreed;
> @@ -1341,27 +1347,33 @@ static void rvin_stop_streaming(struct vb2_queue *vq)
>  		vin->state = STOPPED;
>  	}
>
> -	/* Return all unused buffers. */
> -	return_unused_buffers(vin, VB2_BUF_STATE_ERROR);
> -
>  	spin_unlock_irqrestore(&vin->qlock, flags);
>
>  	rvin_set_stream(vin, 0);
>
>  	/* disable interrupts */
>  	rvin_disable_interrupts(vin);
> +}
> +
> +static void rvin_stop_streaming_vq(struct vb2_queue *vq)
> +{
> +	struct rvin_dev *vin = vb2_get_drv_priv(vq);
> +
> +	rvin_stop_streaming(vin);
>
>  	/* Free scratch buffer. */
>  	dma_free_coherent(vin->dev, vin->format.sizeimage, vin->scratch,
>  			  vin->scratch_phys);
> +
> +	return_unused_buffers(vin, VB2_BUF_STATE_ERROR);
>  }
>
>  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 2fef23470e3ddfe3..4ec8584709c847a9 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -299,4 +299,7 @@ void rvin_crop_scale_comp(struct rvin_dev *vin);
>  int rvin_set_channel_routing(struct rvin_dev *vin, u8 chsel);
>  void rvin_set_alpha(struct rvin_dev *vin, unsigned int alpha);
>
> +int rvin_start_streaming(struct rvin_dev *vin);
> +void rvin_stop_streaming(struct rvin_dev *vin);
> +
>  #endif
> --
> 2.28.0
>



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux