Message-ID: <CAKXuJqiXteBsrFF1q-dziaUvFYXXTk98Q2y7ZcDit0VxONsNmQ@mail.gmail.com>
Date: Mon, 26 Feb 2024 15:49:03 -0600
From: Steev Klimaszewski <steev@...i.org>
To: Doug Anderson <dianders@...omium.org>
Cc: Javier Martinez Canillas <javierm@...hat.com>, 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>
Subject: Re: [PATCH] drm/dp: Don't attempt AUX transfers when eDP panels are
 not powered

On Fri, Feb 16, 2024 at 9:30 AM Doug Anderson <dianders@...omium.org> wrote:
>
> 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

Tested on my Thinkpad X13s, with display on, I get the did when
hexdumping /dev/drm_dp_aux2, with display off I get device/resource
busy.
Tested-by: Steev Klimaszewski <steev@...i.org>

Powered by blists - more mailing lists