[PATCH 00/20] media: atomisp: Misc. cleanups / fixes

[PATCH 00/20] media: atomisp: Misc. cleanups / fixes

[Hans de Goede]

Hi All,

Here is another round of atomisp cleanups / fixes + preparation work
for getting rid of the ugly PCI power-management errors in dmesg
when doing (runtime)suspend/resume.

Regards,

Hans


Hans de Goede (20):
  media: atomisp: Silence: 'atomisp_q_one_s3a_buffer: drop one s3a stat
    which has exp_id xx' log messages
  media: atomisp: Remove accelerator pipe creation code
  media: atomisp: Remove unused QOS defines / structure member
  media: atomisp: Flush queue on atomisp_css_start() error
  media: atomisp: Log an error on failing to alloc private-mem
  media: atomisp: Fix deadlock when the /dev/video# node is closed while
    still streaming
  media: atomisp: Remove 2 unused accelerator mode related functions
  media: atomisp: Remove atomisp_css_yuvpp_configure_viewfinder()
    function
  media: atomisp: Remove unused ia_css_frame_*() functions
  media: atomisp: Drop userptr support from hmm
  media: atomisp: Remove double atomisp_mrfld_power_down()/_up() calls
    from atomisp_reset()
  media: atomisp: Remove atomisp_mrfld_power_down()/_up()
  media: atomisp: Remove clearing of config from atomisp_css_uninit()
  media: atomisp: Remove atomisp_css_suspend()/_resume()
  media: atomisp: Remove sw_contex.power_state checks
  media: atomisp: Remove duplication between runtime-pm and normal-pm
    code
  media: atomisp: Move calling of css_[un]init() to power_on()/_off()
  media: atomisp: Remove atomisp_ospm_dphy_down() call from probe error
    path
  media: atomisp: Remove atomisp_ospm_dphy_up()/_down() functions
  media: atomisp_ov2680: Fix 1280x720 -> 1296x736 resolution

 drivers/staging/media/atomisp/i2c/ov2680.h    |  46 +-
 .../staging/media/atomisp/include/hmm/hmm.h   |   1 -
 .../media/atomisp/include/hmm/hmm_bo.h        |   2 -
 .../staging/media/atomisp/pci/atomisp_cmd.c   |  76 +--
 .../staging/media/atomisp/pci/atomisp_cmd.h   |   8 +-
 .../media/atomisp/pci/atomisp_compat.h        |  11 -
 .../media/atomisp/pci/atomisp_compat_css20.c  |  88 +--
 .../staging/media/atomisp/pci/atomisp_fops.c  |  34 +-
 .../media/atomisp/pci/atomisp_internal.h      |   1 -
 .../staging/media/atomisp/pci/atomisp_ioctl.c |  80 ++-
 .../staging/media/atomisp/pci/atomisp_v4l2.c  | 104 +--
 .../pipe/interface/ia_css_pipe_stagedesc.h    |   5 -
 .../pci/camera/pipe/src/pipe_stagedesc.c      |  21 -
 drivers/staging/media/atomisp/pci/hmm/hmm.c   |  19 +-
 .../staging/media/atomisp/pci/hmm/hmm_bo.c    |  54 +-
 .../media/atomisp/pci/ia_css_frame_public.h   |  74 ---
 .../staging/media/atomisp/pci/ia_css_pipe.h   |   3 +-
 .../media/atomisp/pci/ia_css_pipe_public.h    |  69 --
 .../pci/runtime/debug/src/ia_css_debug.c      |   7 -
 .../atomisp/pci/runtime/frame/src/frame.c     | 121 ----
 .../pipeline/interface/ia_css_pipeline.h      |   2 -
 .../pci/runtime/pipeline/src/pipeline.c       |   8 -
 drivers/staging/media/atomisp/pci/sh_css.c    | 608 +-----------------
 .../media/atomisp/pci/sh_css_internal.h       |  13 -
 .../staging/media/atomisp/pci/sh_css_legacy.h |   1 -
 drivers/staging/media/atomisp/pci/sh_css_sp.c |   2 +-
 26 files changed, 121 insertions(+), 1337 deletions(-)

-- 
2.38.1

[PATCH 01/20] media: atomisp: Silence: 'atomisp_q_one_s3a_buffer: drop one s3a stat which has exp_id xx' log messages

[Hans de Goede]

Standard v4l2 userspace apps do not consume the s3a statistics block
data. Until we have a userspace consumer for this (libcamera), which
might also involve changing the API for this, lower the log level
of these messages to dev_dbg() to avoid them filling up the logs.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_fops.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index f3f5b54bd347..21ca276831ce 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -181,8 +181,8 @@ static int atomisp_q_one_s3a_buffer(struct atomisp_sub_device *asd,
 	} else {
 		list_add_tail(&s3a_buf->list, &asd->s3a_stats_in_css);
 		if (s3a_list == &asd->s3a_stats_ready)
-			dev_warn(asd->isp->dev, "%s: drop one s3a stat which has exp_id %d!\n",
-				 __func__, exp_id);
+			dev_dbg(asd->isp->dev, "%s: drop one s3a stat which has exp_id %d!\n",
+				__func__, exp_id);
 	}
 
 	asd->s3a_bufs_in_css[css_pipe_id]++;
-- 
2.38.1

Re: [PATCH 01/20] media: atomisp: Silence: 'atomisp_q_one_s3a_buffer: drop one s3a stat which has exp_id xx' log messages

[Andy Shevchenko]

On Sun, Nov 20, 2022 at 11:40:42PM +0100, Hans de Goede wrote:
> Standard v4l2 userspace apps do not consume the s3a statistics block
> data. Until we have a userspace consumer for this (libcamera), which
> might also involve changing the API for this, lower the log level
> of these messages to dev_dbg() to avoid them filling up the logs.

...

> -			dev_warn(asd->isp->dev, "%s: drop one s3a stat which has exp_id %d!\n",
> -				 __func__, exp_id);
> +			dev_dbg(asd->isp->dev, "%s: drop one s3a stat which has exp_id %d!\n",
> +				__func__, exp_id);

Please, drop __func__ as well. Most probably this can be run on the systems
with Dynamic Debug enabled, meaning that __func__ and __LINE__ can be enabled
at runtime.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 01/20] media: atomisp: Silence: 'atomisp_q_one_s3a_buffer: drop one s3a stat which has exp_id xx' log messages

[Hans de Goede]

Hi,

On 11/21/22 09:54, Andy Shevchenko wrote:
> On Sun, Nov 20, 2022 at 11:40:42PM +0100, Hans de Goede wrote:
>> Standard v4l2 userspace apps do not consume the s3a statistics block
>> data. Until we have a userspace consumer for this (libcamera), which
>> might also involve changing the API for this, lower the log level
>> of these messages to dev_dbg() to avoid them filling up the logs.
> 
> ...
> 
>> -			dev_warn(asd->isp->dev, "%s: drop one s3a stat which has exp_id %d!\n",
>> -				 __func__, exp_id);
>> +			dev_dbg(asd->isp->dev, "%s: drop one s3a stat which has exp_id %d!\n",
>> +				__func__, exp_id);
> 
> Please, drop __func__ as well. Most probably this can be run on the systems
> with Dynamic Debug enabled, meaning that __func__ and __LINE__ can be enabled
> at runtime.

Fixed in my media-atomisp branch.

Regards,

Hans

[PATCH 02/20] media: atomisp: Remove accelerator pipe creation code

[Hans de Goede]

The ATOMISP_ACC_* custom ioctls and the ACC device node have been removed
in commit a5c17adbadcb ("media: atomisp: Remove the ACC device node").

This means that pipe_configs[pipe_id].acc_extension now never gets set
which causes atomisp_compat_css20.c: __create_pipe() to always skip
creation of pipes with a pipe_id of IA_CSS_PIPE_ID_ACC / a mode of
IA_CSS_PIPE_MODE_ACC.

This allows removing of the acc_pipe creation / handling code
from mainly sh_css.c and a bunch of other places.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../media/atomisp/pci/atomisp_compat_css20.c  |  27 +-
 .../staging/media/atomisp/pci/ia_css_pipe.h   |   3 +-
 .../media/atomisp/pci/ia_css_pipe_public.h    |  69 --
 .../pci/runtime/debug/src/ia_css_debug.c      |   7 -
 .../pci/runtime/pipeline/src/pipeline.c       |   8 -
 drivers/staging/media/atomisp/pci/sh_css.c    | 599 +-----------------
 .../media/atomisp/pci/sh_css_internal.h       |   1 -
 .../staging/media/atomisp/pci/sh_css_legacy.h |   1 -
 8 files changed, 6 insertions(+), 709 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index f282572d69da..c59f9e6b1e2b 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -236,18 +236,6 @@ static void __dump_pipe_config(struct atomisp_sub_device *asd,
 		dev_dbg(isp->dev,
 			"pipe_config.isp_pipe_version:%d.\n",
 			p_config->isp_pipe_version);
-		dev_dbg(isp->dev,
-			"pipe_config.acc_extension=%p.\n",
-			p_config->acc_extension);
-		dev_dbg(isp->dev,
-			"pipe_config.acc_stages=%p.\n",
-			p_config->acc_stages);
-		dev_dbg(isp->dev,
-			"pipe_config.num_acc_stages=%d.\n",
-			p_config->num_acc_stages);
-		dev_dbg(isp->dev,
-			"pipe_config.acc_num_execs=%d.\n",
-			p_config->acc_num_execs);
 		dev_dbg(isp->dev,
 			"pipe_config.default_capture_config.capture_mode=%d.\n",
 			p_config->default_capture_config.mode);
@@ -629,10 +617,6 @@ static void __apply_additional_pipe_config(
 		else
 			stream_env->pipe_configs[pipe_id].enable_dz = false;
 		break;
-	case IA_CSS_PIPE_ID_ACC:
-		stream_env->pipe_configs[pipe_id].mode = IA_CSS_PIPE_MODE_ACC;
-		stream_env->pipe_configs[pipe_id].enable_dz = false;
-		break;
 	default:
 		break;
 	}
@@ -644,7 +628,7 @@ static bool is_pipe_valid_to_current_run_mode(struct atomisp_sub_device *asd,
 	if (!asd)
 		return false;
 
-	if (pipe_id == IA_CSS_PIPE_ID_ACC || pipe_id == IA_CSS_PIPE_ID_YUVPP)
+	if (pipe_id == IA_CSS_PIPE_ID_YUVPP)
 		return true;
 
 	if (asd->vfpp) {
@@ -718,12 +702,7 @@ static int __create_pipe(struct atomisp_sub_device *asd,
 	if (pipe_id >= IA_CSS_PIPE_ID_NUM)
 		return -EINVAL;
 
-	if (pipe_id != IA_CSS_PIPE_ID_ACC &&
-	    !stream_env->pipe_configs[pipe_id].output_info[0].res.width)
-		return 0;
-
-	if (pipe_id == IA_CSS_PIPE_ID_ACC &&
-	    !stream_env->pipe_configs[pipe_id].acc_extension)
+	if (!stream_env->pipe_configs[pipe_id].output_info[0].res.width)
 		return 0;
 
 	if (!is_pipe_valid_to_current_run_mode(asd, pipe_id))
@@ -2141,8 +2120,6 @@ static enum ia_css_pipe_mode __pipe_id_to_pipe_mode(
 		return IA_CSS_PIPE_MODE_CAPTURE;
 	case IA_CSS_PIPE_ID_VIDEO:
 		return IA_CSS_PIPE_MODE_VIDEO;
-	case IA_CSS_PIPE_ID_ACC:
-		return IA_CSS_PIPE_MODE_ACC;
 	case IA_CSS_PIPE_ID_YUVPP:
 		return IA_CSS_PIPE_MODE_YUVPP;
 	default:
diff --git a/drivers/staging/media/atomisp/pci/ia_css_pipe.h b/drivers/staging/media/atomisp/pci/ia_css_pipe.h
index fb58535bff40..22522968b9e6 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_pipe.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_pipe.h
@@ -37,7 +37,6 @@ struct ia_css_preview_settings {
 
 	struct ia_css_pipe *copy_pipe;
 	struct ia_css_pipe *capture_pipe;
-	struct ia_css_pipe *acc_pipe;
 };
 
 #define IA_CSS_DEFAULT_PREVIEW_SETTINGS { \
@@ -156,7 +155,7 @@ struct ia_css_pipe {
 #define IA_CSS_DEFAULT_PIPE { \
 	.config			= DEFAULT_PIPE_CONFIG, \
 	.info			= DEFAULT_PIPE_INFO, \
-	.mode			= IA_CSS_PIPE_ID_ACC, /* (pipe_id) */ \
+	.mode			= IA_CSS_PIPE_ID_VIDEO, /* (pipe_id) */ \
 	.pipeline		= DEFAULT_PIPELINE, \
 	.output_info		= {IA_CSS_BINARY_DEFAULT_FRAME_INFO}, \
 	.bds_output_info	= IA_CSS_BINARY_DEFAULT_FRAME_INFO, \
diff --git a/drivers/staging/media/atomisp/pci/ia_css_pipe_public.h b/drivers/staging/media/atomisp/pci/ia_css_pipe_public.h
index 7352cbf779fb..8ac1586dce4e 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_pipe_public.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_pipe_public.h
@@ -45,7 +45,6 @@ enum ia_css_pipe_mode {
 	IA_CSS_PIPE_MODE_PREVIEW,	/** Preview pipe */
 	IA_CSS_PIPE_MODE_VIDEO,		/** Video pipe */
 	IA_CSS_PIPE_MODE_CAPTURE,	/** Still capture pipe */
-	IA_CSS_PIPE_MODE_ACC,		/** Accelerated pipe */
 	IA_CSS_PIPE_MODE_COPY,		/** Copy pipe, only used for embedded/image data copying */
 	IA_CSS_PIPE_MODE_YUVPP,		/** YUV post processing pipe, used for all use cases with YUV input,
 									for SoC sensor and external ISP */
@@ -95,21 +94,11 @@ struct ia_css_pipe_config {
 	/** output of YUV scaling */
 	struct ia_css_frame_info vf_output_info[IA_CSS_PIPE_MAX_OUTPUT_STAGE];
 	/** output of VF YUV scaling */
-	struct ia_css_fw_info *acc_extension;
-	/** Pipeline extension accelerator */
-	struct ia_css_fw_info **acc_stages;
-	/** Standalone accelerator stages */
-	u32 num_acc_stages;
-	/** Number of standalone accelerator stages */
 	struct ia_css_capture_config default_capture_config;
 	/** Default capture config for initial capture pipe configuration. */
 	struct ia_css_resolution dvs_envelope; /** temporary */
 	enum ia_css_frame_delay dvs_frame_delay;
 	/** indicates the DVS loop delay in frame periods */
-	int acc_num_execs;
-	/** For acceleration pipes only: determine how many times the pipe
-	     should be run. Setting this to -1 means it will run until
-	     stopped. */
 	bool enable_dz;
 	/** Disabling digital zoom for a pipeline, if this is set to false,
 	     then setting a zoom factor will have no effect.
@@ -153,7 +142,6 @@ struct ia_css_pipe_config {
 	.vf_output_info		= {IA_CSS_BINARY_DEFAULT_FRAME_INFO}, \
 	.default_capture_config	= DEFAULT_CAPTURE_CONFIG, \
 	.dvs_frame_delay	= IA_CSS_FRAME_DELAY_1, \
-	.acc_num_execs		= -1, \
 }
 
 /* Pipe info, this struct describes properties of a pipe after it's stream has
@@ -224,9 +212,6 @@ struct ia_css_pipe_info {
 		{{0, 0}, 0, 0, 0, 0}, // second_output_info
 		{{0, 0}, 0, 0, 0, 0}, // vf_output_info
 		{{0, 0}, 0, 0, 0, 0}, // second_vf_output_info
-		NULL,   // acc_extension
-		NULL,   // acc_stages
-		0,      // num_acc_stages
 		{
 			IA_CSS_CAPTURE_MODE_RAW, // mode
 			false, // enable_xnr
@@ -234,7 +219,6 @@ struct ia_css_pipe_info {
 		},      // default_capture_config
 		{0, 0}, // dvs_envelope
 		1,      // dvs_frame_delay
-		-1,     // acc_num_execs
 		true,   // enable_dz
 		NULL,   // p_isp_config
 	};
@@ -426,59 +410,6 @@ int
 ia_css_pipe_dequeue_buffer(struct ia_css_pipe *pipe,
 			   struct ia_css_buffer *buffer);
 
-/* @brief  Set the state (Enable or Disable) of the Extension stage in the
- *          given pipe.
- * @param[in] pipe         Pipe handle.
- * @param[in] fw_handle    Extension firmware Handle (ia_css_fw_info.handle)
- * @param[in] enable       Enable Flag (1 to enable ; 0 to disable)
- *
- * @return
- * 0			: Success
- * -EINVAL		: Invalid Parameters
- * -EBUSY	: Inactive QOS Pipe
- *					(No active stream with this pipe)
- *
- * This function will request state change (enable or disable) for the Extension
- * stage (firmware handle) in the given pipe.
- *
- * Note:
- *	1. Extension can be enabled/disabled only on QOS Extensions
- *	2. Extension can be enabled/disabled only with an active QOS Pipe
- *	3. Initial(Default) state of QOS Extensions is Disabled
- *	4. State change cannot be guaranteed immediately OR on frame boundary
- *
- */
-int
-ia_css_pipe_set_qos_ext_state(struct ia_css_pipe *pipe,
-			      u32 fw_handle,
-			      bool  enable);
-
-/* @brief  Get the state (Enable or Disable) of the Extension stage in the
- *          given pipe.
- * @param[in]  pipe        Pipe handle.
- * @param[in]  fw_handle   Extension firmware Handle (ia_css_fw_info.handle)
- * @param[out] *enable     Enable Flag
- *
- * @return
- * 0			: Success
- * -EINVAL		: Invalid Parameters
- * -EBUSY	: Inactive QOS Pipe
- *					(No active stream with this pipe)
- *
- * This function will query the state of the Extension stage (firmware handle)
- * in the given Pipe.
- *
- * Note:
- *	1. Extension state can be queried only on QOS Extensions
- *	2. Extension can be enabled/disabled only with an active QOS Pipe
- *	3. Initial(Default) state of QOS Extensions is Disabled.
- *
- */
-int
-ia_css_pipe_get_qos_ext_state(struct ia_css_pipe *pipe,
-			      u32 fw_handle,
-			      bool *enable);
-
 /* @brief Get selected configuration settings
  * @param[in]	pipe	The pipe.
  * @param[out]	config	Configuration settings.
diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
index c10c2c598179..bb6204cb42c5 100644
--- a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
+++ b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
@@ -133,7 +133,6 @@ static const char *const pipe_id_to_str[] = {
 	/* [IA_CSS_PIPE_ID_VIDEO]     =*/ "video",
 	/* [IA_CSS_PIPE_ID_CAPTURE]   =*/ "capture",
 	/* [IA_CSS_PIPE_ID_YUVPP]     =*/ "yuvpp",
-	/* [IA_CSS_PIPE_ID_ACC]       =*/ "accelerator"
 };
 
 static char dot_id_input_bin[SH_CSS_MAX_BINARY_NAME + 10];
@@ -2989,16 +2988,10 @@ ia_css_debug_dump_pipe_config(
 		ia_css_debug_dump_frame_info(&config->vf_output_info[i],
 					     "vf_output_info");
 	}
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "acc_extension: %p\n",
-			    config->acc_extension);
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "num_acc_stages: %d\n",
-			    config->num_acc_stages);
 	ia_css_debug_dump_capture_config(&config->default_capture_config);
 	ia_css_debug_dump_resolution(&config->dvs_envelope, "dvs_envelope");
 	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "dvs_frame_delay: %d\n",
 			    config->dvs_frame_delay);
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "acc_num_execs: %d\n",
-			    config->acc_num_execs);
 	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "enable_dz: %d\n",
 			    config->enable_dz);
 	IA_CSS_LEAVE_PRIVATE("");
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 dfc50247ea8e..e9e187649a65 100644
--- a/drivers/staging/media/atomisp/pci/runtime/pipeline/src/pipeline.c
+++ b/drivers/staging/media/atomisp/pci/runtime/pipeline/src/pipeline.c
@@ -774,14 +774,6 @@ ia_css_pipeline_configure_inout_port(struct ia_css_pipeline *me,
 					    (uint8_t)SH_CSS_PORT_OUTPUT,
 					    (uint8_t)SH_CSS_HOST_TYPE, 1);
 		break;
-	case IA_CSS_PIPE_ID_ACC:
-		SH_CSS_PIPE_PORT_CONFIG_SET(me->inout_port_config,
-					    (uint8_t)SH_CSS_PORT_INPUT,
-					    (uint8_t)SH_CSS_HOST_TYPE, 1);
-		SH_CSS_PIPE_PORT_CONFIG_SET(me->inout_port_config,
-					    (uint8_t)SH_CSS_PORT_OUTPUT,
-					    (uint8_t)SH_CSS_HOST_TYPE, 1);
-		break;
 	default:
 		break;
 	}
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 5fae96bf447d..33348a43c1de 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -209,13 +209,6 @@ ia_css_pipe_check_format(struct ia_css_pipe *pipe,
 			 enum ia_css_frame_format format);
 
 /* ISP 2401 */
-static int
-ia_css_pipe_load_extension(struct ia_css_pipe *pipe,
-			   struct ia_css_fw_info *firmware);
-
-static void
-ia_css_pipe_unload_extension(struct ia_css_pipe *pipe,
-			     struct ia_css_fw_info *firmware);
 static void
 ia_css_reset_defaults(struct sh_css *css);
 
@@ -286,10 +279,6 @@ static int
 init_out_frameinfo_defaults(struct ia_css_pipe *pipe,
 			    struct ia_css_frame *out_frame, unsigned int idx);
 
-static int
-sh_css_pipeline_add_acc_stage(struct ia_css_pipeline *pipeline,
-			      const void *acc_fw);
-
 static int
 alloc_continuous_frames(struct ia_css_pipe *pipe, bool init_time);
 
@@ -329,9 +318,6 @@ create_host_capture_pipeline(struct ia_css_pipe *pipe);
 static int
 create_host_yuvpp_pipeline(struct ia_css_pipe *pipe);
 
-static int
-create_host_acc_pipeline(struct ia_css_pipe *pipe);
-
 static unsigned int
 sh_css_get_sw_interrupt_value(unsigned int irq);
 
@@ -362,12 +348,6 @@ static struct sh_css_hmm_buffer_record
 *sh_css_hmm_buffer_record_validate(ia_css_ptr ddr_buffer_addr,
 				   enum ia_css_buffer_type type);
 
-void
-ia_css_get_acc_configs(
-    struct ia_css_pipe *pipe,
-    struct ia_css_isp_config *config);
-
-
 #ifdef ISP2401
 static unsigned int get_crop_lines_for_bayer_order(const struct
 	ia_css_stream_config *config);
@@ -1670,7 +1650,6 @@ map_sp_threads(struct ia_css_stream *stream, bool map)
 	struct ia_css_pipe *main_pipe = NULL;
 	struct ia_css_pipe *copy_pipe = NULL;
 	struct ia_css_pipe *capture_pipe = NULL;
-	struct ia_css_pipe *acc_pipe = NULL;
 	int err = 0;
 	enum ia_css_pipe_id pipe_id;
 
@@ -1691,7 +1670,6 @@ map_sp_threads(struct ia_css_stream *stream, bool map)
 	case IA_CSS_PIPE_ID_PREVIEW:
 		copy_pipe    = main_pipe->pipe_settings.preview.copy_pipe;
 		capture_pipe = main_pipe->pipe_settings.preview.capture_pipe;
-		acc_pipe     = main_pipe->pipe_settings.preview.acc_pipe;
 		break;
 
 	case IA_CSS_PIPE_ID_VIDEO:
@@ -1700,14 +1678,10 @@ map_sp_threads(struct ia_css_stream *stream, bool map)
 		break;
 
 	case IA_CSS_PIPE_ID_CAPTURE:
-	case IA_CSS_PIPE_ID_ACC:
 	default:
 		break;
 	}
 
-	if (acc_pipe)
-		ia_css_pipeline_map(acc_pipe->pipe_num, map);
-
 	if (capture_pipe)
 		ia_css_pipeline_map(capture_pipe->pipe_num, map);
 
@@ -1735,7 +1709,6 @@ static int
 create_host_pipeline_structure(struct ia_css_stream *stream)
 {
 	struct ia_css_pipe *copy_pipe = NULL, *capture_pipe = NULL;
-	struct ia_css_pipe *acc_pipe = NULL;
 	enum ia_css_pipe_id pipe_id;
 	struct ia_css_pipe *main_pipe = NULL;
 	int err = 0;
@@ -1763,7 +1736,6 @@ create_host_pipeline_structure(struct ia_css_stream *stream)
 		copy_pipe_delay = main_pipe->dvs_frame_delay;
 		capture_pipe = main_pipe->pipe_settings.preview.capture_pipe;
 		capture_pipe_delay = IA_CSS_FRAME_DELAY_0;
-		acc_pipe     = main_pipe->pipe_settings.preview.acc_pipe;
 		err = ia_css_pipeline_create(&main_pipe->pipeline, main_pipe->mode,
 					     main_pipe->pipe_num, main_pipe->dvs_frame_delay);
 		break;
@@ -1787,11 +1759,6 @@ create_host_pipeline_structure(struct ia_css_stream *stream)
 					     main_pipe->pipe_num, main_pipe->dvs_frame_delay);
 		break;
 
-	case IA_CSS_PIPE_ID_ACC:
-		err = ia_css_pipeline_create(&main_pipe->pipeline, main_pipe->mode,
-					     main_pipe->pipe_num, main_pipe->dvs_frame_delay);
-		break;
-
 	default:
 		err = -EINVAL;
 	}
@@ -1808,10 +1775,6 @@ create_host_pipeline_structure(struct ia_css_stream *stream)
 					     capture_pipe->pipe_num,
 					     capture_pipe_delay);
 
-	if (!(err) && acc_pipe)
-		err = ia_css_pipeline_create(&acc_pipe->pipeline, acc_pipe->mode,
-					     acc_pipe->pipe_num, main_pipe->dvs_frame_delay);
-
 	/* DH regular multi pipe - not continuous mode: create the next pipelines too */
 	if (!stream->config.continuous) {
 		int i;
@@ -1837,7 +1800,6 @@ static int
 create_host_pipeline(struct ia_css_stream *stream)
 {
 	struct ia_css_pipe *copy_pipe = NULL, *capture_pipe = NULL;
-	struct ia_css_pipe *acc_pipe = NULL;
 	enum ia_css_pipe_id pipe_id;
 	struct ia_css_pipe *main_pipe = NULL;
 	int err = 0;
@@ -1881,27 +1843,17 @@ create_host_pipeline(struct ia_css_stream *stream)
 		}
 	}
 
-#if !defined(ISP2401)
 	/* old isys: need to allocate_mipi_frames() even in IA_CSS_PIPE_MODE_COPY */
-	if (pipe_id != IA_CSS_PIPE_ID_ACC) {
-		err = allocate_mipi_frames(main_pipe, &stream->info);
-		if (err)
-			goto ERR;
-	}
-#elif defined(ISP2401)
-	if ((pipe_id != IA_CSS_PIPE_ID_ACC) &&
-	    (main_pipe->config.mode != IA_CSS_PIPE_MODE_COPY)) {
+	if (!IS_ISP2401 || main_pipe->config.mode != IA_CSS_PIPE_MODE_COPY) {
 		err = allocate_mipi_frames(main_pipe, &stream->info);
 		if (err)
 			goto ERR;
 	}
-#endif
 
 	switch (pipe_id) {
 	case IA_CSS_PIPE_ID_PREVIEW:
 		copy_pipe    = main_pipe->pipe_settings.preview.copy_pipe;
 		capture_pipe = main_pipe->pipe_settings.preview.capture_pipe;
-		acc_pipe     = main_pipe->pipe_settings.preview.acc_pipe;
 		max_input_width =
 		    main_pipe->pipe_settings.preview.preview_binary.info->sp.input.max_width;
 
@@ -1935,12 +1887,6 @@ create_host_pipeline(struct ia_css_stream *stream)
 
 		break;
 
-	case IA_CSS_PIPE_ID_ACC:
-		err = create_host_acc_pipeline(main_pipe);
-		if (err)
-			goto ERR;
-
-		break;
 	default:
 		err = -EINVAL;
 	}
@@ -1960,12 +1906,6 @@ create_host_pipeline(struct ia_css_stream *stream)
 			goto ERR;
 	}
 
-	if (acc_pipe) {
-		err = create_host_acc_pipeline(acc_pipe);
-		if (err)
-			goto ERR;
-	}
-
 	/* DH regular multi pipe - not continuous mode: create the next pipelines too */
 	if (!stream->config.continuous) {
 		int i;
@@ -1984,9 +1924,6 @@ create_host_pipeline(struct ia_css_stream *stream)
 			case IA_CSS_PIPE_ID_YUVPP:
 				err = create_host_yuvpp_pipeline(stream->pipes[i]);
 				break;
-			case IA_CSS_PIPE_ID_ACC:
-				err = create_host_acc_pipeline(stream->pipes[i]);
-				break;
 			default:
 				err = -EINVAL;
 			}
@@ -2037,9 +1974,6 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
 		pipe->mode = IA_CSS_PIPE_ID_VIDEO;
 		memcpy(&pipe->pipe_settings.video, &video, sizeof(video));
 		break;
-	case IA_CSS_PIPE_MODE_ACC:
-		pipe->mode = IA_CSS_PIPE_ID_ACC;
-		break;
 	case IA_CSS_PIPE_MODE_COPY:
 		pipe->mode = IA_CSS_PIPE_ID_CAPTURE;
 		break;
@@ -2156,27 +2090,6 @@ find_pipe_by_num(uint32_t pipe_num)
 	return NULL;
 }
 
-static void sh_css_pipe_free_acc_binaries(
-    struct ia_css_pipe *pipe)
-{
-	struct ia_css_pipeline *pipeline;
-	struct ia_css_pipeline_stage *stage;
-
-	if (!pipe) {
-		IA_CSS_ERROR("NULL input pointer");
-		return;
-	}
-	pipeline = &pipe->pipeline;
-
-	/* loop through the stages and unload them */
-	for (stage = pipeline->stages; stage; stage = stage->next) {
-		struct ia_css_fw_info *firmware = (struct ia_css_fw_info *)
-						  stage->firmware;
-		if (firmware)
-			ia_css_pipe_unload_extension(pipe, firmware);
-	}
-}
-
 int
 ia_css_pipe_destroy(struct ia_css_pipe *pipe)
 {
@@ -2241,9 +2154,6 @@ ia_css_pipe_destroy(struct ia_css_pipe *pipe)
 		ia_css_frame_free_multiple(MAX_NUM_VIDEO_DELAY_FRAMES,
 					   pipe->pipe_settings.capture.delay_frames);
 		break;
-	case IA_CSS_PIPE_MODE_ACC:
-		sh_css_pipe_free_acc_binaries(pipe);
-		break;
 	case IA_CSS_PIPE_MODE_COPY:
 		break;
 	case IA_CSS_PIPE_MODE_YUVPP:
@@ -2261,10 +2171,6 @@ ia_css_pipe_destroy(struct ia_css_pipe *pipe)
 	ia_css_pipeline_destroy(&pipe->pipeline);
 	pipe_release_pipe_num(ia_css_pipe_get_pipe_num(pipe));
 
-	/* Temporarily, not every sh_css_pipe has an acc_extension. */
-	if (pipe->config.acc_extension)
-		ia_css_pipe_unload_extension(pipe, pipe->config.acc_extension);
-
 	kfree(pipe);
 	IA_CSS_LEAVE("err = %d", err);
 	return err;
@@ -3433,31 +3339,6 @@ static int create_host_video_pipeline(struct ia_css_pipe *pipe)
 		}
 	}
 
-	/* Append Extension on Video out, if enabled */
-	if (!need_vf_pp && video_stage && pipe->config.acc_extension &&
-	    (pipe->config.acc_extension->info.isp.type == IA_CSS_ACC_OUTPUT)) {
-		struct ia_css_frame *out = NULL;
-		struct ia_css_frame *in = NULL;
-
-		if ((pipe->config.acc_extension->info.isp.sp.enable.output) &&
-		    (pipe->config.acc_extension->info.isp.sp.enable.in_frame) &&
-		    (pipe->config.acc_extension->info.isp.sp.enable.out_frame)) {
-			/* In/Out Frame mapping to support output frame extension.*/
-			out = video_stage->args.out_frame[0];
-			err = ia_css_frame_allocate_from_info(&in, &pipe->output_info[0]);
-			if (err)
-				goto ERR;
-			video_stage->args.out_frame[0] = in;
-		}
-
-		err = add_firmwares(me, video_binary, pipe->output_stage,
-				    last_output_firmware(pipe->output_stage),
-				    IA_CSS_BINARY_MODE_VIDEO,
-				    in, out, NULL, &video_stage, NULL);
-		if (err)
-			goto ERR;
-	}
-
 	if (need_yuv_pp && video_stage) {
 		struct ia_css_frame *tmp_in_frame = video_stage->args.out_frame[0];
 		struct ia_css_frame *tmp_out_frame = NULL;
@@ -3489,45 +3370,6 @@ static int create_host_video_pipeline(struct ia_css_pipe *pipe)
 	return err;
 }
 
-static int
-create_host_acc_pipeline(struct ia_css_pipe *pipe)
-{
-	int err = 0;
-	const struct ia_css_fw_info *fw;
-	unsigned int i;
-
-	IA_CSS_ENTER_PRIVATE("pipe = %p", pipe);
-	if ((!pipe) || (!pipe->stream)) {
-		IA_CSS_LEAVE_ERR_PRIVATE(-EINVAL);
-		return -EINVAL;
-	}
-
-	pipe->pipeline.num_execs = pipe->config.acc_num_execs;
-	/* Reset pipe_qos_config to default disable all QOS extension stages */
-	if (pipe->config.acc_extension)
-		pipe->pipeline.pipe_qos_config = 0;
-
-	for (fw = pipe->vf_stage; fw; fw = fw->next) {
-		err = sh_css_pipeline_add_acc_stage(&pipe->pipeline, fw);
-		if (err)
-			goto ERR;
-	}
-
-	for (i = 0; i < pipe->config.num_acc_stages; i++) {
-		struct ia_css_fw_info *fw = pipe->config.acc_stages[i];
-
-		err = sh_css_pipeline_add_acc_stage(&pipe->pipeline, fw);
-		if (err)
-			goto ERR;
-	}
-
-	ia_css_pipeline_finalize_stages(&pipe->pipeline, pipe->stream->config.continuous);
-
-ERR:
-	IA_CSS_LEAVE_ERR_PRIVATE(err);
-	return err;
-}
-
 /* Create stages for preview */
 static int
 create_host_preview_pipeline(struct ia_css_pipe *pipe)
@@ -3690,7 +3532,6 @@ preview_start(struct ia_css_pipe *pipe)
 {
 	int err = 0;
 	struct ia_css_pipe *copy_pipe, *capture_pipe;
-	struct ia_css_pipe *acc_pipe;
 	enum sh_css_pipe_config_override copy_ovrd;
 	enum ia_css_input_mode preview_pipe_input_mode;
 	unsigned int thread_id;
@@ -3705,7 +3546,6 @@ preview_start(struct ia_css_pipe *pipe)
 
 	copy_pipe    = pipe->pipe_settings.preview.copy_pipe;
 	capture_pipe = pipe->pipe_settings.preview.capture_pipe;
-	acc_pipe     = pipe->pipe_settings.preview.acc_pipe;
 
 	sh_css_metrics_start_frame();
 
@@ -3764,22 +3604,6 @@ preview_start(struct ia_css_pipe *pipe)
 					(enum mipi_port_id)0);
 	}
 
-	if (acc_pipe) {
-		sh_css_sp_init_pipeline(&acc_pipe->pipeline,
-					IA_CSS_PIPE_ID_ACC,
-					(uint8_t)ia_css_pipe_get_pipe_num(acc_pipe),
-					false,
-					pipe->stream->config.pixels_per_clock == 2,
-					false, /* continuous */
-					false, /* offline */
-					pipe->required_bds_factor,
-					0,
-					IA_CSS_INPUT_MODE_MEMORY,
-					NULL,
-					NULL,
-					(enum mipi_port_id)0);
-	}
-
 	start_pipe(pipe, copy_ovrd, preview_pipe_input_mode);
 
 	IA_CSS_LEAVE_ERR_PRIVATE(err);
@@ -3850,9 +3674,7 @@ ia_css_pipe_enqueue_buffer(struct ia_css_pipe *pipe,
 
 	pipeline = &pipe->pipeline;
 
-	assert(pipeline ||
-	       pipe_id == IA_CSS_PIPE_ID_COPY ||
-	       pipe_id == IA_CSS_PIPE_ID_ACC);
+	assert(pipeline || pipe_id == IA_CSS_PIPE_ID_COPY);
 
 	assert(sizeof(NULL) <= sizeof(ddr_buffer.kernel_ptr));
 	ddr_buffer.kernel_ptr = HOST_ADDRESS(NULL);
@@ -4442,16 +4264,6 @@ ia_css_dequeue_isys_event(struct ia_css_event *event)
 	return err;
 }
 
-static void
-acc_start(struct ia_css_pipe *pipe)
-{
-	assert(pipe);
-	assert(pipe->stream);
-
-	start_pipe(pipe, SH_CSS_PIPE_CONFIG_OVRD_NO_OVRD,
-		   pipe->stream->config.mode);
-}
-
 static int
 sh_css_pipe_start(struct ia_css_stream *stream)
 {
@@ -4496,9 +4308,6 @@ sh_css_pipe_start(struct ia_css_stream *stream)
 	case IA_CSS_PIPE_ID_YUVPP:
 		err = yuvpp_start(pipe);
 		break;
-	case IA_CSS_PIPE_ID_ACC:
-		acc_start(pipe);
-		break;
 	default:
 		err = -EINVAL;
 	}
@@ -4524,10 +4333,6 @@ sh_css_pipe_start(struct ia_css_stream *stream)
 				stream->pipes[i]->stop_requested = false;
 				err = yuvpp_start(stream->pipes[i]);
 				break;
-			case IA_CSS_PIPE_ID_ACC:
-				stream->pipes[i]->stop_requested = false;
-				acc_start(stream->pipes[i]);
-				break;
 			default:
 				err = -EINVAL;
 			}
@@ -4620,22 +4425,6 @@ sh_css_pipe_start(struct ia_css_stream *stream)
 		    (uint8_t)thread_id, 0,  0);
 	}
 
-	/* in case of PREVIEW mode, check whether QOS acc_pipe is available, then start the qos pipe */
-	if (pipe_id == IA_CSS_PIPE_ID_PREVIEW) {
-		struct ia_css_pipe *acc_pipe = NULL;
-
-		acc_pipe = pipe->pipe_settings.preview.acc_pipe;
-
-		if (acc_pipe) {
-			ia_css_pipeline_get_sp_thread_id(ia_css_pipe_get_pipe_num(acc_pipe),
-							 &thread_id);
-			/* by the time we reach here q is initialized and handle is available.*/
-			ia_css_bufq_enqueue_psys_event(
-			    IA_CSS_PSYS_SW_EVENT_START_STREAM,
-			    (uint8_t)thread_id, 0, 0);
-		}
-	}
-
 	stream->started = true;
 
 	IA_CSS_LEAVE_ERR_PRIVATE(err);
@@ -6861,8 +6650,6 @@ sh_css_pipe_load_binaries(struct ia_css_pipe *pipe)
 	case IA_CSS_PIPE_ID_YUVPP:
 		err = load_yuvpp_binaries(pipe);
 		break;
-	case IA_CSS_PIPE_ID_ACC:
-		break;
 	default:
 		err = -EINVAL;
 		break;
@@ -7755,154 +7542,6 @@ ia_css_stream_end_input_frame(const struct ia_css_stream *stream)
 	ia_css_inputfifo_end_frame(stream->config.channel_id);
 }
 
-static void
-append_firmware(struct ia_css_fw_info **l, struct ia_css_fw_info *firmware)
-{
-	IA_CSS_ENTER_PRIVATE("l = %p, firmware = %p", l, firmware);
-	if (!l) {
-		IA_CSS_ERROR("NULL fw_info");
-		IA_CSS_LEAVE_PRIVATE("");
-		return;
-	}
-	while (*l)
-		l = &(*l)->next;
-	*l = firmware;
-	/* when multiple acc extensions are loaded, 'next' can be not NULL */
-	/*firmware->next = NULL;*/
-	IA_CSS_LEAVE_PRIVATE("");
-}
-
-static void
-remove_firmware(struct ia_css_fw_info **l, struct ia_css_fw_info *firmware)
-{
-	assert(*l);
-	assert(firmware);
-	(void)l;
-	(void)firmware;
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE, "remove_firmware() enter:\n");
-
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE, "remove_firmware() leave:\n");
-	return; /* removing single and multiple firmware is handled in acc_unload_extension() */
-}
-
-static int upload_isp_code(struct ia_css_fw_info *firmware)
-{
-	ia_css_ptr binary;
-
-	if (!firmware) {
-		IA_CSS_ERROR("NULL input parameter");
-		return -EINVAL;
-	}
-	binary = firmware->info.isp.xmem_addr;
-
-	if (!binary) {
-		unsigned int size = firmware->blob.size;
-		const unsigned char *blob;
-		const unsigned char *binary_name;
-
-		binary_name =
-		    (const unsigned char *)(IA_CSS_EXT_ISP_PROG_NAME(
-						firmware));
-		blob = binary_name +
-			strlen((const char *)binary_name) +
-			1;
-		binary = sh_css_load_blob(blob, size);
-		firmware->info.isp.xmem_addr = binary;
-	}
-
-	if (!binary)
-		return -ENOMEM;
-	return 0;
-}
-
-static int
-acc_load_extension(struct ia_css_fw_info *firmware)
-{
-	int err;
-	struct ia_css_fw_info *hd = firmware;
-
-	while (hd) {
-		err = upload_isp_code(hd);
-		if (err)
-			return err;
-		hd = hd->next;
-	}
-
-	if (!firmware)
-		return -EINVAL;
-	firmware->loaded = true;
-	return 0;
-}
-
-static void
-acc_unload_extension(struct ia_css_fw_info *firmware)
-{
-	struct ia_css_fw_info *hd = firmware;
-	struct ia_css_fw_info *hdn = NULL;
-
-	if (!firmware) /* should not happen */
-		return;
-	/* unload and remove multiple firmwares */
-	while (hd) {
-		hdn = (hd->next) ? &(*hd->next) : NULL;
-		if (hd->info.isp.xmem_addr) {
-			hmm_free(hd->info.isp.xmem_addr);
-			hd->info.isp.xmem_addr = mmgr_NULL;
-		}
-		hd->isp_code = NULL;
-		hd->next = NULL;
-		hd = hdn;
-	}
-
-	firmware->loaded = false;
-}
-
-/* Load firmware for extension */
-static int
-ia_css_pipe_load_extension(struct ia_css_pipe *pipe,
-			   struct ia_css_fw_info *firmware)
-{
-	int err = 0;
-
-	IA_CSS_ENTER_PRIVATE("fw = %p pipe = %p", firmware, pipe);
-
-	if ((!firmware) || (!pipe)) {
-		IA_CSS_LEAVE_ERR_PRIVATE(-EINVAL);
-		return -EINVAL;
-	}
-
-	if (firmware->info.isp.type == IA_CSS_ACC_OUTPUT)
-		append_firmware(&pipe->output_stage, firmware);
-	else if (firmware->info.isp.type == IA_CSS_ACC_VIEWFINDER)
-		append_firmware(&pipe->vf_stage, firmware);
-	err = acc_load_extension(firmware);
-
-	IA_CSS_LEAVE_ERR_PRIVATE(err);
-	return err;
-}
-
-/* Unload firmware for extension */
-static void
-ia_css_pipe_unload_extension(struct ia_css_pipe *pipe,
-			     struct ia_css_fw_info *firmware)
-{
-	IA_CSS_ENTER_PRIVATE("fw = %p pipe = %p", firmware, pipe);
-
-	if ((!firmware) || (!pipe)) {
-		IA_CSS_ERROR("NULL input parameters");
-		IA_CSS_LEAVE_PRIVATE("");
-		return;
-	}
-
-	if (firmware->info.isp.type == IA_CSS_ACC_OUTPUT)
-		remove_firmware(&pipe->output_stage, firmware);
-	else if (firmware->info.isp.type == IA_CSS_ACC_VIEWFINDER)
-		remove_firmware(&pipe->vf_stage, firmware);
-	acc_unload_extension(firmware);
-
-	IA_CSS_LEAVE_PRIVATE("");
-}
-
 bool
 ia_css_pipeline_uses_params(struct ia_css_pipeline *me)
 {
@@ -7924,35 +7563,6 @@ ia_css_pipeline_uses_params(struct ia_css_pipeline *me)
 	return false;
 }
 
-static int
-sh_css_pipeline_add_acc_stage(struct ia_css_pipeline *pipeline,
-			      const void *acc_fw)
-{
-	struct ia_css_fw_info *fw = (struct ia_css_fw_info *)acc_fw;
-	/* In QoS case, load_extension already called, so skipping */
-	int	err = 0;
-
-	if (!fw->loaded)
-		err = acc_load_extension(fw);
-
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-			    "sh_css_pipeline_add_acc_stage() enter: pipeline=%p, acc_fw=%p\n",
-			    pipeline, acc_fw);
-
-	if (!err) {
-		struct ia_css_pipeline_stage_desc stage_desc;
-
-		ia_css_pipe_get_acc_stage_desc(&stage_desc, NULL, fw);
-		err = ia_css_pipeline_create_and_add_stage(pipeline,
-							   &stage_desc,
-							   NULL);
-	}
-
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-			    "sh_css_pipeline_add_acc_stage() leave: return_err=%d\n", err);
-	return err;
-}
-
 /*
  * @brief Tag a specific frame in continuous capture.
  * Refer to "sh_css_internal.h" for details.
@@ -8177,26 +7787,6 @@ void ia_css_stream_config_defaults(struct ia_css_stream_config *stream_config)
 	stream_config->source.port.rxcount = 0x04040404;
 }
 
-static int
-ia_css_acc_pipe_create(struct ia_css_pipe *pipe)
-{
-	int err = 0;
-
-	if (!pipe) {
-		IA_CSS_ERROR("NULL input parameter");
-		return -EINVAL;
-	}
-
-	/* There is not meaning for num_execs = 0 semantically. Run at least once. */
-	if (pipe->config.acc_num_execs == 0)
-		pipe->config.acc_num_execs = 1;
-
-	if (pipe->config.acc_extension)
-		err = ia_css_pipe_load_extension(pipe, pipe->config.acc_extension);
-
-	return err;
-}
-
 int ia_css_pipe_create(const struct ia_css_pipe_config *config,
 		       struct ia_css_pipe **pipe)
 {
@@ -8257,23 +7847,6 @@ ia_css_pipe_create_extra(const struct ia_css_pipe_config *config,
 	else
 		ia_css_pipe_extra_config_defaults(&internal_pipe->extra_config);
 
-	if (config->mode == IA_CSS_PIPE_MODE_ACC) {
-		/*
-		 * Temporary hack to migrate acceleration to CSS 2.0.
-		 * In the future the code for all pipe types should be
-		 * unified.
-		 */
-		*pipe = internal_pipe;
-		if (!internal_pipe->config.acc_extension &&
-		    internal_pipe->config.num_acc_stages ==
-		    0) { /* if no acc binary and no standalone stage */
-			*pipe = NULL;
-			IA_CSS_LEAVE_ERR_PRIVATE(0);
-			return 0;
-		}
-		return ia_css_acc_pipe_create(internal_pipe);
-	}
-
 	/*
 	 * Use config value when dvs_frame_delay setting equal to 2,
 	 * otherwise always 1 by default
@@ -8368,15 +7941,6 @@ ia_css_pipe_create_extra(const struct ia_css_pipe_config *config,
 			}
 		}
 	}
-	if (internal_pipe->config.acc_extension) {
-		err = ia_css_pipe_load_extension(internal_pipe,
-						 internal_pipe->config.acc_extension);
-		if (err) {
-			IA_CSS_LEAVE_ERR_PRIVATE(err);
-			kvfree(internal_pipe);
-			return err;
-		}
-	}
 	/* set all info to zeroes first */
 	memset(&internal_pipe->info, 0, sizeof(internal_pipe->info));
 
@@ -8524,57 +8088,6 @@ find_pipe(struct ia_css_pipe *pipes[], unsigned int num_pipes,
 	return NULL;
 }
 
-static int
-ia_css_acc_stream_create(struct ia_css_stream *stream)
-{
-	int i;
-	int err = 0;
-
-	IA_CSS_ENTER_PRIVATE("stream = %p", stream);
-
-	if (!stream) {
-		IA_CSS_LEAVE_ERR_PRIVATE(-EINVAL);
-		return -EINVAL;
-	}
-
-	for (i = 0;  i < stream->num_pipes; i++) {
-		struct ia_css_pipe *pipe = stream->pipes[i];
-
-		if (!pipe) {
-			IA_CSS_LEAVE_ERR_PRIVATE(-EINVAL);
-			return -EINVAL;
-		}
-
-		pipe->stream = stream;
-	}
-
-	/* Map SP threads before doing anything. */
-	err = map_sp_threads(stream, true);
-	if (err) {
-		IA_CSS_LEAVE_ERR_PRIVATE(err);
-		return err;
-	}
-
-	for (i = 0;  i < stream->num_pipes; i++) {
-		struct ia_css_pipe *pipe = stream->pipes[i];
-
-		assert(pipe);
-		ia_css_pipe_map_queue(pipe, true);
-	}
-
-	err = create_host_pipeline_structure(stream);
-	if (err) {
-		IA_CSS_LEAVE_ERR_PRIVATE(err);
-		return err;
-	}
-
-	stream->started = false;
-
-	IA_CSS_LEAVE_ERR_PRIVATE(0);
-
-	return 0;
-}
-
 static int
 metadata_info_init(const struct ia_css_metadata_config *mdc,
 		   struct ia_css_metadata_info *md)
@@ -8807,11 +8320,6 @@ ia_css_stream_create(const struct ia_css_stream_config *stream_config,
 		goto ERR;
 	IA_CSS_LOG("isp_params_configs: %p", curr_stream->isp_params_configs);
 
-	if (num_pipes == 1 && pipes[0]->config.mode == IA_CSS_PIPE_MODE_ACC) {
-		*stream = curr_stream;
-		err = ia_css_acc_stream_create(curr_stream);
-		goto ERR;
-	}
 	/* sensor binning */
 	if (!spcopyonly) {
 		sensor_binning_changed =
@@ -8832,7 +8340,6 @@ ia_css_stream_create(const struct ia_css_stream_config *stream_config,
 		/* Search for the preview pipe and create the copy pipe */
 		struct ia_css_pipe *preview_pipe;
 		struct ia_css_pipe *video_pipe;
-		struct ia_css_pipe *acc_pipe;
 		struct ia_css_pipe *capture_pipe = NULL;
 		struct ia_css_pipe *copy_pipe = NULL;
 
@@ -8847,11 +8354,7 @@ ia_css_stream_create(const struct ia_css_stream_config *stream_config,
 					 IA_CSS_PIPE_MODE_PREVIEW, false);
 		video_pipe = find_pipe(pipes, num_pipes,
 				       IA_CSS_PIPE_MODE_VIDEO, false);
-		acc_pipe = find_pipe(pipes, num_pipes, IA_CSS_PIPE_MODE_ACC,
-				     false);
-		if (acc_pipe && num_pipes == 2 && curr_stream->cont_capt)
-			curr_stream->cont_capt =
-			    false; /* preview + QoS case will not need cont_capt switch */
+
 		if (curr_stream->cont_capt) {
 			capture_pipe = find_pipe(pipes, num_pipes,
 						 IA_CSS_PIPE_MODE_CAPTURE,
@@ -8888,9 +8391,6 @@ ia_css_stream_create(const struct ia_css_stream_config *stream_config,
 		}
 		if (video_pipe && curr_stream->cont_capt)
 			video_pipe->pipe_settings.video.capture_pipe = capture_pipe;
-
-		if (preview_pipe && acc_pipe)
-			preview_pipe->pipe_settings.preview.acc_pipe = acc_pipe;
 	}
 	for (i = 0; i < num_pipes; i++) {
 		curr_pipe = pipes[i];
@@ -9738,13 +9238,6 @@ void ia_css_pipe_map_queue(struct ia_css_pipe *pipe, bool map)
 		if (!pipe->stream->config.continuous)
 			ia_css_queue_map(thread_id, IA_CSS_BUFFER_TYPE_OUTPUT_FRAME, map);
 		ia_css_queue_map(thread_id, IA_CSS_BUFFER_TYPE_METADATA, map);
-	} else if (pipe->mode == IA_CSS_PIPE_ID_ACC) {
-		if (need_input_queue)
-			ia_css_queue_map(thread_id, IA_CSS_BUFFER_TYPE_INPUT_FRAME, map);
-		ia_css_queue_map(thread_id, IA_CSS_BUFFER_TYPE_OUTPUT_FRAME, map);
-		ia_css_queue_map(thread_id, IA_CSS_BUFFER_TYPE_PARAMETER_SET, map);
-		ia_css_queue_map(thread_id, IA_CSS_BUFFER_TYPE_PER_FRAME_PARAMETER_SET, map);
-		ia_css_queue_map(thread_id, IA_CSS_BUFFER_TYPE_METADATA, map);
 	} else if (pipe->mode == IA_CSS_PIPE_ID_YUVPP) {
 		unsigned int idx;
 
@@ -9795,92 +9288,6 @@ ia_css_unlock_raw_frame(struct ia_css_stream *stream, uint32_t exp_id)
 	return ret;
 }
 
-/*
- * @brief	Set the state (Enable or Disable) of the Extension stage in the
- *		given pipe.
- */
-int
-ia_css_pipe_set_qos_ext_state(struct ia_css_pipe *pipe, uint32_t fw_handle,
-			      bool enable)
-{
-	unsigned int thread_id;
-	struct ia_css_pipeline_stage *stage;
-	int err = 0;
-
-	IA_CSS_ENTER("");
-
-	/* Parameter Check */
-	if (!pipe || !pipe->stream) {
-		IA_CSS_ERROR("Invalid Pipe.");
-		err = -EINVAL;
-	} else if (!(pipe->config.acc_extension)) {
-		IA_CSS_ERROR("Invalid Pipe(No Extension Firmware)");
-		err = -EINVAL;
-	} else if (!sh_css_sp_is_running()) {
-		IA_CSS_ERROR("Leaving: queue unavailable.");
-		err = -EBUSY;
-	} else {
-		/* Query the threadid and stage_num for the Extension firmware*/
-		ia_css_pipeline_get_sp_thread_id(ia_css_pipe_get_pipe_num(pipe), &thread_id);
-		err = ia_css_pipeline_get_stage_from_fw(&pipe->pipeline, fw_handle, &stage);
-		if (!err) {
-			/* Set the Extension State;. TODO: Add check for stage firmware.type (QOS)*/
-			err = ia_css_bufq_enqueue_psys_event(
-			    (uint8_t)IA_CSS_PSYS_SW_EVENT_STAGE_ENABLE_DISABLE,
-			    (uint8_t)thread_id,
-			    (uint8_t)stage->stage_num,
-			    enable ? 1 : 0);
-			if (!err) {
-				if (enable)
-					SH_CSS_QOS_STAGE_ENABLE(&sh_css_sp_group.pipe[thread_id], stage->stage_num);
-				else
-					SH_CSS_QOS_STAGE_DISABLE(&sh_css_sp_group.pipe[thread_id], stage->stage_num);
-			}
-		}
-	}
-	IA_CSS_LEAVE("err:%d handle:%u enable:%d", err, fw_handle, enable);
-	return err;
-}
-
-/*
- * @brief	Get the state (Enable or Disable) of the Extension stage in the
- * given pipe.
- */
-int
-ia_css_pipe_get_qos_ext_state(struct ia_css_pipe *pipe, uint32_t fw_handle,
-			      bool *enable)
-{
-	struct ia_css_pipeline_stage *stage;
-	unsigned int thread_id;
-	int err = 0;
-
-	IA_CSS_ENTER("");
-
-	/* Parameter Check */
-	if (!pipe || !pipe->stream) {
-		IA_CSS_ERROR("Invalid Pipe.");
-		err = -EINVAL;
-	} else if (!(pipe->config.acc_extension)) {
-		IA_CSS_ERROR("Invalid Pipe (No Extension Firmware).");
-		err = -EINVAL;
-	} else if (!sh_css_sp_is_running()) {
-		IA_CSS_ERROR("Leaving: queue unavailable.");
-		err = -EBUSY;
-	} else {
-		/* Query the threadid and stage_num corresponding to the Extension firmware*/
-		ia_css_pipeline_get_sp_thread_id(ia_css_pipe_get_pipe_num(pipe), &thread_id);
-		err = ia_css_pipeline_get_stage_from_fw(&pipe->pipeline, fw_handle, &stage);
-
-		if (!err) {
-			/* Get the Extension State */
-			*enable = (SH_CSS_QOS_STAGE_IS_ENABLED(&sh_css_sp_group.pipe[thread_id],
-								stage->stage_num)) ? true : false;
-		}
-	}
-	IA_CSS_LEAVE("err:%d handle:%u enable:%d", err, fw_handle, *enable);
-	return err;
-}
-
 static void
 sh_css_hmm_buffer_record_init(void)
 {
diff --git a/drivers/staging/media/atomisp/pci/sh_css_internal.h b/drivers/staging/media/atomisp/pci/sh_css_internal.h
index 435b3cedd1c3..98267707a5fc 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_internal.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_internal.h
@@ -497,7 +497,6 @@ ia_css_metadata_free_multiple(unsigned int num_bufs,
 #define SH_CSS_QOS_STAGE_IS_ALL_DISABLED(pipe) ((pipe)->pipe_qos_config == QOS_ALL_STAGES_DISABLED)
 #define SH_CSS_QOS_MODE_PIPE_ADD(mode, pipe)    ((mode) |= (0x1 << (pipe)->pipe_id))
 #define SH_CSS_QOS_MODE_PIPE_REMOVE(mode, pipe) ((mode) &= ~(0x1 << (pipe)->pipe_id))
-#define SH_CSS_IS_QOS_ONLY_MODE(mode)           ((mode) == (0x1 << IA_CSS_PIPE_ID_ACC))
 
 /* Information for a pipeline */
 struct sh_css_sp_pipeline {
diff --git a/drivers/staging/media/atomisp/pci/sh_css_legacy.h b/drivers/staging/media/atomisp/pci/sh_css_legacy.h
index 567c8d6dcc2c..cdf239b070a8 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_legacy.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_legacy.h
@@ -32,7 +32,6 @@ enum ia_css_pipe_id {
 	IA_CSS_PIPE_ID_VIDEO,
 	IA_CSS_PIPE_ID_CAPTURE,
 	IA_CSS_PIPE_ID_YUVPP,
-	IA_CSS_PIPE_ID_ACC,
 	IA_CSS_PIPE_ID_NUM
 };
 
-- 
2.38.1

[PATCH 03/20] media: atomisp: Remove unused QOS defines / structure member

[Hans de Goede]

Withe the accel code gone this is unused, remove it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../pci/runtime/pipeline/interface/ia_css_pipeline.h     | 2 --
 drivers/staging/media/atomisp/pci/sh_css_internal.h      | 9 ---------
 drivers/staging/media/atomisp/pci/sh_css_sp.c            | 2 +-
 3 files changed, 1 insertion(+), 12 deletions(-)

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 de2c526a58ae..222c381ff3b9 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
@@ -54,7 +54,6 @@ struct ia_css_pipeline {
 	unsigned int inout_port_config;
 	int num_execs;
 	bool acquire_isp_each_stage;
-	u32 pipe_qos_config;
 };
 
 #define DEFAULT_PIPELINE { \
@@ -65,7 +64,6 @@ struct ia_css_pipeline {
 	.dvs_frame_delay	= IA_CSS_FRAME_DELAY_1, \
 	.num_execs		= -1, \
 	.acquire_isp_each_stage	= true, \
-	.pipe_qos_config	= QOS_INVALID \
 }
 
 /* Stage descriptor used to create a new stage in the pipeline */
diff --git a/drivers/staging/media/atomisp/pci/sh_css_internal.h b/drivers/staging/media/atomisp/pci/sh_css_internal.h
index 98267707a5fc..0441d4fec551 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_internal.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_internal.h
@@ -488,15 +488,6 @@ ia_css_metadata_free_multiple(unsigned int num_bufs,
 
 /* Macro for handling pipe_qos_config */
 #define QOS_INVALID                  (~0U)
-#define QOS_ALL_STAGES_DISABLED      (0U)
-#define QOS_STAGE_MASK(num)          (0x00000001 << num)
-#define SH_CSS_IS_QOS_PIPE(pipe)               ((pipe)->pipe_qos_config != QOS_INVALID)
-#define SH_CSS_QOS_STAGE_ENABLE(pipe, num)     ((pipe)->pipe_qos_config |= QOS_STAGE_MASK(num))
-#define SH_CSS_QOS_STAGE_DISABLE(pipe, num)    ((pipe)->pipe_qos_config &= ~QOS_STAGE_MASK(num))
-#define SH_CSS_QOS_STAGE_IS_ENABLED(pipe, num) ((pipe)->pipe_qos_config & QOS_STAGE_MASK(num))
-#define SH_CSS_QOS_STAGE_IS_ALL_DISABLED(pipe) ((pipe)->pipe_qos_config == QOS_ALL_STAGES_DISABLED)
-#define SH_CSS_QOS_MODE_PIPE_ADD(mode, pipe)    ((mode) |= (0x1 << (pipe)->pipe_id))
-#define SH_CSS_QOS_MODE_PIPE_REMOVE(mode, pipe) ((mode) &= ~(0x1 << (pipe)->pipe_id))
 
 /* Information for a pipeline */
 struct sh_css_sp_pipeline {
diff --git a/drivers/staging/media/atomisp/pci/sh_css_sp.c b/drivers/staging/media/atomisp/pci/sh_css_sp.c
index c301298b8ee4..0dd58a7fe2cc 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_sp.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_sp.c
@@ -1266,7 +1266,7 @@ sh_css_sp_init_pipeline(struct ia_css_pipeline *me,
 	sh_css_sp_group.pipe[thread_id].thread_id = thread_id;
 	sh_css_sp_group.pipe[thread_id].pipe_num = pipe_num;
 	sh_css_sp_group.pipe[thread_id].num_execs = me->num_execs;
-	sh_css_sp_group.pipe[thread_id].pipe_qos_config = me->pipe_qos_config;
+	sh_css_sp_group.pipe[thread_id].pipe_qos_config = QOS_INVALID;
 	sh_css_sp_group.pipe[thread_id].required_bds_factor = required_bds_factor;
 	sh_css_sp_group.pipe[thread_id].input_system_mode
 	= (uint32_t)input_mode;
-- 
2.38.1

[PATCH 04/20] media: atomisp: Flush queue on atomisp_css_start() error

[Hans de Goede]

I managed to trigger an atomisp_css_start() error by pushing my test system
towards an OOM situation, this triggered the following WARN_ON() in
__vb2_queue_cancel() in videobuf2-core.c:

        /*
         * If you see this warning, then the driver isn't cleaning up properly
         * after a failed start_streaming(). See the start_streaming()
         * documentation in videobuf2-core.h for more information how buffers
         * should be returned to vb2 in start_streaming().
         */
        if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {

Fix this by calling atomisp_flush_video_pipe() to return any queued buffers
back to the videobuf2-core on an atomisp_css_start() error.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 83710af7690f..43e899457b91 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -1353,8 +1353,10 @@ int atomisp_start_streaming(struct vb2_queue *vq, unsigned int count)
 	asd->params.dvs_6axis = NULL;
 
 	ret = atomisp_css_start(asd, css_pipe_id, false);
-	if (ret)
+	if (ret) {
+		atomisp_flush_video_pipe(pipe, true);
 		goto out_unlock;
+	}
 
 	spin_lock_irqsave(&isp->lock, irqflags);
 	asd->streaming = ATOMISP_DEVICE_STREAMING_ENABLED;
-- 
2.38.1

[PATCH 05/20] media: atomisp: Log an error on failing to alloc private-mem

[Hans de Goede]

I managed to trigger an atomisp_css_start() error by pushing my test
system towards an OOM situation, this resulted in the following errors:

atomisp-isp2 0000:00:03.0: alloc pages err...
atomisp-isp2 0000:00:03.0: hmm_bo_alloc_pages failed.
atomisp-isp2 0000:00:03.0: stream[0] start error.

But it is not entirely clear what the root cause of
the "alloc pages err..." error is. I suspect the root cause is
alloc_pages_bulk_array() failing. Add a log message to make
the root cause more clear if this is hit again.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/hmm/hmm_bo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
index 465ba837f2ed..3c150268db51 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
@@ -637,6 +637,7 @@ static int alloc_private_pages(struct hmm_buffer_object *bo)
 
 	ret = alloc_pages_bulk_array(gfp, bo->pgnr, bo->pages);
 	if (ret != bo->pgnr) {
+		dev_err(atomisp_dev, "alloc_pages_bulk_array() failed\n");
 		free_pages_bulk_array(ret, bo->pages);
 		return -ENOMEM;
 	}
-- 
2.38.1

Re: [PATCH 05/20] media: atomisp: Log an error on failing to alloc private-mem

[Andy Shevchenko]

On Sun, Nov 20, 2022 at 11:40:46PM +0100, Hans de Goede wrote:
> I managed to trigger an atomisp_css_start() error by pushing my test
> system towards an OOM situation, this resulted in the following errors:
> 
> atomisp-isp2 0000:00:03.0: alloc pages err...
> atomisp-isp2 0000:00:03.0: hmm_bo_alloc_pages failed.
> atomisp-isp2 0000:00:03.0: stream[0] start error.
> 
> But it is not entirely clear what the root cause of
> the "alloc pages err..." error is. I suspect the root cause is
> alloc_pages_bulk_array() failing. Add a log message to make
> the root cause more clear if this is hit again.

...

> +		dev_err(atomisp_dev, "alloc_pages_bulk_array() failed\n");

I would put it after calling the below function, but I'm not sure about side
effects (if the below can hang or something else).

>  		free_pages_bulk_array(ret, bo->pages);
>  		return -ENOMEM;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 05/20] media: atomisp: Log an error on failing to alloc private-mem

[Hans de Goede]

Hi,

On 11/21/22 09:58, Andy Shevchenko wrote:
> On Sun, Nov 20, 2022 at 11:40:46PM +0100, Hans de Goede wrote:
>> I managed to trigger an atomisp_css_start() error by pushing my test
>> system towards an OOM situation, this resulted in the following errors:
>>
>> atomisp-isp2 0000:00:03.0: alloc pages err...
>> atomisp-isp2 0000:00:03.0: hmm_bo_alloc_pages failed.
>> atomisp-isp2 0000:00:03.0: stream[0] start error.
>>
>> But it is not entirely clear what the root cause of
>> the "alloc pages err..." error is. I suspect the root cause is
>> alloc_pages_bulk_array() failing. Add a log message to make
>> the root cause more clear if this is hit again.
> 
> ...
> 
>> +		dev_err(atomisp_dev, "alloc_pages_bulk_array() failed\n");
> 
> I would put it after calling the below function, but I'm not sure about side
> effects (if the below can hang or something else).
> 
>>  		free_pages_bulk_array(ret, bo->pages);
>>  		return -ENOMEM;
> 

Fixed in my media-atomisp branch.

Regards,

Hans

[PATCH 06/20] media: atomisp: Fix deadlock when the /dev/video# node is closed while still streaming

[Hans de Goede]

atomisp_release() was taking pipe->vb_queue_mutex + isp->mutex at the
same time. But if the /dev/video# node is closed while still streaming
then vb2_queue_release() will call atomisp_stop_streaming() which takes
isp->mutex itself, leading to a deadlock.

To fix this only take isp->mutex after cleaning up the v4l2_fh /
the vb2_queue. While at it switch to vb2_fop_release() which will take
pipe->vb_queue_mutex for us, which also resolves a FIXME comment.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_fops.c  | 21 +++++--------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index 21ca276831ce..8cff26d42b82 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -861,23 +861,14 @@ static int atomisp_release(struct file *file)
 
 	v4l2_fh_init(&fh.vfh, vdev);
 
-	mutex_lock(&pipe->vb_queue_mutex);
-	mutex_lock(&isp->mutex);
-
 	dev_dbg(isp->dev, "release device %s\n", vdev->name);
 
 	asd->subdev.devnode = vdev;
 
-	/*
-	 * FIXME This if is copied from _vb2_fop_release, this cannot use that
-	 * because that calls v4l2_fh_release() earlier then this function.
-	 * Maybe we can release the fh earlier though, it does not look like
-	 * anything needs it after this.
-	 */
-	if (file->private_data == vdev->queue->owner) {
-		vb2_queue_release(vdev->queue);
-		vdev->queue->owner = NULL;
-	}
+	/* Note file must not be used after this! */
+	vb2_fop_release(file);
+
+	mutex_lock(&isp->mutex);
 
 	pipe->users--;
 	if (pipe->users)
@@ -940,9 +931,7 @@ static int atomisp_release(struct file *file)
 				     V4L2_SEL_TGT_COMPOSE, 0,
 				     &clear_compose);
 	mutex_unlock(&isp->mutex);
-	mutex_unlock(&pipe->vb_queue_mutex);
-
-	return v4l2_fh_release(file);
+	return 0;
 }
 
 const struct v4l2_file_operations atomisp_fops = {
-- 
2.38.1

[PATCH 07/20] media: atomisp: Remove 2 unused accelerator mode related functions

[Hans de Goede]

Remove ia_css_pipe_get_acc_stage_desc() and sh_css_flush(),
after removing the accelerator /dev/video# node and related ioctls
these are no longer used.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../pipe/interface/ia_css_pipe_stagedesc.h    |  5 -----
 .../pci/camera/pipe/src/pipe_stagedesc.c      | 21 -------------------
 drivers/staging/media/atomisp/pci/sh_css.c    |  9 --------
 .../media/atomisp/pci/sh_css_internal.h       |  3 ---
 4 files changed, 38 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/camera/pipe/interface/ia_css_pipe_stagedesc.h b/drivers/staging/media/atomisp/pci/camera/pipe/interface/ia_css_pipe_stagedesc.h
index 40c8145a0797..7a0c988d89ee 100644
--- a/drivers/staging/media/atomisp/pci/camera/pipe/interface/ia_css_pipe_stagedesc.h
+++ b/drivers/staging/media/atomisp/pci/camera/pipe/interface/ia_css_pipe_stagedesc.h
@@ -38,11 +38,6 @@ void ia_css_pipe_get_firmwares_stage_desc(
     const struct ia_css_fw_info *fw,
     unsigned int mode);
 
-void ia_css_pipe_get_acc_stage_desc(
-    struct ia_css_pipeline_stage_desc *stage_desc,
-    struct ia_css_binary *binary,
-    struct ia_css_fw_info *fw);
-
 void ia_css_pipe_get_sp_func_stage_desc(
     struct ia_css_pipeline_stage_desc *stage_desc,
     struct ia_css_frame *out_frame,
diff --git a/drivers/staging/media/atomisp/pci/camera/pipe/src/pipe_stagedesc.c b/drivers/staging/media/atomisp/pci/camera/pipe/src/pipe_stagedesc.c
index 82a24aabe8ce..6c93fa1c683b 100644
--- a/drivers/staging/media/atomisp/pci/camera/pipe/src/pipe_stagedesc.c
+++ b/drivers/staging/media/atomisp/pci/camera/pipe/src/pipe_stagedesc.c
@@ -74,27 +74,6 @@ void ia_css_pipe_get_firmwares_stage_desc(
 	stage_desc->vf_frame = vf_frame;
 }
 
-void ia_css_pipe_get_acc_stage_desc(
-    struct ia_css_pipeline_stage_desc *stage_desc,
-    struct ia_css_binary *binary,
-    struct ia_css_fw_info *fw)
-{
-	unsigned int i;
-
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
-			    "ia_css_pipe_get_acc_stage_desc() enter:\n");
-	stage_desc->binary = binary;
-	stage_desc->firmware = fw;
-	stage_desc->sp_func = IA_CSS_PIPELINE_NO_FUNC;
-	stage_desc->max_input_width = 0;
-	stage_desc->mode = IA_CSS_BINARY_MODE_VF_PP;
-	stage_desc->in_frame = NULL;
-	for (i = 0; i < IA_CSS_BINARY_MAX_OUTPUT_PORTS; i++) {
-		stage_desc->out_frame[i] = NULL;
-	}
-	stage_desc->vf_frame = NULL;
-}
-
 void ia_css_pipe_get_sp_func_stage_desc(
     struct ia_css_pipeline_stage_desc *stage_desc,
     struct ia_css_frame *out_frame,
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index 33348a43c1de..726cb7aa4ecd 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -1629,15 +1629,6 @@ ia_css_enable_isys_event_queue(bool enable)
 	return 0;
 }
 
-/* For Acceleration API: Flush FW (shared buffer pointer) arguments */
-void
-sh_css_flush(struct ia_css_acc_fw *fw)
-{
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "sh_css_flush() enter:\n");
-	if ((fw) && (my_css.flush))
-		my_css.flush(fw);
-}
-
 /*
  * Mapping sp threads. Currently, this is done when a stream is created and
  * pipelines are ready to be converted to sp pipelines. Be careful if you are
diff --git a/drivers/staging/media/atomisp/pci/sh_css_internal.h b/drivers/staging/media/atomisp/pci/sh_css_internal.h
index 0441d4fec551..d98f1323441e 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_internal.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_internal.h
@@ -897,9 +897,6 @@ sh_css_params_init(void);
 void
 sh_css_params_uninit(void);
 
-/* For Acceleration API: Flush FW (shared buffer pointer) arguments */
-void sh_css_flush(struct ia_css_acc_fw *fw);
-
 void
 sh_css_binary_args_reset(struct sh_css_binary_args *args);
 
-- 
2.38.1

[PATCH 08/20] media: atomisp: Remove atomisp_css_yuvpp_configure_viewfinder() function

[Hans de Goede]

Remove atomisp_css_yuvpp_configure_viewfinder(), it is not used anywhere.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../media/atomisp/pci/atomisp_compat.h        |  7 ------
 .../media/atomisp/pci/atomisp_compat_css20.c  | 23 -------------------
 2 files changed, 30 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat.h b/drivers/staging/media/atomisp/pci/atomisp_compat.h
index d1893a0deec1..f73801197dd7 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat.h
@@ -257,13 +257,6 @@ int atomisp_css_yuvpp_configure_output(struct atomisp_sub_device *asd,
 				       unsigned int padded_width,
 				       enum ia_css_frame_format format);
 
-int atomisp_css_yuvpp_configure_viewfinder(
-    struct atomisp_sub_device *asd,
-    unsigned int stream_index,
-    unsigned int width, unsigned int height,
-    unsigned int min_width,
-    enum ia_css_frame_format format);
-
 int atomisp_css_yuvpp_get_output_frame_info(
     struct atomisp_sub_device *asd,
     unsigned int stream_index,
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index c59f9e6b1e2b..adf77c9e481e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -2742,29 +2742,6 @@ int atomisp_css_yuvpp_configure_output(struct atomisp_sub_device *asd,
 	return 0;
 }
 
-int atomisp_css_yuvpp_configure_viewfinder(
-    struct atomisp_sub_device *asd,
-    unsigned int stream_index,
-    unsigned int width, unsigned int height,
-    unsigned int min_width,
-    enum ia_css_frame_format format)
-{
-	struct atomisp_stream_env *stream_env =
-		    &asd->stream_env[stream_index];
-	enum ia_css_pipe_id pipe_id = IA_CSS_PIPE_ID_YUVPP;
-
-	stream_env->pipe_configs[pipe_id].mode =
-	    __pipe_id_to_pipe_mode(asd, pipe_id);
-	stream_env->update_pipe[pipe_id] = true;
-
-	stream_env->pipe_configs[pipe_id].vf_output_info[0].res.width = width;
-	stream_env->pipe_configs[pipe_id].vf_output_info[0].res.height = height;
-	stream_env->pipe_configs[pipe_id].vf_output_info[0].format = format;
-	stream_env->pipe_configs[pipe_id].vf_output_info[0].padded_width =
-	    min_width;
-	return 0;
-}
-
 int atomisp_css_yuvpp_get_output_frame_info(
     struct atomisp_sub_device *asd,
     unsigned int stream_index,
-- 
2.38.1

[PATCH 09/20] media: atomisp: Remove unused ia_css_frame_*() functions

[Hans de Goede]

After the conversion to videobuf2 a bunch of ia_css_frame_*()
functions are unused, remove them.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../media/atomisp/pci/ia_css_frame_public.h   |  74 -----------
 .../atomisp/pci/runtime/frame/src/frame.c     | 121 ------------------
 2 files changed, 195 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/ia_css_frame_public.h b/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
index 32d6d9699c37..7ba464abf447 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
@@ -205,15 +205,6 @@ struct ia_css_frame {
 	.flash_state		= IA_CSS_FRAME_FLASH_STATE_NONE, \
 }
 
-/* @brief Fill a frame with zeros
- *
- * @param	frame		The frame.
- * @return	None
- *
- * Fill a frame with pixel values of zero
- */
-void ia_css_frame_zero(struct ia_css_frame *frame);
-
 /* @brief Allocate a CSS frame structure
  *
  * @param	frame		The allocated frame.
@@ -270,71 +261,6 @@ ia_css_frame_allocate_from_info(struct ia_css_frame **frame,
 void
 ia_css_frame_free(struct ia_css_frame *frame);
 
-/* @brief Allocate a CSS frame structure using a frame info structure.
- *
- * @param	frame	The allocated frame.
- * @param[in]	info	The frame info structure.
- * @return		The error code.
- *
- * Allocate an empty CSS frame with no data buffer using the parameters
- * in the frame info.
- */
-int
-ia_css_frame_create_from_info(struct ia_css_frame **frame,
-			      const struct ia_css_frame_info *info);
-
-/* @brief Set a mapped data buffer to a CSS frame
- *
- * @param[in]	frame       Valid CSS frame pointer
- * @param[in]	mapped_data  Mapped data buffer to be assigned to the CSS frame
- * @param[in]	data_size_bytes  Size of the mapped_data in bytes
- * @return      The error code.
- *
- * Sets a mapped data buffer to this frame. This function can be called multiple
- * times with different buffers or NULL to reset the data pointer. This API
- * would not try free the mapped_data and its the callers responsiblity to
- * free the mapped_data buffer. However if ia_css_frame_free() is called and
- * the frame had a valid data buffer, it would be freed along with the frame.
- */
-int
-ia_css_frame_set_data(struct ia_css_frame *frame,
-		      const ia_css_ptr   mapped_data,
-		      size_t data_size_bytes);
-
-/* @brief Map an existing frame data pointer to a CSS frame.
- *
- * @param	frame		Pointer to the frame to be initialized
- * @param[in]	info		The frame info.
- * @param[in]	data		Pointer to the allocated frame data.
- * @param[in]	attribute	Attributes to be passed to mmgr_mmap.
- * @param[in]	context		Pointer to the a context to be passed to mmgr_mmap.
- * @return			The allocated frame structure.
- *
- * This function maps a pre-allocated pointer into a CSS frame. This can be
- * used when an upper software layer is responsible for allocating the frame
- * data and it wants to share that frame pointer with the CSS code.
- * This function will fill the CSS frame structure just like
- * ia_css_frame_allocate() does, but instead of allocating the memory, it will
- * map the pre-allocated memory into the CSS address space.
- */
-int
-ia_css_frame_map(struct ia_css_frame **frame,
-		 const struct ia_css_frame_info *info,
-		 const void __user *data,
-		 unsigned int pgnr);
-
-/* @brief Unmap a CSS frame structure.
- *
- * @param[in]	frame	Pointer to the CSS frame.
- * @return	None
- *
- * This function unmaps the frame data pointer within a CSS frame and
- * then frees the CSS frame structure. Use this for frame pointers created
- * using ia_css_frame_map().
- */
-void
-ia_css_frame_unmap(struct ia_css_frame *frame);
-
 static inline const struct ia_css_frame_info *
 ia_css_frame_get_info(const struct ia_css_frame *frame)
 {
diff --git a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
index 332b4a39e74d..83bb42e05421 100644
--- a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
+++ b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
@@ -88,12 +88,6 @@ ia_css_elems_bytes_from_info(
 **	CSS API functions, exposed by ia_css.h
 **************************************************************************/
 
-void ia_css_frame_zero(struct ia_css_frame *frame)
-{
-	assert(frame);
-	hmm_set(frame->data, 0, frame->data_bytes);
-}
-
 int ia_css_frame_allocate_from_info(struct ia_css_frame **frame,
 	const struct ia_css_frame_info *info)
 {
@@ -143,121 +137,6 @@ int ia_css_frame_allocate(struct ia_css_frame **frame,
 	return err;
 }
 
-int ia_css_frame_map(struct ia_css_frame **frame,
-				 const struct ia_css_frame_info *info,
-				 const void __user *data,
-				 unsigned int pgnr)
-{
-	int err = 0;
-	struct ia_css_frame *me;
-
-	assert(frame);
-
-	/* Create the frame structure */
-	err = ia_css_frame_create_from_info(&me, info);
-
-	if (err)
-		return err;
-
-	if (pgnr < ((PAGE_ALIGN(me->data_bytes)) >> PAGE_SHIFT)) {
-		dev_err(atomisp_dev,
-			"user space memory size is less than the expected size..\n");
-		err = -ENOMEM;
-		goto error;
-	} else if (pgnr > ((PAGE_ALIGN(me->data_bytes)) >> PAGE_SHIFT)) {
-		dev_err(atomisp_dev,
-			"user space memory size is large than the expected size..\n");
-		err = -ENOMEM;
-		goto error;
-	}
-
-	me->data = hmm_create_from_userdata(me->data_bytes, data);
-	if (me->data == mmgr_NULL)
-		err = -EINVAL;
-
-error:
-	if (err) {
-		kvfree(me);
-		me = NULL;
-	}
-
-	*frame = me;
-
-	return err;
-}
-
-int ia_css_frame_create_from_info(struct ia_css_frame **frame,
-	const struct ia_css_frame_info *info)
-{
-	int err = 0;
-	struct ia_css_frame *me;
-
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-			    "ia_css_frame_create_from_info() enter:\n");
-	if (!frame || !info) {
-		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-				    "ia_css_frame_create_from_info() leave: invalid arguments\n");
-		return -EINVAL;
-	}
-
-	me = frame_create(info->res.width,
-			  info->res.height,
-			  info->format,
-			  info->padded_width,
-			  info->raw_bit_depth,
-			  false);
-	if (!me) {
-		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-				    "ia_css_frame_create_from_info() leave: frame create failed\n");
-		return -ENOMEM;
-	}
-
-	err = ia_css_frame_init_planes(me);
-
-	if (err) {
-		kvfree(me);
-		me = NULL;
-	}
-
-	*frame = me;
-
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-			    "ia_css_frame_create_from_info() leave:\n");
-
-	return err;
-}
-
-int ia_css_frame_set_data(struct ia_css_frame *frame,
-				      const ia_css_ptr mapped_data,
-				      size_t data_bytes)
-{
-	int err = 0;
-
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-			    "ia_css_frame_set_data() enter:\n");
-	if (!frame) {
-		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-				    "ia_css_frame_set_data() leave: NULL frame\n");
-		return -EINVAL;
-	}
-
-	/* If we are setting a valid data.
-	 * Make sure that there is enough
-	 * room for the expected frame format
-	 */
-	if ((mapped_data != mmgr_NULL) && (frame->data_bytes > data_bytes)) {
-		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
-				    "ia_css_frame_set_data() leave: invalid arguments\n");
-		return -EINVAL;
-	}
-
-	frame->data = mapped_data;
-
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_frame_set_data() leave:\n");
-
-	return err;
-}
-
 void ia_css_frame_free(struct ia_css_frame *frame)
 {
 	IA_CSS_ENTER_PRIVATE("frame = %p", frame);
-- 
2.38.1

[PATCH 10/20] media: atomisp: Drop userptr support from hmm

[Hans de Goede]

After the conversion to videobuf2 userptr support is no longer needed,
drop it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/include/hmm/hmm.h   |  1 -
 .../media/atomisp/include/hmm/hmm_bo.h        |  2 -
 drivers/staging/media/atomisp/pci/hmm/hmm.c   | 19 +++----
 .../staging/media/atomisp/pci/hmm/hmm_bo.c    | 53 +------------------
 4 files changed, 7 insertions(+), 68 deletions(-)

diff --git a/drivers/staging/media/atomisp/include/hmm/hmm.h b/drivers/staging/media/atomisp/include/hmm/hmm.h
index b81b8580d405..2bc323b34f89 100644
--- a/drivers/staging/media/atomisp/include/hmm/hmm.h
+++ b/drivers/staging/media/atomisp/include/hmm/hmm.h
@@ -37,7 +37,6 @@ int hmm_init(void);
 void hmm_cleanup(void);
 
 ia_css_ptr hmm_alloc(size_t bytes);
-ia_css_ptr hmm_create_from_userdata(size_t bytes, const void __user *userptr);
 ia_css_ptr hmm_create_from_vmalloc_buf(size_t bytes, void *vmalloc_addr);
 
 void hmm_free(ia_css_ptr ptr);
diff --git a/drivers/staging/media/atomisp/include/hmm/hmm_bo.h b/drivers/staging/media/atomisp/include/hmm/hmm_bo.h
index a51d89f0b5cc..b4c03e0ca9c0 100644
--- a/drivers/staging/media/atomisp/include/hmm/hmm_bo.h
+++ b/drivers/staging/media/atomisp/include/hmm/hmm_bo.h
@@ -74,7 +74,6 @@
 enum hmm_bo_type {
 	HMM_BO_PRIVATE,
 	HMM_BO_VMALLOC,
-	HMM_BO_USER,
 	HMM_BO_LAST,
 };
 
@@ -208,7 +207,6 @@ int hmm_bo_allocated(struct hmm_buffer_object *bo);
  */
 int hmm_bo_alloc_pages(struct hmm_buffer_object *bo,
 		       enum hmm_bo_type type,
-		       const void __user *userptr,
 		       void *vmalloc_addr);
 void hmm_bo_free_pages(struct hmm_buffer_object *bo);
 int hmm_bo_page_allocated(struct hmm_buffer_object *bo);
diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c b/drivers/staging/media/atomisp/pci/hmm/hmm.c
index a262477104fc..e266628be332 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm.c
@@ -42,9 +42,8 @@ static bool hmm_initialized;
 /*
  * p: private
  * v: vmalloc
- * u: user
  */
-static const char hmm_bo_type_string[] = "pvu";
+static const char hmm_bo_type_string[] = "pv";
 
 static ssize_t bo_show(struct device *dev, struct device_attribute *attr,
 		       char *buf, struct list_head *bo_list, bool active)
@@ -168,7 +167,6 @@ void hmm_cleanup(void)
 }
 
 static ia_css_ptr __hmm_alloc(size_t bytes, enum hmm_bo_type type,
-			      const void __user *userptr,
 			      void *vmalloc_addr)
 {
 	unsigned int pgnr;
@@ -193,7 +191,7 @@ static ia_css_ptr __hmm_alloc(size_t bytes, enum hmm_bo_type type,
 	}
 
 	/* Allocate pages for memory */
-	ret = hmm_bo_alloc_pages(bo, type, userptr, vmalloc_addr);
+	ret = hmm_bo_alloc_pages(bo, type, vmalloc_addr);
 	if (ret) {
 		dev_err(atomisp_dev, "hmm_bo_alloc_pages failed.\n");
 		goto alloc_page_err;
@@ -207,8 +205,8 @@ static ia_css_ptr __hmm_alloc(size_t bytes, enum hmm_bo_type type,
 	}
 
 	dev_dbg(atomisp_dev,
-		"%s: pages: 0x%08x (%zu bytes), type: %d, user ptr %p\n",
-		__func__, bo->start, bytes, type, userptr);
+		"%s: pages: 0x%08x (%zu bytes), type: %d, vmalloc %p\n",
+		__func__, bo->start, bytes, type, vmalloc);
 
 	return bo->start;
 
@@ -222,17 +220,12 @@ static ia_css_ptr __hmm_alloc(size_t bytes, enum hmm_bo_type type,
 
 ia_css_ptr hmm_alloc(size_t bytes)
 {
-	return __hmm_alloc(bytes, HMM_BO_PRIVATE, NULL, NULL);
+	return __hmm_alloc(bytes, HMM_BO_PRIVATE, NULL);
 }
 
 ia_css_ptr hmm_create_from_vmalloc_buf(size_t bytes, void *vmalloc_addr)
 {
-	return __hmm_alloc(bytes, HMM_BO_VMALLOC, NULL, vmalloc_addr);
-}
-
-ia_css_ptr hmm_create_from_userdata(size_t bytes, const void __user *userptr)
-{
-	return __hmm_alloc(bytes, HMM_BO_USER, userptr, NULL);
+	return __hmm_alloc(bytes, HMM_BO_VMALLOC, vmalloc_addr);
 }
 
 void hmm_free(ia_css_ptr virt)
diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
index 3c150268db51..11c43e296de1 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
@@ -652,49 +652,6 @@ static int alloc_private_pages(struct hmm_buffer_object *bo)
 	return 0;
 }
 
-static void free_user_pages(struct hmm_buffer_object *bo,
-			    unsigned int page_nr)
-{
-	int i;
-
-	for (i = 0; i < page_nr; i++)
-		put_page(bo->pages[i]);
-}
-
-/*
- * Convert user space virtual address into pages list
- */
-static int alloc_user_pages(struct hmm_buffer_object *bo,
-			    const void __user *userptr)
-{
-	int page_nr;
-
-	userptr = untagged_addr(userptr);
-
-	/* Handle frame buffer allocated in user space */
-	mutex_unlock(&bo->mutex);
-	page_nr = get_user_pages_fast((unsigned long)userptr, bo->pgnr, 1, bo->pages);
-	mutex_lock(&bo->mutex);
-
-	/* can be written by caller, not forced */
-	if (page_nr != bo->pgnr) {
-		dev_err(atomisp_dev,
-			"get_user_pages err: bo->pgnr = %d, pgnr actually pinned = %d.\n",
-			bo->pgnr, page_nr);
-		if (page_nr < 0)
-			page_nr = 0;
-		goto out_of_mem;
-	}
-
-	return 0;
-
-out_of_mem:
-
-	free_user_pages(bo, page_nr);
-
-	return -ENOMEM;
-}
-
 static int alloc_vmalloc_pages(struct hmm_buffer_object *bo, void *vmalloc_addr)
 {
 	void *vaddr = vmalloc_addr;
@@ -716,16 +673,12 @@ static int alloc_vmalloc_pages(struct hmm_buffer_object *bo, void *vmalloc_addr)
  * allocate/free physical pages for the bo.
  *
  * type indicate where are the pages from. currently we have 3 types
- * of memory: HMM_BO_PRIVATE, HMM_BO_VMALLOC, HMM_BO_USER.
+ * of memory: HMM_BO_PRIVATE, HMM_BO_VMALLOC.
  *
  * vmalloc_addr is only valid when type is HMM_BO_VMALLOC.
- *
- * userptr is only valid when type is HMM_BO_USER, it indicates
- * the start address from user space task.
  */
 int hmm_bo_alloc_pages(struct hmm_buffer_object *bo,
 		       enum hmm_bo_type type,
-		       const void __user *userptr,
 		       void *vmalloc_addr)
 {
 	int ret = -EINVAL;
@@ -745,8 +698,6 @@ int hmm_bo_alloc_pages(struct hmm_buffer_object *bo,
 		ret = alloc_private_pages(bo);
 	} else if (type == HMM_BO_VMALLOC) {
 		ret = alloc_vmalloc_pages(bo, vmalloc_addr);
-	} else if (type == HMM_BO_USER) {
-		ret = alloc_user_pages(bo, userptr);
 	} else {
 		dev_err(atomisp_dev, "invalid buffer type.\n");
 		ret = -EINVAL;
@@ -792,8 +743,6 @@ void hmm_bo_free_pages(struct hmm_buffer_object *bo)
 		free_private_bo_pages(bo);
 	else if (bo->type == HMM_BO_VMALLOC)
 		; /* No-op, nothing to do */
-	else if (bo->type == HMM_BO_USER)
-		free_user_pages(bo, bo->pgnr);
 	else
 		dev_err(atomisp_dev, "invalid buffer type.\n");
 
-- 
2.38.1

Re: [PATCH 10/20] media: atomisp: Drop userptr support from hmm

[Andy Shevchenko]

On Sun, Nov 20, 2022 at 11:40:51PM +0100, Hans de Goede wrote:
> After the conversion to videobuf2 userptr support is no longer needed,
> drop it.

...

>  	dev_dbg(atomisp_dev,
> -		"%s: pages: 0x%08x (%zu bytes), type: %d, user ptr %p\n",
> -		__func__, bo->start, bytes, type, userptr);
> +		"%s: pages: 0x%08x (%zu bytes), type: %d, vmalloc %p\n",
> +		__func__, bo->start, bytes, type, vmalloc);

I believe we may drop __func__ here as well.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 10/20] media: atomisp: Drop userptr support from hmm

[Hans de Goede]

Hi,

On 11/21/22 10:01, Andy Shevchenko wrote:
> On Sun, Nov 20, 2022 at 11:40:51PM +0100, Hans de Goede wrote:
>> After the conversion to videobuf2 userptr support is no longer needed,
>> drop it.
> 
> ...
> 
>>  	dev_dbg(atomisp_dev,
>> -		"%s: pages: 0x%08x (%zu bytes), type: %d, user ptr %p\n",
>> -		__func__, bo->start, bytes, type, userptr);
>> +		"%s: pages: 0x%08x (%zu bytes), type: %d, vmalloc %p\n",
>> +		__func__, bo->start, bytes, type, vmalloc);
> 
> I believe we may drop __func__ here as well.

Fixed in my media-atomisp branch.

Regards,

Hans

[PATCH 11/20] media: atomisp: Remove double atomisp_mrfld_power_down()/_up() calls from atomisp_reset()

[Hans de Goede]

atomisp_reset() calls atomisp_mrfld_power_down() after calling
atomisp_runtime_suspend(), which already calls
atomisp_mrfld_power_down() itself.

And the some goes for atomisp_runtime_resume() / atomisp_mrfld_power_up().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_cmd.c  | 16 +++++-----------
 drivers/staging/media/atomisp/pci/atomisp_cmd.h  |  2 --
 drivers/staging/media/atomisp/pci/atomisp_v4l2.c |  4 ++--
 3 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 7f33a2f1e3fc..c69a646aa4d5 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -311,17 +311,11 @@ int atomisp_reset(struct atomisp_device *isp)
 	ret = atomisp_runtime_suspend(isp->dev);
 	if (ret < 0)
 		dev_err(isp->dev, "atomisp_runtime_suspend failed, %d\n", ret);
-	ret = atomisp_mrfld_power_down(isp);
-	if (ret < 0) {
-		dev_err(isp->dev, "can not disable ISP power\n");
-	} else {
-		ret = atomisp_mrfld_power_up(isp);
-		if (ret < 0)
-			dev_err(isp->dev, "can not enable ISP power\n");
-		ret = atomisp_runtime_resume(isp->dev);
-		if (ret < 0)
-			dev_err(isp->dev, "atomisp_runtime_resume failed, %d\n", ret);
-	}
+
+	ret = atomisp_runtime_resume(isp->dev);
+	if (ret < 0)
+		dev_err(isp->dev, "atomisp_runtime_resume failed, %d\n", ret);
+
 	ret = atomisp_css_resume(isp);
 	if (ret)
 		isp->isp_fatal_error = true;
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
index d9110bba8c28..61cbdc1215de 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
@@ -341,8 +341,6 @@ int atomisp_inject_a_fake_event(struct atomisp_sub_device *asd, int *event);
 int atomisp_get_invalid_frame_num(struct video_device *vdev,
 				  int *invalid_frame_num);
 
-int atomisp_mrfld_power_up(struct atomisp_device *isp);
-int atomisp_mrfld_power_down(struct atomisp_device *isp);
 int atomisp_runtime_suspend(struct device *dev);
 int atomisp_runtime_resume(struct device *dev);
 #endif /* __ATOMISP_CMD_H__ */
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index d5bb9906ca6f..d7fb700252fa 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -725,13 +725,13 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
 }
 
 /* Workaround for pmu_nc_set_power_state not ready in MRFLD */
-int atomisp_mrfld_power_down(struct atomisp_device *isp)
+static int atomisp_mrfld_power_down(struct atomisp_device *isp)
 {
 	return atomisp_mrfld_power(isp, false);
 }
 
 /* Workaround for pmu_nc_set_power_state not ready in MRFLD */
-int atomisp_mrfld_power_up(struct atomisp_device *isp)
+static int atomisp_mrfld_power_up(struct atomisp_device *isp)
 {
 	return atomisp_mrfld_power(isp, true);
 }
-- 
2.38.1

[PATCH 12/20] media: atomisp: Remove atomisp_mrfld_power_down()/_up()

[Hans de Goede]

atomisp_mrfld_power_down()/_up() are unnecessary wrappers around
atomisp_mrfld_power() remove them and just call atomisp_mrfld_power()
directly.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_v4l2.c  | 22 +++++--------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index d7fb700252fa..f009a1bfd3ea 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -724,18 +724,6 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
 	return -EBUSY;
 }
 
-/* Workaround for pmu_nc_set_power_state not ready in MRFLD */
-static int atomisp_mrfld_power_down(struct atomisp_device *isp)
-{
-	return atomisp_mrfld_power(isp, false);
-}
-
-/* Workaround for pmu_nc_set_power_state not ready in MRFLD */
-static int atomisp_mrfld_power_up(struct atomisp_device *isp)
-{
-	return atomisp_mrfld_power(isp, true);
-}
-
 int atomisp_runtime_suspend(struct device *dev)
 {
 	struct atomisp_device *isp = (struct atomisp_device *)
@@ -751,7 +739,7 @@ int atomisp_runtime_suspend(struct device *dev)
 	if (ret)
 		return ret;
 	cpu_latency_qos_update_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE);
-	return atomisp_mrfld_power_down(isp);
+	return atomisp_mrfld_power(isp, false);
 }
 
 int atomisp_runtime_resume(struct device *dev)
@@ -760,7 +748,7 @@ int atomisp_runtime_resume(struct device *dev)
 				     dev_get_drvdata(dev);
 	int ret;
 
-	ret = atomisp_mrfld_power_up(isp);
+	ret = atomisp_mrfld_power(isp, true);
 	if (ret)
 		return ret;
 
@@ -817,7 +805,7 @@ static int __maybe_unused atomisp_suspend(struct device *dev)
 		return ret;
 	}
 	cpu_latency_qos_update_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE);
-	return atomisp_mrfld_power_down(isp);
+	return atomisp_mrfld_power(isp, false);
 }
 
 static int __maybe_unused atomisp_resume(struct device *dev)
@@ -826,7 +814,7 @@ static int __maybe_unused atomisp_resume(struct device *dev)
 				     dev_get_drvdata(dev);
 	int ret;
 
-	ret = atomisp_mrfld_power_up(isp);
+	ret = atomisp_mrfld_power(isp, true);
 	if (ret)
 		return ret;
 
@@ -1726,7 +1714,7 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	atomisp_ospm_dphy_down(isp);
 
 	/* Address later when we worry about the ...field chips */
-	if (IS_ENABLED(CONFIG_PM) && atomisp_mrfld_power_down(isp))
+	if (IS_ENABLED(CONFIG_PM) && atomisp_mrfld_power(isp, false))
 		dev_err(&pdev->dev, "Failed to switch off ISP\n");
 
 atomisp_dev_alloc_fail:
-- 
2.38.1

[PATCH 13/20] media: atomisp: Remove clearing of config from atomisp_css_uninit()

[Hans de Goede]

atomisp_css_uninit() only runs when all streams are stopped and
atomisp_css_stop() already clears the config, so the clearing
of the config can be dropped from atomisp_css_uninit().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_compat_css20.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index adf77c9e481e..3237585d3044 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -864,15 +864,6 @@ int atomisp_css_load_firmware(struct atomisp_device *isp)
 
 void atomisp_css_uninit(struct atomisp_device *isp)
 {
-	struct atomisp_sub_device *asd;
-	unsigned int i;
-
-	for (i = 0; i < isp->num_of_streams; i++) {
-		asd = &isp->asd[i];
-		memset(&asd->params.config, 0, sizeof(asd->params.config));
-		asd->params.css_update_params_needed = false;
-	}
-
 	isp->css_initialized = false;
 	ia_css_uninit();
 }
-- 
2.38.1

[PATCH 14/20] media: atomisp: Remove atomisp_css_suspend()/_resume()

[Hans de Goede]

atomisp_css_suspend() is a 1:1 copy of atomisp_css_uninit() and
atomisp_css_resume() is a 1:1 copy of atomisp_css_init().

Remove the 2 copies and have their one caller just call
atomisp_css_uninit() / atomisp_css_init() instead.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_cmd.c   |  4 +--
 .../media/atomisp/pci/atomisp_compat.h        |  4 ---
 .../media/atomisp/pci/atomisp_compat_css20.c  | 29 -------------------
 3 files changed, 2 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index c69a646aa4d5..e80038e46e0e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -307,7 +307,7 @@ int atomisp_reset(struct atomisp_device *isp)
 	int ret = 0;
 
 	dev_dbg(isp->dev, "%s\n", __func__);
-	atomisp_css_suspend(isp);
+	atomisp_css_uninit(isp);
 	ret = atomisp_runtime_suspend(isp->dev);
 	if (ret < 0)
 		dev_err(isp->dev, "atomisp_runtime_suspend failed, %d\n", ret);
@@ -316,7 +316,7 @@ int atomisp_reset(struct atomisp_device *isp)
 	if (ret < 0)
 		dev_err(isp->dev, "atomisp_runtime_resume failed, %d\n", ret);
 
-	ret = atomisp_css_resume(isp);
+	ret = atomisp_css_init(isp);
 	if (ret)
 		isp->isp_fatal_error = true;
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat.h b/drivers/staging/media/atomisp/pci/atomisp_compat.h
index f73801197dd7..7316eb9f974a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat.h
@@ -41,10 +41,6 @@ int atomisp_css_init(struct atomisp_device *isp);
 
 void atomisp_css_uninit(struct atomisp_device *isp);
 
-void atomisp_css_suspend(struct atomisp_device *isp);
-
-int atomisp_css_resume(struct atomisp_device *isp);
-
 void atomisp_css_init_struct(struct atomisp_sub_device *asd);
 
 int atomisp_css_irq_translate(struct atomisp_device *isp,
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index 3237585d3044..9d6008659e02 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -868,35 +868,6 @@ void atomisp_css_uninit(struct atomisp_device *isp)
 	ia_css_uninit();
 }
 
-void atomisp_css_suspend(struct atomisp_device *isp)
-{
-	isp->css_initialized = false;
-	ia_css_uninit();
-}
-
-int atomisp_css_resume(struct atomisp_device *isp)
-{
-	unsigned int mmu_base_addr;
-	int ret;
-
-	ret = hmm_get_mmu_base_addr(isp->dev, &mmu_base_addr);
-	if (ret) {
-		dev_err(isp->dev, "get base address error.\n");
-		return -EINVAL;
-	}
-
-	ret = ia_css_init(isp->dev, &isp->css_env.isp_css_env, NULL,
-			  mmu_base_addr, IA_CSS_IRQ_TYPE_PULSE);
-	if (ret) {
-		dev_err(isp->dev, "re-init css failed.\n");
-		return -EINVAL;
-	}
-	ia_css_enable_isys_event_queue(true);
-
-	isp->css_initialized = true;
-	return 0;
-}
-
 int atomisp_css_irq_translate(struct atomisp_device *isp,
 			      unsigned int *infos)
 {
-- 
2.38.1

[PATCH 15/20] media: atomisp: Remove sw_contex.power_state checks

[Hans de Goede]

Remove the unnecessary sw_contex.power_state checks:

1. atomisp_freq_scaling() and atomisp_stop_streaming() only run when the
   ISP is powered up through runtime-pm, so the checkes are not necessary

2. atomisp_mrfld_pre_power_down() and atomisp_runtime_resume() are only
   called through the driver-core pm handling code which already
   guarantees they are not called when already powered down / up.

3. atomisp_isr() also checks isp->css_initialized which only gets set
   by atomisp_css_init() which runs *after* powering up the ISP and
   which gets cleared by atomisp_css_uninit() *before* before powering
   down the ISP.

So all the checks are unnecessary, remove them as well as the
sw_contex.power_state field itself.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_cmd.c   | 20 +----
 .../media/atomisp/pci/atomisp_internal.h      |  1 -
 .../staging/media/atomisp/pci/atomisp_ioctl.c | 75 +++++++++----------
 .../staging/media/atomisp/pci/atomisp_v4l2.c  | 19 ++---
 4 files changed, 43 insertions(+), 72 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index e80038e46e0e..475ba82b858b 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -207,11 +207,6 @@ int atomisp_freq_scaling(struct atomisp_device *isp,
 	int i, ret;
 	unsigned short fps = 0;
 
-	if (isp->sw_contex.power_state != ATOM_ISP_POWER_UP) {
-		dev_err(isp->dev, "DFS cannot proceed due to no power.\n");
-		return -EINVAL;
-	}
-
 	if ((pdev->device & ATOMISP_PCI_DEVICE_SOC_MASK) ==
 	    ATOMISP_PCI_DEVICE_SOC_CHT && ATOMISP_USE_YUVPP(asd))
 		isp->dfs = &dfs_config_cht_soc;
@@ -511,8 +506,8 @@ irqreturn_t atomisp_isr(int irq, void *dev)
 	int err;
 
 	spin_lock_irqsave(&isp->lock, flags);
-	if (isp->sw_contex.power_state != ATOM_ISP_POWER_UP ||
-	    !isp->css_initialized) {
+
+	if (!isp->css_initialized) {
 		spin_unlock_irqrestore(&isp->lock, flags);
 		return IRQ_HANDLED;
 	}
@@ -5503,7 +5498,6 @@ int atomisp_set_shading_table(struct atomisp_sub_device *asd,
 int atomisp_ospm_dphy_down(struct atomisp_device *isp)
 {
 	struct pci_dev *pdev = to_pci_dev(isp->dev);
-	unsigned long flags;
 	u32 reg;
 
 	dev_dbg(isp->dev, "%s\n", __func__);
@@ -5515,9 +5509,6 @@ int atomisp_ospm_dphy_down(struct atomisp_device *isp)
 	if (!atomisp_dev_users(isp))
 		goto done;
 
-	spin_lock_irqsave(&isp->lock, flags);
-	isp->sw_contex.power_state = ATOM_ISP_POWER_DOWN;
-	spin_unlock_irqrestore(&isp->lock, flags);
 done:
 	/*
 	 * MRFLD IUNIT DPHY is located in an always-power-on island
@@ -5533,14 +5524,7 @@ int atomisp_ospm_dphy_down(struct atomisp_device *isp)
 /*Turn on ISP dphy */
 int atomisp_ospm_dphy_up(struct atomisp_device *isp)
 {
-	unsigned long flags;
-
 	dev_dbg(isp->dev, "%s\n", __func__);
-
-	spin_lock_irqsave(&isp->lock, flags);
-	isp->sw_contex.power_state = ATOM_ISP_POWER_UP;
-	spin_unlock_irqrestore(&isp->lock, flags);
-
 	return 0;
 }
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp_internal.h b/drivers/staging/media/atomisp/pci/atomisp_internal.h
index d9d158cdf09e..653e6d74a966 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_internal.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_internal.h
@@ -195,7 +195,6 @@ struct atomisp_regs {
 };
 
 struct atomisp_sw_contex {
-	int power_state;
 	int running_freq;
 };
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 43e899457b91..cb01ba65c88f 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -1461,10 +1461,11 @@ void atomisp_stop_streaming(struct vb2_queue *vq)
 	struct video_device *vdev = &pipe->vdev;
 	struct atomisp_device *isp = asd->isp;
 	struct pci_dev *pdev = to_pci_dev(isp->dev);
+	bool recreate_streams[MAX_STREAM_NUM] = {0};
 	enum ia_css_pipe_id css_pipe_id;
-	int ret;
-	unsigned long flags;
 	bool first_streamoff = false;
+	unsigned long flags;
+	int i, ret;
 
 	mutex_lock(&isp->mutex);
 
@@ -1563,49 +1564,43 @@ void atomisp_stop_streaming(struct vb2_queue *vq)
 	 * ISP work around, need to reset isp
 	 * Is it correct time to reset ISP when first node does streamoff?
 	 */
-	if (isp->sw_contex.power_state == ATOM_ISP_POWER_UP) {
-		unsigned int i;
-		bool recreate_streams[MAX_STREAM_NUM] = {0};
-
-		if (isp->isp_timeout)
-			dev_err(isp->dev, "%s: Resetting with WA activated",
-				__func__);
-		/*
-		 * It is possible that the other asd stream is in the stage
-		 * that v4l2_setfmt is just get called on it, which will
-		 * create css stream on that stream. But at this point, there
-		 * is no way to destroy the css stream created on that stream.
-		 *
-		 * So force stream destroy here.
-		 */
-		for (i = 0; i < isp->num_of_streams; i++) {
-			if (isp->asd[i].stream_prepared) {
-				atomisp_destroy_pipes_stream_force(&isp->
-								   asd[i]);
-				recreate_streams[i] = true;
-			}
+	if (isp->isp_timeout)
+		dev_err(isp->dev, "%s: Resetting with WA activated",
+			__func__);
+	/*
+	 * It is possible that the other asd stream is in the stage
+	 * that v4l2_setfmt is just get called on it, which will
+	 * create css stream on that stream. But at this point, there
+	 * is no way to destroy the css stream created on that stream.
+	 *
+	 * So force stream destroy here.
+	 */
+	for (i = 0; i < isp->num_of_streams; i++) {
+		if (isp->asd[i].stream_prepared) {
+			atomisp_destroy_pipes_stream_force(&isp->asd[i]);
+			recreate_streams[i] = true;
 		}
+	}
 
-		/* disable  PUNIT/ISP acknowlede/handshake - SRSE=3 */
-		pci_write_config_dword(pdev, PCI_I_CONTROL,
-				       isp->saved_regs.i_control | MRFLD_PCI_I_CONTROL_SRSE_RESET_MASK);
-		dev_err(isp->dev, "atomisp_reset");
-		atomisp_reset(isp);
-		for (i = 0; i < isp->num_of_streams; i++) {
-			if (recreate_streams[i]) {
-				int ret2;
-
-				ret2 = atomisp_create_pipes_stream(&isp->asd[i]);
-				if (ret2) {
-					dev_err(isp->dev, "%s error re-creating streams: %d\n",
-						__func__, ret2);
-					if (!ret)
-						ret = ret2;
-				}
+	/* disable  PUNIT/ISP acknowlede/handshake - SRSE=3 */
+	pci_write_config_dword(pdev, PCI_I_CONTROL,
+			       isp->saved_regs.i_control | MRFLD_PCI_I_CONTROL_SRSE_RESET_MASK);
+	dev_err(isp->dev, "atomisp_reset");
+	atomisp_reset(isp);
+	for (i = 0; i < isp->num_of_streams; i++) {
+		if (recreate_streams[i]) {
+			int ret2;
+
+			ret2 = atomisp_create_pipes_stream(&isp->asd[i]);
+			if (ret2) {
+				dev_err(isp->dev, "%s error re-creating streams: %d\n",
+					__func__, ret2);
+				if (!ret)
+					ret = ret2;
 			}
 		}
-		isp->isp_timeout = false;
 	}
+	isp->isp_timeout = false;
 out_unlock:
 	mutex_unlock(&isp->mutex);
 }
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index f009a1bfd3ea..579cbf502fd1 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -573,11 +573,7 @@ static int atomisp_mrfld_pre_power_down(struct atomisp_device *isp)
 	unsigned long flags;
 
 	spin_lock_irqsave(&isp->lock, flags);
-	if (isp->sw_contex.power_state == ATOM_ISP_POWER_DOWN) {
-		spin_unlock_irqrestore(&isp->lock, flags);
-		dev_dbg(isp->dev, "<%s %d.\n", __func__, __LINE__);
-		return 0;
-	}
+
 	/*
 	 * MRFLD HAS requirement: cannot power off i-unit if
 	 * ISP has IRQ not serviced.
@@ -753,13 +749,11 @@ int atomisp_runtime_resume(struct device *dev)
 		return ret;
 
 	cpu_latency_qos_update_request(&isp->pm_qos, isp->max_isr_latency);
-	if (isp->sw_contex.power_state == ATOM_ISP_POWER_DOWN) {
-		/*Turn on ISP d-phy */
-		ret = atomisp_ospm_dphy_up(isp);
-		if (ret) {
-			dev_err(isp->dev, "Failed to power up ISP!.\n");
-			return -EINVAL;
-		}
+	/*Turn on ISP d-phy */
+	ret = atomisp_ospm_dphy_up(isp);
+	if (ret) {
+		dev_err(isp->dev, "Failed to power up ISP!.\n");
+		return -EINVAL;
 	}
 
 	/*restore register values for iUnit and iUnitPHY registers*/
@@ -1447,7 +1441,6 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 
 	isp->dev = &pdev->dev;
 	isp->base = pcim_iomap_table(pdev)[ATOM_ISP_PCI_BAR];
-	isp->sw_contex.power_state = ATOM_ISP_POWER_UP;
 	isp->saved_regs.ispmmadr = start;
 
 	dev_dbg(&pdev->dev, "atomisp mmio base: %p\n", isp->base);
-- 
2.38.1

Re: [PATCH 15/20] media: atomisp: Remove sw_contex.power_state checks

[Andy Shevchenko]

On Sun, Nov 20, 2022 at 11:40:56PM +0100, Hans de Goede wrote:
> Remove the unnecessary sw_contex.power_state checks:
> 
> 1. atomisp_freq_scaling() and atomisp_stop_streaming() only run when the
>    ISP is powered up through runtime-pm, so the checkes are not necessary
> 
> 2. atomisp_mrfld_pre_power_down() and atomisp_runtime_resume() are only
>    called through the driver-core pm handling code which already
>    guarantees they are not called when already powered down / up.
> 
> 3. atomisp_isr() also checks isp->css_initialized which only gets set
>    by atomisp_css_init() which runs *after* powering up the ISP and
>    which gets cleared by atomisp_css_uninit() *before* before powering
>    down the ISP.
> 
> So all the checks are unnecessary, remove them as well as the
> sw_contex.power_state field itself.

...

> +	/*Turn on ISP d-phy */

While at it, you may add a whitespace after /*.

> +	ret = atomisp_ospm_dphy_up(isp);
> +	if (ret) {
> +		dev_err(isp->dev, "Failed to power up ISP!.\n");
> +		return -EINVAL;
>  	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 15/20] media: atomisp: Remove sw_contex.power_state checks

[Andy Shevchenko]

On Mon, Nov 21, 2022 at 11:09:44AM +0200, Andy Shevchenko wrote:
> On Sun, Nov 20, 2022 at 11:40:56PM +0100, Hans de Goede wrote:
> > Remove the unnecessary sw_contex.power_state checks:
> > 
> > 1. atomisp_freq_scaling() and atomisp_stop_streaming() only run when the
> >    ISP is powered up through runtime-pm, so the checkes are not necessary
> > 
> > 2. atomisp_mrfld_pre_power_down() and atomisp_runtime_resume() are only
> >    called through the driver-core pm handling code which already
> >    guarantees they are not called when already powered down / up.
> > 
> > 3. atomisp_isr() also checks isp->css_initialized which only gets set
> >    by atomisp_css_init() which runs *after* powering up the ISP and
> >    which gets cleared by atomisp_css_uninit() *before* before powering
> >    down the ISP.
> > 
> > So all the checks are unnecessary, remove them as well as the
> > sw_contex.power_state field itself.
> 
> ...
> 
> > +	/*Turn on ISP d-phy */
> 
> While at it, you may add a whitespace after /*.

Okay, it's going to be removed by one of the following changes.
No need to amend.

> > +	ret = atomisp_ospm_dphy_up(isp);
> > +	if (ret) {
> > +		dev_err(isp->dev, "Failed to power up ISP!.\n");
> > +		return -EINVAL;
> >  	}

-- 
With Best Regards,
Andy Shevchenko

[PATCH 16/20] media: atomisp: Remove duplication between runtime-pm and normal-pm code

[Hans de Goede]

atomisp_suspend() contains a 1:1 copy of atomisp_runtime_suspend() and
the same goes for the resume() functions.

Rename the runtime functions to atomisp_power_on()/_off() and use these
as runtime-pm handlers as well as helper in other places to remove
the code duplication.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_cmd.c   |  8 ++--
 .../staging/media/atomisp/pci/atomisp_cmd.h   |  4 +-
 .../staging/media/atomisp/pci/atomisp_v4l2.c  | 46 +++----------------
 3 files changed, 12 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 475ba82b858b..dac19db55625 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -303,13 +303,13 @@ int atomisp_reset(struct atomisp_device *isp)
 
 	dev_dbg(isp->dev, "%s\n", __func__);
 	atomisp_css_uninit(isp);
-	ret = atomisp_runtime_suspend(isp->dev);
+	ret = atomisp_power_off(isp->dev);
 	if (ret < 0)
-		dev_err(isp->dev, "atomisp_runtime_suspend failed, %d\n", ret);
+		dev_err(isp->dev, "atomisp_power_off failed, %d\n", ret);
 
-	ret = atomisp_runtime_resume(isp->dev);
+	ret = atomisp_power_on(isp->dev);
 	if (ret < 0)
-		dev_err(isp->dev, "atomisp_runtime_resume failed, %d\n", ret);
+		dev_err(isp->dev, "atomisp_power_on failed, %d\n", ret);
 
 	ret = atomisp_css_init(isp);
 	if (ret)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
index 61cbdc1215de..f7647edb25fb 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
@@ -341,6 +341,6 @@ int atomisp_inject_a_fake_event(struct atomisp_sub_device *asd, int *event);
 int atomisp_get_invalid_frame_num(struct video_device *vdev,
 				  int *invalid_frame_num);
 
-int atomisp_runtime_suspend(struct device *dev);
-int atomisp_runtime_resume(struct device *dev);
+int atomisp_power_off(struct device *dev);
+int atomisp_power_on(struct device *dev);
 #endif /* __ATOMISP_CMD_H__ */
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 579cbf502fd1..f670517bc141 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -720,7 +720,7 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
 	return -EBUSY;
 }
 
-int atomisp_runtime_suspend(struct device *dev)
+int atomisp_power_off(struct device *dev)
 {
 	struct atomisp_device *isp = (struct atomisp_device *)
 				     dev_get_drvdata(dev);
@@ -738,7 +738,7 @@ int atomisp_runtime_suspend(struct device *dev)
 	return atomisp_mrfld_power(isp, false);
 }
 
-int atomisp_runtime_resume(struct device *dev)
+int atomisp_power_on(struct device *dev)
 {
 	struct atomisp_device *isp = (struct atomisp_device *)
 				     dev_get_drvdata(dev);
@@ -771,7 +771,6 @@ static int __maybe_unused atomisp_suspend(struct device *dev)
 	/* FIXME: only has one isp_subdev at present */
 	struct atomisp_sub_device *asd = &isp->asd[0];
 	unsigned long flags;
-	int ret;
 
 	/*
 	 * FIXME: Suspend is not supported by sensors. Abort if any video
@@ -788,45 +787,12 @@ static int __maybe_unused atomisp_suspend(struct device *dev)
 	}
 	spin_unlock_irqrestore(&isp->lock, flags);
 
-	ret = atomisp_mrfld_pre_power_down(isp);
-	if (ret)
-		return ret;
-
-	/*Turn off the ISP d-phy */
-	ret = atomisp_ospm_dphy_down(isp);
-	if (ret) {
-		dev_err(isp->dev, "fail to power off ISP\n");
-		return ret;
-	}
-	cpu_latency_qos_update_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE);
-	return atomisp_mrfld_power(isp, false);
+	return atomisp_power_off(dev);
 }
 
 static int __maybe_unused atomisp_resume(struct device *dev)
 {
-	struct atomisp_device *isp = (struct atomisp_device *)
-				     dev_get_drvdata(dev);
-	int ret;
-
-	ret = atomisp_mrfld_power(isp, true);
-	if (ret)
-		return ret;
-
-	cpu_latency_qos_update_request(&isp->pm_qos, isp->max_isr_latency);
-
-	/*Turn on ISP d-phy */
-	ret = atomisp_ospm_dphy_up(isp);
-	if (ret) {
-		dev_err(isp->dev, "Failed to power up ISP!.\n");
-		return -EINVAL;
-	}
-
-	/*restore register values for iUnit and iUnitPHY registers*/
-	if (isp->saved_regs.pcicmdsts)
-		atomisp_restore_iunit_reg(isp);
-
-	atomisp_freq_scaling(isp, ATOMISP_DFS_MODE_LOW, true);
-	return 0;
+	return atomisp_power_on(dev);
 }
 
 int atomisp_csi_lane_config(struct atomisp_device *isp)
@@ -1755,8 +1721,8 @@ static const struct pci_device_id atomisp_pci_tbl[] = {
 MODULE_DEVICE_TABLE(pci, atomisp_pci_tbl);
 
 static const struct dev_pm_ops atomisp_pm_ops = {
-	.runtime_suspend = atomisp_runtime_suspend,
-	.runtime_resume = atomisp_runtime_resume,
+	.runtime_suspend = atomisp_power_off,
+	.runtime_resume = atomisp_power_on,
 	.suspend = atomisp_suspend,
 	.resume = atomisp_resume,
 };
-- 
2.38.1

[PATCH 17/20] media: atomisp: Move calling of css_[un]init() to power_on()/_off()

[Hans de Goede]

atomisp_css_init() is always called after calling atomisp_power_on()
either directly or through getting a runtime-pm reference.

Likewise atomisp_css_uninit() is always called after calling
atomisp_power_off().

Move the call site of these 2 functions to inside atomisp_power_on() /
atomisp_power_off() to make this more explicit.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_cmd.c  | 8 +++-----
 drivers/staging/media/atomisp/pci/atomisp_fops.c | 9 ---------
 drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 5 ++++-
 3 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index dac19db55625..7821bbd1e8da 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -302,18 +302,16 @@ int atomisp_reset(struct atomisp_device *isp)
 	int ret = 0;
 
 	dev_dbg(isp->dev, "%s\n", __func__);
-	atomisp_css_uninit(isp);
+
 	ret = atomisp_power_off(isp->dev);
 	if (ret < 0)
 		dev_err(isp->dev, "atomisp_power_off failed, %d\n", ret);
 
 	ret = atomisp_power_on(isp->dev);
-	if (ret < 0)
+	if (ret < 0) {
 		dev_err(isp->dev, "atomisp_power_on failed, %d\n", ret);
-
-	ret = atomisp_css_init(isp);
-	if (ret)
 		isp->isp_fatal_error = true;
+	}
 
 	return ret;
 }
diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index 8cff26d42b82..0123429d94f4 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -809,13 +809,6 @@ static int atomisp_open(struct file *file)
 		goto error;
 	}
 
-	/* Init ISP */
-	if (atomisp_css_init(isp)) {
-		ret = -EINVAL;
-		/* Need to clean up CSS init if it fails. */
-		goto css_error;
-	}
-
 	atomisp_dev_init_struct(isp);
 
 	ret = v4l2_subdev_call(isp->flash, core, s_power, 1);
@@ -840,7 +833,6 @@ static int atomisp_open(struct file *file)
 	return 0;
 
 css_error:
-	atomisp_css_uninit(isp);
 	pm_runtime_put(vdev->v4l2_dev->dev);
 error:
 	mutex_unlock(&isp->mutex);
@@ -909,7 +901,6 @@ static int atomisp_release(struct file *file)
 		goto done;
 
 	atomisp_destroy_pipes_stream_force(asd);
-	atomisp_css_uninit(isp);
 
 	if (defer_fw_load) {
 		ia_css_unload_firmware();
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index f670517bc141..f46046d7ef50 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -726,6 +726,8 @@ int atomisp_power_off(struct device *dev)
 				     dev_get_drvdata(dev);
 	int ret;
 
+	atomisp_css_uninit(isp);
+
 	ret = atomisp_mrfld_pre_power_down(isp);
 	if (ret)
 		return ret;
@@ -761,7 +763,8 @@ int atomisp_power_on(struct device *dev)
 		atomisp_restore_iunit_reg(isp);
 
 	atomisp_freq_scaling(isp, ATOMISP_DFS_MODE_LOW, true);
-	return 0;
+
+	return atomisp_css_init(isp);
 }
 
 static int __maybe_unused atomisp_suspend(struct device *dev)
-- 
2.38.1

Re: [PATCH 17/20] media: atomisp: Move calling of css_[un]init() to power_on()/_off()

[Andy Shevchenko]

On Sun, Nov 20, 2022 at 11:40:58PM +0100, Hans de Goede wrote:
> atomisp_css_init() is always called after calling atomisp_power_on()
> either directly or through getting a runtime-pm reference.
> 
> Likewise atomisp_css_uninit() is always called after calling
> atomisp_power_off().
> 
> Move the call site of these 2 functions to inside atomisp_power_on() /
> atomisp_power_off() to make this more explicit.

...

>  	dev_dbg(isp->dev, "%s\n", __func__);
> -	atomisp_css_uninit(isp);
> +
>  	ret = atomisp_power_off(isp->dev);
>  	if (ret < 0)
>  		dev_err(isp->dev, "atomisp_power_off failed, %d\n", ret);
>  
>  	ret = atomisp_power_on(isp->dev);
> -	if (ret < 0)
> +	if (ret < 0) {
>  		dev_err(isp->dev, "atomisp_power_on failed, %d\n", ret);
> -
> -	ret = atomisp_css_init(isp);
> -	if (ret)

>  		isp->isp_fatal_error = true;

This was only set when css_init() failed, now it may be set even when
power_on() fails. Why is it not a problem? Commit message doesn't shed
a light on this change.

> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 17/20] media: atomisp: Move calling of css_[un]init() to power_on()/_off()

[Hans de Goede]

Hi,

On 11/21/22 10:14, Andy Shevchenko wrote:
> On Sun, Nov 20, 2022 at 11:40:58PM +0100, Hans de Goede wrote:
>> atomisp_css_init() is always called after calling atomisp_power_on()
>> either directly or through getting a runtime-pm reference.
>>
>> Likewise atomisp_css_uninit() is always called after calling
>> atomisp_power_off().
>>
>> Move the call site of these 2 functions to inside atomisp_power_on() /
>> atomisp_power_off() to make this more explicit.
> 
> ...
> 
>>  	dev_dbg(isp->dev, "%s\n", __func__);
>> -	atomisp_css_uninit(isp);
>> +
>>  	ret = atomisp_power_off(isp->dev);
>>  	if (ret < 0)
>>  		dev_err(isp->dev, "atomisp_power_off failed, %d\n", ret);
>>  
>>  	ret = atomisp_power_on(isp->dev);
>> -	if (ret < 0)
>> +	if (ret < 0) {
>>  		dev_err(isp->dev, "atomisp_power_on failed, %d\n", ret);
>> -
>> -	ret = atomisp_css_init(isp);
>> -	if (ret)
> 
>>  		isp->isp_fatal_error = true;
> 
> This was only set when css_init() failed, now it may be set even when
> power_on() fails. Why is it not a problem? Commit message doesn't shed
> a light on this change.

Failing to power-on the device is pretty fatal too, but this is indeed
a behavior change. I have updated the commit message to reflect this.

Regards,

Hans

[PATCH 18/20] media: atomisp: Remove atomisp_ospm_dphy_down() call from probe error path

[Hans de Goede]

The only thing which atomisp_ospm_dphy_down() does is disable the CSI
pins, but if we failed to probe the ISP then these will never have
been enabled (because the ISP never started streaming).

So the atomisp_ospm_dphy_down() call in the probe error path is
unnecessary, remove it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index f46046d7ef50..9cb1363abe72 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -1673,8 +1673,6 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 
 	atomisp_msi_irq_uninit(isp);
 
-	atomisp_ospm_dphy_down(isp);
-
 	/* Address later when we worry about the ...field chips */
 	if (IS_ENABLED(CONFIG_PM) && atomisp_mrfld_power(isp, false))
 		dev_err(&pdev->dev, "Failed to switch off ISP\n");
-- 
2.38.1

[PATCH 19/20] media: atomisp: Remove atomisp_ospm_dphy_up()/_down() functions

[Hans de Goede]

atomisp_ospm_dphy_up() is an empty function now and
atomisp_ospm_dphy_down() contains a couple of checks + goto done
statements which don't matter since the function always ends up at
the done label regardless and then it does 1 pci-config write.

Move the single pci-config write directly to atomisp_power_off()
and remove the atomisp_ospm_dphy_up()/_down() functions.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_cmd.c   | 34 -------------------
 .../staging/media/atomisp/pci/atomisp_cmd.h   |  2 --
 .../staging/media/atomisp/pci/atomisp_v4l2.c  | 24 ++++++-------
 3 files changed, 12 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 7821bbd1e8da..e9ce3b9b527c 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -5492,40 +5492,6 @@ int atomisp_set_shading_table(struct atomisp_sub_device *asd,
 	return ret;
 }
 
-/*Turn off ISP dphy */
-int atomisp_ospm_dphy_down(struct atomisp_device *isp)
-{
-	struct pci_dev *pdev = to_pci_dev(isp->dev);
-	u32 reg;
-
-	dev_dbg(isp->dev, "%s\n", __func__);
-
-	/* if ISP timeout, we can force powerdown */
-	if (isp->isp_timeout)
-		goto done;
-
-	if (!atomisp_dev_users(isp))
-		goto done;
-
-done:
-	/*
-	 * MRFLD IUNIT DPHY is located in an always-power-on island
-	 * MRFLD HW design need all CSI ports are disabled before
-	 * powering down the IUNIT.
-	 */
-	pci_read_config_dword(pdev, MRFLD_PCI_CSI_CONTROL, &reg);
-	reg |= MRFLD_ALL_CSI_PORTS_OFF_MASK;
-	pci_write_config_dword(pdev, MRFLD_PCI_CSI_CONTROL, reg);
-	return 0;
-}
-
-/*Turn on ISP dphy */
-int atomisp_ospm_dphy_up(struct atomisp_device *isp)
-{
-	dev_dbg(isp->dev, "%s\n", __func__);
-	return 0;
-}
-
 int atomisp_exif_makernote(struct atomisp_sub_device *asd,
 			   struct atomisp_makernote_info *config)
 {
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
index f7647edb25fb..b8911491581a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
@@ -273,8 +273,6 @@ int atomisp_set_shading_table(struct atomisp_sub_device *asd,
 int atomisp_offline_capture_configure(struct atomisp_sub_device *asd,
 				      struct atomisp_cont_capture_conf *cvf_config);
 
-int atomisp_ospm_dphy_down(struct atomisp_device *isp);
-int atomisp_ospm_dphy_up(struct atomisp_device *isp);
 int atomisp_exif_makernote(struct atomisp_sub_device *asd,
 			   struct atomisp_makernote_info *config);
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 9cb1363abe72..e786b81921da 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -722,9 +722,10 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
 
 int atomisp_power_off(struct device *dev)
 {
-	struct atomisp_device *isp = (struct atomisp_device *)
-				     dev_get_drvdata(dev);
+	struct atomisp_device *isp = dev_get_drvdata(dev);
+	struct pci_dev *pdev = to_pci_dev(dev);
 	int ret;
+	u32 reg;
 
 	atomisp_css_uninit(isp);
 
@@ -732,10 +733,15 @@ int atomisp_power_off(struct device *dev)
 	if (ret)
 		return ret;
 
-	/*Turn off the ISP d-phy*/
-	ret = atomisp_ospm_dphy_down(isp);
-	if (ret)
-		return ret;
+	/*
+	 * MRFLD IUNIT DPHY is located in an always-power-on island
+	 * MRFLD HW design need all CSI ports are disabled before
+	 * powering down the IUNIT.
+	 */
+	pci_read_config_dword(pdev, MRFLD_PCI_CSI_CONTROL, &reg);
+	reg |= MRFLD_ALL_CSI_PORTS_OFF_MASK;
+	pci_write_config_dword(pdev, MRFLD_PCI_CSI_CONTROL, reg);
+
 	cpu_latency_qos_update_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE);
 	return atomisp_mrfld_power(isp, false);
 }
@@ -751,12 +757,6 @@ int atomisp_power_on(struct device *dev)
 		return ret;
 
 	cpu_latency_qos_update_request(&isp->pm_qos, isp->max_isr_latency);
-	/*Turn on ISP d-phy */
-	ret = atomisp_ospm_dphy_up(isp);
-	if (ret) {
-		dev_err(isp->dev, "Failed to power up ISP!.\n");
-		return -EINVAL;
-	}
 
 	/*restore register values for iUnit and iUnitPHY registers*/
 	if (isp->saved_regs.pcicmdsts)
-- 
2.38.1

[PATCH 20/20] media: atomisp_ov2680: Fix 1280x720 -> 1296x736 resolution

[Hans de Goede]

The ov2680_720p_30fps register init list used for the 1296x736 resolution
sets the hsize register to 1296 and the vsize register to 736.

This is actually the right thing to do when combined with the atomISP2
because the ISP requires 16 bytes padding leaving userspace to see
1280x720.

But the resolution list entries for this was incorrectly reporting
the resolution being send to the ISP as already being 1280x720,
leaving usespace to see 1274x704 as resolution.

Worse then userspace seeing a weird resolution selecting the
1280x720 sensor resolution (which in reality is sending 1296x736
to the ISP was causing the ISP to hang on Cherry Trail based tablets
(Bay Trail works fine for some reason).

This commit also adds a bunch of comments annotating what
the various register writes in the init lists are doing.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/i2c/ov2680.h | 46 +++++++++++-----------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/ov2680.h b/drivers/staging/media/atomisp/i2c/ov2680.h
index 4e351196fe34..7ab337b859ad 100644
--- a/drivers/staging/media/atomisp/i2c/ov2680.h
+++ b/drivers/staging/media/atomisp/i2c/ov2680.h
@@ -485,19 +485,19 @@ static struct ov2680_reg const ov2680_720x592_30fps[] = {
 static struct ov2680_reg const ov2680_800x600_30fps[] = {
 	{0x3086, 0x01},
 	{0x370a, 0x23},
-	{0x3801, 0x00},
+	{0x3801, 0x00}, /* hstart 0 */
 	{0x3802, 0x00},
-	{0x3803, 0x00},
+	{0x3803, 0x00}, /* vstart 0 */
 	{0x3804, 0x06},
-	{0x3805, 0x4f},
+	{0x3805, 0x4f}, /* hend 1615 */
 	{0x3806, 0x04},
-	{0x3807, 0xbf},
+	{0x3807, 0xbf}, /* vend 1215 */
 	{0x3808, 0x03},
-	{0x3809, 0x20},
+	{0x3809, 0x20}, /* hsize 800 */
 	{0x380a, 0x02},
-	{0x380b, 0x58},
+	{0x380b, 0x58}, /* vsize 600 */
 	{0x380c, 0x06},
-	{0x380d, 0xac},
+	{0x380d, 0xac}, /* htotal 1708 */
 	{0x3810, 0x00},
 	{0x3811, 0x00},
 	{0x3812, 0x00},
@@ -524,19 +524,19 @@ static struct ov2680_reg const ov2680_800x600_30fps[] = {
 static struct ov2680_reg const ov2680_720p_30fps[] = {
 	{0x3086, 0x00},
 	{0x370a, 0x21},
-	{0x3801, 0xa0},
+	{0x3801, 0xa0}, /* hstart 160 */
 	{0x3802, 0x00},
-	{0x3803, 0xf2},
+	{0x3803, 0xf2}, /* vstart 242 */
 	{0x3804, 0x05},
-	{0x3805, 0xbf},
+	{0x3805, 0xbf}, /* hend 1471 */
 	{0x3806, 0x03},
-	{0x3807, 0xdd},
+	{0x3807, 0xdd}, /* vend 989 */
 	{0x3808, 0x05},
-	{0x3809, 0x10},
+	{0x3809, 0x10}, /* hsize 1296 */
 	{0x380a, 0x02},
-	{0x380b, 0xe0},
+	{0x380b, 0xe0}, /* vsize 736 */
 	{0x380c, 0x06},
-	{0x380d, 0xa8},
+	{0x380d, 0xa8}, /* htotal 1704 */
 	{0x3810, 0x00},
 	{0x3811, 0x08},
 	{0x3812, 0x00},
@@ -563,19 +563,19 @@ static struct ov2680_reg const ov2680_720p_30fps[] = {
 static struct ov2680_reg const ov2680_1296x976_30fps[] = {
 	{0x3086, 0x00},
 	{0x370a, 0x21},
-	{0x3801, 0xa0},
+	{0x3801, 0xa0}, /* hstart 160 */
 	{0x3802, 0x00},
-	{0x3803, 0x78},
+	{0x3803, 0x78}, /* vstart 120 */
 	{0x3804, 0x05},
-	{0x3805, 0xbf},
+	{0x3805, 0xbf}, /* hend 1471 */
 	{0x3806, 0x04},
-	{0x3807, 0x57},
+	{0x3807, 0x57}, /* vend 1111 */
 	{0x3808, 0x05},
-	{0x3809, 0x10},
+	{0x3809, 0x10}, /* hsize 1296 */
 	{0x380a, 0x03},
-	{0x380b, 0xd0},
+	{0x380b, 0xd0}, /* vsize 976 */
 	{0x380c, 0x06},
-	{0x380d, 0xa8},
+	{0x380d, 0xa8}, /* htotal 1704 */
 	{0x3810, 0x00},
 	{0x3811, 0x08},
 	{0x3812, 0x00},
@@ -820,8 +820,8 @@ static struct ov2680_resolution ov2680_res_preview[] = {
 		.regs = ov2680_1296x976_30fps,
 	},
 	{
-		.width = 1280,
-		.height = 720,
+		.width = 1296,
+		.height = 736,
 		.fps = 60,
 		.pix_clk_freq = 66,
 		.pixels_per_line = 1698,//1704,
-- 
2.38.1

Re: [PATCH 00/20] media: atomisp: Misc. cleanups / fixes

[Andy Shevchenko]

On Sun, Nov 20, 2022 at 11:40:41PM +0100, Hans de Goede wrote:
> Hi All,
> 
> Here is another round of atomisp cleanups / fixes + preparation work
> for getting rid of the ugly PCI power-management errors in dmesg
> when doing (runtime)suspend/resume.

As usual, for non-commented (by me) patches:
Reviewed-by: Andy Shevchenko <andy@kernel.org>

> Regards,
> 
> Hans
> 
> 
> Hans de Goede (20):
>   media: atomisp: Silence: 'atomisp_q_one_s3a_buffer: drop one s3a stat
>     which has exp_id xx' log messages
>   media: atomisp: Remove accelerator pipe creation code
>   media: atomisp: Remove unused QOS defines / structure member
>   media: atomisp: Flush queue on atomisp_css_start() error
>   media: atomisp: Log an error on failing to alloc private-mem
>   media: atomisp: Fix deadlock when the /dev/video# node is closed while
>     still streaming
>   media: atomisp: Remove 2 unused accelerator mode related functions
>   media: atomisp: Remove atomisp_css_yuvpp_configure_viewfinder()
>     function
>   media: atomisp: Remove unused ia_css_frame_*() functions
>   media: atomisp: Drop userptr support from hmm
>   media: atomisp: Remove double atomisp_mrfld_power_down()/_up() calls
>     from atomisp_reset()
>   media: atomisp: Remove atomisp_mrfld_power_down()/_up()
>   media: atomisp: Remove clearing of config from atomisp_css_uninit()
>   media: atomisp: Remove atomisp_css_suspend()/_resume()
>   media: atomisp: Remove sw_contex.power_state checks
>   media: atomisp: Remove duplication between runtime-pm and normal-pm
>     code
>   media: atomisp: Move calling of css_[un]init() to power_on()/_off()
>   media: atomisp: Remove atomisp_ospm_dphy_down() call from probe error
>     path
>   media: atomisp: Remove atomisp_ospm_dphy_up()/_down() functions
>   media: atomisp_ov2680: Fix 1280x720 -> 1296x736 resolution
> 
>  drivers/staging/media/atomisp/i2c/ov2680.h    |  46 +-
>  .../staging/media/atomisp/include/hmm/hmm.h   |   1 -
>  .../media/atomisp/include/hmm/hmm_bo.h        |   2 -
>  .../staging/media/atomisp/pci/atomisp_cmd.c   |  76 +--
>  .../staging/media/atomisp/pci/atomisp_cmd.h   |   8 +-
>  .../media/atomisp/pci/atomisp_compat.h        |  11 -
>  .../media/atomisp/pci/atomisp_compat_css20.c  |  88 +--
>  .../staging/media/atomisp/pci/atomisp_fops.c  |  34 +-
>  .../media/atomisp/pci/atomisp_internal.h      |   1 -
>  .../staging/media/atomisp/pci/atomisp_ioctl.c |  80 ++-
>  .../staging/media/atomisp/pci/atomisp_v4l2.c  | 104 +--
>  .../pipe/interface/ia_css_pipe_stagedesc.h    |   5 -
>  .../pci/camera/pipe/src/pipe_stagedesc.c      |  21 -
>  drivers/staging/media/atomisp/pci/hmm/hmm.c   |  19 +-
>  .../staging/media/atomisp/pci/hmm/hmm_bo.c    |  54 +-
>  .../media/atomisp/pci/ia_css_frame_public.h   |  74 ---
>  .../staging/media/atomisp/pci/ia_css_pipe.h   |   3 +-
>  .../media/atomisp/pci/ia_css_pipe_public.h    |  69 --
>  .../pci/runtime/debug/src/ia_css_debug.c      |   7 -
>  .../atomisp/pci/runtime/frame/src/frame.c     | 121 ----
>  .../pipeline/interface/ia_css_pipeline.h      |   2 -
>  .../pci/runtime/pipeline/src/pipeline.c       |   8 -
>  drivers/staging/media/atomisp/pci/sh_css.c    | 608 +-----------------
>  .../media/atomisp/pci/sh_css_internal.h       |  13 -
>  .../staging/media/atomisp/pci/sh_css_legacy.h |   1 -
>  drivers/staging/media/atomisp/pci/sh_css_sp.c |   2 +-
>  26 files changed, 121 insertions(+), 1337 deletions(-)
> 
> -- 
> 2.38.1
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 00/20] media: atomisp: Misc. cleanups / fixes

[Hans de Goede]

Hi,

On 11/21/22 10:20, Andy Shevchenko wrote:
> On Sun, Nov 20, 2022 at 11:40:41PM +0100, Hans de Goede wrote:
>> Hi All,
>>
>> Here is another round of atomisp cleanups / fixes + preparation work
>> for getting rid of the ugly PCI power-management errors in dmesg
>> when doing (runtime)suspend/resume.
> 
> As usual, for non-commented (by me) patches:
> Reviewed-by: Andy Shevchenko <andy@kernel.org>

Thank you for all the reviews!

The patches with your suggested fixes + Reviewed-by have all
been updated in my media-atomisp branch.

Regards,

Hans

>> Hans de Goede (20):
>>   media: atomisp: Silence: 'atomisp_q_one_s3a_buffer: drop one s3a stat
>>     which has exp_id xx' log messages
>>   media: atomisp: Remove accelerator pipe creation code
>>   media: atomisp: Remove unused QOS defines / structure member
>>   media: atomisp: Flush queue on atomisp_css_start() error
>>   media: atomisp: Log an error on failing to alloc private-mem
>>   media: atomisp: Fix deadlock when the /dev/video# node is closed while
>>     still streaming
>>   media: atomisp: Remove 2 unused accelerator mode related functions
>>   media: atomisp: Remove atomisp_css_yuvpp_configure_viewfinder()
>>     function
>>   media: atomisp: Remove unused ia_css_frame_*() functions
>>   media: atomisp: Drop userptr support from hmm
>>   media: atomisp: Remove double atomisp_mrfld_power_down()/_up() calls
>>     from atomisp_reset()
>>   media: atomisp: Remove atomisp_mrfld_power_down()/_up()
>>   media: atomisp: Remove clearing of config from atomisp_css_uninit()
>>   media: atomisp: Remove atomisp_css_suspend()/_resume()
>>   media: atomisp: Remove sw_contex.power_state checks
>>   media: atomisp: Remove duplication between runtime-pm and normal-pm
>>     code
>>   media: atomisp: Move calling of css_[un]init() to power_on()/_off()
>>   media: atomisp: Remove atomisp_ospm_dphy_down() call from probe error
>>     path
>>   media: atomisp: Remove atomisp_ospm_dphy_up()/_down() functions
>>   media: atomisp_ov2680: Fix 1280x720 -> 1296x736 resolution
>>
>>  drivers/staging/media/atomisp/i2c/ov2680.h    |  46 +-
>>  .../staging/media/atomisp/include/hmm/hmm.h   |   1 -
>>  .../media/atomisp/include/hmm/hmm_bo.h        |   2 -
>>  .../staging/media/atomisp/pci/atomisp_cmd.c   |  76 +--
>>  .../staging/media/atomisp/pci/atomisp_cmd.h   |   8 +-
>>  .../media/atomisp/pci/atomisp_compat.h        |  11 -
>>  .../media/atomisp/pci/atomisp_compat_css20.c  |  88 +--
>>  .../staging/media/atomisp/pci/atomisp_fops.c  |  34 +-
>>  .../media/atomisp/pci/atomisp_internal.h      |   1 -
>>  .../staging/media/atomisp/pci/atomisp_ioctl.c |  80 ++-
>>  .../staging/media/atomisp/pci/atomisp_v4l2.c  | 104 +--
>>  .../pipe/interface/ia_css_pipe_stagedesc.h    |   5 -
>>  .../pci/camera/pipe/src/pipe_stagedesc.c      |  21 -
>>  drivers/staging/media/atomisp/pci/hmm/hmm.c   |  19 +-
>>  .../staging/media/atomisp/pci/hmm/hmm_bo.c    |  54 +-
>>  .../media/atomisp/pci/ia_css_frame_public.h   |  74 ---
>>  .../staging/media/atomisp/pci/ia_css_pipe.h   |   3 +-
>>  .../media/atomisp/pci/ia_css_pipe_public.h    |  69 --
>>  .../pci/runtime/debug/src/ia_css_debug.c      |   7 -
>>  .../atomisp/pci/runtime/frame/src/frame.c     | 121 ----
>>  .../pipeline/interface/ia_css_pipeline.h      |   2 -
>>  .../pci/runtime/pipeline/src/pipeline.c       |   8 -
>>  drivers/staging/media/atomisp/pci/sh_css.c    | 608 +-----------------
>>  .../media/atomisp/pci/sh_css_internal.h       |  13 -
>>  .../staging/media/atomisp/pci/sh_css_legacy.h |   1 -
>>  drivers/staging/media/atomisp/pci/sh_css_sp.c |   2 +-
>>  26 files changed, 121 insertions(+), 1337 deletions(-)
>>
>> -- 
>> 2.38.1
>>
>