Re: [PATCH v2] drm/amdgpu: Add support for drm_privacy_screen
Hi,
On 3/20/22 21:11, Rajat Jain wrote:
> () Hello Hans, Sean,
>
>
>
> On Fri, Mar 11, 2022 at 4:12 AM Hans de Goede wrote:
>>
>> Hi All,
>>
>> On 3/9/22 18:53, Rajat Jain wrote:
>>> On Wed, Mar 9, 2022 at 7:06 AM Sean Paul wrote:
From: Sean Paul
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
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
#include
#include
+#include
#include
#include
#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(>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,
_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, >base);
old_crtc_state =
drm_atomic_get_old_crtc_state(state, >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
#include
#include
+#include
#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(>dm_dp_aux.aux,
>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
Re: [PATCH v2] drm/amdgpu: Add support for drm_privacy_screen
Hi All,
On 3/9/22 18:53, Rajat Jain wrote:
> On Wed, Mar 9, 2022 at 7:06 AM Sean Paul wrote:
>>
>> From: Sean Paul
>>
>> 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
>> 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
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #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(>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,
>> _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, >base);
>> old_crtc_state =
>> drm_atomic_get_old_crtc_state(state, >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
>> #include
>> #include
>> +#include
>> #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(>dm_dp_aux.aux,
>> >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
Re: [PATCH v2] drm/amdgpu: Add support for drm_privacy_screen
Hi Sean,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm/drm-next]
[also build test WARNING on drm-intel/for-linux-next drm-tip/drm-tip
drm-exynos/exynos-drm-next next-20220309]
[cannot apply to tegra-drm/drm/tegra/for-next airlied/drm-next v5.17-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Sean-Paul/drm-amdgpu-Add-support-for-drm_privacy_screen/20220309-230712
base: git://anongit.freedesktop.org/drm/drm drm-next
config: arc-randconfig-r043-20220309
(https://download.01.org/0day-ci/archive/20220310/202203100441.aipzrkhu-...@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/6fedd7f8ea75c68634df4fa3cbacb0ee5f2fc984
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Sean-Paul/drm-amdgpu-Add-support-for-drm_privacy_screen/20220309-230712
git checkout 6fedd7f8ea75c68634df4fa3cbacb0ee5f2fc984
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross
O=build_dir ARCH=arc SHELL=/bin/bash drivers/gpu/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
In file included from
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:44:
drivers/gpu/drm/amd/amdgpu/../display/dmub/inc/dmub_cmd.h: In function
'dmub_rb_flush_pending':
drivers/gpu/drm/amd/amdgpu/../display/dmub/inc/dmub_cmd.h:2961:26: warning:
variable 'temp' set but not used [-Wunused-but-set-variable]
2961 | uint64_t temp;
| ^~~~
In file included from include/linux/device.h:15,
from include/linux/acpi.h:15,
from include/linux/i2c.h:13,
from include/drm/drm_crtc.h:28,
from include/drm/drm_atomic.h:31,
from
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:26:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c: In
function 'amdgpu_dm_initialize_dp_connector':
>> drivers/gpu/drm/amd/amdgpu/../amdgpu/amdgpu.h:41:22: warning: format '%d'
>> expects argument of type 'int', but argument 3 has type 'long int'
>> [-Wformat=]
41 | #define dev_fmt(fmt) "amdgpu: " fmt
| ^~
include/linux/dev_printk.h:110:30: note: in definition of macro
'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__);
\
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt),
##__VA_ARGS__)
|^~~
include/drm/drm_print.h:425:9: note: in expansion of macro 'dev_err'
425 | dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
| ^~~~
include/drm/drm_print.h:438:9: note: in expansion of macro '__drm_printk'
438 | __drm_printk((drm), err,, "*ERROR* " fmt, ##__VA_ARGS__)
| ^~~~
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:533:25:
note: in expansion of macro 'drm_err'
533 | drm_err(dev, "Error getting privacy screen,
ret=%d\n",
| ^~~
In file included from
drivers/gpu/drm/amd/amdgpu/../display/dc/inc/dc_link_ddc.h:29,
from
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_mst_types.c:39:
At top level:
drivers/gpu/drm/amd/amdgpu/../display/include/ddc_service_types.h:128:22:
warning: 'SYNAPTICS_DEVICE_ID' defined but not used [-Wunused-const-variable=]
128 | static const uint8_t SYNAPTICS_DEVICE_ID[] = "SYNA";
| ^~~
drivers/gpu/drm/amd/amdgpu/../display/include/ddc_service_types.h:125:22:
warning: 'DP_SINK_DEVICE_STR_ID_2' defined but not used
[-Wunused-const-variable=]
125 | static const uint8_t DP_SINK_DEVICE_STR_ID_2[] = {7, 1, 8, 7, 5, 0};
| ^~~
drivers/gpu/drm/amd/amdgpu/../display/include/ddc_service_types.h:124:22:
warning: 'DP_SINK_DEVICE_STR_ID_1' defined but not used
[-Wunused-const-variable=]
124 | static const uint8_t DP_SINK_DEVICE_STR_ID_1[] = {7, 1, 8, 7, 3, 0};
| ^~~
vim +41
Re: [PATCH v2] drm/amdgpu: Add support for drm_privacy_screen
On Wed, Mar 9, 2022 at 12:54 PM Rajat Jain wrote:
>
> On Wed, Mar 9, 2022 at 7:06 AM Sean Paul wrote:
> >
> > From: Sean Paul
> >
> > 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
> > 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
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #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(>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,
> > _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, >base);
> > old_crtc_state =
> > drm_atomic_get_old_crtc_state(state, >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
> > #include
> > #include
> > +#include
> > #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(>dm_dp_aux.aux,
> > >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"
>
> My comment applies to
Re: [PATCH v2] drm/amdgpu: Add support for drm_privacy_screen
Hi,
On 3/9/22 16:06, Sean Paul wrote:
> From: Sean Paul
>
> 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
> Link: https://patchwork.freedesktop.org/patch/477640/ #v1
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede
Regards,
Hans
> ---
> 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
> #include
> #include
> +#include
> #include
> #include
> #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(>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,
> _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,
> >base);
> old_crtc_state = drm_atomic_get_old_crtc_state(state,
> >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
> #include
> #include
> +#include
> #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(>dm_dp_aux.aux,
> >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);
> + if (!IS_ERR(privacy_screen)) {
> +
> drm_connector_attach_privacy_screen_provider(>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,
[PATCH v2] drm/amdgpu: Add support for drm_privacy_screen
From: Sean Paul
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
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
#include
#include
+#include
#include
#include
#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(>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,
_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,
>base);
old_crtc_state = drm_atomic_get_old_crtc_state(state,
>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
#include
#include
+#include
#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(>dm_dp_aux.aux,
>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);
+ if (!IS_ERR(privacy_screen)) {
+
drm_connector_attach_privacy_screen_provider(>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, _link_enc_cap);
aconnector->mst_mgr.cbs = _mst_cbs;
--
Sean Paul, Software Engineer, Google / Chromium OS
