On Fri Jul 4 11:21:08 2025 +0200, Hans de Goede wrote:
> The atomisp interrupt handling will free the MIPI / CSI-receiver buffers
> when processing a frame-completion event if the stop_requested flag is set,
> but only in the ISP2400 / BYT, not in the ISP2401 / CHT case.
>
> There are 2 problems with this:
>
> 1. Since this is only done in the BYT case the "mipi frames are not freed."
> warning always triggers on CHT devices.
>
> 2. There are 2 stop_requested flags, ia_css_pipe.stop_requested and
> ia_css_pipeline.stop_requested. The ISR checks the ia_css_pipe flag,
> but atomisp_css_stop() sets the ia_css_pipeline.stop_requested flag.
> So even on BYT freeing the buffers from the ISR never happens.
>
> This likely is a good thing since the buffers get freed on the first
> frame completion event and there might be multiple frames queued up.
>
> Fix things by completely dropping the freeing of the MIPI buffers from
> the ISR as well as the stop_requested flag always freeing the buffers
> from ia_css_uninit().
>
> Also drop the warning since this now always is expected behavior.
>
> Note that ia_css_uninit() get called whenever streaming is stopped
> through atomisp_stop_stream() calling atomisp_reset() so the buffers
> are still freed whenever streaming is stopped.
>
> Signed-off-by: Hans de Goede <ha...@kernel.org>
> Reviewed-by: Andy Shevchenko <a...@kernel.org>
> Link: https://lore.kernel.org/r/20250505210008.152659-5-hdego...@redhat.com
> Signed-off-by: Mauro Carvalho Chehab <mchehab+hua...@kernel.org>
Patch committed.
Thanks,
Mauro Carvalho Chehab
drivers/staging/media/atomisp/pci/ia_css_pipe.h | 2 --
.../runtime/pipeline/interface/ia_css_pipeline.h | 1 -
.../atomisp/pci/runtime/pipeline/src/pipeline.c | 2 --
drivers/staging/media/atomisp/pci/sh_css.c | 27 ----------------------
drivers/staging/media/atomisp/pci/sh_css_mipi.c | 11 ---------
drivers/staging/media/atomisp/pci/sh_css_mipi.h | 2 --
6 files changed, 45 deletions(-)
---
diff --git a/drivers/staging/media/atomisp/pci/ia_css_pipe.h
b/drivers/staging/media/atomisp/pci/ia_css_pipe.h
index c97d2ae356fd..77072694eb42 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_pipe.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_pipe.h
@@ -102,8 +102,6 @@ struct ia_css_yuvpp_settings {
struct osys_object;
struct ia_css_pipe {
- /* TODO: Remove stop_requested and use stop_requested in the pipeline */
- bool stop_requested;
struct ia_css_pipe_config config;
struct ia_css_pipe_extra_config extra_config;
struct ia_css_pipe_info info;
diff --git
a/drivers/staging/media/atomisp/pci/runtime/pipeline/interface/ia_css_pipeline.h
b/drivers/staging/media/atomisp/pci/runtime/pipeline/interface/ia_css_pipeline.h
index 316eaa2070ea..8b7cbf31a1a2 100644
---
a/drivers/staging/media/atomisp/pci/runtime/pipeline/interface/ia_css_pipeline.h
+++
b/drivers/staging/media/atomisp/pci/runtime/pipeline/interface/ia_css_pipeline.h
@@ -34,7 +34,6 @@ struct ia_css_pipeline_stage {
struct ia_css_pipeline {
enum ia_css_pipe_id pipe_id;
u8 pipe_num;
- bool stop_requested;
struct ia_css_pipeline_stage *stages;
struct ia_css_pipeline_stage *current_stage;
unsigned int num_stages;
diff --git a/drivers/staging/media/atomisp/pci/runtime/pipeline/src/pipeline.c
b/drivers/staging/media/atomisp/pci/runtime/pipeline/src/pipeline.c
index aabebe61ec77..cb8d652227a7 100644
--- a/drivers/staging/media/atomisp/pci/runtime/pipeline/src/pipeline.c
+++ b/drivers/staging/media/atomisp/pci/runtime/pipeline/src/pipeline.c
@@ -198,7 +198,6 @@ int ia_css_pipeline_request_stop(struct ia_css_pipeline
*pipeline)
ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
"ia_css_pipeline_request_stop() enter:
pipeline=%p\n",
pipeline);
- pipeline->stop_requested = true;
/* Send stop event to the sp*/
/* This needs improvement, stop on all the pipes available
@@ -663,7 +662,6 @@ static void pipeline_init_defaults(
pipeline->pipe_id = pipe_id;
pipeline->stages = NULL;
- pipeline->stop_requested = false;
pipeline->current_stage = NULL;
memcpy(&pipeline->in_frame, &ia_css_default_frame,
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c
b/drivers/staging/media/atomisp/pci/sh_css.c
index 5a8e8e67aa13..73bd87f43a8c 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -2002,10 +2002,6 @@ ia_css_uninit(void)
sh_css_params_free_default_gdc_lut();
- /* TODO: JB: implement decent check and handling of freeing mipi frames
*/
- if (!mipi_is_free())
- dev_warn(atomisp_dev, "mipi frames are not freed.\n");
-
/* cleanup generic data */
sh_css_params_uninit();
ia_css_refcount_uninit();
@@ -3743,23 +3739,6 @@ ia_css_pipe_dequeue_buffer(struct ia_css_pipe *pipe,
case IA_CSS_BUFFER_TYPE_INPUT_FRAME:
case IA_CSS_BUFFER_TYPE_OUTPUT_FRAME:
case IA_CSS_BUFFER_TYPE_SEC_OUTPUT_FRAME:
- if (pipe && pipe->stop_requested) {
- if (!IS_ISP2401) {
- /*
- * free mipi frames only for
old input
- * system for 2401 it is done in
- * ia_css_stream_destroy call
- */
- return_err =
free_mipi_frames(pipe);
- if (return_err) {
-
IA_CSS_LOG("free_mipi_frames() failed");
-
IA_CSS_LEAVE_ERR(return_err);
- return return_err;
- }
- }
- pipe->stop_requested = false;
- }
- fallthrough;
case IA_CSS_BUFFER_TYPE_VF_OUTPUT_FRAME:
case IA_CSS_BUFFER_TYPE_SEC_VF_OUTPUT_FRAME:
frame = (struct ia_css_frame
*)HOST_ADDRESS(ddr_buffer.kernel_ptr);
@@ -4095,8 +4074,6 @@ sh_css_pipe_start(struct ia_css_stream *stream)
return err;
}
- pipe->stop_requested = false;
-
switch (pipe_id) {
case IA_CSS_PIPE_ID_PREVIEW:
err = preview_start(pipe);
@@ -4120,19 +4097,15 @@ sh_css_pipe_start(struct ia_css_stream *stream)
for (i = 1; i < stream->num_pipes && 0 == err ; i++) {
switch (stream->pipes[i]->mode) {
case IA_CSS_PIPE_ID_PREVIEW:
- stream->pipes[i]->stop_requested = false;
err = preview_start(stream->pipes[i]);
break;
case IA_CSS_PIPE_ID_VIDEO:
- stream->pipes[i]->stop_requested = false;
err = video_start(stream->pipes[i]);
break;
case IA_CSS_PIPE_ID_CAPTURE:
- stream->pipes[i]->stop_requested = false;
err = capture_start(stream->pipes[i]);
break;
case IA_CSS_PIPE_ID_YUVPP:
- stream->pipes[i]->stop_requested = false;
err = yuvpp_start(stream->pipes[i]);
break;
default:
diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.c
b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
index 42f14ed853e1..971b685cdb58 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_mipi.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.c
@@ -185,17 +185,6 @@ mipi_init(void)
ref_count_mipi_allocation[i] = 0;
}
-bool mipi_is_free(void)
-{
- unsigned int i;
-
- for (i = 0; i < N_CSI_PORTS; i++)
- if (ref_count_mipi_allocation[i])
- return false;
-
- return true;
-}
-
/*
* @brief Calculate the required MIPI buffer sizes.
* Based on the stream configuration, calculate the
diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.h
b/drivers/staging/media/atomisp/pci/sh_css_mipi.h
index 6f7389f44baa..b3887ee3c75a 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_mipi.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.h
@@ -14,8 +14,6 @@
void
mipi_init(void);
-bool mipi_is_free(void);
-
int
allocate_mipi_frames(struct ia_css_pipe *pipe, struct ia_css_stream_info
*info);
