Skip to content

Commit

Permalink
Merge pull request #247 from wentongwu/more_vsc
Browse files Browse the repository at this point in the history
patch: v6.8: backport more ivsc patches for MTL
  • Loading branch information
hao-yao authored Jun 24, 2024
2 parents d7bbd9d + 22714ed commit aecec2a
Show file tree
Hide file tree
Showing 5 changed files with 283 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
From 9b5e045029d8bded4c6979874ed3abc347c1415c Mon Sep 17 00:00:00 2001
From: Wentong Wu <[email protected]>
Date: Mon, 27 May 2024 20:38:35 +0800
Subject: [PATCH] mei: vsc: Don't stop/restart mei device during system
suspend/resume

The dynamically created mei client device (mei csi) is used as one V4L2
sub device of the whole video pipeline, and the V4L2 connection graph is
built by software node. The mei_stop() and mei_restart() will delete the
old mei csi client device and create a new mei client device, which will
cause the software node information saved in old mei csi device lost and
the whole video pipeline will be broken.

Removing mei_stop()/mei_restart() during system suspend/resume can fix
the issue above and won't impact hardware actual power saving logic.

Fixes: f6085a96c973 ("mei: vsc: Unregister interrupt handler for system suspend")
Cc: [email protected] # for 6.8+
Reported-by: Hao Yao <[email protected]>
Signed-off-by: Wentong Wu <[email protected]>
Reviewed-by: Sakari Ailus <[email protected]>
Tested-by: Jason Chen <[email protected]>
Tested-by: Sakari Ailus <[email protected]>
Acked-by: Tomas Winkler <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/misc/mei/platform-vsc.c | 39 +++++++++++++--------------------
1 file changed, 15 insertions(+), 24 deletions(-)

diff --git a/drivers/misc/mei/platform-vsc.c b/drivers/misc/mei/platform-vsc.c
index b543e6b9f3cf..1ec65d87488a 100644
--- a/drivers/misc/mei/platform-vsc.c
+++ b/drivers/misc/mei/platform-vsc.c
@@ -399,41 +399,32 @@ static void mei_vsc_remove(struct platform_device *pdev)

static int mei_vsc_suspend(struct device *dev)
{
- struct mei_device *mei_dev = dev_get_drvdata(dev);
- struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
+ struct mei_device *mei_dev;
+ int ret = 0;

- mei_stop(mei_dev);
+ mei_dev = dev_get_drvdata(dev);
+ if (!mei_dev)
+ return -ENODEV;

- mei_disable_interrupts(mei_dev);
+ mutex_lock(&mei_dev->device_lock);

- vsc_tp_free_irq(hw->tp);
+ if (!mei_write_is_idle(mei_dev))
+ ret = -EAGAIN;

- return 0;
+ mutex_unlock(&mei_dev->device_lock);
+
+ return ret;
}

static int mei_vsc_resume(struct device *dev)
{
- struct mei_device *mei_dev = dev_get_drvdata(dev);
- struct mei_vsc_hw *hw = mei_dev_to_vsc_hw(mei_dev);
- int ret;
-
- ret = vsc_tp_request_irq(hw->tp);
- if (ret)
- return ret;
-
- ret = mei_restart(mei_dev);
- if (ret)
- goto err_free;
+ struct mei_device *mei_dev;

- /* start timer if stopped in suspend */
- schedule_delayed_work(&mei_dev->timer_work, HZ);
+ mei_dev = dev_get_drvdata(dev);
+ if (!mei_dev)
+ return -ENODEV;

return 0;
-
-err_free:
- vsc_tp_free_irq(hw->tp);
-
- return ret;
}

static DEFINE_SIMPLE_DEV_PM_OPS(mei_vsc_pm_ops, mei_vsc_suspend, mei_vsc_resume);
--
2.34.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
From 8790622ea947b245bf00b00cc05784110355ecbb Mon Sep 17 00:00:00 2001
From: Wentong Wu <[email protected]>
Date: Sun, 23 Jun 2024 11:20:41 +0800
Subject: [PATCH] mei: vsc: Prevent timeout error with added delay
post-firmware download

After completing the firmware download, the firmware requires some
time to become functional. This change introduces additional sleep
time before the first read operation to prevent a confusing timeout
error in vsc_tp_xfer().

Fixes: 566f5ca97680 ("mei: Add transport driver for IVSC device")
Cc: [email protected] # for 6.8+
Signed-off-by: Wentong Wu <[email protected]>
Tested-by: Jason Chen <[email protected]>
---
drivers/misc/mei/platform-vsc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/mei/platform-vsc.c b/drivers/misc/mei/platform-vsc.c
index 1ec65d87488a..d02f6e881139 100644
--- a/drivers/misc/mei/platform-vsc.c
+++ b/drivers/misc/mei/platform-vsc.c
@@ -28,8 +28,8 @@

#define MEI_VSC_MAX_MSG_SIZE 512

-#define MEI_VSC_POLL_DELAY_US (50 * USEC_PER_MSEC)
-#define MEI_VSC_POLL_TIMEOUT_US (200 * USEC_PER_MSEC)
+#define MEI_VSC_POLL_DELAY_US (100 * USEC_PER_MSEC)
+#define MEI_VSC_POLL_TIMEOUT_US (400 * USEC_PER_MSEC)

#define mei_dev_to_vsc_hw(dev) ((struct mei_vsc_hw *)((dev)->hw))

--
2.34.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
From 6bc52e063e3478cf5dd538b3dc0a163111957b07 Mon Sep 17 00:00:00 2001
From: Wentong Wu <[email protected]>
Date: Mon, 3 Jun 2024 09:44:06 +0800
Subject: [PATCH 1/3] media: ivsc: csi: don't count privacy on as error

Prior to the ongoing command privacy is on, it would return -1 to
indicate the current privacy status, and the ongoing command would
be well executed by firmware as well, so this is not error. This
patch changes its behavior to notify privacy on directly by V4L2
privacy control instead of reporting error.

Fixes: 29006e196a56 ("media: pci: intel: ivsc: Add CSI submodule")
Reported-by: Hao Yao <[email protected]>
Signed-off-by: Wentong Wu <[email protected]>
Tested-by: Jason Chen <[email protected]>
---
drivers/media/pci/intel/ivsc/mei_csi.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/intel/ivsc/mei_csi.c b/drivers/media/pci/intel/ivsc/mei_csi.c
index 89b582a221ab..05c0acb78939 100644
--- a/drivers/media/pci/intel/ivsc/mei_csi.c
+++ b/drivers/media/pci/intel/ivsc/mei_csi.c
@@ -190,7 +190,11 @@ static int mei_csi_send(struct mei_csi *csi, u8 *buf, size_t len)

/* command response status */
ret = csi->cmd_response.status;
- if (ret) {
+ if (ret == -1) {
+ /* notify privacy on instead of reporting error */
+ ret = 0;
+ v4l2_ctrl_s_ctrl(csi->privacy_ctrl, 1);
+ } else if (ret) {
ret = -EINVAL;
goto out;
}
--
2.34.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
From 12ce4727869b46e93c8d01064621593cfb240404 Mon Sep 17 00:00:00 2001
From: Wentong Wu <[email protected]>
Date: Mon, 3 Jun 2024 10:33:28 +0800
Subject: [PATCH 2/3] media: ivsc: csi: add separate lock for v4l2 control
handler

There're possibilities that privacy status change notification happens
in the middle of the ongoing mei command which already takes the command
lock, but v4l2_ctrl_s_ctrl() would also need the same lock prior to this
patch, so this may results in circular locking problem. This patch adds
one dedicated lock for v4l2 control handler to avoid described issue.

Reported-by: Hao Yao <[email protected]>
Signed-off-by: Wentong Wu <[email protected]>
Tested-by: Jason Chen <[email protected]>
---
drivers/media/pci/intel/ivsc/mei_csi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/intel/ivsc/mei_csi.c b/drivers/media/pci/intel/ivsc/mei_csi.c
index 05c0acb78939..a46a012b301f 100644
--- a/drivers/media/pci/intel/ivsc/mei_csi.c
+++ b/drivers/media/pci/intel/ivsc/mei_csi.c
@@ -126,6 +126,8 @@ struct mei_csi {
struct v4l2_ctrl_handler ctrl_handler;
struct v4l2_ctrl *freq_ctrl;
struct v4l2_ctrl *privacy_ctrl;
+ /* lock for v4l2 controls */
+ struct mutex ctrl_lock;
unsigned int remote_pad;
/* start streaming or not */
int streaming;
@@ -563,11 +565,13 @@ static int mei_csi_init_controls(struct mei_csi *csi)
u32 max;
int ret;

+ mutex_init(&csi->ctrl_lock);
+
ret = v4l2_ctrl_handler_init(&csi->ctrl_handler, 2);
if (ret)
return ret;

- csi->ctrl_handler.lock = &csi->lock;
+ csi->ctrl_handler.lock = &csi->ctrl_lock;

max = ARRAY_SIZE(link_freq_menu_items) - 1;
csi->freq_ctrl = v4l2_ctrl_new_int_menu(&csi->ctrl_handler,
@@ -756,6 +760,7 @@ static int mei_csi_probe(struct mei_cl_device *cldev,

err_ctrl_handler:
v4l2_ctrl_handler_free(&csi->ctrl_handler);
+ mutex_destroy(&csi->ctrl_lock);
v4l2_async_nf_unregister(&csi->notifier);
v4l2_async_nf_cleanup(&csi->notifier);

@@ -775,6 +780,7 @@ static void mei_csi_remove(struct mei_cl_device *cldev)
v4l2_async_nf_unregister(&csi->notifier);
v4l2_async_nf_cleanup(&csi->notifier);
v4l2_ctrl_handler_free(&csi->ctrl_handler);
+ mutex_destroy(&csi->ctrl_lock);
v4l2_async_unregister_subdev(&csi->subdev);
v4l2_subdev_cleanup(&csi->subdev);
media_entity_cleanup(&csi->subdev.entity);
--
2.34.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
From 5611cf035ba882092d9c708420910153f332902b Mon Sep 17 00:00:00 2001
From: Wentong Wu <[email protected]>
Date: Mon, 3 Jun 2024 10:57:24 +0800
Subject: [PATCH 3/3] media: ivsc: csi: remove privacy status in struct mei_csi

The privacy status is maintained by privacy_ctrl, on which all
of the privacy status changes will go through, so there is no
point in maintaining one more element any more.

Reported-by: Hao Yao <[email protected]>
Signed-off-by: Wentong Wu <[email protected]>
Tested-by: Jason Chen <[email protected]>
---
drivers/media/pci/intel/ivsc/mei_csi.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/media/pci/intel/ivsc/mei_csi.c b/drivers/media/pci/intel/ivsc/mei_csi.c
index a46a012b301f..bff0176fc8f1 100644
--- a/drivers/media/pci/intel/ivsc/mei_csi.c
+++ b/drivers/media/pci/intel/ivsc/mei_csi.c
@@ -138,9 +138,6 @@ struct mei_csi {
u32 nr_of_lanes;
/* frequency of the CSI-2 link */
u64 link_freq;
-
- /* privacy status */
- enum ivsc_privacy_status status;
};

static const struct v4l2_mbus_framefmt mei_csi_format_mbus_default = {
@@ -271,10 +268,9 @@ static void mei_csi_rx(struct mei_cl_device *cldev)

switch (notif.cmd_id) {
case CSI_PRIVACY_NOTIF:
- if (notif.cont.cont < CSI_PRIVACY_MAX) {
- csi->status = notif.cont.cont;
- v4l2_ctrl_s_ctrl(csi->privacy_ctrl, csi->status);
- }
+ if (notif.cont.cont < CSI_PRIVACY_MAX)
+ v4l2_ctrl_s_ctrl(csi->privacy_ctrl,
+ notif.cont.cont == CSI_PRIVACY_ON);
break;
case CSI_SET_OWNER:
case CSI_SET_CONF:
--
2.34.1

0 comments on commit aecec2a

Please sign in to comment.