Messages in this thread | | | Date | Fri, 16 Feb 2024 09:38:21 +0100 | From | neil.armstrong@linaro ... | Subject | Re: [PATCH] drm/dp: Don't attempt AUX transfers when eDP panels are not powered |
| |
+ Bjorn + Konrad + Johan
On 15/02/2024 18:08, Doug Anderson wrote: > Hi, > > On Thu, Feb 15, 2024 at 8:53 AM Neil Armstrong > <neil.armstrong@linaro.org> 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@intel.com> wrote: >>>> >>>> On Wed, 14 Feb 2024, Doug Anderson <dianders@chromium.org> wrote: >>>>> Hi, >>>>> >>>>> On Tue, Feb 13, 2024 at 10:25 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote: >>>>>> >>>>>> On Wed, Feb 14, 2024 at 2:23 PM Douglas Anderson <dianders@chromium.org> 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@chromium.org> >>>>>>> --- >>>>>> >>>>>> Reviewed-by: Hsin-Yi Wang <hsinyi@chromium.org> >>>>> >>>>> 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 ! > > Sounds good. I know Eizan (CCed, but also a ChromeOS person) was going > to poke at it a bit and seemed willing to provide a Tested-by. I'll > let him chime in. > > ...but perhaps some better evidence that it's not breaking hardware is > that we actually landed this into one (but not all) ChromeOS kernel > trees [1] and it's rolled out to at least "beta" channel without > anyone screaming. Normally we like things to land upstream before > picking, but in this case we needed to land another patch to gather > in-field data [2] that would have made the problem even worse. > > 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?
Bjorn, Konrad, Johan,
Could one of you somehow try this on the X13s or other Laptop using eDP ?
Thanks, Neil
> > [1] https://crrev.com/c/5277322 > [2] https://crrev.com/c/5277736
| |