Message-ID: <CAD=FV=WwfB=u2DVE0ux9cD3OCCccgcWCdJBxW2-9gg4qSFSv+A@mail.gmail.com>
Date: Fri, 16 Feb 2024 07:30:03 -0800
From: Doug Anderson <dianders@...omium.org>
To: Javier Martinez Canillas <javierm@...hat.com>
Cc: neil.armstrong@...aro.org, Jani Nikula <jani.nikula@...el.com>, 
	Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Hsin-Yi Wang <hsinyi@...omium.org>, 
	dri-devel@...ts.freedesktop.org, eizan@...omium.org, 
	Ankit Nautiyal <ankit.k.nautiyal@...el.com>, Daniel Vetter <daniel@...ll.ch>, 
	David Airlie <airlied@...il.com>, Heiner Kallweit <hkallweit1@...il.com>, 
	Imre Deak <imre.deak@...el.com>, Jessica Zhang <quic_jesszhan@...cinc.com>, 
	Sam Ravnborg <sam@...nborg.org>, Stanislav Lisovskiy <stanislav.lisovskiy@...el.com>, 
	linux-kernel@...r.kernel.org, Bjorn Andersson <andersson@...nel.org>, 
	Konrad Dybcio <konrad.dybcio@...aro.org>, Johan Hovold <johan+linaro@...nel.org>, 
	Steev Klimaszewski <steev@...i.org>
Subject: Re: [PATCH] drm/dp: Don't attempt AUX transfers when eDP panels are
 not powered

Hi,

On Fri, Feb 16, 2024 at 12:21 AM Javier Martinez Canillas
<javierm@...hat.com> wrote:
>
> > The kernel tree we landed on was the v5.15 tree, which is currently
> > serving all Qualcomm sc7180-based Chromebooks, all Mediatek 8173
> > Chromebooks and all Mediatek 8186 Chromebooks. There are also a pile
> > of x86 Chromebooks running our v5.15 kernel. This code shouldn't
> > affect them because (unless I'm mistaken) they don't use the two
> > affected panel drivers. In any case, I haven't heard any screams from
> > them either. Given my landing plans of "the week of the 26th", this
> > still gives another 1.5 weeks for any screams to reach my ears.
> >
> > ...or are you looking for non-ChromeOS test reports? I'm not sure how
> > to encourage those. I suppose sometimes folks at Red Hat end up
> > stumbling over similar panel problems to those of us in ChromeOS.
> > Maybe +Javier would be interested in providing a Tested-by?
> >
>
> I do have a SC7180 based HP X2 Chromebook and could test your patch (not
> today but I could do it early next week), although I haven't followed so
> if you could please let me know what exactly do you want me to verify.
>
> AFAIU the problem is that the fwupd daemon tries to scan DP busses even if
> the panel is turned off and that's what takes a lot of time (due retries),
> and your patch makes the driver to bail out immediately ? If that's the
> case, I guess that just starting fwupd daemon with an without your patch
> while the panel is turned off could be a good test ?

Sorry! I wasn't trying to sign you up for extra work. I'm not
convinced that any extra verification on a Chromebook is all that
valuable since that's pretty covered by the fact that we've already
pushed this patch out to real users on Chromebooks. I think Neil was
hoping for some confirmation that my patch didn't break someone else's
hardware. I think maybe good enough is if you have some type of
hardware that uses eDP and that you could verify that my patch does
break anything about it?

I'm not aware of anyone with extensive DP AUX character device usage.
I guess I thought of Javier because I remembered him at least also
using fwupd and some of the fwupd plugins try to talk to DP things
over the DP AUX character device.

If someone is really in a testing mood and wanted to stress the char
device, I guess something simple like "hexdump -C /dev/drm_dp_aux*"
for whatever eDP AUX is associated with eDP would at least do some
reading. You could stress turning the display on and off while doing
that with and without my patch. Presumably it will be better (error
out more quickly) with my patch.

If you wanted to stress the i2c path, you could do something like this
(the grep assumes you're using ti-sn65dsi86 as your eDP bridge chip,
but hopefully easy to adjust):

bus=$(i2cdetect -l | grep sn65 | sed 's/i2c-\([0-9]*\).*$/\1/')
i2cdump ${bus} 0x50 i

That should dump your EDID. Again it should error out quickly when the
panel is off after my patch but should start working again when the
panel is on.


Hmmm, thinking about all the above, I guess there is one case that
_could_ be broken by my patch. I really hope not, though. If someone
has a panel that's on an always-on rail or on a shared rail with some
other device (like the touchscreen) that's keeping the panel power on
then without my patch it would be possible to do DP AUX transactions
even when the panel was "off" from Linux's point of view. It would
have worked mostly due to luck, but now luck will run out and it will
stop working. I really hope nobody has userspace that is relying on
this, but I suppose it's always possible that somewhere, someone's
userspace is. If you are or know of someone who is then please shout.

-Doug

Powered by blists - more mailing lists