Re: [PATCH] drm/msm/dp: move link_ready out of HPD event thread

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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.




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux