* [PATCH] drm: xlnx: zynqmp_dpsub: Fix missing drm_bridge_add() call
@ 2024-03-12 8:51 ` Tomi Valkeinen
0 siblings, 0 replies; 4+ messages in thread
From: Tomi Valkeinen @ 2024-03-12 8:51 UTC (permalink / raw)
To: Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Michal Simek
Cc: dri-devel, linux-arm-kernel, linux-kernel, Vishal Sagar,
Anatoliy Klymenko, Tomi Valkeinen
The driver creates a bridge, but never calls drm_bridge_add() when
non-live input is used. This leaves the bridge's hpd_mutex
uninitialized, leading to:
WARNING: CPU: 0 PID: 9 at kernel/locking/mutex.c:582 __mutex_lock+0x708/0x840
Add the bridge add & remove calls so that the bridge gets managed
correctly.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Fixes: 561671612394 ("drm: xlnx: zynqmp_dpsub: Add support for live video input")
---
drivers/gpu/drm/xlnx/zynqmp_dp.c | 4 ++++
drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 4 ----
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index a0606fab0e22..9f750740dfb8 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1761,6 +1761,8 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
dpsub->dp = dp;
+ drm_bridge_add(dpsub->bridge);
+
dev_dbg(dp->dev, "ZynqMP DisplayPort Tx probed with %u lanes\n",
dp->num_lanes);
@@ -1789,4 +1791,6 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
zynqmp_dp_phy_exit(dp);
zynqmp_dp_reset(dp, true);
+
+ drm_bridge_remove(dpsub->bridge);
}
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
index 88eb33acd5f0..3933c4f1a44f 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
@@ -260,8 +260,6 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev)
ret = zynqmp_dpsub_drm_init(dpsub);
if (ret)
goto err_disp;
- } else {
- drm_bridge_add(dpsub->bridge);
}
dev_info(&pdev->dev, "ZynqMP DisplayPort Subsystem driver probed");
@@ -288,8 +286,6 @@ static void zynqmp_dpsub_remove(struct platform_device *pdev)
if (dpsub->drm)
zynqmp_dpsub_drm_cleanup(dpsub);
- else
- drm_bridge_remove(dpsub->bridge);
zynqmp_disp_remove(dpsub);
zynqmp_dp_remove(dpsub);
---
base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
change-id: 20240312-xilinx-dp-lock-fix-cf68f43a7bab
Best regards,
--
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] drm: xlnx: zynqmp_dpsub: Fix missing drm_bridge_add() call
@ 2024-03-12 8:51 ` Tomi Valkeinen
0 siblings, 0 replies; 4+ messages in thread
From: Tomi Valkeinen @ 2024-03-12 8:51 UTC (permalink / raw)
To: Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Michal Simek
Cc: dri-devel, linux-arm-kernel, linux-kernel, Vishal Sagar,
Anatoliy Klymenko, Tomi Valkeinen
The driver creates a bridge, but never calls drm_bridge_add() when
non-live input is used. This leaves the bridge's hpd_mutex
uninitialized, leading to:
WARNING: CPU: 0 PID: 9 at kernel/locking/mutex.c:582 __mutex_lock+0x708/0x840
Add the bridge add & remove calls so that the bridge gets managed
correctly.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Fixes: 561671612394 ("drm: xlnx: zynqmp_dpsub: Add support for live video input")
---
drivers/gpu/drm/xlnx/zynqmp_dp.c | 4 ++++
drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 4 ----
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index a0606fab0e22..9f750740dfb8 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1761,6 +1761,8 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
dpsub->dp = dp;
+ drm_bridge_add(dpsub->bridge);
+
dev_dbg(dp->dev, "ZynqMP DisplayPort Tx probed with %u lanes\n",
dp->num_lanes);
@@ -1789,4 +1791,6 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
zynqmp_dp_phy_exit(dp);
zynqmp_dp_reset(dp, true);
+
+ drm_bridge_remove(dpsub->bridge);
}
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
index 88eb33acd5f0..3933c4f1a44f 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
@@ -260,8 +260,6 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev)
ret = zynqmp_dpsub_drm_init(dpsub);
if (ret)
goto err_disp;
- } else {
- drm_bridge_add(dpsub->bridge);
}
dev_info(&pdev->dev, "ZynqMP DisplayPort Subsystem driver probed");
@@ -288,8 +286,6 @@ static void zynqmp_dpsub_remove(struct platform_device *pdev)
if (dpsub->drm)
zynqmp_dpsub_drm_cleanup(dpsub);
- else
- drm_bridge_remove(dpsub->bridge);
zynqmp_disp_remove(dpsub);
zynqmp_dp_remove(dpsub);
---
base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
change-id: 20240312-xilinx-dp-lock-fix-cf68f43a7bab
Best regards,
--
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm: xlnx: zynqmp_dpsub: Fix missing drm_bridge_add() call
2024-03-12 8:51 ` Tomi Valkeinen
@ 2024-04-26 9:25 ` Laurent Pinchart
-1 siblings, 0 replies; 4+ messages in thread
From: Laurent Pinchart @ 2024-04-26 9:25 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Michal Simek, dri-devel,
linux-arm-kernel, linux-kernel, Vishal Sagar, Anatoliy Klymenko
Hi Tomi,
Thank you for the patch.
On Tue, Mar 12, 2024 at 10:51:15AM +0200, Tomi Valkeinen wrote:
> The driver creates a bridge, but never calls drm_bridge_add() when
> non-live input is used. This leaves the bridge's hpd_mutex
> uninitialized, leading to:
>
> WARNING: CPU: 0 PID: 9 at kernel/locking/mutex.c:582 __mutex_lock+0x708/0x840
>
> Add the bridge add & remove calls so that the bridge gets managed
> correctly.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Fixes: 561671612394 ("drm: xlnx: zynqmp_dpsub: Add support for live video input")
> ---
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 4 ++++
> drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 4 ----
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index a0606fab0e22..9f750740dfb8 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1761,6 +1761,8 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
>
> dpsub->dp = dp;
>
> + drm_bridge_add(dpsub->bridge);
> +
This means that the bridge will be exposed to users before
zynqmp_disp_probe() is called, opening the door to a potential
use-before-init. The risk is mostly theoretical at this point I believe,
but it's still not a direction I'd like to that. Could you call
drm_bridge_add() in zynqmp_dpsub_probe(), between zynqmp_disp_probe()
and zynqmp_dpsub_drm_init() ?
> dev_dbg(dp->dev, "ZynqMP DisplayPort Tx probed with %u lanes\n",
> dp->num_lanes);
>
> @@ -1789,4 +1791,6 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
>
> zynqmp_dp_phy_exit(dp);
> zynqmp_dp_reset(dp, true);
> +
> + drm_bridge_remove(dpsub->bridge);
> }
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> index 88eb33acd5f0..3933c4f1a44f 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> @@ -260,8 +260,6 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev)
> ret = zynqmp_dpsub_drm_init(dpsub);
> if (ret)
> goto err_disp;
> - } else {
> - drm_bridge_add(dpsub->bridge);
> }
>
> dev_info(&pdev->dev, "ZynqMP DisplayPort Subsystem driver probed");
> @@ -288,8 +286,6 @@ static void zynqmp_dpsub_remove(struct platform_device *pdev)
>
> if (dpsub->drm)
> zynqmp_dpsub_drm_cleanup(dpsub);
> - else
> - drm_bridge_remove(dpsub->bridge);
>
> zynqmp_disp_remove(dpsub);
> zynqmp_dp_remove(dpsub);
>
> ---
> base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
> change-id: 20240312-xilinx-dp-lock-fix-cf68f43a7bab
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm: xlnx: zynqmp_dpsub: Fix missing drm_bridge_add() call
@ 2024-04-26 9:25 ` Laurent Pinchart
0 siblings, 0 replies; 4+ messages in thread
From: Laurent Pinchart @ 2024-04-26 9:25 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Michal Simek, dri-devel,
linux-arm-kernel, linux-kernel, Vishal Sagar, Anatoliy Klymenko
Hi Tomi,
Thank you for the patch.
On Tue, Mar 12, 2024 at 10:51:15AM +0200, Tomi Valkeinen wrote:
> The driver creates a bridge, but never calls drm_bridge_add() when
> non-live input is used. This leaves the bridge's hpd_mutex
> uninitialized, leading to:
>
> WARNING: CPU: 0 PID: 9 at kernel/locking/mutex.c:582 __mutex_lock+0x708/0x840
>
> Add the bridge add & remove calls so that the bridge gets managed
> correctly.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Fixes: 561671612394 ("drm: xlnx: zynqmp_dpsub: Add support for live video input")
> ---
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 4 ++++
> drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 4 ----
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index a0606fab0e22..9f750740dfb8 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1761,6 +1761,8 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
>
> dpsub->dp = dp;
>
> + drm_bridge_add(dpsub->bridge);
> +
This means that the bridge will be exposed to users before
zynqmp_disp_probe() is called, opening the door to a potential
use-before-init. The risk is mostly theoretical at this point I believe,
but it's still not a direction I'd like to that. Could you call
drm_bridge_add() in zynqmp_dpsub_probe(), between zynqmp_disp_probe()
and zynqmp_dpsub_drm_init() ?
> dev_dbg(dp->dev, "ZynqMP DisplayPort Tx probed with %u lanes\n",
> dp->num_lanes);
>
> @@ -1789,4 +1791,6 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
>
> zynqmp_dp_phy_exit(dp);
> zynqmp_dp_reset(dp, true);
> +
> + drm_bridge_remove(dpsub->bridge);
> }
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> index 88eb33acd5f0..3933c4f1a44f 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c
> @@ -260,8 +260,6 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev)
> ret = zynqmp_dpsub_drm_init(dpsub);
> if (ret)
> goto err_disp;
> - } else {
> - drm_bridge_add(dpsub->bridge);
> }
>
> dev_info(&pdev->dev, "ZynqMP DisplayPort Subsystem driver probed");
> @@ -288,8 +286,6 @@ static void zynqmp_dpsub_remove(struct platform_device *pdev)
>
> if (dpsub->drm)
> zynqmp_dpsub_drm_cleanup(dpsub);
> - else
> - drm_bridge_remove(dpsub->bridge);
>
> zynqmp_disp_remove(dpsub);
> zynqmp_dp_remove(dpsub);
>
> ---
> base-commit: e8f897f4afef0031fe618a8e94127a0934896aba
> change-id: 20240312-xilinx-dp-lock-fix-cf68f43a7bab
--
Regards,
Laurent Pinchart
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-26 9:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-12 8:51 [PATCH] drm: xlnx: zynqmp_dpsub: Fix missing drm_bridge_add() call Tomi Valkeinen
2024-03-12 8:51 ` Tomi Valkeinen
2024-04-26 9:25 ` Laurent Pinchart
2024-04-26 9:25 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.