Re: [PATCH] drm/dp: Don't attempt AUX transfers when eDP panels are not powered

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

 



On 28/02/2024 17:40, Doug Anderson wrote:
Neil,

On Thu, Feb 15, 2024 at 8:53 AM Neil Armstrong
<neil.armstrong@xxxxxxxxxx> wrote:

Hi Doug,

On 15/02/2024 16:08, Doug Anderson wrote:
Hi,

On Thu, Feb 15, 2024 at 2:24 AM Jani Nikula <jani.nikula@xxxxxxxxx> wrote:

On Wed, 14 Feb 2024, Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
Hi,

On Tue, Feb 13, 2024 at 10:25 PM Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> wrote:

On Wed, Feb 14, 2024 at 2:23 PM Douglas Anderson <dianders@xxxxxxxxxxxx> wrote:

If an eDP panel is not powered on then any attempts to talk to it over
the DP AUX channel will timeout. Unfortunately these attempts may be
quite slow. Userspace can initiate these attempts either via a
/dev/drm_dp_auxN device or via the created i2c device.

Making the DP AUX drivers timeout faster is a difficult proposition.
In theory we could just poll the panel's HPD line in the AUX transfer
function and immediately return an error there. However, this is
easier said than done. For one thing, there's no hard requirement to
hook the HPD line up for eDP panels and it's OK to just delay a fixed
amount. For another thing, the HPD line may not be fast to probe. On
parade-ps8640 we need to wait for the bridge chip's firmware to boot
before we can get the HPD line and this is a slow process.

The fact that the transfers are taking so long to timeout is causing
real problems. The open source fwupd daemon sometimes scans DP busses
looking for devices whose firmware need updating. If it happens to
scan while a panel is turned off this scan can take a long time. The
fwupd daemon could try to be smarter and only scan when eDP panels are
turned on, but we can also improve the behavior in the kernel.

Let's let eDP panels drivers specify that a panel is turned off and
then modify the common AUX transfer code not to attempt a transfer in
this case.

Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
---

Reviewed-by: Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx>

Thanks for the review!

Given that this touches core DRM code and that I never got
confirmation that Jani's concerns were addressed with my previous
response, I'm still going to wait a little while before applying. I'm
on vacation for most of next week, but if there are no further replies
between now and then I'll plan to apply this to "drm-misc-next" the
week of Feb 26th. If someone else wants to apply this before I do then
I certainly won't object. Jani: if you feel this needs more discussion
or otherwise object to this patch landing then please yell. Likewise
if anyone else in the community wants to throw in their opinion, feel
free.

Sorry for dropping the ball after my initial response. I simply have not
had the time to look into this.

It would be great to get, say, drm-misc maintainer ack on this before
merging. It's not fair for me to stall this any longer, I'll trust their
judgement.

Reasonable?

I'd be more than happy for one of the drm-misc maintainers to Ack.
I'll move Maxime, Thomas, and Maarten to the "To:" line to see if that
helps get through their filters.

I'll like some test reports to be sure it doesn't break anything,
then I'll be happy to give my ack !

Are you looking for any more test reports at this point? Eizan did
some testing and provided a tag, though this was also on ChromeOS.
Steev also tested on two non-ChromeOS environments and provided his
tag. It's also been another two weeks of this being rolled out to some
Chromebook users and I haven't heard any reports of problems. If
somehow something was missed, I'm happy to follow-up and provide
additional fixes if some report comes in later.

Sure, thx I think you can apply it now

Acked-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>

Thanks,
Neil


Thanks!

-Doug




[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