From: Sebastian Wick <sebastian.wick@redhat.com>
To: dri-devel@lists.freedesktop.org
Cc: "Sebastian Wick" <sebastian@sebastianwick.net>,
"Xaver Hugl" <xaver.hugl@gmail.com>,
"Simon Ser" <contact@emersion.fr>,
"Pekka Paalanen" <pekka.paalanen@collabora.com>,
"Harry Wentland" <harry.wentland@amd.com>,
"Leo Li" <sunpeng.li@amd.com>,
"Rodrigo Siqueira" <Rodrigo.Siqueira@amd.com>,
"Alex Deucher" <alexander.deucher@amd.com>,
"Christian König" <christian.koenig@amd.com>,
"Xinhui Pan" <Xinhui.Pan@amd.com>,
"David Airlie" <airlied@gmail.com>,
"Daniel Vetter" <daniel@ffwll.ch>,
"Alex Hung" <alex.hung@amd.com>,
"Hamza Mahfooz" <hamza.mahfooz@amd.com>,
"Roman Li" <roman.li@amd.com>,
"Mario Limonciello" <mario.limonciello@amd.com>,
"Wayne Lin" <Wayne.Lin@amd.com>,
amd-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: [PATCH 2/2] Revert "drm/amd/display: add panel_power_savings sysfs entry to eDP connectors"
Date: Tue, 6 Aug 2024 20:42:10 +0200 [thread overview]
Message-ID: <20240806184214.224672-2-sebastian.wick@redhat.com> (raw)
In-Reply-To: <20240806184214.224672-1-sebastian.wick@redhat.com>
From: Sebastian Wick <sebastian@sebastianwick.net>
This reverts commit 63d0b87213a0ba241b3fcfba3fe7b0aed0cd1cc5.
The panel_power_savings sysfs entry can be used to change the displayed
colorimetry which breaks color managed setups.
The "do not break userspace" rule which was violated here is enough
reason to revert this commit.
The bigger problem is that this feature is part of the display chain
which is supposed to be controlled by KMS. This sysfs entry bypasses the
DRM master process and splits control to two independent processes which
do not know about each other. This is what caused the broken user space.
It also causes modesets which can be extremely confusing for the DRM
master process, causing unexpected timings.
We should in general not allow anything other than KMS to control the
display path. If we make an exception to this rule, this must be first
discussed on dri-devel with all the stakeholders approving the
exception.
This has not happened which is the second reason to revert this commit.
Signed-off-by: Sebastian Wick <sebastian.wick@redhat.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 80 -------------------
1 file changed, 80 deletions(-)
diff --git ./drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c ../drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 4d4c75173fc3..16c9051d9ccf 100644
--- ./drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ ../drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6772,82 +6772,10 @@ int amdgpu_dm_connector_atomic_get_property(struct drm_connector *connector,
return ret;
}
-/**
- * DOC: panel power savings
- *
- * The display manager allows you to set your desired **panel power savings**
- * level (between 0-4, with 0 representing off), e.g. using the following::
- *
- * # echo 3 > /sys/class/drm/card0-eDP-1/amdgpu/panel_power_savings
- *
- * Modifying this value can have implications on color accuracy, so tread
- * carefully.
- */
-
-static ssize_t panel_power_savings_show(struct device *device,
- struct device_attribute *attr,
- char *buf)
-{
- struct drm_connector *connector = dev_get_drvdata(device);
- struct drm_device *dev = connector->dev;
- u8 val;
-
- drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
- val = to_dm_connector_state(connector->state)->abm_level ==
- ABM_LEVEL_IMMEDIATE_DISABLE ? 0 :
- to_dm_connector_state(connector->state)->abm_level;
- drm_modeset_unlock(&dev->mode_config.connection_mutex);
-
- return sysfs_emit(buf, "%u\n", val);
-}
-
-static 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;
-
- ret = kstrtol(buf, 0, &val);
-
- if (ret)
- return ret;
-
- if (val < 0 || val > 4)
- return -EINVAL;
-
- drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
- to_dm_connector_state(connector->state)->abm_level = val ?:
- ABM_LEVEL_IMMEDIATE_DISABLE;
- drm_modeset_unlock(&dev->mode_config.connection_mutex);
-
- drm_kms_helper_hotplug_event(dev);
-
- return count;
-}
-
-static DEVICE_ATTR_RW(panel_power_savings);
-
-static struct attribute *amdgpu_attrs[] = {
- &dev_attr_panel_power_savings.attr,
- NULL
-};
-
-static const struct attribute_group amdgpu_group = {
- .name = "amdgpu",
- .attrs = amdgpu_attrs
-};
-
static void amdgpu_dm_connector_unregister(struct drm_connector *connector)
{
struct amdgpu_dm_connector *amdgpu_dm_connector = to_amdgpu_dm_connector(connector);
- if (connector->connector_type == DRM_MODE_CONNECTOR_eDP &&
- amdgpu_dm_abm_level < 0)
- sysfs_remove_group(&connector->kdev->kobj, &amdgpu_group);
-
drm_dp_aux_unregister(&amdgpu_dm_connector->dm_dp_aux.aux);
}
@@ -6952,14 +6880,6 @@ amdgpu_dm_connector_late_register(struct drm_connector *connector)
to_amdgpu_dm_connector(connector);
int r;
- if (connector->connector_type == DRM_MODE_CONNECTOR_eDP &&
- amdgpu_dm_abm_level < 0) {
- r = sysfs_create_group(&connector->kdev->kobj,
- &amdgpu_group);
- if (r)
- return r;
- }
-
amdgpu_dm_register_backlight_device(amdgpu_dm_connector);
if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) ||
--
2.45.2
next prev parent reply other threads:[~2024-08-06 18:42 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-06 18:42 [PATCH 1/2] Revert "drm/amd/display: Don't register panel_power_savings on OLED panels" Sebastian Wick
2024-08-06 18:42 ` Sebastian Wick [this message]
2024-08-07 5:13 ` [PATCH 2/2] Revert "drm/amd/display: add panel_power_savings sysfs entry to eDP connectors" Mario Limonciello
2024-08-07 13:52 ` Leo Li
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=20240806184214.224672-2-sebastian.wick@redhat.com \
--to=sebastian.wick@redhat.com \
--cc=Rodrigo.Siqueira@amd.com \
--cc=Wayne.Lin@amd.com \
--cc=Xinhui.Pan@amd.com \
--cc=airlied@gmail.com \
--cc=alex.hung@amd.com \
--cc=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=contact@emersion.fr \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=hamza.mahfooz@amd.com \
--cc=harry.wentland@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mario.limonciello@amd.com \
--cc=pekka.paalanen@collabora.com \
--cc=roman.li@amd.com \
--cc=sebastian@sebastianwick.net \
--cc=sunpeng.li@amd.com \
--cc=xaver.hugl@gmail.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
all inboxes | Powered by JetHome®