[PATCH 0/5] media: intel/ipu6: Cleanups and small refactor

[PATCH 0/5] media: intel/ipu6: Cleanups and small refactor

[Stanislaw Gruszka]

Some clenaups and abstract buf ready function away from FW interface
to make it reusable with ipu7.

Stanislaw Gruszka (5):
  media: intel/ipu6: Add missing new line character in error message
  media: intel/ipu6: Make two functions static
  media: intel/ipu6: Use timestamp value directly
  media: intel/ipu6: Abstart buf ready function
  media: intel/ipu6: Remove unused dev field from ipu6_isys_queue

 .../media/pci/intel/ipu6/ipu6-isys-queue.c    | 51 ++++++++++---------
 .../media/pci/intel/ipu6/ipu6-isys-queue.h    |  5 --
 2 files changed, 28 insertions(+), 28 deletions(-)

-- 
2.34.1

[PATCH 1/5] media: intel/ipu6: Add missing new line character in error message

[Stanislaw Gruszka]

End error message with new line.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-isys-queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
index 72f5f987ef48..9a19695abbaf 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
@@ -797,7 +797,7 @@ void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
 		return;
 	}
 
-	dev_err(dev, "Failed to find a matching video buffer");
+	dev_err(dev, "Failed to find a matching video buffer\n");
 
 	spin_unlock_irqrestore(&aq->lock, flags);
 }
-- 
2.34.1

[PATCH 2/5] media: intel/ipu6: Make two functions static

[Stanislaw Gruszka]

Make function used only in one file static and remove from header file.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-isys-queue.c | 7 ++++---
 drivers/media/pci/intel/ipu6/ipu6-isys-queue.h | 4 ----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
index 9a19695abbaf..3984b9d43919 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
@@ -697,8 +697,9 @@ static u64 get_sof_ns_delta(struct ipu6_isys_video *av,
 	return ipu6_buttress_tsc_ticks_to_ns(delta, isp);
 }
 
-void ipu6_isys_buf_calc_sequence_time(struct ipu6_isys_buffer *ib,
-				      struct ipu6_fw_isys_resp_info_abi *info)
+static void
+ipu6_isys_buf_calc_sequence_time(struct ipu6_isys_buffer *ib,
+				 struct ipu6_fw_isys_resp_info_abi *info)
 {
 	struct vb2_buffer *vb = ipu6_isys_buffer_to_vb2_buffer(ib);
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -721,7 +722,7 @@ void ipu6_isys_buf_calc_sequence_time(struct ipu6_isys_buffer *ib,
 		vbuf->vb2_buf.timestamp);
 }
 
-void ipu6_isys_queue_buf_done(struct ipu6_isys_buffer *ib)
+static void ipu6_isys_queue_buf_done(struct ipu6_isys_buffer *ib)
 {
 	struct vb2_buffer *vb = ipu6_isys_buffer_to_vb2_buffer(ib);
 
diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
index fe8fc796a58f..1a277b0fb4b5 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
@@ -69,10 +69,6 @@ void
 ipu6_isys_buf_to_fw_frame_buf(struct ipu6_fw_isys_frame_buff_set_abi *set,
 			      struct ipu6_isys_stream *stream,
 			      struct ipu6_isys_buffer_list *bl);
-void
-ipu6_isys_buf_calc_sequence_time(struct ipu6_isys_buffer *ib,
-				 struct ipu6_fw_isys_resp_info_abi *info);
-void ipu6_isys_queue_buf_done(struct ipu6_isys_buffer *ib);
 void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
 			       struct ipu6_fw_isys_resp_info_abi *info);
 int ipu6_isys_queue_init(struct ipu6_isys_queue *aq);
-- 
2.34.1

[PATCH 3/5] media: intel/ipu6: Use timestamp value directly

[Stanislaw Gruszka]

Remove pointer for fw abi structure when setting frame sequence and
time, use timestamp value from the structure directly.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 .../media/pci/intel/ipu6/ipu6-isys-queue.c    | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
index 3984b9d43919..c68dc577daf0 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
@@ -652,10 +652,8 @@ static void stop_streaming(struct vb2_queue *q)
 }
 
 static unsigned int
-get_sof_sequence_by_timestamp(struct ipu6_isys_stream *stream,
-			      struct ipu6_fw_isys_resp_info_abi *info)
+get_sof_sequence_by_timestamp(struct ipu6_isys_stream *stream, u64 time)
 {
-	u64 time = (u64)info->timestamp[1] << 32 | info->timestamp[0];
 	struct ipu6_isys *isys = stream->isys;
 	struct device *dev = &isys->adev->auxdev.dev;
 	unsigned int i;
@@ -681,8 +679,7 @@ get_sof_sequence_by_timestamp(struct ipu6_isys_stream *stream,
 	return 0;
 }
 
-static u64 get_sof_ns_delta(struct ipu6_isys_video *av,
-			    struct ipu6_fw_isys_resp_info_abi *info)
+static u64 get_sof_ns_delta(struct ipu6_isys_video *av, u64 timestamp)
 {
 	struct ipu6_bus_device *adev = av->isys->adev;
 	struct ipu6_device *isp = adev->isp;
@@ -692,14 +689,13 @@ static u64 get_sof_ns_delta(struct ipu6_isys_video *av,
 	if (!tsc_now)
 		return 0;
 
-	delta = tsc_now - ((u64)info->timestamp[1] << 32 | info->timestamp[0]);
+	delta = tsc_now - timestamp;
 
 	return ipu6_buttress_tsc_ticks_to_ns(delta, isp);
 }
 
 static void
-ipu6_isys_buf_calc_sequence_time(struct ipu6_isys_buffer *ib,
-				 struct ipu6_fw_isys_resp_info_abi *info)
+ipu6_isys_buf_calc_sequence_time(struct ipu6_isys_buffer *ib, u64 time)
 {
 	struct vb2_buffer *vb = ipu6_isys_buffer_to_vb2_buffer(ib);
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -710,8 +706,8 @@ ipu6_isys_buf_calc_sequence_time(struct ipu6_isys_buffer *ib,
 	u64 ns;
 	u32 sequence;
 
-	ns = ktime_get_ns() - get_sof_ns_delta(av, info);
-	sequence = get_sof_sequence_by_timestamp(stream, info);
+	ns = ktime_get_ns() - get_sof_ns_delta(av, time);
+	sequence = get_sof_sequence_by_timestamp(stream, time);
 
 	vbuf->vb2_buf.timestamp = ns;
 	vbuf->sequence = sequence;
@@ -749,6 +745,7 @@ void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
 	unsigned long flags;
 	bool first = true;
 	struct vb2_v4l2_buffer *buf;
+	u64 time = (u64)info->timestamp[1] << 32 | info->timestamp[0];
 
 	spin_lock_irqsave(&aq->lock, flags);
 	if (list_empty(&aq->active)) {
@@ -791,7 +788,7 @@ void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
 		list_del(&ib->head);
 		spin_unlock_irqrestore(&aq->lock, flags);
 
-		ipu6_isys_buf_calc_sequence_time(ib, info);
+		ipu6_isys_buf_calc_sequence_time(ib, time);
 
 		ipu6_isys_queue_buf_done(ib);
 
-- 
2.34.1

[PATCH 4/5] media: intel/ipu6: Abstart buf ready function

[Stanislaw Gruszka]

Extract values needed by ipu6_queue_buf_ready() function from fw abi
structure. This will allow to reuse same buf ready code when fw abi
change.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 .../media/pci/intel/ipu6/ipu6-isys-queue.c    | 28 ++++++++++++-------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
index c68dc577daf0..c7f302df2dcc 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
@@ -734,10 +734,11 @@ static void ipu6_isys_queue_buf_done(struct ipu6_isys_buffer *ib)
 	}
 }
 
-void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
-			       struct ipu6_fw_isys_resp_info_abi *info)
+static void
+ipu6_stream_buf_ready(struct ipu6_isys_stream *stream, u8 pin_id, u32 addr,
+		      u64 time, bool error_check)
 {
-	struct ipu6_isys_queue *aq = stream->output_pins[info->pin_id].aq;
+	struct ipu6_isys_queue *aq = stream->output_pins[pin_id].aq;
 	struct ipu6_isys *isys = stream->isys;
 	struct device *dev = &isys->adev->auxdev.dev;
 	struct ipu6_isys_buffer *ib;
@@ -745,7 +746,6 @@ void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
 	unsigned long flags;
 	bool first = true;
 	struct vb2_v4l2_buffer *buf;
-	u64 time = (u64)info->timestamp[1] << 32 | info->timestamp[0];
 
 	spin_lock_irqsave(&aq->lock, flags);
 	if (list_empty(&aq->active)) {
@@ -757,23 +757,22 @@ void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
 	list_for_each_entry_reverse(ib, &aq->active, head) {
 		struct ipu6_isys_video_buffer *ivb;
 		struct vb2_v4l2_buffer *vvb;
-		dma_addr_t addr;
+		dma_addr_t dma_addr;
 
 		vb = ipu6_isys_buffer_to_vb2_buffer(ib);
 		vvb = to_vb2_v4l2_buffer(vb);
 		ivb = vb2_buffer_to_ipu6_isys_video_buffer(vvb);
-		addr = ivb->dma_addr;
+		dma_addr = ivb->dma_addr;
 
-		if (info->pin.addr != addr) {
+		if (addr != dma_addr) {
 			if (first)
 				dev_err(dev, "Unexpected buffer address %pad\n",
-					&addr);
+					&dma_addr);
 			first = false;
 			continue;
 		}
 
-		if (info->error_info.error ==
-		    IPU6_FW_ISYS_ERROR_HW_REPORTED_STR2MMIO) {
+		if (error_check) {
 			/*
 			 * Check for error message:
 			 * 'IPU6_FW_ISYS_ERROR_HW_REPORTED_STR2MMIO'
@@ -800,6 +799,15 @@ void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
 	spin_unlock_irqrestore(&aq->lock, flags);
 }
 
+void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
+			       struct ipu6_fw_isys_resp_info_abi *info)
+{
+	u64 time = (u64)info->timestamp[1] << 32 | info->timestamp[0];
+	bool err = (info->error_info.error == IPU6_FW_ISYS_ERROR_HW_REPORTED_STR2MMIO);
+
+	ipu6_stream_buf_ready(stream, info->pin_id, info->pin.addr, time, err);
+}
+
 static const struct vb2_ops ipu6_isys_queue_ops = {
 	.queue_setup = ipu6_isys_queue_setup,
 	.buf_init = ipu6_isys_buf_init,
-- 
2.34.1

[PATCH 5/5] media: intel/ipu6: Remove unused dev field from ipu6_isys_queue

[Stanislaw Gruszka]

With current code base dev is only writend but never read.

Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
---
 drivers/media/pci/intel/ipu6/ipu6-isys-queue.c | 1 -
 drivers/media/pci/intel/ipu6/ipu6-isys-queue.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
index c7f302df2dcc..dd11eae1eeca 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
@@ -841,7 +841,6 @@ int ipu6_isys_queue_init(struct ipu6_isys_queue *aq)
 	if (ret)
 		return ret;
 
-	aq->dev = &adev->auxdev.dev;
 	aq->vbq.dev = &adev->isp->pdev->dev;
 	spin_lock_init(&aq->lock);
 	INIT_LIST_HEAD(&aq->active);
diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
index 1a277b0fb4b5..b865428a0fce 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
@@ -20,7 +20,6 @@ struct ipu6_isys_stream;
 struct ipu6_isys_queue {
 	struct vb2_queue vbq;
 	struct list_head node;
-	struct device *dev;
 	/*
 	 * @lock: serialise access to queued and pre_streamon_queued
 	 */
-- 
2.34.1

Re: [PATCH 5/5] media: intel/ipu6: Remove unused dev field from ipu6_isys_queue

[Sakari Ailus]

On Thu, Mar 13, 2025 at 11:24:27AM +0100, Stanislaw Gruszka wrote:
> With current code base dev is only writend but never read.

s/writend/written/

Maybe add: Remove it as redundant.

> 
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
>  drivers/media/pci/intel/ipu6/ipu6-isys-queue.c | 1 -
>  drivers/media/pci/intel/ipu6/ipu6-isys-queue.h | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> index c7f302df2dcc..dd11eae1eeca 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> @@ -841,7 +841,6 @@ int ipu6_isys_queue_init(struct ipu6_isys_queue *aq)
>  	if (ret)
>  		return ret;
>  
> -	aq->dev = &adev->auxdev.dev;
>  	aq->vbq.dev = &adev->isp->pdev->dev;
>  	spin_lock_init(&aq->lock);
>  	INIT_LIST_HEAD(&aq->active);
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
> index 1a277b0fb4b5..b865428a0fce 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
> @@ -20,7 +20,6 @@ struct ipu6_isys_stream;
>  struct ipu6_isys_queue {
>  	struct vb2_queue vbq;
>  	struct list_head node;
> -	struct device *dev;
>  	/*
>  	 * @lock: serialise access to queued and pre_streamon_queued
>  	 */

-- 
Sakari Ailus

Re: [PATCH 4/5] media: intel/ipu6: Abstart buf ready function

[Sakari Ailus]

On Thu, Mar 13, 2025 at 11:24:26AM +0100, Stanislaw Gruszka wrote:
> Extract values needed by ipu6_queue_buf_ready() function from fw abi
> structure. This will allow to reuse same buf ready code when fw abi
> change.
> 
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
> ---
>  .../media/pci/intel/ipu6/ipu6-isys-queue.c    | 28 ++++++++++++-------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> index c68dc577daf0..c7f302df2dcc 100644
> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> @@ -734,10 +734,11 @@ static void ipu6_isys_queue_buf_done(struct ipu6_isys_buffer *ib)
>  	}
>  }
>  
> -void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
> -			       struct ipu6_fw_isys_resp_info_abi *info)
> +static void
> +ipu6_stream_buf_ready(struct ipu6_isys_stream *stream, u8 pin_id, u32 addr,
> +		      u64 time, bool error_check)
>  {
> -	struct ipu6_isys_queue *aq = stream->output_pins[info->pin_id].aq;
> +	struct ipu6_isys_queue *aq = stream->output_pins[pin_id].aq;
>  	struct ipu6_isys *isys = stream->isys;
>  	struct device *dev = &isys->adev->auxdev.dev;
>  	struct ipu6_isys_buffer *ib;
> @@ -745,7 +746,6 @@ void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
>  	unsigned long flags;
>  	bool first = true;
>  	struct vb2_v4l2_buffer *buf;
> -	u64 time = (u64)info->timestamp[1] << 32 | info->timestamp[0];
>  
>  	spin_lock_irqsave(&aq->lock, flags);
>  	if (list_empty(&aq->active)) {
> @@ -757,23 +757,22 @@ void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
>  	list_for_each_entry_reverse(ib, &aq->active, head) {
>  		struct ipu6_isys_video_buffer *ivb;
>  		struct vb2_v4l2_buffer *vvb;
> -		dma_addr_t addr;
> +		dma_addr_t dma_addr;
>  
>  		vb = ipu6_isys_buffer_to_vb2_buffer(ib);
>  		vvb = to_vb2_v4l2_buffer(vb);
>  		ivb = vb2_buffer_to_ipu6_isys_video_buffer(vvb);
> -		addr = ivb->dma_addr;
> +		dma_addr = ivb->dma_addr;
>  
> -		if (info->pin.addr != addr) {
> +		if (addr != dma_addr) {
>  			if (first)
>  				dev_err(dev, "Unexpected buffer address %pad\n",
> -					&addr);
> +					&dma_addr);
>  			first = false;
>  			continue;
>  		}
>  
> -		if (info->error_info.error ==
> -		    IPU6_FW_ISYS_ERROR_HW_REPORTED_STR2MMIO) {
> +		if (error_check) {
>  			/*
>  			 * Check for error message:
>  			 * 'IPU6_FW_ISYS_ERROR_HW_REPORTED_STR2MMIO'
> @@ -800,6 +799,15 @@ void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
>  	spin_unlock_irqrestore(&aq->lock, flags);
>  }
>  
> +void ipu6_isys_queue_buf_ready(struct ipu6_isys_stream *stream,
> +			       struct ipu6_fw_isys_resp_info_abi *info)
> +{
> +	u64 time = (u64)info->timestamp[1] << 32 | info->timestamp[0];
> +	bool err = (info->error_info.error == IPU6_FW_ISYS_ERROR_HW_REPORTED_STR2MMIO);

Redundant parentheses.

> +
> +	ipu6_stream_buf_ready(stream, info->pin_id, info->pin.addr, time, err);
> +}
> +
>  static const struct vb2_ops ipu6_isys_queue_ops = {
>  	.queue_setup = ipu6_isys_queue_setup,
>  	.buf_init = ipu6_isys_buf_init,

-- 
Sakari Ailus