Reporting a port as connected if nothing is attached to them leads to any i2c transactions on this port trying to use an uninitialized i2c adapter, fix this.
Let's account for this case even if branch devices have no good reason to report a port as unplugged with their peer device type set to 'none'.
Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device case") References: https://gitlab.freedesktop.org/drm/intel/-/issues/2987 References: https://gitlab.freedesktop.org/drm/intel/-/issues/1963 Cc: Wayne Lin Wayne.Lin@amd.com Cc: Lyude Paul lyude@redhat.com Cc: stable@vger.kernel.org # v5.5+ Cc: intel-gfx@lists.freedesktop.org Signed-off-by: Imre Deak imre.deak@intel.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index e82b596d646c..deb7995f42fa 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4224,6 +4224,7 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
switch (port->pdt) { case DP_PEER_DEVICE_NONE: + break; case DP_PEER_DEVICE_MST_BRANCHING: if (!port->mcs) ret = connector_status_connected;
Caching EDIDs for physical ports prevents updating the EDID if a port gets reconnected via a Connection Status Notification message, fix this.
Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device case") Cc: Wayne Lin Wayne.Lin@amd.com Cc: Lyude Paul lyude@redhat.com Signed-off-by: Imre Deak imre.deak@intel.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index deb7995f42fa..309afe61afdd 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2302,7 +2302,8 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb, }
if (port->pdt != DP_PEER_DEVICE_NONE && - drm_dp_mst_is_end_device(port->pdt, port->mcs)) { + drm_dp_mst_is_end_device(port->pdt, port->mcs) && + port->port_num >= DP_MST_LOGICAL_PORT_0) { port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); drm_connector_set_tile_property(port->connector);
[AMD Public Use]
-----Original Message----- From: Imre Deak imre.deak@intel.com Sent: Monday, February 1, 2021 8:02 PM To: dri-devel@lists.freedesktop.org Cc: Lin, Wayne Wayne.Lin@amd.com; Lyude Paul lyude@redhat.com Subject: [PATCH 2/4] drm/dp_mst: Don't cache EDIDs for physical ports
Caching EDIDs for physical ports prevents updating the EDID if a port gets reconnected via a Connection Status Notification message, fix this.
Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device case") Cc: Wayne Lin Wayne.Lin@amd.com Cc: Lyude Paul lyude@redhat.com Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index deb7995f42fa..309afe61afdd 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2302,7 +2302,8 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb, }
if (port->pdt != DP_PEER_DEVICE_NONE &&
- drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
- drm_dp_mst_is_end_device(port->pdt, port->mcs) &&
- port->port_num >= DP_MST_LOGICAL_PORT_0) {
Hi Imre Deak,
Thanks for the patch! Just curious that you mean we don't want to fetch the EDID of the sst monitor like below case? Src->MST device ->SST monitor I thought we still need to get the EDID even the monitor is connected to the physical output port of mst device. Maybe what we should fix here is why the EDID is not get updated once reconnected via CSN message?
Thanks!
port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); drm_connector_set_tile_property(port->connector); -- 2.25.1
Regards, Wayne Lin
On Tue, Feb 02, 2021 at 03:38:16AM +0000, Lin, Wayne wrote:
[AMD Public Use]
-----Original Message----- From: Imre Deak imre.deak@intel.com Sent: Monday, February 1, 2021 8:02 PM To: dri-devel@lists.freedesktop.org Cc: Lin, Wayne Wayne.Lin@amd.com; Lyude Paul lyude@redhat.com Subject: [PATCH 2/4] drm/dp_mst: Don't cache EDIDs for physical ports
Caching EDIDs for physical ports prevents updating the EDID if a port gets reconnected via a Connection Status Notification message, fix this.
Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device case") Cc: Wayne Lin Wayne.Lin@amd.com Cc: Lyude Paul lyude@redhat.com Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index deb7995f42fa..309afe61afdd 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2302,7 +2302,8 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb, }
if (port->pdt != DP_PEER_DEVICE_NONE &&
- drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
- drm_dp_mst_is_end_device(port->pdt, port->mcs) &&
- port->port_num >= DP_MST_LOGICAL_PORT_0) {
Hi Imre Deak,
Thanks for the patch! Just curious that you mean we don't want to fetch the EDID of the sst monitor like below case?
Src->MST device ->SST monitor
The intention of the mst cached_edid logic is to cache the EDID for logical ports where the EDID cannot change anyway. The EDID on physical ports is fetched during connector probing just as for any other connector.
I thought we still need to get the EDID even the monitor is connected to the physical output port of mst device.
For sinks attached to phyisical ports we get the EDID whenever probing the corresponding connector.
Maybe what we should fix here is why the EDID is not get updated once reconnected via CSN message?
This patch fixes the problem that we stopped updating the EDID for physical connectors. After this change it will get updated when probing such connectors.
Thanks!
port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); drm_connector_set_tile_property(port->connector); -- 2.25.1
Regards, Wayne Lin
[AMD Official Use Only - Internal Distribution Only]
-----Original Message----- From: Imre Deak imre.deak@intel.com Sent: Tuesday, February 2, 2021 7:22 PM To: Lin, Wayne Wayne.Lin@amd.com Cc: dri-devel@lists.freedesktop.org; Lyude Paul lyude@redhat.com Subject: Re: [PATCH 2/4] drm/dp_mst: Don't cache EDIDs for physical ports
On Tue, Feb 02, 2021 at 03:38:16AM +0000, Lin, Wayne wrote:
[AMD Public Use]
-----Original Message----- From: Imre Deak imre.deak@intel.com Sent: Monday, February 1, 2021 8:02 PM To: dri-devel@lists.freedesktop.org Cc: Lin, Wayne Wayne.Lin@amd.com; Lyude Paul lyude@redhat.com Subject: [PATCH 2/4] drm/dp_mst: Don't cache EDIDs for physical ports
Caching EDIDs for physical ports prevents updating the EDID if a port gets reconnected via a Connection Status Notification message, fix this.
Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device case") Cc: Wayne Lin Wayne.Lin@amd.com Cc: Lyude Paul lyude@redhat.com Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index deb7995f42fa..309afe61afdd 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2302,7 +2302,8 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb, }
if (port->pdt != DP_PEER_DEVICE_NONE &&
- drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
- drm_dp_mst_is_end_device(port->pdt, port->mcs) &&
- port->port_num >= DP_MST_LOGICAL_PORT_0) {
Hi Imre Deak,
Thanks for the patch! Just curious that you mean we don't want to fetch the EDID of the sst monitor like below case?
Src->MST device ->SST monitorThe intention of the mst cached_edid logic is to cache the EDID for logical ports where the EDID cannot change anyway. The EDID on physical ports is fetched during connector probing just as for any other connector.
I thought we still need to get the EDID even the monitor is connected to the physical output port of mst device.
For sinks attached to phyisical ports we get the EDID whenever probing the corresponding connector.
Maybe what we should fix here is why the EDID is not get updated once reconnected via CSN message?
This patch fixes the problem that we stopped updating the EDID for physical connectors. After this change it will get updated when probing such connectors.
Appreciate for the explanation. Thanks!
Thanks!
port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); drm_connector_set_tile_property(port->connector); -- 2.25.1
Regards, Wayne Lin
Regards, Wayne Lin
On Mon, Feb 01, 2021 at 02:01:43PM +0200, Imre Deak wrote:
Caching EDIDs for physical ports prevents updating the EDID if a port gets reconnected via a Connection Status Notification message, fix this.
Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device case") Cc: Wayne Lin Wayne.Lin@amd.com Cc: Lyude Paul lyude@redhat.com Signed-off-by: Imre Deak imre.deak@intel.com
Pushed patches 2-4 to drm-misc-next, thanks for the review.
drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index deb7995f42fa..309afe61afdd 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2302,7 +2302,8 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb, }
if (port->pdt != DP_PEER_DEVICE_NONE &&
drm_dp_mst_is_end_device(port->pdt, port->mcs)) {
drm_dp_mst_is_end_device(port->pdt, port->mcs) &&port->port_num >= DP_MST_LOGICAL_PORT_0) {-- 2.25.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
drm_get_edid() already updates the tile property since commit 2de3a078497b ("drm/dp: Set the connector's TILE property even for DP SST connectors") so no need to update it after calling this function.
Cc: Lyude Paul lyude@redhat.com Signed-off-by: Imre Deak imre.deak@intel.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 309afe61afdd..43a40660136c 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -2303,11 +2303,9 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb,
if (port->pdt != DP_PEER_DEVICE_NONE && drm_dp_mst_is_end_device(port->pdt, port->mcs) && - port->port_num >= DP_MST_LOGICAL_PORT_0) { + port->port_num >= DP_MST_LOGICAL_PORT_0) port->cached_edid = drm_get_edid(port->connector, &port->aux.ddc); - drm_connector_set_tile_property(port->connector); - }
drm_connector_register(port->connector); return;
Use the macro defined for the first logical port instead of the corresponding magic number.
Cc: Lyude Paul lyude@redhat.com Signed-off-by: Imre Deak imre.deak@intel.com --- drivers/gpu/drm/drm_dp_mst_topology.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 43a40660136c..0a54506c2773 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4232,9 +4232,8 @@ drm_dp_mst_detect_port(struct drm_connector *connector, case DP_PEER_DEVICE_SST_SINK: ret = connector_status_connected; /* for logical ports - cache the EDID */ - if (port->port_num >= 8 && !port->cached_edid) { + if (port->port_num >= DP_MST_LOGICAL_PORT_0 && !port->cached_edid) port->cached_edid = drm_get_edid(connector, &port->aux.ddc); - } break; case DP_PEER_DEVICE_DP_LEGACY_CONV: if (port->ldps)
For the whole series:
Reviewed-by: Lyude Paul lyude@redhat.com
On Mon, 2021-02-01 at 14:01 +0200, Imre Deak wrote:
Reporting a port as connected if nothing is attached to them leads to any i2c transactions on this port trying to use an uninitialized i2c adapter, fix this.
Let's account for this case even if branch devices have no good reason to report a port as unplugged with their peer device type set to 'none'.
Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device case") References: https://gitlab.freedesktop.org/drm/intel/-/issues/2987 References: https://gitlab.freedesktop.org/drm/intel/-/issues/1963 Cc: Wayne Lin Wayne.Lin@amd.com Cc: Lyude Paul lyude@redhat.com Cc: stable@vger.kernel.org # v5.5+ Cc: intel-gfx@lists.freedesktop.org Signed-off-by: Imre Deak imre.deak@intel.com
drivers/gpu/drm/drm_dp_mst_topology.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index e82b596d646c..deb7995f42fa 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4224,6 +4224,7 @@ drm_dp_mst_detect_port(struct drm_connector *connector, switch (port->pdt) { case DP_PEER_DEVICE_NONE: + break; case DP_PEER_DEVICE_MST_BRANCHING: if (!port->mcs) ret = connector_status_connected;
On Mon, Feb 01, 2021 at 02:01:42PM +0200, Imre Deak wrote:
Reporting a port as connected if nothing is attached to them leads to any i2c transactions on this port trying to use an uninitialized i2c adapter, fix this.
Let's account for this case even if branch devices have no good reason to report a port as unplugged with their peer device type set to 'none'.
Fixes: db1a07956968 ("drm/dp_mst: Handle SST-only branch device case") References: https://gitlab.freedesktop.org/drm/intel/-/issues/2987 References: https://gitlab.freedesktop.org/drm/intel/-/issues/1963 Cc: Wayne Lin Wayne.Lin@amd.com Cc: Lyude Paul lyude@redhat.com Cc: stable@vger.kernel.org # v5.5+ Cc: intel-gfx@lists.freedesktop.org Signed-off-by: Imre Deak imre.deak@intel.com
Thanks for the report and review, I pushed this one patch to drm-misc-fixes.
I fixed a typo in the commit message.
drivers/gpu/drm/drm_dp_mst_topology.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index e82b596d646c..deb7995f42fa 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4224,6 +4224,7 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
switch (port->pdt) { case DP_PEER_DEVICE_NONE:
break;-- 2.25.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
dri-devel@lists.freedesktop.org
-
Imre Deak -
Lin, Wayne -
Lyude Paul