[PATCH v2] drm/amdgpu: Add support for drm_privacy_screen
Hans de Goede
hdegoede at redhat.com
Fri Mar 11 12:12:05 UTC 2022
Hi All,
On 3/9/22 18:53, Rajat Jain wrote:
> On Wed, Mar 9, 2022 at 7:06 AM Sean Paul <sean at poorly.run> wrote:
>>
>> From: Sean Paul <seanpaul at chromium.org>
>>
>> This patch adds the necessary hooks to make amdgpu aware of privacy
>> screens. On devices with privacy screen drivers (such as thinkpad-acpi),
>> the amdgpu driver will defer probe until it's ready and then sync the sw
>> and hw state on each commit the connector is involved and enabled.
>>
>> Changes in v2:
>> -Tweaked the drm_privacy_screen_get() error check to avoid logging
>> errors when privacy screen is absent (Hans)
>>
>> Signed-off-by: Sean Paul <seanpaul at chromium.org>
>> Link: https://patchwork.freedesktop.org/patch/477640/ #v1
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +++++++++
>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
>> .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 16 +++++++++++++++-
>> 3 files changed, 27 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 2ab675123ae3..e2cfae56c020 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -26,6 +26,7 @@
>> #include <drm/drm_aperture.h>
>> #include <drm/drm_drv.h>
>> #include <drm/drm_gem.h>
>> +#include <drm/drm_privacy_screen_consumer.h>
>> #include <drm/drm_vblank.h>
>> #include <drm/drm_managed.h>
>> #include "amdgpu_drv.h"
>> @@ -1988,6 +1989,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>> {
>> struct drm_device *ddev;
>> struct amdgpu_device *adev;
>> + struct drm_privacy_screen *privacy_screen;
>> unsigned long flags = ent->driver_data;
>> int ret, retry = 0, i;
>> bool supports_atomic = false;
>> @@ -2063,6 +2065,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>> size = pci_resource_len(pdev, 0);
>> is_fw_fb = amdgpu_is_fw_framebuffer(base, size);
>>
>> + /* If the LCD panel has a privacy screen, defer probe until its ready */
>> + privacy_screen = drm_privacy_screen_get(&pdev->dev, NULL);
>> + if (IS_ERR(privacy_screen) && PTR_ERR(privacy_screen) == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> +
>> + drm_privacy_screen_put(privacy_screen);
>> +
>> /* Get rid of things like offb */
>> ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &amdgpu_kms_driver);
>> if (ret)
>> 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 e1d3db3fe8de..9e2bb6523add 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -9781,6 +9781,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
>> if (acrtc) {
>> new_crtc_state = drm_atomic_get_new_crtc_state(state, &acrtc->base);
>> old_crtc_state = drm_atomic_get_old_crtc_state(state, &acrtc->base);
>> +
>> + /* Sync the privacy screen state between hw and sw */
>> + drm_connector_update_privacy_screen(new_con_state);
>> }
>>
>> /* Skip any modesets/resets */
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> index 740435ae3997..594a8002975a 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
>> @@ -27,6 +27,7 @@
>> #include <drm/drm_atomic_helper.h>
>> #include <drm/dp/drm_dp_mst_helper.h>
>> #include <drm/dp/drm_dp_helper.h>
>> +#include <drm/drm_privacy_screen_consumer.h>
>> #include "dm_services.h"
>> #include "amdgpu.h"
>> #include "amdgpu_dm.h"
>> @@ -506,6 +507,7 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,
>> struct amdgpu_dm_connector *aconnector,
>> int link_index)
>> {
>> + struct drm_device *dev = dm->ddev;
>> struct dc_link_settings max_link_enc_cap = {0};
>>
>> aconnector->dm_dp_aux.aux.name =
>> @@ -519,8 +521,20 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,
>> drm_dp_cec_register_connector(&aconnector->dm_dp_aux.aux,
>> &aconnector->base);
>>
>> - if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP)
>> + if (aconnector->base.connector_type == DRM_MODE_CONNECTOR_eDP) {
>> + struct drm_privacy_screen *privacy_screen)
>> +
>> + /* Reference given up in drm_connector_cleanup() */
>> + privacy_screen = drm_privacy_screen_get(dev->dev, NULL);
>
> Can we try to be more specific when looking up for privacy screen, e.g.:
>
> privacy_screen = drm_privacy_screen_get(dev->dev, "eDP-1");
> (and then also making the corresponding change in arch_init_data[] in
> drm_privacy_screen_x86.c"
So I just checked and yes I think we can be more specific at least
for the thinkpad_acpi supported models. See the attached patch
which has been tested on a ThinkPad T14 gen 1 with a builtin privacy-screen.
Rajat, can you adjust the chrome code in drivers/gpu/drm/drm_privacy_screen_x86.c
to match and check that with the chrome code changes, my patch does not break
things on chromebooks? (Note your changes will need to be squashed into the
patch so that we change all of this in one go) .
Sean, same request to you, can you adjust your amdgpu patch to match
the i915 changes in the attached patch and then check if a kernel
with both changes still works ?
Regards,
Hans
>
> My comment applies to this driver as well as to i915. The reason being
> today there is only 1 internal display with privacy screen so we can
> just assume that there shall be only 1 privacy-screen and that shall
> apply to eDP-1/internal display. But dual display systems are not far
> enough out, and perhaps external monitors with inbuilt
> privacy-screens?
>
> Essentially the gap today is that today on Chromeos ACPI side, the
> device GOOG0010 represents the privacy-screen attached to the internal
> display/eDP-1, but there isn't a way to make it clear in the i915 and
> now amdgpu code.
>
> Thanks,
>
> Rajat
>
>
>> + if (!IS_ERR(privacy_screen)) {
>> + drm_connector_attach_privacy_screen_provider(&aconnector->base,
>> + privacy_screen);
>> + } else if (PTR_ERR(privacy_screen) != -ENODEV) {
>> + drm_err(dev, "Error getting privacy screen, ret=%d\n",
>> + PTR_ERR(privacy_screen));
>> + }
>> return;
>> + }
>>
>> dc_link_dp_get_max_link_enc_cap(aconnector->dc_link, &max_link_enc_cap);
>> aconnector->mst_mgr.cbs = &dm_mst_cbs;
>> --
>> Sean Paul, Software Engineer, Google / Chromium OS
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-privacy-screen-Hardcode.patch
Type: text/x-patch
Size: 3121 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20220311/aab43184/attachment-0001.bin>
More information about the amd-gfx
mailing list