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.
