Hi Marek, Michael, On 14.09.2020 22:01, Michael Tretter wrote: > Hi, > > On Mon, 14 Sep 2020 14:31:19 +0200, Marek Szyprowski wrote: >> On 14.09.2020 10:29, Marek Szyprowski wrote: >>> On 11.09.2020 15:54, Michael Tretter wrote: >>>> Make the exynos_dsi driver a full drm bridge that can be found and used >>>> from other drivers. >>>> >>>> Other drivers can only attach to the bridge, if a mipi dsi device >>>> already attached to the bridge. This allows to defer the probe of the >>>> display pipe until the downstream bridges are available, too. >>>> >>>> Signed-off-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx> >>> This one (and the whole series applied) still fails on Exynos boards: >>> >>> [drm] Exynos DRM: using 11c00000.fimd device for DMA mapping operations >>> exynos-drm exynos-drm: bound 11c00000.fimd (ops fimd_component_ops) >>> OF: graph: no port node found in /soc/dsi@11c80000 >>> 8<--- cut here --- >>> Unable to handle kernel NULL pointer dereference at virtual address 00000084 >>> pgd = (ptrval) >>> [00000084] *pgd=00000000 >>> Internal error: Oops: 5 [#1] PREEMPT SMP ARM >>> Modules linked in: >>> CPU: 1 PID: 1 Comm: swapper/0 Not tainted >>> 5.9.0-rc4-next-20200911-00010-g417dc70d70ec #1608 >>> Hardware name: Samsung Exynos (Flattened Device Tree) >>> PC is at drm_bridge_attach+0x18/0x164 >>> LR is at exynos_dsi_bind+0x88/0xa8 >>> pc : [<c0628c08>] lr : [<c064d560>] psr: 20000013 >>> sp : ef0dfca8 ip : 00000002 fp : c13190e0 >>> r10: 00000000 r9 : ee46d580 r8 : c13190e0 >>> r7 : ee438800 r6 : 00000018 r5 : ef253810 r4 : ef39e840 >>> r3 : 00000000 r2 : 00000018 r1 : ef39e888 r0 : ef39e840 >>> Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none >>> Control: 10c5387d Table: 4000404a DAC: 00000051 >>> Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) >>> Stack: (0xef0dfca8 to 0xef0e0000) >>> ... >>> [<c0628c08>] (drm_bridge_attach) from [<c064d560>] >>> (exynos_dsi_bind+0x88/0xa8) >>> [<c064d560>] (exynos_dsi_bind) from [<c066a800>] >>> (component_bind_all+0xfc/0x290) >>> [<c066a800>] (component_bind_all) from [<c0649dc0>] >>> (exynos_drm_bind+0xe4/0x19c) >>> [<c0649dc0>] (exynos_drm_bind) from [<c066ad74>] >>> (try_to_bring_up_master+0x1e4/0x2c4) >>> [<c066ad74>] (try_to_bring_up_master) from [<c066b2b4>] >>> (component_master_add_with_match+0xd4/0x108) >>> [<c066b2b4>] (component_master_add_with_match) from [<c0649ae8>] >>> (exynos_drm_platform_probe+0xe4/0x110) >>> [<c0649ae8>] (exynos_drm_platform_probe) from [<c0674e6c>] >>> (platform_drv_probe+0x6c/0xa4) >>> [<c0674e6c>] (platform_drv_probe) from [<c067242c>] >>> (really_probe+0x200/0x4fc) >>> [<c067242c>] (really_probe) from [<c06728f0>] >>> (driver_probe_device+0x78/0x1fc) >>> [<c06728f0>] (driver_probe_device) from [<c0672cd8>] >>> (device_driver_attach+0x58/0x60) >>> [<c0672cd8>] (device_driver_attach) from [<c0672dbc>] >>> (__driver_attach+0xdc/0x174) >>> [<c0672dbc>] (__driver_attach) from [<c06701b4>] >>> (bus_for_each_dev+0x68/0xb4) >>> [<c06701b4>] (bus_for_each_dev) from [<c06714e8>] >>> (bus_add_driver+0x158/0x214) >>> [<c06714e8>] (bus_add_driver) from [<c0673c1c>] (driver_register+0x78/0x110) >>> [<c0673c1c>] (driver_register) from [<c0649ca8>] >>> (exynos_drm_init+0xe4/0x118) >>> [<c0649ca8>] (exynos_drm_init) from [<c0102484>] >>> (do_one_initcall+0x8c/0x42c) >>> [<c0102484>] (do_one_initcall) from [<c11011c0>] >>> (kernel_init_freeable+0x190/0x1dc) >>> [<c11011c0>] (kernel_init_freeable) from [<c0af7880>] >>> (kernel_init+0x8/0x118) >>> [<c0af7880>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20) >>> Exception stack(0xef0dffb0 to 0xef0dfff8) >>> ... >>> ---[ end trace ee27f313f9ed9da1 ]--- >>> >>> # arm-linux-gnueabi-addr2line -e vmlinux c0628c08 >>> drivers/gpu/drm/drm_bridge.c:184 (discriminator 1) >>> >>> I will try to debug it a bit more today. >> The above crash has been caused by lack of in_bridge initialization to >> NULL in exynos_dsi_bind() in this patch. However, fixing it reveals >> another issue: >> >> [drm] Exynos DRM: using 11c00000.fimd device for DMA mapping operations >> exynos-drm exynos-drm: bound 11c00000.fimd (ops fimd_component_ops) >> OF: graph: no port node found in /soc/dsi@11c80000 >> 8<--- cut here --- >> Unable to handle kernel NULL pointer dereference at virtual address 00000280 >> pgd = (ptrval) >> [00000280] *pgd=00000000 >> Internal error: Oops: 5 [#1] PREEMPT SMP ARM >> Modules linked in: >> CPU: 0 PID: 1 Comm: swapper/0 Not tainted >> 5.9.0-rc4-next-20200911-00010-g417dc70d70ec-dirty #1613 >> Hardware name: Samsung Exynos (Flattened Device Tree) >> PC is at __mutex_lock+0x54/0xb18 >> LR is at lock_is_held_type+0x80/0x138 >> pc : [<c0afc920>] lr : [<c0af63e8>] psr: 60000013 >> sp : ef0dfd30 ip : 33937b74 fp : c13193c8 >> r10: c1208eec r9 : 00000000 r8 : ee45f808 >> r7 : c19561a4 r6 : 00000000 r5 : 00000000 r4 : 0000024c >> r3 : 00000000 r2 : 00204140 r1 : c124f13c r0 : 00000000 >> Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none >> Control: 10c5387d Table: 4000404a DAC: 00000051 >> Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) >> Stack: (0xef0dfd30 to 0xef0e0000) >> ... >> [<c0afc920>] (__mutex_lock) from [<c0afd400>] (mutex_lock_nested+0x1c/0x24) >> [<c0afd400>] (mutex_lock_nested) from [<c064d4b8>] >> (__exynos_dsi_host_attach+0x20/0x6c) >> [<c064d4b8>] (__exynos_dsi_host_attach) from [<c064d914>] >> (exynos_dsi_host_attach+0x70/0x194) >> [<c064d914>] (exynos_dsi_host_attach) from [<c0656b64>] >> (s6e8aa0_probe+0x1b0/0x218) >> [<c0656b64>] (s6e8aa0_probe) from [<c0672530>] (really_probe+0x200/0x4fc) >> [<c0672530>] (really_probe) from [<c06729f4>] >> (driver_probe_device+0x78/0x1fc) >> [<c06729f4>] (driver_probe_device) from [<c0672ddc>] >> (device_driver_attach+0x58/0x60) >> [<c0672ddc>] (device_driver_attach) from [<c0672ec0>] >> (__driver_attach+0xdc/0x174) >> [<c0672ec0>] (__driver_attach) from [<c06702b8>] >> (bus_for_each_dev+0x68/0xb4) >> [<c06702b8>] (bus_for_each_dev) from [<c06715ec>] >> (bus_add_driver+0x158/0x214) >> [<c06715ec>] (bus_add_driver) from [<c0673d20>] (driver_register+0x78/0x110) >> [<c0673d20>] (driver_register) from [<c0102484>] >> (do_one_initcall+0x8c/0x42c) >> [<c0102484>] (do_one_initcall) from [<c11011c0>] >> (kernel_init_freeable+0x190/0x1dc) >> [<c11011c0>] (kernel_init_freeable) from [<c0af7988>] >> (kernel_init+0x8/0x118) >> [<c0af7988>] (kernel_init) from [<c0100114>] (ret_from_fork+0x14/0x20) >> Exception stack(0xef0dffb0 to 0xef0dfff8) >> ... >> ---[ end trace c06e996ec2e8234d ]--- >> >> This means that dsi->encoder.dev is not initialized in >> __exynos_dsi_host_attach(). >> >> This happens, because drm_bridge_attach() in exynos_dsi_bind() returned >> earlier -517 (deferred probe), what causes cleanup of encoder and >> release of all drm resources. >> >> Then however, the panel tries to register itself and >> exynos_dsi_host_attach() tries to access the released encoder (which is >> zeroed in drm_encoder_release) and rest of resources, what causes failure. >> >> It looks that something is missing. Maybe mipi host has to be registered >> later, when bridge is ready? I have no idea how it is handled before >> this patch. Andrzej, could you comment it a bit? > I intentionally changed the order, because if another bridge follows in the > pipeline, the probe of the drm driver has to be deferred until some bridge > provides a connector. The next bridge registers itself via the host_attach > function and the deferral is ensured via the bind for the bind/unbind API or > the bridge_attach function otherwise. > > On the other hand, the bridge does not have an encoder until the mipi device > has been attached. > > As a solution, the exynos dsi driver must initialize the encoder in > exynos_dsi_probe instead of in exynos_dsi_bind and access the encoder via > exynos_dsi instead of the bridge. > > Can you try to move everything except samsung_dsim_bind from exynos_dsi_bind > to exynos_dsi_probe (respectively for unbind) and report if it fixes the > crash. The original behaviour is that encoder (exynos_dsi) is registered regardless of sink presence (initially panel, later also bridge) - it avoids multiple issues with deferred probe, device driver bind/unbind and module load/unload. Appearance or disappearance of sink is reported to host nicely via DSI attach/detach callbacks - and it is reflected in drm world as change state of the connector. Registering DSI host in bind and unregistering in unbind assures that if mipi_dsi device is attached/detached the drm device is always present - it makes device/driver binding race free and allows to avoid additional locking. Moving DSI host registration to probe changes everything, for sure it breaks the nice feature of DSI attach/detach callbacks and apparently can cause different issues depending on device bind order. I will try to look at the patches tomorrow and maybe I can find more constructive comments :) Regards Andrzej > > Michael > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://protect2.fireeye.com/v1/url?k=4f0be936-129547ac-4f0a6279-0cc47a336fae-e9aecfc5418740e8&q=1&e=1d4b0871-5b85-47f3-9506-79c768643aee&u=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel
