If panel_bridge_attach() happens after DRM device registration, the created connector will not be registered by the DRM core anymore. Fix this by registering it explicitely in such case.
This fixes the following issue observed on Samsung Exynos4210-based Trats board with a DSI panel (the panel driver is registed after the Exynos DRM component device is bound):
$ ./modetest -c -Mexynos could not get connector 56: No such file or directory Segmentation fault
While touching this, move the connector reset() call also under the DRM device registered check, because otherwise it is not really needed.
Fixes: 934aef885f9d ("drm: bridge: panel: Reset the connector state pointer") Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- Here is a bit more backgroud on this issue is available in this thread: https://lore.kernel.org/all/f0474a95-4ba3-a74f-d7de-ef7aab12bc86@samsung.com... --- drivers/gpu/drm/bridge/panel.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index ff1c37b2e6e5..0ee563eb2b6f 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -83,8 +83,11 @@ static int panel_bridge_attach(struct drm_bridge *bridge, drm_connector_attach_encoder(&panel_bridge->connector, bridge->encoder);
- if (connector->funcs->reset) - connector->funcs->reset(connector); + if (bridge->dev->registered) { + if (connector->funcs->reset) + connector->funcs->reset(connector); + drm_connector_register(connector); + }
return 0; }
On Tue, Apr 19, 2022 at 2:44 PM Marek Szyprowski m.szyprowski@samsung.com wrote:
If panel_bridge_attach() happens after DRM device registration, the created connector will not be registered by the DRM core anymore. Fix this by registering it explicitely in such case.
This fixes the following issue observed on Samsung Exynos4210-based Trats board with a DSI panel (the panel driver is registed after the Exynos DRM component device is bound):
$ ./modetest -c -Mexynos could not get connector 56: No such file or directory Segmentation fault
While touching this, move the connector reset() call also under the DRM device registered check, because otherwise it is not really needed.
Fixes: 934aef885f9d ("drm: bridge: panel: Reset the connector state pointer") Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
Here is a bit more backgroud on this issue is available in this thread: https://lore.kernel.org/all/f0474a95-4ba3-a74f-d7de-ef7aab12bc86@samsung.com...
drivers/gpu/drm/bridge/panel.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index ff1c37b2e6e5..0ee563eb2b6f 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -83,8 +83,11 @@ static int panel_bridge_attach(struct drm_bridge *bridge, drm_connector_attach_encoder(&panel_bridge->connector, bridge->encoder);
if (connector->funcs->reset)connector->funcs->reset(connector);
if (bridge->dev->registered) {if (connector->funcs->reset)connector->funcs->reset(connector);drm_connector_register(connector);}
Reviewed-by: Jagan Teki jagan@amarulasolutions.com
On Tue, 19 Apr 2022 at 11:41, Jagan Teki jagan@amarulasolutions.com wrote:
On Tue, Apr 19, 2022 at 2:44 PM Marek Szyprowski m.szyprowski@samsung.com wrote:
If panel_bridge_attach() happens after DRM device registration, the created connector will not be registered by the DRM core anymore. Fix this by registering it explicitely in such case.
This fixes the following issue observed on Samsung Exynos4210-based Trats board with a DSI panel (the panel driver is registed after the Exynos DRM component device is bound):
$ ./modetest -c -Mexynos could not get connector 56: No such file or directory Segmentation fault
While touching this, move the connector reset() call also under the DRM device registered check, because otherwise it is not really needed.
Fixes: 934aef885f9d ("drm: bridge: panel: Reset the connector state pointer") Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
Here is a bit more backgroud on this issue is available in this thread: https://lore.kernel.org/all/f0474a95-4ba3-a74f-d7de-ef7aab12bc86@samsung.com...
drivers/gpu/drm/bridge/panel.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index ff1c37b2e6e5..0ee563eb2b6f 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -83,8 +83,11 @@ static int panel_bridge_attach(struct drm_bridge *bridge, drm_connector_attach_encoder(&panel_bridge->connector, bridge->encoder);
if (connector->funcs->reset)connector->funcs->reset(connector);
if (bridge->dev->registered) {if (connector->funcs->reset)connector->funcs->reset(connector);drm_connector_register(connector);}Reviewed-by: Jagan Teki jagan@amarulasolutions.com
Fixed typos in commit message.
Reviewed-by: Robert Foss robert.foss@linaro.org
Applied to drm-misc-next
On Tue, Apr 19, 2022 at 06:16:07PM +0200, Robert Foss wrote:
On Tue, 19 Apr 2022 at 11:41, Jagan Teki jagan@amarulasolutions.com wrote:
On Tue, Apr 19, 2022 at 2:44 PM Marek Szyprowski m.szyprowski@samsung.com wrote:
If panel_bridge_attach() happens after DRM device registration, the created connector will not be registered by the DRM core anymore. Fix this by registering it explicitely in such case.
This fixes the following issue observed on Samsung Exynos4210-based Trats board with a DSI panel (the panel driver is registed after the Exynos DRM component device is bound):
$ ./modetest -c -Mexynos could not get connector 56: No such file or directory Segmentation fault
While touching this, move the connector reset() call also under the DRM device registered check, because otherwise it is not really needed.
Fixes: 934aef885f9d ("drm: bridge: panel: Reset the connector state pointer") Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
Here is a bit more backgroud on this issue is available in this thread: https://lore.kernel.org/all/f0474a95-4ba3-a74f-d7de-ef7aab12bc86@samsung.com...
drivers/gpu/drm/bridge/panel.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index ff1c37b2e6e5..0ee563eb2b6f 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -83,8 +83,11 @@ static int panel_bridge_attach(struct drm_bridge *bridge, drm_connector_attach_encoder(&panel_bridge->connector, bridge->encoder);
if (connector->funcs->reset)connector->funcs->reset(connector);
if (bridge->dev->registered) {if (connector->funcs->reset)connector->funcs->reset(connector);drm_connector_register(connector);}Reviewed-by: Jagan Teki jagan@amarulasolutions.com
Fixed typos in commit message.
Reviewed-by: Robert Foss robert.foss@linaro.org
Applied to drm-misc-next
Doesn't this open the door to various race conditions ?
Also, what happens if the panel bridge is detached and reattached ? If I recall correctly, registering new connectors at runtime is at least partly supported for DP MST, but I'm not sure about unregistration.
On Tue, Apr 19, 2022 at 11:08 PM Laurent Pinchart laurent.pinchart@ideasonboard.com wrote:
On Tue, Apr 19, 2022 at 06:16:07PM +0200, Robert Foss wrote:
On Tue, 19 Apr 2022 at 11:41, Jagan Teki jagan@amarulasolutions.com wrote:
On Tue, Apr 19, 2022 at 2:44 PM Marek Szyprowski m.szyprowski@samsung.com wrote:
If panel_bridge_attach() happens after DRM device registration, the created connector will not be registered by the DRM core anymore. Fix this by registering it explicitely in such case.
This fixes the following issue observed on Samsung Exynos4210-based Trats board with a DSI panel (the panel driver is registed after the Exynos DRM component device is bound):
$ ./modetest -c -Mexynos could not get connector 56: No such file or directory Segmentation fault
While touching this, move the connector reset() call also under the DRM device registered check, because otherwise it is not really needed.
Fixes: 934aef885f9d ("drm: bridge: panel: Reset the connector state pointer") Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
Here is a bit more backgroud on this issue is available in this thread: https://lore.kernel.org/all/f0474a95-4ba3-a74f-d7de-ef7aab12bc86@samsung.com...
drivers/gpu/drm/bridge/panel.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index ff1c37b2e6e5..0ee563eb2b6f 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -83,8 +83,11 @@ static int panel_bridge_attach(struct drm_bridge *bridge, drm_connector_attach_encoder(&panel_bridge->connector, bridge->encoder);
if (connector->funcs->reset)connector->funcs->reset(connector);
if (bridge->dev->registered) {if (connector->funcs->reset)connector->funcs->reset(connector);drm_connector_register(connector);}Reviewed-by: Jagan Teki jagan@amarulasolutions.com
Fixed typos in commit message.
Reviewed-by: Robert Foss robert.foss@linaro.org
Applied to drm-misc-next
Doesn't this open the door to various race conditions ?
Also, what happens if the panel bridge is detached and reattached ? If I
I believe the dev->registered is the check for those already registered.
Jagan.
Hi Laurent,
On 19.04.2022 19:37, Laurent Pinchart wrote:
On Tue, Apr 19, 2022 at 06:16:07PM +0200, Robert Foss wrote:
On Tue, 19 Apr 2022 at 11:41, Jagan Teki jagan@amarulasolutions.com wrote:
On Tue, Apr 19, 2022 at 2:44 PM Marek Szyprowski m.szyprowski@samsung.com wrote:
If panel_bridge_attach() happens after DRM device registration, the created connector will not be registered by the DRM core anymore. Fix this by registering it explicitely in such case.
This fixes the following issue observed on Samsung Exynos4210-based Trats board with a DSI panel (the panel driver is registed after the Exynos DRM component device is bound):
$ ./modetest -c -Mexynos could not get connector 56: No such file or directory Segmentation fault
While touching this, move the connector reset() call also under the DRM device registered check, because otherwise it is not really needed.
Fixes: 934aef885f9d ("drm: bridge: panel: Reset the connector state pointer") Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
Here is a bit more backgroud on this issue is available in this thread: https://lore.kernel.org/all/f0474a95-4ba3-a74f-d7de-ef7aab12bc86@samsung.com...
drivers/gpu/drm/bridge/panel.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c index ff1c37b2e6e5..0ee563eb2b6f 100644 --- a/drivers/gpu/drm/bridge/panel.c +++ b/drivers/gpu/drm/bridge/panel.c @@ -83,8 +83,11 @@ static int panel_bridge_attach(struct drm_bridge *bridge, drm_connector_attach_encoder(&panel_bridge->connector, bridge->encoder);
if (connector->funcs->reset)connector->funcs->reset(connector);
if (bridge->dev->registered) {if (connector->funcs->reset)connector->funcs->reset(connector);drm_connector_register(connector);}Reviewed-by: Jagan Teki jagan@amarulasolutions.com
Fixed typos in commit message.
Reviewed-by: Robert Foss robert.foss@linaro.org
Applied to drm-misc-next
Doesn't this open the door to various race conditions ?
This only patch restores the old behavior of the Exynos DSI driver. IIRC Andrzej already checked the possible races and said that the late connector registration is fine.
Also, what happens if the panel bridge is detached and reattached ? If I recall correctly, registering new connectors at runtime is at least partly supported for DP MST, but I'm not sure about unregistration.
Well, indeed the panel unregistration is broken now:
# rmmod panel_samsung_s6e8aa0 ------------[ cut here ]------------ WARNING: CPU: 1 PID: 1336 at drivers/gpu/drm/drm_connector.c:462 drm_connector_cleanup+0x26c/0x288 Modules linked in: cmac bnep hci_uart btbcm btintel bluetooth s5p_csis s5p_fimc exynos4_is_common ecdh_generic ecc v4l2_fwnode v4l2_async s5p_mfc brcmfmac cfg80211 brcmutil panel_samsung_s6e8aa0(-) s5p_jpeg videobuf2_dma_contig videobuf2_memops v4l2_mem2mem videobuf2_v4l2 videobuf2_common videodev mc CPU: 1 PID: 1336 Comm: rmmod Not tainted 5.18.0-rc3-next-20220420-00033-g871e1a91a67f #4866 Hardware name: Samsung Exynos (Flattened Device Tree) unwind_backtrace from show_stack+0x10/0x14 show_stack from dump_stack_lvl+0x58/0x70 dump_stack_lvl from __warn+0xc8/0x218 __warn from warn_slowpath_fmt+0x5c/0xb4 warn_slowpath_fmt from drm_connector_cleanup+0x26c/0x288 drm_connector_cleanup from exynos_dsi_host_detach+0x24/0x74 exynos_dsi_host_detach from s6e8aa0_remove+0xc/0x1c [panel_samsung_s6e8aa0] s6e8aa0_remove [panel_samsung_s6e8aa0] from device_release_driver_internal+0x1a4/0x20c device_release_driver_internal from driver_detach+0x44/0x80 driver_detach from bus_remove_driver+0x60/0xd8 bus_remove_driver from sys_delete_module+0x138/0x22c sys_delete_module from ret_fast_syscall+0x0/0x1c Exception stack(0xf0c01fa8 to 0xf0c01ff0) 1fa0: 005224d8 00000002 00522514 00000800 e2a92e00 e2a92e00 1fc0: 005224d8 00000002 005224d8 00000081 be9daf02 be9dac1c be9dae08 00000001 1fe0: 00520f70 be9dabb4 00503ac4 b6e2a0fc irq event stamp: 3465 hardirqs last enabled at (3481): [<c015a448>] finish_task_switch+0x110/0x368 hardirqs last disabled at (3490): [<c019d210>] __up_console_sem+0x3c/0x60 softirqs last enabled at (3460): [<c0101680>] __do_softirq+0x348/0x610 softirqs last disabled at (3423): [<c012ed64>] __irq_exit_rcu+0x144/0x1ec ---[ end trace 0000000000000000 ]--- 8<--- cut here ---
This is however just yet another issue caused by the recent Exynos DSI driver conversion to DRM bridge. In v5.17 it worked fine. I will try to look into that issue later, this is not related to the $subject.
Subsequent panel registration (after it has been first unregistered) seems to be broken for ages. It worked fine in v4.19, but then it got broken somewhere before v4.20. I've added this to my todo list (with low priority though).
Best regards
dri-devel@lists.freedesktop.org
-
Jagan Teki -
Laurent Pinchart -
Marek Szyprowski -
Robert Foss