* [PATCH 0/4] anx7625: Cleanup, fixes, and implement wait_hpd_asserted
@ 2022-06-29 16:05 ` Hsin-Yi Wang
0 siblings, 0 replies; 22+ messages in thread
From: Hsin-Yi Wang @ 2022-06-29 16:05 UTC (permalink / raw)
To: Robert Foss, Xin Ji
Cc: Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, David Airlie, Daniel Vetter, Sam Ravnborg,
Thomas Zimmermann, Maxime Ripard, dri-devel, linux-kernel
This series contains:
Cleanup:
- Convert to use devm_i2c_new_dummy_device()
- Use pm_runtime_force_suspend(resume)
Fixes:
- Fix NULL pointer crash when using edp-panel
and Impelment wait_hpd_asserted() callback.
The patches are not related to each other, but they are all
anx7625 patches so they are all stacked in this series.
Hsin-Yi Wang (4):
drm/bridge: anx7625: Convert to devm_i2c_new_dummy_device()
drm/bridge: anx7625: Use pm_runtime_force_suspend(resume)
drm/bridge: anx7625: Fix NULL pointer crash when using edp-panel
drm/bridge: anx7625: Add wait_hpd_asserted() callback
drivers/gpu/drm/bridge/analogix/anx7625.c | 179 ++++++++--------------
1 file changed, 65 insertions(+), 114 deletions(-)
--
2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 0/4] anx7625: Cleanup, fixes, and implement wait_hpd_asserted
@ 2022-06-29 16:05 ` Hsin-Yi Wang
0 siblings, 0 replies; 22+ messages in thread
From: Hsin-Yi Wang @ 2022-06-29 16:05 UTC (permalink / raw)
To: Robert Foss, Xin Ji
Cc: Jonas Karlman, David Airlie, Thomas Zimmermann, dri-devel,
Neil Armstrong, linux-kernel, Jernej Skrabec, Laurent Pinchart,
Andrzej Hajda, Sam Ravnborg, Maxime Ripard
This series contains:
Cleanup:
- Convert to use devm_i2c_new_dummy_device()
- Use pm_runtime_force_suspend(resume)
Fixes:
- Fix NULL pointer crash when using edp-panel
and Impelment wait_hpd_asserted() callback.
The patches are not related to each other, but they are all
anx7625 patches so they are all stacked in this series.
Hsin-Yi Wang (4):
drm/bridge: anx7625: Convert to devm_i2c_new_dummy_device()
drm/bridge: anx7625: Use pm_runtime_force_suspend(resume)
drm/bridge: anx7625: Fix NULL pointer crash when using edp-panel
drm/bridge: anx7625: Add wait_hpd_asserted() callback
drivers/gpu/drm/bridge/analogix/anx7625.c | 179 ++++++++--------------
1 file changed, 65 insertions(+), 114 deletions(-)
--
2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/4] drm/bridge: anx7625: Convert to devm_i2c_new_dummy_device()
2022-06-29 16:05 ` Hsin-Yi Wang
@ 2022-06-29 16:05 ` Hsin-Yi Wang
-1 siblings, 0 replies; 22+ messages in thread
From: Hsin-Yi Wang @ 2022-06-29 16:05 UTC (permalink / raw)
To: Robert Foss, Xin Ji
Cc: Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, David Airlie, Daniel Vetter, Sam Ravnborg,
Thomas Zimmermann, Maxime Ripard, dri-devel, linux-kernel
Simplify the resource management.
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 96 +++++++----------------
1 file changed, 27 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 3710fa9ee0acd..f89e8151475f7 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2436,82 +2436,44 @@ static const struct drm_bridge_funcs anx7625_bridge_funcs = {
static int anx7625_register_i2c_dummy_clients(struct anx7625_data *ctx,
struct i2c_client *client)
{
- int err = 0;
+ struct device *dev = &ctx->client->dev;
- ctx->i2c.tx_p0_client = i2c_new_dummy_device(client->adapter,
- TX_P0_ADDR >> 1);
+ ctx->i2c.tx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ TX_P0_ADDR >> 1);
if (IS_ERR(ctx->i2c.tx_p0_client))
return PTR_ERR(ctx->i2c.tx_p0_client);
- ctx->i2c.tx_p1_client = i2c_new_dummy_device(client->adapter,
- TX_P1_ADDR >> 1);
- if (IS_ERR(ctx->i2c.tx_p1_client)) {
- err = PTR_ERR(ctx->i2c.tx_p1_client);
- goto free_tx_p0;
- }
+ ctx->i2c.tx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ TX_P1_ADDR >> 1);
+ if (IS_ERR(ctx->i2c.tx_p1_client))
+ return PTR_ERR(ctx->i2c.tx_p1_client);
- ctx->i2c.tx_p2_client = i2c_new_dummy_device(client->adapter,
- TX_P2_ADDR >> 1);
- if (IS_ERR(ctx->i2c.tx_p2_client)) {
- err = PTR_ERR(ctx->i2c.tx_p2_client);
- goto free_tx_p1;
- }
+ ctx->i2c.tx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ TX_P2_ADDR >> 1);
+ if (IS_ERR(ctx->i2c.tx_p2_client))
+ return PTR_ERR(ctx->i2c.tx_p2_client);
- ctx->i2c.rx_p0_client = i2c_new_dummy_device(client->adapter,
- RX_P0_ADDR >> 1);
- if (IS_ERR(ctx->i2c.rx_p0_client)) {
- err = PTR_ERR(ctx->i2c.rx_p0_client);
- goto free_tx_p2;
- }
+ ctx->i2c.rx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ RX_P0_ADDR >> 1);
+ if (IS_ERR(ctx->i2c.rx_p0_client))
+ return PTR_ERR(ctx->i2c.rx_p0_client);
- ctx->i2c.rx_p1_client = i2c_new_dummy_device(client->adapter,
- RX_P1_ADDR >> 1);
- if (IS_ERR(ctx->i2c.rx_p1_client)) {
- err = PTR_ERR(ctx->i2c.rx_p1_client);
- goto free_rx_p0;
- }
+ ctx->i2c.rx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ RX_P1_ADDR >> 1);
+ if (IS_ERR(ctx->i2c.rx_p1_client))
+ return PTR_ERR(ctx->i2c.rx_p1_client);
- ctx->i2c.rx_p2_client = i2c_new_dummy_device(client->adapter,
- RX_P2_ADDR >> 1);
- if (IS_ERR(ctx->i2c.rx_p2_client)) {
- err = PTR_ERR(ctx->i2c.rx_p2_client);
- goto free_rx_p1;
- }
+ ctx->i2c.rx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ RX_P2_ADDR >> 1);
+ if (IS_ERR(ctx->i2c.rx_p2_client))
+ return PTR_ERR(ctx->i2c.rx_p2_client);
- ctx->i2c.tcpc_client = i2c_new_dummy_device(client->adapter,
- TCPC_INTERFACE_ADDR >> 1);
- if (IS_ERR(ctx->i2c.tcpc_client)) {
- err = PTR_ERR(ctx->i2c.tcpc_client);
- goto free_rx_p2;
- }
+ ctx->i2c.tcpc_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ TCPC_INTERFACE_ADDR >> 1);
+ if (IS_ERR(ctx->i2c.tcpc_client))
+ return PTR_ERR(ctx->i2c.tcpc_client);
return 0;
-
-free_rx_p2:
- i2c_unregister_device(ctx->i2c.rx_p2_client);
-free_rx_p1:
- i2c_unregister_device(ctx->i2c.rx_p1_client);
-free_rx_p0:
- i2c_unregister_device(ctx->i2c.rx_p0_client);
-free_tx_p2:
- i2c_unregister_device(ctx->i2c.tx_p2_client);
-free_tx_p1:
- i2c_unregister_device(ctx->i2c.tx_p1_client);
-free_tx_p0:
- i2c_unregister_device(ctx->i2c.tx_p0_client);
-
- return err;
-}
-
-static void anx7625_unregister_i2c_dummy_clients(struct anx7625_data *ctx)
-{
- i2c_unregister_device(ctx->i2c.tx_p0_client);
- i2c_unregister_device(ctx->i2c.tx_p1_client);
- i2c_unregister_device(ctx->i2c.tx_p2_client);
- i2c_unregister_device(ctx->i2c.rx_p0_client);
- i2c_unregister_device(ctx->i2c.rx_p1_client);
- i2c_unregister_device(ctx->i2c.rx_p2_client);
- i2c_unregister_device(ctx->i2c.tcpc_client);
}
static int __maybe_unused anx7625_runtime_pm_suspend(struct device *dev)
@@ -2723,8 +2685,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (!platform->pdata.low_power_mode)
pm_runtime_put_sync_suspend(&client->dev);
- anx7625_unregister_i2c_dummy_clients(platform);
-
free_wq:
if (platform->workqueue)
destroy_workqueue(platform->workqueue);
@@ -2754,8 +2714,6 @@ static int anx7625_i2c_remove(struct i2c_client *client)
if (!platform->pdata.low_power_mode)
pm_runtime_put_sync_suspend(&client->dev);
- anx7625_unregister_i2c_dummy_clients(platform);
-
if (platform->pdata.audio_en)
anx7625_unregister_audio(platform);
--
2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 1/4] drm/bridge: anx7625: Convert to devm_i2c_new_dummy_device()
@ 2022-06-29 16:05 ` Hsin-Yi Wang
0 siblings, 0 replies; 22+ messages in thread
From: Hsin-Yi Wang @ 2022-06-29 16:05 UTC (permalink / raw)
To: Robert Foss, Xin Ji
Cc: Jonas Karlman, David Airlie, Thomas Zimmermann, dri-devel,
Neil Armstrong, linux-kernel, Jernej Skrabec, Laurent Pinchart,
Andrzej Hajda, Sam Ravnborg, Maxime Ripard
Simplify the resource management.
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 96 +++++++----------------
1 file changed, 27 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 3710fa9ee0acd..f89e8151475f7 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2436,82 +2436,44 @@ static const struct drm_bridge_funcs anx7625_bridge_funcs = {
static int anx7625_register_i2c_dummy_clients(struct anx7625_data *ctx,
struct i2c_client *client)
{
- int err = 0;
+ struct device *dev = &ctx->client->dev;
- ctx->i2c.tx_p0_client = i2c_new_dummy_device(client->adapter,
- TX_P0_ADDR >> 1);
+ ctx->i2c.tx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ TX_P0_ADDR >> 1);
if (IS_ERR(ctx->i2c.tx_p0_client))
return PTR_ERR(ctx->i2c.tx_p0_client);
- ctx->i2c.tx_p1_client = i2c_new_dummy_device(client->adapter,
- TX_P1_ADDR >> 1);
- if (IS_ERR(ctx->i2c.tx_p1_client)) {
- err = PTR_ERR(ctx->i2c.tx_p1_client);
- goto free_tx_p0;
- }
+ ctx->i2c.tx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ TX_P1_ADDR >> 1);
+ if (IS_ERR(ctx->i2c.tx_p1_client))
+ return PTR_ERR(ctx->i2c.tx_p1_client);
- ctx->i2c.tx_p2_client = i2c_new_dummy_device(client->adapter,
- TX_P2_ADDR >> 1);
- if (IS_ERR(ctx->i2c.tx_p2_client)) {
- err = PTR_ERR(ctx->i2c.tx_p2_client);
- goto free_tx_p1;
- }
+ ctx->i2c.tx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ TX_P2_ADDR >> 1);
+ if (IS_ERR(ctx->i2c.tx_p2_client))
+ return PTR_ERR(ctx->i2c.tx_p2_client);
- ctx->i2c.rx_p0_client = i2c_new_dummy_device(client->adapter,
- RX_P0_ADDR >> 1);
- if (IS_ERR(ctx->i2c.rx_p0_client)) {
- err = PTR_ERR(ctx->i2c.rx_p0_client);
- goto free_tx_p2;
- }
+ ctx->i2c.rx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ RX_P0_ADDR >> 1);
+ if (IS_ERR(ctx->i2c.rx_p0_client))
+ return PTR_ERR(ctx->i2c.rx_p0_client);
- ctx->i2c.rx_p1_client = i2c_new_dummy_device(client->adapter,
- RX_P1_ADDR >> 1);
- if (IS_ERR(ctx->i2c.rx_p1_client)) {
- err = PTR_ERR(ctx->i2c.rx_p1_client);
- goto free_rx_p0;
- }
+ ctx->i2c.rx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ RX_P1_ADDR >> 1);
+ if (IS_ERR(ctx->i2c.rx_p1_client))
+ return PTR_ERR(ctx->i2c.rx_p1_client);
- ctx->i2c.rx_p2_client = i2c_new_dummy_device(client->adapter,
- RX_P2_ADDR >> 1);
- if (IS_ERR(ctx->i2c.rx_p2_client)) {
- err = PTR_ERR(ctx->i2c.rx_p2_client);
- goto free_rx_p1;
- }
+ ctx->i2c.rx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ RX_P2_ADDR >> 1);
+ if (IS_ERR(ctx->i2c.rx_p2_client))
+ return PTR_ERR(ctx->i2c.rx_p2_client);
- ctx->i2c.tcpc_client = i2c_new_dummy_device(client->adapter,
- TCPC_INTERFACE_ADDR >> 1);
- if (IS_ERR(ctx->i2c.tcpc_client)) {
- err = PTR_ERR(ctx->i2c.tcpc_client);
- goto free_rx_p2;
- }
+ ctx->i2c.tcpc_client = devm_i2c_new_dummy_device(dev, client->adapter,
+ TCPC_INTERFACE_ADDR >> 1);
+ if (IS_ERR(ctx->i2c.tcpc_client))
+ return PTR_ERR(ctx->i2c.tcpc_client);
return 0;
-
-free_rx_p2:
- i2c_unregister_device(ctx->i2c.rx_p2_client);
-free_rx_p1:
- i2c_unregister_device(ctx->i2c.rx_p1_client);
-free_rx_p0:
- i2c_unregister_device(ctx->i2c.rx_p0_client);
-free_tx_p2:
- i2c_unregister_device(ctx->i2c.tx_p2_client);
-free_tx_p1:
- i2c_unregister_device(ctx->i2c.tx_p1_client);
-free_tx_p0:
- i2c_unregister_device(ctx->i2c.tx_p0_client);
-
- return err;
-}
-
-static void anx7625_unregister_i2c_dummy_clients(struct anx7625_data *ctx)
-{
- i2c_unregister_device(ctx->i2c.tx_p0_client);
- i2c_unregister_device(ctx->i2c.tx_p1_client);
- i2c_unregister_device(ctx->i2c.tx_p2_client);
- i2c_unregister_device(ctx->i2c.rx_p0_client);
- i2c_unregister_device(ctx->i2c.rx_p1_client);
- i2c_unregister_device(ctx->i2c.rx_p2_client);
- i2c_unregister_device(ctx->i2c.tcpc_client);
}
static int __maybe_unused anx7625_runtime_pm_suspend(struct device *dev)
@@ -2723,8 +2685,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (!platform->pdata.low_power_mode)
pm_runtime_put_sync_suspend(&client->dev);
- anx7625_unregister_i2c_dummy_clients(platform);
-
free_wq:
if (platform->workqueue)
destroy_workqueue(platform->workqueue);
@@ -2754,8 +2714,6 @@ static int anx7625_i2c_remove(struct i2c_client *client)
if (!platform->pdata.low_power_mode)
pm_runtime_put_sync_suspend(&client->dev);
- anx7625_unregister_i2c_dummy_clients(platform);
-
if (platform->pdata.audio_en)
anx7625_unregister_audio(platform);
--
2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/4] drm/bridge: anx7625: Use pm_runtime_force_suspend(resume)
2022-06-29 16:05 ` Hsin-Yi Wang
@ 2022-06-29 16:05 ` Hsin-Yi Wang
-1 siblings, 0 replies; 22+ messages in thread
From: Hsin-Yi Wang @ 2022-06-29 16:05 UTC (permalink / raw)
To: Robert Foss, Xin Ji
Cc: Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, David Airlie, Daniel Vetter, Sam Ravnborg,
Thomas Zimmermann, Maxime Ripard, dri-devel, linux-kernel
There's no need to check for IRQ or disable it in suspend.
Use pm_runtime_force_suspend(resume) to make sure anx7625 is powered off
correctly. Make the system suspend/resume and pm runtime suspend/resume
more consistant.
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 33 ++---------------------
1 file changed, 2 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index f89e8151475f7..478f5af381c7d 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2504,38 +2504,9 @@ static int __maybe_unused anx7625_runtime_pm_resume(struct device *dev)
return 0;
}
-static int __maybe_unused anx7625_resume(struct device *dev)
-{
- struct anx7625_data *ctx = dev_get_drvdata(dev);
-
- if (!ctx->pdata.intp_irq)
- return 0;
-
- if (!pm_runtime_enabled(dev) || !pm_runtime_suspended(dev)) {
- enable_irq(ctx->pdata.intp_irq);
- anx7625_runtime_pm_resume(dev);
- }
-
- return 0;
-}
-
-static int __maybe_unused anx7625_suspend(struct device *dev)
-{
- struct anx7625_data *ctx = dev_get_drvdata(dev);
-
- if (!ctx->pdata.intp_irq)
- return 0;
-
- if (!pm_runtime_enabled(dev) || !pm_runtime_suspended(dev)) {
- anx7625_runtime_pm_suspend(dev);
- disable_irq(ctx->pdata.intp_irq);
- }
-
- return 0;
-}
-
static const struct dev_pm_ops anx7625_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(anx7625_suspend, anx7625_resume)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
SET_RUNTIME_PM_OPS(anx7625_runtime_pm_suspend,
anx7625_runtime_pm_resume, NULL)
};
--
2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/4] drm/bridge: anx7625: Use pm_runtime_force_suspend(resume)
@ 2022-06-29 16:05 ` Hsin-Yi Wang
0 siblings, 0 replies; 22+ messages in thread
From: Hsin-Yi Wang @ 2022-06-29 16:05 UTC (permalink / raw)
To: Robert Foss, Xin Ji
Cc: Jonas Karlman, David Airlie, Thomas Zimmermann, dri-devel,
Neil Armstrong, linux-kernel, Jernej Skrabec, Laurent Pinchart,
Andrzej Hajda, Sam Ravnborg, Maxime Ripard
There's no need to check for IRQ or disable it in suspend.
Use pm_runtime_force_suspend(resume) to make sure anx7625 is powered off
correctly. Make the system suspend/resume and pm runtime suspend/resume
more consistant.
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 33 ++---------------------
1 file changed, 2 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index f89e8151475f7..478f5af381c7d 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2504,38 +2504,9 @@ static int __maybe_unused anx7625_runtime_pm_resume(struct device *dev)
return 0;
}
-static int __maybe_unused anx7625_resume(struct device *dev)
-{
- struct anx7625_data *ctx = dev_get_drvdata(dev);
-
- if (!ctx->pdata.intp_irq)
- return 0;
-
- if (!pm_runtime_enabled(dev) || !pm_runtime_suspended(dev)) {
- enable_irq(ctx->pdata.intp_irq);
- anx7625_runtime_pm_resume(dev);
- }
-
- return 0;
-}
-
-static int __maybe_unused anx7625_suspend(struct device *dev)
-{
- struct anx7625_data *ctx = dev_get_drvdata(dev);
-
- if (!ctx->pdata.intp_irq)
- return 0;
-
- if (!pm_runtime_enabled(dev) || !pm_runtime_suspended(dev)) {
- anx7625_runtime_pm_suspend(dev);
- disable_irq(ctx->pdata.intp_irq);
- }
-
- return 0;
-}
-
static const struct dev_pm_ops anx7625_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(anx7625_suspend, anx7625_resume)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
SET_RUNTIME_PM_OPS(anx7625_runtime_pm_suspend,
anx7625_runtime_pm_resume, NULL)
};
--
2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/4] drm/bridge: anx7625: Fix NULL pointer crash when using edp-panel
2022-06-29 16:05 ` Hsin-Yi Wang
@ 2022-06-29 16:05 ` Hsin-Yi Wang
-1 siblings, 0 replies; 22+ messages in thread
From: Hsin-Yi Wang @ 2022-06-29 16:05 UTC (permalink / raw)
To: Robert Foss, Xin Ji
Cc: Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, David Airlie, Daniel Vetter, Sam Ravnborg,
Thomas Zimmermann, Maxime Ripard, dri-devel, linux-kernel
Move devm_of_dp_aux_populate_ep_devices() after pm runtime and i2c setup
to avoid NULL pointer crash.
edp-panel probe (generic_edp_panel_probe) calls pm_runtime_get_sync() to
read EDID. At this time, bridge should have pm runtime enabled and i2c
clients ready.
Fixes: adca62ec370c ("drm/bridge: anx7625: Support reading edid through aux channel")
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 478f5af381c7d..59ddeba33652b 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2590,14 +2590,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
platform->aux.dev = dev;
platform->aux.transfer = anx7625_aux_transfer;
drm_dp_aux_init(&platform->aux);
- devm_of_dp_aux_populate_ep_devices(&platform->aux);
-
- ret = anx7625_parse_dt(dev, pdata);
- if (ret) {
- if (ret != -EPROBE_DEFER)
- DRM_DEV_ERROR(dev, "fail to parse DT : %d\n", ret);
- goto free_wq;
- }
if (anx7625_register_i2c_dummy_clients(platform, client) != 0) {
ret = -ENOMEM;
@@ -2613,6 +2605,15 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (ret)
goto free_wq;
+ devm_of_dp_aux_populate_ep_devices(&platform->aux);
+
+ ret = anx7625_parse_dt(dev, pdata);
+ if (ret) {
+ if (ret != -EPROBE_DEFER)
+ DRM_DEV_ERROR(dev, "fail to parse DT : %d\n", ret);
+ goto free_wq;
+ }
+
if (!platform->pdata.low_power_mode) {
anx7625_disable_pd_protocol(platform);
pm_runtime_get_sync(dev);
--
2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/4] drm/bridge: anx7625: Fix NULL pointer crash when using edp-panel
@ 2022-06-29 16:05 ` Hsin-Yi Wang
0 siblings, 0 replies; 22+ messages in thread
From: Hsin-Yi Wang @ 2022-06-29 16:05 UTC (permalink / raw)
To: Robert Foss, Xin Ji
Cc: Jonas Karlman, David Airlie, Thomas Zimmermann, dri-devel,
Neil Armstrong, linux-kernel, Jernej Skrabec, Laurent Pinchart,
Andrzej Hajda, Sam Ravnborg, Maxime Ripard
Move devm_of_dp_aux_populate_ep_devices() after pm runtime and i2c setup
to avoid NULL pointer crash.
edp-panel probe (generic_edp_panel_probe) calls pm_runtime_get_sync() to
read EDID. At this time, bridge should have pm runtime enabled and i2c
clients ready.
Fixes: adca62ec370c ("drm/bridge: anx7625: Support reading edid through aux channel")
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 478f5af381c7d..59ddeba33652b 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2590,14 +2590,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
platform->aux.dev = dev;
platform->aux.transfer = anx7625_aux_transfer;
drm_dp_aux_init(&platform->aux);
- devm_of_dp_aux_populate_ep_devices(&platform->aux);
-
- ret = anx7625_parse_dt(dev, pdata);
- if (ret) {
- if (ret != -EPROBE_DEFER)
- DRM_DEV_ERROR(dev, "fail to parse DT : %d\n", ret);
- goto free_wq;
- }
if (anx7625_register_i2c_dummy_clients(platform, client) != 0) {
ret = -ENOMEM;
@@ -2613,6 +2605,15 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (ret)
goto free_wq;
+ devm_of_dp_aux_populate_ep_devices(&platform->aux);
+
+ ret = anx7625_parse_dt(dev, pdata);
+ if (ret) {
+ if (ret != -EPROBE_DEFER)
+ DRM_DEV_ERROR(dev, "fail to parse DT : %d\n", ret);
+ goto free_wq;
+ }
+
if (!platform->pdata.low_power_mode) {
anx7625_disable_pd_protocol(platform);
pm_runtime_get_sync(dev);
--
2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/4] drm/bridge: anx7625: Add wait_hpd_asserted() callback
2022-06-29 16:05 ` Hsin-Yi Wang
@ 2022-06-29 16:05 ` Hsin-Yi Wang
-1 siblings, 0 replies; 22+ messages in thread
From: Hsin-Yi Wang @ 2022-06-29 16:05 UTC (permalink / raw)
To: Robert Foss, Xin Ji
Cc: Andrzej Hajda, Neil Armstrong, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, David Airlie, Daniel Vetter, Sam Ravnborg,
Thomas Zimmermann, Maxime Ripard, dri-devel, linux-kernel
Move hpd polling check into wait_hpd_asserted() callback. For the cases
that aux transfer function wasn't used, do hpd polling check after pm
runtime resume, which will power on the bridge.
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 33 ++++++++++++++++++-----
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 59ddeba33652b..ea5a0b86fe52a 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1443,23 +1443,24 @@ static int anx7625_read_hpd_status_p0(struct anx7625_data *ctx)
return anx7625_reg_read(ctx, ctx->i2c.rx_p0_client, SYSTEM_STSTUS);
}
-static void anx7625_hpd_polling(struct anx7625_data *ctx)
+static int _anx7625_hpd_polling(struct anx7625_data *ctx,
+ unsigned long wait_us)
{
int ret, val;
struct device *dev = &ctx->client->dev;
/* Interrupt mode, no need poll HPD status, just return */
if (ctx->pdata.intp_irq)
- return;
+ return 0;
ret = readx_poll_timeout(anx7625_read_hpd_status_p0,
ctx, val,
((val & HPD_STATUS) || (val < 0)),
- 5000,
- 5000 * 100);
+ wait_us / 100,
+ wait_us);
if (ret) {
DRM_DEV_ERROR(dev, "no hpd.\n");
- return;
+ return ret;
}
DRM_DEV_DEBUG_DRIVER(dev, "system status: 0x%x. HPD raise up.\n", val);
@@ -1472,6 +1473,23 @@ static void anx7625_hpd_polling(struct anx7625_data *ctx)
if (!ctx->pdata.panel_bridge && ctx->bridge_attached)
drm_helper_hpd_irq_event(ctx->bridge.dev);
+
+ return 0;
+}
+
+static int anx7625_wait_hpd_asserted(struct drm_dp_aux *aux,
+ unsigned long wait_us)
+{
+ struct anx7625_data *ctx = container_of(aux, struct anx7625_data, aux);
+ struct device *dev = &ctx->client->dev;
+ int ret;
+
+ pm_runtime_get_sync(dev);
+ ret = _anx7625_hpd_polling(ctx, wait_us);
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
+ return ret;
}
static void anx7625_remove_edid(struct anx7625_data *ctx)
@@ -1741,6 +1759,7 @@ static struct edid *anx7625_get_edid(struct anx7625_data *ctx)
}
pm_runtime_get_sync(dev);
+ _anx7625_hpd_polling(ctx, 5000 * 100);
edid_num = sp_tx_edid_read(ctx, p_edid->edid_raw_data);
pm_runtime_put_sync(dev);
@@ -2378,6 +2397,7 @@ static void anx7625_bridge_atomic_enable(struct drm_bridge *bridge,
ctx->connector = connector;
pm_runtime_get_sync(dev);
+ _anx7625_hpd_polling(ctx, 5000 * 100);
anx7625_dp_start(ctx);
}
@@ -2497,7 +2517,6 @@ static int __maybe_unused anx7625_runtime_pm_resume(struct device *dev)
mutex_lock(&ctx->lock);
anx7625_power_on_init(ctx);
- anx7625_hpd_polling(ctx);
mutex_unlock(&ctx->lock);
@@ -2589,6 +2608,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
platform->aux.name = "anx7625-aux";
platform->aux.dev = dev;
platform->aux.transfer = anx7625_aux_transfer;
+ platform->aux.wait_hpd_asserted = anx7625_wait_hpd_asserted;
drm_dp_aux_init(&platform->aux);
if (anx7625_register_i2c_dummy_clients(platform, client) != 0) {
@@ -2617,6 +2637,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (!platform->pdata.low_power_mode) {
anx7625_disable_pd_protocol(platform);
pm_runtime_get_sync(dev);
+ _anx7625_hpd_polling(platform, 5000 * 100);
}
/* Add work function */
--
2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/4] drm/bridge: anx7625: Add wait_hpd_asserted() callback
@ 2022-06-29 16:05 ` Hsin-Yi Wang
0 siblings, 0 replies; 22+ messages in thread
From: Hsin-Yi Wang @ 2022-06-29 16:05 UTC (permalink / raw)
To: Robert Foss, Xin Ji
Cc: Jonas Karlman, David Airlie, Thomas Zimmermann, dri-devel,
Neil Armstrong, linux-kernel, Jernej Skrabec, Laurent Pinchart,
Andrzej Hajda, Sam Ravnborg, Maxime Ripard
Move hpd polling check into wait_hpd_asserted() callback. For the cases
that aux transfer function wasn't used, do hpd polling check after pm
runtime resume, which will power on the bridge.
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 33 ++++++++++++++++++-----
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 59ddeba33652b..ea5a0b86fe52a 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1443,23 +1443,24 @@ static int anx7625_read_hpd_status_p0(struct anx7625_data *ctx)
return anx7625_reg_read(ctx, ctx->i2c.rx_p0_client, SYSTEM_STSTUS);
}
-static void anx7625_hpd_polling(struct anx7625_data *ctx)
+static int _anx7625_hpd_polling(struct anx7625_data *ctx,
+ unsigned long wait_us)
{
int ret, val;
struct device *dev = &ctx->client->dev;
/* Interrupt mode, no need poll HPD status, just return */
if (ctx->pdata.intp_irq)
- return;
+ return 0;
ret = readx_poll_timeout(anx7625_read_hpd_status_p0,
ctx, val,
((val & HPD_STATUS) || (val < 0)),
- 5000,
- 5000 * 100);
+ wait_us / 100,
+ wait_us);
if (ret) {
DRM_DEV_ERROR(dev, "no hpd.\n");
- return;
+ return ret;
}
DRM_DEV_DEBUG_DRIVER(dev, "system status: 0x%x. HPD raise up.\n", val);
@@ -1472,6 +1473,23 @@ static void anx7625_hpd_polling(struct anx7625_data *ctx)
if (!ctx->pdata.panel_bridge && ctx->bridge_attached)
drm_helper_hpd_irq_event(ctx->bridge.dev);
+
+ return 0;
+}
+
+static int anx7625_wait_hpd_asserted(struct drm_dp_aux *aux,
+ unsigned long wait_us)
+{
+ struct anx7625_data *ctx = container_of(aux, struct anx7625_data, aux);
+ struct device *dev = &ctx->client->dev;
+ int ret;
+
+ pm_runtime_get_sync(dev);
+ ret = _anx7625_hpd_polling(ctx, wait_us);
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
+ return ret;
}
static void anx7625_remove_edid(struct anx7625_data *ctx)
@@ -1741,6 +1759,7 @@ static struct edid *anx7625_get_edid(struct anx7625_data *ctx)
}
pm_runtime_get_sync(dev);
+ _anx7625_hpd_polling(ctx, 5000 * 100);
edid_num = sp_tx_edid_read(ctx, p_edid->edid_raw_data);
pm_runtime_put_sync(dev);
@@ -2378,6 +2397,7 @@ static void anx7625_bridge_atomic_enable(struct drm_bridge *bridge,
ctx->connector = connector;
pm_runtime_get_sync(dev);
+ _anx7625_hpd_polling(ctx, 5000 * 100);
anx7625_dp_start(ctx);
}
@@ -2497,7 +2517,6 @@ static int __maybe_unused anx7625_runtime_pm_resume(struct device *dev)
mutex_lock(&ctx->lock);
anx7625_power_on_init(ctx);
- anx7625_hpd_polling(ctx);
mutex_unlock(&ctx->lock);
@@ -2589,6 +2608,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
platform->aux.name = "anx7625-aux";
platform->aux.dev = dev;
platform->aux.transfer = anx7625_aux_transfer;
+ platform->aux.wait_hpd_asserted = anx7625_wait_hpd_asserted;
drm_dp_aux_init(&platform->aux);
if (anx7625_register_i2c_dummy_clients(platform, client) != 0) {
@@ -2617,6 +2637,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (!platform->pdata.low_power_mode) {
anx7625_disable_pd_protocol(platform);
pm_runtime_get_sync(dev);
+ _anx7625_hpd_polling(platform, 5000 * 100);
}
/* Add work function */
--
2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] drm/bridge: anx7625: Convert to devm_i2c_new_dummy_device()
2022-06-29 16:05 ` Hsin-Yi Wang
@ 2022-07-06 2:29 ` Xin Ji
-1 siblings, 0 replies; 22+ messages in thread
From: Xin Ji @ 2022-07-06 2:29 UTC (permalink / raw)
To: Hsin-Yi Wang
Cc: Robert Foss, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Sam Ravnborg, Thomas Zimmermann, Maxime Ripard, dri-devel,
linux-kernel
Hi Hsin-Yi, thanks for your patch, looks good to me.
Reviewed-by: Xin Ji <xji@analogixsemi.com>
On Thu, Jun 30, 2022 at 12:05:47AM +0800, Hsin-Yi Wang wrote:
> Simplify the resource management.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 96 +++++++----------------
> 1 file changed, 27 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 3710fa9ee0acd..f89e8151475f7 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -2436,82 +2436,44 @@ static const struct drm_bridge_funcs anx7625_bridge_funcs = {
> static int anx7625_register_i2c_dummy_clients(struct anx7625_data *ctx,
> struct i2c_client *client)
> {
> - int err = 0;
> + struct device *dev = &ctx->client->dev;
>
> - ctx->i2c.tx_p0_client = i2c_new_dummy_device(client->adapter,
> - TX_P0_ADDR >> 1);
> + ctx->i2c.tx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + TX_P0_ADDR >> 1);
> if (IS_ERR(ctx->i2c.tx_p0_client))
> return PTR_ERR(ctx->i2c.tx_p0_client);
>
> - ctx->i2c.tx_p1_client = i2c_new_dummy_device(client->adapter,
> - TX_P1_ADDR >> 1);
> - if (IS_ERR(ctx->i2c.tx_p1_client)) {
> - err = PTR_ERR(ctx->i2c.tx_p1_client);
> - goto free_tx_p0;
> - }
> + ctx->i2c.tx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + TX_P1_ADDR >> 1);
> + if (IS_ERR(ctx->i2c.tx_p1_client))
> + return PTR_ERR(ctx->i2c.tx_p1_client);
>
> - ctx->i2c.tx_p2_client = i2c_new_dummy_device(client->adapter,
> - TX_P2_ADDR >> 1);
> - if (IS_ERR(ctx->i2c.tx_p2_client)) {
> - err = PTR_ERR(ctx->i2c.tx_p2_client);
> - goto free_tx_p1;
> - }
> + ctx->i2c.tx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + TX_P2_ADDR >> 1);
> + if (IS_ERR(ctx->i2c.tx_p2_client))
> + return PTR_ERR(ctx->i2c.tx_p2_client);
>
> - ctx->i2c.rx_p0_client = i2c_new_dummy_device(client->adapter,
> - RX_P0_ADDR >> 1);
> - if (IS_ERR(ctx->i2c.rx_p0_client)) {
> - err = PTR_ERR(ctx->i2c.rx_p0_client);
> - goto free_tx_p2;
> - }
> + ctx->i2c.rx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + RX_P0_ADDR >> 1);
> + if (IS_ERR(ctx->i2c.rx_p0_client))
> + return PTR_ERR(ctx->i2c.rx_p0_client);
>
> - ctx->i2c.rx_p1_client = i2c_new_dummy_device(client->adapter,
> - RX_P1_ADDR >> 1);
> - if (IS_ERR(ctx->i2c.rx_p1_client)) {
> - err = PTR_ERR(ctx->i2c.rx_p1_client);
> - goto free_rx_p0;
> - }
> + ctx->i2c.rx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + RX_P1_ADDR >> 1);
> + if (IS_ERR(ctx->i2c.rx_p1_client))
> + return PTR_ERR(ctx->i2c.rx_p1_client);
>
> - ctx->i2c.rx_p2_client = i2c_new_dummy_device(client->adapter,
> - RX_P2_ADDR >> 1);
> - if (IS_ERR(ctx->i2c.rx_p2_client)) {
> - err = PTR_ERR(ctx->i2c.rx_p2_client);
> - goto free_rx_p1;
> - }
> + ctx->i2c.rx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + RX_P2_ADDR >> 1);
> + if (IS_ERR(ctx->i2c.rx_p2_client))
> + return PTR_ERR(ctx->i2c.rx_p2_client);
>
> - ctx->i2c.tcpc_client = i2c_new_dummy_device(client->adapter,
> - TCPC_INTERFACE_ADDR >> 1);
> - if (IS_ERR(ctx->i2c.tcpc_client)) {
> - err = PTR_ERR(ctx->i2c.tcpc_client);
> - goto free_rx_p2;
> - }
> + ctx->i2c.tcpc_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + TCPC_INTERFACE_ADDR >> 1);
> + if (IS_ERR(ctx->i2c.tcpc_client))
> + return PTR_ERR(ctx->i2c.tcpc_client);
>
> return 0;
> -
> -free_rx_p2:
> - i2c_unregister_device(ctx->i2c.rx_p2_client);
> -free_rx_p1:
> - i2c_unregister_device(ctx->i2c.rx_p1_client);
> -free_rx_p0:
> - i2c_unregister_device(ctx->i2c.rx_p0_client);
> -free_tx_p2:
> - i2c_unregister_device(ctx->i2c.tx_p2_client);
> -free_tx_p1:
> - i2c_unregister_device(ctx->i2c.tx_p1_client);
> -free_tx_p0:
> - i2c_unregister_device(ctx->i2c.tx_p0_client);
> -
> - return err;
> -}
> -
> -static void anx7625_unregister_i2c_dummy_clients(struct anx7625_data *ctx)
> -{
> - i2c_unregister_device(ctx->i2c.tx_p0_client);
> - i2c_unregister_device(ctx->i2c.tx_p1_client);
> - i2c_unregister_device(ctx->i2c.tx_p2_client);
> - i2c_unregister_device(ctx->i2c.rx_p0_client);
> - i2c_unregister_device(ctx->i2c.rx_p1_client);
> - i2c_unregister_device(ctx->i2c.rx_p2_client);
> - i2c_unregister_device(ctx->i2c.tcpc_client);
> }
>
> static int __maybe_unused anx7625_runtime_pm_suspend(struct device *dev)
> @@ -2723,8 +2685,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> if (!platform->pdata.low_power_mode)
> pm_runtime_put_sync_suspend(&client->dev);
>
> - anx7625_unregister_i2c_dummy_clients(platform);
> -
> free_wq:
> if (platform->workqueue)
> destroy_workqueue(platform->workqueue);
> @@ -2754,8 +2714,6 @@ static int anx7625_i2c_remove(struct i2c_client *client)
> if (!platform->pdata.low_power_mode)
> pm_runtime_put_sync_suspend(&client->dev);
>
> - anx7625_unregister_i2c_dummy_clients(platform);
> -
> if (platform->pdata.audio_en)
> anx7625_unregister_audio(platform);
>
> --
> 2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] drm/bridge: anx7625: Convert to devm_i2c_new_dummy_device()
@ 2022-07-06 2:29 ` Xin Ji
0 siblings, 0 replies; 22+ messages in thread
From: Xin Ji @ 2022-07-06 2:29 UTC (permalink / raw)
To: Hsin-Yi Wang
Cc: Thomas Zimmermann, Jonas Karlman, David Airlie, Robert Foss,
dri-devel, Neil Armstrong, linux-kernel, Jernej Skrabec,
Laurent Pinchart, Andrzej Hajda, Sam Ravnborg, Maxime Ripard
Hi Hsin-Yi, thanks for your patch, looks good to me.
Reviewed-by: Xin Ji <xji@analogixsemi.com>
On Thu, Jun 30, 2022 at 12:05:47AM +0800, Hsin-Yi Wang wrote:
> Simplify the resource management.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 96 +++++++----------------
> 1 file changed, 27 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 3710fa9ee0acd..f89e8151475f7 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -2436,82 +2436,44 @@ static const struct drm_bridge_funcs anx7625_bridge_funcs = {
> static int anx7625_register_i2c_dummy_clients(struct anx7625_data *ctx,
> struct i2c_client *client)
> {
> - int err = 0;
> + struct device *dev = &ctx->client->dev;
>
> - ctx->i2c.tx_p0_client = i2c_new_dummy_device(client->adapter,
> - TX_P0_ADDR >> 1);
> + ctx->i2c.tx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + TX_P0_ADDR >> 1);
> if (IS_ERR(ctx->i2c.tx_p0_client))
> return PTR_ERR(ctx->i2c.tx_p0_client);
>
> - ctx->i2c.tx_p1_client = i2c_new_dummy_device(client->adapter,
> - TX_P1_ADDR >> 1);
> - if (IS_ERR(ctx->i2c.tx_p1_client)) {
> - err = PTR_ERR(ctx->i2c.tx_p1_client);
> - goto free_tx_p0;
> - }
> + ctx->i2c.tx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + TX_P1_ADDR >> 1);
> + if (IS_ERR(ctx->i2c.tx_p1_client))
> + return PTR_ERR(ctx->i2c.tx_p1_client);
>
> - ctx->i2c.tx_p2_client = i2c_new_dummy_device(client->adapter,
> - TX_P2_ADDR >> 1);
> - if (IS_ERR(ctx->i2c.tx_p2_client)) {
> - err = PTR_ERR(ctx->i2c.tx_p2_client);
> - goto free_tx_p1;
> - }
> + ctx->i2c.tx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + TX_P2_ADDR >> 1);
> + if (IS_ERR(ctx->i2c.tx_p2_client))
> + return PTR_ERR(ctx->i2c.tx_p2_client);
>
> - ctx->i2c.rx_p0_client = i2c_new_dummy_device(client->adapter,
> - RX_P0_ADDR >> 1);
> - if (IS_ERR(ctx->i2c.rx_p0_client)) {
> - err = PTR_ERR(ctx->i2c.rx_p0_client);
> - goto free_tx_p2;
> - }
> + ctx->i2c.rx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + RX_P0_ADDR >> 1);
> + if (IS_ERR(ctx->i2c.rx_p0_client))
> + return PTR_ERR(ctx->i2c.rx_p0_client);
>
> - ctx->i2c.rx_p1_client = i2c_new_dummy_device(client->adapter,
> - RX_P1_ADDR >> 1);
> - if (IS_ERR(ctx->i2c.rx_p1_client)) {
> - err = PTR_ERR(ctx->i2c.rx_p1_client);
> - goto free_rx_p0;
> - }
> + ctx->i2c.rx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + RX_P1_ADDR >> 1);
> + if (IS_ERR(ctx->i2c.rx_p1_client))
> + return PTR_ERR(ctx->i2c.rx_p1_client);
>
> - ctx->i2c.rx_p2_client = i2c_new_dummy_device(client->adapter,
> - RX_P2_ADDR >> 1);
> - if (IS_ERR(ctx->i2c.rx_p2_client)) {
> - err = PTR_ERR(ctx->i2c.rx_p2_client);
> - goto free_rx_p1;
> - }
> + ctx->i2c.rx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + RX_P2_ADDR >> 1);
> + if (IS_ERR(ctx->i2c.rx_p2_client))
> + return PTR_ERR(ctx->i2c.rx_p2_client);
>
> - ctx->i2c.tcpc_client = i2c_new_dummy_device(client->adapter,
> - TCPC_INTERFACE_ADDR >> 1);
> - if (IS_ERR(ctx->i2c.tcpc_client)) {
> - err = PTR_ERR(ctx->i2c.tcpc_client);
> - goto free_rx_p2;
> - }
> + ctx->i2c.tcpc_client = devm_i2c_new_dummy_device(dev, client->adapter,
> + TCPC_INTERFACE_ADDR >> 1);
> + if (IS_ERR(ctx->i2c.tcpc_client))
> + return PTR_ERR(ctx->i2c.tcpc_client);
>
> return 0;
> -
> -free_rx_p2:
> - i2c_unregister_device(ctx->i2c.rx_p2_client);
> -free_rx_p1:
> - i2c_unregister_device(ctx->i2c.rx_p1_client);
> -free_rx_p0:
> - i2c_unregister_device(ctx->i2c.rx_p0_client);
> -free_tx_p2:
> - i2c_unregister_device(ctx->i2c.tx_p2_client);
> -free_tx_p1:
> - i2c_unregister_device(ctx->i2c.tx_p1_client);
> -free_tx_p0:
> - i2c_unregister_device(ctx->i2c.tx_p0_client);
> -
> - return err;
> -}
> -
> -static void anx7625_unregister_i2c_dummy_clients(struct anx7625_data *ctx)
> -{
> - i2c_unregister_device(ctx->i2c.tx_p0_client);
> - i2c_unregister_device(ctx->i2c.tx_p1_client);
> - i2c_unregister_device(ctx->i2c.tx_p2_client);
> - i2c_unregister_device(ctx->i2c.rx_p0_client);
> - i2c_unregister_device(ctx->i2c.rx_p1_client);
> - i2c_unregister_device(ctx->i2c.rx_p2_client);
> - i2c_unregister_device(ctx->i2c.tcpc_client);
> }
>
> static int __maybe_unused anx7625_runtime_pm_suspend(struct device *dev)
> @@ -2723,8 +2685,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> if (!platform->pdata.low_power_mode)
> pm_runtime_put_sync_suspend(&client->dev);
>
> - anx7625_unregister_i2c_dummy_clients(platform);
> -
> free_wq:
> if (platform->workqueue)
> destroy_workqueue(platform->workqueue);
> @@ -2754,8 +2714,6 @@ static int anx7625_i2c_remove(struct i2c_client *client)
> if (!platform->pdata.low_power_mode)
> pm_runtime_put_sync_suspend(&client->dev);
>
> - anx7625_unregister_i2c_dummy_clients(platform);
> -
> if (platform->pdata.audio_en)
> anx7625_unregister_audio(platform);
>
> --
> 2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] drm/bridge: anx7625: Use pm_runtime_force_suspend(resume)
2022-06-29 16:05 ` Hsin-Yi Wang
@ 2022-07-06 2:30 ` Xin Ji
-1 siblings, 0 replies; 22+ messages in thread
From: Xin Ji @ 2022-07-06 2:30 UTC (permalink / raw)
To: Hsin-Yi Wang
Cc: Robert Foss, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Sam Ravnborg, Thomas Zimmermann, Maxime Ripard, dri-devel,
linux-kernel
Hi Hsin-Yi, thanks for your patch, looks good to me.
Reviewed-by: Xin Ji <xji@analogixsemi.com>
Thanks,
Xin
On Thu, Jun 30, 2022 at 12:05:48AM +0800, Hsin-Yi Wang wrote:
> There's no need to check for IRQ or disable it in suspend.
>
> Use pm_runtime_force_suspend(resume) to make sure anx7625 is powered off
> correctly. Make the system suspend/resume and pm runtime suspend/resume
> more consistant.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 33 ++---------------------
> 1 file changed, 2 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index f89e8151475f7..478f5af381c7d 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -2504,38 +2504,9 @@ static int __maybe_unused anx7625_runtime_pm_resume(struct device *dev)
> return 0;
> }
>
> -static int __maybe_unused anx7625_resume(struct device *dev)
> -{
> - struct anx7625_data *ctx = dev_get_drvdata(dev);
> -
> - if (!ctx->pdata.intp_irq)
> - return 0;
> -
> - if (!pm_runtime_enabled(dev) || !pm_runtime_suspended(dev)) {
> - enable_irq(ctx->pdata.intp_irq);
> - anx7625_runtime_pm_resume(dev);
> - }
> -
> - return 0;
> -}
> -
> -static int __maybe_unused anx7625_suspend(struct device *dev)
> -{
> - struct anx7625_data *ctx = dev_get_drvdata(dev);
> -
> - if (!ctx->pdata.intp_irq)
> - return 0;
> -
> - if (!pm_runtime_enabled(dev) || !pm_runtime_suspended(dev)) {
> - anx7625_runtime_pm_suspend(dev);
> - disable_irq(ctx->pdata.intp_irq);
> - }
> -
> - return 0;
> -}
> -
> static const struct dev_pm_ops anx7625_pm_ops = {
> - SET_SYSTEM_SLEEP_PM_OPS(anx7625_suspend, anx7625_resume)
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> SET_RUNTIME_PM_OPS(anx7625_runtime_pm_suspend,
> anx7625_runtime_pm_resume, NULL)
> };
> --
> 2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] drm/bridge: anx7625: Use pm_runtime_force_suspend(resume)
@ 2022-07-06 2:30 ` Xin Ji
0 siblings, 0 replies; 22+ messages in thread
From: Xin Ji @ 2022-07-06 2:30 UTC (permalink / raw)
To: Hsin-Yi Wang
Cc: Thomas Zimmermann, Jonas Karlman, David Airlie, Robert Foss,
dri-devel, Neil Armstrong, linux-kernel, Jernej Skrabec,
Laurent Pinchart, Andrzej Hajda, Sam Ravnborg, Maxime Ripard
Hi Hsin-Yi, thanks for your patch, looks good to me.
Reviewed-by: Xin Ji <xji@analogixsemi.com>
Thanks,
Xin
On Thu, Jun 30, 2022 at 12:05:48AM +0800, Hsin-Yi Wang wrote:
> There's no need to check for IRQ or disable it in suspend.
>
> Use pm_runtime_force_suspend(resume) to make sure anx7625 is powered off
> correctly. Make the system suspend/resume and pm runtime suspend/resume
> more consistant.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 33 ++---------------------
> 1 file changed, 2 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index f89e8151475f7..478f5af381c7d 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -2504,38 +2504,9 @@ static int __maybe_unused anx7625_runtime_pm_resume(struct device *dev)
> return 0;
> }
>
> -static int __maybe_unused anx7625_resume(struct device *dev)
> -{
> - struct anx7625_data *ctx = dev_get_drvdata(dev);
> -
> - if (!ctx->pdata.intp_irq)
> - return 0;
> -
> - if (!pm_runtime_enabled(dev) || !pm_runtime_suspended(dev)) {
> - enable_irq(ctx->pdata.intp_irq);
> - anx7625_runtime_pm_resume(dev);
> - }
> -
> - return 0;
> -}
> -
> -static int __maybe_unused anx7625_suspend(struct device *dev)
> -{
> - struct anx7625_data *ctx = dev_get_drvdata(dev);
> -
> - if (!ctx->pdata.intp_irq)
> - return 0;
> -
> - if (!pm_runtime_enabled(dev) || !pm_runtime_suspended(dev)) {
> - anx7625_runtime_pm_suspend(dev);
> - disable_irq(ctx->pdata.intp_irq);
> - }
> -
> - return 0;
> -}
> -
> static const struct dev_pm_ops anx7625_pm_ops = {
> - SET_SYSTEM_SLEEP_PM_OPS(anx7625_suspend, anx7625_resume)
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> + pm_runtime_force_resume)
> SET_RUNTIME_PM_OPS(anx7625_runtime_pm_suspend,
> anx7625_runtime_pm_resume, NULL)
> };
> --
> 2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] drm/bridge: anx7625: Fix NULL pointer crash when using edp-panel
2022-06-29 16:05 ` Hsin-Yi Wang
@ 2022-07-06 2:31 ` Xin Ji
-1 siblings, 0 replies; 22+ messages in thread
From: Xin Ji @ 2022-07-06 2:31 UTC (permalink / raw)
To: Hsin-Yi Wang
Cc: Robert Foss, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Sam Ravnborg, Thomas Zimmermann, Maxime Ripard, dri-devel,
linux-kernel
Hi Hsin-Yi, thanks for your patch, it looks good to me.
Reviewed-by: Xin Ji <xji@analogixsemi.com>
Thanks,
Xin
On Thu, Jun 30, 2022 at 12:05:49AM +0800, Hsin-Yi Wang wrote:
> Move devm_of_dp_aux_populate_ep_devices() after pm runtime and i2c setup
> to avoid NULL pointer crash.
>
> edp-panel probe (generic_edp_panel_probe) calls pm_runtime_get_sync() to
> read EDID. At this time, bridge should have pm runtime enabled and i2c
> clients ready.
>
> Fixes: adca62ec370c ("drm/bridge: anx7625: Support reading edid through aux channel")
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 478f5af381c7d..59ddeba33652b 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -2590,14 +2590,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> platform->aux.dev = dev;
> platform->aux.transfer = anx7625_aux_transfer;
> drm_dp_aux_init(&platform->aux);
> - devm_of_dp_aux_populate_ep_devices(&platform->aux);
> -
> - ret = anx7625_parse_dt(dev, pdata);
> - if (ret) {
> - if (ret != -EPROBE_DEFER)
> - DRM_DEV_ERROR(dev, "fail to parse DT : %d\n", ret);
> - goto free_wq;
> - }
>
> if (anx7625_register_i2c_dummy_clients(platform, client) != 0) {
> ret = -ENOMEM;
> @@ -2613,6 +2605,15 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> if (ret)
> goto free_wq;
>
> + devm_of_dp_aux_populate_ep_devices(&platform->aux);
> +
> + ret = anx7625_parse_dt(dev, pdata);
> + if (ret) {
> + if (ret != -EPROBE_DEFER)
> + DRM_DEV_ERROR(dev, "fail to parse DT : %d\n", ret);
> + goto free_wq;
> + }
> +
> if (!platform->pdata.low_power_mode) {
> anx7625_disable_pd_protocol(platform);
> pm_runtime_get_sync(dev);
> --
> 2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] drm/bridge: anx7625: Fix NULL pointer crash when using edp-panel
@ 2022-07-06 2:31 ` Xin Ji
0 siblings, 0 replies; 22+ messages in thread
From: Xin Ji @ 2022-07-06 2:31 UTC (permalink / raw)
To: Hsin-Yi Wang
Cc: Thomas Zimmermann, Jonas Karlman, David Airlie, Robert Foss,
dri-devel, Neil Armstrong, linux-kernel, Jernej Skrabec,
Laurent Pinchart, Andrzej Hajda, Sam Ravnborg, Maxime Ripard
Hi Hsin-Yi, thanks for your patch, it looks good to me.
Reviewed-by: Xin Ji <xji@analogixsemi.com>
Thanks,
Xin
On Thu, Jun 30, 2022 at 12:05:49AM +0800, Hsin-Yi Wang wrote:
> Move devm_of_dp_aux_populate_ep_devices() after pm runtime and i2c setup
> to avoid NULL pointer crash.
>
> edp-panel probe (generic_edp_panel_probe) calls pm_runtime_get_sync() to
> read EDID. At this time, bridge should have pm runtime enabled and i2c
> clients ready.
>
> Fixes: adca62ec370c ("drm/bridge: anx7625: Support reading edid through aux channel")
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 478f5af381c7d..59ddeba33652b 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -2590,14 +2590,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> platform->aux.dev = dev;
> platform->aux.transfer = anx7625_aux_transfer;
> drm_dp_aux_init(&platform->aux);
> - devm_of_dp_aux_populate_ep_devices(&platform->aux);
> -
> - ret = anx7625_parse_dt(dev, pdata);
> - if (ret) {
> - if (ret != -EPROBE_DEFER)
> - DRM_DEV_ERROR(dev, "fail to parse DT : %d\n", ret);
> - goto free_wq;
> - }
>
> if (anx7625_register_i2c_dummy_clients(platform, client) != 0) {
> ret = -ENOMEM;
> @@ -2613,6 +2605,15 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> if (ret)
> goto free_wq;
>
> + devm_of_dp_aux_populate_ep_devices(&platform->aux);
> +
> + ret = anx7625_parse_dt(dev, pdata);
> + if (ret) {
> + if (ret != -EPROBE_DEFER)
> + DRM_DEV_ERROR(dev, "fail to parse DT : %d\n", ret);
> + goto free_wq;
> + }
> +
> if (!platform->pdata.low_power_mode) {
> anx7625_disable_pd_protocol(platform);
> pm_runtime_get_sync(dev);
> --
> 2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4/4] drm/bridge: anx7625: Add wait_hpd_asserted() callback
2022-06-29 16:05 ` Hsin-Yi Wang
@ 2022-07-06 2:32 ` Xin Ji
-1 siblings, 0 replies; 22+ messages in thread
From: Xin Ji @ 2022-07-06 2:32 UTC (permalink / raw)
To: Hsin-Yi Wang
Cc: Robert Foss, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Sam Ravnborg, Thomas Zimmermann, Maxime Ripard, dri-devel,
linux-kernel
Hi Hsin-Yi, thanks for your patch, it looks good to me.
Reviewed-by: Xin Ji <xji@analogixsemi.com>
Thanks,
Xin
On Thu, Jun 30, 2022 at 12:05:50AM +0800, Hsin-Yi Wang wrote:
> Move hpd polling check into wait_hpd_asserted() callback. For the cases
> that aux transfer function wasn't used, do hpd polling check after pm
> runtime resume, which will power on the bridge.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 33 ++++++++++++++++++-----
> 1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 59ddeba33652b..ea5a0b86fe52a 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -1443,23 +1443,24 @@ static int anx7625_read_hpd_status_p0(struct anx7625_data *ctx)
> return anx7625_reg_read(ctx, ctx->i2c.rx_p0_client, SYSTEM_STSTUS);
> }
>
> -static void anx7625_hpd_polling(struct anx7625_data *ctx)
> +static int _anx7625_hpd_polling(struct anx7625_data *ctx,
> + unsigned long wait_us)
> {
> int ret, val;
> struct device *dev = &ctx->client->dev;
>
> /* Interrupt mode, no need poll HPD status, just return */
> if (ctx->pdata.intp_irq)
> - return;
> + return 0;
>
> ret = readx_poll_timeout(anx7625_read_hpd_status_p0,
> ctx, val,
> ((val & HPD_STATUS) || (val < 0)),
> - 5000,
> - 5000 * 100);
> + wait_us / 100,
> + wait_us);
> if (ret) {
> DRM_DEV_ERROR(dev, "no hpd.\n");
> - return;
> + return ret;
> }
>
> DRM_DEV_DEBUG_DRIVER(dev, "system status: 0x%x. HPD raise up.\n", val);
> @@ -1472,6 +1473,23 @@ static void anx7625_hpd_polling(struct anx7625_data *ctx)
>
> if (!ctx->pdata.panel_bridge && ctx->bridge_attached)
> drm_helper_hpd_irq_event(ctx->bridge.dev);
> +
> + return 0;
> +}
> +
> +static int anx7625_wait_hpd_asserted(struct drm_dp_aux *aux,
> + unsigned long wait_us)
> +{
> + struct anx7625_data *ctx = container_of(aux, struct anx7625_data, aux);
> + struct device *dev = &ctx->client->dev;
> + int ret;
> +
> + pm_runtime_get_sync(dev);
> + ret = _anx7625_hpd_polling(ctx, wait_us);
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
> +
> + return ret;
> }
>
> static void anx7625_remove_edid(struct anx7625_data *ctx)
> @@ -1741,6 +1759,7 @@ static struct edid *anx7625_get_edid(struct anx7625_data *ctx)
> }
>
> pm_runtime_get_sync(dev);
> + _anx7625_hpd_polling(ctx, 5000 * 100);
> edid_num = sp_tx_edid_read(ctx, p_edid->edid_raw_data);
> pm_runtime_put_sync(dev);
>
> @@ -2378,6 +2397,7 @@ static void anx7625_bridge_atomic_enable(struct drm_bridge *bridge,
> ctx->connector = connector;
>
> pm_runtime_get_sync(dev);
> + _anx7625_hpd_polling(ctx, 5000 * 100);
>
> anx7625_dp_start(ctx);
> }
> @@ -2497,7 +2517,6 @@ static int __maybe_unused anx7625_runtime_pm_resume(struct device *dev)
> mutex_lock(&ctx->lock);
>
> anx7625_power_on_init(ctx);
> - anx7625_hpd_polling(ctx);
>
> mutex_unlock(&ctx->lock);
>
> @@ -2589,6 +2608,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> platform->aux.name = "anx7625-aux";
> platform->aux.dev = dev;
> platform->aux.transfer = anx7625_aux_transfer;
> + platform->aux.wait_hpd_asserted = anx7625_wait_hpd_asserted;
> drm_dp_aux_init(&platform->aux);
>
> if (anx7625_register_i2c_dummy_clients(platform, client) != 0) {
> @@ -2617,6 +2637,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> if (!platform->pdata.low_power_mode) {
> anx7625_disable_pd_protocol(platform);
> pm_runtime_get_sync(dev);
> + _anx7625_hpd_polling(platform, 5000 * 100);
> }
>
> /* Add work function */
> --
> 2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4/4] drm/bridge: anx7625: Add wait_hpd_asserted() callback
@ 2022-07-06 2:32 ` Xin Ji
0 siblings, 0 replies; 22+ messages in thread
From: Xin Ji @ 2022-07-06 2:32 UTC (permalink / raw)
To: Hsin-Yi Wang
Cc: Thomas Zimmermann, Jonas Karlman, David Airlie, Robert Foss,
dri-devel, Neil Armstrong, linux-kernel, Jernej Skrabec,
Laurent Pinchart, Andrzej Hajda, Sam Ravnborg, Maxime Ripard
Hi Hsin-Yi, thanks for your patch, it looks good to me.
Reviewed-by: Xin Ji <xji@analogixsemi.com>
Thanks,
Xin
On Thu, Jun 30, 2022 at 12:05:50AM +0800, Hsin-Yi Wang wrote:
> Move hpd polling check into wait_hpd_asserted() callback. For the cases
> that aux transfer function wasn't used, do hpd polling check after pm
> runtime resume, which will power on the bridge.
>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
> drivers/gpu/drm/bridge/analogix/anx7625.c | 33 ++++++++++++++++++-----
> 1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 59ddeba33652b..ea5a0b86fe52a 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -1443,23 +1443,24 @@ static int anx7625_read_hpd_status_p0(struct anx7625_data *ctx)
> return anx7625_reg_read(ctx, ctx->i2c.rx_p0_client, SYSTEM_STSTUS);
> }
>
> -static void anx7625_hpd_polling(struct anx7625_data *ctx)
> +static int _anx7625_hpd_polling(struct anx7625_data *ctx,
> + unsigned long wait_us)
> {
> int ret, val;
> struct device *dev = &ctx->client->dev;
>
> /* Interrupt mode, no need poll HPD status, just return */
> if (ctx->pdata.intp_irq)
> - return;
> + return 0;
>
> ret = readx_poll_timeout(anx7625_read_hpd_status_p0,
> ctx, val,
> ((val & HPD_STATUS) || (val < 0)),
> - 5000,
> - 5000 * 100);
> + wait_us / 100,
> + wait_us);
> if (ret) {
> DRM_DEV_ERROR(dev, "no hpd.\n");
> - return;
> + return ret;
> }
>
> DRM_DEV_DEBUG_DRIVER(dev, "system status: 0x%x. HPD raise up.\n", val);
> @@ -1472,6 +1473,23 @@ static void anx7625_hpd_polling(struct anx7625_data *ctx)
>
> if (!ctx->pdata.panel_bridge && ctx->bridge_attached)
> drm_helper_hpd_irq_event(ctx->bridge.dev);
> +
> + return 0;
> +}
> +
> +static int anx7625_wait_hpd_asserted(struct drm_dp_aux *aux,
> + unsigned long wait_us)
> +{
> + struct anx7625_data *ctx = container_of(aux, struct anx7625_data, aux);
> + struct device *dev = &ctx->client->dev;
> + int ret;
> +
> + pm_runtime_get_sync(dev);
> + ret = _anx7625_hpd_polling(ctx, wait_us);
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
> +
> + return ret;
> }
>
> static void anx7625_remove_edid(struct anx7625_data *ctx)
> @@ -1741,6 +1759,7 @@ static struct edid *anx7625_get_edid(struct anx7625_data *ctx)
> }
>
> pm_runtime_get_sync(dev);
> + _anx7625_hpd_polling(ctx, 5000 * 100);
> edid_num = sp_tx_edid_read(ctx, p_edid->edid_raw_data);
> pm_runtime_put_sync(dev);
>
> @@ -2378,6 +2397,7 @@ static void anx7625_bridge_atomic_enable(struct drm_bridge *bridge,
> ctx->connector = connector;
>
> pm_runtime_get_sync(dev);
> + _anx7625_hpd_polling(ctx, 5000 * 100);
>
> anx7625_dp_start(ctx);
> }
> @@ -2497,7 +2517,6 @@ static int __maybe_unused anx7625_runtime_pm_resume(struct device *dev)
> mutex_lock(&ctx->lock);
>
> anx7625_power_on_init(ctx);
> - anx7625_hpd_polling(ctx);
>
> mutex_unlock(&ctx->lock);
>
> @@ -2589,6 +2608,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> platform->aux.name = "anx7625-aux";
> platform->aux.dev = dev;
> platform->aux.transfer = anx7625_aux_transfer;
> + platform->aux.wait_hpd_asserted = anx7625_wait_hpd_asserted;
> drm_dp_aux_init(&platform->aux);
>
> if (anx7625_register_i2c_dummy_clients(platform, client) != 0) {
> @@ -2617,6 +2637,7 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> if (!platform->pdata.low_power_mode) {
> anx7625_disable_pd_protocol(platform);
> pm_runtime_get_sync(dev);
> + _anx7625_hpd_polling(platform, 5000 * 100);
> }
>
> /* Add work function */
> --
> 2.37.0.rc0.161.g10f37bed90-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] drm/bridge: anx7625: Convert to devm_i2c_new_dummy_device()
2022-07-06 2:29 ` Xin Ji
@ 2022-07-06 12:31 ` Robert Foss
-1 siblings, 0 replies; 22+ messages in thread
From: Robert Foss @ 2022-07-06 12:31 UTC (permalink / raw)
To: Xin Ji
Cc: Hsin-Yi Wang, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Sam Ravnborg, Thomas Zimmermann, Maxime Ripard, dri-devel,
linux-kernel
Hey Hsin-Yi,
On Wed, 6 Jul 2022 at 04:29, Xin Ji <xji@analogixsemi.com> wrote:
>
> Hi Hsin-Yi, thanks for your patch, looks good to me.
>
> Reviewed-by: Xin Ji <xji@analogixsemi.com>
>
> On Thu, Jun 30, 2022 at 12:05:47AM +0800, Hsin-Yi Wang wrote:
> > Simplify the resource management.
> >
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > ---
> > drivers/gpu/drm/bridge/analogix/anx7625.c | 96 +++++++----------------
> > 1 file changed, 27 insertions(+), 69 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > index 3710fa9ee0acd..f89e8151475f7 100644
> > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > @@ -2436,82 +2436,44 @@ static const struct drm_bridge_funcs anx7625_bridge_funcs = {
> > static int anx7625_register_i2c_dummy_clients(struct anx7625_data *ctx,
> > struct i2c_client *client)
> > {
> > - int err = 0;
> > + struct device *dev = &ctx->client->dev;
> >
> > - ctx->i2c.tx_p0_client = i2c_new_dummy_device(client->adapter,
> > - TX_P0_ADDR >> 1);
> > + ctx->i2c.tx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + TX_P0_ADDR >> 1);
> > if (IS_ERR(ctx->i2c.tx_p0_client))
> > return PTR_ERR(ctx->i2c.tx_p0_client);
> >
> > - ctx->i2c.tx_p1_client = i2c_new_dummy_device(client->adapter,
> > - TX_P1_ADDR >> 1);
> > - if (IS_ERR(ctx->i2c.tx_p1_client)) {
> > - err = PTR_ERR(ctx->i2c.tx_p1_client);
> > - goto free_tx_p0;
> > - }
> > + ctx->i2c.tx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + TX_P1_ADDR >> 1);
> > + if (IS_ERR(ctx->i2c.tx_p1_client))
> > + return PTR_ERR(ctx->i2c.tx_p1_client);
> >
> > - ctx->i2c.tx_p2_client = i2c_new_dummy_device(client->adapter,
> > - TX_P2_ADDR >> 1);
> > - if (IS_ERR(ctx->i2c.tx_p2_client)) {
> > - err = PTR_ERR(ctx->i2c.tx_p2_client);
> > - goto free_tx_p1;
> > - }
> > + ctx->i2c.tx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + TX_P2_ADDR >> 1);
> > + if (IS_ERR(ctx->i2c.tx_p2_client))
> > + return PTR_ERR(ctx->i2c.tx_p2_client);
> >
> > - ctx->i2c.rx_p0_client = i2c_new_dummy_device(client->adapter,
> > - RX_P0_ADDR >> 1);
> > - if (IS_ERR(ctx->i2c.rx_p0_client)) {
> > - err = PTR_ERR(ctx->i2c.rx_p0_client);
> > - goto free_tx_p2;
> > - }
> > + ctx->i2c.rx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + RX_P0_ADDR >> 1);
> > + if (IS_ERR(ctx->i2c.rx_p0_client))
> > + return PTR_ERR(ctx->i2c.rx_p0_client);
> >
> > - ctx->i2c.rx_p1_client = i2c_new_dummy_device(client->adapter,
> > - RX_P1_ADDR >> 1);
> > - if (IS_ERR(ctx->i2c.rx_p1_client)) {
> > - err = PTR_ERR(ctx->i2c.rx_p1_client);
> > - goto free_rx_p0;
> > - }
> > + ctx->i2c.rx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + RX_P1_ADDR >> 1);
> > + if (IS_ERR(ctx->i2c.rx_p1_client))
> > + return PTR_ERR(ctx->i2c.rx_p1_client);
> >
> > - ctx->i2c.rx_p2_client = i2c_new_dummy_device(client->adapter,
> > - RX_P2_ADDR >> 1);
> > - if (IS_ERR(ctx->i2c.rx_p2_client)) {
> > - err = PTR_ERR(ctx->i2c.rx_p2_client);
> > - goto free_rx_p1;
> > - }
> > + ctx->i2c.rx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + RX_P2_ADDR >> 1);
> > + if (IS_ERR(ctx->i2c.rx_p2_client))
> > + return PTR_ERR(ctx->i2c.rx_p2_client);
> >
> > - ctx->i2c.tcpc_client = i2c_new_dummy_device(client->adapter,
> > - TCPC_INTERFACE_ADDR >> 1);
> > - if (IS_ERR(ctx->i2c.tcpc_client)) {
> > - err = PTR_ERR(ctx->i2c.tcpc_client);
> > - goto free_rx_p2;
> > - }
> > + ctx->i2c.tcpc_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + TCPC_INTERFACE_ADDR >> 1);
> > + if (IS_ERR(ctx->i2c.tcpc_client))
> > + return PTR_ERR(ctx->i2c.tcpc_client);
> >
> > return 0;
> > -
> > -free_rx_p2:
> > - i2c_unregister_device(ctx->i2c.rx_p2_client);
> > -free_rx_p1:
> > - i2c_unregister_device(ctx->i2c.rx_p1_client);
> > -free_rx_p0:
> > - i2c_unregister_device(ctx->i2c.rx_p0_client);
> > -free_tx_p2:
> > - i2c_unregister_device(ctx->i2c.tx_p2_client);
> > -free_tx_p1:
> > - i2c_unregister_device(ctx->i2c.tx_p1_client);
> > -free_tx_p0:
> > - i2c_unregister_device(ctx->i2c.tx_p0_client);
> > -
> > - return err;
> > -}
> > -
> > -static void anx7625_unregister_i2c_dummy_clients(struct anx7625_data *ctx)
> > -{
> > - i2c_unregister_device(ctx->i2c.tx_p0_client);
> > - i2c_unregister_device(ctx->i2c.tx_p1_client);
> > - i2c_unregister_device(ctx->i2c.tx_p2_client);
> > - i2c_unregister_device(ctx->i2c.rx_p0_client);
> > - i2c_unregister_device(ctx->i2c.rx_p1_client);
> > - i2c_unregister_device(ctx->i2c.rx_p2_client);
> > - i2c_unregister_device(ctx->i2c.tcpc_client);
> > }
> >
> > static int __maybe_unused anx7625_runtime_pm_suspend(struct device *dev)
> > @@ -2723,8 +2685,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> > if (!platform->pdata.low_power_mode)
> > pm_runtime_put_sync_suspend(&client->dev);
> >
> > - anx7625_unregister_i2c_dummy_clients(platform);
> > -
> > free_wq:
> > if (platform->workqueue)
> > destroy_workqueue(platform->workqueue);
> > @@ -2754,8 +2714,6 @@ static int anx7625_i2c_remove(struct i2c_client *client)
> > if (!platform->pdata.low_power_mode)
> > pm_runtime_put_sync_suspend(&client->dev);
> >
> > - anx7625_unregister_i2c_dummy_clients(platform);
> > -
> > if (platform->pdata.audio_en)
> > anx7625_unregister_audio(platform);
> >
> > --
> > 2.37.0.rc0.161.g10f37bed90-goog
Can you fix the checkpatch --strict warnings and formatting issues in
this series?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] drm/bridge: anx7625: Convert to devm_i2c_new_dummy_device()
@ 2022-07-06 12:31 ` Robert Foss
0 siblings, 0 replies; 22+ messages in thread
From: Robert Foss @ 2022-07-06 12:31 UTC (permalink / raw)
To: Xin Ji
Cc: Jonas Karlman, David Airlie, Thomas Zimmermann, dri-devel,
Neil Armstrong, linux-kernel, Jernej Skrabec, Laurent Pinchart,
Andrzej Hajda, Hsin-Yi Wang, Sam Ravnborg, Maxime Ripard
Hey Hsin-Yi,
On Wed, 6 Jul 2022 at 04:29, Xin Ji <xji@analogixsemi.com> wrote:
>
> Hi Hsin-Yi, thanks for your patch, looks good to me.
>
> Reviewed-by: Xin Ji <xji@analogixsemi.com>
>
> On Thu, Jun 30, 2022 at 12:05:47AM +0800, Hsin-Yi Wang wrote:
> > Simplify the resource management.
> >
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > ---
> > drivers/gpu/drm/bridge/analogix/anx7625.c | 96 +++++++----------------
> > 1 file changed, 27 insertions(+), 69 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > index 3710fa9ee0acd..f89e8151475f7 100644
> > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > @@ -2436,82 +2436,44 @@ static const struct drm_bridge_funcs anx7625_bridge_funcs = {
> > static int anx7625_register_i2c_dummy_clients(struct anx7625_data *ctx,
> > struct i2c_client *client)
> > {
> > - int err = 0;
> > + struct device *dev = &ctx->client->dev;
> >
> > - ctx->i2c.tx_p0_client = i2c_new_dummy_device(client->adapter,
> > - TX_P0_ADDR >> 1);
> > + ctx->i2c.tx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + TX_P0_ADDR >> 1);
> > if (IS_ERR(ctx->i2c.tx_p0_client))
> > return PTR_ERR(ctx->i2c.tx_p0_client);
> >
> > - ctx->i2c.tx_p1_client = i2c_new_dummy_device(client->adapter,
> > - TX_P1_ADDR >> 1);
> > - if (IS_ERR(ctx->i2c.tx_p1_client)) {
> > - err = PTR_ERR(ctx->i2c.tx_p1_client);
> > - goto free_tx_p0;
> > - }
> > + ctx->i2c.tx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + TX_P1_ADDR >> 1);
> > + if (IS_ERR(ctx->i2c.tx_p1_client))
> > + return PTR_ERR(ctx->i2c.tx_p1_client);
> >
> > - ctx->i2c.tx_p2_client = i2c_new_dummy_device(client->adapter,
> > - TX_P2_ADDR >> 1);
> > - if (IS_ERR(ctx->i2c.tx_p2_client)) {
> > - err = PTR_ERR(ctx->i2c.tx_p2_client);
> > - goto free_tx_p1;
> > - }
> > + ctx->i2c.tx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + TX_P2_ADDR >> 1);
> > + if (IS_ERR(ctx->i2c.tx_p2_client))
> > + return PTR_ERR(ctx->i2c.tx_p2_client);
> >
> > - ctx->i2c.rx_p0_client = i2c_new_dummy_device(client->adapter,
> > - RX_P0_ADDR >> 1);
> > - if (IS_ERR(ctx->i2c.rx_p0_client)) {
> > - err = PTR_ERR(ctx->i2c.rx_p0_client);
> > - goto free_tx_p2;
> > - }
> > + ctx->i2c.rx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + RX_P0_ADDR >> 1);
> > + if (IS_ERR(ctx->i2c.rx_p0_client))
> > + return PTR_ERR(ctx->i2c.rx_p0_client);
> >
> > - ctx->i2c.rx_p1_client = i2c_new_dummy_device(client->adapter,
> > - RX_P1_ADDR >> 1);
> > - if (IS_ERR(ctx->i2c.rx_p1_client)) {
> > - err = PTR_ERR(ctx->i2c.rx_p1_client);
> > - goto free_rx_p0;
> > - }
> > + ctx->i2c.rx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + RX_P1_ADDR >> 1);
> > + if (IS_ERR(ctx->i2c.rx_p1_client))
> > + return PTR_ERR(ctx->i2c.rx_p1_client);
> >
> > - ctx->i2c.rx_p2_client = i2c_new_dummy_device(client->adapter,
> > - RX_P2_ADDR >> 1);
> > - if (IS_ERR(ctx->i2c.rx_p2_client)) {
> > - err = PTR_ERR(ctx->i2c.rx_p2_client);
> > - goto free_rx_p1;
> > - }
> > + ctx->i2c.rx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + RX_P2_ADDR >> 1);
> > + if (IS_ERR(ctx->i2c.rx_p2_client))
> > + return PTR_ERR(ctx->i2c.rx_p2_client);
> >
> > - ctx->i2c.tcpc_client = i2c_new_dummy_device(client->adapter,
> > - TCPC_INTERFACE_ADDR >> 1);
> > - if (IS_ERR(ctx->i2c.tcpc_client)) {
> > - err = PTR_ERR(ctx->i2c.tcpc_client);
> > - goto free_rx_p2;
> > - }
> > + ctx->i2c.tcpc_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > + TCPC_INTERFACE_ADDR >> 1);
> > + if (IS_ERR(ctx->i2c.tcpc_client))
> > + return PTR_ERR(ctx->i2c.tcpc_client);
> >
> > return 0;
> > -
> > -free_rx_p2:
> > - i2c_unregister_device(ctx->i2c.rx_p2_client);
> > -free_rx_p1:
> > - i2c_unregister_device(ctx->i2c.rx_p1_client);
> > -free_rx_p0:
> > - i2c_unregister_device(ctx->i2c.rx_p0_client);
> > -free_tx_p2:
> > - i2c_unregister_device(ctx->i2c.tx_p2_client);
> > -free_tx_p1:
> > - i2c_unregister_device(ctx->i2c.tx_p1_client);
> > -free_tx_p0:
> > - i2c_unregister_device(ctx->i2c.tx_p0_client);
> > -
> > - return err;
> > -}
> > -
> > -static void anx7625_unregister_i2c_dummy_clients(struct anx7625_data *ctx)
> > -{
> > - i2c_unregister_device(ctx->i2c.tx_p0_client);
> > - i2c_unregister_device(ctx->i2c.tx_p1_client);
> > - i2c_unregister_device(ctx->i2c.tx_p2_client);
> > - i2c_unregister_device(ctx->i2c.rx_p0_client);
> > - i2c_unregister_device(ctx->i2c.rx_p1_client);
> > - i2c_unregister_device(ctx->i2c.rx_p2_client);
> > - i2c_unregister_device(ctx->i2c.tcpc_client);
> > }
> >
> > static int __maybe_unused anx7625_runtime_pm_suspend(struct device *dev)
> > @@ -2723,8 +2685,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> > if (!platform->pdata.low_power_mode)
> > pm_runtime_put_sync_suspend(&client->dev);
> >
> > - anx7625_unregister_i2c_dummy_clients(platform);
> > -
> > free_wq:
> > if (platform->workqueue)
> > destroy_workqueue(platform->workqueue);
> > @@ -2754,8 +2714,6 @@ static int anx7625_i2c_remove(struct i2c_client *client)
> > if (!platform->pdata.low_power_mode)
> > pm_runtime_put_sync_suspend(&client->dev);
> >
> > - anx7625_unregister_i2c_dummy_clients(platform);
> > -
> > if (platform->pdata.audio_en)
> > anx7625_unregister_audio(platform);
> >
> > --
> > 2.37.0.rc0.161.g10f37bed90-goog
Can you fix the checkpatch --strict warnings and formatting issues in
this series?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] drm/bridge: anx7625: Convert to devm_i2c_new_dummy_device()
2022-07-06 12:31 ` Robert Foss
@ 2022-07-06 12:55 ` Hsin-Yi Wang
-1 siblings, 0 replies; 22+ messages in thread
From: Hsin-Yi Wang @ 2022-07-06 12:55 UTC (permalink / raw)
To: Robert Foss
Cc: Xin Ji, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, David Airlie, Daniel Vetter,
Sam Ravnborg, Thomas Zimmermann, Maxime Ripard, dri-devel,
linux-kernel
On Wed, Jul 6, 2022 at 8:31 PM Robert Foss <robert.foss@linaro.org> wrote:
>
> Hey Hsin-Yi,
>
> On Wed, 6 Jul 2022 at 04:29, Xin Ji <xji@analogixsemi.com> wrote:
> >
> > Hi Hsin-Yi, thanks for your patch, looks good to me.
> >
> > Reviewed-by: Xin Ji <xji@analogixsemi.com>
> >
> > On Thu, Jun 30, 2022 at 12:05:47AM +0800, Hsin-Yi Wang wrote:
> > > Simplify the resource management.
> > >
> > > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > > ---
> > > drivers/gpu/drm/bridge/analogix/anx7625.c | 96 +++++++----------------
> > > 1 file changed, 27 insertions(+), 69 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > index 3710fa9ee0acd..f89e8151475f7 100644
> > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > @@ -2436,82 +2436,44 @@ static const struct drm_bridge_funcs anx7625_bridge_funcs = {
> > > static int anx7625_register_i2c_dummy_clients(struct anx7625_data *ctx,
> > > struct i2c_client *client)
> > > {
> > > - int err = 0;
> > > + struct device *dev = &ctx->client->dev;
> > >
> > > - ctx->i2c.tx_p0_client = i2c_new_dummy_device(client->adapter,
> > > - TX_P0_ADDR >> 1);
> > > + ctx->i2c.tx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + TX_P0_ADDR >> 1);
> > > if (IS_ERR(ctx->i2c.tx_p0_client))
> > > return PTR_ERR(ctx->i2c.tx_p0_client);
> > >
> > > - ctx->i2c.tx_p1_client = i2c_new_dummy_device(client->adapter,
> > > - TX_P1_ADDR >> 1);
> > > - if (IS_ERR(ctx->i2c.tx_p1_client)) {
> > > - err = PTR_ERR(ctx->i2c.tx_p1_client);
> > > - goto free_tx_p0;
> > > - }
> > > + ctx->i2c.tx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + TX_P1_ADDR >> 1);
> > > + if (IS_ERR(ctx->i2c.tx_p1_client))
> > > + return PTR_ERR(ctx->i2c.tx_p1_client);
> > >
> > > - ctx->i2c.tx_p2_client = i2c_new_dummy_device(client->adapter,
> > > - TX_P2_ADDR >> 1);
> > > - if (IS_ERR(ctx->i2c.tx_p2_client)) {
> > > - err = PTR_ERR(ctx->i2c.tx_p2_client);
> > > - goto free_tx_p1;
> > > - }
> > > + ctx->i2c.tx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + TX_P2_ADDR >> 1);
> > > + if (IS_ERR(ctx->i2c.tx_p2_client))
> > > + return PTR_ERR(ctx->i2c.tx_p2_client);
> > >
> > > - ctx->i2c.rx_p0_client = i2c_new_dummy_device(client->adapter,
> > > - RX_P0_ADDR >> 1);
> > > - if (IS_ERR(ctx->i2c.rx_p0_client)) {
> > > - err = PTR_ERR(ctx->i2c.rx_p0_client);
> > > - goto free_tx_p2;
> > > - }
> > > + ctx->i2c.rx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + RX_P0_ADDR >> 1);
> > > + if (IS_ERR(ctx->i2c.rx_p0_client))
> > > + return PTR_ERR(ctx->i2c.rx_p0_client);
> > >
> > > - ctx->i2c.rx_p1_client = i2c_new_dummy_device(client->adapter,
> > > - RX_P1_ADDR >> 1);
> > > - if (IS_ERR(ctx->i2c.rx_p1_client)) {
> > > - err = PTR_ERR(ctx->i2c.rx_p1_client);
> > > - goto free_rx_p0;
> > > - }
> > > + ctx->i2c.rx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + RX_P1_ADDR >> 1);
> > > + if (IS_ERR(ctx->i2c.rx_p1_client))
> > > + return PTR_ERR(ctx->i2c.rx_p1_client);
> > >
> > > - ctx->i2c.rx_p2_client = i2c_new_dummy_device(client->adapter,
> > > - RX_P2_ADDR >> 1);
> > > - if (IS_ERR(ctx->i2c.rx_p2_client)) {
> > > - err = PTR_ERR(ctx->i2c.rx_p2_client);
> > > - goto free_rx_p1;
> > > - }
> > > + ctx->i2c.rx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + RX_P2_ADDR >> 1);
> > > + if (IS_ERR(ctx->i2c.rx_p2_client))
> > > + return PTR_ERR(ctx->i2c.rx_p2_client);
> > >
> > > - ctx->i2c.tcpc_client = i2c_new_dummy_device(client->adapter,
> > > - TCPC_INTERFACE_ADDR >> 1);
> > > - if (IS_ERR(ctx->i2c.tcpc_client)) {
> > > - err = PTR_ERR(ctx->i2c.tcpc_client);
> > > - goto free_rx_p2;
> > > - }
> > > + ctx->i2c.tcpc_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + TCPC_INTERFACE_ADDR >> 1);
> > > + if (IS_ERR(ctx->i2c.tcpc_client))
> > > + return PTR_ERR(ctx->i2c.tcpc_client);
> > >
> > > return 0;
> > > -
> > > -free_rx_p2:
> > > - i2c_unregister_device(ctx->i2c.rx_p2_client);
> > > -free_rx_p1:
> > > - i2c_unregister_device(ctx->i2c.rx_p1_client);
> > > -free_rx_p0:
> > > - i2c_unregister_device(ctx->i2c.rx_p0_client);
> > > -free_tx_p2:
> > > - i2c_unregister_device(ctx->i2c.tx_p2_client);
> > > -free_tx_p1:
> > > - i2c_unregister_device(ctx->i2c.tx_p1_client);
> > > -free_tx_p0:
> > > - i2c_unregister_device(ctx->i2c.tx_p0_client);
> > > -
> > > - return err;
> > > -}
> > > -
> > > -static void anx7625_unregister_i2c_dummy_clients(struct anx7625_data *ctx)
> > > -{
> > > - i2c_unregister_device(ctx->i2c.tx_p0_client);
> > > - i2c_unregister_device(ctx->i2c.tx_p1_client);
> > > - i2c_unregister_device(ctx->i2c.tx_p2_client);
> > > - i2c_unregister_device(ctx->i2c.rx_p0_client);
> > > - i2c_unregister_device(ctx->i2c.rx_p1_client);
> > > - i2c_unregister_device(ctx->i2c.rx_p2_client);
> > > - i2c_unregister_device(ctx->i2c.tcpc_client);
> > > }
> > >
> > > static int __maybe_unused anx7625_runtime_pm_suspend(struct device *dev)
> > > @@ -2723,8 +2685,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> > > if (!platform->pdata.low_power_mode)
> > > pm_runtime_put_sync_suspend(&client->dev);
> > >
> > > - anx7625_unregister_i2c_dummy_clients(platform);
> > > -
> > > free_wq:
> > > if (platform->workqueue)
> > > destroy_workqueue(platform->workqueue);
> > > @@ -2754,8 +2714,6 @@ static int anx7625_i2c_remove(struct i2c_client *client)
> > > if (!platform->pdata.low_power_mode)
> > > pm_runtime_put_sync_suspend(&client->dev);
> > >
> > > - anx7625_unregister_i2c_dummy_clients(platform);
> > > -
> > > if (platform->pdata.audio_en)
> > > anx7625_unregister_audio(platform);
> > >
> > > --
> > > 2.37.0.rc0.161.g10f37bed90-goog
>
> Can you fix the checkpatch --strict warnings and formatting issues in
> this series?
Done. Fix them in v2.
Thanks.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] drm/bridge: anx7625: Convert to devm_i2c_new_dummy_device()
@ 2022-07-06 12:55 ` Hsin-Yi Wang
0 siblings, 0 replies; 22+ messages in thread
From: Hsin-Yi Wang @ 2022-07-06 12:55 UTC (permalink / raw)
To: Robert Foss
Cc: Jonas Karlman, David Airlie, Thomas Zimmermann, dri-devel,
Neil Armstrong, linux-kernel, Jernej Skrabec, Laurent Pinchart,
Andrzej Hajda, Sam Ravnborg, Xin Ji, Maxime Ripard
On Wed, Jul 6, 2022 at 8:31 PM Robert Foss <robert.foss@linaro.org> wrote:
>
> Hey Hsin-Yi,
>
> On Wed, 6 Jul 2022 at 04:29, Xin Ji <xji@analogixsemi.com> wrote:
> >
> > Hi Hsin-Yi, thanks for your patch, looks good to me.
> >
> > Reviewed-by: Xin Ji <xji@analogixsemi.com>
> >
> > On Thu, Jun 30, 2022 at 12:05:47AM +0800, Hsin-Yi Wang wrote:
> > > Simplify the resource management.
> > >
> > > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > > ---
> > > drivers/gpu/drm/bridge/analogix/anx7625.c | 96 +++++++----------------
> > > 1 file changed, 27 insertions(+), 69 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > index 3710fa9ee0acd..f89e8151475f7 100644
> > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> > > @@ -2436,82 +2436,44 @@ static const struct drm_bridge_funcs anx7625_bridge_funcs = {
> > > static int anx7625_register_i2c_dummy_clients(struct anx7625_data *ctx,
> > > struct i2c_client *client)
> > > {
> > > - int err = 0;
> > > + struct device *dev = &ctx->client->dev;
> > >
> > > - ctx->i2c.tx_p0_client = i2c_new_dummy_device(client->adapter,
> > > - TX_P0_ADDR >> 1);
> > > + ctx->i2c.tx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + TX_P0_ADDR >> 1);
> > > if (IS_ERR(ctx->i2c.tx_p0_client))
> > > return PTR_ERR(ctx->i2c.tx_p0_client);
> > >
> > > - ctx->i2c.tx_p1_client = i2c_new_dummy_device(client->adapter,
> > > - TX_P1_ADDR >> 1);
> > > - if (IS_ERR(ctx->i2c.tx_p1_client)) {
> > > - err = PTR_ERR(ctx->i2c.tx_p1_client);
> > > - goto free_tx_p0;
> > > - }
> > > + ctx->i2c.tx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + TX_P1_ADDR >> 1);
> > > + if (IS_ERR(ctx->i2c.tx_p1_client))
> > > + return PTR_ERR(ctx->i2c.tx_p1_client);
> > >
> > > - ctx->i2c.tx_p2_client = i2c_new_dummy_device(client->adapter,
> > > - TX_P2_ADDR >> 1);
> > > - if (IS_ERR(ctx->i2c.tx_p2_client)) {
> > > - err = PTR_ERR(ctx->i2c.tx_p2_client);
> > > - goto free_tx_p1;
> > > - }
> > > + ctx->i2c.tx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + TX_P2_ADDR >> 1);
> > > + if (IS_ERR(ctx->i2c.tx_p2_client))
> > > + return PTR_ERR(ctx->i2c.tx_p2_client);
> > >
> > > - ctx->i2c.rx_p0_client = i2c_new_dummy_device(client->adapter,
> > > - RX_P0_ADDR >> 1);
> > > - if (IS_ERR(ctx->i2c.rx_p0_client)) {
> > > - err = PTR_ERR(ctx->i2c.rx_p0_client);
> > > - goto free_tx_p2;
> > > - }
> > > + ctx->i2c.rx_p0_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + RX_P0_ADDR >> 1);
> > > + if (IS_ERR(ctx->i2c.rx_p0_client))
> > > + return PTR_ERR(ctx->i2c.rx_p0_client);
> > >
> > > - ctx->i2c.rx_p1_client = i2c_new_dummy_device(client->adapter,
> > > - RX_P1_ADDR >> 1);
> > > - if (IS_ERR(ctx->i2c.rx_p1_client)) {
> > > - err = PTR_ERR(ctx->i2c.rx_p1_client);
> > > - goto free_rx_p0;
> > > - }
> > > + ctx->i2c.rx_p1_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + RX_P1_ADDR >> 1);
> > > + if (IS_ERR(ctx->i2c.rx_p1_client))
> > > + return PTR_ERR(ctx->i2c.rx_p1_client);
> > >
> > > - ctx->i2c.rx_p2_client = i2c_new_dummy_device(client->adapter,
> > > - RX_P2_ADDR >> 1);
> > > - if (IS_ERR(ctx->i2c.rx_p2_client)) {
> > > - err = PTR_ERR(ctx->i2c.rx_p2_client);
> > > - goto free_rx_p1;
> > > - }
> > > + ctx->i2c.rx_p2_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + RX_P2_ADDR >> 1);
> > > + if (IS_ERR(ctx->i2c.rx_p2_client))
> > > + return PTR_ERR(ctx->i2c.rx_p2_client);
> > >
> > > - ctx->i2c.tcpc_client = i2c_new_dummy_device(client->adapter,
> > > - TCPC_INTERFACE_ADDR >> 1);
> > > - if (IS_ERR(ctx->i2c.tcpc_client)) {
> > > - err = PTR_ERR(ctx->i2c.tcpc_client);
> > > - goto free_rx_p2;
> > > - }
> > > + ctx->i2c.tcpc_client = devm_i2c_new_dummy_device(dev, client->adapter,
> > > + TCPC_INTERFACE_ADDR >> 1);
> > > + if (IS_ERR(ctx->i2c.tcpc_client))
> > > + return PTR_ERR(ctx->i2c.tcpc_client);
> > >
> > > return 0;
> > > -
> > > -free_rx_p2:
> > > - i2c_unregister_device(ctx->i2c.rx_p2_client);
> > > -free_rx_p1:
> > > - i2c_unregister_device(ctx->i2c.rx_p1_client);
> > > -free_rx_p0:
> > > - i2c_unregister_device(ctx->i2c.rx_p0_client);
> > > -free_tx_p2:
> > > - i2c_unregister_device(ctx->i2c.tx_p2_client);
> > > -free_tx_p1:
> > > - i2c_unregister_device(ctx->i2c.tx_p1_client);
> > > -free_tx_p0:
> > > - i2c_unregister_device(ctx->i2c.tx_p0_client);
> > > -
> > > - return err;
> > > -}
> > > -
> > > -static void anx7625_unregister_i2c_dummy_clients(struct anx7625_data *ctx)
> > > -{
> > > - i2c_unregister_device(ctx->i2c.tx_p0_client);
> > > - i2c_unregister_device(ctx->i2c.tx_p1_client);
> > > - i2c_unregister_device(ctx->i2c.tx_p2_client);
> > > - i2c_unregister_device(ctx->i2c.rx_p0_client);
> > > - i2c_unregister_device(ctx->i2c.rx_p1_client);
> > > - i2c_unregister_device(ctx->i2c.rx_p2_client);
> > > - i2c_unregister_device(ctx->i2c.tcpc_client);
> > > }
> > >
> > > static int __maybe_unused anx7625_runtime_pm_suspend(struct device *dev)
> > > @@ -2723,8 +2685,6 @@ static int anx7625_i2c_probe(struct i2c_client *client,
> > > if (!platform->pdata.low_power_mode)
> > > pm_runtime_put_sync_suspend(&client->dev);
> > >
> > > - anx7625_unregister_i2c_dummy_clients(platform);
> > > -
> > > free_wq:
> > > if (platform->workqueue)
> > > destroy_workqueue(platform->workqueue);
> > > @@ -2754,8 +2714,6 @@ static int anx7625_i2c_remove(struct i2c_client *client)
> > > if (!platform->pdata.low_power_mode)
> > > pm_runtime_put_sync_suspend(&client->dev);
> > >
> > > - anx7625_unregister_i2c_dummy_clients(platform);
> > > -
> > > if (platform->pdata.audio_en)
> > > anx7625_unregister_audio(platform);
> > >
> > > --
> > > 2.37.0.rc0.161.g10f37bed90-goog
>
> Can you fix the checkpatch --strict warnings and formatting issues in
> this series?
Done. Fix them in v2.
Thanks.
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2022-07-06 12:56 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-29 16:05 [PATCH 0/4] anx7625: Cleanup, fixes, and implement wait_hpd_asserted Hsin-Yi Wang
2022-06-29 16:05 ` Hsin-Yi Wang
2022-06-29 16:05 ` [PATCH 1/4] drm/bridge: anx7625: Convert to devm_i2c_new_dummy_device() Hsin-Yi Wang
2022-06-29 16:05 ` Hsin-Yi Wang
2022-07-06 2:29 ` Xin Ji
2022-07-06 2:29 ` Xin Ji
2022-07-06 12:31 ` Robert Foss
2022-07-06 12:31 ` Robert Foss
2022-07-06 12:55 ` Hsin-Yi Wang
2022-07-06 12:55 ` Hsin-Yi Wang
2022-06-29 16:05 ` [PATCH 2/4] drm/bridge: anx7625: Use pm_runtime_force_suspend(resume) Hsin-Yi Wang
2022-06-29 16:05 ` Hsin-Yi Wang
2022-07-06 2:30 ` Xin Ji
2022-07-06 2:30 ` Xin Ji
2022-06-29 16:05 ` [PATCH 3/4] drm/bridge: anx7625: Fix NULL pointer crash when using edp-panel Hsin-Yi Wang
2022-06-29 16:05 ` Hsin-Yi Wang
2022-07-06 2:31 ` Xin Ji
2022-07-06 2:31 ` Xin Ji
2022-06-29 16:05 ` [PATCH 4/4] drm/bridge: anx7625: Add wait_hpd_asserted() callback Hsin-Yi Wang
2022-06-29 16:05 ` Hsin-Yi Wang
2022-07-06 2:32 ` Xin Ji
2022-07-06 2:32 ` Xin Ji
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.