Messages in this thread | | | Date | Fri, 16 Feb 2024 11:29:54 +0100 | Subject | Re: [PATCH v2] drm/amd/display: add panel_power_savings sysfs entry to eDP connectors | From | Christian König <> |
| |
Am 02.02.24 um 16:28 schrieb Hamza Mahfooz: > We want programs besides the compositor to be able to enable or disable > panel power saving features.
Well I don't know the full background, but that is usually a no-go.
> However, since they are currently only > configurable through DRM properties, that isn't possible.
Maybe I'm repeating others, but I wanted to point it out explicitly: This is intentional and not that easily changeable.
If it's a common DRM property, e.g. something standardized among all drivers, then amdgpu is not allowed to circumvent that.
Regards, Christian.
> So, to remedy > that issue introduce a new "panel_power_savings" sysfs attribute. > > Cc: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com> > --- > v2: hide ABM_LEVEL_IMMEDIATE_DISABLE in the read case, force an atomic > commit when setting the value, call sysfs_remove_group() in > amdgpu_dm_connector_unregister() and add some documentation. > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 76 +++++++++++++++++++ > 1 file changed, 76 insertions(+) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 8590c9f1dda6..3c62489d03dc 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -6436,10 +6436,79 @@ 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); > > + sysfs_remove_group(&connector->kdev->kobj, &amdgpu_group); > drm_dp_aux_unregister(&amdgpu_dm_connector->dm_dp_aux.aux); > } > > @@ -6541,6 +6610,13 @@ amdgpu_dm_connector_late_register(struct drm_connector *connector) > to_amdgpu_dm_connector(connector); > int r; > > + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) { > + 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) ||
| |