Re: [PATCH 7/8] media: rkisp1: Add struct rkisp1_params_buffer

[Dan Scally <dan.scally@xxxxxxxxxxxxxxxx>]

Hi Jacopo

On 05/06/2024 17:54, Jacopo Mondi wrote:
> Create the 'struct rkisp1_params_buffer' type that wraps a
> vb2_v4l2_buffer and contain a pointer to an optional copy of
> the parameters buffer that will be used to copy the user-provided
> configuration buffer in the following patches.
>
> Replace usage of 'struct rkisp1_buffer' with 'struct
> rkisp1_params_buffer' in rkisp1-params.c to prepare for that.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>


The contents look fine (one teeny tiny thing below), though I think I'd personally push this before 
5/8 and squash 8/8 into 5/8 - up to you.


Reviewed-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>

> ---
>   .../platform/rockchip/rkisp1/rkisp1-common.h  | 16 +++++++++++++-
>   .../platform/rockchip/rkisp1/rkisp1-params.c  | 21 ++++++++++---------
>   2 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index f9df5ed96c98..3118f1974246 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -232,7 +232,7 @@ struct rkisp1_vdev_node {
>   
>   /*
>    * struct rkisp1_buffer - A container for the vb2 buffers used by the video devices:
> - *			  params, stats, mainpath, selfpath
> + *			  stats, mainpath, selfpath
>    *
>    * @vb:		vb2 buffer
>    * @queue:	entry of the buffer in the queue
> @@ -244,6 +244,20 @@ struct rkisp1_buffer {
>   	dma_addr_t buff_addr[VIDEO_MAX_PLANES];
>   };
>   
> +/*
> + * struct rkisp1_params_buffer - A container for the vb2 buffers used by the
> + *				 params video device
> + *
> + * @vb:		vb2 buffer
> + * @queue:	entry of the buffer in the queue
> + * cfg:		scratch buffer used for the extensible parameters format
s/cfg/@cfg
> + */
> +struct rkisp1_params_buffer {
> +	struct vb2_v4l2_buffer vb;
> +	struct list_head queue;
> +	struct rkisp1_ext_params_cfg *cfg;
> +};
> +
>   /*
>    * struct rkisp1_dummy_buffer - A buffer to write the next frame to in case
>    *				there are no vb2 buffers available.
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> index c081fd490b2b..4adaf084ce6e 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
> @@ -2016,20 +2016,21 @@ static int rkisp1_ext_params_lsc_config(struct rkisp1_params *params,
>   }
>   
>   static bool rkisp1_params_get_buffer(struct rkisp1_params *params,
> -				     struct rkisp1_buffer **buf,
> +				     struct rkisp1_params_buffer **buf,
>   				     void **cfg)
>   {
>   	if (list_empty(&params->params))
>   		return false;
>   
> -	*buf = list_first_entry(&params->params, struct rkisp1_buffer, queue);
> +	*buf = list_first_entry(&params->params, struct rkisp1_params_buffer,
> +				queue);
>   	*cfg = vb2_plane_vaddr(&(*buf)->vb.vb2_buf, 0);
>   
>   	return true;
>   }
>   
>   static void rkisp1_params_complete_buffer(struct rkisp1_params *params,
> -					  struct rkisp1_buffer *buf,
> +					  struct rkisp1_params_buffer *buf,
>   					  unsigned int frame_sequence,
>   					  enum vb2_buffer_state state)
>   {
> @@ -2042,7 +2043,7 @@ static void rkisp1_params_complete_buffer(struct rkisp1_params *params,
>   void rkisp1_params_isr(struct rkisp1_device *rkisp1)
>   {
>   	struct rkisp1_params *params = &rkisp1->params;
> -	struct rkisp1_buffer *buf;
> +	struct rkisp1_params_buffer *buf;
>   	int ret = 0;
>   	void *cfg;
>   
> @@ -2129,7 +2130,7 @@ int rkisp1_params_pre_configure(struct rkisp1_params *params,
>   				enum v4l2_ycbcr_encoding ycbcr_encoding)
>   {
>   	struct rkisp1_cif_isp_hst_config hst = rkisp1_hst_params_default_config;
> -	struct rkisp1_buffer *buf;
> +	struct rkisp1_params_buffer *buf;
>   	int ret = 0;
>   	void *cfg;
>   
> @@ -2193,7 +2194,7 @@ int rkisp1_params_pre_configure(struct rkisp1_params *params,
>   
>   int rkisp1_params_post_configure(struct rkisp1_params *params)
>   {
> -	struct rkisp1_buffer *buf;
> +	struct rkisp1_params_buffer *buf;
>   	int ret = 0;
>   	void *cfg;
>   
> @@ -2409,8 +2410,8 @@ static int rkisp1_params_vb2_queue_setup(struct vb2_queue *vq,
>   static void rkisp1_params_vb2_buf_queue(struct vb2_buffer *vb)
>   {
>   	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> -	struct rkisp1_buffer *params_buf =
> -		container_of(vbuf, struct rkisp1_buffer, vb);
> +	struct rkisp1_params_buffer *params_buf =
> +		container_of(vbuf, struct rkisp1_params_buffer, vb);
>   	struct vb2_queue *vq = vb->vb2_queue;
>   	struct rkisp1_params *params = vq->drv_priv;
>   
> @@ -2436,7 +2437,7 @@ static int rkisp1_params_vb2_buf_prepare(struct vb2_buffer *vb)
>   static void rkisp1_params_vb2_stop_streaming(struct vb2_queue *vq)
>   {
>   	struct rkisp1_params *params = vq->drv_priv;
> -	struct rkisp1_buffer *buf;
> +	struct rkisp1_params_buffer *buf;
>   	LIST_HEAD(tmp_list);
>   
>   	/*
> @@ -2482,7 +2483,7 @@ static int rkisp1_params_init_vb2_queue(struct vb2_queue *q,
>   	q->drv_priv = params;
>   	q->ops = &rkisp1_params_vb2_ops;
>   	q->mem_ops = &vb2_vmalloc_memops;
> -	q->buf_struct_size = sizeof(struct rkisp1_buffer);
> +	q->buf_struct_size = sizeof(struct rkisp1_params_buffer);
>   	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>   	q->lock = &node->vlock;
>