Re: [Intel-gfx] [PATCH] drm/dp/mst: Read the extended DPCD capabilities during system resume
If you add a Cc: to sta...@vger.kernel.org then this is:
Reviewed-by: Lyude Paul
I assume you have the privileges to push this to drm-misc-next
On Tue, 2022-06-14 at 12:45 +0300, Imre Deak wrote:
> The WD22TB4 Thunderbolt dock at least will revert its DP_MAX_LINK_RATE
> from HBR3 to HBR2 after system suspend/resume if the DP_DP13_DPCD_REV
> registers are not read subsequently also as required.
>
> Fix this by reading DP_DP13_DPCD_REV registers as well, matching what is
> done during connector detection. While at it also fix up the same call
> in drm_dp_mst_dump_topology().
>
> Cc: Lyude Paul
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/display/drm_dp_mst_topology.c | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 67b3b9697da7f..18f2b6075b780 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -3860,9 +3860,7 @@ int drm_dp_mst_topology_mgr_resume(struct
> drm_dp_mst_topology_mgr *mgr,
> if (!mgr->mst_primary)
> goto out_fail;
>
> - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
> - DP_RECEIVER_CAP_SIZE);
> - if (ret != DP_RECEIVER_CAP_SIZE) {
> + if (drm_dp_read_dpcd_caps(mgr->aux, mgr->dpcd) < 0) {
> drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during
> suspend?\n");
> goto out_fail;
> }
> @@ -4911,8 +4909,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
> u8 buf[DP_PAYLOAD_TABLE_SIZE];
> int ret;
>
> - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> DP_RECEIVER_CAP_SIZE);
> - if (ret) {
> + if (drm_dp_read_dpcd_caps(mgr->aux, buf) < 0) {
> seq_printf(m, "dpcd read failed\n");
> goto out;
> }
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Re: [Intel-gfx] [PATCH] drm/dp/mst: Read the extended DPCD capabilities during system resume
On Tue, 14 Jun 2022, Imre Deak wrote:
> On Tue, Jun 14, 2022 at 03:32:59PM +0300, Jani Nikula wrote:
>> On Tue, 14 Jun 2022, Imre Deak wrote:
>> > The WD22TB4 Thunderbolt dock at least will revert its DP_MAX_LINK_RATE
>> > from HBR3 to HBR2 after system suspend/resume if the DP_DP13_DPCD_REV
>> > registers are not read subsequently also as required.
>>
>> Does it actually change the behaviour depending on whether the dpcd is
>> read or not, or is this just about the resume path overwriting mgr->dpcd
>> with stuff from DP_DPCD_REV?
>
> Yes, the reading out DP_DP13_DPCD_REV has a side-effect, see
> https://gitlab.freedesktop.org/drm/intel/-/issues/5292#note_1343399
:o
Okay, the commit message is fine as-is.
>
>> drm_dp_mst_topology_mgr_set_mst() does use drm_dp_read_dpcd_caps() for
>> reading the caps, which would normally set mgr->dpcd from
>> DP_DP13_DPCD_REV.
>
> Right, but at that point DP_DP13_DPCD_REV returns HBR2 w/o this change.
>
>> BR,
>> Jani.
>>
>> >
>> > Fix this by reading DP_DP13_DPCD_REV registers as well, matching what is
>> > done during connector detection. While at it also fix up the same call
>> > in drm_dp_mst_dump_topology().
>> >
>> > Cc: Lyude Paul
>> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292
>> > Signed-off-by: Imre Deak
>> > ---
>> > drivers/gpu/drm/display/drm_dp_mst_topology.c | 7 ++-
>> > 1 file changed, 2 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> > b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> > index 67b3b9697da7f..18f2b6075b780 100644
>> > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> > @@ -3860,9 +3860,7 @@ int drm_dp_mst_topology_mgr_resume(struct
>> > drm_dp_mst_topology_mgr *mgr,
>> >if (!mgr->mst_primary)
>> >goto out_fail;
>> >
>> > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
>> > - DP_RECEIVER_CAP_SIZE);
>> > - if (ret != DP_RECEIVER_CAP_SIZE) {
>> > + if (drm_dp_read_dpcd_caps(mgr->aux, mgr->dpcd) < 0) {
>> >drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during
>> > suspend?\n");
>> >goto out_fail;
>> >}
>> > @@ -4911,8 +4909,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>> >u8 buf[DP_PAYLOAD_TABLE_SIZE];
>> >int ret;
>> >
>> > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
>> > DP_RECEIVER_CAP_SIZE);
>> > - if (ret) {
>> > + if (drm_dp_read_dpcd_caps(mgr->aux, buf) < 0) {
>> >seq_printf(m, "dpcd read failed\n");
>> >goto out;
>> >}
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH] drm/dp/mst: Read the extended DPCD capabilities during system resume
On Tue, Jun 14, 2022 at 03:32:59PM +0300, Jani Nikula wrote:
> On Tue, 14 Jun 2022, Imre Deak wrote:
> > The WD22TB4 Thunderbolt dock at least will revert its DP_MAX_LINK_RATE
> > from HBR3 to HBR2 after system suspend/resume if the DP_DP13_DPCD_REV
> > registers are not read subsequently also as required.
>
> Does it actually change the behaviour depending on whether the dpcd is
> read or not, or is this just about the resume path overwriting mgr->dpcd
> with stuff from DP_DPCD_REV?
Yes, the reading out DP_DP13_DPCD_REV has a side-effect, see
https://gitlab.freedesktop.org/drm/intel/-/issues/5292#note_1343399
> drm_dp_mst_topology_mgr_set_mst() does use drm_dp_read_dpcd_caps() for
> reading the caps, which would normally set mgr->dpcd from
> DP_DP13_DPCD_REV.
Right, but at that point DP_DP13_DPCD_REV returns HBR2 w/o this change.
> BR,
> Jani.
>
> >
> > Fix this by reading DP_DP13_DPCD_REV registers as well, matching what is
> > done during connector detection. While at it also fix up the same call
> > in drm_dp_mst_dump_topology().
> >
> > Cc: Lyude Paul
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292
> > Signed-off-by: Imre Deak
> > ---
> > drivers/gpu/drm/display/drm_dp_mst_topology.c | 7 ++-
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > index 67b3b9697da7f..18f2b6075b780 100644
> > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> > @@ -3860,9 +3860,7 @@ int drm_dp_mst_topology_mgr_resume(struct
> > drm_dp_mst_topology_mgr *mgr,
> > if (!mgr->mst_primary)
> > goto out_fail;
> >
> > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
> > - DP_RECEIVER_CAP_SIZE);
> > - if (ret != DP_RECEIVER_CAP_SIZE) {
> > + if (drm_dp_read_dpcd_caps(mgr->aux, mgr->dpcd) < 0) {
> > drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during
> > suspend?\n");
> > goto out_fail;
> > }
> > @@ -4911,8 +4909,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
> > u8 buf[DP_PAYLOAD_TABLE_SIZE];
> > int ret;
> >
> > - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> > DP_RECEIVER_CAP_SIZE);
> > - if (ret) {
> > + if (drm_dp_read_dpcd_caps(mgr->aux, buf) < 0) {
> > seq_printf(m, "dpcd read failed\n");
> > goto out;
> > }
>
> --
> Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH] drm/dp/mst: Read the extended DPCD capabilities during system resume
On Tue, 14 Jun 2022, Jani Nikula wrote:
> On Tue, 14 Jun 2022, Imre Deak wrote:
>> The WD22TB4 Thunderbolt dock at least will revert its DP_MAX_LINK_RATE
>> from HBR3 to HBR2 after system suspend/resume if the DP_DP13_DPCD_REV
>> registers are not read subsequently also as required.
>
> Does it actually change the behaviour depending on whether the dpcd is
> read or not, or is this just about the resume path overwriting mgr->dpcd
> with stuff from DP_DPCD_REV?
>
> drm_dp_mst_topology_mgr_set_mst() does use drm_dp_read_dpcd_caps() for
> reading the caps, which would normally set mgr->dpcd from
> DP_DP13_DPCD_REV.
Oh,
Reviewed-by: Jani Nikula
on the changes, I'm just wondering about the statement in the commit
message.
>
> BR,
> Jani.
>
>>
>> Fix this by reading DP_DP13_DPCD_REV registers as well, matching what is
>> done during connector detection. While at it also fix up the same call
>> in drm_dp_mst_dump_topology().
>>
>> Cc: Lyude Paul
>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292
>> Signed-off-by: Imre Deak
>> ---
>> drivers/gpu/drm/display/drm_dp_mst_topology.c | 7 ++-
>> 1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> index 67b3b9697da7f..18f2b6075b780 100644
>> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
>> @@ -3860,9 +3860,7 @@ int drm_dp_mst_topology_mgr_resume(struct
>> drm_dp_mst_topology_mgr *mgr,
>> if (!mgr->mst_primary)
>> goto out_fail;
>>
>> -ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
>> - DP_RECEIVER_CAP_SIZE);
>> -if (ret != DP_RECEIVER_CAP_SIZE) {
>> +if (drm_dp_read_dpcd_caps(mgr->aux, mgr->dpcd) < 0) {
>> drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during
>> suspend?\n");
>> goto out_fail;
>> }
>> @@ -4911,8 +4909,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>> u8 buf[DP_PAYLOAD_TABLE_SIZE];
>> int ret;
>>
>> -ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
>> DP_RECEIVER_CAP_SIZE);
>> -if (ret) {
>> +if (drm_dp_read_dpcd_caps(mgr->aux, buf) < 0) {
>> seq_printf(m, "dpcd read failed\n");
>> goto out;
>> }
--
Jani Nikula, Intel Open Source Graphics Center
Re: [Intel-gfx] [PATCH] drm/dp/mst: Read the extended DPCD capabilities during system resume
On Tue, 14 Jun 2022, Imre Deak wrote:
> The WD22TB4 Thunderbolt dock at least will revert its DP_MAX_LINK_RATE
> from HBR3 to HBR2 after system suspend/resume if the DP_DP13_DPCD_REV
> registers are not read subsequently also as required.
Does it actually change the behaviour depending on whether the dpcd is
read or not, or is this just about the resume path overwriting mgr->dpcd
with stuff from DP_DPCD_REV?
drm_dp_mst_topology_mgr_set_mst() does use drm_dp_read_dpcd_caps() for
reading the caps, which would normally set mgr->dpcd from
DP_DP13_DPCD_REV.
BR,
Jani.
>
> Fix this by reading DP_DP13_DPCD_REV registers as well, matching what is
> done during connector detection. While at it also fix up the same call
> in drm_dp_mst_dump_topology().
>
> Cc: Lyude Paul
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292
> Signed-off-by: Imre Deak
> ---
> drivers/gpu/drm/display/drm_dp_mst_topology.c | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 67b3b9697da7f..18f2b6075b780 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -3860,9 +3860,7 @@ int drm_dp_mst_topology_mgr_resume(struct
> drm_dp_mst_topology_mgr *mgr,
> if (!mgr->mst_primary)
> goto out_fail;
>
> - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
> -DP_RECEIVER_CAP_SIZE);
> - if (ret != DP_RECEIVER_CAP_SIZE) {
> + if (drm_dp_read_dpcd_caps(mgr->aux, mgr->dpcd) < 0) {
> drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during
> suspend?\n");
> goto out_fail;
> }
> @@ -4911,8 +4909,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
> u8 buf[DP_PAYLOAD_TABLE_SIZE];
> int ret;
>
> - ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
> DP_RECEIVER_CAP_SIZE);
> - if (ret) {
> + if (drm_dp_read_dpcd_caps(mgr->aux, buf) < 0) {
> seq_printf(m, "dpcd read failed\n");
> goto out;
> }
--
Jani Nikula, Intel Open Source Graphics Center
[Intel-gfx] [PATCH] drm/dp/mst: Read the extended DPCD capabilities during system resume
The WD22TB4 Thunderbolt dock at least will revert its DP_MAX_LINK_RATE
from HBR3 to HBR2 after system suspend/resume if the DP_DP13_DPCD_REV
registers are not read subsequently also as required.
Fix this by reading DP_DP13_DPCD_REV registers as well, matching what is
done during connector detection. While at it also fix up the same call
in drm_dp_mst_dump_topology().
Cc: Lyude Paul
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292
Signed-off-by: Imre Deak
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 67b3b9697da7f..18f2b6075b780 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -3860,9 +3860,7 @@ int drm_dp_mst_topology_mgr_resume(struct
drm_dp_mst_topology_mgr *mgr,
if (!mgr->mst_primary)
goto out_fail;
- ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, mgr->dpcd,
- DP_RECEIVER_CAP_SIZE);
- if (ret != DP_RECEIVER_CAP_SIZE) {
+ if (drm_dp_read_dpcd_caps(mgr->aux, mgr->dpcd) < 0) {
drm_dbg_kms(mgr->dev, "dpcd read failed - undocked during
suspend?\n");
goto out_fail;
}
@@ -4911,8 +4909,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
u8 buf[DP_PAYLOAD_TABLE_SIZE];
int ret;
- ret = drm_dp_dpcd_read(mgr->aux, DP_DPCD_REV, buf,
DP_RECEIVER_CAP_SIZE);
- if (ret) {
+ if (drm_dp_read_dpcd_caps(mgr->aux, buf) < 0) {
seq_printf(m, "dpcd read failed\n");
goto out;
}
--
2.30.2
