|
[AMD Official Use Only - Internal Distribution Only]
I took a closer look at this and there seems to be an issue in the function.
Crtc being null is a valid case here. The sequence that leads to this is, unplug -> disable crtc/release vcpi slots then hotplug. The issue is that pos->port is not guaranteed to be released in
drm_dp_atomic_release_vcpi_slots() so list_for_each_entry(pos, &mst_state->vcpis, next) {}
might still iterate thought it.
So, when a hotplug is done we still loop through the old port which has port! = null, crtc = null, and vpci = 0. I didn't find anything that I can use to remove the port from the list. So, a potential solution to this would be to add a check for vpci
= 0 and skip that port.
Thoughts/Suggestions?
Bhawan
From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> on behalf of Lakha, Bhawanpreet <Bhawanpreet.Lakha@xxxxxxx>
Sent: August 14, 2020 2:52 PM To: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>; Lipski, Mikita <Mikita.Lipski@xxxxxxx>; Kazlauskas, Nicholas <Nicholas.Kazlauskas@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx> Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx <dri-devel@xxxxxxxxxxxxxxxxxxxxx> Subject: Re: [PATCH] drm/dp_mst: Don't return error code when crtc is null pos->port->connector?
This is checking the crtc not the connector. The crtc can be null if its disabled.
Since it is happening after a unplug->hotplug, I guess we are missing something in the disable sequence and the old connector is still in the list.
Bhawan
>>-----Original Message-----
>>From: dri-devel <dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
>>Bhawanpreet Lakha
>>Sent: Friday, August 14, 2020 1:02 PM
>>To: mikita.lipski@xxxxxxx; nicholas.kazlauskas@xxxxxxx;
>>alexander.deucher@xxxxxxx
>>Cc: Bhawanpreet Lakha <Bhawanpreet.Lakha@xxxxxxx>; dri-
>>devel@xxxxxxxxxxxxxxxxxxxxx; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>>Subject: [PATCH] drm/dp_mst: Don't return error code when crtc is null
>>
>>[Why]
>>In certain cases the crtc can be NULL and returning -EINVAL causes
>>atomic check to fail when it shouln't. This leads to valid
>>configurations failing because atomic check fails.
>
>So is this a bug fix or an exception case, or an expected possibility?
>
>From my reading of the function comments, it is not clear that pos->port->connector
>might be NULL for some reason.
>A better explanation of why this would occur would make this a much more
>useful commit message.
>
>My reading is that you ran into this issue an are masking it with this fix.
>
>Rather than this is a real possibility and this is the correct fix.
>
>Mike
>
>>[How]
>>Don't early return if crtc is null
>>
>>Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@xxxxxxx>
>>---
>> drivers/gpu/drm/drm_dp_mst_topology.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
>>b/drivers/gpu/drm/drm_dp_mst_topology.c
>>index 70c4b7afed12..bc90a1485699 100644
>>--- a/drivers/gpu/drm/drm_dp_mst_topology.c
>>+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
>>@@ -5037,8 +5037,8 @@ int drm_dp_mst_add_affected_dsc_crtcs(struct
>>drm_atomic_state *state, struct drm
>>
>> crtc = conn_state->crtc;
>>
>>- if (WARN_ON(!crtc))
>>- return -EINVAL;
>>+ if (!crtc)
>>+ continue;
>>
>> if (!drm_dp_mst_dsc_aux_for_port(pos->port))
>> continue;
>>--
>>2.17.1
>>
>>_______________________________________________
>>dri-devel mailing list
>>dri-devel@xxxxxxxxxxxxxxxxxxxxx
>>https://nam11.safelinks.protection.outlook.com/?url=""
|
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
