Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
* [PATCH 0/6] media: iris: misc fixes for fluster, seek and concurrency issues
@ 2025-12-24  6:27 Dikshita Agarwal
  2025-12-24  6:27 ` [PATCH 1/6] media: iris: Add buffer to list only after successful allocation Dikshita Agarwal
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Dikshita Agarwal @ 2025-12-24  6:27 UTC (permalink / raw)
  To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab, Hans Verkuil, Stefan Schmidt
  Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue,
	Dikshita Agarwal, Vishnu Reddy

This series contains a set of fixes addressing issues uncovered during
fluster test, resolution-change, seek and concurrency on Iris.

Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
Dikshita Agarwal (5):
      media: iris: Add buffer to list only after successful allocation
      media: iris: Skip resolution set on first IPSC
      media: iris: gen1: Destroy internal buffers after FW releases
      Revert "media: iris: Add sanity check for stop streaming"
      media: iris: gen2: Add sanity check for session stop

Vishnu Reddy (1):
      media: iris: Prevent output buffer queuing before stream-on completes

 drivers/media/platform/qcom/iris/iris_buffer.c         |  7 +++++--
 .../media/platform/qcom/iris/iris_hfi_gen1_command.c   |  4 +++-
 .../media/platform/qcom/iris/iris_hfi_gen2_command.c   |  3 +++
 drivers/media/platform/qcom/iris/iris_vb2.c            | 18 ++++++++++--------
 4 files changed, 21 insertions(+), 11 deletions(-)
---
base-commit: 1f2353f5a1af995efbf7bea44341aa0d03460b28
change-id: 20251216-iris-fixes-4be01def1c7f

Best regards,
-- 
Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/6] media: iris: Add buffer to list only after successful allocation
  2025-12-24  6:27 [PATCH 0/6] media: iris: misc fixes for fluster, seek and concurrency issues Dikshita Agarwal
@ 2025-12-24  6:27 ` Dikshita Agarwal
  2025-12-25  8:49   ` Bryan O'Donoghue
  2025-12-24  6:27 ` [PATCH 2/6] media: iris: Skip resolution set on first IPSC Dikshita Agarwal
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Dikshita Agarwal @ 2025-12-24  6:27 UTC (permalink / raw)
  To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab, Hans Verkuil, Stefan Schmidt
  Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue,
	Dikshita Agarwal

Move `list_add_tail()` to after `dma_alloc_attrs()` succeeds when creating
internal buffers. Previously, the buffer was enqueued in `buffers->list`
before the DMA allocation. If the allocation failed, the function returned
`-ENOMEM` while leaving a partially initialized buffer in the list, which
could lead to inconsistent state and potential leaks.

By adding the buffer to the list only after `dma_alloc_attrs()` succeeds,
we ensure the list contains only valid, fully initialized buffers.

Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_buffer.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
index b89b1ee06cce151e7c04a80956380d154643c116..f1f003a787bf22db6f048c9e682ba8ed2f39bc21 100644
--- a/drivers/media/platform/qcom/iris/iris_buffer.c
+++ b/drivers/media/platform/qcom/iris/iris_buffer.c
@@ -351,12 +351,15 @@ static int iris_create_internal_buffer(struct iris_inst *inst,
 	buffer->index = index;
 	buffer->buffer_size = buffers->size;
 	buffer->dma_attrs = DMA_ATTR_WRITE_COMBINE | DMA_ATTR_NO_KERNEL_MAPPING;
-	list_add_tail(&buffer->list, &buffers->list);
 
 	buffer->kvaddr = dma_alloc_attrs(core->dev, buffer->buffer_size,
 					 &buffer->device_addr, GFP_KERNEL, buffer->dma_attrs);
-	if (!buffer->kvaddr)
+	if (!buffer->kvaddr) {
+		kfree(buffer);
 		return -ENOMEM;
+	}
+
+	list_add_tail(&buffer->list, &buffers->list);
 
 	return 0;
 }

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/6] media: iris: Skip resolution set on first IPSC
  2025-12-24  6:27 [PATCH 0/6] media: iris: misc fixes for fluster, seek and concurrency issues Dikshita Agarwal
  2025-12-24  6:27 ` [PATCH 1/6] media: iris: Add buffer to list only after successful allocation Dikshita Agarwal
@ 2025-12-24  6:27 ` Dikshita Agarwal
  2025-12-25  8:51   ` Bryan O'Donoghue
  2025-12-24  6:27 ` [PATCH 3/6] media: iris: gen1: Destroy internal buffers after FW releases Dikshita Agarwal
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Dikshita Agarwal @ 2025-12-24  6:27 UTC (permalink / raw)
  To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab, Hans Verkuil, Stefan Schmidt
  Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue,
	Dikshita Agarwal

The resolution property is not supposed to be set during reconfig.
Existing iris_drc_pending(inst) check is insufficient, as it doesn't
cover the first port setting change.

Extend the conditional check to also skip resolution setting when
the instance is in IRIS_INST_SUB_FIRST_IPSC.

Fixes: caf205548769 ("media: iris: Avoid updating frame size to firmware during reconfig")
Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
index 52da7ef7bab08fb1cb2ac804ccc6e3c7f9677890..5087e51daa842515e9d62730680fb237bf274efa 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
@@ -733,7 +733,7 @@ static int iris_hfi_gen1_set_resolution(struct iris_inst *inst, u32 plane)
 	struct hfi_framesize fs;
 	int ret;
 
-	if (!iris_drc_pending(inst)) {
+	if (!iris_drc_pending(inst) && !(inst->sub_state & IRIS_INST_SUB_FIRST_IPSC)) {
 		fs.buffer_type = HFI_BUFFER_INPUT;
 		fs.width = inst->fmt_src->fmt.pix_mp.width;
 		fs.height = inst->fmt_src->fmt.pix_mp.height;

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/6] media: iris: gen1: Destroy internal buffers after FW releases
  2025-12-24  6:27 [PATCH 0/6] media: iris: misc fixes for fluster, seek and concurrency issues Dikshita Agarwal
  2025-12-24  6:27 ` [PATCH 1/6] media: iris: Add buffer to list only after successful allocation Dikshita Agarwal
  2025-12-24  6:27 ` [PATCH 2/6] media: iris: Skip resolution set on first IPSC Dikshita Agarwal
@ 2025-12-24  6:27 ` Dikshita Agarwal
  2025-12-28 15:51   ` Bryan O'Donoghue
  2025-12-24  6:27 ` [PATCH 4/6] Revert "media: iris: Add sanity check for stop streaming" Dikshita Agarwal
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Dikshita Agarwal @ 2025-12-24  6:27 UTC (permalink / raw)
  To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab, Hans Verkuil, Stefan Schmidt
  Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue,
	Dikshita Agarwal

After the firmware releases internal buffers, the driver was not
destroying them. This left stale allocations that were no longer used,
especially across resolution changes where new buffers are allocated per
the updated requirements. As a result, memory was wasted until session
close.

Destroy internal buffers once the release response is received from the
firmware.

Fixes: 73702f45db81 ("media: iris: allocate, initialize and queue internal buffers")
Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
index 5087e51daa842515e9d62730680fb237bf274efa..5ff71e25597b61587c674142feb99626e402c893 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
@@ -441,6 +441,8 @@ static int iris_hfi_gen1_session_unset_buffers(struct iris_inst *inst, struct ir
 		goto exit;
 
 	ret = iris_wait_for_session_response(inst, false);
+	if (!ret)
+		ret = iris_destroy_internal_buffer(inst, buf);
 
 exit:
 	kfree(pkt);

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/6] Revert "media: iris: Add sanity check for stop streaming"
  2025-12-24  6:27 [PATCH 0/6] media: iris: misc fixes for fluster, seek and concurrency issues Dikshita Agarwal
                   ` (2 preceding siblings ...)
  2025-12-24  6:27 ` [PATCH 3/6] media: iris: gen1: Destroy internal buffers after FW releases Dikshita Agarwal
@ 2025-12-24  6:27 ` Dikshita Agarwal
  2025-12-28 15:53   ` Bryan O'Donoghue
  2025-12-24  6:27 ` [PATCH 5/6] media: iris: gen2: Add sanity check for session stop Dikshita Agarwal
  2025-12-24  6:27 ` [PATCH 6/6] media: iris: Prevent output buffer queuing before stream-on completes Dikshita Agarwal
  5 siblings, 1 reply; 12+ messages in thread
From: Dikshita Agarwal @ 2025-12-24  6:27 UTC (permalink / raw)
  To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab, Hans Verkuil, Stefan Schmidt
  Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue,
	Dikshita Agarwal

Revert the check that skipped stop_streaming when the instance was in
IRIS_INST_ERROR, as it caused multiple regressions:

1. Buffers were not returned to vb2 when the instance was already in
   error state, triggering warnings in the vb2 core because buffer
   completion was skipped.

2. If a session failed early (e.g. unsupported configuration), the
   instance transitioned to IRIS_INST_ERROR. When userspace attempted
   to stop streaming for cleanup, stop_streaming was skipped due to the
   added check, preventing proper teardown and leaving the firmware
   in an inconsistent state.

Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_vb2.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_vb2.c b/drivers/media/platform/qcom/iris/iris_vb2.c
index db8768d8a8f61c9ceb04e423d0a769d35114e20e..139b821f7952feb33b21a7045aef9e8a4782aa3c 100644
--- a/drivers/media/platform/qcom/iris/iris_vb2.c
+++ b/drivers/media/platform/qcom/iris/iris_vb2.c
@@ -231,8 +231,6 @@ void iris_vb2_stop_streaming(struct vb2_queue *q)
 		return;
 
 	mutex_lock(&inst->lock);
-	if (inst->state == IRIS_INST_ERROR)
-		goto exit;
 
 	if (!V4L2_TYPE_IS_OUTPUT(q->type) &&
 	    !V4L2_TYPE_IS_CAPTURE(q->type))
@@ -243,10 +241,10 @@ void iris_vb2_stop_streaming(struct vb2_queue *q)
 		goto exit;
 
 exit:
-	if (ret) {
-		iris_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
+	iris_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
+	if (ret)
 		iris_inst_change_state(inst, IRIS_INST_ERROR);
-	}
+
 	mutex_unlock(&inst->lock);
 }
 

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/6] media: iris: gen2: Add sanity check for session stop
  2025-12-24  6:27 [PATCH 0/6] media: iris: misc fixes for fluster, seek and concurrency issues Dikshita Agarwal
                   ` (3 preceding siblings ...)
  2025-12-24  6:27 ` [PATCH 4/6] Revert "media: iris: Add sanity check for stop streaming" Dikshita Agarwal
@ 2025-12-24  6:27 ` Dikshita Agarwal
  2025-12-28 15:55   ` Bryan O'Donoghue
  2025-12-24  6:27 ` [PATCH 6/6] media: iris: Prevent output buffer queuing before stream-on completes Dikshita Agarwal
  5 siblings, 1 reply; 12+ messages in thread
From: Dikshita Agarwal @ 2025-12-24  6:27 UTC (permalink / raw)
  To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab, Hans Verkuil, Stefan Schmidt
  Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue,
	Dikshita Agarwal

In iris_kill_session, inst->state is set to IRIS_INST_ERROR and
session_close is executed, which will kfree(inst_hfi_gen2->packet).
If stop_streaming is called afterward, it will cause a crash.

Add a NULL check for inst_hfi_gen2->packet before sendling STOP packet
to firmware to fix that.

Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
index 6a772db2ec33fb002d8884753a41dc98b3a8439d..59e41adcce9aadd7c60bb1d369d68a4954f62aef 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
@@ -963,6 +963,9 @@ static int iris_hfi_gen2_session_stop(struct iris_inst *inst, u32 plane)
 	struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
 	int ret = 0;
 
+	if (!inst_hfi_gen2->packet)
+		return -EINVAL;
+
 	reinit_completion(&inst->completion);
 
 	iris_hfi_gen2_packet_session_command(inst,

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/6] media: iris: Prevent output buffer queuing before stream-on completes
  2025-12-24  6:27 [PATCH 0/6] media: iris: misc fixes for fluster, seek and concurrency issues Dikshita Agarwal
                   ` (4 preceding siblings ...)
  2025-12-24  6:27 ` [PATCH 5/6] media: iris: gen2: Add sanity check for session stop Dikshita Agarwal
@ 2025-12-24  6:27 ` Dikshita Agarwal
  5 siblings, 0 replies; 12+ messages in thread
From: Dikshita Agarwal @ 2025-12-24  6:27 UTC (permalink / raw)
  To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
	Mauro Carvalho Chehab, Hans Verkuil, Stefan Schmidt
  Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue,
	Dikshita Agarwal, Vishnu Reddy

From: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>

During normal playback, stream-on for input is followed by output, and
only after input stream-on does actual streaming begin. However, when
gst-play performs a seek, both input and output streams are stopped,
and on restart, output stream-on occurs first. At this point, firmware
has not yet started streaming. Queuing output buffers before the firmware
begins streaming causes it to process buffers in an invalid state, leading
to an error response. These buffers are returned to the driver as errors,
forcing the driver into an error state and stopping playback.

Fix this by deferring output buffer queuing until stream-on completes.
Input buffers can still be queued before stream-on as required.

Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
 drivers/media/platform/qcom/iris/iris_vb2.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_vb2.c b/drivers/media/platform/qcom/iris/iris_vb2.c
index 139b821f7952feb33b21a7045aef9e8a4782aa3c..bf0b8400996ece5c9d449b99609a302da726bf9a 100644
--- a/drivers/media/platform/qcom/iris/iris_vb2.c
+++ b/drivers/media/platform/qcom/iris/iris_vb2.c
@@ -193,10 +193,14 @@ int iris_vb2_start_streaming(struct vb2_queue *q, unsigned int count)
 	buf_type = iris_v4l2_type_to_driver(q->type);
 
 	if (inst->domain == DECODER) {
-		if (inst->state == IRIS_INST_STREAMING)
+		if (buf_type == BUF_INPUT)
+			ret = iris_queue_deferred_buffers(inst, BUF_INPUT);
+
+		if (!ret && inst->state == IRIS_INST_STREAMING) {
 			ret = iris_queue_internal_deferred_buffers(inst, BUF_DPB);
-		if (!ret)
-			ret = iris_queue_deferred_buffers(inst, buf_type);
+			if (!ret)
+				ret = iris_queue_deferred_buffers(inst, BUF_OUTPUT);
+		}
 	} else {
 		if (inst->state == IRIS_INST_STREAMING) {
 			ret = iris_queue_deferred_buffers(inst, BUF_INPUT);

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/6] media: iris: Add buffer to list only after successful allocation
  2025-12-24  6:27 ` [PATCH 1/6] media: iris: Add buffer to list only after successful allocation Dikshita Agarwal
@ 2025-12-25  8:49   ` Bryan O'Donoghue
  0 siblings, 0 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2025-12-25  8:49 UTC (permalink / raw)
  To: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
	Bryan O'Donoghue, Mauro Carvalho Chehab, Hans Verkuil,
	Stefan Schmidt
  Cc: linux-media, linux-arm-msm, linux-kernel

On 24/12/2025 06:27, Dikshita Agarwal wrote:
> Move `list_add_tail()` to after `dma_alloc_attrs()` succeeds when creating
> internal buffers. Previously, the buffer was enqueued in `buffers->list`
> before the DMA allocation. If the allocation failed, the function returned
> `-ENOMEM` while leaving a partially initialized buffer in the list, which
> could lead to inconsistent state and potential leaks.
> 
> By adding the buffer to the list only after `dma_alloc_attrs()` succeeds,
> we ensure the list contains only valid, fully initialized buffers.
> 
> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/iris/iris_buffer.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
> index b89b1ee06cce151e7c04a80956380d154643c116..f1f003a787bf22db6f048c9e682ba8ed2f39bc21 100644
> --- a/drivers/media/platform/qcom/iris/iris_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
> @@ -351,12 +351,15 @@ static int iris_create_internal_buffer(struct iris_inst *inst,
>   	buffer->index = index;
>   	buffer->buffer_size = buffers->size;
>   	buffer->dma_attrs = DMA_ATTR_WRITE_COMBINE | DMA_ATTR_NO_KERNEL_MAPPING;
> -	list_add_tail(&buffer->list, &buffers->list);
>   
>   	buffer->kvaddr = dma_alloc_attrs(core->dev, buffer->buffer_size,
>   					 &buffer->device_addr, GFP_KERNEL, buffer->dma_attrs);
> -	if (!buffer->kvaddr)
> +	if (!buffer->kvaddr) {
> +		kfree(buffer);
>   		return -ENOMEM;
> +	}
> +
> +	list_add_tail(&buffer->list, &buffers->list);
>   
>   	return 0;
>   }
> 

Missing an appropriate Fixes: tag.

Reviewed-by: Bryan O'Donoghue <bod@kernel.org>

---
bod

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/6] media: iris: Skip resolution set on first IPSC
  2025-12-24  6:27 ` [PATCH 2/6] media: iris: Skip resolution set on first IPSC Dikshita Agarwal
@ 2025-12-25  8:51   ` Bryan O'Donoghue
  0 siblings, 0 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2025-12-25  8:51 UTC (permalink / raw)
  To: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
	Bryan O'Donoghue, Mauro Carvalho Chehab, Hans Verkuil,
	Stefan Schmidt
  Cc: linux-media, linux-arm-msm, linux-kernel

On 24/12/2025 06:27, Dikshita Agarwal wrote:
> The resolution property is not supposed to be set during reconfig.
> Existing iris_drc_pending(inst) check is insufficient, as it doesn't
> cover the first port setting change.
> 
> Extend the conditional check to also skip resolution setting when
> the instance is in IRIS_INST_SUB_FIRST_IPSC.
> 
> Fixes: caf205548769 ("media: iris: Avoid updating frame size to firmware during reconfig")
> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> index 52da7ef7bab08fb1cb2ac804ccc6e3c7f9677890..5087e51daa842515e9d62730680fb237bf274efa 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> @@ -733,7 +733,7 @@ static int iris_hfi_gen1_set_resolution(struct iris_inst *inst, u32 plane)
>   	struct hfi_framesize fs;
>   	int ret;
>   
> -	if (!iris_drc_pending(inst)) {
> +	if (!iris_drc_pending(inst) && !(inst->sub_state & IRIS_INST_SUB_FIRST_IPSC)) {
>   		fs.buffer_type = HFI_BUFFER_INPUT;
>   		fs.width = inst->fmt_src->fmt.pix_mp.width;
>   		fs.height = inst->fmt_src->fmt.pix_mp.height;
> 

Take this on trust I guess.

Reviewed-by: Bryan O'Donoghue <bod@kernel.org>

---
bod

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/6] media: iris: gen1: Destroy internal buffers after FW releases
  2025-12-24  6:27 ` [PATCH 3/6] media: iris: gen1: Destroy internal buffers after FW releases Dikshita Agarwal
@ 2025-12-28 15:51   ` Bryan O'Donoghue
  0 siblings, 0 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2025-12-28 15:51 UTC (permalink / raw)
  To: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
	Mauro Carvalho Chehab, Hans Verkuil, Stefan Schmidt
  Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue

On 24/12/2025 06:27, Dikshita Agarwal wrote:
> After the firmware releases internal buffers, the driver was not
> destroying them. This left stale allocations that were no longer used,
> especially across resolution changes where new buffers are allocated per
> the updated requirements. As a result, memory was wasted until session
> close.
> 
> Destroy internal buffers once the release response is received from the
> firmware.
> 
> Fixes: 73702f45db81 ("media: iris: allocate, initialize and queue internal buffers")
> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> index 5087e51daa842515e9d62730680fb237bf274efa..5ff71e25597b61587c674142feb99626e402c893 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c
> @@ -441,6 +441,8 @@ static int iris_hfi_gen1_session_unset_buffers(struct iris_inst *inst, struct ir
>   		goto exit;
> 
>   	ret = iris_wait_for_session_response(inst, false);
> +	if (!ret)
> +		ret = iris_destroy_internal_buffer(inst, buf);
> 
>   exit:
>   	kfree(pkt);
> 
> --
> 2.34.1
> 

Seems resonable.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

---
bod

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/6] Revert "media: iris: Add sanity check for stop streaming"
  2025-12-24  6:27 ` [PATCH 4/6] Revert "media: iris: Add sanity check for stop streaming" Dikshita Agarwal
@ 2025-12-28 15:53   ` Bryan O'Donoghue
  0 siblings, 0 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2025-12-28 15:53 UTC (permalink / raw)
  To: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
	Mauro Carvalho Chehab, Hans Verkuil, Stefan Schmidt
  Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue

On 24/12/2025 06:27, Dikshita Agarwal wrote:
> Revert the check that skipped stop_streaming when the instance was in
> IRIS_INST_ERROR, as it caused multiple regressions:
> 
> 1. Buffers were not returned to vb2 when the instance was already in
>     error state, triggering warnings in the vb2 core because buffer
>     completion was skipped.
> 
> 2. If a session failed early (e.g. unsupported configuration), the
>     instance transitioned to IRIS_INST_ERROR. When userspace attempted
>     to stop streaming for cleanup, stop_streaming was skipped due to the
>     added check, preventing proper teardown and leaving the firmware
>     in an inconsistent state.
> 
> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/iris/iris_vb2.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_vb2.c b/drivers/media/platform/qcom/iris/iris_vb2.c
> index db8768d8a8f61c9ceb04e423d0a769d35114e20e..139b821f7952feb33b21a7045aef9e8a4782aa3c 100644
> --- a/drivers/media/platform/qcom/iris/iris_vb2.c
> +++ b/drivers/media/platform/qcom/iris/iris_vb2.c
> @@ -231,8 +231,6 @@ void iris_vb2_stop_streaming(struct vb2_queue *q)
>   		return;
> 
>   	mutex_lock(&inst->lock);
> -	if (inst->state == IRIS_INST_ERROR)
> -		goto exit;
> 
>   	if (!V4L2_TYPE_IS_OUTPUT(q->type) &&
>   	    !V4L2_TYPE_IS_CAPTURE(q->type))
> @@ -243,10 +241,10 @@ void iris_vb2_stop_streaming(struct vb2_queue *q)
>   		goto exit;
> 
>   exit:
> -	if (ret) {
> -		iris_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
> +	iris_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
> +	if (ret)
>   		iris_inst_change_state(inst, IRIS_INST_ERROR);
> -	}
> +
>   	mutex_unlock(&inst->lock);
>   }
> 
> 
> --
> 2.34.1
> 

A revert like this should very obliviously have a Fixes:

---
bod

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/6] media: iris: gen2: Add sanity check for session stop
  2025-12-24  6:27 ` [PATCH 5/6] media: iris: gen2: Add sanity check for session stop Dikshita Agarwal
@ 2025-12-28 15:55   ` Bryan O'Donoghue
  0 siblings, 0 replies; 12+ messages in thread
From: Bryan O'Donoghue @ 2025-12-28 15:55 UTC (permalink / raw)
  To: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
	Mauro Carvalho Chehab, Hans Verkuil, Stefan Schmidt
  Cc: linux-media, linux-arm-msm, linux-kernel, Bryan O'Donoghue

On 24/12/2025 06:27, Dikshita Agarwal wrote:
> In iris_kill_session, inst->state is set to IRIS_INST_ERROR and
> session_close is executed, which will kfree(inst_hfi_gen2->packet).
> If stop_streaming is called afterward, it will cause a crash.
> 
> Add a NULL check for inst_hfi_gen2->packet before sendling STOP packet
> to firmware to fix that.
> 
> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> index 6a772db2ec33fb002d8884753a41dc98b3a8439d..59e41adcce9aadd7c60bb1d369d68a4954f62aef 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c
> @@ -963,6 +963,9 @@ static int iris_hfi_gen2_session_stop(struct iris_inst *inst, u32 plane)
>   	struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
>   	int ret = 0;
> 
> +	if (!inst_hfi_gen2->packet)
> +		return -EINVAL;
> +
>   	reinit_completion(&inst->completion);
> 
>   	iris_hfi_gen2_packet_session_command(inst,
> 
> --
> 2.34.1
> 

Most of these patches need Fixes: please add.

---
bod

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-12-28 15:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-24  6:27 [PATCH 0/6] media: iris: misc fixes for fluster, seek and concurrency issues Dikshita Agarwal
2025-12-24  6:27 ` [PATCH 1/6] media: iris: Add buffer to list only after successful allocation Dikshita Agarwal
2025-12-25  8:49   ` Bryan O'Donoghue
2025-12-24  6:27 ` [PATCH 2/6] media: iris: Skip resolution set on first IPSC Dikshita Agarwal
2025-12-25  8:51   ` Bryan O'Donoghue
2025-12-24  6:27 ` [PATCH 3/6] media: iris: gen1: Destroy internal buffers after FW releases Dikshita Agarwal
2025-12-28 15:51   ` Bryan O'Donoghue
2025-12-24  6:27 ` [PATCH 4/6] Revert "media: iris: Add sanity check for stop streaming" Dikshita Agarwal
2025-12-28 15:53   ` Bryan O'Donoghue
2025-12-24  6:27 ` [PATCH 5/6] media: iris: gen2: Add sanity check for session stop Dikshita Agarwal
2025-12-28 15:55   ` Bryan O'Donoghue
2025-12-24  6:27 ` [PATCH 6/6] media: iris: Prevent output buffer queuing before stream-on completes Dikshita Agarwal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox