Add the necessary definitions to the qcom-cpufreq-nvmem driver to
support basic cpufreq scaling on the Qualcomm MSM8909 SoC. In practice
the necessary power domains vary depending on the actual PMIC the SoC
was combined with. With PM8909 the VDD_APC power domain is shared with
VDD_CX so the RPM firmware handles all voltage adjustments, while with
PM8916 and PM660 Linux is responsible to do adaptive voltage scaling
of a dedicated CPU regulator using CPR.
Signed-off-by: Stephan Gerhold <stephan.gerhold(a)kernkonzept.com>
---
Stephan Gerhold (4):
cpufreq: qcom-nvmem: Enable virtual power domain devices
cpufreq: dt: platdev: Add MSM8909 to blocklist
dt-bindings: cpufreq: qcom-nvmem: Document MSM8909
cpufreq: qcom-nvmem: Add MSM8909
.../bindings/cpufreq/qcom-cpufreq-nvmem.yaml | 1 +
drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
drivers/cpufreq/qcom-cpufreq-nvmem.c | 47 +++++++++++++++++++++-
3 files changed, 48 insertions(+), 1 deletion(-)
---
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
change-id: 20230906-msm8909-cpufreq-dff238de9ff3
Best regards,
--
Stephan Gerhold <stephan.gerhold(a)kernkonzept.com>
Kernkonzept GmbH at Dresden, Germany, HRB 31129, CEO Dr.-Ing. Michael Hohmuth
This is an automatic generated email to let you know that the following patch were queued:
Subject: media: imx355: Enable runtime PM before registering async sub-device
Author: Bingbu Cao <bingbu.cao(a)intel.com>
Date: Wed Nov 22 17:46:06 2023 +0800
As the sensor device maybe accessible right after its async sub-device is
registered, such as ipu-bridge will try to power up sensor by sensor's
client device's runtime PM from the async notifier callback, if runtime PM
is not enabled, it will fail.
So runtime PM should be ready before its async sub-device is registered
and accessible by others.
Fixes: df0b5c4a7ddd ("media: add imx355 camera sensor driver")
Cc: stable(a)vger.kernel.org
Signed-off-by: Bingbu Cao <bingbu.cao(a)intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus(a)linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
drivers/media/i2c/imx355.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
---
diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
index e1b1d2fc79dd..8c995c58743a 100644
--- a/drivers/media/i2c/imx355.c
+++ b/drivers/media/i2c/imx355.c
@@ -1747,10 +1747,6 @@ static int imx355_probe(struct i2c_client *client)
goto error_handler_free;
}
- ret = v4l2_async_register_subdev_sensor(&imx355->sd);
- if (ret < 0)
- goto error_media_entity;
-
/*
* Device is already turned on by i2c-core with ACPI domain PM.
* Enable runtime PM and turn off the device.
@@ -1759,9 +1755,15 @@ static int imx355_probe(struct i2c_client *client)
pm_runtime_enable(&client->dev);
pm_runtime_idle(&client->dev);
+ ret = v4l2_async_register_subdev_sensor(&imx355->sd);
+ if (ret < 0)
+ goto error_media_entity_runtime_pm;
+
return 0;
-error_media_entity:
+error_media_entity_runtime_pm:
+ pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
media_entity_cleanup(&imx355->sd.entity);
error_handler_free:
This is an automatic generated email to let you know that the following patch were queued:
Subject: media: ov01a10: Enable runtime PM before registering async sub-device
Author: Bingbu Cao <bingbu.cao(a)intel.com>
Date: Wed Nov 22 17:46:07 2023 +0800
As the sensor device maybe accessible right after its async sub-device is
registered, such as ipu-bridge will try to power up sensor by sensor's
client device's runtime PM from the async notifier callback, if runtime PM
is not enabled, it will fail.
So runtime PM should be ready before its async sub-device is registered
and accessible by others.
It also sets the runtime PM status to active as the sensor was turned
on by i2c-core.
Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver")
Cc: stable(a)vger.kernel.org
Signed-off-by: Bingbu Cao <bingbu.cao(a)intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus(a)linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
drivers/media/i2c/ov01a10.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
---
diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 7cca9294ea31..5606437f37d0 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -862,6 +862,7 @@ static void ov01a10_remove(struct i2c_client *client)
v4l2_ctrl_handler_free(sd->ctrl_handler);
pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
}
static int ov01a10_probe(struct i2c_client *client)
@@ -909,17 +910,26 @@ static int ov01a10_probe(struct i2c_client *client)
goto err_media_entity_cleanup;
}
+ /*
+ * Device is already turned on by i2c-core with ACPI domain PM.
+ * Enable runtime PM and turn off the device.
+ */
+ pm_runtime_set_active(&client->dev);
+ pm_runtime_enable(dev);
+ pm_runtime_idle(dev);
+
ret = v4l2_async_register_subdev_sensor(&ov01a10->sd);
if (ret < 0) {
dev_err(dev, "Failed to register subdev: %d\n", ret);
- goto err_media_entity_cleanup;
+ goto err_pm_disable;
}
- pm_runtime_enable(dev);
- pm_runtime_idle(dev);
-
return 0;
+err_pm_disable:
+ pm_runtime_disable(dev);
+ pm_runtime_set_suspended(&client->dev);
+
err_media_entity_cleanup:
media_entity_cleanup(&ov01a10->sd.entity);
This is an automatic generated email to let you know that the following patch were queued:
Subject: media: ov13b10: Enable runtime PM before registering async sub-device
Author: Bingbu Cao <bingbu.cao(a)intel.com>
Date: Wed Nov 22 17:46:08 2023 +0800
As the sensor device maybe accessible right after its async sub-device is
registered, such as ipu-bridge will try to power up sensor by sensor's
client device's runtime PM from the async notifier callback, if runtime PM
is not enabled, it will fail.
So runtime PM should be ready before its async sub-device is registered
and accessible by others.
Fixes: 7ee850546822 ("media: Add sensor driver support for the ov13b10 camera.")
Cc: stable(a)vger.kernel.org
Signed-off-by: Bingbu Cao <bingbu.cao(a)intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus(a)linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
drivers/media/i2c/ov13b10.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
---
diff --git a/drivers/media/i2c/ov13b10.c b/drivers/media/i2c/ov13b10.c
index c06411d5ee2b..73c844aa5697 100644
--- a/drivers/media/i2c/ov13b10.c
+++ b/drivers/media/i2c/ov13b10.c
@@ -1554,24 +1554,27 @@ static int ov13b10_probe(struct i2c_client *client)
goto error_handler_free;
}
- ret = v4l2_async_register_subdev_sensor(&ov13b->sd);
- if (ret < 0)
- goto error_media_entity;
/*
* Device is already turned on by i2c-core with ACPI domain PM.
* Enable runtime PM and turn off the device.
*/
-
/* Set the device's state to active if it's in D0 state. */
if (full_power)
pm_runtime_set_active(&client->dev);
pm_runtime_enable(&client->dev);
pm_runtime_idle(&client->dev);
+ ret = v4l2_async_register_subdev_sensor(&ov13b->sd);
+ if (ret < 0)
+ goto error_media_entity_runtime_pm;
+
return 0;
-error_media_entity:
+error_media_entity_runtime_pm:
+ pm_runtime_disable(&client->dev);
+ if (full_power)
+ pm_runtime_set_suspended(&client->dev);
media_entity_cleanup(&ov13b->sd.entity);
error_handler_free:
@@ -1594,6 +1597,7 @@ static void ov13b10_remove(struct i2c_client *client)
ov13b10_free_controls(ov13b);
pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
}
static DEFINE_RUNTIME_DEV_PM_OPS(ov13b10_pm_ops, ov13b10_suspend,
This is an automatic generated email to let you know that the following patch were queued:
Subject: media: ov9734: Enable runtime PM before registering async sub-device
Author: Bingbu Cao <bingbu.cao(a)intel.com>
Date: Wed Nov 22 17:46:09 2023 +0800
As the sensor device maybe accessible right after its async sub-device is
registered, such as ipu-bridge will try to power up sensor by sensor's
client device's runtime PM from the async notifier callback, if runtime PM
is not enabled, it will fail.
So runtime PM should be ready before its async sub-device is registered
and accessible by others.
Fixes: d3f863a63fe4 ("media: i2c: Add ov9734 image sensor driver")
Cc: stable(a)vger.kernel.org
Signed-off-by: Bingbu Cao <bingbu.cao(a)intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus(a)linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
drivers/media/i2c/ov9734.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
---
diff --git a/drivers/media/i2c/ov9734.c b/drivers/media/i2c/ov9734.c
index 2b766bfc98fc..d99728597431 100644
--- a/drivers/media/i2c/ov9734.c
+++ b/drivers/media/i2c/ov9734.c
@@ -893,6 +893,7 @@ static void ov9734_remove(struct i2c_client *client)
media_entity_cleanup(&sd->entity);
v4l2_ctrl_handler_free(sd->ctrl_handler);
pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
mutex_destroy(&ov9734->mutex);
}
@@ -938,13 +939,6 @@ static int ov9734_probe(struct i2c_client *client)
goto probe_error_v4l2_ctrl_handler_free;
}
- ret = v4l2_async_register_subdev_sensor(&ov9734->sd);
- if (ret < 0) {
- dev_err(&client->dev, "failed to register V4L2 subdev: %d",
- ret);
- goto probe_error_media_entity_cleanup;
- }
-
/*
* Device is already turned on by i2c-core with ACPI domain PM.
* Enable runtime PM and turn off the device.
@@ -953,9 +947,18 @@ static int ov9734_probe(struct i2c_client *client)
pm_runtime_enable(&client->dev);
pm_runtime_idle(&client->dev);
+ ret = v4l2_async_register_subdev_sensor(&ov9734->sd);
+ if (ret < 0) {
+ dev_err(&client->dev, "failed to register V4L2 subdev: %d",
+ ret);
+ goto probe_error_media_entity_cleanup_pm;
+ }
+
return 0;
-probe_error_media_entity_cleanup:
+probe_error_media_entity_cleanup_pm:
+ pm_runtime_disable(&client->dev);
+ pm_runtime_set_suspended(&client->dev);
media_entity_cleanup(&ov9734->sd.entity);
probe_error_v4l2_ctrl_handler_free:
The driver always registers pin configurations in device tree. This can
cause some inconvenience to users, as pin configurations in the base
device tree cannot be disabled in the device tree overlay, even when the
relevant devices are not used.
Ignore disabled pin configuration nodes in device tree.
Fixes: 447976ab62c5 ("pinctrl: starfive: Add StarFive JH7110 sys controller driver")
Cc: stable(a)vger.kernel.org
Signed-off-by: Nam Cao <namcao(a)linutronix.de>
---
drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
index 640f827a9b2c..b4f799572689 100644
--- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
+++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
@@ -135,7 +135,7 @@ static int jh7110_dt_node_to_map(struct pinctrl_dev *pctldev,
int ret;
ngroups = 0;
- for_each_child_of_node(np, child)
+ for_each_available_child_of_node(np, child)
ngroups += 1;
nmaps = 2 * ngroups;
@@ -150,7 +150,7 @@ static int jh7110_dt_node_to_map(struct pinctrl_dev *pctldev,
nmaps = 0;
ngroups = 0;
mutex_lock(&sfp->mutex);
- for_each_child_of_node(np, child) {
+ for_each_available_child_of_node(np, child) {
int npins = of_property_count_u32_elems(child, "pinmux");
int *pins;
u32 *pinmux;
--
2.39.2
This is an automatic generated email to let you know that the following patch were queued:
Subject: media: i2c: st-mipid02: correct format propagation
Author: Alain Volmat <alain.volmat(a)foss.st.com>
Date: Mon Nov 13 15:57:30 2023 +0100
Use a copy of the struct v4l2_subdev_format when propagating
format from the sink to source pad in order to avoid impacting the
sink format returned to the application.
Thanks to Jacopo Mondi for pointing the issue.
Fixes: 6c01e6f3f27b ("media: st-mipid02: Propagate format from sink to source pad")
Signed-off-by: Alain Volmat <alain.volmat(a)foss.st.com>
Cc: stable(a)vger.kernel.org
Reviewed-by: Jacopo Mondi <jacopo.mondi(a)ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally(a)ideasonboard.com>
Reviewed-by: Benjamin Mugnier <benjamin.mugnier(a)foss.st.com>
Signed-off-by: Sakari Ailus <sakari.ailus(a)linux.intel.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco(a)xs4all.nl>
drivers/media/i2c/st-mipid02.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
---
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c
index b08a249b5fdd..914f915749a8 100644
--- a/drivers/media/i2c/st-mipid02.c
+++ b/drivers/media/i2c/st-mipid02.c
@@ -769,6 +769,7 @@ static void mipid02_set_fmt_sink(struct v4l2_subdev *sd,
struct v4l2_subdev_format *format)
{
struct mipid02_dev *bridge = to_mipid02_dev(sd);
+ struct v4l2_subdev_format source_fmt;
struct v4l2_mbus_framefmt *fmt;
format->format.code = get_fmt_code(format->format.code);
@@ -780,8 +781,12 @@ static void mipid02_set_fmt_sink(struct v4l2_subdev *sd,
*fmt = format->format;
- /* Propagate the format change to the source pad */
- mipid02_set_fmt_source(sd, sd_state, format);
+ /*
+ * Propagate the format change to the source pad, taking
+ * care not to update the format pointer given back to user
+ */
+ source_fmt = *format;
+ mipid02_set_fmt_source(sd, sd_state, &source_fmt);
}
static int mipid02_set_fmt(struct v4l2_subdev *sd,
Invoke drm_plane_helper_funcs.end_fb_access before
drm_atomic_helper_commit_hw_done(). The latter function hands over
ownership of the plane state to the following commit, which might
free it. Releasing resources in end_fb_access then operates on undefined
state. This bug has been observed with non-blocking commits when they
are being queued up quickly.
Here is an example stack trace from the bug report. The plane state has
been free'd already, so the pages for drm_gem_fb_vunmap() are gone.
Unable to handle kernel paging request at virtual address 0000000100000049
[...]
drm_gem_fb_vunmap+0x18/0x74
drm_gem_end_shadow_fb_access+0x1c/0x2c
drm_atomic_helper_cleanup_planes+0x58/0xd8
drm_atomic_helper_commit_tail+0x90/0xa0
commit_tail+0x15c/0x188
commit_work+0x14/0x20
Fix this by running end_fb_access immediately after updating all planes
in drm_atomic_helper_commit_planes(). The existing clean-up helper
drm_atomic_helper_cleanup_planes() now only handles cleanup_fb.
For aborted commits, roll back from drm_atomic_helper_prepare_planes()
in the new helper drm_atomic_helper_unprepare_planes(). This case is
different from regular cleanup, as we have to release the new state;
regular cleanup releases the old state. The new helper also invokes
cleanup_fb for all planes.
The changes mostly involve DRM's atomic helpers. Only two drivers, i915
and nouveau, implement their own commit function. Update them to invoke
drm_atomic_helper_unprepare_planes(). Drivers with custom commit_tail
function do not require changes.
v4:
* fix documentation (kernel test robot)
v3:
* add drm_atomic_helper_unprepare_planes() for rolling back
* use correct state for end_fb_access
v2:
* fix test in drm_atomic_helper_cleanup_planes()
Reported-by: Alyssa Ross <hi(a)alyssa.is>
Closes: https://lore.kernel.org/dri-devel/87leazm0ya.fsf@alyssa.is/
Suggested-by: Daniel Vetter <daniel(a)ffwll.ch>
Fixes: 94d879eaf7fb ("drm/atomic-helper: Add {begin,end}_fb_access to plane helpers")
Tested-by: Alyssa Ross <hi(a)alyssa.is>
Reviewed-by: Alyssa Ross <hi(a)alyssa.is>
Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de>
Cc: <stable(a)vger.kernel.org> # v6.2+
---
drivers/gpu/drm/drm_atomic_helper.c | 78 +++++++++++++-------
drivers/gpu/drm/i915/display/intel_display.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
include/drm/drm_atomic_helper.h | 2 +
4 files changed, 56 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index c3f677130def0..a920fbae714c8 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2012,7 +2012,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
return ret;
drm_atomic_helper_async_commit(dev, state);
- drm_atomic_helper_cleanup_planes(dev, state);
+ drm_atomic_helper_unprepare_planes(dev, state);
return 0;
}
@@ -2072,7 +2072,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
return 0;
err:
- drm_atomic_helper_cleanup_planes(dev, state);
+ drm_atomic_helper_unprepare_planes(dev, state);
return ret;
}
EXPORT_SYMBOL(drm_atomic_helper_commit);
@@ -2650,6 +2650,39 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_atomic_helper_prepare_planes);
+/**
+ * drm_atomic_helper_unprepare_planes - release plane resources on aborts
+ * @dev: DRM device
+ * @state: atomic state object with old state structures
+ *
+ * This function cleans up plane state, specifically framebuffers, from the
+ * atomic state. It undoes the effects of drm_atomic_helper_prepare_planes()
+ * when aborting an atomic commit. For cleaning up after a successful commit
+ * use drm_atomic_helper_cleanup_planes().
+ */
+void drm_atomic_helper_unprepare_planes(struct drm_device *dev,
+ struct drm_atomic_state *state)
+{
+ struct drm_plane *plane;
+ struct drm_plane_state *new_plane_state;
+ int i;
+
+ for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+ const struct drm_plane_helper_funcs *funcs = plane->helper_private;
+
+ if (funcs->end_fb_access)
+ funcs->end_fb_access(plane, new_plane_state);
+ }
+
+ for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+ const struct drm_plane_helper_funcs *funcs = plane->helper_private;
+
+ if (funcs->cleanup_fb)
+ funcs->cleanup_fb(plane, new_plane_state);
+ }
+}
+EXPORT_SYMBOL(drm_atomic_helper_unprepare_planes);
+
static bool plane_crtc_active(const struct drm_plane_state *state)
{
return state->crtc && state->crtc->state->active;
@@ -2784,6 +2817,17 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
funcs->atomic_flush(crtc, old_state);
}
+
+ /*
+ * Signal end of framebuffer access here before hw_done. After hw_done,
+ * a later commit might have already released the plane state.
+ */
+ for_each_old_plane_in_state(old_state, plane, old_plane_state, i) {
+ const struct drm_plane_helper_funcs *funcs = plane->helper_private;
+
+ if (funcs->end_fb_access)
+ funcs->end_fb_access(plane, old_plane_state);
+ }
}
EXPORT_SYMBOL(drm_atomic_helper_commit_planes);
@@ -2911,40 +2955,22 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_planes_on_crtc);
* configuration. Hence the old configuration must be perserved in @old_state to
* be able to call this function.
*
- * This function must also be called on the new state when the atomic update
- * fails at any point after calling drm_atomic_helper_prepare_planes().
+ * This function may not be called on the new state when the atomic update
+ * fails at any point after calling drm_atomic_helper_prepare_planes(). Use
+ * drm_atomic_helper_unprepare_planes() in this case.
*/
void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
struct drm_atomic_state *old_state)
{
struct drm_plane *plane;
- struct drm_plane_state *old_plane_state, *new_plane_state;
+ struct drm_plane_state *old_plane_state;
int i;
- for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) {
+ for_each_old_plane_in_state(old_state, plane, old_plane_state, i) {
const struct drm_plane_helper_funcs *funcs = plane->helper_private;
- if (funcs->end_fb_access)
- funcs->end_fb_access(plane, new_plane_state);
- }
-
- for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) {
- const struct drm_plane_helper_funcs *funcs;
- struct drm_plane_state *plane_state;
-
- /*
- * This might be called before swapping when commit is aborted,
- * in which case we have to cleanup the new state.
- */
- if (old_plane_state == plane->state)
- plane_state = new_plane_state;
- else
- plane_state = old_plane_state;
-
- funcs = plane->helper_private;
-
if (funcs->cleanup_fb)
- funcs->cleanup_fb(plane, plane_state);
+ funcs->cleanup_fb(plane, old_plane_state);
}
}
EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 5cf162628b95e..ace834c9e8f9f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7354,7 +7354,7 @@ int intel_atomic_commit(struct drm_device *dev, struct drm_atomic_state *_state,
for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i)
intel_color_cleanup_commit(new_crtc_state);
- drm_atomic_helper_cleanup_planes(dev, &state->base);
+ drm_atomic_helper_unprepare_planes(dev, &state->base);
intel_runtime_pm_put(&dev_priv->runtime_pm, state->wakeref);
return ret;
}
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 11fe75b68e95c..8d37a694b7724 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2476,7 +2476,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
err_cleanup:
if (ret)
- drm_atomic_helper_cleanup_planes(dev, state);
+ drm_atomic_helper_unprepare_planes(dev, state);
done:
pm_runtime_put_autosuspend(dev->dev);
return ret;
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 536a0b0091c3a..006b5c977ad77 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -97,6 +97,8 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
int drm_atomic_helper_prepare_planes(struct drm_device *dev,
struct drm_atomic_state *state);
+void drm_atomic_helper_unprepare_planes(struct drm_device *dev,
+ struct drm_atomic_state *state);
#define DRM_PLANE_COMMIT_ACTIVE_ONLY BIT(0)
#define DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET BIT(1)
--
2.43.0
Invoke drm_plane_helper_funcs.end_fb_access before
drm_atomic_helper_commit_hw_done(). The latter function hands over
ownership of the plane state to the following commit, which might
free it. Releasing resources in end_fb_access then operates on undefined
state. This bug has been observed with non-blocking commits when they
are being queued up quickly.
Here is an example stack trace from the bug report. The plane state has
been free'd already, so the pages for drm_gem_fb_vunmap() are gone.
Unable to handle kernel paging request at virtual address 0000000100000049
[...]
drm_gem_fb_vunmap+0x18/0x74
drm_gem_end_shadow_fb_access+0x1c/0x2c
drm_atomic_helper_cleanup_planes+0x58/0xd8
drm_atomic_helper_commit_tail+0x90/0xa0
commit_tail+0x15c/0x188
commit_work+0x14/0x20
Fix this by running end_fb_access immediately after updating all planes
in drm_atomic_helper_commit_planes(). The existing clean-up helper
drm_atomic_helper_cleanup_planes() now only handles cleanup_fb.
For aborted commits, roll back from drm_atomic_helper_prepare_planes()
in the new helper drm_atomic_helper_unprepare_planes(). This case is
different from regular cleanup, as we have to release the new state;
regular cleanup releases the old state. The new helper also invokes
cleanup_fb for all planes.
The changes mostly involve DRM's atomic helpers. Only two drivers, i915
and nouveau, implement their own commit function. Update them to invoke
drm_atomic_helper_unprepare_planes(). Drivers with custom commit_tail
function do not require changes.
v3:
* add drm_atomic_helper_unprepare_planes() for rolling back
* use correct state for end_fb_access
v2:
* fix test in drm_atomic_helper_cleanup_planes()
Reported-by: Alyssa Ross <hi(a)alyssa.is>
Closes: https://lore.kernel.org/dri-devel/87leazm0ya.fsf@alyssa.is/
Suggested-by: Daniel Vetter <daniel(a)ffwll.ch>
Fixes: 94d879eaf7fb ("drm/atomic-helper: Add {begin,end}_fb_access to plane helpers")
Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de>
Cc: <stable(a)vger.kernel.org> # v6.2+
---
drivers/gpu/drm/drm_atomic_helper.c | 78 +++++++++++++-------
drivers/gpu/drm/i915/display/intel_display.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +-
include/drm/drm_atomic_helper.h | 2 +
4 files changed, 56 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index c3f677130def0..9adec3eb78563 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2012,7 +2012,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
return ret;
drm_atomic_helper_async_commit(dev, state);
- drm_atomic_helper_cleanup_planes(dev, state);
+ drm_atomic_helper_unprepare_planes(dev, state);
return 0;
}
@@ -2072,7 +2072,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
return 0;
err:
- drm_atomic_helper_cleanup_planes(dev, state);
+ drm_atomic_helper_unprepare_planes(dev, state);
return ret;
}
EXPORT_SYMBOL(drm_atomic_helper_commit);
@@ -2650,6 +2650,39 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_atomic_helper_prepare_planes);
+/**
+ * drm_atomic_helper_unprepare_planes - release plane resources on aborts
+ * @dev: DRM device
+ * @old_state: atomic state object with old state structures
+ *
+ * This function cleans up plane state, specifically framebuffers, from the
+ * atomic state. It undoes the effects of drm_atomic_helper_prepare_planes()
+ * when aborting an atomic commit. For cleaning up after a successful commit
+ * use drm_atomic_helper_cleanup_planes().
+ */
+void drm_atomic_helper_unprepare_planes(struct drm_device *dev,
+ struct drm_atomic_state *state)
+{
+ struct drm_plane *plane;
+ struct drm_plane_state *new_plane_state;
+ int i;
+
+ for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+ const struct drm_plane_helper_funcs *funcs = plane->helper_private;
+
+ if (funcs->end_fb_access)
+ funcs->end_fb_access(plane, new_plane_state);
+ }
+
+ for_each_new_plane_in_state(state, plane, new_plane_state, i) {
+ const struct drm_plane_helper_funcs *funcs = plane->helper_private;
+
+ if (funcs->cleanup_fb)
+ funcs->cleanup_fb(plane, new_plane_state);
+ }
+}
+EXPORT_SYMBOL(drm_atomic_helper_unprepare_planes);
+
static bool plane_crtc_active(const struct drm_plane_state *state)
{
return state->crtc && state->crtc->state->active;
@@ -2784,6 +2817,17 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
funcs->atomic_flush(crtc, old_state);
}
+
+ /*
+ * Signal end of framebuffer access here before hw_done. After hw_done,
+ * a later commit might have already released the plane state.
+ */
+ for_each_old_plane_in_state(old_state, plane, old_plane_state, i) {
+ const struct drm_plane_helper_funcs *funcs = plane->helper_private;
+
+ if (funcs->end_fb_access)
+ funcs->end_fb_access(plane, old_plane_state);
+ }
}
EXPORT_SYMBOL(drm_atomic_helper_commit_planes);
@@ -2911,40 +2955,22 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_planes_on_crtc);
* configuration. Hence the old configuration must be perserved in @old_state to
* be able to call this function.
*
- * This function must also be called on the new state when the atomic update
- * fails at any point after calling drm_atomic_helper_prepare_planes().
+ * This function may not be called on the new state when the atomic update
+ * fails at any point after calling drm_atomic_helper_prepare_planes(). Use
+ * drm_atomic_helper_unprepare_planes() in this case.
*/
void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
struct drm_atomic_state *old_state)
{
struct drm_plane *plane;
- struct drm_plane_state *old_plane_state, *new_plane_state;
+ struct drm_plane_state *old_plane_state;
int i;
- for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) {
+ for_each_old_plane_in_state(old_state, plane, old_plane_state, i) {
const struct drm_plane_helper_funcs *funcs = plane->helper_private;
- if (funcs->end_fb_access)
- funcs->end_fb_access(plane, new_plane_state);
- }
-
- for_each_oldnew_plane_in_state(old_state, plane, old_plane_state, new_plane_state, i) {
- const struct drm_plane_helper_funcs *funcs;
- struct drm_plane_state *plane_state;
-
- /*
- * This might be called before swapping when commit is aborted,
- * in which case we have to cleanup the new state.
- */
- if (old_plane_state == plane->state)
- plane_state = new_plane_state;
- else
- plane_state = old_plane_state;
-
- funcs = plane->helper_private;
-
if (funcs->cleanup_fb)
- funcs->cleanup_fb(plane, plane_state);
+ funcs->cleanup_fb(plane, old_plane_state);
}
}
EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 5cf162628b95e..ace834c9e8f9f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7354,7 +7354,7 @@ int intel_atomic_commit(struct drm_device *dev, struct drm_atomic_state *_state,
for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i)
intel_color_cleanup_commit(new_crtc_state);
- drm_atomic_helper_cleanup_planes(dev, &state->base);
+ drm_atomic_helper_unprepare_planes(dev, &state->base);
intel_runtime_pm_put(&dev_priv->runtime_pm, state->wakeref);
return ret;
}
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 11fe75b68e95c..8d37a694b7724 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2476,7 +2476,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
err_cleanup:
if (ret)
- drm_atomic_helper_cleanup_planes(dev, state);
+ drm_atomic_helper_unprepare_planes(dev, state);
done:
pm_runtime_put_autosuspend(dev->dev);
return ret;
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 536a0b0091c3a..006b5c977ad77 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -97,6 +97,8 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
int drm_atomic_helper_prepare_planes(struct drm_device *dev,
struct drm_atomic_state *state);
+void drm_atomic_helper_unprepare_planes(struct drm_device *dev,
+ struct drm_atomic_state *state);
#define DRM_PLANE_COMMIT_ACTIVE_ONLY BIT(0)
#define DRM_PLANE_COMMIT_NO_DISABLE_AFTER_MODESET BIT(1)
--
2.43.0