* [PATCH v7 1/8] drm/bridge: analogix: Do not use device's drvdata
2018-01-09 14:48 [PATCH v7 00/10] rockchip: kevin: Enable edp display Thierry Escande
@ 2018-01-09 14:48 ` Thierry Escande
2018-01-09 14:48 ` [PATCH v7 3/8] drm/rockchip: analogix_dp: Add a sanity check for rockchip_drm_psr_register() Thierry Escande
` (5 subsequent siblings)
6 siblings, 0 replies; 22+ messages in thread
From: Thierry Escande @ 2018-01-09 14:48 UTC (permalink / raw)
To: Rob Herring, Archit Taneja, Daniel Vetter, Neil Armstrong,
Laurent Pinchart, Sandy Huang
Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jeffy Chen,
Sean Paul, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
From: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
The driver that instantiates the bridge should own the drvdata, as all
driver model callbacks (probe, remove, shutdown, PM ops, etc.) are also
owned by its driver struct. Moreover, storing two different pointer
types in driver data depending on driver initialization status is barely
a good practice and in fact has led to many bugs in this driver.
Let's clean up this mess and change Analogix entry points to simply
accept some opaque struct pointer, adjusting their users at the same
time to avoid breaking the compilation.
Signed-off-by: Tomasz Figa <tfiga-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Signed-off-by: Thierry Escande <thierry.escande-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Reviewed-by: Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Reviewed-by: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Acked-by: Jingoo Han <jingoohan1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 50 +++++++++-------------
drivers/gpu/drm/exynos/exynos_dp.c | 26 ++++++-----
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 48 ++++++++++++---------
include/drm/bridge/analogix_dp.h | 19 ++++----
4 files changed, 74 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index a8905049b9da..878f61b65cb2 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -98,17 +98,15 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
return 0;
}
-int analogix_dp_psr_supported(struct device *dev)
+int analogix_dp_psr_supported(struct analogix_dp_device *dp)
{
- struct analogix_dp_device *dp = dev_get_drvdata(dev);
return dp->psr_support;
}
EXPORT_SYMBOL_GPL(analogix_dp_psr_supported);
-int analogix_dp_enable_psr(struct device *dev)
+int analogix_dp_enable_psr(struct analogix_dp_device *dp)
{
- struct analogix_dp_device *dp = dev_get_drvdata(dev);
struct edp_vsc_psr psr_vsc;
if (!dp->psr_support)
@@ -129,9 +127,8 @@ int analogix_dp_enable_psr(struct device *dev)
}
EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
-int analogix_dp_disable_psr(struct device *dev)
+int analogix_dp_disable_psr(struct analogix_dp_device *dp)
{
- struct analogix_dp_device *dp = dev_get_drvdata(dev);
struct edp_vsc_psr psr_vsc;
int ret;
@@ -1283,8 +1280,9 @@ static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux,
return analogix_dp_transfer(dp, msg);
}
-int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
- struct analogix_dp_plat_data *plat_data)
+struct analogix_dp_device *
+analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
+ struct analogix_dp_plat_data *plat_data)
{
struct platform_device *pdev = to_platform_device(dev);
struct analogix_dp_device *dp;
@@ -1294,14 +1292,12 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
if (!plat_data) {
dev_err(dev, "Invalided input plat_data\n");
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
}
dp = devm_kzalloc(dev, sizeof(struct analogix_dp_device), GFP_KERNEL);
if (!dp)
- return -ENOMEM;
-
- dev_set_drvdata(dev, dp);
+ return ERR_PTR(-ENOMEM);
dp->dev = &pdev->dev;
dp->dpms_mode = DRM_MODE_DPMS_OFF;
@@ -1318,7 +1314,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
ret = analogix_dp_dt_parse_pdata(dp);
if (ret)
- return ret;
+ return ERR_PTR(ret);
dp->phy = devm_phy_get(dp->dev, "dp");
if (IS_ERR(dp->phy)) {
@@ -1332,14 +1328,14 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
if (ret == -ENOSYS || ret == -ENODEV)
dp->phy = NULL;
else
- return ret;
+ return ERR_PTR(ret);
}
}
dp->clock = devm_clk_get(&pdev->dev, "dp");
if (IS_ERR(dp->clock)) {
dev_err(&pdev->dev, "failed to get clock\n");
- return PTR_ERR(dp->clock);
+ return ERR_CAST(dp->clock);
}
clk_prepare_enable(dp->clock);
@@ -1348,7 +1344,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
dp->reg_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(dp->reg_base))
- return PTR_ERR(dp->reg_base);
+ return ERR_CAST(dp->reg_base);
dp->force_hpd = of_property_read_bool(dev->of_node, "force-hpd");
@@ -1369,7 +1365,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
"hpd_gpio");
if (ret) {
dev_err(&pdev->dev, "failed to get hpd gpio\n");
- return ret;
+ return ERR_PTR(ret);
}
dp->irq = gpio_to_irq(dp->hpd_gpio);
irq_flags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
@@ -1381,7 +1377,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
if (dp->irq == -ENXIO) {
dev_err(&pdev->dev, "failed to get irq\n");
- return -ENODEV;
+ return ERR_PTR(-ENODEV);
}
pm_runtime_enable(dev);
@@ -1422,7 +1418,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
phy_power_off(dp->phy);
pm_runtime_put(dev);
- return 0;
+ return dp;
err_disable_pm_runtime:
@@ -1430,15 +1426,12 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
pm_runtime_put(dev);
pm_runtime_disable(dev);
- return ret;
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(analogix_dp_bind);
-void analogix_dp_unbind(struct device *dev, struct device *master,
- void *data)
+void analogix_dp_unbind(struct analogix_dp_device *dp)
{
- struct analogix_dp_device *dp = dev_get_drvdata(dev);
-
analogix_dp_bridge_disable(dp->bridge);
dp->connector.funcs->destroy(&dp->connector);
dp->encoder->funcs->destroy(dp->encoder);
@@ -1451,16 +1444,14 @@ void analogix_dp_unbind(struct device *dev, struct device *master,
}
drm_dp_aux_unregister(&dp->aux);
- pm_runtime_disable(dev);
+ pm_runtime_disable(dp->dev);
clk_disable_unprepare(dp->clock);
}
EXPORT_SYMBOL_GPL(analogix_dp_unbind);
#ifdef CONFIG_PM
-int analogix_dp_suspend(struct device *dev)
+int analogix_dp_suspend(struct analogix_dp_device *dp)
{
- struct analogix_dp_device *dp = dev_get_drvdata(dev);
-
clk_disable_unprepare(dp->clock);
if (dp->plat_data->panel) {
@@ -1472,9 +1463,8 @@ int analogix_dp_suspend(struct device *dev)
}
EXPORT_SYMBOL_GPL(analogix_dp_suspend);
-int analogix_dp_resume(struct device *dev)
+int analogix_dp_resume(struct analogix_dp_device *dp)
{
- struct analogix_dp_device *dp = dev_get_drvdata(dev);
int ret;
ret = clk_prepare_enable(dp->clock);
diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index 39629e7a80b9..f7e5b2c405ed 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -41,6 +41,7 @@ struct exynos_dp_device {
struct device *dev;
struct videomode vm;
+ struct analogix_dp_device *adp;
struct analogix_dp_plat_data plat_data;
};
@@ -157,13 +158,6 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
struct drm_device *drm_dev = data;
int ret;
- /*
- * Just like the probe function said, we don't need the
- * device drvrate anymore, we should leave the charge to
- * analogix dp driver, set the device drvdata to NULL.
- */
- dev_set_drvdata(dev, NULL);
-
dp->dev = dev;
dp->drm_dev = drm_dev;
@@ -190,13 +184,19 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
dp->plat_data.encoder = encoder;
- return analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
+ dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
+ if (IS_ERR(dp->adp))
+ return PTR_ERR(dp->adp);
+
+ return 0;
}
static void exynos_dp_unbind(struct device *dev, struct device *master,
void *data)
{
- return analogix_dp_unbind(dev, master, data);
+ struct exynos_dp_device *dp = dev_get_drvdata(dev);
+
+ return analogix_dp_unbind(dp->adp);
}
static const struct component_ops exynos_dp_ops = {
@@ -257,12 +257,16 @@ static int exynos_dp_remove(struct platform_device *pdev)
#ifdef CONFIG_PM
static int exynos_dp_suspend(struct device *dev)
{
- return analogix_dp_suspend(dev);
+ struct exynos_dp_device *dp = dev_get_drvdata(dev);
+
+ return analogix_dp_suspend(dp->adp);
}
static int exynos_dp_resume(struct device *dev)
{
- return analogix_dp_resume(dev);
+ struct exynos_dp_device *dp = dev_get_drvdata(dev);
+
+ return analogix_dp_resume(dp->adp);
}
#endif
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 1262120a3834..8a58ad80f509 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -77,6 +77,7 @@ struct rockchip_dp_device {
const struct rockchip_dp_chip_data *data;
+ struct analogix_dp_device *adp;
struct analogix_dp_plat_data plat_data;
};
@@ -84,7 +85,7 @@ static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled)
{
struct rockchip_dp_device *dp = to_dp(encoder);
- if (!analogix_dp_psr_supported(dp->dev))
+ if (!analogix_dp_psr_supported(dp->adp))
return;
DRM_DEV_DEBUG(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit");
@@ -114,9 +115,9 @@ static void analogix_dp_psr_work(struct work_struct *work)
mutex_lock(&dp->psr_lock);
if (dp->psr_state == EDP_VSC_PSR_STATE_ACTIVE)
- analogix_dp_enable_psr(dp->dev);
+ analogix_dp_enable_psr(dp->adp);
else
- analogix_dp_disable_psr(dp->dev);
+ analogix_dp_disable_psr(dp->adp);
mutex_unlock(&dp->psr_lock);
}
@@ -334,13 +335,6 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
struct drm_device *drm_dev = data;
int ret;
- /*
- * Just like the probe function said, we don't need the
- * device drvrate anymore, we should leave the charge to
- * analogix dp driver, set the device drvdata to NULL.
- */
- dev_set_drvdata(dev, NULL);
-
dp_data = of_device_get_match_data(dev);
if (!dp_data)
return -ENODEV;
@@ -367,7 +361,11 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
rockchip_drm_psr_register(&dp->encoder, analogix_dp_psr_set);
- return analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
+ dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
+ if (IS_ERR(dp->adp))
+ return PTR_ERR(dp->adp);
+
+ return 0;
}
static void rockchip_dp_unbind(struct device *dev, struct device *master,
@@ -376,8 +374,7 @@ static void rockchip_dp_unbind(struct device *dev, struct device *master,
struct rockchip_dp_device *dp = dev_get_drvdata(dev);
rockchip_drm_psr_unregister(&dp->encoder);
-
- analogix_dp_unbind(dev, master, data);
+ analogix_dp_unbind(dp->adp);
}
static const struct component_ops rockchip_dp_component_ops = {
@@ -407,11 +404,6 @@ static int rockchip_dp_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
- /*
- * We just use the drvdata until driver run into component
- * add function, and then we would set drvdata to null, so
- * that analogix dp driver could take charge of the drvdata.
- */
platform_set_drvdata(pdev, dp);
return component_add(dev, &rockchip_dp_component_ops);
@@ -424,10 +416,26 @@ static int rockchip_dp_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM_SLEEP
+static int rockchip_dp_suspend(struct device *dev)
+{
+ struct rockchip_dp_device *dp = dev_get_drvdata(dev);
+
+ return analogix_dp_suspend(dp->adp);
+}
+
+static int rockchip_dp_resume(struct device *dev)
+{
+ struct rockchip_dp_device *dp = dev_get_drvdata(dev);
+
+ return analogix_dp_resume(dp->adp);
+}
+#endif
+
static const struct dev_pm_ops rockchip_dp_pm_ops = {
#ifdef CONFIG_PM_SLEEP
- .suspend = analogix_dp_suspend,
- .resume_early = analogix_dp_resume,
+ .suspend = rockchip_dp_suspend,
+ .resume_early = rockchip_dp_resume,
#endif
};
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index c99d6eaef1ac..5518fc75dd6e 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -13,6 +13,8 @@
#include <drm/drm_crtc.h>
+struct analogix_dp_device;
+
enum analogix_dp_devtype {
EXYNOS_DP,
RK3288_DP,
@@ -38,16 +40,17 @@ struct analogix_dp_plat_data {
struct drm_connector *);
};
-int analogix_dp_psr_supported(struct device *dev);
-int analogix_dp_enable_psr(struct device *dev);
-int analogix_dp_disable_psr(struct device *dev);
+int analogix_dp_psr_supported(struct analogix_dp_device *dp);
+int analogix_dp_enable_psr(struct analogix_dp_device *dp);
+int analogix_dp_disable_psr(struct analogix_dp_device *dp);
-int analogix_dp_resume(struct device *dev);
-int analogix_dp_suspend(struct device *dev);
+int analogix_dp_resume(struct analogix_dp_device *dp);
+int analogix_dp_suspend(struct analogix_dp_device *dp);
-int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
- struct analogix_dp_plat_data *plat_data);
-void analogix_dp_unbind(struct device *dev, struct device *master, void *data);
+struct analogix_dp_device *
+analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
+ struct analogix_dp_plat_data *plat_data);
+void analogix_dp_unbind(struct analogix_dp_device *dp);
int analogix_dp_start_crc(struct drm_connector *connector);
int analogix_dp_stop_crc(struct drm_connector *connector);
--
2.14.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 1/8] drm/bridge: analogix: Do not use device's drvdata
@ 2018-01-09 14:48 ` Thierry Escande
0 siblings, 0 replies; 22+ messages in thread
From: Thierry Escande @ 2018-01-09 14:48 UTC (permalink / raw)
To: Rob Herring, Archit Taneja, Daniel Vetter, Neil Armstrong,
Laurent Pinchart, Sandy Huang
Cc: Jeffy Chen, Sean Paul, dri-devel, linux-rockchip, linux-kernel
From: Jeffy Chen <jeffy.chen@rock-chips.com>
The driver that instantiates the bridge should own the drvdata, as all
driver model callbacks (probe, remove, shutdown, PM ops, etc.) are also
owned by its driver struct. Moreover, storing two different pointer
types in driver data depending on driver initialization status is barely
a good practice and in fact has led to many bugs in this driver.
Let's clean up this mess and change Analogix entry points to simply
accept some opaque struct pointer, adjusting their users at the same
time to avoid breaking the compilation.
Signed-off-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Acked-by: Jingoo Han <jingoohan1@gmail.com>
Acked-by: Archit Taneja <architt@codeaurora.org>
---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 50 +++++++++-------------
drivers/gpu/drm/exynos/exynos_dp.c | 26 ++++++-----
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 48 ++++++++++++---------
include/drm/bridge/analogix_dp.h | 19 ++++----
4 files changed, 74 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index a8905049b9da..878f61b65cb2 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -98,17 +98,15 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
return 0;
}
-int analogix_dp_psr_supported(struct device *dev)
+int analogix_dp_psr_supported(struct analogix_dp_device *dp)
{
- struct analogix_dp_device *dp = dev_get_drvdata(dev);
return dp->psr_support;
}
EXPORT_SYMBOL_GPL(analogix_dp_psr_supported);
-int analogix_dp_enable_psr(struct device *dev)
+int analogix_dp_enable_psr(struct analogix_dp_device *dp)
{
- struct analogix_dp_device *dp = dev_get_drvdata(dev);
struct edp_vsc_psr psr_vsc;
if (!dp->psr_support)
@@ -129,9 +127,8 @@ int analogix_dp_enable_psr(struct device *dev)
}
EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
-int analogix_dp_disable_psr(struct device *dev)
+int analogix_dp_disable_psr(struct analogix_dp_device *dp)
{
- struct analogix_dp_device *dp = dev_get_drvdata(dev);
struct edp_vsc_psr psr_vsc;
int ret;
@@ -1283,8 +1280,9 @@ static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux,
return analogix_dp_transfer(dp, msg);
}
-int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
- struct analogix_dp_plat_data *plat_data)
+struct analogix_dp_device *
+analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
+ struct analogix_dp_plat_data *plat_data)
{
struct platform_device *pdev = to_platform_device(dev);
struct analogix_dp_device *dp;
@@ -1294,14 +1292,12 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
if (!plat_data) {
dev_err(dev, "Invalided input plat_data\n");
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
}
dp = devm_kzalloc(dev, sizeof(struct analogix_dp_device), GFP_KERNEL);
if (!dp)
- return -ENOMEM;
-
- dev_set_drvdata(dev, dp);
+ return ERR_PTR(-ENOMEM);
dp->dev = &pdev->dev;
dp->dpms_mode = DRM_MODE_DPMS_OFF;
@@ -1318,7 +1314,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
ret = analogix_dp_dt_parse_pdata(dp);
if (ret)
- return ret;
+ return ERR_PTR(ret);
dp->phy = devm_phy_get(dp->dev, "dp");
if (IS_ERR(dp->phy)) {
@@ -1332,14 +1328,14 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
if (ret == -ENOSYS || ret == -ENODEV)
dp->phy = NULL;
else
- return ret;
+ return ERR_PTR(ret);
}
}
dp->clock = devm_clk_get(&pdev->dev, "dp");
if (IS_ERR(dp->clock)) {
dev_err(&pdev->dev, "failed to get clock\n");
- return PTR_ERR(dp->clock);
+ return ERR_CAST(dp->clock);
}
clk_prepare_enable(dp->clock);
@@ -1348,7 +1344,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
dp->reg_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(dp->reg_base))
- return PTR_ERR(dp->reg_base);
+ return ERR_CAST(dp->reg_base);
dp->force_hpd = of_property_read_bool(dev->of_node, "force-hpd");
@@ -1369,7 +1365,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
"hpd_gpio");
if (ret) {
dev_err(&pdev->dev, "failed to get hpd gpio\n");
- return ret;
+ return ERR_PTR(ret);
}
dp->irq = gpio_to_irq(dp->hpd_gpio);
irq_flags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
@@ -1381,7 +1377,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
if (dp->irq == -ENXIO) {
dev_err(&pdev->dev, "failed to get irq\n");
- return -ENODEV;
+ return ERR_PTR(-ENODEV);
}
pm_runtime_enable(dev);
@@ -1422,7 +1418,7 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
phy_power_off(dp->phy);
pm_runtime_put(dev);
- return 0;
+ return dp;
err_disable_pm_runtime:
@@ -1430,15 +1426,12 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
pm_runtime_put(dev);
pm_runtime_disable(dev);
- return ret;
+ return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(analogix_dp_bind);
-void analogix_dp_unbind(struct device *dev, struct device *master,
- void *data)
+void analogix_dp_unbind(struct analogix_dp_device *dp)
{
- struct analogix_dp_device *dp = dev_get_drvdata(dev);
-
analogix_dp_bridge_disable(dp->bridge);
dp->connector.funcs->destroy(&dp->connector);
dp->encoder->funcs->destroy(dp->encoder);
@@ -1451,16 +1444,14 @@ void analogix_dp_unbind(struct device *dev, struct device *master,
}
drm_dp_aux_unregister(&dp->aux);
- pm_runtime_disable(dev);
+ pm_runtime_disable(dp->dev);
clk_disable_unprepare(dp->clock);
}
EXPORT_SYMBOL_GPL(analogix_dp_unbind);
#ifdef CONFIG_PM
-int analogix_dp_suspend(struct device *dev)
+int analogix_dp_suspend(struct analogix_dp_device *dp)
{
- struct analogix_dp_device *dp = dev_get_drvdata(dev);
-
clk_disable_unprepare(dp->clock);
if (dp->plat_data->panel) {
@@ -1472,9 +1463,8 @@ int analogix_dp_suspend(struct device *dev)
}
EXPORT_SYMBOL_GPL(analogix_dp_suspend);
-int analogix_dp_resume(struct device *dev)
+int analogix_dp_resume(struct analogix_dp_device *dp)
{
- struct analogix_dp_device *dp = dev_get_drvdata(dev);
int ret;
ret = clk_prepare_enable(dp->clock);
diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index 39629e7a80b9..f7e5b2c405ed 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -41,6 +41,7 @@ struct exynos_dp_device {
struct device *dev;
struct videomode vm;
+ struct analogix_dp_device *adp;
struct analogix_dp_plat_data plat_data;
};
@@ -157,13 +158,6 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
struct drm_device *drm_dev = data;
int ret;
- /*
- * Just like the probe function said, we don't need the
- * device drvrate anymore, we should leave the charge to
- * analogix dp driver, set the device drvdata to NULL.
- */
- dev_set_drvdata(dev, NULL);
-
dp->dev = dev;
dp->drm_dev = drm_dev;
@@ -190,13 +184,19 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
dp->plat_data.encoder = encoder;
- return analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
+ dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
+ if (IS_ERR(dp->adp))
+ return PTR_ERR(dp->adp);
+
+ return 0;
}
static void exynos_dp_unbind(struct device *dev, struct device *master,
void *data)
{
- return analogix_dp_unbind(dev, master, data);
+ struct exynos_dp_device *dp = dev_get_drvdata(dev);
+
+ return analogix_dp_unbind(dp->adp);
}
static const struct component_ops exynos_dp_ops = {
@@ -257,12 +257,16 @@ static int exynos_dp_remove(struct platform_device *pdev)
#ifdef CONFIG_PM
static int exynos_dp_suspend(struct device *dev)
{
- return analogix_dp_suspend(dev);
+ struct exynos_dp_device *dp = dev_get_drvdata(dev);
+
+ return analogix_dp_suspend(dp->adp);
}
static int exynos_dp_resume(struct device *dev)
{
- return analogix_dp_resume(dev);
+ struct exynos_dp_device *dp = dev_get_drvdata(dev);
+
+ return analogix_dp_resume(dp->adp);
}
#endif
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 1262120a3834..8a58ad80f509 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -77,6 +77,7 @@ struct rockchip_dp_device {
const struct rockchip_dp_chip_data *data;
+ struct analogix_dp_device *adp;
struct analogix_dp_plat_data plat_data;
};
@@ -84,7 +85,7 @@ static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled)
{
struct rockchip_dp_device *dp = to_dp(encoder);
- if (!analogix_dp_psr_supported(dp->dev))
+ if (!analogix_dp_psr_supported(dp->adp))
return;
DRM_DEV_DEBUG(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit");
@@ -114,9 +115,9 @@ static void analogix_dp_psr_work(struct work_struct *work)
mutex_lock(&dp->psr_lock);
if (dp->psr_state == EDP_VSC_PSR_STATE_ACTIVE)
- analogix_dp_enable_psr(dp->dev);
+ analogix_dp_enable_psr(dp->adp);
else
- analogix_dp_disable_psr(dp->dev);
+ analogix_dp_disable_psr(dp->adp);
mutex_unlock(&dp->psr_lock);
}
@@ -334,13 +335,6 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
struct drm_device *drm_dev = data;
int ret;
- /*
- * Just like the probe function said, we don't need the
- * device drvrate anymore, we should leave the charge to
- * analogix dp driver, set the device drvdata to NULL.
- */
- dev_set_drvdata(dev, NULL);
-
dp_data = of_device_get_match_data(dev);
if (!dp_data)
return -ENODEV;
@@ -367,7 +361,11 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
rockchip_drm_psr_register(&dp->encoder, analogix_dp_psr_set);
- return analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
+ dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
+ if (IS_ERR(dp->adp))
+ return PTR_ERR(dp->adp);
+
+ return 0;
}
static void rockchip_dp_unbind(struct device *dev, struct device *master,
@@ -376,8 +374,7 @@ static void rockchip_dp_unbind(struct device *dev, struct device *master,
struct rockchip_dp_device *dp = dev_get_drvdata(dev);
rockchip_drm_psr_unregister(&dp->encoder);
-
- analogix_dp_unbind(dev, master, data);
+ analogix_dp_unbind(dp->adp);
}
static const struct component_ops rockchip_dp_component_ops = {
@@ -407,11 +404,6 @@ static int rockchip_dp_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
- /*
- * We just use the drvdata until driver run into component
- * add function, and then we would set drvdata to null, so
- * that analogix dp driver could take charge of the drvdata.
- */
platform_set_drvdata(pdev, dp);
return component_add(dev, &rockchip_dp_component_ops);
@@ -424,10 +416,26 @@ static int rockchip_dp_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM_SLEEP
+static int rockchip_dp_suspend(struct device *dev)
+{
+ struct rockchip_dp_device *dp = dev_get_drvdata(dev);
+
+ return analogix_dp_suspend(dp->adp);
+}
+
+static int rockchip_dp_resume(struct device *dev)
+{
+ struct rockchip_dp_device *dp = dev_get_drvdata(dev);
+
+ return analogix_dp_resume(dp->adp);
+}
+#endif
+
static const struct dev_pm_ops rockchip_dp_pm_ops = {
#ifdef CONFIG_PM_SLEEP
- .suspend = analogix_dp_suspend,
- .resume_early = analogix_dp_resume,
+ .suspend = rockchip_dp_suspend,
+ .resume_early = rockchip_dp_resume,
#endif
};
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index c99d6eaef1ac..5518fc75dd6e 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -13,6 +13,8 @@
#include <drm/drm_crtc.h>
+struct analogix_dp_device;
+
enum analogix_dp_devtype {
EXYNOS_DP,
RK3288_DP,
@@ -38,16 +40,17 @@ struct analogix_dp_plat_data {
struct drm_connector *);
};
-int analogix_dp_psr_supported(struct device *dev);
-int analogix_dp_enable_psr(struct device *dev);
-int analogix_dp_disable_psr(struct device *dev);
+int analogix_dp_psr_supported(struct analogix_dp_device *dp);
+int analogix_dp_enable_psr(struct analogix_dp_device *dp);
+int analogix_dp_disable_psr(struct analogix_dp_device *dp);
-int analogix_dp_resume(struct device *dev);
-int analogix_dp_suspend(struct device *dev);
+int analogix_dp_resume(struct analogix_dp_device *dp);
+int analogix_dp_suspend(struct analogix_dp_device *dp);
-int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
- struct analogix_dp_plat_data *plat_data);
-void analogix_dp_unbind(struct device *dev, struct device *master, void *data);
+struct analogix_dp_device *
+analogix_dp_bind(struct device *dev, struct drm_device *drm_dev,
+ struct analogix_dp_plat_data *plat_data);
+void analogix_dp_unbind(struct analogix_dp_device *dp);
int analogix_dp_start_crc(struct drm_connector *connector);
int analogix_dp_stop_crc(struct drm_connector *connector);
--
2.14.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v7 7/8] drm/bridge/synopsys: dw-hdmi: Do not use device's drvdata
2018-01-09 14:48 [PATCH v7 00/10] rockchip: kevin: Enable edp display Thierry Escande
@ 2018-01-09 14:48 ` Thierry Escande
2018-01-09 14:48 ` [PATCH v7 3/8] drm/rockchip: analogix_dp: Add a sanity check for rockchip_drm_psr_register() Thierry Escande
` (5 subsequent siblings)
6 siblings, 0 replies; 22+ messages in thread
From: Thierry Escande @ 2018-01-09 14:48 UTC (permalink / raw)
To: Rob Herring, Archit Taneja, Daniel Vetter, Neil Armstrong,
Laurent Pinchart, Sandy Huang
Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jeffy Chen,
Sean Paul, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
From: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Let plat drivers own the drvdata, so that they could cleanup resources
in their unbind().
Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Signed-off-by: Thierry Escande <thierry.escande-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Reviewed-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 43 ++++++++++-------------------
drivers/gpu/drm/imx/dw_hdmi-imx.c | 22 +++++++++------
drivers/gpu/drm/meson/meson_dw_hdmi.c | 20 ++++++++++----
drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c | 14 ++++++++--
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 23 ++++++++-------
include/drm/bridge/dw_hdmi.h | 17 ++++++------
6 files changed, 77 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 1cc63a18b7d5..f0380bcae513 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2073,7 +2073,7 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
return ret;
}
-void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
+void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
{
mutex_lock(&hdmi->mutex);
@@ -2099,13 +2099,6 @@ void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
}
mutex_unlock(&hdmi->mutex);
}
-
-void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense)
-{
- struct dw_hdmi *hdmi = dev_get_drvdata(dev);
-
- __dw_hdmi_setup_rx_sense(hdmi, hpd, rx_sense);
-}
EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
@@ -2141,9 +2134,8 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
*/
if (intr_stat &
(HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) {
- __dw_hdmi_setup_rx_sense(hdmi,
- phy_stat & HDMI_PHY_HPD,
- phy_stat & HDMI_PHY_RX_SENSE);
+ dw_hdmi_setup_rx_sense(hdmi, phy_stat & HDMI_PHY_HPD,
+ phy_stat & HDMI_PHY_RX_SENSE);
if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
cec_notifier_set_phys_addr(hdmi->cec_notifier,
@@ -2533,8 +2525,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
if (hdmi->i2c)
dw_hdmi_i2c_init(hdmi);
- platform_set_drvdata(pdev, hdmi);
-
return hdmi;
err_iahb:
@@ -2584,25 +2574,23 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
/* -----------------------------------------------------------------------------
* Probe/remove API, used from platforms based on the DRM bridge API.
*/
-int dw_hdmi_probe(struct platform_device *pdev,
- const struct dw_hdmi_plat_data *plat_data)
+struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
+ const struct dw_hdmi_plat_data *plat_data)
{
struct dw_hdmi *hdmi;
hdmi = __dw_hdmi_probe(pdev, plat_data);
if (IS_ERR(hdmi))
- return PTR_ERR(hdmi);
+ return hdmi;
drm_bridge_add(&hdmi->bridge);
- return 0;
+ return hdmi;
}
EXPORT_SYMBOL_GPL(dw_hdmi_probe);
-void dw_hdmi_remove(struct platform_device *pdev)
+void dw_hdmi_remove(struct dw_hdmi *hdmi)
{
- struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
-
drm_bridge_remove(&hdmi->bridge);
__dw_hdmi_remove(hdmi);
@@ -2612,31 +2600,30 @@ EXPORT_SYMBOL_GPL(dw_hdmi_remove);
/* -----------------------------------------------------------------------------
* Bind/unbind API, used from platforms based on the component framework.
*/
-int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
- const struct dw_hdmi_plat_data *plat_data)
+struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
+ struct drm_encoder *encoder,
+ const struct dw_hdmi_plat_data *plat_data)
{
struct dw_hdmi *hdmi;
int ret;
hdmi = __dw_hdmi_probe(pdev, plat_data);
if (IS_ERR(hdmi))
- return PTR_ERR(hdmi);
+ return hdmi;
ret = drm_bridge_attach(encoder, &hdmi->bridge, NULL);
if (ret) {
__dw_hdmi_remove(hdmi);
DRM_ERROR("Failed to initialize bridge with drm\n");
- return ret;
+ return ERR_PTR(ret);
}
- return 0;
+ return hdmi;
}
EXPORT_SYMBOL_GPL(dw_hdmi_bind);
-void dw_hdmi_unbind(struct device *dev)
+void dw_hdmi_unbind(struct dw_hdmi *hdmi)
{
- struct dw_hdmi *hdmi = dev_get_drvdata(dev);
-
__dw_hdmi_remove(hdmi);
}
EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index b62763aa8706..b01d03e02ce0 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -26,6 +26,8 @@ struct imx_hdmi {
struct device *dev;
struct drm_encoder encoder;
struct regmap *regmap;
+
+ struct dw_hdmi *hdmi;
};
static inline struct imx_hdmi *enc_to_imx_hdmi(struct drm_encoder *e)
@@ -239,22 +241,24 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master,
drm_encoder_init(drm, encoder, &dw_hdmi_imx_encoder_funcs,
DRM_MODE_ENCODER_TMDS, NULL);
- ret = dw_hdmi_bind(pdev, encoder, plat_data);
+ hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
+ if (IS_ERR(hdmi->hdmi)) {
+ encoder->funcs->destroy(encoder);
+ return PTR_ERR(hdmi->hdmi);
+ }
- /*
- * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
- * which would have called the encoder cleanup. Do it manually.
- */
- if (ret)
- drm_encoder_cleanup(encoder);
+ dev_set_drvdata(dev, hdmi);
- return ret;
+ return 0;
}
static void dw_hdmi_imx_unbind(struct device *dev, struct device *master,
void *data)
{
- return dw_hdmi_unbind(dev);
+ struct imx_hdmi *hdmi = dev_get_drvdata(dev);
+
+ dw_hdmi_unbind(hdmi->hdmi);
+ hdmi->encoder.funcs->destroy(&hdmi->encoder);
}
static const struct component_ops dw_hdmi_imx_ops = {
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 17de3afd98f6..3f1ec9a5cbfb 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,6 +140,8 @@ struct meson_dw_hdmi {
struct clk *venci_clk;
struct regulator *hdmi_supply;
u32 irq_stat;
+
+ struct dw_hdmi *hdmi;
};
#define encoder_to_meson_dw_hdmi(x) \
container_of(x, struct meson_dw_hdmi, encoder)
@@ -528,7 +530,7 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id)
if (stat & HDMITX_TOP_INTR_HPD_RISE)
hpd_connected = true;
- dw_hdmi_setup_rx_sense(dw_hdmi->dev, hpd_connected,
+ dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected,
hpd_connected);
drm_helper_hpd_irq_event(dw_hdmi->encoder.dev);
@@ -878,9 +880,14 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
dw_plat_data->input_bus_format = MEDIA_BUS_FMT_YUV8_1X24;
dw_plat_data->input_bus_encoding = V4L2_YCBCR_ENC_709;
- ret = dw_hdmi_bind(pdev, encoder, &meson_dw_hdmi->dw_plat_data);
- if (ret)
- return ret;
+ meson_dw_hdmi->hdmi = dw_hdmi_bind(pdev, encoder,
+ &meson_dw_hdmi->dw_plat_data);
+ if (IS_ERR(meson_dw_hdmi->hdmi)) {
+ encoder->funcs->destroy(encoder);
+ return PTR_ERR(meson_dw_hdmi->hdmi);
+ }
+
+ dev_set_drvdata(dev, meson_dw_hdmi);
DRM_DEBUG_DRIVER("HDMI controller initialized\n");
@@ -890,7 +897,10 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
static void meson_dw_hdmi_unbind(struct device *dev, struct device *master,
void *data)
{
- dw_hdmi_unbind(dev);
+ struct meson_dw_hdmi *meson_dw_hdmi = dev_get_drvdata(dev);
+
+ dw_hdmi_unbind(meson_dw_hdmi->hdmi);
+ meson_dw_hdmi->encoder.funcs->destroy(&meson_dw_hdmi->encoder);
}
static const struct component_ops meson_dw_hdmi_ops = {
diff --git a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
index dc85b53d58ef..76210ae25094 100644
--- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
@@ -68,12 +68,22 @@ static const struct dw_hdmi_plat_data rcar_dw_hdmi_plat_data = {
static int rcar_dw_hdmi_probe(struct platform_device *pdev)
{
- return dw_hdmi_probe(pdev, &rcar_dw_hdmi_plat_data);
+ struct dw_hdmi *hdmi;
+
+ hdmi = dw_hdmi_probe(pdev, &rcar_dw_hdmi_plat_data);
+ if (IS_ERR(hdmi))
+ return PTR_ERR(hdmi);
+
+ platform_set_drvdata(pdev, hdmi);
+
+ return 0;
}
static int rcar_dw_hdmi_remove(struct platform_device *pdev)
{
- dw_hdmi_remove(pdev);
+ struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
+
+ dw_hdmi_remove(hdmi);
return 0;
}
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 1eb02a82fd91..791ab938f998 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -48,6 +48,8 @@ struct rockchip_hdmi {
const struct rockchip_hdmi_chip_data *chip_data;
struct clk *vpll_clk;
struct clk *grf_clk;
+
+ struct dw_hdmi *hdmi;
};
#define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x)
@@ -164,7 +166,6 @@ static const struct dw_hdmi_phy_config rockchip_phy_config[] = {
static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
{
struct device_node *np = hdmi->dev->of_node;
- int ret;
hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
if (IS_ERR(hdmi->regmap)) {
@@ -377,22 +378,24 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
drm_encoder_init(drm, encoder, &dw_hdmi_rockchip_encoder_funcs,
DRM_MODE_ENCODER_TMDS, NULL);
- ret = dw_hdmi_bind(pdev, encoder, plat_data);
+ hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
+ if (IS_ERR(hdmi->hdmi)) {
+ encoder->funcs->destroy(encoder);
+ return PTR_ERR(hdmi->hdmi);
+ }
- /*
- * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
- * which would have called the encoder cleanup. Do it manually.
- */
- if (ret)
- drm_encoder_cleanup(encoder);
+ dev_set_drvdata(dev, hdmi);
- return ret;
+ return 0;
}
static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master,
void *data)
{
- return dw_hdmi_unbind(dev);
+ struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
+
+ dw_hdmi_unbind(hdmi->hdmi);
+ hdmi->encoder.funcs->destroy(&hdmi->encoder);
}
static const struct component_ops dw_hdmi_rockchip_ops = {
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 182f83283e24..29b8900caaf7 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -143,14 +143,15 @@ struct dw_hdmi_plat_data {
unsigned long mpixelclock);
};
-int dw_hdmi_probe(struct platform_device *pdev,
- const struct dw_hdmi_plat_data *plat_data);
-void dw_hdmi_remove(struct platform_device *pdev);
-void dw_hdmi_unbind(struct device *dev);
-int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
- const struct dw_hdmi_plat_data *plat_data);
-
-void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense);
+struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
+ const struct dw_hdmi_plat_data *plat_data);
+void dw_hdmi_remove(struct dw_hdmi *hdmi);
+void dw_hdmi_unbind(struct dw_hdmi *hdmi);
+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_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
--
2.14.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 7/8] drm/bridge/synopsys: dw-hdmi: Do not use device's drvdata
@ 2018-01-09 14:48 ` Thierry Escande
0 siblings, 0 replies; 22+ messages in thread
From: Thierry Escande @ 2018-01-09 14:48 UTC (permalink / raw)
To: Rob Herring, Archit Taneja, Daniel Vetter, Neil Armstrong,
Laurent Pinchart, Sandy Huang
Cc: Jeffy Chen, Sean Paul, dri-devel, linux-rockchip, linux-kernel
From: Jeffy Chen <jeffy.chen@rock-chips.com>
Let plat drivers own the drvdata, so that they could cleanup resources
in their unbind().
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 43 ++++++++++-------------------
drivers/gpu/drm/imx/dw_hdmi-imx.c | 22 +++++++++------
drivers/gpu/drm/meson/meson_dw_hdmi.c | 20 ++++++++++----
drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c | 14 ++++++++--
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 23 ++++++++-------
include/drm/bridge/dw_hdmi.h | 17 ++++++------
6 files changed, 77 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 1cc63a18b7d5..f0380bcae513 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2073,7 +2073,7 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
return ret;
}
-void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
+void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
{
mutex_lock(&hdmi->mutex);
@@ -2099,13 +2099,6 @@ void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
}
mutex_unlock(&hdmi->mutex);
}
-
-void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense)
-{
- struct dw_hdmi *hdmi = dev_get_drvdata(dev);
-
- __dw_hdmi_setup_rx_sense(hdmi, hpd, rx_sense);
-}
EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
@@ -2141,9 +2134,8 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
*/
if (intr_stat &
(HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) {
- __dw_hdmi_setup_rx_sense(hdmi,
- phy_stat & HDMI_PHY_HPD,
- phy_stat & HDMI_PHY_RX_SENSE);
+ dw_hdmi_setup_rx_sense(hdmi, phy_stat & HDMI_PHY_HPD,
+ phy_stat & HDMI_PHY_RX_SENSE);
if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
cec_notifier_set_phys_addr(hdmi->cec_notifier,
@@ -2533,8 +2525,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
if (hdmi->i2c)
dw_hdmi_i2c_init(hdmi);
- platform_set_drvdata(pdev, hdmi);
-
return hdmi;
err_iahb:
@@ -2584,25 +2574,23 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
/* -----------------------------------------------------------------------------
* Probe/remove API, used from platforms based on the DRM bridge API.
*/
-int dw_hdmi_probe(struct platform_device *pdev,
- const struct dw_hdmi_plat_data *plat_data)
+struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
+ const struct dw_hdmi_plat_data *plat_data)
{
struct dw_hdmi *hdmi;
hdmi = __dw_hdmi_probe(pdev, plat_data);
if (IS_ERR(hdmi))
- return PTR_ERR(hdmi);
+ return hdmi;
drm_bridge_add(&hdmi->bridge);
- return 0;
+ return hdmi;
}
EXPORT_SYMBOL_GPL(dw_hdmi_probe);
-void dw_hdmi_remove(struct platform_device *pdev)
+void dw_hdmi_remove(struct dw_hdmi *hdmi)
{
- struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
-
drm_bridge_remove(&hdmi->bridge);
__dw_hdmi_remove(hdmi);
@@ -2612,31 +2600,30 @@ EXPORT_SYMBOL_GPL(dw_hdmi_remove);
/* -----------------------------------------------------------------------------
* Bind/unbind API, used from platforms based on the component framework.
*/
-int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
- const struct dw_hdmi_plat_data *plat_data)
+struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
+ struct drm_encoder *encoder,
+ const struct dw_hdmi_plat_data *plat_data)
{
struct dw_hdmi *hdmi;
int ret;
hdmi = __dw_hdmi_probe(pdev, plat_data);
if (IS_ERR(hdmi))
- return PTR_ERR(hdmi);
+ return hdmi;
ret = drm_bridge_attach(encoder, &hdmi->bridge, NULL);
if (ret) {
__dw_hdmi_remove(hdmi);
DRM_ERROR("Failed to initialize bridge with drm\n");
- return ret;
+ return ERR_PTR(ret);
}
- return 0;
+ return hdmi;
}
EXPORT_SYMBOL_GPL(dw_hdmi_bind);
-void dw_hdmi_unbind(struct device *dev)
+void dw_hdmi_unbind(struct dw_hdmi *hdmi)
{
- struct dw_hdmi *hdmi = dev_get_drvdata(dev);
-
__dw_hdmi_remove(hdmi);
}
EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
index b62763aa8706..b01d03e02ce0 100644
--- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
+++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
@@ -26,6 +26,8 @@ struct imx_hdmi {
struct device *dev;
struct drm_encoder encoder;
struct regmap *regmap;
+
+ struct dw_hdmi *hdmi;
};
static inline struct imx_hdmi *enc_to_imx_hdmi(struct drm_encoder *e)
@@ -239,22 +241,24 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master,
drm_encoder_init(drm, encoder, &dw_hdmi_imx_encoder_funcs,
DRM_MODE_ENCODER_TMDS, NULL);
- ret = dw_hdmi_bind(pdev, encoder, plat_data);
+ hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
+ if (IS_ERR(hdmi->hdmi)) {
+ encoder->funcs->destroy(encoder);
+ return PTR_ERR(hdmi->hdmi);
+ }
- /*
- * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
- * which would have called the encoder cleanup. Do it manually.
- */
- if (ret)
- drm_encoder_cleanup(encoder);
+ dev_set_drvdata(dev, hdmi);
- return ret;
+ return 0;
}
static void dw_hdmi_imx_unbind(struct device *dev, struct device *master,
void *data)
{
- return dw_hdmi_unbind(dev);
+ struct imx_hdmi *hdmi = dev_get_drvdata(dev);
+
+ dw_hdmi_unbind(hdmi->hdmi);
+ hdmi->encoder.funcs->destroy(&hdmi->encoder);
}
static const struct component_ops dw_hdmi_imx_ops = {
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 17de3afd98f6..3f1ec9a5cbfb 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -140,6 +140,8 @@ struct meson_dw_hdmi {
struct clk *venci_clk;
struct regulator *hdmi_supply;
u32 irq_stat;
+
+ struct dw_hdmi *hdmi;
};
#define encoder_to_meson_dw_hdmi(x) \
container_of(x, struct meson_dw_hdmi, encoder)
@@ -528,7 +530,7 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id)
if (stat & HDMITX_TOP_INTR_HPD_RISE)
hpd_connected = true;
- dw_hdmi_setup_rx_sense(dw_hdmi->dev, hpd_connected,
+ dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected,
hpd_connected);
drm_helper_hpd_irq_event(dw_hdmi->encoder.dev);
@@ -878,9 +880,14 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
dw_plat_data->input_bus_format = MEDIA_BUS_FMT_YUV8_1X24;
dw_plat_data->input_bus_encoding = V4L2_YCBCR_ENC_709;
- ret = dw_hdmi_bind(pdev, encoder, &meson_dw_hdmi->dw_plat_data);
- if (ret)
- return ret;
+ meson_dw_hdmi->hdmi = dw_hdmi_bind(pdev, encoder,
+ &meson_dw_hdmi->dw_plat_data);
+ if (IS_ERR(meson_dw_hdmi->hdmi)) {
+ encoder->funcs->destroy(encoder);
+ return PTR_ERR(meson_dw_hdmi->hdmi);
+ }
+
+ dev_set_drvdata(dev, meson_dw_hdmi);
DRM_DEBUG_DRIVER("HDMI controller initialized\n");
@@ -890,7 +897,10 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
static void meson_dw_hdmi_unbind(struct device *dev, struct device *master,
void *data)
{
- dw_hdmi_unbind(dev);
+ struct meson_dw_hdmi *meson_dw_hdmi = dev_get_drvdata(dev);
+
+ dw_hdmi_unbind(meson_dw_hdmi->hdmi);
+ meson_dw_hdmi->encoder.funcs->destroy(&meson_dw_hdmi->encoder);
}
static const struct component_ops meson_dw_hdmi_ops = {
diff --git a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
index dc85b53d58ef..76210ae25094 100644
--- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
+++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
@@ -68,12 +68,22 @@ static const struct dw_hdmi_plat_data rcar_dw_hdmi_plat_data = {
static int rcar_dw_hdmi_probe(struct platform_device *pdev)
{
- return dw_hdmi_probe(pdev, &rcar_dw_hdmi_plat_data);
+ struct dw_hdmi *hdmi;
+
+ hdmi = dw_hdmi_probe(pdev, &rcar_dw_hdmi_plat_data);
+ if (IS_ERR(hdmi))
+ return PTR_ERR(hdmi);
+
+ platform_set_drvdata(pdev, hdmi);
+
+ return 0;
}
static int rcar_dw_hdmi_remove(struct platform_device *pdev)
{
- dw_hdmi_remove(pdev);
+ struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
+
+ dw_hdmi_remove(hdmi);
return 0;
}
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 1eb02a82fd91..791ab938f998 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -48,6 +48,8 @@ struct rockchip_hdmi {
const struct rockchip_hdmi_chip_data *chip_data;
struct clk *vpll_clk;
struct clk *grf_clk;
+
+ struct dw_hdmi *hdmi;
};
#define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x)
@@ -164,7 +166,6 @@ static const struct dw_hdmi_phy_config rockchip_phy_config[] = {
static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
{
struct device_node *np = hdmi->dev->of_node;
- int ret;
hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
if (IS_ERR(hdmi->regmap)) {
@@ -377,22 +378,24 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
drm_encoder_init(drm, encoder, &dw_hdmi_rockchip_encoder_funcs,
DRM_MODE_ENCODER_TMDS, NULL);
- ret = dw_hdmi_bind(pdev, encoder, plat_data);
+ hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
+ if (IS_ERR(hdmi->hdmi)) {
+ encoder->funcs->destroy(encoder);
+ return PTR_ERR(hdmi->hdmi);
+ }
- /*
- * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
- * which would have called the encoder cleanup. Do it manually.
- */
- if (ret)
- drm_encoder_cleanup(encoder);
+ dev_set_drvdata(dev, hdmi);
- return ret;
+ return 0;
}
static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master,
void *data)
{
- return dw_hdmi_unbind(dev);
+ struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
+
+ dw_hdmi_unbind(hdmi->hdmi);
+ hdmi->encoder.funcs->destroy(&hdmi->encoder);
}
static const struct component_ops dw_hdmi_rockchip_ops = {
diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
index 182f83283e24..29b8900caaf7 100644
--- a/include/drm/bridge/dw_hdmi.h
+++ b/include/drm/bridge/dw_hdmi.h
@@ -143,14 +143,15 @@ struct dw_hdmi_plat_data {
unsigned long mpixelclock);
};
-int dw_hdmi_probe(struct platform_device *pdev,
- const struct dw_hdmi_plat_data *plat_data);
-void dw_hdmi_remove(struct platform_device *pdev);
-void dw_hdmi_unbind(struct device *dev);
-int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
- const struct dw_hdmi_plat_data *plat_data);
-
-void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense);
+struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
+ const struct dw_hdmi_plat_data *plat_data);
+void dw_hdmi_remove(struct dw_hdmi *hdmi);
+void dw_hdmi_unbind(struct dw_hdmi *hdmi);
+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_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
--
2.14.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v7 7/8] drm/bridge/synopsys: dw-hdmi: Do not use device's drvdata
2018-01-09 14:48 ` Thierry Escande
@ 2018-01-10 9:33 ` Archit Taneja
-1 siblings, 0 replies; 22+ messages in thread
From: Archit Taneja @ 2018-01-10 9:33 UTC (permalink / raw)
To: Thierry Escande, Rob Herring, Daniel Vetter, Neil Armstrong,
Laurent Pinchart, Sandy Huang
Cc: linux-rockchip, Jeffy Chen, linux-kernel, dri-devel
On 01/09/2018 08:18 PM, Thierry Escande wrote:
> From: Jeffy Chen <jeffy.chen@rock-chips.com>
>
> Let plat drivers own the drvdata, so that they could cleanup resources
> in their unbind().
>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Archit Taneja <architt@codeaurora.org>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 43 ++++++++++-------------------
> drivers/gpu/drm/imx/dw_hdmi-imx.c | 22 +++++++++------
> drivers/gpu/drm/meson/meson_dw_hdmi.c | 20 ++++++++++----
> drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c | 14 ++++++++--
> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 23 ++++++++-------
> include/drm/bridge/dw_hdmi.h | 17 ++++++------
> 6 files changed, 77 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 1cc63a18b7d5..f0380bcae513 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2073,7 +2073,7 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
> return ret;
> }
>
> -void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> +void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> {
> mutex_lock(&hdmi->mutex);
>
> @@ -2099,13 +2099,6 @@ void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> }
> mutex_unlock(&hdmi->mutex);
> }
> -
> -void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense)
> -{
> - struct dw_hdmi *hdmi = dev_get_drvdata(dev);
> -
> - __dw_hdmi_setup_rx_sense(hdmi, hpd, rx_sense);
> -}
> EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
>
> static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
> @@ -2141,9 +2134,8 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
> */
> if (intr_stat &
> (HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) {
> - __dw_hdmi_setup_rx_sense(hdmi,
> - phy_stat & HDMI_PHY_HPD,
> - phy_stat & HDMI_PHY_RX_SENSE);
> + dw_hdmi_setup_rx_sense(hdmi, phy_stat & HDMI_PHY_HPD,
> + phy_stat & HDMI_PHY_RX_SENSE);
>
> if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
> cec_notifier_set_phys_addr(hdmi->cec_notifier,
> @@ -2533,8 +2525,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
> if (hdmi->i2c)
> dw_hdmi_i2c_init(hdmi);
>
> - platform_set_drvdata(pdev, hdmi);
> -
> return hdmi;
>
> err_iahb:
> @@ -2584,25 +2574,23 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
> /* -----------------------------------------------------------------------------
> * Probe/remove API, used from platforms based on the DRM bridge API.
> */
> -int dw_hdmi_probe(struct platform_device *pdev,
> - const struct dw_hdmi_plat_data *plat_data)
> +struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> + const struct dw_hdmi_plat_data *plat_data)
> {
> struct dw_hdmi *hdmi;
>
> hdmi = __dw_hdmi_probe(pdev, plat_data);
> if (IS_ERR(hdmi))
> - return PTR_ERR(hdmi);
> + return hdmi;
>
> drm_bridge_add(&hdmi->bridge);
>
> - return 0;
> + return hdmi;
> }
> EXPORT_SYMBOL_GPL(dw_hdmi_probe);
>
> -void dw_hdmi_remove(struct platform_device *pdev)
> +void dw_hdmi_remove(struct dw_hdmi *hdmi)
> {
> - struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
> -
> drm_bridge_remove(&hdmi->bridge);
>
> __dw_hdmi_remove(hdmi);
> @@ -2612,31 +2600,30 @@ EXPORT_SYMBOL_GPL(dw_hdmi_remove);
> /* -----------------------------------------------------------------------------
> * Bind/unbind API, used from platforms based on the component framework.
> */
> -int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
> - const struct dw_hdmi_plat_data *plat_data)
> +struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
> + struct drm_encoder *encoder,
> + const struct dw_hdmi_plat_data *plat_data)
> {
> struct dw_hdmi *hdmi;
> int ret;
>
> hdmi = __dw_hdmi_probe(pdev, plat_data);
> if (IS_ERR(hdmi))
> - return PTR_ERR(hdmi);
> + return hdmi;
>
> ret = drm_bridge_attach(encoder, &hdmi->bridge, NULL);
> if (ret) {
> __dw_hdmi_remove(hdmi);
> DRM_ERROR("Failed to initialize bridge with drm\n");
> - return ret;
> + return ERR_PTR(ret);
> }
>
> - return 0;
> + return hdmi;
> }
> EXPORT_SYMBOL_GPL(dw_hdmi_bind);
>
> -void dw_hdmi_unbind(struct device *dev)
> +void dw_hdmi_unbind(struct dw_hdmi *hdmi)
> {
> - struct dw_hdmi *hdmi = dev_get_drvdata(dev);
> -
> __dw_hdmi_remove(hdmi);
> }
> EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
> diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> index b62763aa8706..b01d03e02ce0 100644
> --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> @@ -26,6 +26,8 @@ struct imx_hdmi {
> struct device *dev;
> struct drm_encoder encoder;
> struct regmap *regmap;
> +
> + struct dw_hdmi *hdmi;
> };
>
> static inline struct imx_hdmi *enc_to_imx_hdmi(struct drm_encoder *e)
> @@ -239,22 +241,24 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master,
> drm_encoder_init(drm, encoder, &dw_hdmi_imx_encoder_funcs,
> DRM_MODE_ENCODER_TMDS, NULL);
>
> - ret = dw_hdmi_bind(pdev, encoder, plat_data);
> + hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
> + if (IS_ERR(hdmi->hdmi)) {
> + encoder->funcs->destroy(encoder);
> + return PTR_ERR(hdmi->hdmi);
> + }
>
> - /*
> - * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
> - * which would have called the encoder cleanup. Do it manually.
> - */
> - if (ret)
> - drm_encoder_cleanup(encoder);
> + dev_set_drvdata(dev, hdmi);
>
> - return ret;
> + return 0;
> }
>
> static void dw_hdmi_imx_unbind(struct device *dev, struct device *master,
> void *data)
> {
> - return dw_hdmi_unbind(dev);
> + struct imx_hdmi *hdmi = dev_get_drvdata(dev);
> +
> + dw_hdmi_unbind(hdmi->hdmi);
> + hdmi->encoder.funcs->destroy(&hdmi->encoder);
> }
>
> static const struct component_ops dw_hdmi_imx_ops = {
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 17de3afd98f6..3f1ec9a5cbfb 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -140,6 +140,8 @@ struct meson_dw_hdmi {
> struct clk *venci_clk;
> struct regulator *hdmi_supply;
> u32 irq_stat;
> +
> + struct dw_hdmi *hdmi;
> };
> #define encoder_to_meson_dw_hdmi(x) \
> container_of(x, struct meson_dw_hdmi, encoder)
> @@ -528,7 +530,7 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id)
> if (stat & HDMITX_TOP_INTR_HPD_RISE)
> hpd_connected = true;
>
> - dw_hdmi_setup_rx_sense(dw_hdmi->dev, hpd_connected,
> + dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected,
> hpd_connected);
>
> drm_helper_hpd_irq_event(dw_hdmi->encoder.dev);
> @@ -878,9 +880,14 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
> dw_plat_data->input_bus_format = MEDIA_BUS_FMT_YUV8_1X24;
> dw_plat_data->input_bus_encoding = V4L2_YCBCR_ENC_709;
>
> - ret = dw_hdmi_bind(pdev, encoder, &meson_dw_hdmi->dw_plat_data);
> - if (ret)
> - return ret;
> + meson_dw_hdmi->hdmi = dw_hdmi_bind(pdev, encoder,
> + &meson_dw_hdmi->dw_plat_data);
> + if (IS_ERR(meson_dw_hdmi->hdmi)) {
> + encoder->funcs->destroy(encoder);
> + return PTR_ERR(meson_dw_hdmi->hdmi);
> + }
> +
> + dev_set_drvdata(dev, meson_dw_hdmi);
>
> DRM_DEBUG_DRIVER("HDMI controller initialized\n");
>
> @@ -890,7 +897,10 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
> static void meson_dw_hdmi_unbind(struct device *dev, struct device *master,
> void *data)
> {
> - dw_hdmi_unbind(dev);
> + struct meson_dw_hdmi *meson_dw_hdmi = dev_get_drvdata(dev);
> +
> + dw_hdmi_unbind(meson_dw_hdmi->hdmi);
> + meson_dw_hdmi->encoder.funcs->destroy(&meson_dw_hdmi->encoder);
> }
>
> static const struct component_ops meson_dw_hdmi_ops = {
> diff --git a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> index dc85b53d58ef..76210ae25094 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> @@ -68,12 +68,22 @@ static const struct dw_hdmi_plat_data rcar_dw_hdmi_plat_data = {
>
> static int rcar_dw_hdmi_probe(struct platform_device *pdev)
> {
> - return dw_hdmi_probe(pdev, &rcar_dw_hdmi_plat_data);
> + struct dw_hdmi *hdmi;
> +
> + hdmi = dw_hdmi_probe(pdev, &rcar_dw_hdmi_plat_data);
> + if (IS_ERR(hdmi))
> + return PTR_ERR(hdmi);
> +
> + platform_set_drvdata(pdev, hdmi);
> +
> + return 0;
> }
>
> static int rcar_dw_hdmi_remove(struct platform_device *pdev)
> {
> - dw_hdmi_remove(pdev);
> + struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
> +
> + dw_hdmi_remove(hdmi);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index 1eb02a82fd91..791ab938f998 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -48,6 +48,8 @@ struct rockchip_hdmi {
> const struct rockchip_hdmi_chip_data *chip_data;
> struct clk *vpll_clk;
> struct clk *grf_clk;
> +
> + struct dw_hdmi *hdmi;
> };
>
> #define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x)
> @@ -164,7 +166,6 @@ static const struct dw_hdmi_phy_config rockchip_phy_config[] = {
> static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
> {
> struct device_node *np = hdmi->dev->of_node;
> - int ret;
>
> hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> if (IS_ERR(hdmi->regmap)) {
> @@ -377,22 +378,24 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
> drm_encoder_init(drm, encoder, &dw_hdmi_rockchip_encoder_funcs,
> DRM_MODE_ENCODER_TMDS, NULL);
>
> - ret = dw_hdmi_bind(pdev, encoder, plat_data);
> + hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
> + if (IS_ERR(hdmi->hdmi)) {
> + encoder->funcs->destroy(encoder);
> + return PTR_ERR(hdmi->hdmi);
> + }
>
> - /*
> - * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
> - * which would have called the encoder cleanup. Do it manually.
> - */
> - if (ret)
> - drm_encoder_cleanup(encoder);
> + dev_set_drvdata(dev, hdmi);
>
> - return ret;
> + return 0;
> }
>
> static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master,
> void *data)
> {
> - return dw_hdmi_unbind(dev);
> + struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
> +
> + dw_hdmi_unbind(hdmi->hdmi);
> + hdmi->encoder.funcs->destroy(&hdmi->encoder);
> }
>
> static const struct component_ops dw_hdmi_rockchip_ops = {
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 182f83283e24..29b8900caaf7 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -143,14 +143,15 @@ struct dw_hdmi_plat_data {
> unsigned long mpixelclock);
> };
>
> -int dw_hdmi_probe(struct platform_device *pdev,
> - const struct dw_hdmi_plat_data *plat_data);
> -void dw_hdmi_remove(struct platform_device *pdev);
> -void dw_hdmi_unbind(struct device *dev);
> -int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
> - const struct dw_hdmi_plat_data *plat_data);
> -
> -void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense);
> +struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> + const struct dw_hdmi_plat_data *plat_data);
> +void dw_hdmi_remove(struct dw_hdmi *hdmi);
> +void dw_hdmi_unbind(struct dw_hdmi *hdmi);
> +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_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
>
> void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
> void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v7 7/8] drm/bridge/synopsys: dw-hdmi: Do not use device's drvdata
@ 2018-01-10 9:33 ` Archit Taneja
0 siblings, 0 replies; 22+ messages in thread
From: Archit Taneja @ 2018-01-10 9:33 UTC (permalink / raw)
To: Thierry Escande, Rob Herring, Daniel Vetter, Neil Armstrong,
Laurent Pinchart, Sandy Huang
Cc: Jeffy Chen, Sean Paul, dri-devel, linux-rockchip, linux-kernel
On 01/09/2018 08:18 PM, Thierry Escande wrote:
> From: Jeffy Chen <jeffy.chen@rock-chips.com>
>
> Let plat drivers own the drvdata, so that they could cleanup resources
> in their unbind().
>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Archit Taneja <architt@codeaurora.org>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 43 ++++++++++-------------------
> drivers/gpu/drm/imx/dw_hdmi-imx.c | 22 +++++++++------
> drivers/gpu/drm/meson/meson_dw_hdmi.c | 20 ++++++++++----
> drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c | 14 ++++++++--
> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 23 ++++++++-------
> include/drm/bridge/dw_hdmi.h | 17 ++++++------
> 6 files changed, 77 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 1cc63a18b7d5..f0380bcae513 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -2073,7 +2073,7 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void *dev_id)
> return ret;
> }
>
> -void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> +void dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> {
> mutex_lock(&hdmi->mutex);
>
> @@ -2099,13 +2099,6 @@ void __dw_hdmi_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense)
> }
> mutex_unlock(&hdmi->mutex);
> }
> -
> -void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense)
> -{
> - struct dw_hdmi *hdmi = dev_get_drvdata(dev);
> -
> - __dw_hdmi_setup_rx_sense(hdmi, hpd, rx_sense);
> -}
> EXPORT_SYMBOL_GPL(dw_hdmi_setup_rx_sense);
>
> static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
> @@ -2141,9 +2134,8 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
> */
> if (intr_stat &
> (HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_IH_PHY_STAT0_HPD)) {
> - __dw_hdmi_setup_rx_sense(hdmi,
> - phy_stat & HDMI_PHY_HPD,
> - phy_stat & HDMI_PHY_RX_SENSE);
> + dw_hdmi_setup_rx_sense(hdmi, phy_stat & HDMI_PHY_HPD,
> + phy_stat & HDMI_PHY_RX_SENSE);
>
> if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
> cec_notifier_set_phys_addr(hdmi->cec_notifier,
> @@ -2533,8 +2525,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
> if (hdmi->i2c)
> dw_hdmi_i2c_init(hdmi);
>
> - platform_set_drvdata(pdev, hdmi);
> -
> return hdmi;
>
> err_iahb:
> @@ -2584,25 +2574,23 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
> /* -----------------------------------------------------------------------------
> * Probe/remove API, used from platforms based on the DRM bridge API.
> */
> -int dw_hdmi_probe(struct platform_device *pdev,
> - const struct dw_hdmi_plat_data *plat_data)
> +struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> + const struct dw_hdmi_plat_data *plat_data)
> {
> struct dw_hdmi *hdmi;
>
> hdmi = __dw_hdmi_probe(pdev, plat_data);
> if (IS_ERR(hdmi))
> - return PTR_ERR(hdmi);
> + return hdmi;
>
> drm_bridge_add(&hdmi->bridge);
>
> - return 0;
> + return hdmi;
> }
> EXPORT_SYMBOL_GPL(dw_hdmi_probe);
>
> -void dw_hdmi_remove(struct platform_device *pdev)
> +void dw_hdmi_remove(struct dw_hdmi *hdmi)
> {
> - struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
> -
> drm_bridge_remove(&hdmi->bridge);
>
> __dw_hdmi_remove(hdmi);
> @@ -2612,31 +2600,30 @@ EXPORT_SYMBOL_GPL(dw_hdmi_remove);
> /* -----------------------------------------------------------------------------
> * Bind/unbind API, used from platforms based on the component framework.
> */
> -int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
> - const struct dw_hdmi_plat_data *plat_data)
> +struct dw_hdmi *dw_hdmi_bind(struct platform_device *pdev,
> + struct drm_encoder *encoder,
> + const struct dw_hdmi_plat_data *plat_data)
> {
> struct dw_hdmi *hdmi;
> int ret;
>
> hdmi = __dw_hdmi_probe(pdev, plat_data);
> if (IS_ERR(hdmi))
> - return PTR_ERR(hdmi);
> + return hdmi;
>
> ret = drm_bridge_attach(encoder, &hdmi->bridge, NULL);
> if (ret) {
> __dw_hdmi_remove(hdmi);
> DRM_ERROR("Failed to initialize bridge with drm\n");
> - return ret;
> + return ERR_PTR(ret);
> }
>
> - return 0;
> + return hdmi;
> }
> EXPORT_SYMBOL_GPL(dw_hdmi_bind);
>
> -void dw_hdmi_unbind(struct device *dev)
> +void dw_hdmi_unbind(struct dw_hdmi *hdmi)
> {
> - struct dw_hdmi *hdmi = dev_get_drvdata(dev);
> -
> __dw_hdmi_remove(hdmi);
> }
> EXPORT_SYMBOL_GPL(dw_hdmi_unbind);
> diff --git a/drivers/gpu/drm/imx/dw_hdmi-imx.c b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> index b62763aa8706..b01d03e02ce0 100644
> --- a/drivers/gpu/drm/imx/dw_hdmi-imx.c
> +++ b/drivers/gpu/drm/imx/dw_hdmi-imx.c
> @@ -26,6 +26,8 @@ struct imx_hdmi {
> struct device *dev;
> struct drm_encoder encoder;
> struct regmap *regmap;
> +
> + struct dw_hdmi *hdmi;
> };
>
> static inline struct imx_hdmi *enc_to_imx_hdmi(struct drm_encoder *e)
> @@ -239,22 +241,24 @@ static int dw_hdmi_imx_bind(struct device *dev, struct device *master,
> drm_encoder_init(drm, encoder, &dw_hdmi_imx_encoder_funcs,
> DRM_MODE_ENCODER_TMDS, NULL);
>
> - ret = dw_hdmi_bind(pdev, encoder, plat_data);
> + hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
> + if (IS_ERR(hdmi->hdmi)) {
> + encoder->funcs->destroy(encoder);
> + return PTR_ERR(hdmi->hdmi);
> + }
>
> - /*
> - * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
> - * which would have called the encoder cleanup. Do it manually.
> - */
> - if (ret)
> - drm_encoder_cleanup(encoder);
> + dev_set_drvdata(dev, hdmi);
>
> - return ret;
> + return 0;
> }
>
> static void dw_hdmi_imx_unbind(struct device *dev, struct device *master,
> void *data)
> {
> - return dw_hdmi_unbind(dev);
> + struct imx_hdmi *hdmi = dev_get_drvdata(dev);
> +
> + dw_hdmi_unbind(hdmi->hdmi);
> + hdmi->encoder.funcs->destroy(&hdmi->encoder);
> }
>
> static const struct component_ops dw_hdmi_imx_ops = {
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 17de3afd98f6..3f1ec9a5cbfb 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -140,6 +140,8 @@ struct meson_dw_hdmi {
> struct clk *venci_clk;
> struct regulator *hdmi_supply;
> u32 irq_stat;
> +
> + struct dw_hdmi *hdmi;
> };
> #define encoder_to_meson_dw_hdmi(x) \
> container_of(x, struct meson_dw_hdmi, encoder)
> @@ -528,7 +530,7 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id)
> if (stat & HDMITX_TOP_INTR_HPD_RISE)
> hpd_connected = true;
>
> - dw_hdmi_setup_rx_sense(dw_hdmi->dev, hpd_connected,
> + dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected,
> hpd_connected);
>
> drm_helper_hpd_irq_event(dw_hdmi->encoder.dev);
> @@ -878,9 +880,14 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
> dw_plat_data->input_bus_format = MEDIA_BUS_FMT_YUV8_1X24;
> dw_plat_data->input_bus_encoding = V4L2_YCBCR_ENC_709;
>
> - ret = dw_hdmi_bind(pdev, encoder, &meson_dw_hdmi->dw_plat_data);
> - if (ret)
> - return ret;
> + meson_dw_hdmi->hdmi = dw_hdmi_bind(pdev, encoder,
> + &meson_dw_hdmi->dw_plat_data);
> + if (IS_ERR(meson_dw_hdmi->hdmi)) {
> + encoder->funcs->destroy(encoder);
> + return PTR_ERR(meson_dw_hdmi->hdmi);
> + }
> +
> + dev_set_drvdata(dev, meson_dw_hdmi);
>
> DRM_DEBUG_DRIVER("HDMI controller initialized\n");
>
> @@ -890,7 +897,10 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
> static void meson_dw_hdmi_unbind(struct device *dev, struct device *master,
> void *data)
> {
> - dw_hdmi_unbind(dev);
> + struct meson_dw_hdmi *meson_dw_hdmi = dev_get_drvdata(dev);
> +
> + dw_hdmi_unbind(meson_dw_hdmi->hdmi);
> + meson_dw_hdmi->encoder.funcs->destroy(&meson_dw_hdmi->encoder);
> }
>
> static const struct component_ops meson_dw_hdmi_ops = {
> diff --git a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> index dc85b53d58ef..76210ae25094 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> @@ -68,12 +68,22 @@ static const struct dw_hdmi_plat_data rcar_dw_hdmi_plat_data = {
>
> static int rcar_dw_hdmi_probe(struct platform_device *pdev)
> {
> - return dw_hdmi_probe(pdev, &rcar_dw_hdmi_plat_data);
> + struct dw_hdmi *hdmi;
> +
> + hdmi = dw_hdmi_probe(pdev, &rcar_dw_hdmi_plat_data);
> + if (IS_ERR(hdmi))
> + return PTR_ERR(hdmi);
> +
> + platform_set_drvdata(pdev, hdmi);
> +
> + return 0;
> }
>
> static int rcar_dw_hdmi_remove(struct platform_device *pdev)
> {
> - dw_hdmi_remove(pdev);
> + struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
> +
> + dw_hdmi_remove(hdmi);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index 1eb02a82fd91..791ab938f998 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -48,6 +48,8 @@ struct rockchip_hdmi {
> const struct rockchip_hdmi_chip_data *chip_data;
> struct clk *vpll_clk;
> struct clk *grf_clk;
> +
> + struct dw_hdmi *hdmi;
> };
>
> #define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x)
> @@ -164,7 +166,6 @@ static const struct dw_hdmi_phy_config rockchip_phy_config[] = {
> static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
> {
> struct device_node *np = hdmi->dev->of_node;
> - int ret;
>
> hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf");
> if (IS_ERR(hdmi->regmap)) {
> @@ -377,22 +378,24 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
> drm_encoder_init(drm, encoder, &dw_hdmi_rockchip_encoder_funcs,
> DRM_MODE_ENCODER_TMDS, NULL);
>
> - ret = dw_hdmi_bind(pdev, encoder, plat_data);
> + hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
> + if (IS_ERR(hdmi->hdmi)) {
> + encoder->funcs->destroy(encoder);
> + return PTR_ERR(hdmi->hdmi);
> + }
>
> - /*
> - * If dw_hdmi_bind() fails we'll never call dw_hdmi_unbind(),
> - * which would have called the encoder cleanup. Do it manually.
> - */
> - if (ret)
> - drm_encoder_cleanup(encoder);
> + dev_set_drvdata(dev, hdmi);
>
> - return ret;
> + return 0;
> }
>
> static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master,
> void *data)
> {
> - return dw_hdmi_unbind(dev);
> + struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
> +
> + dw_hdmi_unbind(hdmi->hdmi);
> + hdmi->encoder.funcs->destroy(&hdmi->encoder);
> }
>
> static const struct component_ops dw_hdmi_rockchip_ops = {
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index 182f83283e24..29b8900caaf7 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -143,14 +143,15 @@ struct dw_hdmi_plat_data {
> unsigned long mpixelclock);
> };
>
> -int dw_hdmi_probe(struct platform_device *pdev,
> - const struct dw_hdmi_plat_data *plat_data);
> -void dw_hdmi_remove(struct platform_device *pdev);
> -void dw_hdmi_unbind(struct device *dev);
> -int dw_hdmi_bind(struct platform_device *pdev, struct drm_encoder *encoder,
> - const struct dw_hdmi_plat_data *plat_data);
> -
> -void dw_hdmi_setup_rx_sense(struct device *dev, bool hpd, bool rx_sense);
> +struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
> + const struct dw_hdmi_plat_data *plat_data);
> +void dw_hdmi_remove(struct dw_hdmi *hdmi);
> +void dw_hdmi_unbind(struct dw_hdmi *hdmi);
> +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_setup_rx_sense(struct dw_hdmi *hdmi, bool hpd, bool rx_sense);
>
> void dw_hdmi_set_sample_rate(struct dw_hdmi *hdmi, unsigned int rate);
> void dw_hdmi_audio_enable(struct dw_hdmi *hdmi);
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v7 8/8] drm/rockchip: dw_hdmi: Fix error handling path
2018-01-09 14:48 [PATCH v7 00/10] rockchip: kevin: Enable edp display Thierry Escande
@ 2018-01-09 14:48 ` Thierry Escande
2018-01-09 14:48 ` [PATCH v7 3/8] drm/rockchip: analogix_dp: Add a sanity check for rockchip_drm_psr_register() Thierry Escande
` (5 subsequent siblings)
6 siblings, 0 replies; 22+ messages in thread
From: Thierry Escande @ 2018-01-09 14:48 UTC (permalink / raw)
To: Rob Herring, Archit Taneja, Daniel Vetter, Neil Armstrong,
Laurent Pinchart, Sandy Huang
Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jeffy Chen,
Sean Paul, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
From: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Add missing clk_disable_unprepare() in bind()'s error handling path and
unbind().
Also inline clk_prepare_enable() with bind().
Fixes: 12b9f204e804 ("drm: bridge/dw_hdmi: add rockchip rk3288 support")
Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Signed-off-by: Thierry Escande <thierry.escande-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 791ab938f998..0b2c1a5dd1e6 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -193,13 +193,6 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
return PTR_ERR(hdmi->grf_clk);
}
- ret = clk_prepare_enable(hdmi->vpll_clk);
- if (ret) {
- DRM_DEV_ERROR(hdmi->dev,
- "Failed to enable HDMI vpll: %d\n", ret);
- return ret;
- }
-
return 0;
}
@@ -374,6 +367,13 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
return ret;
}
+ ret = clk_prepare_enable(hdmi->vpll_clk);
+ if (ret) {
+ DRM_DEV_ERROR(hdmi->dev, "Failed to enable HDMI vpll: %d\n",
+ ret);
+ return ret;
+ }
+
drm_encoder_helper_add(encoder, &dw_hdmi_rockchip_encoder_helper_funcs);
drm_encoder_init(drm, encoder, &dw_hdmi_rockchip_encoder_funcs,
DRM_MODE_ENCODER_TMDS, NULL);
@@ -381,6 +381,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
if (IS_ERR(hdmi->hdmi)) {
encoder->funcs->destroy(encoder);
+ clk_disable_unprepare(hdmi->vpll_clk);
return PTR_ERR(hdmi->hdmi);
}
@@ -396,6 +397,7 @@ static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master,
dw_hdmi_unbind(hdmi->hdmi);
hdmi->encoder.funcs->destroy(&hdmi->encoder);
+ clk_disable_unprepare(hdmi->vpll_clk);
}
static const struct component_ops dw_hdmi_rockchip_ops = {
--
2.14.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 8/8] drm/rockchip: dw_hdmi: Fix error handling path
@ 2018-01-09 14:48 ` Thierry Escande
0 siblings, 0 replies; 22+ messages in thread
From: Thierry Escande @ 2018-01-09 14:48 UTC (permalink / raw)
To: Rob Herring, Archit Taneja, Daniel Vetter, Neil Armstrong,
Laurent Pinchart, Sandy Huang
Cc: Jeffy Chen, Sean Paul, dri-devel, linux-rockchip, linux-kernel
From: Jeffy Chen <jeffy.chen@rock-chips.com>
Add missing clk_disable_unprepare() in bind()'s error handling path and
unbind().
Also inline clk_prepare_enable() with bind().
Fixes: 12b9f204e804 ("drm: bridge/dw_hdmi: add rockchip rk3288 support")
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index 791ab938f998..0b2c1a5dd1e6 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -193,13 +193,6 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi *hdmi)
return PTR_ERR(hdmi->grf_clk);
}
- ret = clk_prepare_enable(hdmi->vpll_clk);
- if (ret) {
- DRM_DEV_ERROR(hdmi->dev,
- "Failed to enable HDMI vpll: %d\n", ret);
- return ret;
- }
-
return 0;
}
@@ -374,6 +367,13 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
return ret;
}
+ ret = clk_prepare_enable(hdmi->vpll_clk);
+ if (ret) {
+ DRM_DEV_ERROR(hdmi->dev, "Failed to enable HDMI vpll: %d\n",
+ ret);
+ return ret;
+ }
+
drm_encoder_helper_add(encoder, &dw_hdmi_rockchip_encoder_helper_funcs);
drm_encoder_init(drm, encoder, &dw_hdmi_rockchip_encoder_funcs,
DRM_MODE_ENCODER_TMDS, NULL);
@@ -381,6 +381,7 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
hdmi->hdmi = dw_hdmi_bind(pdev, encoder, plat_data);
if (IS_ERR(hdmi->hdmi)) {
encoder->funcs->destroy(encoder);
+ clk_disable_unprepare(hdmi->vpll_clk);
return PTR_ERR(hdmi->hdmi);
}
@@ -396,6 +397,7 @@ static void dw_hdmi_rockchip_unbind(struct device *dev, struct device *master,
dw_hdmi_unbind(hdmi->hdmi);
hdmi->encoder.funcs->destroy(&hdmi->encoder);
+ clk_disable_unprepare(hdmi->vpll_clk);
}
static const struct component_ops dw_hdmi_rockchip_ops = {
--
2.14.1
^ permalink raw reply related [flat|nested] 22+ messages in thread