| Message ID | 20230725073234.55892-10-angelogioacchino.delregno@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MediaTek DisplayPort: support eDP and aux-bus | expand |
Hi AngeloGioacchino, > 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. 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. 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); >
Hi, On Fri, Aug 25, 2023 at 02:01:09PM +0200, Michael Walle wrote: > Hi AngeloGioacchino, > > > 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? > > 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). 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); > >
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); >> >
On Fri, Aug 25, 2023 at 05:42:59PM +0200, Michael Walle wrote: > 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..) Right, given it has an HPD pin it should be possible to hotplug. Although hotplugging a panel to eDP doesn't seem common, so Angelo even removed HPD irq support for eDP in patch 11 of this series. The unit I have is a mt8195-cherry-tomato, a Chromebook, so I couldn't really test hotplugging the internal display. > > 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 ]--- I tried reproducing this on mt8192-asurada-spherion and mt8195-cherry-tomato but wasn't able to. However, I did see another issue [ 3.183314] mediatek-drm mediatek-drm.9.auto: Not creating crtc 0 because component 14 is disabled or missing [ 3.199404] Bogus possible_crtcs: [ENCODER:31:TMDS-31] possible_crtcs=0x2 (full crtc mask=0x1) [ 3.208081] WARNING: CPU: 6 PID: 68 at drivers/gpu/drm/drm_mode_config.c:626 drm_mode_config_validate+0x1c8/0x548 [ 3.224789] Modules linked in: [ 3.227838] CPU: 6 PID: 68 Comm: kworker/u16:1 Not tainted 6.5.0-rc3-next-20230728+ #100 [ 3.235918] Hardware name: Google Spherion (rev0 - 3) (DT) [ 3.241391] Workqueue: events_unbound deferred_probe_work_func [ 3.247216] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 3.254167] pc : drm_mode_config_validate+0x1c8/0x548 [ 3.259209] lr : drm_mode_config_validate+0x1c8/0x548 [ 3.264250] sp : ffff8000804e3970 [ 3.267552] x29: ffff8000804e3980 x28: ffff4841827c1880 x27: 0000000000000001 [ 3.274677] x26: 0000000000000001 x25: ffff4841825f5ab0 x24: ffff4841825f5ab0 [ 3.281801] x23: ffff484182469880 x22: ffffa80dbfbdde28 x21: ffffa80dbfbddba8 [ 3.288925] x20: ffff4841825f5800 x19: ffff4841825f5aa8 x18: 0000000000000030 [ 3.296050] x17: 6628203278303d73 x16: 637472635f656c62 x15: 6973736f70205d31 [ 3.303174] x14: 332d53444d543a31 x13: 293178303d6b7361 x12: 6d2063747263206c [ 3.310298] x11: 6c75662820327830 x10: 3d73637472635f65 x9 : ffffa80dbdd3805c [ 3.317422] x8 : 455b203a73637472 x7 : 205d343034393931 x6 : ffffa80dbe6365d8 [ 3.324546] x5 : ffffa80dc0fcc48f x4 : ffffa80dc0049b40 x3 : 00000000ffffffff [ 3.331671] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff4841809d5e80 [ 3.338796] Call trace: [ 3.341232] drm_mode_config_validate+0x1c8/0x548 [ 3.345924] drm_dev_register+0x198/0x248 [ 3.345931] mtk_drm_bind+0x2cc/0x590 [ 3.345936] try_to_bring_up_aggregate_device+0x1f8/0x308 [ 3.345940] __component_add+0xac/0x1a0 [ 3.345942] component_add+0x1c/0x30 [ 3.345944] mtk_dpi_probe+0x1c0/0x300 [ 3.358100] platform_probe+0x70/0xe8 [ 3.358106] really_probe+0x18c/0x3d8 [ 3.358108] __driver_probe_device+0x84/0x180 [ 3.358109] driver_probe_device+0x44/0x120 [ 3.358111] __device_attach_driver+0xc4/0x168 [ 3.358113] bus_for_each_drv+0x8c/0xf0 [ 3.367146] __device_attach+0xb0/0x1e8 [ 3.367148] device_initial_probe+0x1c/0x30 [ 3.367150] bus_probe_device+0xb4/0xc0 [ 3.367153] deferred_probe_work_func+0xa4/0x100 [ 3.367155] process_one_work+0x1ec/0x480 [ 3.374543] worker_thread+0x74/0x448 [ 3.374545] kthread+0x120/0x130 [ 3.374548] ret_from_fork+0x10/0x20 The mtk-dpi driver populates its encoder's possible_crtcs from the result of mtk_drm_find_possible_crtc_by_comp(), and this function assumes the CRTC for the main path will always have ID 0, and the external path ID 1, but when the main path components are disabled, the external path CRTC becomes the one with ID 0. So we'd need to make that function return the crtc id dynamically based on whether the components for each path are enabled or not. With that function hacked to force the right crtc ID, I was able to have a working external DP, with the eDP pipeline disabled on both mt8192 and mt8195. Thanks, Nícolas
Hi Nícolas, >> 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 ]--- > > I tried reproducing this on mt8192-asurada-spherion and > mt8195-cherry-tomato but > wasn't able to. However, I did see another issue Yeah sorry, I tried to reproduce my initial oops but messed my DT up and ended up with no path enabled at all. > [ 3.183314] mediatek-drm mediatek-drm.9.auto: Not creating crtc 0 > because component 14 is disabled or missing > [ 3.199404] Bogus possible_crtcs: [ENCODER:31:TMDS-31] > possible_crtcs=0x2 (full crtc mask=0x1) > [ 3.208081] WARNING: CPU: 6 PID: 68 at > drivers/gpu/drm/drm_mode_config.c:626 > drm_mode_config_validate+0x1c8/0x548 > [ 3.224789] Modules linked in: > [ 3.227838] CPU: 6 PID: 68 Comm: kworker/u16:1 Not tainted > 6.5.0-rc3-next-20230728+ #100 > [ 3.235918] Hardware name: Google Spherion (rev0 - 3) (DT) > [ 3.241391] Workqueue: events_unbound deferred_probe_work_func > [ 3.247216] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS > BTYPE=--) > [ 3.254167] pc : drm_mode_config_validate+0x1c8/0x548 > [ 3.259209] lr : drm_mode_config_validate+0x1c8/0x548 > [ 3.264250] sp : ffff8000804e3970 > [ 3.267552] x29: ffff8000804e3980 x28: ffff4841827c1880 x27: > 0000000000000001 > [ 3.274677] x26: 0000000000000001 x25: ffff4841825f5ab0 x24: > ffff4841825f5ab0 > [ 3.281801] x23: ffff484182469880 x22: ffffa80dbfbdde28 x21: > ffffa80dbfbddba8 > [ 3.288925] x20: ffff4841825f5800 x19: ffff4841825f5aa8 x18: > 0000000000000030 > [ 3.296050] x17: 6628203278303d73 x16: 637472635f656c62 x15: > 6973736f70205d31 > [ 3.303174] x14: 332d53444d543a31 x13: 293178303d6b7361 x12: > 6d2063747263206c > [ 3.310298] x11: 6c75662820327830 x10: 3d73637472635f65 x9 : > ffffa80dbdd3805c > [ 3.317422] x8 : 455b203a73637472 x7 : 205d343034393931 x6 : > ffffa80dbe6365d8 > [ 3.324546] x5 : ffffa80dc0fcc48f x4 : ffffa80dc0049b40 x3 : > 00000000ffffffff > [ 3.331671] x2 : 0000000000000000 x1 : 0000000000000000 x0 : > ffff4841809d5e80 > [ 3.338796] Call trace: > [ 3.341232] drm_mode_config_validate+0x1c8/0x548 > [ 3.345924] drm_dev_register+0x198/0x248 > [ 3.345931] mtk_drm_bind+0x2cc/0x590 > [ 3.345936] try_to_bring_up_aggregate_device+0x1f8/0x308 > [ 3.345940] __component_add+0xac/0x1a0 > [ 3.345942] component_add+0x1c/0x30 > [ 3.345944] mtk_dpi_probe+0x1c0/0x300 > [ 3.358100] platform_probe+0x70/0xe8 > [ 3.358106] really_probe+0x18c/0x3d8 > [ 3.358108] __driver_probe_device+0x84/0x180 > [ 3.358109] driver_probe_device+0x44/0x120 > [ 3.358111] __device_attach_driver+0xc4/0x168 > [ 3.358113] bus_for_each_drv+0x8c/0xf0 > [ 3.367146] __device_attach+0xb0/0x1e8 > [ 3.367148] device_initial_probe+0x1c/0x30 > [ 3.367150] bus_probe_device+0xb4/0xc0 > [ 3.367153] deferred_probe_work_func+0xa4/0x100 > [ 3.367155] process_one_work+0x1ec/0x480 > [ 3.374543] worker_thread+0x74/0x448 > [ 3.374545] kthread+0x120/0x130 > [ 3.374548] ret_from_fork+0x10/0x20 That was what I was seeing in the first place, yes. (Any yeah, no oops, but a WARN()). > The mtk-dpi driver populates its encoder's possible_crtcs from the > result of > mtk_drm_find_possible_crtc_by_comp(), and this function assumes the > CRTC for the > main path will always have ID 0, and the external path ID 1, but when > the > main path components are disabled, the external path CRTC becomes the > one with > ID 0. > > So we'd need to make that function return the crtc id dynamically based > on > whether the components for each path are enabled or not. > > With that function hacked to force the right crtc ID, I was able to > have a > working external DP, with the eDP pipeline disabled on both mt8192 and > mt8195. Thanks for the hint, where to look at. While digging through the code I realized that all the outputs and pipelines are harcoded. Doh. For all the mediatek SoCs. Looks like major restriction to me. E.g. there is also DSI and HDMI output on the mt8195. I looked at the downstream linux and there, the output is not part of the pipeline. Are you aware of any work in that direction? -michael
On Tue, Aug 29, 2023 at 03:30:24PM +0200, Michael Walle wrote: > Hi Nícolas, > > > > 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 ]--- > > > > I tried reproducing this on mt8192-asurada-spherion and > > mt8195-cherry-tomato but > > wasn't able to. However, I did see another issue > > Yeah sorry, I tried to reproduce my initial oops but messed my DT up > and ended up with no path enabled at all. No worries, thanks for sending fixes for both! > [..] > > The mtk-dpi driver populates its encoder's possible_crtcs from the > > result of > > mtk_drm_find_possible_crtc_by_comp(), and this function assumes the CRTC > > for the > > main path will always have ID 0, and the external path ID 1, but when > > the > > main path components are disabled, the external path CRTC becomes the > > one with > > ID 0. > > > > So we'd need to make that function return the crtc id dynamically based > > on > > whether the components for each path are enabled or not. > > > > With that function hacked to force the right crtc ID, I was able to have > > a > > working external DP, with the eDP pipeline disabled on both mt8192 and > > mt8195. > > Thanks for the hint, where to look at. > > While digging through the code I realized that all the outputs and pipelines > are harcoded. Doh. For all the mediatek SoCs. Looks like major restriction > to > me. E.g. there is also DSI and HDMI output on the mt8195. I looked at the > downstream linux and there, the output is not part of the pipeline. Are you > aware of any work in that direction? I'm not sure I get what output and pipelines are hardcoded that you're referring to (besides the one in the mtk-dsi/dpi driver that you already sent the patch fixing). And I'm not familiar with the DSI and HDMI output support on MT8195, so I can't help with that. Thanks, Nícolas
>> While digging through the code I realized that all the outputs and >> pipelines >> are harcoded. Doh. For all the mediatek SoCs. Looks like major >> restriction >> to >> me. E.g. there is also DSI and HDMI output on the mt8195. I looked at >> the >> downstream linux and there, the output is not part of the pipeline. >> Are you >> aware of any work in that direction? > > I'm not sure I get what output and pipelines are hardcoded that you're > referring > to (besides the one in the mtk-dsi/dpi driver that you already sent the > patch > fixing). Have a look at [1]. That main path ends with the DP_INTF0 which is the eDP output. But from what I understand that path can also use the DSI output. But that is a pattern for all the paths in that file. Looks like all supported boards in the kernel will have the output type for a given SoC/path (or maybe the mt8195 is the first one which supports different output interfaces). If you have a look at the mediatek kernel instead [2], there the last part of the path is not fixed, but there is also a .conn_routes property by which you seem to be able to choose the actual output interface. I was just curious if you know of any development for that (or similar) in the kernel. -michael > And I'm not familiar with the DSI and HDMI output support on MT8195, so > I can't > help with that. > > Thanks, > Nícolas [1] https://elixir.bootlin.com/linux/v6.5/source/drivers/gpu/drm/mediatek/mtk_drm_drv.c#L210 [2] https://gitlab.com/mediatek/aiot/bsp/linux/-/blob/mtk-v5.15-dev/drivers/gpu/drm/mediatek/mtk_drm_drv.c?ref_type=heads#L425
On Wed, Aug 30, 2023 at 7:11 PM Michael Walle <mwalle@kernel.org> wrote: > > >> While digging through the code I realized that all the outputs and > >> pipelines > >> are harcoded. Doh. For all the mediatek SoCs. Looks like major > >> restriction > >> to > >> me. E.g. there is also DSI and HDMI output on the mt8195. I looked at > >> the > >> downstream linux and there, the output is not part of the pipeline. > >> Are you > >> aware of any work in that direction? > > > > I'm not sure I get what output and pipelines are hardcoded that you're > > referring > > to (besides the one in the mtk-dsi/dpi driver that you already sent the > > patch > > fixing). > > Have a look at [1]. That main path ends with the DP_INTF0 which is the > eDP output. But from what I understand that path can also use the DSI > output. But that is a pattern for all the paths in that file. Looks like > all supported boards in the kernel will have the output type for a given > SoC/path (or maybe the mt8195 is the first one which supports different > output interfaces). > > If you have a look at the mediatek kernel instead [2], there the last > part of the path is not fixed, but there is also a .conn_routes property > by which you seem to be able to choose the actual output interface. > > I was just curious if you know of any development for that (or similar) > in the kernel. This is probably because support for this SoC began with Chromebooks, which have fixed and defined uses for the pipelines. I suspect that what you are working on is much more flexible. The driver should be made to allow dynamic selection of outputs, as is commonly seen with other drivers, but I don't know if that's on anyone's TODO list. ChenYu > > And I'm not familiar with the DSI and HDMI output support on MT8195, so > > I can't > > help with that. > > > > Thanks, > > Nícolas > > [1] > https://elixir.bootlin.com/linux/v6.5/source/drivers/gpu/drm/mediatek/mtk_drm_drv.c#L210 > [2] > https://gitlab.com/mediatek/aiot/bsp/linux/-/blob/mtk-v5.15-dev/drivers/gpu/drm/mediatek/mtk_drm_drv.c?ref_type=heads#L425
Hi, >> I was just curious if you know of any development for that (or >> similar) >> in the kernel. > > This is probably because support for this SoC began with Chromebooks, > which have fixed and defined uses for the pipelines. I suspect that > what you are working on is much more flexible. Yes. that is correct. > The driver should be made to allow dynamic selection of outputs, as > is commonly seen with other drivers, but I don't know if that's on > anyone's TODO list. Do you have any pointers where to look at? -michael
On Fri, Sep 1, 2023 at 6:00 PM Michael Walle <mwalle@kernel.org> wrote: > > Hi, > > >> I was just curious if you know of any development for that (or > >> similar) > >> in the kernel. > > > > This is probably because support for this SoC began with Chromebooks, > > which have fixed and defined uses for the pipelines. I suspect that > > what you are working on is much more flexible. > > Yes. that is correct. > > > The driver should be made to allow dynamic selection of outputs, as > > is commonly seen with other drivers, but I don't know if that's on > > anyone's TODO list. > > Do you have any pointers where to look at? There's a series [1] called "Add connector dynamic selection capability". Not sure if it does what you want though. I haven't looked into it. ChenYu [1] https://lore.kernel.org/all/20230809181525.7561-1-jason-jh.lin@mediatek.com/
diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig index b451dee64d34..76cab28e010c 100644 --- a/drivers/gpu/drm/mediatek/Kconfig +++ b/drivers/gpu/drm/mediatek/Kconfig @@ -26,6 +26,7 @@ config DRM_MEDIATEK_DP select PHY_MTK_DP select DRM_DISPLAY_HELPER select DRM_DISPLAY_DP_HELPER + select DRM_DP_AUX_BUS help DRM/KMS Display Port driver for MediaTek SoCs. diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c index 1b4219e6a00b..18c944bfc7ce 100644 --- a/drivers/gpu/drm/mediatek/mtk_dp.c +++ b/drivers/gpu/drm/mediatek/mtk_dp.c @@ -4,6 +4,7 @@ * Copyright (c) 2022 BayLibre */ +#include <drm/display/drm_dp_aux_bus.h> #include <drm/display/drm_dp.h> #include <drm/display/drm_dp_helper.h> #include <drm/drm_atomic_helper.h> @@ -1313,9 +1314,11 @@ static void mtk_dp_power_disable(struct mtk_dp *mtk_dp) static void mtk_dp_initialize_priv_data(struct mtk_dp *mtk_dp) { + bool plugged_in = (mtk_dp->bridge.type == DRM_MODE_CONNECTOR_eDP); + mtk_dp->train_info.link_rate = DP_LINK_BW_5_4; mtk_dp->train_info.lane_count = mtk_dp->max_lanes; - mtk_dp->train_info.cable_plugged_in = false; + mtk_dp->train_info.cable_plugged_in = plugged_in; mtk_dp->info.format = DP_PIXELFORMAT_RGB; memset(&mtk_dp->info.vm, 0, sizeof(struct videomode)); @@ -1617,6 +1620,16 @@ static int mtk_dp_parse_capabilities(struct mtk_dp *mtk_dp) u8 val; ssize_t ret; + /* + * If we're eDP and capabilities were already parsed we can skip + * reading again because eDP panels aren't hotpluggable hence the + * caps and training information won't ever change in a boot life + */ + if (mtk_dp->bridge.type == DRM_MODE_CONNECTOR_eDP && + mtk_dp->rx_cap[DP_MAX_LINK_RATE] && + mtk_dp->train_info.sink_ssc) + return 0; + ret = drm_dp_read_dpcd_caps(&mtk_dp->aux, mtk_dp->rx_cap); if (ret < 0) return ret; @@ -2030,15 +2043,14 @@ static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge, static ssize_t mtk_dp_aux_transfer(struct drm_dp_aux *mtk_aux, struct drm_dp_aux_msg *msg) { - struct mtk_dp *mtk_dp; + struct mtk_dp *mtk_dp = container_of(mtk_aux, struct mtk_dp, aux); bool is_read; u8 request; size_t accessed_bytes = 0; int ret; - mtk_dp = container_of(mtk_aux, struct mtk_dp, aux); - - if (!mtk_dp->train_info.cable_plugged_in) { + if (mtk_dp->bridge.type != DRM_MODE_CONNECTOR_eDP && + !mtk_dp->train_info.cable_plugged_in) { ret = -EAGAIN; goto err; } @@ -2501,6 +2513,28 @@ static int mtk_dp_register_phy(struct mtk_dp *mtk_dp) return 0; } +static int mtk_dp_edp_link_panel(struct drm_dp_aux *mtk_aux) +{ + struct mtk_dp *mtk_dp = container_of(mtk_aux, struct mtk_dp, aux); + struct device *dev = mtk_aux->dev; + int ret; + + mtk_dp->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0); + + /* Power off the DP and AUX: either detection is done, or no panel present */ + mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE, + DP_PWR_STATE_BANDGAP_TPLL, + DP_PWR_STATE_MASK); + mtk_dp_power_disable(mtk_dp); + + if (IS_ERR(mtk_dp->next_bridge)) { + ret = PTR_ERR(mtk_dp->next_bridge); + mtk_dp->next_bridge = NULL; + return ret; + } + return 0; +} + static int mtk_dp_probe(struct platform_device *pdev) { struct mtk_dp *mtk_dp; @@ -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; - 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); @@ -2577,6 +2604,43 @@ static int mtk_dp_probe(struct platform_device *pdev) mtk_dp->need_debounce = true; timer_setup(&mtk_dp->debounce_timer, mtk_dp_debounce_timer, 0); + if (mtk_dp->bridge.type == DRM_MODE_CONNECTOR_eDP) { + /* + * Set the data lanes to idle in case the bootloader didn't + * properly close the eDP port to avoid stalls and then + * reinitialize, reset and power on the AUX block. + */ + mtk_dp_set_idle_pattern(mtk_dp, true); + mtk_dp_initialize_aux_settings(mtk_dp); + mtk_dp_power_enable(mtk_dp); + + /* + * Power on the AUX to allow reading the EDID from aux-bus: + * please note that it is necessary to call power off in the + * .done_probing() callback (mtk_dp_edp_link_panel), as only + * there we can safely assume that we finished reading EDID. + */ + mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE, + DP_PWR_STATE_BANDGAP_TPLL_LANE, + DP_PWR_STATE_MASK); + + ret = devm_of_dp_aux_populate_bus(&mtk_dp->aux, mtk_dp_edp_link_panel); + if (ret) { + /* -ENODEV this means that the panel is not on the aux-bus */ + if (ret == -ENODEV) { + ret = mtk_dp_edp_link_panel(&mtk_dp->aux); + if (ret) + return ret; + } else { + mtk_dp_update_bits(mtk_dp, MTK_DP_TOP_PWR_STATE, + DP_PWR_STATE_BANDGAP_TPLL, + DP_PWR_STATE_MASK); + mtk_dp_power_disable(mtk_dp); + return ret; + } + } + } + pm_runtime_enable(dev); pm_runtime_get_sync(dev);