[PATCH 4/5] drm/amd/display: Commit panel_power_savings writes from the kernel

[Hugo Sales <hugo@hsal.es>]

panel_power_savings_store() currently writes the level into
connector->state and sends a hotplug uevent, and the hardware only gets it
on a later atomic commit that includes the connector. Hyprland adds the
connector to a commit only on a modeset or when a connector property it
sets changes, so it never applies the level

Programs that write the sysfs file, such as power-profiles-daemon, aren't
the DRM master and can't commit themselves, so they depend on the master
taking the uevent as a cue to commit the connector, but nothing documents
they should. A master that wants to control ABM sets the 'adaptive
backlight modulation' property, and from then on writes return -EBUSY. The
file has to work without the master's help, like the backlight device,
whose writes reach DC without a commit

Commit the level from the store instead, with a duplicated connector state
and no modeset. The write returns once the level is applied, or with the
commit's error, so no uevent is needed. A connector that is off gets the
level when it is next enabled

Document the new behaviour in the kernel-doc

Signed-off-by: Hugo Sales <hugo@hsal.es>
---
This needs "drm/amd/display: Don't force a modeset when the ABM level
changes". Without it, atomic check sees the changed level, asks for a
modeset and the commit fails with -EINVAL

A failed write used to leave the level stored and send the uevent anyway.
Now the level only changes through the commit, so the store reports the
error. power-profiles-daemon logs a failed write and tries again on the
next profile, AC or battery change, because it compares against the value
it reads back. It ignores the hotplug uevent. Compositors used it only as
a cue to commit the connector, which the store now does itself

The commit is blocking, so a write can hold the CRTC locks for up to a
frame while a compositor commit waits. The other way round, while the
store's commit is pending, a nonblocking commit from the compositor on the
same CRTC fails with -EBUSY, as it would behind any pending commit

The store takes a runtime PM reference around the commit, like the
atomic ioctl path, and its locks are interruptible
---
 .../amd/display/amdgpu_dm/amdgpu_dm_backlight.c    | 90 +++++++++++++++++++---
 1 file changed, 79 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c
index 27546febf90d3..a661fd558686f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_backlight.c
@@ -40,7 +40,10 @@
 
 #include <linux/backlight.h>
 #include <linux/minmax.h>
+#include <linux/pm_runtime.h>
 #include <linux/power_supply.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_utils.h>
 
@@ -691,6 +694,18 @@ EXPORT_IF_KUNIT(amdgpu_dm_setup_backlight_device);
  *
  * Modifying this value can have implications on color accuracy, so tread
  * carefully.
+ *
+ * A write commits the new level before it returns, without a modeset. The
+ * commit waits for any commit already pending on the connector's CRTC, and
+ * while it is pending, a nonblocking commit from the DRM master on that CRTC
+ * fails with -EBUSY. If the connector or its CRTC is off, the driver stores
+ * the level and applies it when the connector is next enabled
+ *
+ * If the commit fails, the write returns its error and the level stays as it
+ * was. While the CRTC needs a modeset, as after a DSC override in debugfs,
+ * writes fail with -EINVAL until a commit that allows a modeset clears that
+ * state. Writes fail with -EBUSY once the DRM master has set the connector's
+ * 'adaptive backlight modulation' property to a value other than 'sysfs'
  */
 
 STATIC_IFN_KUNIT
@@ -712,13 +727,75 @@ ssize_t panel_power_savings_show(struct device *device,
 }
 EXPORT_IF_KUNIT(panel_power_savings_show);
 
+static int panel_power_savings_apply(struct drm_connector *connector,
+				     unsigned int level)
+{
+	struct drm_connector_state *conn_state;
+	struct dm_connector_state *dm_state;
+	struct drm_device *dev = connector->dev;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_atomic_commit *state;
+	int ret, idx;
+
+	if (!drm_dev_enter(dev, &idx))
+		return -ENODEV;
+
+	ret = pm_runtime_resume_and_get(dev->dev);
+	if (ret < 0)
+		goto dev_exit;
+
+	state = drm_atomic_commit_alloc(dev);
+	if (!state) {
+		ret = -ENOMEM;
+		goto pm_put;
+	}
+
+	/* Kernel commits allow a modeset by default, which a level never needs */
+	state->allow_modeset = false;
+
+	drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
+	state->acquire_ctx = &ctx;
+retry:
+	conn_state = drm_atomic_get_connector_state(state, connector);
+	if (IS_ERR(conn_state)) {
+		ret = PTR_ERR(conn_state);
+		goto out;
+	}
+
+	dm_state = to_dm_connector_state(conn_state);
+	if (dm_state->abm_sysfs_forbidden) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	dm_state->abm_level = level ?: ABM_LEVEL_IMMEDIATE_DISABLE;
+
+	ret = drm_atomic_commit(state);
+out:
+	if (ret == -EDEADLK) {
+		drm_atomic_commit_clear(state);
+		ret = drm_modeset_backoff(&ctx);
+		if (!ret)
+			goto retry;
+	}
+
+	drm_atomic_commit_put(state);
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+pm_put:
+	pm_runtime_put_autosuspend(dev->dev);
+dev_exit:
+	drm_dev_exit(idx);
+
+	return ret;
+}
+
 STATIC_IFN_KUNIT
 ssize_t panel_power_savings_store(struct device *device,
 				 struct device_attribute *attr,
 				 const char *buf, size_t count)
 {
 	struct drm_connector *connector = dev_get_drvdata(device);
-	struct drm_device *dev = connector->dev;
 	long val;
 	int ret;
 
@@ -730,19 +807,10 @@ ssize_t panel_power_savings_store(struct device *device,
 	if (val < 0 || val > 4)
 		return -EINVAL;
 
-	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
-	if (to_dm_connector_state(connector->state)->abm_sysfs_forbidden)
-		ret = -EBUSY;
-	else
-		to_dm_connector_state(connector->state)->abm_level = val ?:
-			ABM_LEVEL_IMMEDIATE_DISABLE;
-	drm_modeset_unlock(&dev->mode_config.connection_mutex);
-
+	ret = panel_power_savings_apply(connector, val);
 	if (ret)
 		return ret;
 
-	drm_kms_helper_hotplug_event(dev);
-
 	return count;
 }
 EXPORT_IF_KUNIT(panel_power_savings_store);

-- 
2.56.0