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 lyude@redhat.com Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292 Signed-off-by: Imre Deak imre.deak@intel.com --- 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; }
On Tue, 14 Jun 2022, Imre Deak imre.deak@intel.com 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 lyude@redhat.com Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292 Signed-off-by: Imre Deak imre.deak@intel.com
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;
On Tue, 14 Jun 2022, Jani Nikula jani.nikula@linux.intel.com wrote:
On Tue, 14 Jun 2022, Imre Deak imre.deak@intel.com 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 jani.nikula@intel.com
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 lyude@redhat.com Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292 Signed-off-by: Imre Deak imre.deak@intel.com
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;
On Tue, Jun 14, 2022 at 03:32:59PM +0300, Jani Nikula wrote:
On Tue, 14 Jun 2022, Imre Deak imre.deak@intel.com 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 lyude@redhat.com Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292 Signed-off-by: Imre Deak imre.deak@intel.com
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
On Tue, 14 Jun 2022, Imre Deak imre.deak@intel.com wrote:
On Tue, Jun 14, 2022 at 03:32:59PM +0300, Jani Nikula wrote:
On Tue, 14 Jun 2022, Imre Deak imre.deak@intel.com 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 lyude@redhat.com Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292 Signed-off-by: Imre Deak imre.deak@intel.com
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
If you add a Cc: to stable@vger.kernel.org then this is:
Reviewed-by: Lyude Paul lyude@redhat.com
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 lyude@redhat.com Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5292 Signed-off-by: Imre Deak imre.deak@intel.com
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; }
dri-devel@lists.freedesktop.org
-
Imre Deak -
Jani Nikula -
Lyude Paul