From 5b8c1d30dc358c07aa0ef7676c2ddc3787abcf86 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Thu, 8 Dec 2022 09:12:00 +0100 Subject: media: atomisp: use vb2_start_streaming_called() Don't touch q->start_streaming_called directly, use the vb2_start_streaming_called() function instead. Link: https://lore.kernel.org/r/bc6c24ec-72ea-64a1-9061-311cc7339827@xs4all.nl Signed-off-by: Hans Verkuil Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index cb01ba65c88f..4f35e8f8250a 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -636,10 +636,10 @@ static int atomisp_enum_input(struct file *file, void *fh, static unsigned int atomisp_subdev_streaming_count(struct atomisp_sub_device *asd) { - return asd->video_out_preview.vb_queue.start_streaming_called - + asd->video_out_capture.vb_queue.start_streaming_called - + asd->video_out_video_capture.vb_queue.start_streaming_called - + asd->video_out_vf.vb_queue.start_streaming_called; + return vb2_start_streaming_called(&asd->video_out_preview.vb_queue) + + vb2_start_streaming_called(&asd->video_out_capture.vb_queue) + + vb2_start_streaming_called(&asd->video_out_video_capture.vb_queue) + + vb2_start_streaming_called(&asd->video_out_vf.vb_queue); } unsigned int atomisp_streaming_count(struct atomisp_device *isp) -- cgit v1.2.3 From 3376f06932f85eda824597f6ef93fccbbb92b64f Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 22 Dec 2022 23:00:48 +0100 Subject: media: atomisp: Propagate set_fmt() errors in queue_setup() If set_fmt() fails make queue_setup() actually return the error instead of returning 0. This fixes the following oops on set_fmt() failures: [ 1060.378662] ------------[ cut here ]------------ [ 1060.378805] WARNING: CPU: 0 PID: 2080 at drivers/media/common/videobuf2/videobuf2-core.c:840 vb2_core_reqbufs+0x3f7/0x430 [videobuf2_common] ... [ 1060.381414] RIP: 0010:vb2_core_reqbufs+0x3f7/0x430 [videobuf2_common] ... [ 1060.382066] vb2_ioctl_reqbufs+0x9d/0xe0 [videobuf2_v4l2] [ 1060.382181] __video_do_ioctl+0x18e/0x3c0 [videodev] Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_fops.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c index acea7492847d..d953240cc908 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c @@ -80,7 +80,7 @@ static int atomisp_queue_setup(struct vb2_queue *vq, out: mutex_unlock(&pipe->asd->isp->mutex); - return 0; + return ret; } static int atomisp_buf_init(struct vb2_buffer *vb) -- cgit v1.2.3 From 60ec70a71a9f9975a5d2dd4a7d97c20da0e41976 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 28 Dec 2022 23:11:47 +0100 Subject: media: atomisp: Only set default_run_mode on first open of a stream/asd Calling v4l2_ctrl_s_ctrl(asd->run_mode, pipe->default_run_mode) when the stream is already active (through another /dev/video# node) causes the stream to stop. Move the call to set the default run-mode so that it is only done on the first open of one of the 4 /dev/video# nodes of one of the 2 streams (atomisp-sub-devices / asd-s). Fixes: 2c45e343c581 ("media: atomisp: set per-device's default mode") Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_fops.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c index d953240cc908..ccdd780234a7 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c @@ -821,13 +821,13 @@ init_subdev: goto done; atomisp_subdev_init_struct(asd); + /* Ensure that a mode is set */ + v4l2_ctrl_s_ctrl(asd->run_mode, pipe->default_run_mode); done: pipe->users++; mutex_unlock(&isp->mutex); - /* Ensure that a mode is set */ - v4l2_ctrl_s_ctrl(asd->run_mode, pipe->default_run_mode); return 0; -- cgit v1.2.3 From 2e18e118c22594cced8121e6ab7ca27a60bcfc29 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 21 Jan 2023 16:54:03 +0100 Subject: media: atomisp: Fix WARN() when the vb2 start_streaming callback fails The videobuf2-core expects buffers to be put back in the queued state when the vb2 start_streaming callback fails. But the atomisp atomisp_flush_video_pipe() would unconditionally return them to the core in an error state. This triggers the following warning in the videobuf2-core: drivers/media/common/videobuf2/videobuf2-core.c:1652: /* * If done_list is not empty, then start_streaming() didn't call * vb2_buffer_done(vb, VB2_BUF_STATE_QUEUED) but STATE_ERROR or * STATE_DONE. */ WARN_ON(!list_empty(&q->done_list)); Fix this by adding a state argument to atomisp_flush_video_pipe() and use VB2_BUF_STATE_QUEUED as state when atomisp_start_streaming() fails. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_cmd.c | 17 +++++++++-------- drivers/staging/media/atomisp/pci/atomisp_cmd.h | 3 ++- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 4 ++-- 3 files changed, 13 insertions(+), 11 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c index d8c7e7367386..e798fa7de5a1 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c @@ -679,7 +679,8 @@ void atomisp_buffer_done(struct ia_css_frame *frame, enum vb2_buffer_state state vb2_buffer_done(&frame->vb.vb2_buf, state); } -void atomisp_flush_video_pipe(struct atomisp_video_pipe *pipe, bool warn_on_css_frames) +void atomisp_flush_video_pipe(struct atomisp_video_pipe *pipe, enum vb2_buffer_state state, + bool warn_on_css_frames) { struct ia_css_frame *frame, *_frame; unsigned long irqflags; @@ -689,15 +690,15 @@ void atomisp_flush_video_pipe(struct atomisp_video_pipe *pipe, bool warn_on_css_ list_for_each_entry_safe(frame, _frame, &pipe->buffers_in_css, queue) { if (warn_on_css_frames) dev_warn(pipe->isp->dev, "Warning: CSS frames queued on flush\n"); - atomisp_buffer_done(frame, VB2_BUF_STATE_ERROR); + atomisp_buffer_done(frame, state); } list_for_each_entry_safe(frame, _frame, &pipe->activeq, queue) - atomisp_buffer_done(frame, VB2_BUF_STATE_ERROR); + atomisp_buffer_done(frame, state); list_for_each_entry_safe(frame, _frame, &pipe->buffers_waiting_for_param, queue) { pipe->frame_request_config_id[frame->vb.vb2_buf.index] = 0; - atomisp_buffer_done(frame, VB2_BUF_STATE_ERROR); + atomisp_buffer_done(frame, state); } spin_unlock_irqrestore(&pipe->irq_lock, irqflags); @@ -706,10 +707,10 @@ void atomisp_flush_video_pipe(struct atomisp_video_pipe *pipe, bool warn_on_css_ /* Returns queued buffers back to video-core */ void atomisp_flush_bufs_and_wakeup(struct atomisp_sub_device *asd) { - atomisp_flush_video_pipe(&asd->video_out_capture, false); - atomisp_flush_video_pipe(&asd->video_out_vf, false); - atomisp_flush_video_pipe(&asd->video_out_preview, false); - atomisp_flush_video_pipe(&asd->video_out_video_capture, false); + atomisp_flush_video_pipe(&asd->video_out_capture, VB2_BUF_STATE_ERROR, false); + atomisp_flush_video_pipe(&asd->video_out_vf, VB2_BUF_STATE_ERROR, false); + atomisp_flush_video_pipe(&asd->video_out_preview, VB2_BUF_STATE_ERROR, false); + atomisp_flush_video_pipe(&asd->video_out_video_capture, VB2_BUF_STATE_ERROR, false); } /* clean out the parameters that did not apply */ diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h index b8911491581a..e31ba24ec6d5 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h @@ -57,7 +57,8 @@ struct atomisp_video_pipe *atomisp_to_video_pipe(struct video_device *dev); int atomisp_reset(struct atomisp_device *isp); int atomisp_buffers_in_css(struct atomisp_video_pipe *pipe); void atomisp_buffer_done(struct ia_css_frame *frame, enum vb2_buffer_state state); -void atomisp_flush_video_pipe(struct atomisp_video_pipe *pipe, bool warn_on_css_frames); +void atomisp_flush_video_pipe(struct atomisp_video_pipe *pipe, enum vb2_buffer_state state, + bool warn_on_css_frames); void atomisp_flush_bufs_and_wakeup(struct atomisp_sub_device *asd); void atomisp_clear_css_buffer_counters(struct atomisp_sub_device *asd); diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index 4f35e8f8250a..e534e1f67572 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -1354,7 +1354,7 @@ int atomisp_start_streaming(struct vb2_queue *vq, unsigned int count) ret = atomisp_css_start(asd, css_pipe_id, false); if (ret) { - atomisp_flush_video_pipe(pipe, true); + atomisp_flush_video_pipe(pipe, VB2_BUF_STATE_QUEUED, true); goto out_unlock; } @@ -1530,7 +1530,7 @@ void atomisp_stop_streaming(struct vb2_queue *vq) css_pipe_id = atomisp_get_css_pipe_id(asd); atomisp_css_stop(asd, css_pipe_id, false); - atomisp_flush_video_pipe(pipe, true); + atomisp_flush_video_pipe(pipe, VB2_BUF_STATE_ERROR, true); atomisp_subdev_cleanup_pending_events(asd); stopsensor: -- cgit v1.2.3 From bcc5997250a4e4d44056fb49367ed46fb97bc300 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 28 Dec 2022 20:31:43 +0100 Subject: media: atomisp: Check buffer index is in range inside atomisp_qbuf_wrapper() Check buffer index is in range inside atomisp_qbuf_wrapper() before using it do index pipe->frame_request_config_id[]. Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index e534e1f67572..ef6eaad9b634 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -1067,13 +1067,23 @@ error: return -ENOMEM; } +/* + * FIXME the abuse of buf->reserved2 in the qbuf and dqbuf wrappers comes from + * the original atomisp buffer handling and should be replaced with proper V4L2 + * per frame parameters use. + * + * Once this is fixed these wrappers can be removed, replacing them with direct + * calls to vb2_ioctl_[d]qbuf(). + */ static int atomisp_qbuf_wrapper(struct file *file, void *fh, struct v4l2_buffer *buf) { struct video_device *vdev = video_devdata(file); struct atomisp_device *isp = video_get_drvdata(vdev); struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev); - /* FIXME this abuse of buf->reserved2 comes from the original atomisp buffer handling */ + if (buf->index >= vdev->queue->num_buffers) + return -EINVAL; + if (!atomisp_is_vf_pipe(pipe) && (buf->reserved2 & ATOMISP_BUFFER_HAS_PER_FRAME_SETTING)) { /* this buffer will have a per-frame parameter */ @@ -1106,7 +1116,6 @@ static int atomisp_dqbuf_wrapper(struct file *file, void *fh, struct v4l2_buffer vb = pipe->vb_queue.bufs[buf->index]; frame = vb_to_frame(vb); - /* FIXME this abuse of buf->reserved* comes from the original atomisp buffer handling */ buf->reserved = asd->frame_status[buf->index]; /* -- cgit v1.2.3 From 0c144c9308a66b4a8d7eb9a0f58d251999870fd1 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 1 Dec 2022 22:59:40 +0100 Subject: media: atomisp: Fix regulator registers on BYT devices with CRC PMIC The Crystal Cove PMIC used on some BYT/CHT devices has different revisions when paired with Bay Trail (BYT) vs Cherry Trail (CHT) SoCs. The current hardcoded values are only valid for CHT devices, change the code so that it uses the correct register values on both BYT and CHT. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- .../media/atomisp/pci/atomisp_gmin_platform.c | 24 ++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c index 3d41fab661cf..f7106ddf5c45 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c @@ -57,8 +57,12 @@ enum clock_rate { #define LDO_1P8V_OFF 0x58 /* ... bottom bit is "enabled" */ /* CRYSTAL COVE PMIC register set */ -#define CRYSTAL_1P8V_REG 0x57 -#define CRYSTAL_2P8V_REG 0x5d +#define CRYSTAL_BYT_1P8V_REG 0x5d +#define CRYSTAL_BYT_2P8V_REG 0x66 + +#define CRYSTAL_CHT_1P8V_REG 0x57 +#define CRYSTAL_CHT_2P8V_REG 0x5d + #define CRYSTAL_ON 0x63 #define CRYSTAL_OFF 0x62 @@ -843,6 +847,7 @@ static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on) struct gmin_subdev *gs = find_gmin_subdev(subdev); int ret; int value; + int reg; if (!gs || gs->v1p8_on == on) return 0; @@ -898,10 +903,15 @@ static int gmin_v1p8_ctrl(struct v4l2_subdev *subdev, int on) LDO10_REG, value, 0xff); break; case PMIC_CRYSTALCOVE: + if (IS_ISP2401) + reg = CRYSTAL_CHT_1P8V_REG; + else + reg = CRYSTAL_BYT_1P8V_REG; + value = on ? CRYSTAL_ON : CRYSTAL_OFF; ret = gmin_i2c_write(subdev->dev, gs->pwm_i2c_addr, - CRYSTAL_1P8V_REG, value, 0xff); + reg, value, 0xff); break; default: dev_err(subdev->dev, "Couldn't set power mode for v1p8\n"); @@ -918,6 +928,7 @@ static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on) struct gmin_subdev *gs = find_gmin_subdev(subdev); int ret; int value; + int reg; if (WARN_ON(!gs)) return -ENODEV; @@ -974,10 +985,15 @@ static int gmin_v2p8_ctrl(struct v4l2_subdev *subdev, int on) LDO9_REG, value, 0xff); break; case PMIC_CRYSTALCOVE: + if (IS_ISP2401) + reg = CRYSTAL_CHT_2P8V_REG; + else + reg = CRYSTAL_BYT_2P8V_REG; + value = on ? CRYSTAL_ON : CRYSTAL_OFF; ret = gmin_i2c_write(subdev->dev, gs->pwm_i2c_addr, - CRYSTAL_2P8V_REG, value, 0xff); + reg, value, 0xff); break; default: dev_err(subdev->dev, "Couldn't set power mode for v2p8\n"); -- cgit v1.2.3 From 21b86873711be1aa019ee8991c293efd09c022fd Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 21 Nov 2022 15:06:09 +0100 Subject: media: atomisp: Remove atomisp_sw_contex struct Remove the atomisp_sw_contex struct, it has only 1 member: running_freq, instead store running_freq directly. While at it also change running_freq from an int to an unsigned int, all values stored in it are unsigned and it is compared to the also unsigned new_freq variable. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_cmd.c | 4 ++-- drivers/staging/media/atomisp/pci/atomisp_fops.c | 2 +- drivers/staging/media/atomisp/pci/atomisp_internal.h | 6 +----- 3 files changed, 4 insertions(+), 8 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c index e798fa7de5a1..33accdf8f3b4 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c @@ -280,14 +280,14 @@ int atomisp_freq_scaling(struct atomisp_device *isp, done: dev_dbg(isp->dev, "DFS target frequency=%d.\n", new_freq); - if ((new_freq == isp->sw_contex.running_freq) && !force) + if ((new_freq == isp->running_freq) && !force) return 0; dev_dbg(isp->dev, "Programming DFS frequency to %d\n", new_freq); ret = write_target_freq_to_hw(isp, new_freq); if (!ret) { - isp->sw_contex.running_freq = new_freq; + isp->running_freq = new_freq; trace_ipu_pstate(new_freq, -1); } return ret; diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c index ccdd780234a7..8d5522bff578 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c @@ -681,7 +681,7 @@ static void atomisp_dev_init_struct(struct atomisp_device *isp) * For Merrifield, frequency is scalable. * After boot-up, the default frequency is 200MHz. */ - isp->sw_contex.running_freq = ISP_FREQ_200MHZ; + isp->running_freq = ISP_FREQ_200MHZ; } static void atomisp_subdev_init_struct(struct atomisp_sub_device *asd) diff --git a/drivers/staging/media/atomisp/pci/atomisp_internal.h b/drivers/staging/media/atomisp/pci/atomisp_internal.h index 653e6d74a966..675007d7d9af 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_internal.h +++ b/drivers/staging/media/atomisp/pci/atomisp_internal.h @@ -194,10 +194,6 @@ struct atomisp_regs { u32 csi_access_viol; }; -struct atomisp_sw_contex { - int running_freq; -}; - #define ATOMISP_DEVICE_STREAMING_DISABLED 0 #define ATOMISP_DEVICE_STREAMING_ENABLED 1 #define ATOMISP_DEVICE_STREAMING_STOPPING 2 @@ -242,7 +238,6 @@ struct atomisp_device { struct v4l2_subdev *motor; struct atomisp_regs saved_regs; - struct atomisp_sw_contex sw_contex; struct atomisp_css_env css_env; /* isp timeout status flag */ @@ -257,6 +252,7 @@ struct atomisp_device { unsigned int mipi_frame_size; const struct atomisp_dfs_config *dfs; unsigned int hpll_freq; + unsigned int running_freq; bool css_initialized; }; -- cgit v1.2.3 From 553a64b7e7cef7690203bb07bd0280dd142c1015 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 26 Nov 2022 23:02:49 +0100 Subject: media: atomisp: Move power-management over to a custom pm-domain The atomisp does not use standard PCI power-management through the PCI config space. Instead this driver directly tells the P-Unit to disable the ISP over the IOSF. The standard PCI subsystem pm_ops will try to access the config space before (resume) / after (suspend) this driver has turned the ISP on / off, resulting in the following errors: Unable to change power state from D0 to D3hot, device inaccessible Unable to change power state from D3cold to D0, device inaccessible Getting logged into dmesg a whole bunch of time during boot as well as every time the camera is used. To avoid these errors use a custom pm_domain instead of standard driver pm-callbacks so that all the PCI subsys suspend / resume handling is skipped and call pci_save_state() / pci_restore_state() ourselves. Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- .../staging/media/atomisp/pci/atomisp_internal.h | 1 + drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 44 ++++++++++++++++------ 2 files changed, 33 insertions(+), 12 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_internal.h b/drivers/staging/media/atomisp/pci/atomisp_internal.h index 675007d7d9af..fa38d91420cf 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_internal.h +++ b/drivers/staging/media/atomisp/pci/atomisp_internal.h @@ -210,6 +210,7 @@ struct atomisp_device { void __iomem *base; const struct firmware *firmware; + struct dev_pm_domain pm_domain; struct pm_qos_request pm_qos; s32 max_isr_latency; diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index e786b81921da..e994a4a5284e 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -19,6 +19,7 @@ */ #include #include +#include #include #include #include @@ -524,7 +525,7 @@ static int atomisp_save_iunit_reg(struct atomisp_device *isp) return 0; } -static int __maybe_unused atomisp_restore_iunit_reg(struct atomisp_device *isp) +static int atomisp_restore_iunit_reg(struct atomisp_device *isp) { struct pci_dev *pdev = to_pci_dev(isp->dev); @@ -662,6 +663,7 @@ static void punit_ddr_dvfs_enable(bool enable) static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable) { + struct pci_dev *pdev = to_pci_dev(isp->dev); unsigned long timeout; u32 val = enable ? MRFLD_ISPSSPM0_IUNIT_POWER_ON : MRFLD_ISPSSPM0_IUNIT_POWER_OFF; @@ -703,6 +705,7 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable) tmp = (tmp >> MRFLD_ISPSSPM0_ISPSSS_OFFSET) & MRFLD_ISPSSPM0_ISPSSC_MASK; if (tmp == val) { trace_ipu_cstate(enable); + pdev->current_state = enable ? PCI_D0 : PCI_D3cold; return 0; } @@ -743,6 +746,7 @@ int atomisp_power_off(struct device *dev) pci_write_config_dword(pdev, MRFLD_PCI_CSI_CONTROL, reg); cpu_latency_qos_update_request(&isp->pm_qos, PM_QOS_DEFAULT_VALUE); + pci_save_state(pdev); return atomisp_mrfld_power(isp, false); } @@ -756,6 +760,7 @@ int atomisp_power_on(struct device *dev) if (ret) return ret; + pci_restore_state(to_pci_dev(dev)); cpu_latency_qos_update_request(&isp->pm_qos, isp->max_isr_latency); /*restore register values for iUnit and iUnitPHY registers*/ @@ -767,7 +772,7 @@ int atomisp_power_on(struct device *dev) return atomisp_css_init(isp); } -static int __maybe_unused atomisp_suspend(struct device *dev) +static int atomisp_suspend(struct device *dev) { struct atomisp_device *isp = (struct atomisp_device *) dev_get_drvdata(dev); @@ -790,10 +795,12 @@ static int __maybe_unused atomisp_suspend(struct device *dev) } spin_unlock_irqrestore(&isp->lock, flags); + pm_runtime_resume(dev); + return atomisp_power_off(dev); } -static int __maybe_unused atomisp_resume(struct device *dev) +static int atomisp_resume(struct device *dev) { return atomisp_power_on(dev); } @@ -1603,6 +1610,26 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i /* save the iunit context only once after all the values are init'ed. */ atomisp_save_iunit_reg(isp); + /* + * The atomisp does not use standard PCI power-management through the + * PCI config space. Instead this driver directly tells the P-Unit to + * disable the ISP over the IOSF. The standard PCI subsystem pm_ops will + * try to access the config space before (resume) / after (suspend) this + * driver has turned the ISP on / off, resulting in the following errors: + * + * "Unable to change power state from D0 to D3hot, device inaccessible" + * "Unable to change power state from D3cold to D0, device inaccessible" + * + * To avoid these errors override the pm_domain so that all the PCI + * subsys suspend / resume handling is skipped. + */ + isp->pm_domain.ops.runtime_suspend = atomisp_power_off; + isp->pm_domain.ops.runtime_resume = atomisp_power_on; + isp->pm_domain.ops.suspend = atomisp_suspend; + isp->pm_domain.ops.resume = atomisp_resume; + + dev_pm_domain_set(&pdev->dev, &isp->pm_domain); + pm_runtime_put_noidle(&pdev->dev); pm_runtime_allow(&pdev->dev); @@ -1645,6 +1672,7 @@ css_init_fail: request_irq_fail: hmm_cleanup(); pm_runtime_get_noresume(&pdev->dev); + dev_pm_domain_set(&pdev->dev, NULL); atomisp_unregister_entities(isp); register_entities_fail: atomisp_uninitialize_modules(isp); @@ -1697,6 +1725,7 @@ static void atomisp_pci_remove(struct pci_dev *pdev) pm_runtime_forbid(&pdev->dev); pm_runtime_get_noresume(&pdev->dev); + dev_pm_domain_set(&pdev->dev, NULL); cpu_latency_qos_remove_request(&isp->pm_qos); atomisp_msi_irq_uninit(isp); @@ -1721,17 +1750,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_power_off, - .runtime_resume = atomisp_power_on, - .suspend = atomisp_suspend, - .resume = atomisp_resume, -}; static struct pci_driver atomisp_pci_driver = { - .driver = { - .pm = &atomisp_pm_ops, - }, .name = "atomisp-isp2", .id_table = atomisp_pci_tbl, .probe = atomisp_pci_probe, -- cgit v1.2.3 From d8ba8ba6d5d1a559b882b2ebd0d35e9d517924ff Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 26 Nov 2022 23:08:25 +0100 Subject: media: atomisp: Silence "isys dma store at addr, val" debug messages These are clearly debug messages, printing these all the time is not useful. Silence these by simply removing them altogether. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- .../staging/media/atomisp/pci/css_2401_system/host/isys_dma_private.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/css_2401_system/host/isys_dma_private.h b/drivers/staging/media/atomisp/pci/css_2401_system/host/isys_dma_private.h index a313e1dc7c71..d65fe9ec9049 100644 --- a/drivers/staging/media/atomisp/pci/css_2401_system/host/isys_dma_private.h +++ b/drivers/staging/media/atomisp/pci/css_2401_system/host/isys_dma_private.h @@ -34,8 +34,6 @@ void isys2401_dma_reg_store(const isys2401_dma_ID_t dma_id, reg_loc = ISYS2401_DMA_BASE[dma_id] + (reg * sizeof(hrt_data)); - ia_css_print("isys dma store at addr(0x%x) val(%u)\n", reg_loc, - (unsigned int)value); ia_css_device_store_uint32(reg_loc, value); } -- cgit v1.2.3 From e6548795bb10af1e7aa6668224a47fb294ff24d8 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 15 Jan 2023 15:27:39 +0100 Subject: media: atomisp: Remove non working doorbell check from punit_ddr_dvfs_enable() punit_ddr_dvfs_enable() is only used on CHT devices and there dmesg gets filled with: "DDR DVFS, door bell is not cleared within 3ms" messages, so clearly the doorbell checking is not working. This check was added by: https://github.com/intel/ProductionKernelQuilts/blob/master/uefi/cht-m1stable/patches/cam-0340-atomisp-add-door-bell-for-ddr-dvfs-on-cht.patch Which commit message says: "PUNIT interface added to check Req_ACK of freq status". This suggests that the doorbell mechanism may only be available with certain PUNIT fw versions and it seems that many CHT devices do not have this fw version; that or the doorbell mechanism is not working for other reasons. Revert cam-0340-atomisp-add-door-bell-for-ddr-dvfs-on-cht.patch, replacing the doorbell check with a msleep(20) this fixes dmesg getting filled with error messages. Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index e994a4a5284e..9eea8ffbc3d6 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -638,27 +638,16 @@ done: */ static void punit_ddr_dvfs_enable(bool enable) { - int door_bell = 1 << 8; - int max_wait = 30; int reg; iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, MRFLD_ISPSSDVFS, ®); if (enable) { reg &= ~(MRFLD_BIT0 | MRFLD_BIT1); } else { - reg |= (MRFLD_BIT1 | door_bell); + reg |= MRFLD_BIT1; reg &= ~(MRFLD_BIT0); } iosf_mbi_write(BT_MBI_UNIT_PMC, MBI_REG_WRITE, MRFLD_ISPSSDVFS, reg); - - /* Check Req_ACK to see freq status, wait until door_bell is cleared */ - while ((reg & door_bell) && max_wait--) { - iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ, MRFLD_ISPSSDVFS, ®); - usleep_range(100, 500); - } - - if (max_wait == -1) - pr_info("DDR DVFS, door bell is not cleared within 3ms\n"); } static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable) @@ -671,8 +660,10 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable) dev_dbg(isp->dev, "IUNIT power-%s.\n", enable ? "on" : "off"); /* WA for P-Unit, if DVFS enabled, ISP timeout observed */ - if (IS_CHT && enable) + if (IS_CHT && enable) { punit_ddr_dvfs_enable(false); + msleep(20); + } /* * FIXME:WA for ECS28A, with this sleep, CTS -- cgit v1.2.3 From 94afce19ff62a89e4c161b0ca7cee2cd4a6454e7 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 15 Jan 2023 15:37:41 +0100 Subject: media: atomisp: Remove useless msleep(10) before power-on on BYT On BYT on poweron/runtime-resume the code is doing: 1. Do nothing 2. msleep(10) 3. Start actual poweron sequence Since the runtime resume can happen at any moment, waiting 10ms after it does not really make any sense. According to both the comment and to: https://github.com/intel/ProductionKernelQuilts/blob/master/uefi/cht-m1stable/patches/cam-0341-atomisp-WA-sleep-10ms-when-power-up-ISP-on-byt.patch Which is the patch which originally added this this was added as a workaround for a single test failing on a single model tablet/laptop. So lets just drop this. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 8 -------- 1 file changed, 8 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 9eea8ffbc3d6..aa05c69a5c6b 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -665,14 +665,6 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable) msleep(20); } - /* - * FIXME:WA for ECS28A, with this sleep, CTS - * android.hardware.camera2.cts.CameraDeviceTest#testCameraDeviceAbort - * PASS, no impact on other platforms - */ - if (IS_BYT && enable) - msleep(10); - /* Write to ISPSSPM0 bit[1:0] to power on/off the IUNIT */ iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, MRFLD_ISPSSPM0, val, MRFLD_ISPSSPM0_ISPSSC_MASK); -- cgit v1.2.3 From 8b3332b278756f1f40ed74275da9bd2762986363 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 12 Dec 2022 00:21:54 +0100 Subject: media: atomisp: Remove custom ATOMISP_IOC_ISP_MAKERNOTE ioctl This ioctl simply returns a couple of fixed sensor parameters. With libcamera these fixed parameters are instead stored in a table with sensor-name to parameters mappings (camera_sensor_properties.cpp), so this custom ioctl is not necessary; and it currently has no users. Remove the ioctl and also remove the custom v4l2-ctrls underpinning the ioctl. This is part of a patch-series which tries to remove atomisp specific / custom code from the sensor drivers, with as end goal to make the atomisp drivers regular camera sensor drivers. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_cmd.c | 36 ----------------------- drivers/staging/media/atomisp/pci/atomisp_cmd.h | 3 -- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 7 ----- 3 files changed, 46 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c index 33accdf8f3b4..6ee845375a94 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c @@ -5493,42 +5493,6 @@ out: return ret; } -int atomisp_exif_makernote(struct atomisp_sub_device *asd, - struct atomisp_makernote_info *config) -{ - struct v4l2_control ctrl; - struct atomisp_device *isp = asd->isp; - - ctrl.id = V4L2_CID_FOCAL_ABSOLUTE; - if (v4l2_g_ctrl - (isp->inputs[asd->input_curr].camera->ctrl_handler, &ctrl)) { - dev_warn(isp->dev, "failed to g_ctrl for focal length\n"); - return -EINVAL; - } else { - config->focal_length = ctrl.value; - } - - ctrl.id = V4L2_CID_FNUMBER_ABSOLUTE; - if (v4l2_g_ctrl - (isp->inputs[asd->input_curr].camera->ctrl_handler, &ctrl)) { - dev_warn(isp->dev, "failed to g_ctrl for f-number\n"); - return -EINVAL; - } else { - config->f_number_curr = ctrl.value; - } - - ctrl.id = V4L2_CID_FNUMBER_RANGE; - if (v4l2_g_ctrl - (isp->inputs[asd->input_curr].camera->ctrl_handler, &ctrl)) { - dev_warn(isp->dev, "failed to g_ctrl for f number range\n"); - return -EINVAL; - } else { - config->f_number_range = ctrl.value; - } - - return 0; -} - int atomisp_offline_capture_configure(struct atomisp_sub_device *asd, struct atomisp_cont_capture_conf *cvf_config) { diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h index e31ba24ec6d5..2dfb4abfe463 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h @@ -274,9 +274,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_exif_makernote(struct atomisp_sub_device *asd, - struct atomisp_makernote_info *config); - void atomisp_free_internal_buffers(struct atomisp_sub_device *asd); int atomisp_s_ae_window(struct atomisp_sub_device *asd, diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index ef6eaad9b634..13fc6074dfde 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -1640,7 +1640,6 @@ static int atomisp_g_ctrl(struct file *file, void *fh, switch (control->id) { case V4L2_CID_IRIS_ABSOLUTE: case V4L2_CID_EXPOSURE_ABSOLUTE: - case V4L2_CID_FNUMBER_ABSOLUTE: case V4L2_CID_2A_STATUS: case V4L2_CID_AUTO_N_PRESET_WHITE_BALANCE: case V4L2_CID_EXPOSURE: @@ -1837,7 +1836,6 @@ static int atomisp_camera_g_ext_ctrls(struct file *file, void *fh, case V4L2_CID_EXPOSURE_ABSOLUTE: case V4L2_CID_EXPOSURE_AUTO: case V4L2_CID_IRIS_ABSOLUTE: - case V4L2_CID_FNUMBER_ABSOLUTE: case V4L2_CID_BIN_FACTOR_HORZ: case V4L2_CID_BIN_FACTOR_VERT: case V4L2_CID_3A_LOCK: @@ -1949,7 +1947,6 @@ static int atomisp_camera_s_ext_ctrls(struct file *file, void *fh, case V4L2_CID_EXPOSURE_AUTO: case V4L2_CID_EXPOSURE_METERING: case V4L2_CID_IRIS_ABSOLUTE: - case V4L2_CID_FNUMBER_ABSOLUTE: case V4L2_CID_VCM_TIMING: case V4L2_CID_VCM_SLEW: case V4L2_CID_3A_LOCK: @@ -2285,10 +2282,6 @@ static long atomisp_vidioc_default(struct file *file, void *fh, err = atomisp_fixed_pattern_table(asd, arg); break; - case ATOMISP_IOC_ISP_MAKERNOTE: - err = atomisp_exif_makernote(asd, arg); - break; - case ATOMISP_IOC_G_SENSOR_MODE_DATA: err = atomisp_get_sensor_mode_data(asd, arg); break; -- cgit v1.2.3 From 7f04875057eb68e6b371f05ff055134dba1e27d5 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 12 Dec 2022 22:19:07 +0100 Subject: media: atomisp: Remove custom ATOMISP_IOC_G_SENSOR_MODE_DATA ioctl This ioctl returns a number of fixed sensor parameters + a number of mode-specific parameters. With libcamera these fixed parameters are instead stored in a table with sensor-name to parameters mappings (camera_sensor_properties.cpp); and the variable parameters can be derived from the set fmt. So this custom ioctl is not necessary; and it currently has no users. Remove the ioctl and all the sensor drivers xxxx_get_intg_factor() helpers which return this info. This is part of a patch-series which tries to remove atomisp specific / custom code from the sensor drivers, with as end goal to make the atomisp drivers regular camera sensor drivers. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_cmd.c | 19 ------------------- drivers/staging/media/atomisp/pci/atomisp_cmd.h | 3 --- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 4 ---- 3 files changed, 26 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c index 6ee845375a94..b9e7ad57040e 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c @@ -4212,25 +4212,6 @@ int atomisp_digital_zoom(struct atomisp_sub_device *asd, int flag, return 0; } -/* - * Function to get sensor specific info for current resolution, - * which will be used for auto exposure conversion. - */ -int atomisp_get_sensor_mode_data(struct atomisp_sub_device *asd, - struct atomisp_sensor_mode_data *config) -{ - struct camera_mipi_info *mipi_info; - struct atomisp_device *isp = asd->isp; - - mipi_info = atomisp_to_sensor_mipi_info( - isp->inputs[asd->input_curr].camera); - if (!mipi_info) - return -EINVAL; - - memcpy(config, &mipi_info->data, sizeof(*config)); - return 0; -} - static void __atomisp_update_stream_env(struct atomisp_sub_device *asd, u16 stream_index, struct atomisp_input_stream_info *stream_info) { diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h index 2dfb4abfe463..733b9f8cd06f 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h @@ -259,9 +259,6 @@ int atomisp_makeup_css_parameters(struct atomisp_sub_device *asd, int atomisp_compare_grid(struct atomisp_sub_device *asd, struct atomisp_grid_info *atomgrid); -int atomisp_get_sensor_mode_data(struct atomisp_sub_device *asd, - struct atomisp_sensor_mode_data *config); - /* This function looks up the closest available resolution. */ int atomisp_try_fmt(struct video_device *vdev, struct v4l2_pix_format *f, bool *res_overflow); diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index 13fc6074dfde..1e1464abf32b 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -2282,10 +2282,6 @@ static long atomisp_vidioc_default(struct file *file, void *fh, err = atomisp_fixed_pattern_table(asd, arg); break; - case ATOMISP_IOC_G_SENSOR_MODE_DATA: - err = atomisp_get_sensor_mode_data(asd, arg); - break; - case ATOMISP_IOC_G_MOTOR_PRIV_INT_DATA: if (motor) err = v4l2_subdev_call(motor, core, ioctl, cmd, arg); -- cgit v1.2.3 From 159a61da965a3f099b35c98e9f635e6d14d87d7a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 12 Dec 2022 22:32:22 +0100 Subject: media: atomisp: Remove V4L2_CID_BIN_FACTOR_HORZ/_VERT The bin-factor-x and bin-factor-y ctrls are only used internally to get a single value to pass to atomisp_css_input_set_binning_factor(), which is supposed to tune the lens-shading correction for the binning factor. But all sensor drivers return either 0 or 1 for this, with 0 meaning unset and 1 meaning no-binning. Even though some modes do actually do binning ... Also note that the removed atomisp_get_sensor_bin_factor() would fall back to 0 if either the x and y factor differ or if the ctrls are not implemented (not all sensor drivers implement them). Simply always pass 0 to atomisp_css_input_set_binning_factor(). This is part of a patch-series which tries to remove atomisp specific / custom code from the sensor drivers, with as end goal to make the atomisp drivers regular camera sensor drivers. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 20 ------------ drivers/staging/media/atomisp/pci/atomisp_subdev.c | 36 +--------------------- 2 files changed, 1 insertion(+), 55 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index 1e1464abf32b..b7872d7ac0c3 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -172,24 +172,6 @@ static struct v4l2_queryctrl ci_v4l2_controls[] = { .step = 1, .default_value = 1, }, - { - .id = V4L2_CID_BIN_FACTOR_HORZ, - .type = V4L2_CTRL_TYPE_INTEGER, - .name = "Horizontal binning factor", - .minimum = 0, - .maximum = 10, - .step = 1, - .default_value = 0, - }, - { - .id = V4L2_CID_BIN_FACTOR_VERT, - .type = V4L2_CTRL_TYPE_INTEGER, - .name = "Vertical binning factor", - .minimum = 0, - .maximum = 10, - .step = 1, - .default_value = 0, - }, { .id = V4L2_CID_2A_STATUS, .type = V4L2_CTRL_TYPE_BITMASK, @@ -1836,8 +1818,6 @@ static int atomisp_camera_g_ext_ctrls(struct file *file, void *fh, case V4L2_CID_EXPOSURE_ABSOLUTE: case V4L2_CID_EXPOSURE_AUTO: case V4L2_CID_IRIS_ABSOLUTE: - case V4L2_CID_BIN_FACTOR_HORZ: - case V4L2_CID_BIN_FACTOR_VERT: case V4L2_CID_3A_LOCK: case V4L2_CID_TEST_PATTERN: case V4L2_CID_TEST_PATTERN_COLOR_R: diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c index cadc468b4c2f..fc9e07bf63ae 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c @@ -574,40 +574,6 @@ static int isp_subdev_set_selection(struct v4l2_subdev *sd, sel->target, sel->flags, &sel->r); } -static int atomisp_get_sensor_bin_factor(struct atomisp_sub_device *asd) -{ - struct v4l2_control ctrl = {0}; - struct atomisp_device *isp = asd->isp; - int hbin, vbin; - int ret; - - if (isp->inputs[asd->input_curr].type == FILE_INPUT || - isp->inputs[asd->input_curr].type == TEST_PATTERN) - return 0; - - ctrl.id = V4L2_CID_BIN_FACTOR_HORZ; - ret = - v4l2_g_ctrl(isp->inputs[asd->input_curr].camera->ctrl_handler, - &ctrl); - hbin = ctrl.value; - ctrl.id = V4L2_CID_BIN_FACTOR_VERT; - ret |= - v4l2_g_ctrl(isp->inputs[asd->input_curr].camera->ctrl_handler, - &ctrl); - vbin = ctrl.value; - - /* - * ISP needs to know binning factor from sensor. - * In case horizontal and vertical sensor's binning factors - * are different or sensor does not support binning factor CID, - * ISP will apply default 0 value. - */ - if (ret || hbin != vbin) - hbin = 0; - - return hbin; -} - void atomisp_subdev_set_ffmt(struct v4l2_subdev *sd, struct v4l2_subdev_state *sd_state, uint32_t which, @@ -645,7 +611,7 @@ void atomisp_subdev_set_ffmt(struct v4l2_subdev *sd, ATOMISP_INPUT_STREAM_GENERAL, ffmt); atomisp_css_input_set_binning_factor(isp_sd, ATOMISP_INPUT_STREAM_GENERAL, - atomisp_get_sensor_bin_factor(isp_sd)); + 0); atomisp_css_input_set_bayer_order(isp_sd, ATOMISP_INPUT_STREAM_GENERAL, fc->bayer_order); atomisp_css_input_set_format(isp_sd, ATOMISP_INPUT_STREAM_GENERAL, -- cgit v1.2.3 From 8972ed6ea7a00a39eab8f652e6f2f16a40ed2c3b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 28 Dec 2022 20:14:00 +0100 Subject: media: atomisp: Remove deferred firmware loading support Make atomisp behave like any other drivers and have it load the firmware at probe time (as it was already doing by default). The deferred firmware loading support needlessly complicates the v4l2_file_operations.open callback (atomisp_open()), getting in the way of allowing multiple opens like a normal v4l2 device would. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_fops.c | 25 -------------- drivers/staging/media/atomisp/pci/atomisp_fops.h | 2 -- drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 42 ++++++++---------------- 3 files changed, 14 insertions(+), 55 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c index 8d5522bff578..c99d115d196f 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c @@ -757,25 +757,6 @@ static int atomisp_open(struct file *file) mutex_lock(&isp->mutex); asd->subdev.devnode = vdev; - /* Deferred firmware loading case. */ - if (isp->css_env.isp_css_fw.bytes == 0) { - dev_err(isp->dev, "Deferred firmware load.\n"); - isp->firmware = atomisp_load_firmware(isp); - if (!isp->firmware) { - dev_err(isp->dev, "Failed to load ISP firmware.\n"); - ret = -ENOENT; - goto error; - } - ret = atomisp_css_load_firmware(isp); - if (ret) { - dev_err(isp->dev, "Failed to init css.\n"); - goto error; - } - /* No need to keep FW in memory anymore. */ - release_firmware(isp->firmware); - isp->firmware = NULL; - isp->css_env.isp_css_fw.data = NULL; - } if (!isp->input_cnt) { dev_err(isp->dev, "no camera attached\n"); @@ -901,12 +882,6 @@ static int atomisp_release(struct file *file) atomisp_destroy_pipes_stream_force(asd); - if (defer_fw_load) { - ia_css_unload_firmware(); - isp->css_env.isp_css_fw.data = NULL; - isp->css_env.isp_css_fw.bytes = 0; - } - ret = v4l2_subdev_call(isp->flash, core, s_power, 0); if (ret < 0 && ret != -ENODEV && ret != -ENOIOCTLCMD) dev_warn(isp->dev, "Failed to power-off flash\n"); diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.h b/drivers/staging/media/atomisp/pci/atomisp_fops.h index 10e43126b693..2efc5245e571 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_fops.h +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.h @@ -33,6 +33,4 @@ int atomisp_qbuffers_to_css(struct atomisp_sub_device *asd); extern const struct v4l2_file_operations atomisp_fops; -extern bool defer_fw_load; - #endif /* __ATOMISP_FOPS_H__ */ diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index aa05c69a5c6b..2a949d3dc5d1 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -58,12 +58,6 @@ static uint skip_fwload; module_param(skip_fwload, uint, 0644); MODULE_PARM_DESC(skip_fwload, "Skip atomisp firmware load"); -/* memory optimization: deferred firmware loading */ -bool defer_fw_load; -module_param(defer_fw_load, bool, 0644); -MODULE_PARM_DESC(defer_fw_load, - "Defer FW loading until device is opened (default:disable)"); - /* cross componnet debug message flag */ int dbg_level; module_param(dbg_level, int, 0644); @@ -1514,21 +1508,17 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i isp->max_isr_latency = ATOMISP_MAX_ISR_LATENCY; /* Load isp firmware from user space */ - if (!defer_fw_load) { - isp->firmware = atomisp_load_firmware(isp); - if (!isp->firmware) { - err = -ENOENT; - dev_dbg(&pdev->dev, "Firmware load failed\n"); - goto load_fw_fail; - } + isp->firmware = atomisp_load_firmware(isp); + if (!isp->firmware) { + err = -ENOENT; + dev_dbg(&pdev->dev, "Firmware load failed\n"); + goto load_fw_fail; + } - err = sh_css_check_firmware_version(isp->dev, isp->firmware->data); - if (err) { - dev_dbg(&pdev->dev, "Firmware version check failed\n"); - goto fw_validation_fail; - } - } else { - dev_info(&pdev->dev, "Firmware load will be deferred\n"); + err = sh_css_check_firmware_version(isp->dev, isp->firmware->data); + if (err) { + dev_dbg(&pdev->dev, "Firmware version check failed\n"); + goto fw_validation_fail; } pci_set_master(pdev); @@ -1628,14 +1618,10 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i } /* Load firmware into ISP memory */ - if (!defer_fw_load) { - err = atomisp_css_load_firmware(isp); - if (err) { - dev_err(&pdev->dev, "Failed to init css.\n"); - goto css_init_fail; - } - } else { - dev_dbg(&pdev->dev, "Skip css init.\n"); + err = atomisp_css_load_firmware(isp); + if (err) { + dev_err(&pdev->dev, "Failed to init css.\n"); + goto css_init_fail; } /* Clear FW image from memory */ release_firmware(isp->firmware); -- cgit v1.2.3 From 20734fcae96c8e5fd164e7afe40088ffe4e71803 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 28 Dec 2022 22:48:15 +0100 Subject: media: atomisp: Drop atomisp_init_pipe() atomisp_init_pipe() does 3 things: 1. Init a bunch of list-heads / locks 2. Init the vb_queue for the videodev (aka pipe) 3. zero the per-frame parameters related variables of the pipe 1. and 2. really should not be done at file-open time, but once at probe. Currently the code is getting away with doing this on every videodev-open because only 1 open is allowed at a time. 1. is already done at probe time by atomisp_init_subdev_pipe(), move 2. to atomisp_init_subdev_pipe() so that it is also done once at probe. As for 3. The per-frame parameters can only be set from a qbuf ioctl, which can only happen after a reqbufs ioctl and atomisp_buf_cleanup already zeros the per-frame parameters when the buffers are released, so 3. is not necessary at all. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_fops.c | 40 +----------------- drivers/staging/media/atomisp/pci/atomisp_fops.h | 1 + drivers/staging/media/atomisp/pci/atomisp_subdev.c | 49 +++++++++++++++++----- 3 files changed, 41 insertions(+), 49 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c index c99d115d196f..78af97a64362 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c @@ -624,7 +624,7 @@ static void atomisp_buf_cleanup(struct vb2_buffer *vb) hmm_free(frame->data); } -static const struct vb2_ops atomisp_vb2_ops = { +const struct vb2_ops atomisp_vb2_ops = { .queue_setup = atomisp_queue_setup, .buf_init = atomisp_buf_init, .buf_cleanup = atomisp_buf_cleanup, @@ -633,40 +633,6 @@ static const struct vb2_ops atomisp_vb2_ops = { .stop_streaming = atomisp_stop_streaming, }; -static int atomisp_init_pipe(struct atomisp_video_pipe *pipe) -{ - int ret; - - /* init locks */ - spin_lock_init(&pipe->irq_lock); - mutex_init(&pipe->vb_queue_mutex); - - /* Init videobuf2 queue structure */ - pipe->vb_queue.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - pipe->vb_queue.io_modes = VB2_MMAP | VB2_USERPTR; - pipe->vb_queue.buf_struct_size = sizeof(struct ia_css_frame); - pipe->vb_queue.ops = &atomisp_vb2_ops; - pipe->vb_queue.mem_ops = &vb2_vmalloc_memops; - pipe->vb_queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; - ret = vb2_queue_init(&pipe->vb_queue); - if (ret) - return ret; - - pipe->vdev.queue = &pipe->vb_queue; - pipe->vdev.queue->lock = &pipe->vb_queue_mutex; - - INIT_LIST_HEAD(&pipe->activeq); - INIT_LIST_HEAD(&pipe->buffers_waiting_for_param); - INIT_LIST_HEAD(&pipe->per_frame_params); - memset(pipe->frame_request_config_id, 0, - VIDEO_MAX_FRAME * sizeof(unsigned int)); - memset(pipe->frame_params, 0, - VIDEO_MAX_FRAME * - sizeof(struct atomisp_css_params_with_list *)); - - return 0; -} - static void atomisp_dev_init_struct(struct atomisp_device *isp) { unsigned int i; @@ -773,10 +739,6 @@ static int atomisp_open(struct file *file) return -EBUSY; } - ret = atomisp_init_pipe(pipe); - if (ret) - goto error; - if (atomisp_dev_users(isp)) { dev_dbg(isp->dev, "skip init isp in open\n"); goto init_subdev; diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.h b/drivers/staging/media/atomisp/pci/atomisp_fops.h index 2efc5245e571..883c1851c1c9 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_fops.h +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.h @@ -31,6 +31,7 @@ unsigned int atomisp_sub_dev_users(struct atomisp_sub_device *asd); int atomisp_qbuffers_to_css(struct atomisp_sub_device *asd); +extern const struct vb2_ops atomisp_vb2_ops; extern const struct v4l2_file_operations atomisp_fops; #endif /* __ATOMISP_FOPS_H__ */ diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c index fc9e07bf63ae..c32db4ffb778 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c @@ -25,9 +25,11 @@ #include #include +#include #include "atomisp_cmd.h" #include "atomisp_common.h" #include "atomisp_compat.h" +#include "atomisp_fops.h" #include "atomisp_internal.h" const struct atomisp_in_fmt_conv atomisp_in_fmt_conv[] = { @@ -1023,14 +1025,31 @@ static const struct v4l2_ctrl_config ctrl_depth_mode = { .def = 0, }; -static void atomisp_init_subdev_pipe(struct atomisp_sub_device *asd, - struct atomisp_video_pipe *pipe, enum v4l2_buf_type buf_type) +static int atomisp_init_subdev_pipe(struct atomisp_sub_device *asd, + struct atomisp_video_pipe *pipe, enum v4l2_buf_type buf_type) { + int ret; + pipe->type = buf_type; pipe->asd = asd; pipe->isp = asd->isp; spin_lock_init(&pipe->irq_lock); mutex_init(&pipe->vb_queue_mutex); + + /* Init videobuf2 queue structure */ + pipe->vb_queue.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; + pipe->vb_queue.io_modes = VB2_MMAP | VB2_USERPTR; + pipe->vb_queue.buf_struct_size = sizeof(struct ia_css_frame); + pipe->vb_queue.ops = &atomisp_vb2_ops; + pipe->vb_queue.mem_ops = &vb2_vmalloc_memops; + pipe->vb_queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; + ret = vb2_queue_init(&pipe->vb_queue); + if (ret) + return ret; + + pipe->vdev.queue = &pipe->vb_queue; + pipe->vdev.queue->lock = &pipe->vb_queue_mutex; + INIT_LIST_HEAD(&pipe->buffers_in_css); INIT_LIST_HEAD(&pipe->activeq); INIT_LIST_HEAD(&pipe->buffers_waiting_for_param); @@ -1040,6 +1059,8 @@ static void atomisp_init_subdev_pipe(struct atomisp_sub_device *asd, memset(pipe->frame_params, 0, VIDEO_MAX_FRAME * sizeof(struct atomisp_css_params_with_list *)); + + return 0; } /* @@ -1085,17 +1106,25 @@ static int isp_subdev_init_entities(struct atomisp_sub_device *asd) if (ret < 0) return ret; - atomisp_init_subdev_pipe(asd, &asd->video_out_preview, - V4L2_BUF_TYPE_VIDEO_CAPTURE); + ret = atomisp_init_subdev_pipe(asd, &asd->video_out_preview, + V4L2_BUF_TYPE_VIDEO_CAPTURE); + if (ret) + return ret; - atomisp_init_subdev_pipe(asd, &asd->video_out_vf, - V4L2_BUF_TYPE_VIDEO_CAPTURE); + ret = atomisp_init_subdev_pipe(asd, &asd->video_out_vf, + V4L2_BUF_TYPE_VIDEO_CAPTURE); + if (ret) + return ret; - atomisp_init_subdev_pipe(asd, &asd->video_out_capture, - V4L2_BUF_TYPE_VIDEO_CAPTURE); + ret = atomisp_init_subdev_pipe(asd, &asd->video_out_capture, + V4L2_BUF_TYPE_VIDEO_CAPTURE); + if (ret) + return ret; - atomisp_init_subdev_pipe(asd, &asd->video_out_video_capture, - V4L2_BUF_TYPE_VIDEO_CAPTURE); + ret = atomisp_init_subdev_pipe(asd, &asd->video_out_video_capture, + V4L2_BUF_TYPE_VIDEO_CAPTURE); + if (ret) + return ret; ret = atomisp_video_init(&asd->video_out_capture, "CAPTURE", ATOMISP_RUN_MODE_STILL_CAPTURE); -- cgit v1.2.3 From d24a42b9a643c4999a1710c3a30387208a1b60f6 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 28 Dec 2022 22:56:03 +0100 Subject: media: atomisp: Remove unnecessary memset(foo, 0, sizeof(foo)) calls The memory for all of struct atomisp_video_pipe is kzalloc()-ed in atomisp_subdev_init() so there is no need to memset parts of struct atomisp_video_pipe to 0. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_subdev.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c index c32db4ffb778..eb8f319fca5c 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c @@ -1054,11 +1054,6 @@ static int atomisp_init_subdev_pipe(struct atomisp_sub_device *asd, INIT_LIST_HEAD(&pipe->activeq); INIT_LIST_HEAD(&pipe->buffers_waiting_for_param); INIT_LIST_HEAD(&pipe->per_frame_params); - memset(pipe->frame_request_config_id, - 0, VIDEO_MAX_FRAME * sizeof(unsigned int)); - memset(pipe->frame_params, - 0, VIDEO_MAX_FRAME * - sizeof(struct atomisp_css_params_with_list *)); return 0; } -- cgit v1.2.3 From 5141562bf46935cb6b41c0e871283b06690f6c52 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 30 Dec 2022 19:17:13 +0100 Subject: media: atomisp: Do not turn off sensor when the atomisp-sub-dev does not own it The atomisp driver creates 8 /dev/video# device nodes. 4 nodes (preview / video / viewfinder / capture) for each of 2 possible streams aka atomisp-sub-device-s (asd-s). Both streams start with asd->input_curr set to 0 (to the first sensor), opening + releasing a file-handle on one of the nodes of an asd, while streaming from the other asd causes the sensor to get turned off, leading to the stream failing. The atomisp-code already tracks which asd "owns" a specific sensor, use this to only turn the sensor off if it is owned by the asd. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_fops.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c index 78af97a64362..833c7aac8f0a 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c @@ -828,13 +828,17 @@ static int atomisp_release(struct file *file) atomisp_css_free_stat_buffers(asd); atomisp_free_internal_buffers(asd); - ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera, - core, s_power, 0); - if (ret) - dev_warn(isp->dev, "Failed to power-off sensor\n"); - /* clear the asd field to show this camera is not used */ - isp->inputs[asd->input_curr].asd = NULL; + if (isp->inputs[asd->input_curr].asd == asd) { + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera, + core, s_power, 0); + if (ret) + dev_warn(isp->dev, "Failed to power-off sensor\n"); + + /* clear the asd field to show this camera is not used */ + isp->inputs[asd->input_curr].asd = NULL; + } + spin_lock_irqsave(&isp->lock, flags); asd->streaming = ATOMISP_DEVICE_STREAMING_DISABLED; spin_unlock_irqrestore(&isp->lock, flags); -- cgit v1.2.3 From 3f1125db16a5358ac59fd09fab87aa7b7ea622ce Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 13 Jan 2023 18:06:06 +0100 Subject: media: atomisp: Allow sensor drivers without a s_power callback The s_power callback for v4l2-subdevs has been deprecated, allow sensor drivers without a s_power callback to work by ignoring the -ENOIOCTLCMD return value. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_fops.c | 2 +- drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c index 833c7aac8f0a..ce01479bdd68 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c @@ -832,7 +832,7 @@ static int atomisp_release(struct file *file) if (isp->inputs[asd->input_curr].asd == asd) { ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera, core, s_power, 0); - if (ret) + if (ret && ret != -ENOIOCTLCMD) dev_warn(isp->dev, "Failed to power-off sensor\n"); /* clear the asd field to show this camera is not used */ diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c index b7872d7ac0c3..d1314bdbf7d5 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c @@ -700,7 +700,7 @@ static int atomisp_s_input(struct file *file, void *fh, unsigned int input) asd->input_curr != input) { ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera, core, s_power, 0); - if (ret) + if (ret && ret != -ENOIOCTLCMD) dev_warn(isp->dev, "Failed to power-off sensor\n"); /* clear the asd field to show this camera is not used */ @@ -709,7 +709,7 @@ static int atomisp_s_input(struct file *file, void *fh, unsigned int input) /* powe on the new sensor */ ret = v4l2_subdev_call(isp->inputs[input].camera, core, s_power, 1); - if (ret) { + if (ret && ret != -ENOIOCTLCMD) { dev_err(isp->dev, "Failed to power-on sensor\n"); return ret; } -- cgit v1.2.3 From ba49e91e01876be72186faac75eafd2c0944e581 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 14 Jan 2023 19:10:39 +0100 Subject: media: atomisp: Remove atomisp_gmin_find_subdev() atomisp_gmin_find_subdev() can be used to lookup a subdev given its i2c-adapter + i2c-client-address. But the only caller of it reads this from the intel_v4l2_subdev_table struct and that same struct already contains a pointer to the v4l2_subdev. So this function is not necessary, drop it and modify its only caller to directly take the subdev from the intel_v4l2_subdev_table struct. Also drop struct intel_v4l2_subdev_i2c_board_info since that now no longer is used. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- .../media/atomisp/pci/atomisp_gmin_platform.c | 27 ----------- drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 54 +++------------------- 2 files changed, 7 insertions(+), 74 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c index f7106ddf5c45..ea363f25196c 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c @@ -149,7 +149,6 @@ int atomisp_register_i2c_module(struct v4l2_subdev *subdev, enum intel_v4l2_subdev_type type) { int i; - struct i2c_board_info *bi; struct gmin_subdev *gs; struct i2c_client *client = v4l2_get_subdevdata(subdev); struct acpi_device *adev = ACPI_COMPANION(&client->dev); @@ -183,36 +182,10 @@ int atomisp_register_i2c_module(struct v4l2_subdev *subdev, pdata.subdevs[i].type = type; pdata.subdevs[i].port = gs->csi_port; pdata.subdevs[i].subdev = subdev; - pdata.subdevs[i].v4l2_subdev.i2c_adapter_id = client->adapter->nr; - - /* Convert i2c_client to i2c_board_info */ - bi = &pdata.subdevs[i].v4l2_subdev.board_info; - memcpy(bi->type, client->name, I2C_NAME_SIZE); - bi->flags = client->flags; - bi->addr = client->addr; - bi->irq = client->irq; - bi->platform_data = plat_data; - return 0; } EXPORT_SYMBOL_GPL(atomisp_register_i2c_module); -struct v4l2_subdev *atomisp_gmin_find_subdev(struct i2c_adapter *adapter, - struct i2c_board_info *board_info) -{ - int i; - - for (i = 0; i < MAX_SUBDEVS && pdata.subdevs[i].type; i++) { - struct intel_v4l2_subdev_table *sd = &pdata.subdevs[i]; - - if (sd->v4l2_subdev.i2c_adapter_id == adapter->nr && - sd->v4l2_subdev.board_info.addr == board_info->addr) - return sd->subdev; - } - return NULL; -} -EXPORT_SYMBOL_GPL(atomisp_gmin_find_subdev); - int atomisp_gmin_remove_subdev(struct v4l2_subdev *sd) { int i, j; diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c index 2a949d3dc5d1..ba628f7cf385 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c @@ -937,45 +937,9 @@ static int atomisp_subdev_probe(struct atomisp_device *isp) /* FIXME: should, instead, use I2C probe */ for (subdevs = pdata->subdevs; subdevs->type; ++subdevs) { - struct v4l2_subdev *subdev; - struct i2c_board_info *board_info = - &subdevs->v4l2_subdev.board_info; - struct i2c_adapter *adapter = - i2c_get_adapter(subdevs->v4l2_subdev.i2c_adapter_id); - - dev_info(isp->dev, "Probing Subdev %s\n", board_info->type); - - if (!adapter) { - dev_err(isp->dev, - "Failed to find i2c adapter for subdev %s\n", - board_info->type); - break; - } - - /* In G-Min, the sensor devices will already be probed - * (via ACPI) and registered, do not create new - * ones */ - subdev = atomisp_gmin_find_subdev(adapter, board_info); - if (!subdev) { - dev_warn(isp->dev, "Subdev %s not found\n", - board_info->type); - continue; - } - ret = v4l2_device_register_subdev(&isp->v4l2_dev, subdev); - if (ret) { - dev_warn(isp->dev, "Subdev %s detection fail\n", - board_info->type); + ret = v4l2_device_register_subdev(&isp->v4l2_dev, subdevs->subdev); + if (ret) continue; - } - - if (!subdev) { - dev_warn(isp->dev, "Subdev %s detection fail\n", - board_info->type); - continue; - } - - dev_info(isp->dev, "Subdev %s successfully register\n", - board_info->type); switch (subdevs->type) { case RAW_CAMERA: @@ -992,7 +956,7 @@ static int atomisp_subdev_probe(struct atomisp_device *isp) isp->inputs[isp->input_cnt].type = subdevs->type; isp->inputs[isp->input_cnt].port = subdevs->port; - isp->inputs[isp->input_cnt].camera = subdev; + isp->inputs[isp->input_cnt].camera = subdevs->subdev; isp->inputs[isp->input_cnt].sensor_index = 0; /* * initialize the subdev frame size, then next we can @@ -1004,22 +968,18 @@ static int atomisp_subdev_probe(struct atomisp_device *isp) break; case CAMERA_MOTOR: if (isp->motor) { - dev_warn(isp->dev, - "too many atomisp motors, ignored %s\n", - board_info->type); + dev_warn(isp->dev, "too many atomisp motors\n"); continue; } - isp->motor = subdev; + isp->motor = subdevs->subdev; break; case LED_FLASH: case XENON_FLASH: if (isp->flash) { - dev_warn(isp->dev, - "too many atomisp flash devices, ignored %s\n", - board_info->type); + dev_warn(isp->dev, "too many atomisp flash devices\n"); continue; } - isp->flash = subdev; + isp->flash = subdevs->subdev; break; default: dev_dbg(isp->dev, "unknown subdev probed\n"); -- cgit v1.2.3 From f05cf2545ba86156d6acc024b26f35f21636bb83 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 15 Jan 2023 22:09:57 +0100 Subject: media: atomisp: Add atomisp_register_sensor_no_gmin() helper The DSDT of all Windows BYT / CHT devices which I have seen has proper ACPI powermagement for the clk and regulators used by the sensors. So there is no need for the whole custom atomisp_gmin custom code to disable the ACPI pm and directly poke at the PMIC for this. Add new atomisp_register_sensor_no_gmin() + atomisp_unregister_subdev() helpers which allow registering a sensor with the atomisp code without using any of the atomisp_gmin power-management code. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- .../media/atomisp/pci/atomisp_gmin_platform.c | 61 ++++++++++++++++++++++ 1 file changed, 61 insertions(+) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c index ea363f25196c..34497b4b91d2 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c @@ -1084,6 +1084,67 @@ static int gmin_csi_cfg(struct v4l2_subdev *sd, int flag) return 0; } +int atomisp_register_sensor_no_gmin(struct v4l2_subdev *subdev, u32 lanes, + enum atomisp_input_format format, + enum atomisp_bayer_order bayer_order) +{ + struct i2c_client *client = v4l2_get_subdevdata(subdev); + struct acpi_device *adev = ACPI_COMPANION(&client->dev); + int i, ret, clock_num, port = 0; + + if (adev) { + /* Get ACPI _PR0 derived clock to determine the csi_port default */ + if (acpi_device_power_manageable(adev)) { + clock_num = atomisp_get_acpi_power(&client->dev); + + /* Compare clock to CsiPort 1 pmc-clock used in the CHT/BYT reference designs */ + if (IS_ISP2401) + port = clock_num == 4 ? 1 : 0; + else + port = clock_num == 0 ? 1 : 0; + } + + port = gmin_get_var_int(&client->dev, false, "CsiPort", port); + lanes = gmin_get_var_int(&client->dev, false, "CsiLanes", lanes); + } + + for (i = 0; i < MAX_SUBDEVS; i++) + if (!pdata.subdevs[i].type) + break; + + if (i >= MAX_SUBDEVS) { + dev_err(&client->dev, "Error too many subdevs already registered\n"); + return -ENOMEM; + } + + ret = camera_sensor_csi_alloc(subdev, port, lanes, format, bayer_order); + if (ret) + return ret; + + pdata.subdevs[i].type = RAW_CAMERA; + pdata.subdevs[i].port = port; + pdata.subdevs[i].subdev = subdev; + return 0; +} +EXPORT_SYMBOL_GPL(atomisp_register_sensor_no_gmin); + +void atomisp_unregister_subdev(struct v4l2_subdev *subdev) +{ + int i; + + for (i = 0; i < MAX_SUBDEVS; i++) { + if (pdata.subdevs[i].subdev != subdev) + continue; + + camera_sensor_csi_free(subdev); + pdata.subdevs[i].subdev = NULL; + pdata.subdevs[i].type = 0; + pdata.subdevs[i].port = 0; + break; + } +} +EXPORT_SYMBOL_GPL(atomisp_unregister_subdev); + static struct camera_vcm_control *gmin_get_vcm_ctrl(struct v4l2_subdev *subdev, char *camera_module) { -- cgit v1.2.3 From f629e3865701a666f7881fc009d0f0a446421fab Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 22 Jan 2023 16:51:57 +0100 Subject: media: atomisp: Drop ffmt local var from atomisp_set_fmt() ffmt is a local variable pointing to a substruct of another local variable which really just makes the code harder to read / follow, so drop it. Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_cmd.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c index b9e7ad57040e..eb05288d8fb1 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c @@ -4992,7 +4992,6 @@ int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f) struct v4l2_subdev_format vformat = { .which = V4L2_SUBDEV_FORMAT_ACTIVE, }; - struct v4l2_mbus_framefmt *ffmt = &vformat.format; struct v4l2_rect isp_sink_crop; u16 source_pad = atomisp_subdev_source_pad(vdev); struct v4l2_subdev_fh fh; @@ -5031,17 +5030,17 @@ int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f) /* Ensure that the resolution is equal or below the maximum supported */ vformat.which = V4L2_SUBDEV_FORMAT_ACTIVE; - v4l2_fill_mbus_format(ffmt, &f->fmt.pix, format_bridge->mbus_code); - ffmt->height += padding_h; - ffmt->width += padding_w; + v4l2_fill_mbus_format(&vformat.format, &f->fmt.pix, format_bridge->mbus_code); + vformat.format.height += padding_h; + vformat.format.width += padding_w; ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera, pad, set_fmt, NULL, &vformat); if (ret) return ret; - f->fmt.pix.width = ffmt->width - padding_w; - f->fmt.pix.height = ffmt->height - padding_h; + f->fmt.pix.width = vformat.format.width - padding_w; + f->fmt.pix.height = vformat.format.height - padding_h; snr_fmt = f->fmt.pix; backup_fmt = snr_fmt; -- cgit v1.2.3 From edcb14e5139b09d2e2dc9e9bcf4c1bbdd4ad2e7c Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 22 Jan 2023 16:47:26 +0100 Subject: media: atomisp: Stop overriding padding w/h to 12 on BYT atomisp_set_fmt() first does: v4l2_fill_mbus_format(&vformat.format, ...); vformat.format.height += padding_h; vformat.format.width += padding_w; ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera, pad, set_fmt, NULL, &vformat); if (ret) return ret; f->fmt.pix.width = vformat.format.width - padding_w; f->fmt.pix.height = vformat.format.height - padding_h; this happens with the original padding w/h = 16 values and then later on it calls: ret = atomisp_set_fmt_to_snr(vdev, &s_fmt, f->fmt.pix.pixelformat, padding_w, padding_h, dvs_env_w, dvs_env_h); Which repeats the above structure. If at that point padding w/h are changed to 12 then it will now request a different output-size of the sensor driver. The sensor drivers so far have actually been ignoring this since they use v4l2_find_nearest_size() on a fixed resolution list and the nearest resolution will be the one from the earlier calls where padding w/h was 16. But there really is no reason for sensor drivers to use a fixed resolution list. They make lower resolutions using cropping so they can make any resolution as long as width/height are even numbers. Dropping the fixed-resolution list limit from sensors on BYT results in trying to start streaming failing because the resolution set to the sensor now no longer matches with the resolution used during the initial part of the configuration done by atomisp_set_fmt(). Drop the BYT specific overriding of the padding_w/h to 12, so that the padding in the first and second s_fmt calls made to the sensor matches, to fix stream start failing when the fixed resolution list is dropped. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_cmd.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c index eb05288d8fb1..47f18ac5e40e 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c @@ -5163,9 +5163,6 @@ int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f) if (!atomisp_subdev_format_conversion(asd, source_pad)) { padding_w = 0; padding_h = 0; - } else if (IS_BYT) { - padding_w = 12; - padding_h = 12; } /* construct resolution supported by isp */ -- cgit v1.2.3 From cb90b196647282fc197804d680ad9d86889e27e4 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 22 Jan 2023 20:48:13 +0100 Subject: media: atomisp: Put sensor ACPI devices in D3 before disable ACPI power-resources The device core will call ACPI to turn the device (i2c_client) for a sensor on / put it in D0 before calling its probe() method. This takes a reference on all of the ACPI power-resources belonging to the device. Since the atomisp_gmin_platform code disables ACPI power-resource management and does its own pm, this reference never gets released. This is a problem for ACPI power-resources which are shared with other devices since those now never get turned off again (nor back on again). Explicitly put the device in D3 before disabling the ACPI power-resource management. Note that atomisp_register_i2c_module() runs near the end of the sensor driver's probe() function, after the driver is done with probing the hw. So the power-resouces (the same resources as directly controlled by the atomisp platform code) getting turned off (a second time, as they are already off) is not a problem. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c index 34497b4b91d2..7fc7dfa56172 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c +++ b/drivers/staging/media/atomisp/pci/atomisp_gmin_platform.c @@ -161,6 +161,14 @@ int atomisp_register_i2c_module(struct v4l2_subdev *subdev, * tickled during suspend/resume. This has caused power and * performance issues on multiple devices. */ + + /* + * Turn off the device before disabling ACPI power resources + * (the sensor driver has already probed it at this point). + * This avoids leaking the reference count of the (possibly shared) + * ACPI power resources which were enabled/referenced before probe(). + */ + acpi_device_set_power(adev, ACPI_STATE_D3_COLD); adev->power.flags.power_resources = 0; for (i = 0; i < MAX_SUBDEVS; i++) -- cgit v1.2.3 From 15b5128cafd5760c777945ec24cfadf45b6c1206 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 22 Jan 2023 20:59:23 +0100 Subject: media: atomisp: Remove isp_subdev_link_setup() Looking at isp_subdev_link_setup(), this function can never work, it does a switch-case like this: switch (local->index | is_media_entity_v4l2_subdev(remote->entity)) with cases like this: case ATOMISP_SUBDEV_PAD_SINK | MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN where ATOMISP_SUBDEV_PAD_SINK matches an index (0-4) and MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN is 0x00020000, but is_media_entity_v4l2_subdev(remote->entity) does not return MEDIA_ENT_F_* values, it return a bool, so 0 or 1 which means that non of the cases can ever match the input value. Looking at the rest of the function all it ever does (if it would actually hit one of the cases) is set the atomisp_sub_device struct's input member. And checking the rest of the atomisp code that member is never read. Also userspace does not actually setup media-controller links when using the atomisp /dev/video$ nodes since all the links are fixed. So isp_subdev_link_setup() never runs. Remove the unnecessary and broken isp_subdev_link_setup() function and also remove the unused atomisp_sub_device struct's input member. Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_subdev.c | 79 ---------------------- drivers/staging/media/atomisp/pci/atomisp_subdev.h | 13 ---- 2 files changed, 92 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c index eb8f319fca5c..52d1936b8c87 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c @@ -714,85 +714,8 @@ static void isp_subdev_init_params(struct atomisp_sub_device *asd) } } -/* -* isp_subdev_link_setup - Setup isp subdev connections -* @entity: ispsubdev media entity -* @local: Pad at the local end of the link -* @remote: Pad at the remote end of the link -* @flags: Link flags -* -* return -EINVAL or zero on success -*/ -static int isp_subdev_link_setup(struct media_entity *entity, - const struct media_pad *local, - const struct media_pad *remote, u32 flags) -{ - struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity); - struct atomisp_sub_device *isp_sd = v4l2_get_subdevdata(sd); - struct atomisp_device *isp = isp_sd->isp; - unsigned int i; - - switch (local->index | is_media_entity_v4l2_subdev(remote->entity)) { - case ATOMISP_SUBDEV_PAD_SINK | MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN: - /* Read from the sensor CSI2-ports. */ - if (!(flags & MEDIA_LNK_FL_ENABLED)) { - isp_sd->input = ATOMISP_SUBDEV_INPUT_NONE; - break; - } - - if (isp_sd->input != ATOMISP_SUBDEV_INPUT_NONE) - return -EBUSY; - - for (i = 0; i < ATOMISP_CAMERA_NR_PORTS; i++) { - if (remote->entity != &isp->csi2_port[i].subdev.entity) - continue; - - isp_sd->input = ATOMISP_SUBDEV_INPUT_CSI2_PORT1 + i; - return 0; - } - - return -EINVAL; - - case ATOMISP_SUBDEV_PAD_SINK | MEDIA_ENT_F_OLD_BASE: - /* read from memory */ - if (flags & MEDIA_LNK_FL_ENABLED) { - if (isp_sd->input >= ATOMISP_SUBDEV_INPUT_CSI2_PORT1 && - isp_sd->input < (ATOMISP_SUBDEV_INPUT_CSI2_PORT1 - + ATOMISP_CAMERA_NR_PORTS)) - return -EBUSY; - isp_sd->input = ATOMISP_SUBDEV_INPUT_MEMORY; - } else { - if (isp_sd->input == ATOMISP_SUBDEV_INPUT_MEMORY) - isp_sd->input = ATOMISP_SUBDEV_INPUT_NONE; - } - break; - - case ATOMISP_SUBDEV_PAD_SOURCE_PREVIEW | MEDIA_ENT_F_OLD_BASE: - /* always write to memory */ - break; - - case ATOMISP_SUBDEV_PAD_SOURCE_VF | MEDIA_ENT_F_OLD_BASE: - /* always write to memory */ - break; - - case ATOMISP_SUBDEV_PAD_SOURCE_CAPTURE | MEDIA_ENT_F_OLD_BASE: - /* always write to memory */ - break; - - case ATOMISP_SUBDEV_PAD_SOURCE_VIDEO | MEDIA_ENT_F_OLD_BASE: - /* always write to memory */ - break; - - default: - return -EINVAL; - } - - return 0; -} - /* media operations */ static const struct media_entity_operations isp_subdev_media_ops = { - .link_setup = isp_subdev_link_setup, .link_validate = v4l2_subdev_link_validate, /* .set_power = v4l2_subdev_set_power, */ }; @@ -1071,8 +994,6 @@ static int isp_subdev_init_entities(struct atomisp_sub_device *asd) struct media_entity *me = &sd->entity; int ret; - asd->input = ATOMISP_SUBDEV_INPUT_NONE; - v4l2_subdev_init(sd, &isp_subdev_v4l2_ops); sprintf(sd->name, "ATOMISP_SUBDEV_%d", asd->index); v4l2_set_subdevdata(sd, asd); diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.h b/drivers/staging/media/atomisp/pci/atomisp_subdev.h index bd2872cbb50c..daa6077a83bd 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.h +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.h @@ -30,18 +30,6 @@ /* EXP_ID's ranger is 1 ~ 250 */ #define ATOMISP_MAX_EXP_ID (250) -enum atomisp_subdev_input_entity { - ATOMISP_SUBDEV_INPUT_NONE, - ATOMISP_SUBDEV_INPUT_MEMORY, - ATOMISP_SUBDEV_INPUT_CSI2, - /* - * The following enum for CSI2 port must go together in one row. - * Otherwise it breaks the code logic. - */ - ATOMISP_SUBDEV_INPUT_CSI2_PORT1, - ATOMISP_SUBDEV_INPUT_CSI2_PORT2, - ATOMISP_SUBDEV_INPUT_CSI2_PORT3, -}; #define ATOMISP_SUBDEV_PAD_SINK 0 /* capture output for still frames */ @@ -267,7 +255,6 @@ struct atomisp_sub_device { struct atomisp_pad_format fmt[ATOMISP_SUBDEV_PADS_NUM]; u16 capture_pad; /* main capture pad; defines much of isp config */ - enum atomisp_subdev_input_entity input; unsigned int output; struct atomisp_video_pipe video_out_capture; /* capture output */ struct atomisp_video_pipe video_out_vf; /* viewfinder output */ -- cgit v1.2.3 From c7c49ac854d0d3ef8ff8ab7e667482faa813e757 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 29 Jan 2023 21:49:10 +0100 Subject: media: atomisp: Remove csi2_link_setup() Looking at csi2_link_setup(), this function can never work, it does a switch-case like this: switch (local->index | is_media_entity_v4l2_subdev(remote->entity)) with cases like this: case ATOMISP_SUBDEV_PAD_SOURCE | MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN where ATOMISP_SUBDEV_PAD_SOURCE matches an index (0-1) and MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN is 0x00020000, but is_media_entity_v4l2_subdev(remote->entity) does not return MEDIA_ENT_F_* values, it return a bool, so 0 or 1 which means that non of the cases can ever match the input value. Looking at the rest of the function all it ever does (if it would actually hit one of the cases) is set the atomisp_mipi_csi2_device struct's output member. And checking the rest of the atomisp code that member is never read. Also userspace does not actually setup media-controller links when using the atomisp /dev/video$ nodes since all the links are fixed. So csi2_link_setup() never runs. Remove the unnecessary and broken csi2_link_setup() function and also remove the unused atomisp_mipi_csi2_device struct's output member. Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_csi2.c | 39 ------------------------ drivers/staging/media/atomisp/pci/atomisp_csi2.h | 5 --- 2 files changed, 44 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_csi2.c b/drivers/staging/media/atomisp/pci/atomisp_csi2.c index 4a9268bac8a9..7853a23b9f53 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_csi2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_csi2.c @@ -175,47 +175,8 @@ static const struct v4l2_subdev_ops csi2_ops = { .pad = &csi2_pad_ops, }; -/* - * csi2_link_setup - Setup CSI2 connections. - * @entity : Pointer to media entity structure - * @local : Pointer to local pad array - * @remote : Pointer to remote pad array - * @flags : Link flags - * return -EINVAL or zero on success - */ -static int csi2_link_setup(struct media_entity *entity, - const struct media_pad *local, - const struct media_pad *remote, u32 flags) -{ - struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity); - struct atomisp_mipi_csi2_device *csi2 = v4l2_get_subdevdata(sd); - u32 result = local->index | is_media_entity_v4l2_subdev(remote->entity); - - switch (result) { - case CSI2_PAD_SOURCE | MEDIA_ENT_F_OLD_BASE: - /* not supported yet */ - return -EINVAL; - - case CSI2_PAD_SOURCE | MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN: - if (flags & MEDIA_LNK_FL_ENABLED) { - if (csi2->output & ~CSI2_OUTPUT_ISP_SUBDEV) - return -EBUSY; - csi2->output |= CSI2_OUTPUT_ISP_SUBDEV; - } else { - csi2->output &= ~CSI2_OUTPUT_ISP_SUBDEV; - } - break; - - default: - /* Link from camera to CSI2 is fixed... */ - return -EINVAL; - } - return 0; -} - /* media operations */ static const struct media_entity_operations csi2_media_ops = { - .link_setup = csi2_link_setup, .link_validate = v4l2_subdev_link_validate, }; diff --git a/drivers/staging/media/atomisp/pci/atomisp_csi2.h b/drivers/staging/media/atomisp/pci/atomisp_csi2.h index e35711be8a37..b245b2f5ce99 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_csi2.h +++ b/drivers/staging/media/atomisp/pci/atomisp_csi2.h @@ -25,9 +25,6 @@ #define CSI2_PAD_SOURCE 1 #define CSI2_PADS_NUM 2 -#define CSI2_OUTPUT_ISP_SUBDEV BIT(0) -#define CSI2_OUTPUT_MEMORY BIT(1) - struct atomisp_device; struct v4l2_device; struct atomisp_sub_device; @@ -39,8 +36,6 @@ struct atomisp_mipi_csi2_device { struct v4l2_ctrl_handler ctrls; struct atomisp_device *isp; - - u32 output; /* output direction */ }; int atomisp_csi2_set_ffmt(struct v4l2_subdev *sd, -- cgit v1.2.3 From c47060369f9cf6724af70945aef4ee3907a4a5ce Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 29 Jan 2023 22:01:20 +0100 Subject: media: atomisp: Properly initialize function field of media-entity links Don't use MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN to initialize the function field of various media-entity links. This fixes the following warnings showing up in dmesg: atomisp-isp2 0000:00:03.0: Entity type for entity ATOM ISP CSI2-port0 was not initialized! atomisp-isp2 0000:00:03.0: Entity type for entity ATOM ISP CSI2-port1 was not initialized! atomisp-isp2 0000:00:03.0: Entity type for entity ATOM ISP CSI2-port2 was not initialized! atomisp-isp2 0000:00:03.0: Entity type for entity tpg_subdev was not initialized! atomisp-isp2 0000:00:03.0: Entity type for entity ATOMISP_SUBDEV_0 was not initialized! atomisp-isp2 0000:00:03.0: Entity type for entity ATOMISP_SUBDEV_1 was not initialized! Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/atomisp_csi2.c | 2 +- drivers/staging/media/atomisp/pci/atomisp_subdev.c | 2 +- drivers/staging/media/atomisp/pci/atomisp_tpg.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/atomisp_csi2.c b/drivers/staging/media/atomisp/pci/atomisp_csi2.c index 7853a23b9f53..b00bc0b7aaad 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_csi2.c +++ b/drivers/staging/media/atomisp/pci/atomisp_csi2.c @@ -203,7 +203,7 @@ static int mipi_csi2_init_entities(struct atomisp_mipi_csi2_device *csi2, pads[CSI2_PAD_SINK].flags = MEDIA_PAD_FL_SINK; me->ops = &csi2_media_ops; - me->function = MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN; + me->function = MEDIA_ENT_F_VID_IF_BRIDGE; ret = media_entity_pads_init(me, CSI2_PADS_NUM, pads); if (ret < 0) return ret; diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c index 52d1936b8c87..9cfb85c61db6 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c +++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c @@ -1017,7 +1017,7 @@ static int isp_subdev_init_entities(struct atomisp_sub_device *asd) MEDIA_BUS_FMT_SBGGR10_1X10; me->ops = &isp_subdev_media_ops; - me->function = MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN; + me->function = MEDIA_ENT_F_PROC_VIDEO_ISP; ret = media_entity_pads_init(me, ATOMISP_SUBDEV_PADS_NUM, pads); if (ret < 0) return ret; diff --git a/drivers/staging/media/atomisp/pci/atomisp_tpg.c b/drivers/staging/media/atomisp/pci/atomisp_tpg.c index e29a96da5f98..074826a5b706 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_tpg.c +++ b/drivers/staging/media/atomisp/pci/atomisp_tpg.c @@ -152,7 +152,7 @@ int atomisp_tpg_init(struct atomisp_device *isp) v4l2_set_subdevdata(sd, tpg); pads[0].flags = MEDIA_PAD_FL_SINK; - me->function = MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN; + me->function = MEDIA_ENT_F_PROC_VIDEO_ISP; ret = media_entity_pads_init(me, 1, pads); if (ret < 0) -- cgit v1.2.3 From 2e82f054b58546efd062595dcfa448ee3d048273 Mon Sep 17 00:00:00 2001 From: Brent Pappas Date: Wed, 18 Jan 2023 17:07:39 +0100 Subject: media: atomisp: pci: Replace bytes macros with functions Replace the function-like macros FPNTBL_BYTES(), SCTBL_BYTES(), and MORPH_PLANE_BYTES() with functions to comply with Linux coding style standards. Replace multiplication with calls to array_size() and array3_size() to prevent accidental arithmetic overflow. Link: https://lore.kernel.org/r/20230118160739.26059-1-bpappas@pappasbrent.com Signed-off-by: Brent Pappas Reviewed-by: Andy Shevchenko Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/sh_css_params.c | 38 ++++++++++++++--------- 1 file changed, 23 insertions(+), 15 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c index f08564f58242..588f2adab058 100644 --- a/drivers/staging/media/atomisp/pci/sh_css_params.c +++ b/drivers/staging/media/atomisp/pci/sh_css_params.c @@ -98,17 +98,27 @@ #include "sh_css_frac.h" #include "ia_css_bufq.h" -#define FPNTBL_BYTES(binary) \ - (sizeof(char) * (binary)->in_frame_info.res.height * \ - (binary)->in_frame_info.padded_width) +static size_t fpntbl_bytes(const struct ia_css_binary *binary) +{ + return array3_size(sizeof(char), + binary->in_frame_info.res.height, + binary->in_frame_info.padded_width); +} -#define SCTBL_BYTES(binary) \ - (sizeof(unsigned short) * (binary)->sctbl_height * \ - (binary)->sctbl_aligned_width_per_color * IA_CSS_SC_NUM_COLORS) +static size_t sctbl_bytes(const struct ia_css_binary *binary) +{ + return size_mul(sizeof(unsigned short), + array3_size(binary->sctbl_height, + binary->sctbl_aligned_width_per_color, + IA_CSS_SC_NUM_COLORS)); +} -#define MORPH_PLANE_BYTES(binary) \ - (SH_CSS_MORPH_TABLE_ELEM_BYTES * (binary)->morph_tbl_aligned_width * \ - (binary)->morph_tbl_height) +static size_t morph_plane_bytes(const struct ia_css_binary *binary) +{ + return array3_size(SH_CSS_MORPH_TABLE_ELEM_BYTES, + binary->morph_tbl_aligned_width, + binary->morph_tbl_height); +} /* We keep a second copy of the ptr struct for the SP to access. Again, this would not be necessary on the chip. */ @@ -3279,7 +3289,7 @@ sh_css_params_write_to_ddr_internal( if (binary->info->sp.enable.fpnr) { buff_realloced = reallocate_buffer(&ddr_map->fpn_tbl, &ddr_map_size->fpn_tbl, - (size_t)(FPNTBL_BYTES(binary)), + fpntbl_bytes(binary), params->config_changed[IA_CSS_FPN_ID], &err); if (err) { @@ -3304,7 +3314,7 @@ sh_css_params_write_to_ddr_internal( buff_realloced = reallocate_buffer(&ddr_map->sc_tbl, &ddr_map_size->sc_tbl, - SCTBL_BYTES(binary), + sctbl_bytes(binary), params->sc_table_changed, &err); if (err) { @@ -3538,8 +3548,7 @@ sh_css_params_write_to_ddr_internal( buff_realloced |= reallocate_buffer(virt_addr_tetra_x[i], virt_size_tetra_x[i], - (size_t) - (MORPH_PLANE_BYTES(binary)), + morph_plane_bytes(binary), params->morph_table_changed, &err); if (err) { @@ -3549,8 +3558,7 @@ sh_css_params_write_to_ddr_internal( buff_realloced |= reallocate_buffer(virt_addr_tetra_y[i], virt_size_tetra_y[i], - (size_t) - (MORPH_PLANE_BYTES(binary)), + morph_plane_bytes(binary), params->morph_table_changed, &err); if (err) { -- cgit v1.2.3 From 197ec0f48d7a3aff984c2502af315e5d3e48eedb Mon Sep 17 00:00:00 2001 From: Brent Pappas Date: Fri, 20 Jan 2023 19:26:25 +0100 Subject: media: atomisp: pci: hive_isp_css_common: host: vmem: Replace SUBWORD macros with functions Replace the macros SUBWORD() and INV_SUBWORD() with functions to comply with Linux coding style standards. Link: https://lore.kernel.org/r/20230120182625.23227-1-bpappas@pappasbrent.com Signed-off-by: Brent Pappas Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- .../atomisp/pci/hive_isp_css_common/host/vmem.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/vmem.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/vmem.c index 6620f091442f..d9cdfbc50197 100644 --- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/vmem.c +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/vmem.c @@ -28,10 +28,18 @@ typedef hive_uedge *hive_wide; /* Copied from SDK: sim_semantics.c */ /* subword bits move like this: MSB[____xxxx____]LSB -> MSB[00000000xxxx]LSB */ -#define SUBWORD(w, start, end) (((w) & (((1ULL << ((end) - 1)) - 1) << 1 | 1)) >> (start)) +static inline hive_uedge +subword(hive_uedge w, unsigned int start, unsigned int end) +{ + return (w & (((1ULL << (end - 1)) - 1) << 1 | 1)) >> start; +} /* inverse subword bits move like this: MSB[xxxx____xxxx]LSB -> MSB[xxxx0000xxxx]LSB */ -#define INV_SUBWORD(w, start, end) ((w) & (~(((1ULL << ((end) - 1)) - 1) << 1 | 1) | ((1ULL << (start)) - 1))) +static inline hive_uedge +inv_subword(hive_uedge w, unsigned int start, unsigned int end) +{ + return w & (~(((1ULL << (end - 1)) - 1) << 1 | 1) | ((1ULL << start) - 1)); +} #define uedge_bits (8 * sizeof(hive_uedge)) #define move_lower_bits(target, target_bit, src, src_bit) move_subword(target, target_bit, src, 0, src_bit) @@ -50,18 +58,18 @@ move_subword( unsigned int start_bit = target_bit % uedge_bits; unsigned int subword_width = src_end - src_start; - hive_uedge src_subword = SUBWORD(src, src_start, src_end); + hive_uedge src_subword = subword(src, src_start, src_end); if (subword_width + start_bit > uedge_bits) { /* overlap */ hive_uedge old_val1; - hive_uedge old_val0 = INV_SUBWORD(target[start_elem], start_bit, uedge_bits); + hive_uedge old_val0 = inv_subword(target[start_elem], start_bit, uedge_bits); target[start_elem] = old_val0 | (src_subword << start_bit); - old_val1 = INV_SUBWORD(target[start_elem + 1], 0, + old_val1 = inv_subword(target[start_elem + 1], 0, subword_width + start_bit - uedge_bits); target[start_elem + 1] = old_val1 | (src_subword >> (uedge_bits - start_bit)); } else { - hive_uedge old_val = INV_SUBWORD(target[start_elem], start_bit, + hive_uedge old_val = inv_subword(target[start_elem], start_bit, start_bit + subword_width); target[start_elem] = old_val | (src_subword << start_bit); -- cgit v1.2.3 From 738dfb32f1305478c2c1b87e997142cefad4ad7e Mon Sep 17 00:00:00 2001 From: Brent Pappas Date: Fri, 20 Jan 2023 18:14:08 +0100 Subject: media: atomisp: pci: sh_css: Inline single invocation of macro STATS_ENABLED() Inline the single invocation of the macro STATS_ENABLED(). The macro abstraction is not necessary because the logic behind it is only used once. Link: https://lore.kernel.org/r/20230120171408.16099-1-bpappas@pappasbrent.com Signed-off-by: Brent Pappas Reviewed-by: Andy Shevchenko Reviewed-by: Dan Carpenter Signed-off-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/pci/sh_css.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/staging/media/atomisp/pci') diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c index 726cb7aa4ecd..93789500416f 100644 --- a/drivers/staging/media/atomisp/pci/sh_css.c +++ b/drivers/staging/media/atomisp/pci/sh_css.c @@ -97,9 +97,6 @@ */ #define JPEG_BYTES (16 * 1024 * 1024) -#define STATS_ENABLED(stage) (stage && stage->binary && stage->binary->info && \ - (stage->binary->info->sp.enable.s3a || stage->binary->info->sp.enable.dis)) - struct sh_css my_css; int __printf(1, 0) (*sh_css_printf)(const char *fmt, va_list args) = NULL; @@ -3743,7 +3740,9 @@ ia_css_pipe_enqueue_buffer(struct ia_css_pipe *pipe, * The SP will read the params after it got * empty 3a and dis */ - if (STATS_ENABLED(stage)) { + if (stage->binary && stage->binary->info && + (stage->binary->info->sp.enable.s3a || + stage->binary->info->sp.enable.dis)) { /* there is a stage that needs it */ return_err = ia_css_bufq_enqueue_buffer(thread_id, queue_id, -- cgit v1.2.3