Re: [PATCH v15 00/16] drm: Add Samsung MIPI DSIM bridge

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hi Jagan,

On 04.03.2023 19:59, Jagan Teki wrote:
> On Sat, Mar 4, 2023 at 3:56 AM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> On 03.03.2023 15:51, Jagan Teki wrote:
>>> This series supports common bridge support for Samsung MIPI DSIM
>>> which is used in Exynos and i.MX8MM SoC's.
>>>
>>> The final bridge supports both the Exynos and i.MX8M Mini/Nano/Plus.
>>>
>>> Inki Dae: please note that this series added on top of exynos-drm-next
>>> since few exynos dsi changes are not been part of drm-misc-next.
>>> Request you to pick these via exynos-drm-next, or let me know if you
>>> have any comments?
>> I gave it a try on Exynos TM2e and unfortunately it nukes again:
>>
>> exynos-drm exynos-drm: bound 13970000.hdmi (ops hdmi_component_ops)
>> Unable to handle kernel paging request at virtual address 003d454d414e5675
>> ...
>> [003d454d414e5675] address between user and kernel address ranges
>> Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
>> Modules linked in:
>> CPU: 4 PID: 9 Comm: kworker/u16:0 Not tainted 6.2.0-next-20230303+ #13341
>> Hardware name: Samsung TM2E board (DT)
>> Workqueue: events_unbound deferred_probe_work_func
>> pstate: 000000c5 (nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : drm_connector_list_iter_next+0x58/0x100
>> lr : drm_connector_list_iter_next+0x2c/0x100
>> sp : ffff80000bbab910
>> ...
>> Call trace:
>>    drm_connector_list_iter_next+0x58/0x100
>>    drm_mode_config_reset+0xfc/0x144
>>    exynos_drm_bind+0x160/0x1b8
>>    try_to_bring_up_aggregate_device+0x168/0x1d4
>>    __component_add+0xa8/0x170
>>    component_add+0x14/0x20
>>    hdmi_probe+0x3fc/0x6d4
>>    platform_probe+0x68/0xd8
>>    really_probe+0x148/0x2b4
>>    __driver_probe_device+0x78/0xe0
>>    driver_probe_device+0xd8/0x160
>>    __device_attach_driver+0xb8/0x138
>>    bus_for_each_drv+0x84/0xe0
>>    __device_attach+0xa8/0x1b0
>>    device_initial_probe+0x14/0x20
>>    bus_probe_device+0xb0/0xb4
>>    deferred_probe_work_func+0x8c/0xc8
>>    process_one_work+0x288/0x6c8
>>    worker_thread+0x24c/0x450
>>    kthread+0x118/0x11c
>>    ret_from_fork+0x10/0x20
> This looks not related to dsi to me since there is no exynos_drm_dsi
> call in the trace. look hdmi related. Moreover, I think the exynos dsi
> worked well on v10 and I couldn't find any potential differences in
> terms of call flow change.
> https://gitlab.com/openedev/kernel/-/commits/imx8mm-dsi-v10

Well, the issue is definitely related to this patchset. On Friday, due 
to other kernel messages, I missed the most important part of the log:

[drm] Exynos DRM: using 13800000.decon device for DMA mapping operations
exynos-drm exynos-drm: bound 13800000.decon (ops decon_component_ops)
exynos-drm exynos-drm: bound 13880000.decon (ops decon_component_ops)
exynos-dsi 13900000.dsi: [drm:samsung_dsim_host_attach] Attached s6e3hf2 
device
exynos-dsi 13900000.dsi: request interrupt failed with -22
panel-samsung-s6e3ha2: probe of 13900000.dsi.0 failed with error -22
exynos-drm exynos-drm: bound 13900000.dsi (ops exynos_dsi_component_ops)
exynos-drm exynos-drm: bound 13930000.mic (ops exynos_mic_component_ops)

It looks that the are at least 2 issues. The first one related to TE 
interrupt registration, the second is broken error path, which should 
free allocated resources and stop DRM from binding/initialization.

This patch fixes the issue (TE gpio/interrupt is optional!):

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c 
b/drivers/gpu/drm/bridge/samsung-dsim.c
index b4a5348b763c..ed83495fe105 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1518,7 +1518,9 @@ static int samsung_dsim_register_te_irq(struct 
samsung_dsim *dsi, struct device
         int ret;

         dsi->te_gpio = devm_gpiod_get_optional(dev, "te", GPIOD_IN);
-       if (IS_ERR(dsi->te_gpio))
+       if (!dsi->te_gpio)
+               return 0;
+       else if (IS_ERR(dsi->te_gpio))
                 return dev_err_probe(dev, PTR_ERR(dsi->te_gpio), 
"failed to get te GPIO\n");

         te_gpio_irq = gpiod_to_irq(dsi->te_gpio);


The error path in samsung_dsim_host_attach() after 
samsung_dsim_register_te_irq() failure is wrong. It lacks cleaning up 
the allocated resources (removing the bridge, detaing the dsi device).

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland