[v5,15/21] drm: bridge: dw_hdmi: Add common suspend helper
Commit Message
Add a dw_hdmi_suspend() helper as the suspend-side counterpart to
dw_hdmi_resume() and call it from the i.MX8MP, Amlogic and Rockchip
platform PM suspend callbacks.
This is a preparatory change to route suspend handling through common
dw-hdmi code so follow-up changes can add shared suspend cleanup in one
place.
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
v5: New patch
---
drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c | 4 ++++
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 5 +++++
drivers/gpu/drm/meson/meson_dw_hdmi.c | 2 ++
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 11 ++++++++++-
include/drm/bridge/dw_hdmi.h | 1 +
5 files changed, 22 insertions(+), 1 deletion(-)
Comments
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] NULL pointer dereference in dw_hdmi_rockchip_suspend if system suspends before component bind.
--
commit abc5ed0537352dd111abc436d5fc9ba8787addd8
Author: Jonas Karlman <jonas@kwiboo.se>
drm: bridge: dw_hdmi: Add common suspend helper
This commit adds a dw_hdmi_suspend() helper as the suspend-side counterpart
to dw_hdmi_resume() and calls it from the i.MX8MP, Amlogic and Rockchip
platform PM suspend callbacks. This routes suspend handling through common
dw-hdmi code.
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index 0dc1eb5d2ae39..9677424075eba 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -651,6 +651,15 @@ static void dw_hdmi_rockchip_remove(struct platform_device *pdev)
> component_del(&pdev->dev, &dw_hdmi_rockchip_ops);
> }
>
> +static int __maybe_unused dw_hdmi_rockchip_suspend(struct device *dev)
> +{
> + struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
> +
> + dw_hdmi_suspend(hdmi->hdmi);
Could hdmi be NULL here if the system suspends before the component is bound?
In dw_hdmi-rockchip.c, the probe function calls component_add(), so the
platform device is registered before the driver's private data is allocated
and set via platform_set_drvdata() during the bind callback.
If a system suspend occurs after probe but before bind (e.g., if the DRM
master is deferred waiting on other components), dev_get_drvdata(dev) will
return NULL. Accessing hdmi->hdmi would then cause a NULL pointer
dereference.
Although the pre-existing dw_hdmi_rockchip_resume() function suffers from the
same missing check, this patch introduces the issue into the newly added
suspend path.
Should an explicit check for !hdmi be added here, similar to how it is
handled in meson_dw_hdmi_pm_suspend()?
> +
> + return 0;
> +}
> +
> static int __maybe_unused dw_hdmi_rockchip_resume(struct device *dev)
> {
@@ -176,6 +176,10 @@ static void imx8mp_dw_hdmi_remove(struct platform_device *pdev)
static int imx8mp_dw_hdmi_pm_suspend(struct device *dev)
{
+ struct imx8mp_hdmi *hdmi = dev_get_drvdata(dev);
+
+ dw_hdmi_suspend(hdmi->dw_hdmi);
+
return 0;
}
@@ -3568,6 +3568,11 @@ void dw_hdmi_unbind(struct dw_hdmi *hdmi)
}
EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
+void dw_hdmi_suspend(struct dw_hdmi *hdmi)
+{
+}
+EXPORT_SYMBOL_GPL(dw_hdmi_suspend);
+
void dw_hdmi_resume(struct dw_hdmi *hdmi)
{
dw_hdmi_init_hw(hdmi);
@@ -809,6 +809,8 @@ static int __maybe_unused meson_dw_hdmi_pm_suspend(struct device *dev)
if (!meson_dw_hdmi)
return 0;
+ dw_hdmi_suspend(meson_dw_hdmi->hdmi);
+
/* Reset TOP */
meson_dw_hdmi->data->top_write(meson_dw_hdmi,
HDMITX_TOP_SW_RESET, 0);
@@ -651,6 +651,15 @@ static void dw_hdmi_rockchip_remove(struct platform_device *pdev)
component_del(&pdev->dev, &dw_hdmi_rockchip_ops);
}
+static int __maybe_unused dw_hdmi_rockchip_suspend(struct device *dev)
+{
+ struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
+
+ dw_hdmi_suspend(hdmi->hdmi);
+
+ return 0;
+}
+
static int __maybe_unused dw_hdmi_rockchip_resume(struct device *dev)
{
struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
@@ -661,7 +670,7 @@ static int __maybe_unused dw_hdmi_rockchip_resume(struct device *dev)
}
static const struct dev_pm_ops dw_hdmi_rockchip_pm = {
- SET_SYSTEM_SLEEP_PM_OPS(NULL, dw_hdmi_rockchip_resume)
+ SET_SYSTEM_SLEEP_PM_OPS(dw_hdmi_rockchip_suspend, dw_hdmi_rockchip_resume)
};
struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
@@ -184,6 +184,7 @@ struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
struct drm_encoder *encoder,
const struct dw_hdmi_plat_data *plat_data);
+void dw_hdmi_suspend(struct dw_hdmi *hdmi);
void dw_hdmi_resume(struct dw_hdmi *hdmi);
void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);