lkml.org 
[lkml]   [2024]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] drm/msm/dp: move link_ready out of HPD event thread
From


On 3/12/2024 9:59 AM, Johan Hovold wrote:
> On Tue, Mar 12, 2024 at 05:41:23PM +0100, Johan Hovold wrote:
>> On Tue, Mar 12, 2024 at 11:09:11AM +0100, Johan Hovold wrote:
>>> On Fri, Mar 08, 2024 at 01:45:32PM -0800, Abhinav Kumar wrote:
>>
>>>> @@ -466,6 +466,8 @@ static int dp_display_notify_disconnect(struct device *dev)
>>>> {
>>>> struct dp_display_private *dp = dev_get_dp_display_private(dev);
>>>>
>>>> + dp->dp_display.link_ready = false;
>>>
>>> As I also pointed out in the other thread, setting link_ready to false
>>> here means that any spurious connect event (during physical disconnect)
>>> will always be processed, something which can currently lead to a leaked
>>> runtime pm reference.
>>>
>>> Wasting some power is of course preferred over crashing the machine, but
>>> please take it into consideration anyway.
>>>
>>> Especially if your intention with this patch was to address the resets
>>> we saw with sc8280xp which are gone since the HPD notify revert (which
>>> fixed the hotplug detect issue that left the bridge in a
>>> half-initialised state).
>>
>> Heh. This is getting ridiculous. I just tried running with this patch
>> and it again breaks hotplug detect in a VT console and in X (where I
>> could enable a reconnected external display by running xrandr twice
>> before).
>>
>> So, please, do not apply this one.
>
> To make things worse, I indeed also hit the reset when disconnecting
> after such a failed hotplug.
>
> Johan

Ack, I will hold off till I analyze your issues more which you have
listed in separate replies. Especially about the spurious connect, I
believe you are trying to mention that, by adding logs, you are able to
delay the processing of a connect event to *make* it like a spurious
one? In case, I got this part wrong, can you pls explain the spurious
connect scenario again?

A short response on why this change was made is that commit can be
issued by userspace or the fbdev client. So userspace involvement only
makes commit happen from a different path. It would be incorrect to
assume the issues from the earlier bug and the current one are different
only because there was userspace involvement in that one and not this.

Because in the end, it manifests itself in the same way that
atomic_enable() did not go through after an atomic_disable() and the
next atomic_disable() crashes.

Reverting the hpd_notify patch only eliminated some paths but I think
both you and me agree the issue can still happen and in the very same
way. So till someone else reports this issue, till HPD is reworked, I
wanted to do whats possible to avoid this situation. If users are fine
with what we have, I have no inclination to push this.


\
 
 \ /
  Last update: 2024-05-27 15:45    [W:0.089 / U:0.632 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site