RE: [RFC v2] drm/connector: Add support for privacy-screen properties (v2)
> -Original Message-
> From: Hans de Goede
> Sent: Monday, May 11, 2020 12:47 PM
> To: Maarten Lankhorst; Maxime Ripard; Thomas Zimmermann; Daniel Vetter; David
> Airlie; Rajat Jain; Jani Nikula
> Cc: Hans de Goede; Pekka Paalanen; Limonciello, Mario; Quintanilla, Sonny;
> Jared Dominguez; Mark Pearson; dri-devel@lists.freedesktop.org
> Subject: [RFC v2] drm/connector: Add support for privacy-screen properties
> (v2)
>
>
> [EXTERNAL EMAIL]
>
> From: Rajat Jain
>
> Add support for generic electronic privacy screen properties, that
> can be added by systems that have an integrated EPS.
>
> Changes in v2 (Hans de Goede)
> - Create 2 properties, "privacy-screen sw-state" and
> "privacy-screen hw-state", to deal with devices where the OS might be
> locked out of making state changes
> - Write kerneldoc explaining how the 2 properties work together, what
> happens when changes to the state are made outside of the DRM code's
> control, etc.
>
> Signed-off-by: Rajat Jain
> Co-authored-by: Hans de Goede
> Signed-off-by: Hans de Goede
> ---
> Documentation/gpu/drm-kms.rst | 2 +
> drivers/gpu/drm/drm_atomic_uapi.c | 6 ++
> drivers/gpu/drm/drm_connector.c | 100 ++
> include/drm/drm_connector.h | 50 +++
> 4 files changed, 158 insertions(+)
>
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 906771e03103..b72b1e0db343 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -445,6 +445,8 @@ Property Types and Blob Property Support
> .. kernel-doc:: drivers/gpu/drm/drm_property.c
> :export:
>
> +.. _standard_connector_properties:
> +
> Standard Connector Properties
> -
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index a1e5e262bae2..e56a11208515 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -766,6 +766,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> fence_ptr);
> } else if (property == connector->max_bpc_property) {
> state->max_requested_bpc = val;
> + } else if (property == connector->privacy_screen_sw_state_property) {
> + state->privacy_screen_sw_state = val;
> } else if (connector->funcs->atomic_set_property) {
> return connector->funcs->atomic_set_property(connector,
> state, property, val);
> @@ -842,6 +844,10 @@ drm_atomic_connector_get_property(struct drm_connector
> *connector,
> *val = 0;
> } else if (property == connector->max_bpc_property) {
> *val = state->max_requested_bpc;
> + } else if (property == connector->privacy_screen_sw_state_property) {
> + *val = state->privacy_screen_sw_state;
> + } else if (property == connector->privacy_screen_hw_state_property) {
> + *val = state->privacy_screen_hw_state;
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 644f0ad10671..01360edc2376 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1186,6 +1186,45 @@ static const struct drm_prop_enum_list dp_colorspaces[]
> = {
> * can also expose this property to external outputs, in which case they
> * must support "None", which should be the default (since external screens
> * have a built-in scaler).
> + *
> + * privacy-screen sw-state, privacy-screen hw-state:
> + * These 2 optional properties can be used to query the state of the
> + * electronic privacy screen that is available on some displays; and in
> + * some cases also control the state. If a driver implements these
> + * properties then both properties must be present.
> + *
> + * "privacy-screen hw-state" is read-only and reflects the actual state
> + * of the privacy-screen, possible values: "Enabled", "Disabled,
> + * "Enabled, locked", "Disabled, locked". The locked states indicate
> + * that the state cannot be changed through the DRM API. E.g. there
> + * might be devices where the firmware-setup options, or a hardware
> + * slider-switch, offer always on / off modes.
> + *
> + * "privacy-screen sw-state" can be set to change the privacy-screen state
> + * when not locked. In this case the driver must update the hw-state
> + * property to reflect the new state on completion of the commit of the
> + * sw-state property. Setting the sw-state property when the hw-state is
> + * locked must be interpreted by the driver as a request to change the
> + * state to the set state when the hw-state becomes unlocked. E.g. if
> + *
RE: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
> -Original Message- > From: Takashi Iwai > Sent: Thursday, August 15, 2019 9:57 AM > To: Alex Deucher > Cc: Karol Herbst; Limonciello, Mario; nouveau; Rafael J . Wysocki; LKML; > dri-devel; > Linux ACPI Mailing List; Alex Hung; Ben Skeggs; David Airlie > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to > enable dGPU direct output" > > > [EXTERNAL EMAIL] > > On Thu, 15 Aug 2019 16:37:05 +0200, > Alex Deucher wrote: > > > > On Thu, Aug 15, 2019 at 10:25 AM Karol Herbst wrote: > > > > > > On Thu, Aug 15, 2019 at 4:20 PM wrote: > > > > > > > > > > There are definitely going to be regressions on machines in the > > > > > > field > with the > > > > > > in tree drivers by reverting this. I think we should have an > > > > > > answer for all > of > > > > > those > > > > > > before this revert is accepted. > > > > > > > > > > > > Regarding systems with Intel+NVIDIA, we'll have to work with > > > > > > partners > to > > > > > collect > > > > > > some information on the impact of reverting this. > > > > > > > > > > > > When this is used on a system with Intel+AMD the ASL configures AMD > GPU to > > > > > use > > > > > > "Hybrid Graphics" when on Windows and "Power Express" and > "Switchable > > > > > Graphics" > > > > > > when on Linux. > > > > > > > > > > and what's exactly the difference between those? And what's the actual > > > > > issue here? > > > > > > > > DP/HDMI is not detected unless plugged in at bootup. It's due to > > > > missing > HPD > > > > events. > > > > > > > > > > afaik Lyude was working on fixing all that, at least for some drivers. > > > If there is something wrong, we still should fix the drivers, not > > > adding ACPI workarounds. > > > > > > Alex: do you know if there are remaining issues regarding that with > > > amdgpu? > > > > There was an issue with hpd events not making it to the audio side > > when things were powered down that was fixed with this patch set: > > https://patchwork.freedesktop.org/patch/316793/ > > Those patches depended on a bunch of alsa changes as well which may > > have not been available in the distro used for a particular OEM > > program. > > FYI, the corresponding commit for ALSA part is destined for 5.4 > kernel: > > https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/commit/?id=ade > 49db337a9d44ac5835cfce1ee873549011b27 > > BTW, Nouveau should suffer from the same problem. The patch to add > the audio component support is found at: > https://patchwork.freedesktop.org/patch/319131/ > > It sounds like 5.3rcX won't be a useful check then. So am I correct to understand that everything related to the AMD failures described in this thread should be in linux-next at this point? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
> -Original Message- > From: Karol Herbst > Sent: Thursday, August 15, 2019 9:25 AM > To: Limonciello, Mario > Cc: Dave Airlie; LKML; Linux ACPI Mailing List; dri-devel; nouveau; Rafael J . > Wysocki; Alex Hung; Ben Skeggs; David Airlie > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to > enable dGPU direct output" > > > [EXTERNAL EMAIL] > > On Thu, Aug 15, 2019 at 4:20 PM wrote: > > > > > > There are definitely going to be regressions on machines in the field > > > > with > the > > > > in tree drivers by reverting this. I think we should have an answer > > > > for all of > > > those > > > > before this revert is accepted. > > > > > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to > > > collect > > > > some information on the impact of reverting this. > > > > > > > > When this is used on a system with Intel+AMD the ASL configures AMD > GPU to > > > use > > > > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable > > > Graphics" > > > > when on Linux. > > > > > > and what's exactly the difference between those? And what's the actual > > > issue here? > > > > DP/HDMI is not detected unless plugged in at bootup. It's due to missing > > HPD > > events. > > > > afaik Lyude was working on fixing all that, at least for some drivers. > If there is something wrong, we still should fix the drivers, not > adding ACPI workarounds. I don't disagree, but timing is frequently a limitation if you want the hardware to work when you put it on the market. The whole idea behind the OSI string was that it could be reverted when the time was right. From this discussion it very well may be for systems with AMD GPU, but it needs to be checked again. > > Alex: do you know if there are remaining issues regarding that with amdgpu? > > > > > > > We already have the PRIME offloading in place and if that's not > > > enough, we should work on extending it, not adding some ACPI based > > > workarounds, because that's exactly how that looks like. > > > > > > Also, was this discussed with anybody involved in the drm subsystem? > > > > > > > > > > > I feel we need a knob and/or DMI detection to affect the changes that > > > > the > ASL > > > > normally performs. > > > > > > Why do we have to do that on a firmware level at all? > > > > Folks from AMD Graphics team recommended this approach. I should clarify this is from the folks on AMD graphics team that interact to OEMs like Dell which are not necessarily the same folks who work on the drivers directly. > From their perspective > > it's not a workaround. They view this as a different architecture for AMD > graphics driver on > > Windows and AMD graphics w/ amdgpu driver. They have different ASL paths > used for > > each. > > @alex: is this true? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
> -Original Message- > From: linux-acpi-ow...@vger.kernel.org On > Behalf Of Dave Airlie > Sent: Wednesday, August 14, 2019 5:48 PM > To: Karol Herbst > Cc: LKML; Linux ACPI; dri-devel; nouveau; Rafael J . Wysocki; Alex Hung; Ben > Skeggs; Dave Airlie > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to > enable dGPU direct output" > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst wrote: > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c. > > > > The original commit message didn't even make sense. AMD _does_ support it > > and > > it works with Nouveau as well. > > > > Also what was the issue being solved here? No references to any bugs and not > > even explaining any issue at all isn't the way we do things. > > > > And even if it means a muxed design, then the fix is to make it work inside > > the > > driver, not adding some hacky workaround through ACPI tricks. > > > > And what out of tree drivers do or do not support we don't care one bit > > anyway. > > > > I think the reverts should be merged via Rafael's tree as the original > patches went in via there, and we should get them in asap. > > Acked-by: Dave Airlie > Dave. There are definitely going to be regressions on machines in the field with the in tree drivers by reverting this. I think we should have an answer for all of those before this revert is accepted. Regarding systems with Intel+NVIDIA, we'll have to work with partners to collect some information on the impact of reverting this. When this is used on a system with Intel+AMD the ASL configures AMD GPU to use "Hybrid Graphics" when on Windows and "Power Express" and "Switchable Graphics" when on Linux. I feel we need a knob and/or DMI detection to affect the changes that the ASL normally performs. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
> > There are definitely going to be regressions on machines in the field with > > the > > in tree drivers by reverting this. I think we should have an answer for > > all of > those > > before this revert is accepted. > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners to > collect > > some information on the impact of reverting this. > > > > When this is used on a system with Intel+AMD the ASL configures AMD GPU to > use > > "Hybrid Graphics" when on Windows and "Power Express" and "Switchable > Graphics" > > when on Linux. > > and what's exactly the difference between those? And what's the actual > issue here? DP/HDMI is not detected unless plugged in at bootup. It's due to missing HPD events. > > We already have the PRIME offloading in place and if that's not > enough, we should work on extending it, not adding some ACPI based > workarounds, because that's exactly how that looks like. > > Also, was this discussed with anybody involved in the drm subsystem? > > > > > I feel we need a knob and/or DMI detection to affect the changes that the > > ASL > > normally performs. > > Why do we have to do that on a firmware level at all? Folks from AMD Graphics team recommended this approach. From their perspective it's not a workaround. They view this as a different architecture for AMD graphics driver on Windows and AMD graphics w/ amdgpu driver. They have different ASL paths used for each. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI string to enable dGPU direct output"
> On Thu, Aug 15, 2019 at 10:15 AM Karol Herbst wrote: > > > > On Thu, Aug 15, 2019 at 4:13 PM Alex Deucher > wrote: > > > > > > On Thu, Aug 15, 2019 at 10:04 AM Karol Herbst wrote: > > > > > > > > On Thu, Aug 15, 2019 at 3:56 PM wrote: > > > > > > > > > > > -Original Message- > > > > > > From: linux-acpi-ow...@vger.kernel.org ow...@vger.kernel.org> On > > > > > > Behalf Of Dave Airlie > > > > > > Sent: Wednesday, August 14, 2019 5:48 PM > > > > > > To: Karol Herbst > > > > > > Cc: LKML; Linux ACPI; dri-devel; nouveau; Rafael J . Wysocki; Alex > > > > > > Hung; > Ben > > > > > > Skeggs; Dave Airlie > > > > > > Subject: Re: [Nouveau] [PATCH 1/7] Revert "ACPI / OSI: Add OEM _OSI > string to > > > > > > enable dGPU direct output" > > > > > > > > > > > > On Thu, 15 Aug 2019 at 07:31, Karol Herbst > wrote: > > > > > > > > > > > > > > This reverts commit 28586a51eea666d5531bcaef2f68e4abbd87242c. > > > > > > > > > > > > > > The original commit message didn't even make sense. AMD _does_ > support it and > > > > > > > it works with Nouveau as well. > > > > > > > > > > > > > > Also what was the issue being solved here? No references to any > > > > > > > bugs > and not > > > > > > > even explaining any issue at all isn't the way we do things. > > > > > > > > > > > > > > And even if it means a muxed design, then the fix is to make it > > > > > > > work > inside the > > > > > > > driver, not adding some hacky workaround through ACPI tricks. > > > > > > > > > > > > > > And what out of tree drivers do or do not support we don't care > > > > > > > one > bit anyway. > > > > > > > > > > > > > > > > > > > I think the reverts should be merged via Rafael's tree as the > > > > > > original > > > > > > patches went in via there, and we should get them in asap. > > > > > > > > > > > > Acked-by: Dave Airlie > > > > > > Dave. > > > > > > > > > > There are definitely going to be regressions on machines in the field > > > > > with > the > > > > > in tree drivers by reverting this. I think we should have an answer > > > > > for all > of those > > > > > before this revert is accepted. > > > > > > > > > > Regarding systems with Intel+NVIDIA, we'll have to work with partners > > > > > to > collect > > > > > some information on the impact of reverting this. > > > > > > > > > > When this is used on a system with Intel+AMD the ASL configures AMD > GPU to use > > > > > "Hybrid Graphics" when on Windows and "Power Express" and > "Switchable Graphics" > > > > > when on Linux. > > > > > > > > and what's exactly the difference between those? And what's the actual > > > > issue here? > > > > > > Hybrid Graphics is the new "standard" way of handling these laptops. > > > It uses the standard _PR3 APCI method to handle dGPU power. Support > > > for this was added to Linux relatively later compared to when the > > > laptops were launched. "Power Express" used the other AMD specific > > > ATPX ACPI method to handle dGPU power. The driver supports both so > > > either method will work. > > > > > > Alex > > > > > > > thanks for clarifying. But that still means that we won't need such > > workarounds for AMD users, right? amdgpu handles hybrid graphics just > > fine, right? > > Yeah it should, assuming you have a new enough kernel which supports > HG, which has been several years at this point IIRC. > > Alex > Can you define how new of a kernel is a new enough kernel? Looking on my side these problems were on new hardware (Precision 7740) and are checked as recently as start of this summer, w/ kernel 4.15. We can arrange to have it checked again on 5.3rcX w/ the OSI disabled. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
