lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 15 Feb 2024 12:15:08 -0600
From: Mario Limonciello <mario.limonciello@....com>
To: Harry Wentland <harry.wentland@....com>,
 Hamza Mahfooz <hamza.mahfooz@....com>, amd-gfx@...ts.freedesktop.org
Cc: Leo Li <sunpeng.li@....com>, Rodrigo Siqueira <Rodrigo.Siqueira@....com>,
 Alex Deucher <alexander.deucher@....com>,
 Christian König <christian.koenig@....com>,
 "Pan, Xinhui" <Xinhui.Pan@....com>, David Airlie <airlied@...il.com>,
 Daniel Vetter <daniel@...ll.ch>, Alex Hung <alex.hung@....com>,
 Srinivasan Shanmugam <srinivasan.shanmugam@....com>,
 Wayne Lin <wayne.lin@....com>, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/amd/display: add panel_power_savings sysfs entry
 to eDP connectors

On 2/15/2024 11:54, Harry Wentland wrote:
> On 2024-02-02 11:20, Mario Limonciello wrote:
>> On 2/2/2024 09:28, Hamza Mahfooz wrote:
>>> We want programs besides the compositor to be able to enable or disable
>>> panel power saving features. However, since they are currently only
>>> configurable through DRM properties, that isn't possible. So, to remedy
>>> that issue introduce a new "panel_power_savings" sysfs attribute.
>>>
> 
> I've been trying to avoid looking at this too closely, partly because
> I want ABM enablement by default, with control for users. But the
> more I think about this the more uncomfortable I get. The key for my
> discomfort is that we're going around the back of DRM master to set
> a DRM property. This is apt to create lots of weird behaviors,
> especially if compositors also decide to implement support for the
> abm_level property and then potentially fight PPD, or other users
> of this sysfs.

I feel the problem is that specifically both DRM master and sysfs can 
simultaneously fight over the value for ABM.

This isn't a totally new problem because previously the user and DRM
master could fight over this (with the user setting it on command line
and DRM master overriding that).  That was fixed in a follow up patch
though in that the DRM property isn't attached if a user sets the value
on the command line.

I feel the solution to these concerns is that we should make a knob that 
controls whether the DRM property is created or the sysfs file is 
created but not let them happen simultaneously.

We already have amdgpu.abmlevel=-1 is the only time sysfs is created. 
I'd say we should drop the drm property in that case and add a case for 
amdgpu.abmlevel=-2 which will only make the drm property and not the 
sysfs value.  With that done it turns into:

-2  : DRM master is in control
-1  : sysfs is in control.  Software like PPD will tune it as needed.
0-4 : User is in control.



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ