Re: [PATCH 16/18] media: allegro: pass buffers through firmware

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

 



On 2/17/20 4:13 PM, Michael Tretter wrote:
> As we know which buffers are processed by the codec from the address in
> the ENCODE_FRAME response, we can queue multiple buffers in the firmware
> and retrieve the buffer from the response of the firmware. This enables
> the firmware to use the internal scheduling the codec and avoids round
> trips through the driver when fetching the next frame.
> 
> Remove buffers that have been passed to the firmware from the m2m buffer
> queue and put them into a shadow queue for tracking the buffer in the
> driver. When we receive a ENCODE_FRAME response from the firmware, get
> the buffer from the shadow queue and finish the buffer.
> 
> Furthermore, it is necessary to finish the job straight after passing
> the buffer to the firmware to allow the V4L2 framework to send further
> buffers to the driver.
> 
> Signed-off-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx>
> ---
>  .../staging/media/allegro-dvt/allegro-core.c  | 104 +++++++++++++++---
>  1 file changed, 89 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c b/drivers/staging/media/allegro-dvt/allegro-core.c
> index 4f525920c194..80d3383b84f8 100644
> --- a/drivers/staging/media/allegro-dvt/allegro-core.c
> +++ b/drivers/staging/media/allegro-dvt/allegro-core.c
> @@ -226,6 +226,10 @@ struct allegro_channel {
>  	struct list_head buffers_reference;
>  	struct list_head buffers_intermediate;
>  
> +	struct list_head source_shadow_list;
> +	struct list_head stream_shadow_list;
> +	struct mutex shadow_list_lock;

This lock is never used in interrupt context, right? Just checking.

Also add a comment explaining what the lock protects.

> +
>  	struct list_head list;
>  	struct completion completion;
>  
> @@ -247,6 +251,14 @@ allegro_get_state(struct allegro_channel *channel)
>  	return channel->state;
>  }
>  
> +struct allegro_m2m_buffer {
> +	struct v4l2_m2m_buffer buf;
> +	struct list_head head;
> +};
> +
> +#define to_allegro_m2m_buffer(__buf) \
> +	container_of(__buf, struct allegro_m2m_buffer, buf)
> +
>  struct fw_info {
>  	unsigned int id;
>  	unsigned int id_codec;
> @@ -1570,6 +1582,43 @@ static void allegro_channel_buf_done(struct allegro_channel *channel,
>  	v4l2_m2m_buf_done(buf, state);
>  }
>  
> +static u64 allegro_put_buffer(struct allegro_channel *channel,
> +			      struct list_head *list,
> +			      struct vb2_v4l2_buffer *buffer)
> +{
> +	struct v4l2_m2m_buffer *b = container_of(buffer,
> +						 struct v4l2_m2m_buffer, vb);
> +	struct allegro_m2m_buffer *shadow = to_allegro_m2m_buffer(b);
> +
> +	mutex_lock(&channel->shadow_list_lock);
> +	list_add_tail(&shadow->head, list);
> +	mutex_unlock(&channel->shadow_list_lock);
> +
> +	return (u64) buffer;
> +}
> +
> +static struct vb2_v4l2_buffer *allegro_get_buffer(struct allegro_channel *channel,
> +						  struct list_head *list,
> +						  u64 handle)
> +{
> +	struct allegro_dev *dev = channel->dev;
> +	struct allegro_m2m_buffer *shadow;
> +	u64 found;
> +
> +	mutex_lock(&channel->shadow_list_lock);
> +	shadow = list_first_entry(list, struct allegro_m2m_buffer, head);
> +	list_del_init(&shadow->head);
> +	mutex_unlock(&channel->shadow_list_lock);
> +
> +	found = (u64) (&shadow->buf.vb);
> +	if (handle != found)
> +		v4l2_warn(&dev->v4l2_dev,
> +			  "channel %d: output buffer mismatch 0x%llx, expected 0x%llx\n",
> +			  channel->mcu_channel_id, handle, found);
> +
> +	return &shadow->buf.vb;

This function never returns NULL...

> +}
> +
>  static void allegro_channel_finish_frame(struct allegro_channel *channel,
>  		struct mcu_msg_encode_frame_response *msg)
>  {
> @@ -1585,13 +1634,17 @@ static void allegro_channel_finish_frame(struct allegro_channel *channel,
>  	ssize_t len;
>  	ssize_t free;
>  
> -	src_buf = v4l2_m2m_src_buf_remove(channel->fh.m2m_ctx);
> -	dst_buf = v4l2_m2m_dst_buf_remove(channel->fh.m2m_ctx);
> +	src_buf = allegro_get_buffer(channel, &channel->source_shadow_list, msg->src_handle);
> +	if (!src_buf)

...but this it checked here...

> +		v4l2_warn(&dev->v4l2_dev,
> +			  "channel %d: invalid source buffer\n",
> +			  channel->mcu_channel_id);
>  
> -	if ((u64)src_buf != msg->src_handle || (u64)dst_buf != msg->stream_id)
> -		v4l2_err(&dev->v4l2_dev,
> -			 "channel %d: check failed\n",
> -			 channel->mcu_channel_id);
> +	dst_buf = allegro_get_buffer(channel, &channel->stream_shadow_list, msg->stream_id);
> +	if (!dst_buf)

...and here. That doesn't look right.

> +		v4l2_warn(&dev->v4l2_dev,
> +			  "channel %d: invalid stream buffer\n",
> +			  channel->mcu_channel_id);
>  
>  	dst_buf->sequence = channel->csequence++;
>  
> @@ -1718,8 +1771,6 @@ static void allegro_channel_finish_frame(struct allegro_channel *channel,
>  	v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_DONE);
>  
>  	allegro_channel_buf_done(channel, dst_buf, state);
> -
> -	v4l2_m2m_job_finish(dev->m2m_dev, channel->fh.m2m_ctx);
>  }
>  
>  static int allegro_handle_init(struct allegro_dev *dev,
> @@ -2312,16 +2363,31 @@ static void allegro_stop_streaming(struct vb2_queue *q)
>  	struct allegro_channel *channel = vb2_get_drv_priv(q);
>  	struct allegro_dev *dev = channel->dev;
>  	struct vb2_v4l2_buffer *buffer;
> +	struct allegro_m2m_buffer *shadow, *tmp;
>  
>  	v4l2_dbg(2, debug, &dev->v4l2_dev,
>  		 "%s: stop streaming\n",
>  		 V4L2_TYPE_IS_OUTPUT(q->type) ? "output" : "capture");
>  
>  	if (V4L2_TYPE_IS_OUTPUT(q->type)) {
> +		mutex_lock(&channel->shadow_list_lock);
> +		list_for_each_entry_safe(shadow, tmp, &channel->source_shadow_list, head) {
> +			list_del(&shadow->head);
> +			v4l2_m2m_buf_done(&shadow->buf.vb, VB2_BUF_STATE_ERROR);
> +		}
> +		mutex_unlock(&channel->shadow_list_lock);
> +
>  		allegro_set_state(channel, ALLEGRO_STATE_STOPPED);
>  		while ((buffer = v4l2_m2m_src_buf_remove(channel->fh.m2m_ctx)))
>  			v4l2_m2m_buf_done(buffer, VB2_BUF_STATE_ERROR);
>  	} else if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> +		mutex_lock(&channel->shadow_list_lock);
> +		list_for_each_entry_safe(shadow, tmp, &channel->stream_shadow_list, head) {
> +			list_del(&shadow->head);
> +			v4l2_m2m_buf_done(&shadow->buf.vb, VB2_BUF_STATE_ERROR);
> +		}
> +		mutex_unlock(&channel->shadow_list_lock);
> +
>  		allegro_destroy_channel(channel);
>  		while ((buffer = v4l2_m2m_dst_buf_remove(channel->fh.m2m_ctx)))
>  			v4l2_m2m_buf_done(buffer, VB2_BUF_STATE_ERROR);
> @@ -2352,7 +2418,7 @@ static int allegro_queue_init(void *priv,
>  	src_vq->drv_priv = channel;
>  	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
>  	src_vq->ops = &allegro_queue_ops;
> -	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> +	src_vq->buf_struct_size = sizeof(struct allegro_m2m_buffer);
>  	src_vq->lock = &channel->dev->lock;
>  	err = vb2_queue_init(src_vq);
>  	if (err)
> @@ -2365,7 +2431,7 @@ static int allegro_queue_init(void *priv,
>  	dst_vq->drv_priv = channel;
>  	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
>  	dst_vq->ops = &allegro_queue_ops;
> -	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> +	dst_vq->buf_struct_size = sizeof(struct allegro_m2m_buffer);
>  	dst_vq->lock = &channel->dev->lock;
>  	err = vb2_queue_init(dst_vq);
>  	if (err)
> @@ -2457,6 +2523,9 @@ static int allegro_open(struct file *file)
>  	v4l2_fh_add(&channel->fh);
>  
>  	init_completion(&channel->completion);
> +	INIT_LIST_HEAD(&channel->source_shadow_list);
> +	INIT_LIST_HEAD(&channel->stream_shadow_list);
> +	mutex_init(&channel->shadow_list_lock);
>  
>  	channel->dev = dev;
>  
> @@ -2957,18 +3026,23 @@ static void allegro_device_run(void *priv)
>  	dma_addr_t src_uv;
>  	dma_addr_t dst_addr;
>  	unsigned long dst_size;
> +	u64 src_handle;
> +	u64 dst_handle;
>  
> -	dst_buf = v4l2_m2m_next_dst_buf(channel->fh.m2m_ctx);
> +	dst_buf = v4l2_m2m_dst_buf_remove(channel->fh.m2m_ctx);
>  	dst_addr = vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
>  	dst_size = vb2_plane_size(&dst_buf->vb2_buf, 0);
> -	allegro_mcu_send_put_stream_buffer(dev, channel, dst_addr, dst_size, (u64)dst_buf);
> +	dst_handle = allegro_put_buffer(channel, &channel->stream_shadow_list, dst_buf);
> +	allegro_mcu_send_put_stream_buffer(dev, channel, dst_addr, dst_size, dst_handle);
>  
> -	src_buf = v4l2_m2m_next_src_buf(channel->fh.m2m_ctx);
> +	src_buf = v4l2_m2m_src_buf_remove(channel->fh.m2m_ctx);
>  	src_buf->sequence = channel->osequence++;
> -
>  	src_y = vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
>  	src_uv = src_y + (channel->stride * channel->height);
> -	allegro_mcu_send_encode_frame(dev, channel, src_y, src_uv, (u64)src_buf);
> +	src_handle = allegro_put_buffer(channel, &channel->source_shadow_list, src_buf);
> +	allegro_mcu_send_encode_frame(dev, channel, src_y, src_uv, src_handle);
> +
> +	v4l2_m2m_job_finish(dev->m2m_dev, channel->fh.m2m_ctx);
>  }
>  
>  static const struct v4l2_m2m_ops allegro_m2m_ops = {
> 

Regards,

	Hans



[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