Re: drm/msm: Second DisplayPort regression in 6.8-rc1

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

 



Hi Johan

On 2/19/2024 2:41 AM, Johan Hovold wrote:
On Sat, Feb 17, 2024 at 04:14:58PM +0100, Johan Hovold wrote:
On Wed, Feb 14, 2024 at 02:52:06PM +0100, Johan Hovold wrote:
On Tue, Feb 13, 2024 at 10:00:13AM -0800, Abhinav Kumar wrote:

Since Dmitry had trouble reproducing this issue I took a closer look at
the DRM aux bridge series that Abhinav pointed and was able to track
down the bridge regressions and come up with a reproducer. I just posted
a series fixing this here:

	https://lore.kernel.org/lkml/20240217150228.5788-1-johan+linaro@xxxxxxxxxx/

As I mentioned in the cover letter, I am still seeing intermittent hard
resets around the time that the DRM subsystem is initialising, which
suggests that we may be dealing with two separate DRM regressions here
however.

If the hard resets are triggered by something like unclocked hardware,
perhaps that bit could this be related to the runtime PM rework?

It seems my initial suspicion that at least some of these regressions
were related to the runtime PM work was correct. The hard resets happens
when the DP controller is runtime suspended after being probed:

[   16.748475] bus: 'platform': __driver_probe_device: matched device ae00000.display-subsystem with driver msm-mdss
[   16.759444] msm-mdss ae00000.display-subsystem: Adding to iommu group 21
[   16.795226] bus: 'platform': __driver_probe_device: matched device ae01000.display-controller with driver msm_dpu
[   16.807542] probe of ae01000.display-controller returned -517 after 3 usecs
[   16.821552] bus: 'platform': __driver_probe_device: matched device ae90000.displayport-controller with driver msm-dp-display
[   16.837749] probe of ae90000.displayport-controller returned -517 after 1 usecs
[  OK  ] Listening on Load/Save RF Kill Swit[   16.854659] bus: 'platform': __dch Status /dev/rfkill Watch.
[   16.868458] probe of ae98000.displayport-controller returned -517 after 2 usecs
[   16.880012] bus: 'platform': __driver_probe_device: matched device aea0000.displayport-controller with driver msm-dp-display
[   16.891856] probe of aea0000.displayport-controller returned -517 after 2 usecs
[   16.903825] probe of ae00000.display-subsystem returned 0 after 144497 usecs
[   16.911636] bus: 'platform': __driver_probe_device: matched device ae01000.display-controller with driver msm_dpu
[   16.942092] probe of ae01000.display-controller returned 0 after 19593 usecs
          Starting Load/Save Screen Backligh…rightness[   16.959146] bus: 'platform': _ of backlight:backlight...
[   16.995355] msm-dp-display ae90000.displayport-controller: dp_display_probe - probe tail
[   17.004032] probe of ae90000.displayport-controller returned 0 after 30225 usecs
[   17.012308] bus: 'platform': __driver_probe_device: matched device ae98000.displayport-controller with driver msm-dp-display
[   17.050193] msm-dp-display ae98000.displayport-controller: dp_display_probe - probe tail
          Starting Network Name Resolution...
[   17.058925] probe of ae98000.displayport-controller returned 0 after 34774 usecs
[   17.074925] bus: 'platform': __driver_probe_device: matched device aea0000.displayport-controller with driver msm-dp-display
[        Starting Network Time Synchronization...
[   17.112000] msm-dp-display aea0000.displayport-controller: dp_display_probe - populate aux bus
[   17.125208] msm-dp-display aea0000.displayport-controller: dp_pm_runtime_resume
          Starting Record System Boot/Shutdown in UTMP...
          Starting Virtual Console Setup...
[  OK  ] Finished Load/Save Screen Backlight Brightness of backlight:backlight.
[   17.197909] msm-dp-display aea0000.displayport-controller: dp_pm_runtime_suspend
[   17.198079] probe of aea0Format: Log Type - Time(microsec) - Message - Optional Info
Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
S - QC_IMAGE_VERSION_STRING=BOOT.MXF.1.1-00470-MAKENA-1
S - IMAGE_VARIANT_STRING=SocMakenaWP
S - OEM_IMAGE_VERSION_STRING=crm-ubuntu92

   < machine is reset by hypervisor >

Presumably the reset happens when controller is being shut down while
still being used by the EFI framebuffer.


I am not sure if we can conclude like that. Even if we shut off the controller when the framebuffer was still being fetched that should only cause a blank screen and not a reset because we really don't trigger a new register write / read while its fetching so as such there is no new hardware access.

One thing I must accept is that there are two differences between sc8280xp where we are hitting these resets and sc7180/sc7280 chromebooks where we tested it more thoroughly without any such issues:

1) with the chromebooks we have depthcharge and not the QC UEFI.

If we are suspecting a hand-off issue here, will it help if we try to disable the display in EFI by using "fastboot oem select-display-panel none" (assuming this is a fastboot enabled device) and see if you still hit the reset issue?

2) chromebooks used "internal_hpd" whereas the pmic_glink method used in the sc8280xp.

I am still checking if there are any code paths in the eDP/DP driver left exposed due to this difference with pm_runtime which can cause this. I am wondering if some sort of drm tracing will help to narrow down the reset point.

In the cases where the machines survives boot, the controller is never
suspended.

When investigating this I've also seen intermittent:

	[drm:dp_display_probe [msm]] *ERROR* device tree parsing failed


So this error I think is because in dp_parser_parse() ---> dp_parser_ctrl_res(), we also have a devm_phy_get().

This can return -EDEFER if the phy driver has not yet probed.

I checked the other things inside dp_parser_parse(), others calls seem to be purely DT parsing except this one. I think to avoid the confusion, we should move devm_phy_get() outside of DT parsing into a separate call or atleast add an error log inside devm_phy_get() failure below to indicate that it deferred

        io->phy = devm_phy_get(&pdev->dev, "dp");
        if (IS_ERR(io->phy))
                return PTR_ERR(io->phy);

If my hypothesis is correct on this, then this error log (even though misleading) should be harmless for this issue because if we hit DRM_ERROR("device tree parsing failed\n"); we will skip the devm_pm_runtime_enable().

which also appears to be related to the runtime PM rework:

	https://lore.kernel.org/lkml/1701472789-25951-1-git-send-email-quic_khsieh@xxxxxxxxxxx/

I believe this is enough evidence to conclude that this second
regression is introduced by commit 5814b8bf086a ("drm/msm/dp:
incorporate pm_runtime framework into DP driver"):

#regzbot introduced: 5814b8bf086a

Has anyone given some thought to how the framebuffer handover is
supposed to work? It seems we're currently just relying on luck with
timing.



Johan




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux