On 4/8/2024 2:12 PM, Dmitry Baryshkov wrote:
On Mon, 8 Apr 2024 at 22:43, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote:
On 4/7/2024 11:48 AM, Bjorn Andersson wrote:
On Fri, Apr 05, 2024 at 08:15:47PM -0700, Abhinav Kumar wrote:
From: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
[..]
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index d80f89581760..bfb6dfff27e8 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1665,7 +1665,7 @@ void dp_bridge_hpd_notify(struct drm_bridge *bridge,
return;
if (!dp_display->link_ready && status == connector_status_connected)
- dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0);
+ dp_hpd_plug_handle(dp, 0);
If I read the code correctly, and we get an external connect event
inbetween a previous disconnect and the related disable call, this
should result in a PLUG_INT being injected into the queue still.
Will that not cause the same problem?
Regards,
Bjorn
Yes, your observation is correct and I had asked the same question to
kuogee before taking over this change and posting.
Should it then have the Co-developed-by trailers?
hmmm, perhaps but that didnt result in any code change between v2 and
v3, so I didnt add it.
We will have to handle that case separately. I don't have a good
solution yet for it without requiring further rework or we drop the
below snippet.
if (state == ST_DISCONNECT_PENDING) {
/* wait until ST_DISCONNECTED */
dp_add_event(dp, EV_HPD_PLUG_INT, 0, 1); /* delay = 1 */
mutex_unlock(&dp->event_mutex);
return 0;
}
I will need sometime to address that use-case as I need to see if we can
handle that better and then drop the the DISCONNECT_PENDING state to
address this fully. But it needs more testing.
But, we will need this patch anyway because without this we will not be
able to fix even the most regular and commonly seen case of basic
connect/disconnect receiving complementary events.
Hmm, no. We need to drop the HPD state machine, not to patch it. Once
the driver has proper detect() callback, there will be no
complementary events. That is a proper way to fix the code, not these
kinds of band-aids patches.
I had discussed this part too :)
I totally agree we should fix .detect()'s behavior to just match cable
connect/disconnect and not link_ready status.
But that alone would not have fixed this issue. If HPD thread does not
get scheduled and plug_handle() was not executed, .detect() would have
still returned old status as we will update the cable status only in
plug_handle() / unplug_handle() to have a common API between internal
and external hpd execution.
So we need to do both, make .detect() return correct status AND drop hpd
event thread processing.
But, dropping the hpd event thread processing alone was fixing the
complimentary events issue. So kuogee had only this part in this patch.
else if (dp_display->link_ready && status == connector_status_disconnected)
- dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0);
+ dp_hpd_unplug_handle(dp, 0);
}
--
2.43.2
