Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus
From: Michael Walle
Date: Fri Aug 25 2023 - 11:44:17 EST
Hi Nicolas,
> For the eDP case we can support using aux-bus on MediaTek DP: this
> gives us the possibility to declare our panel as generic "panel-edp"
> which will automatically configure the timings and available modes
> via the EDID that we read from it.
>
> To do this, move the panel parsing at the end of the probe function
> so that the hardware is initialized beforehand and also initialize
> the DPTX AUX block and power both on as, when we populate the
> aux-bus, the panel driver will trigger an EDID read to perform
> panel detection.
>
> Last but not least, since now the AUX transfers can happen in the
> separated aux-bus, it was necessary to add an exclusion for the
> cable_plugged_in check in `mtk_dp_aux_transfer()` and the easiest
> way to do this is to simply ignore checking that when the bridge
> type is eDP.
This patch breaks my board based on the MT8195 which only has one
DisplayPort output port. I suspect it might also break the
mt8195-cherry
board.
Do you mean that your board does not have an internal display, only the
one
output port? If so, why are you enabling the nodes for the internal
display path
in your board specific DT?
Well, that depends. The board actually has an eDP socket, but because we
are an OEM, there might be no display connected to it. (And I haven't
tried it yet). It should probably go into an own device tree or overlay
if it is used. I agree. But it looked like it was auto-detectable (it
even has a HDP pin on the eDP socket, not sure about its use case..)
But the real reason I've enabled it was because I'll get an kernel
oops otherwise. I thought it might be some quirk that you'll need both,
because eDP will register even if theres no display - as you've
mentioned below.
Here's the splat:
[ 3.237064] mediatek-drm mediatek-drm.10.auto: bound
1c110000.vpp-merge (ops mtk_disp_merge_component_ops)
[ 3.238274] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0
because component 8 is disabled or missing
[ 3.239504] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0
because component 9 is disabled or missing
[ 3.240741] Unable to handle kernel NULL pointer dereference at
virtual address 00000000000004a0
[ 3.241841] Mem abort info:
[ 3.242192] ESR = 0x0000000096000004
[ 3.242662] EC = 0x25: DABT (current EL), IL = 32 bits
[ 3.243328] SET = 0, FnV = 0
[ 3.243710] EA = 0, S1PTW = 0
[ 3.244104] FSC = 0x04: level 0 translation fault
[ 3.244717] Data abort info:
[ 3.245078] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[ 3.245765] CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[ 3.246398] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[ 3.247063] [00000000000004a0] user address but active_mm is swapper
[ 3.247860] Internal error: Oops: 0000000096000004 [#1] SMP
[ 3.248559] Modules linked in:
[ 3.248945] CPU: 4 PID: 11 Comm: kworker/u16:0 Not tainted
6.5.0-rc7-next-20230821+ #2225
[ 3.249970] Hardware name: Kontron 3.5"-SBC-i1200 (DT)
[ 3.250614] Workqueue: events_unbound deferred_probe_work_func
[ 3.251347] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS
BTYPE=--)
[ 3.252220] pc : mtk_drm_crtc_dma_dev_get+0x8/0x18
[ 3.252824] lr : mtk_drm_bind+0x458/0x558
[ 3.253326] sp : ffff800082b23a20
[ 3.253741] x29: ffff800082b23a20 x28: ffff000002c78880 x27:
ffff8000816466d0
[ 3.254635] x26: ffff000002c6f010 x25: 0000000000000003 x24:
0000000000000000
[ 3.255529] x23: ffff000002c78880 x22: 0000000000000002 x21:
0000000000000000
[ 3.256423] x20: ffff000006516800 x19: ffff000002c78880 x18:
ffffffffffffffff
[ 3.257317] x17: 6f63206573756163 x16: 6562203020637472 x15:
6320676e69746165
[ 3.258211] x14: 726320746f4e203a x13: 676e697373696d20 x12:
726f2064656c6261
[ 3.259106] x11: 7369642073692039 x10: ffff80008275c0c0 x9 :
ffff80008091ebf8
[ 3.260000] x8 : 00000000ffffefff x7 : ffff80008275c0c0 x6 :
80000000fffff000
[ 3.260895] x5 : 000000000000bff4 x4 : 0000000000000000 x3 :
ffff000006516ae0
[ 3.261789] x2 : ffff000006516ae0 x1 : 0000000000000000 x0 :
0000000000000000
[ 3.262684] Call trace:
[ 3.262991] mtk_drm_crtc_dma_dev_get+0x8/0x18
[ 3.263549] try_to_bring_up_aggregate_device+0x16c/0x1e0
[ 3.264227] __component_add+0xac/0x180
[ 3.264708] component_add+0x1c/0x30
[ 3.265158] mtk_disp_rdma_probe+0x17c/0x270
[ 3.265695] platform_probe+0x70/0xd0
[ 3.266155] really_probe+0x150/0x2c0
[ 3.266615] __driver_probe_device+0x80/0x140
[ 3.267162] driver_probe_device+0x44/0x170
[ 3.267687] __device_attach_driver+0xc0/0x148
[ 3.268245] bus_for_each_drv+0x88/0xf0
[ 3.268727] __device_attach+0xa4/0x198
[ 3.269208] device_initial_probe+0x1c/0x30
[ 3.269732] bus_probe_device+0xb4/0xc0
[ 3.270214] deferred_probe_work_func+0x90/0xd0
[ 3.270783] process_one_work+0x144/0x3a0
[ 3.271289] worker_thread+0x2ac/0x4b8
[ 3.271761] kthread+0xec/0xf8
[ 3.272145] ret_from_fork+0x10/0x20
[ 3.272597] Code: 814f7858 ffff8000 aa1e03e9 d503201f (f9425000)
[ 3.273361] ---[ end trace 0000000000000000 ]---
Here's the complete bootlog:
https://pastebin.com/raw/SekMYBj4
Any help is where to look is appreciated.
While the mediatek-dpi driver finds the DP port:
[ 3.131645] mediatek-dpi 1c113000.dp-intf: Found bridge node:
/soc/dp-tx@1c600000
The probing of the eDP is deferred:
[ 13.289009] platform 1c015000.dp-intf: deferred probe pending
So I don't know why, but to make dp_intf1 work, it seems that dp_intf0
must be probed successfully. After this patch, the edp (which is
connected to the dp_intf1) probe will return with an -ENODEV and
the previous call to devm_drm_bridge_add() will be rolled back.
The MediaTek DRM driver uses the component framework, so it waits for
all of its
components to register until it binds them all (which includes both
intf0 and
intf1, unless they're disabled on the DT).
Oh with "I don't know why, but to make dp_intf1 work.." I meant the
splat
above. I figured that dpi won't probe if a dependency is still deferred
;)
-michael
It's true that before this patch no panel being found for edp-tx
wouldn't
prevent it to probe, but it really should.
Thanks,
Nícolas
Before this patch, bridge_add() was called in any case (in the
error case with next_bridge = NULL) and the mediatek-dpi probed
like that:
[ 3.121011] mediatek-dpi 1c015000.dp-intf: Found bridge node:
/soc/edp-tx@1c500000
[ 3.122111] mediatek-dpi 1c113000.dp-intf: Found bridge node:
/soc/dp-tx@1c600000
Eventually resulting in a framebuffer device:
[ 4.451081] mediatek-drm mediatek-drm.8.auto: [drm] fb0:
mediatekdrmfb frame buffer device
NB, somehow this series broke the initial display output. I always
have
to replug the DisplayPort to get some output. I'll dig deeper into
that
later.
..
> @@ -2519,21 +2553,14 @@ static int mtk_dp_probe(struct platform_device *pdev)
> return dev_err_probe(dev, mtk_dp->irq,
> "failed to request dp irq resource\n");
>
> - mtk_dp->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0);
> - if (IS_ERR(mtk_dp->next_bridge) &&
> - PTR_ERR(mtk_dp->next_bridge) == -ENODEV)
> - mtk_dp->next_bridge = NULL;
In my case, this branch was taken.
-michael
> - else if (IS_ERR(mtk_dp->next_bridge))
> - return dev_err_probe(dev, PTR_ERR(mtk_dp->next_bridge),
> - "Failed to get bridge\n");
> -
> ret = mtk_dp_dt_parse(mtk_dp, pdev);
> if (ret)
> return dev_err_probe(dev, ret, "Failed to parse dt\n");
>
> - drm_dp_aux_init(&mtk_dp->aux);
> mtk_dp->aux.name = "aux_mtk_dp";
> + mtk_dp->aux.dev = dev;
> mtk_dp->aux.transfer = mtk_dp_aux_transfer;
> + drm_dp_aux_init(&mtk_dp->aux);
>
> spin_lock_init(&mtk_dp->irq_thread_lock);
>