From: Hugo Sales <hugo@hsal.es>
To: sunpeng.li@amd.com, siqueira@igalia.com,
amd-gfx@lists.freedesktop.org, alexander.deucher@amd.com,
harry.wentland@amd.com
Cc: Hugo Sales <hugo@hsal.es>,
airlied@gmail.com, linux-kernel@vger.kernel.org,
mario.limonciello@amd.com, christian.koenig@amd.com,
simona@ffwll.ch, linux-doc@vger.kernel.org,
bhawanpreet.lakha@amd.com, ivan.lipski@amd.com,
dri-devel@lists.freedesktop.org, ray.wu@amd.com,
alex.hung@amd.com
Subject: [PATCH 5/5] drm/amd/display: Test panel_power_savings commits
Date: Mon, 2 Nov 2026 10:17:58 +0000 [thread overview]
Message-ID: <20261102-abm-sysfs-commit-v1-5-e5addfd9539a@hsal.es> (raw)
In-Reply-To: <20261102-abm-sysfs-commit-v1-0-e5addfd9539a@hsal.es>
Add KUnit cases for a panel_power_savings write to a connector bound to a
CRTC. With the CRTC on or off, the store commits a connector state that
carries the level. When atomic check rejects the commit, the write returns
the error and the connector keeps its old level. The existing store tests
also check that a write replaces the connector state, and that a forbidden
write keeps the state and level and drops its runtime PM reference. For
the new cases, the pipe helper can route the connector to the CRTC, set
whether the CRTC is active and take CRTC helper funcs
Signed-off-by: Hugo Sales <hugo@hsal.es>
---
.../amdgpu_dm/tests/amdgpu_dm_backlight_test.c | 157 ++++++++++++++++++++-
1 file changed, 150 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c
index 35e245b078f7a..d58418c88592c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_backlight_test.c
@@ -10,6 +10,7 @@
#include <linux/pm_runtime.h>
#include <drm/drm_atomic_helper.h>
+#include <drm/drm_atomic_uapi.h>
#include <drm/drm_connector.h>
#include <drm/drm_kunit_helpers.h>
#include <drm/drm_managed.h>
@@ -564,16 +565,43 @@ static const struct drm_connector_helper_funcs dm_test_pps_connector_helper_func
.get_modes = dm_test_pps_get_modes,
};
+static int dm_test_pps_crtc_atomic_check_fail(struct drm_crtc *crtc,
+ struct drm_atomic_commit *state)
+{
+ return -ENOSPC;
+}
+
+static const struct drm_crtc_helper_funcs dm_test_pps_crtc_helper_funcs_fail = {
+ .atomic_check = dm_test_pps_crtc_atomic_check_fail,
+};
+
+/* The generic helpers accept a commit that allows a modeset it doesn't need */
+static int dm_test_pps_crtc_atomic_check_no_modeset(struct drm_crtc *crtc,
+ struct drm_atomic_commit *state)
+{
+ return state->allow_modeset ? -EINVAL : 0;
+}
+
+static const struct drm_crtc_helper_funcs dm_test_pps_crtc_helper_funcs_no_modeset = {
+ .atomic_check = dm_test_pps_crtc_atomic_check_no_modeset,
+};
+
/*
* Build a primary plane, a CRTC, an encoder and an eDP connector that the
- * store can commit to. The KUnit device uses the generic atomic helpers, so a
- * commit runs drm_atomic_helper_check() and drm_atomic_helper_commit() rather
- * than the DM implementations
+ * store can commit to. With @bound, the connector's current state is routed
+ * to the CRTC, which is enabled and has @active as its active state. The KUnit
+ * device uses the generic atomic helpers, so a commit runs
+ * drm_atomic_helper_check() and drm_atomic_helper_commit() rather than the DM
+ * implementations. A NULL @crtc_helper_funcs gives the KUnit helper's defaults
*/
static struct drm_connector *
-setup_panel_power_savings_pipe(struct kunit *test, struct device **device_out)
+setup_panel_power_savings_pipe(struct kunit *test, struct device **device_out,
+ bool bound, bool active,
+ const struct drm_crtc_helper_funcs *crtc_helper_funcs)
{
struct amdgpu_dm_connector *aconnector;
+ struct drm_display_mode *mode;
+ struct drm_crtc_state *crtc_state;
struct drm_connector *connector;
struct drm_encoder *encoder;
struct amdgpu_device *adev;
@@ -589,7 +617,7 @@ setup_panel_power_savings_pipe(struct kunit *test, struct device **device_out)
NULL, NULL, 0, NULL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, primary);
crtc = drm_kunit_helper_create_crtc(test, &adev->ddev, primary, NULL,
- NULL, NULL);
+ NULL, crtc_helper_funcs);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc);
encoder = drmm_kzalloc(&adev->ddev, sizeof(*encoder), GFP_KERNEL);
@@ -609,6 +637,21 @@ setup_panel_power_savings_pipe(struct kunit *test, struct device **device_out)
drm_mode_config_reset(&adev->ddev);
+ if (bound) {
+ /* A connector state bound to a CRTC holds a connector reference */
+ connector->state->crtc = crtc;
+ drm_connector_get(connector);
+ connector->state->best_encoder = encoder;
+ crtc_state = crtc->state;
+ crtc_state->connector_mask = drm_connector_mask(connector);
+ crtc_state->encoder_mask = drm_encoder_mask(encoder);
+ crtc_state->enable = true;
+ crtc_state->active = active;
+ mode = drm_kunit_display_mode_from_cea_vic(test, &adev->ddev, 1);
+ KUNIT_ASSERT_NOT_NULL(test, mode);
+ KUNIT_ASSERT_EQ(test, drm_atomic_set_mode_for_crtc(crtc_state, mode), 0);
+ }
+
device = kunit_kzalloc(test, sizeof(*device), GFP_KERNEL);
KUNIT_ASSERT_NOT_NULL(test, device);
dev_set_drvdata(device, connector);
@@ -617,6 +660,14 @@ setup_panel_power_savings_pipe(struct kunit *test, struct device **device_out)
return connector;
}
+static void dm_test_pps_expect_no_pm_reference(struct kunit *test,
+ struct drm_connector *connector)
+{
+#ifdef CONFIG_PM
+ KUNIT_EXPECT_EQ(test, atomic_read(&connector->dev->dev->power.usage_count), 0);
+#endif
+}
+
static void dm_test_free_sysfs_buf(void *data)
{
free_page((unsigned long)data);
@@ -677,17 +728,24 @@ static void dm_test_panel_power_savings_show_reports_level(struct kunit *test)
/**
* dm_test_panel_power_savings_store_sets_disable - Test zero maps to disable
* @test: The KUnit test context
+ *
+ * With no CRTC on the connector, the store commits a connector state that
+ * carries the disable sentinel
*/
static void dm_test_panel_power_savings_store_sets_disable(struct kunit *test)
{
+ struct drm_connector_state *old_state;
struct drm_connector *connector;
struct device *device;
size_t count = strlen("0");
- connector = setup_panel_power_savings_pipe(test, &device);
+ connector = setup_panel_power_savings_pipe(test, &device, false, false, NULL);
+ to_dm_connector_state(connector->state)->abm_level = 2;
+ old_state = connector->state;
KUNIT_EXPECT_EQ(test, panel_power_savings_store(device, NULL, "0", count),
(ssize_t)count);
+ KUNIT_EXPECT_PTR_NE(test, connector->state, old_state);
KUNIT_EXPECT_EQ(test, to_dm_connector_state(connector->state)->abm_level,
ABM_LEVEL_IMMEDIATE_DISABLE);
}
@@ -695,6 +753,9 @@ static void dm_test_panel_power_savings_store_sets_disable(struct kunit *test)
/**
* dm_test_panel_power_savings_store_forbidden - Test forbidden update
* @test: The KUnit test context
+ *
+ * Once the DRM master owns the level, the store fails without committing and
+ * drops its runtime PM reference
*/
static void dm_test_panel_power_savings_store_forbidden(struct kunit *test)
{
@@ -702,11 +763,15 @@ static void dm_test_panel_power_savings_store_forbidden(struct kunit *test)
struct drm_connector *connector;
struct device *device;
- connector = setup_panel_power_savings_pipe(test, &device);
+ connector = setup_panel_power_savings_pipe(test, &device, false, false, NULL);
dm_state = to_dm_connector_state(connector->state);
dm_state->abm_sysfs_forbidden = true;
+ dm_state->abm_level = 2;
KUNIT_EXPECT_EQ(test, panel_power_savings_store(device, NULL, "1", 1), -EBUSY);
+ KUNIT_EXPECT_PTR_EQ(test, connector->state, &dm_state->base);
+ KUNIT_EXPECT_EQ(test, dm_state->abm_level, 2);
+ dm_test_pps_expect_no_pm_reference(test, connector);
}
/**
@@ -755,6 +820,81 @@ static void dm_test_panel_power_savings_store_rejects_out_of_range(struct kunit
KUNIT_EXPECT_EQ(test, panel_power_savings_store(device, NULL, "5", 1), -EINVAL);
}
+/**
+ * dm_test_panel_power_savings_store_inactive_crtc - Test store with the CRTC off
+ * @test: The KUnit test context
+ *
+ * With the connector's CRTC enabled but inactive, the store still commits a
+ * connector state that carries the level, without allowing a modeset, and
+ * drops its runtime PM reference
+ */
+static void dm_test_panel_power_savings_store_inactive_crtc(struct kunit *test)
+{
+ struct drm_connector_state *old_state;
+ struct drm_connector *connector;
+ struct device *device;
+
+ connector = setup_panel_power_savings_pipe(test, &device, true, false,
+ &dm_test_pps_crtc_helper_funcs_no_modeset);
+ old_state = connector->state;
+
+ KUNIT_EXPECT_EQ(test, panel_power_savings_store(device, NULL, "3", 1), 1);
+ KUNIT_EXPECT_PTR_NE(test, connector->state, old_state);
+ KUNIT_EXPECT_EQ(test, to_dm_connector_state(connector->state)->abm_level, 3);
+ KUNIT_EXPECT_FALSE(test, drm_atomic_crtc_needs_modeset(connector->state->crtc->state));
+ dm_test_pps_expect_no_pm_reference(test, connector);
+}
+
+/**
+ * dm_test_panel_power_savings_store_active_crtc - Test store with the CRTC on
+ * @test: The KUnit test context
+ *
+ * With the connector's CRTC active, the store commits a connector state that
+ * carries the level, without allowing a modeset, and drops its runtime PM
+ * reference
+ */
+static void dm_test_panel_power_savings_store_active_crtc(struct kunit *test)
+{
+ struct drm_connector_state *old_state;
+ struct drm_connector *connector;
+ struct device *device;
+
+ connector = setup_panel_power_savings_pipe(test, &device, true, true,
+ &dm_test_pps_crtc_helper_funcs_no_modeset);
+ old_state = connector->state;
+
+ KUNIT_EXPECT_EQ(test, panel_power_savings_store(device, NULL, "2", 1), 1);
+ KUNIT_EXPECT_PTR_NE(test, connector->state, old_state);
+ KUNIT_EXPECT_EQ(test, to_dm_connector_state(connector->state)->abm_level, 2);
+ KUNIT_EXPECT_FALSE(test, drm_atomic_crtc_needs_modeset(connector->state->crtc->state));
+ dm_test_pps_expect_no_pm_reference(test, connector);
+}
+
+/**
+ * dm_test_panel_power_savings_store_commit_fails - Test store when the commit fails
+ * @test: The KUnit test context
+ *
+ * When atomic check rejects the commit, the write returns the error, the
+ * current connector state keeps the old level and the runtime PM reference is
+ * dropped
+ */
+static void dm_test_panel_power_savings_store_commit_fails(struct kunit *test)
+{
+ struct drm_connector_state *old_state;
+ struct drm_connector *connector;
+ struct device *device;
+
+ connector = setup_panel_power_savings_pipe(test, &device, true, true,
+ &dm_test_pps_crtc_helper_funcs_fail);
+ to_dm_connector_state(connector->state)->abm_level = 1;
+ old_state = connector->state;
+
+ KUNIT_EXPECT_EQ(test, panel_power_savings_store(device, NULL, "4", 1), -ENOSPC);
+ KUNIT_EXPECT_PTR_EQ(test, connector->state, old_state);
+ KUNIT_EXPECT_EQ(test, to_dm_connector_state(connector->state)->abm_level, 1);
+ dm_test_pps_expect_no_pm_reference(test, connector);
+}
+
/* Tests for amdgpu_dm_backlight_get_device_index() */
/**
@@ -2017,6 +2157,9 @@ static struct kunit_case dm_backlight_test_cases[] = {
KUNIT_CASE(dm_test_panel_power_savings_store_forbidden),
KUNIT_CASE(dm_test_panel_power_savings_store_rejects_invalid_text),
KUNIT_CASE(dm_test_panel_power_savings_store_rejects_out_of_range),
+ KUNIT_CASE(dm_test_panel_power_savings_store_inactive_crtc),
+ KUNIT_CASE(dm_test_panel_power_savings_store_active_crtc),
+ KUNIT_CASE(dm_test_panel_power_savings_store_commit_fails),
/* amdgpu_dm_backlight_get_device_index */
KUNIT_CASE(dm_test_backlight_device_index_matches_second),
KUNIT_CASE(dm_test_backlight_device_index_missing_fallback),
--
2.56.0
prev parent reply other threads:[~2026-11-04 14:15 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-02 10:17 [PATCH 0/5] drm/amd/display: Commit panel_power_savings writes from the kernel Hugo Sales
2026-11-02 10:17 ` [PATCH 1/5] drm/amd/display: Render the panel power savings kernel-doc Hugo Sales
2026-11-02 10:17 ` [PATCH 2/5] drm/amd/display: Don't force a modeset when the ABM level changes Hugo Sales
2026-11-02 10:17 ` [PATCH 3/5] drm/amd/display: Give the panel_power_savings store tests a full pipe Hugo Sales
2026-11-02 10:17 ` [PATCH 4/5] drm/amd/display: Commit panel_power_savings writes from the kernel Hugo Sales
2026-11-02 10:17 ` Hugo Sales [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261102-abm-sysfs-commit-v1-5-e5addfd9539a@hsal.es \
--to=hugo@hsal.es \
--cc=airlied@gmail.com \
--cc=alex.hung@amd.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=bhawanpreet.lakha@amd.com \
--cc=christian.koenig@amd.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=harry.wentland@amd.com \
--cc=ivan.lipski@amd.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mario.limonciello@amd.com \
--cc=ray.wu@amd.com \
--cc=simona@ffwll.ch \
--cc=siqueira@igalia.com \
--cc=sunpeng.li@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox