[PATCH v2] drm/bridge: anx7625: Use pm_runtime_force_{suspend, resume}
Use pm_runtime_force_{suspend,resume} as system suspend/resume hook, to ensure that anx7625 is always powered off on suspend. Also add a device link between anx7625 driver and the encoder, to ensure that bridge_disable will be called before suspend. Signed-off-by: Pi-Hsun Shih --- Changes from v1: * Use device link to ensure suspend resume order, instead of manually calling force resume. This is the second approach mentioned in v1 of this patch (https://lore.kernel.org/patchwork/patch/1459569/#1655836). An issue was found that the anx7625 driver won't power off when used as eDP bridge on Asurada board if suspend is entered via VT2. The reason is that in this case, anx7625_suspend won't power off anx7625 (since intp_irq is not set). And anx7625_bridge_disable is only called indirectly by other driver's (mediatek-drm) suspend. pm_runtime_put_sync won't do anything since it's already in system suspend. If not in VT2, the bridge disable is indirectly called when Chrome stops, so anx7625 will be powered off correctly. To fix the issue, the suspend resume hooks are changed to pm_runtime_force_{suspend,resume} to ensure the runtime suspend / resume is always called correctly when system suspend / resume. (Note that IRQ no longer needs to be disabled on suspend after commit f03ab6629c7b ("drm/bridge: anx7625: Make hpd workqueue freezable")) Also adds a stateless device link to ensure that the bridge disable is called before anx7625 is suspended. --- drivers/gpu/drm/bridge/analogix/anx7625.c | 54 +-- drivers/gpu/drm/bridge/analogix/anx7625.h | 1 + 2 files changed, 22 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 7519b7a0f29d..e248f0da2f8b 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1331,6 +1331,8 @@ static void anx7625_bridge_detach(struct drm_bridge *bridge) mipi_dsi_detach(ctx->dsi); mipi_dsi_device_unregister(ctx->dsi); } + if (ctx->link) + device_link_del(ctx->link); } static int anx7625_bridge_attach(struct drm_bridge *bridge, @@ -1355,6 +1357,13 @@ static int anx7625_bridge_attach(struct drm_bridge *bridge, return err; } + ctx->link = device_link_add(bridge->dev->dev, dev, DL_FLAG_STATELESS); + if (!ctx->link) { + DRM_DEV_ERROR(dev, "device link creation failed"); + err = -EINVAL; + goto detach_dsi; + } + if (ctx->pdata.panel_bridge) { err = drm_bridge_attach(bridge->encoder, ctx->pdata.panel_bridge, @@ -1362,13 +1371,22 @@ static int anx7625_bridge_attach(struct drm_bridge *bridge, if (err) { DRM_DEV_ERROR(dev, "Fail to attach panel bridge: %d\n", err); - return err; + goto remove_device_link; } } ctx->bridge_attached = 1; return 0; + +remove_device_link: + device_link_del(ctx->link); +detach_dsi: + if (ctx->dsi) { + mipi_dsi_detach(ctx->dsi); + mipi_dsi_device_unregister(ctx->dsi); + } + return err; } static enum drm_mode_status @@ -1705,39 +1723,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); - flush_workqueue(ctx->workqueue); - } - - 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) }; diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h index 034c3840028f..c941b7a32859 100644 --- a/drivers/gpu/drm/bridge/analo
[PATCH v2 1/2] drm/bridge: anx7625: refactor power control to use runtime PM framework
The driver originally use an atomic_t for keep track of the power status, which makes the driver more complicated than needed, and has some race condition as it's possible to have the power on and power off sequence going at the same time. This patch remove the usage of the atomic_t power_status, and use the kernel runtime power management framework instead. Signed-off-by: Pi-Hsun Shih --- drivers/gpu/drm/bridge/analogix/anx7625.c | 148 +- drivers/gpu/drm/bridge/analogix/anx7625.h | 1 - 2 files changed, 63 insertions(+), 86 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 23283ba0c4f9..f56f8cf1f3bd 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -1005,33 +1006,6 @@ static void anx7625_power_on_init(struct anx7625_data *ctx) } } -static void anx7625_chip_control(struct anx7625_data *ctx, int state) -{ - struct device *dev = &ctx->client->dev; - - DRM_DEV_DEBUG_DRIVER(dev, "before set, power_state(%d).\n", -atomic_read(&ctx->power_status)); - - if (!ctx->pdata.low_power_mode) - return; - - if (state) { - atomic_inc(&ctx->power_status); - if (atomic_read(&ctx->power_status) == 1) - anx7625_power_on_init(ctx); - } else { - if (atomic_read(&ctx->power_status)) { - atomic_dec(&ctx->power_status); - - if (atomic_read(&ctx->power_status) == 0) - anx7625_power_standby(ctx); - } - } - - DRM_DEV_DEBUG_DRIVER(dev, "after set, power_state(%d).\n", -atomic_read(&ctx->power_status)); -} - static void anx7625_init_gpio(struct anx7625_data *platform) { struct device *dev = &platform->client->dev; @@ -1061,9 +1035,6 @@ static void anx7625_stop_dp_work(struct anx7625_data *ctx) ctx->hpd_status = 0; ctx->hpd_high_cnt = 0; ctx->display_timing_valid = 0; - - if (ctx->pdata.low_power_mode == 0) - anx7625_disable_pd_protocol(ctx); } static void anx7625_start_dp_work(struct anx7625_data *ctx) @@ -1105,49 +1076,26 @@ static void anx7625_hpd_polling(struct anx7625_data *ctx) int ret, val; struct device *dev = &ctx->client->dev; - if (atomic_read(&ctx->power_status) != 1) { - DRM_DEV_DEBUG_DRIVER(dev, "No need to poling HPD status.\n"); - return; - } - ret = readx_poll_timeout(anx7625_read_hpd_status_p0, ctx, val, ((val & HPD_STATUS) || (val < 0)), 5000, 5000 * 100); if (ret) { - DRM_DEV_ERROR(dev, "HPD polling timeout!\n"); - } else { - DRM_DEV_DEBUG_DRIVER(dev, "HPD raise up.\n"); - anx7625_reg_write(ctx, ctx->i2c.tcpc_client, - INTR_ALERT_1, 0xFF); - anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, - INTERFACE_CHANGE_INT, 0); + DRM_DEV_ERROR(dev, "no hpd.\n"); + return; } - anx7625_start_dp_work(ctx); -} - -static void anx7625_disconnect_check(struct anx7625_data *ctx) -{ - if (atomic_read(&ctx->power_status) == 0) - anx7625_stop_dp_work(ctx); -} - -static void anx7625_low_power_mode_check(struct anx7625_data *ctx, -int state) -{ - struct device *dev = &ctx->client->dev; + DRM_DEV_DEBUG_DRIVER(dev, "system status: 0x%x. HPD raise up.\n", val); + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, + INTR_ALERT_1, 0xFF); + anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + INTERFACE_CHANGE_INT, 0); - DRM_DEV_DEBUG_DRIVER(dev, "low power mode check, state(%d).\n", state); + anx7625_start_dp_work(ctx); - if (ctx->pdata.low_power_mode) { - anx7625_chip_control(ctx, state); - if (state) - anx7625_hpd_polling(ctx); - else - anx7625_disconnect_check(ctx); - } + if (!ctx->pdata.panel_bridge && ctx->bridge_attached) + drm_helper_hpd_irq_event(ctx->bridge.dev); } static void anx7625_remove_edid(struct anx7625_data *ctx) @@ -1180,9 +1128,6 @@ static int anx7625_hpd_change_detect(struct anx7625_data *ctx)
[PATCH v2 2/2] drm/bridge: anx7625: add suspend / resume hooks
Add suspend / resume hooks for anx7625 driver, that power off the device on suspend and power on the device on resume if it was previously powered. Signed-off-by: Pi-Hsun Shih --- drivers/gpu/drm/bridge/analogix/anx7625.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index f56f8cf1f3bd..176d395c1a9f 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1705,7 +1705,34 @@ 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)) + 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); + + return 0; +} + static const struct dev_pm_ops anx7625_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(anx7625_suspend, anx7625_resume) SET_RUNTIME_PM_OPS(anx7625_runtime_pm_suspend, anx7625_runtime_pm_resume, NULL) }; -- 2.31.1.607.g51e8a6a459-goog
[PATCH 1/2] drm/bridge: anx7625: refactor power control to use runtime PM framework
The driver originally use an atomic_t for keep track of the power status, which makes the driver more complicated than needed, and has some race condition as it's possible to have the power on and power off sequence going at the same time. This patch remove the usage of the atomic_t power_status, and use the kernel runtime power management framework instead. Signed-off-by: Pi-Hsun Shih Change-Id: I58e19680b6d9ffb04be2a90f458400a1433925aa --- drivers/gpu/drm/bridge/analogix/anx7625.c | 147 +- drivers/gpu/drm/bridge/analogix/anx7625.h | 1 - 2 files changed, 62 insertions(+), 86 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 23283ba0c4f9..0d90cd63fc27 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -1005,33 +1006,6 @@ static void anx7625_power_on_init(struct anx7625_data *ctx) } } -static void anx7625_chip_control(struct anx7625_data *ctx, int state) -{ - struct device *dev = &ctx->client->dev; - - DRM_DEV_DEBUG_DRIVER(dev, "before set, power_state(%d).\n", -atomic_read(&ctx->power_status)); - - if (!ctx->pdata.low_power_mode) - return; - - if (state) { - atomic_inc(&ctx->power_status); - if (atomic_read(&ctx->power_status) == 1) - anx7625_power_on_init(ctx); - } else { - if (atomic_read(&ctx->power_status)) { - atomic_dec(&ctx->power_status); - - if (atomic_read(&ctx->power_status) == 0) - anx7625_power_standby(ctx); - } - } - - DRM_DEV_DEBUG_DRIVER(dev, "after set, power_state(%d).\n", -atomic_read(&ctx->power_status)); -} - static void anx7625_init_gpio(struct anx7625_data *platform) { struct device *dev = &platform->client->dev; @@ -1061,9 +1035,6 @@ static void anx7625_stop_dp_work(struct anx7625_data *ctx) ctx->hpd_status = 0; ctx->hpd_high_cnt = 0; ctx->display_timing_valid = 0; - - if (ctx->pdata.low_power_mode == 0) - anx7625_disable_pd_protocol(ctx); } static void anx7625_start_dp_work(struct anx7625_data *ctx) @@ -1105,49 +1076,26 @@ static void anx7625_hpd_polling(struct anx7625_data *ctx) int ret, val; struct device *dev = &ctx->client->dev; - if (atomic_read(&ctx->power_status) != 1) { - DRM_DEV_DEBUG_DRIVER(dev, "No need to poling HPD status.\n"); - return; - } - ret = readx_poll_timeout(anx7625_read_hpd_status_p0, ctx, val, ((val & HPD_STATUS) || (val < 0)), 5000, 5000 * 100); if (ret) { - DRM_DEV_ERROR(dev, "HPD polling timeout!\n"); - } else { - DRM_DEV_DEBUG_DRIVER(dev, "HPD raise up.\n"); - anx7625_reg_write(ctx, ctx->i2c.tcpc_client, - INTR_ALERT_1, 0xFF); - anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, - INTERFACE_CHANGE_INT, 0); + DRM_DEV_ERROR(dev, "no hpd.\n"); + return; } - anx7625_start_dp_work(ctx); -} - -static void anx7625_disconnect_check(struct anx7625_data *ctx) -{ - if (atomic_read(&ctx->power_status) == 0) - anx7625_stop_dp_work(ctx); -} - -static void anx7625_low_power_mode_check(struct anx7625_data *ctx, -int state) -{ - struct device *dev = &ctx->client->dev; + DRM_DEV_DEBUG_DRIVER(dev, "system status: 0x%x. HPD raise up.\n", val); + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, + INTR_ALERT_1, 0xFF); + anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + INTERFACE_CHANGE_INT, 0); - DRM_DEV_DEBUG_DRIVER(dev, "low power mode check, state(%d).\n", state); + anx7625_start_dp_work(ctx); - if (ctx->pdata.low_power_mode) { - anx7625_chip_control(ctx, state); - if (state) - anx7625_hpd_polling(ctx); - else - anx7625_disconnect_check(ctx); - } + if (!ctx->pdata.panel_bridge && ctx->bridge_attached) + drm_helper_hpd_irq_event(ctx->bridge.dev); } static void anx7625_remove_edid(struct anx7625_data *ctx) @@ -1180,9 +1128,6 @@ static int
[PATCH 2/2] drm/bridge: anx7625: add suspend / resume hooks
Add suspend / resume hooks for anx7625 driver, that power off the device on suspend and power on the device on resume if it was previously powered. Signed-off-by: Pi-Hsun Shih Change-Id: I62122cc2a4eafdfce4859cbb419edc39875ebe3b --- drivers/gpu/drm/bridge/analogix/anx7625.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 0d90cd63fc27..dd23db9bc3d4 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1705,7 +1705,34 @@ 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)) + 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); + + return 0; +} + static const struct dev_pm_ops anx7625_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(anx7625_suspend, anx7625_resume) \ SET_RUNTIME_PM_OPS(anx7625_runtime_pm_suspend, anx7625_runtime_pm_resume, NULL) }; -- 2.31.1.607.g51e8a6a459-goog
[PATCH v3 2/2] drm/bridge: anx7625: add suspend / resume hooks
Add suspend / resume hooks for anx7625 driver, that power off the device on suspend and power on the device on resume if it was previously powered. Signed-off-by: Pi-Hsun Shih --- Changes from v2: * No change. --- drivers/gpu/drm/bridge/analogix/anx7625.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index e1bf31eafe22..b165ef71e00f 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1705,7 +1705,34 @@ 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)) + 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); + + return 0; +} + static const struct dev_pm_ops anx7625_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(anx7625_suspend, anx7625_resume) SET_RUNTIME_PM_OPS(anx7625_runtime_pm_suspend, anx7625_runtime_pm_resume, NULL) }; -- 2.31.1.607.g51e8a6a459-goog
[PATCH v3 1/2] drm/bridge: anx7625: refactor power control to use runtime PM framework
The driver originally use an atomic_t for keep track of the power status, which makes the driver more complicated than needed, and has some race condition as it's possible to have the power on and power off sequence going at the same time. This patch remove the usage of the atomic_t power_status, and use the kernel runtime power management framework instead. Signed-off-by: Pi-Hsun Shih --- Changes from v2: * Add missing .pm field to anx7625_driver. --- drivers/gpu/drm/bridge/analogix/anx7625.c | 149 ++ drivers/gpu/drm/bridge/analogix/anx7625.h | 1 - 2 files changed, 64 insertions(+), 86 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 23283ba0c4f9..e1bf31eafe22 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -1005,33 +1006,6 @@ static void anx7625_power_on_init(struct anx7625_data *ctx) } } -static void anx7625_chip_control(struct anx7625_data *ctx, int state) -{ - struct device *dev = &ctx->client->dev; - - DRM_DEV_DEBUG_DRIVER(dev, "before set, power_state(%d).\n", -atomic_read(&ctx->power_status)); - - if (!ctx->pdata.low_power_mode) - return; - - if (state) { - atomic_inc(&ctx->power_status); - if (atomic_read(&ctx->power_status) == 1) - anx7625_power_on_init(ctx); - } else { - if (atomic_read(&ctx->power_status)) { - atomic_dec(&ctx->power_status); - - if (atomic_read(&ctx->power_status) == 0) - anx7625_power_standby(ctx); - } - } - - DRM_DEV_DEBUG_DRIVER(dev, "after set, power_state(%d).\n", -atomic_read(&ctx->power_status)); -} - static void anx7625_init_gpio(struct anx7625_data *platform) { struct device *dev = &platform->client->dev; @@ -1061,9 +1035,6 @@ static void anx7625_stop_dp_work(struct anx7625_data *ctx) ctx->hpd_status = 0; ctx->hpd_high_cnt = 0; ctx->display_timing_valid = 0; - - if (ctx->pdata.low_power_mode == 0) - anx7625_disable_pd_protocol(ctx); } static void anx7625_start_dp_work(struct anx7625_data *ctx) @@ -1105,49 +1076,26 @@ static void anx7625_hpd_polling(struct anx7625_data *ctx) int ret, val; struct device *dev = &ctx->client->dev; - if (atomic_read(&ctx->power_status) != 1) { - DRM_DEV_DEBUG_DRIVER(dev, "No need to poling HPD status.\n"); - return; - } - ret = readx_poll_timeout(anx7625_read_hpd_status_p0, ctx, val, ((val & HPD_STATUS) || (val < 0)), 5000, 5000 * 100); if (ret) { - DRM_DEV_ERROR(dev, "HPD polling timeout!\n"); - } else { - DRM_DEV_DEBUG_DRIVER(dev, "HPD raise up.\n"); - anx7625_reg_write(ctx, ctx->i2c.tcpc_client, - INTR_ALERT_1, 0xFF); - anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, - INTERFACE_CHANGE_INT, 0); + DRM_DEV_ERROR(dev, "no hpd.\n"); + return; } - anx7625_start_dp_work(ctx); -} - -static void anx7625_disconnect_check(struct anx7625_data *ctx) -{ - if (atomic_read(&ctx->power_status) == 0) - anx7625_stop_dp_work(ctx); -} - -static void anx7625_low_power_mode_check(struct anx7625_data *ctx, -int state) -{ - struct device *dev = &ctx->client->dev; + DRM_DEV_DEBUG_DRIVER(dev, "system status: 0x%x. HPD raise up.\n", val); + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, + INTR_ALERT_1, 0xFF); + anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + INTERFACE_CHANGE_INT, 0); - DRM_DEV_DEBUG_DRIVER(dev, "low power mode check, state(%d).\n", state); + anx7625_start_dp_work(ctx); - if (ctx->pdata.low_power_mode) { - anx7625_chip_control(ctx, state); - if (state) - anx7625_hpd_polling(ctx); - else - anx7625_disconnect_check(ctx); - } + if (!ctx->pdata.panel_bridge && ctx->bridge_attached) + drm_helper_hpd_irq_event(ctx->bridge.dev); } static void anx7625_remove_edid(struct anx7625_data *ctx) @@ -1180,9 +112
[PATCH v4 1/2] drm/bridge: anx7625: refactor power control to use runtime PM framework
The driver originally use an atomic_t for keep track of the power status, which makes the driver more complicated than needed, and has some race condition as it's possible to have the power on and power off sequence going at the same time. This patch remove the usage of the atomic_t power_status, and use the kernel runtime power management framework instead. Signed-off-by: Pi-Hsun Shih --- Changes from v3: * Maintain separate powered state, since the power state is not sync with runtime PM suspended state on suspend / resume, and the irq callback shouldn't be run while suspending. Changes from v2: * Add missing .pm field to anx7625_driver. --- drivers/gpu/drm/bridge/analogix/anx7625.c | 151 ++ drivers/gpu/drm/bridge/analogix/anx7625.h | 2 +- 2 files changed, 67 insertions(+), 86 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 23283ba0c4f9..b0a53d6a2343 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -1005,33 +1006,6 @@ static void anx7625_power_on_init(struct anx7625_data *ctx) } } -static void anx7625_chip_control(struct anx7625_data *ctx, int state) -{ - struct device *dev = &ctx->client->dev; - - DRM_DEV_DEBUG_DRIVER(dev, "before set, power_state(%d).\n", -atomic_read(&ctx->power_status)); - - if (!ctx->pdata.low_power_mode) - return; - - if (state) { - atomic_inc(&ctx->power_status); - if (atomic_read(&ctx->power_status) == 1) - anx7625_power_on_init(ctx); - } else { - if (atomic_read(&ctx->power_status)) { - atomic_dec(&ctx->power_status); - - if (atomic_read(&ctx->power_status) == 0) - anx7625_power_standby(ctx); - } - } - - DRM_DEV_DEBUG_DRIVER(dev, "after set, power_state(%d).\n", -atomic_read(&ctx->power_status)); -} - static void anx7625_init_gpio(struct anx7625_data *platform) { struct device *dev = &platform->client->dev; @@ -1061,9 +1035,6 @@ static void anx7625_stop_dp_work(struct anx7625_data *ctx) ctx->hpd_status = 0; ctx->hpd_high_cnt = 0; ctx->display_timing_valid = 0; - - if (ctx->pdata.low_power_mode == 0) - anx7625_disable_pd_protocol(ctx); } static void anx7625_start_dp_work(struct anx7625_data *ctx) @@ -1105,49 +1076,26 @@ static void anx7625_hpd_polling(struct anx7625_data *ctx) int ret, val; struct device *dev = &ctx->client->dev; - if (atomic_read(&ctx->power_status) != 1) { - DRM_DEV_DEBUG_DRIVER(dev, "No need to poling HPD status.\n"); - return; - } - ret = readx_poll_timeout(anx7625_read_hpd_status_p0, ctx, val, ((val & HPD_STATUS) || (val < 0)), 5000, 5000 * 100); if (ret) { - DRM_DEV_ERROR(dev, "HPD polling timeout!\n"); - } else { - DRM_DEV_DEBUG_DRIVER(dev, "HPD raise up.\n"); - anx7625_reg_write(ctx, ctx->i2c.tcpc_client, - INTR_ALERT_1, 0xFF); - anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, - INTERFACE_CHANGE_INT, 0); + DRM_DEV_ERROR(dev, "no hpd.\n"); + return; } - anx7625_start_dp_work(ctx); -} - -static void anx7625_disconnect_check(struct anx7625_data *ctx) -{ - if (atomic_read(&ctx->power_status) == 0) - anx7625_stop_dp_work(ctx); -} - -static void anx7625_low_power_mode_check(struct anx7625_data *ctx, -int state) -{ - struct device *dev = &ctx->client->dev; + DRM_DEV_DEBUG_DRIVER(dev, "system status: 0x%x. HPD raise up.\n", val); + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, + INTR_ALERT_1, 0xFF); + anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + INTERFACE_CHANGE_INT, 0); - DRM_DEV_DEBUG_DRIVER(dev, "low power mode check, state(%d).\n", state); + anx7625_start_dp_work(ctx); - if (ctx->pdata.low_power_mode) { - anx7625_chip_control(ctx, state); - if (state) - anx7625_hpd_polling(ctx); - else - anx7625_disconnect_check(ctx); - } + if (!
[PATCH v4 2/2] drm/bridge: anx7625: add suspend / resume hooks
Add suspend / resume hooks for anx7625 driver, that power off the device on suspend and power on the device on resume if it was previously powered. Signed-off-by: Pi-Hsun Shih --- Changes from v2, v3: * No change. --- drivers/gpu/drm/bridge/analogix/anx7625.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index b0a53d6a2343..4f5537c9b3d4 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1707,7 +1707,34 @@ 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)) + 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); + + return 0; +} + static const struct dev_pm_ops anx7625_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(anx7625_suspend, anx7625_resume) SET_RUNTIME_PM_OPS(anx7625_runtime_pm_suspend, anx7625_runtime_pm_resume, NULL) }; -- 2.31.1.751.gd2f1c929bd-goog
[PATCH v5 2/2] drm/bridge: anx7625: add suspend / resume hooks
Add suspend / resume hooks for anx7625 driver, that power off the device on suspend and power on the device on resume if it was previously powered. Signed-off-by: Pi-Hsun Shih --- Changes in v5: * Disable irq and flush workqueue in suspend hook, so the irq handler won't be run during suspend. Changes in v3, v4: * No change. --- drivers/gpu/drm/bridge/analogix/anx7625.c | 32 +++ 1 file changed, 32 insertions(+) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index e1bf31eafe22..8fb76ca66e5b 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1705,7 +1705,39 @@ 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); + flush_workqueue(ctx->workqueue); + } + + return 0; +} + static const struct dev_pm_ops anx7625_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(anx7625_suspend, anx7625_resume) SET_RUNTIME_PM_OPS(anx7625_runtime_pm_suspend, anx7625_runtime_pm_resume, NULL) }; -- 2.31.1.751.gd2f1c929bd-goog
[PATCH v5 1/2] drm/bridge: anx7625: refactor power control to use runtime PM framework
The driver originally use an atomic_t for keep track of the power status, which makes the driver more complicated than needed, and has some race condition as it's possible to have the power on and power off sequence going at the same time. This patch remove the usage of the atomic_t power_status, and use the kernel runtime power management framework instead. Signed-off-by: Pi-Hsun Shih --- Changes in v5: * Revert changes from v4, and move fix to the suspend / resume hook (2/2 in series). Changes in v4: * Maintain separate powered state, since the power state is not sync with runtime PM suspended state on suspend / resume, and the irq callback shouldn't be run while suspending. Changes in v3: * Add missing .pm field to anx7625_driver. --- drivers/gpu/drm/bridge/analogix/anx7625.c | 149 ++ drivers/gpu/drm/bridge/analogix/anx7625.h | 1 - 2 files changed, 64 insertions(+), 86 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 23283ba0c4f9..e1bf31eafe22 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -1005,33 +1006,6 @@ static void anx7625_power_on_init(struct anx7625_data *ctx) } } -static void anx7625_chip_control(struct anx7625_data *ctx, int state) -{ - struct device *dev = &ctx->client->dev; - - DRM_DEV_DEBUG_DRIVER(dev, "before set, power_state(%d).\n", -atomic_read(&ctx->power_status)); - - if (!ctx->pdata.low_power_mode) - return; - - if (state) { - atomic_inc(&ctx->power_status); - if (atomic_read(&ctx->power_status) == 1) - anx7625_power_on_init(ctx); - } else { - if (atomic_read(&ctx->power_status)) { - atomic_dec(&ctx->power_status); - - if (atomic_read(&ctx->power_status) == 0) - anx7625_power_standby(ctx); - } - } - - DRM_DEV_DEBUG_DRIVER(dev, "after set, power_state(%d).\n", -atomic_read(&ctx->power_status)); -} - static void anx7625_init_gpio(struct anx7625_data *platform) { struct device *dev = &platform->client->dev; @@ -1061,9 +1035,6 @@ static void anx7625_stop_dp_work(struct anx7625_data *ctx) ctx->hpd_status = 0; ctx->hpd_high_cnt = 0; ctx->display_timing_valid = 0; - - if (ctx->pdata.low_power_mode == 0) - anx7625_disable_pd_protocol(ctx); } static void anx7625_start_dp_work(struct anx7625_data *ctx) @@ -1105,49 +1076,26 @@ static void anx7625_hpd_polling(struct anx7625_data *ctx) int ret, val; struct device *dev = &ctx->client->dev; - if (atomic_read(&ctx->power_status) != 1) { - DRM_DEV_DEBUG_DRIVER(dev, "No need to poling HPD status.\n"); - return; - } - ret = readx_poll_timeout(anx7625_read_hpd_status_p0, ctx, val, ((val & HPD_STATUS) || (val < 0)), 5000, 5000 * 100); if (ret) { - DRM_DEV_ERROR(dev, "HPD polling timeout!\n"); - } else { - DRM_DEV_DEBUG_DRIVER(dev, "HPD raise up.\n"); - anx7625_reg_write(ctx, ctx->i2c.tcpc_client, - INTR_ALERT_1, 0xFF); - anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, - INTERFACE_CHANGE_INT, 0); + DRM_DEV_ERROR(dev, "no hpd.\n"); + return; } - anx7625_start_dp_work(ctx); -} - -static void anx7625_disconnect_check(struct anx7625_data *ctx) -{ - if (atomic_read(&ctx->power_status) == 0) - anx7625_stop_dp_work(ctx); -} - -static void anx7625_low_power_mode_check(struct anx7625_data *ctx, -int state) -{ - struct device *dev = &ctx->client->dev; + DRM_DEV_DEBUG_DRIVER(dev, "system status: 0x%x. HPD raise up.\n", val); + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, + INTR_ALERT_1, 0xFF); + anx7625_reg_write(ctx, ctx->i2c.rx_p0_client, + INTERFACE_CHANGE_INT, 0); - DRM_DEV_DEBUG_DRIVER(dev, "low power mode check, state(%d).\n", state); + anx7625_start_dp_work(ctx); - if (ctx->pdata.low_power_mode) { - anx7625_chip_control(ctx, state); - if (state) - anx7625_hpd_polling(
[PATCH] drm/bridge: anx7625: Synchronously run runtime suspend.
Originally when using pm_runtime_put, there's a chance that the runtime suspend hook will be run after the following anx7625_bridge_mode_set call, resulting in the display_timing_valid field to be cleared, and the following power on fail. Change all pm_runtime_put to pm_runtime_put_sync, so all power off operations are guaranteed to be done after the call returns. Fixes: 60487584a79a ("drm/bridge: anx7625: refactor power control to use runtime PM framework") Signed-off-by: Pi-Hsun Shih --- drivers/gpu/drm/bridge/analogix/anx7625.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 29493cc2d300..7519b7a0f29d 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1255,7 +1255,7 @@ static struct edid *anx7625_get_edid(struct anx7625_data *ctx) pm_runtime_get_sync(dev); edid_num = sp_tx_edid_read(ctx, p_edid->edid_raw_data); - pm_runtime_put(dev); + pm_runtime_put_sync(dev); if (edid_num < 1) { DRM_DEV_ERROR(dev, "Fail to read EDID: %d\n", edid_num); @@ -1573,7 +1573,7 @@ static void anx7625_bridge_disable(struct drm_bridge *bridge) anx7625_dp_stop(ctx); - pm_runtime_put(dev); + pm_runtime_put_sync(dev); } static enum drm_connector_status base-commit: 7a42b92b6d30c3f09629c7d5ada9e3de2aba01af -- 2.31.1.751.gd2f1c929bd-goog
[PATCH] drm/bridge: anx7625: Use pm_runtime_force_{suspend,resume}
Use pm_runtime_force_suspend and pm_runtime_force_resume to ensure that anx7625 would always be powered off when suspended. Also update the bridge enable hook to always ensure that the anx7625 is powered on before starting DP operations. Fixes: 409776fa3c42 ("drm/bridge: anx7625: add suspend / resume hooks") Signed-off-by: Pi-Hsun Shih --- An issue was found that the anx7625 driver won't power off when used as eDP bridge on Asurada board if suspend is entered via VT2. The reason is that in this case, anx7625_suspend won't power off anx7625 (since intp_irq is not set). And anx7625_bridge_disable is only called indirectly by other driver's (mediatek-drm) suspend. pm_runtime_put_sync won't do anything since it's already in system suspend. If not in VT2, the bridge disable is indirectly called when Chrome stops, so anx7625 will be powered off correctly. To fix the issue, the suspend resume hooks are changed to pm_runtime_force_{suspend,resume} to ensure the runtime suspend / resume is always called correctly when system suspend / resume. (Note that IRQ no longer needs to be disabled on suspend after commit f03ab6629c7b ("drm/bridge: anx7625: Make hpd workqueue freezable")) Since bridge disable is called indirectly by mediatek-drm driver's suspend, it might happens after anx7625 suspend is called. So a check if the driver is already suspended via pm_runtime_force_suspend is also added, to ensure that the anx7625_dp_stop won't be called when power is off. And also since bridge enable might happens before anx7625 resume is called, a check to that is also added, and would force resume the device in this case. I'm not sure if the approach to fix this is the most appropriate way, since using pm_runtime_force_resume in bridge enable kinda feels hacky to me. I'm open to any suggestions. --- drivers/gpu/drm/bridge/analogix/anx7625.c | 55 +-- 1 file changed, 20 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index a3d82377066b..9d0f5dc88b16 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1559,7 +1559,20 @@ static void anx7625_bridge_enable(struct drm_bridge *bridge) DRM_DEV_DEBUG_DRIVER(dev, "drm enable\n"); - pm_runtime_get_sync(dev); + /* +* The only case where pm_runtime is disabled here is when the function +* is called other driver's resume hook by +* drm_mode_config_helper_resume, but when the pm_runtime_force_resume +* hasn't been called on this device. +* +* pm_runtime_get_sync won't power on anx7625 in this case since we're +* in system resume, so instead we force resume anx7625 to make sure +* the following anx7625_dp_start would succeed. +*/ + if (pm_runtime_enabled(dev)) + pm_runtime_get_sync(dev); + else + pm_runtime_force_resume(dev); anx7625_dp_start(ctx); } @@ -1571,9 +1584,10 @@ static void anx7625_bridge_disable(struct drm_bridge *bridge) DRM_DEV_DEBUG_DRIVER(dev, "drm disable\n"); - anx7625_dp_stop(ctx); - - pm_runtime_put_sync(dev); + if (pm_runtime_enabled(dev)) { + anx7625_dp_stop(ctx); + pm_runtime_put_sync(dev); + } } static enum drm_connector_status @@ -1705,38 +1719,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) }; base-commit: c0d438dbc0b74901f1901d97a6c84f38daa0c831 -- 2.32.0.93.g670b81a890-goog
Re: [PATCH] drm/bridge: anx7625: Use pm_runtime_force_{suspend, resume}
) On Wed, Jul 14, 2021 at 6:32 PM Daniel Vetter wrote: > > On Wed, Jul 14, 2021 at 02:01:59PM +0800, Pi-Hsun Shih wrote: > > Use pm_runtime_force_suspend and pm_runtime_force_resume to ensure that > > anx7625 would always be powered off when suspended. Also update the > > bridge enable hook to always ensure that the anx7625 is powered on > > before starting DP operations. > > > > Fixes: 409776fa3c42 ("drm/bridge: anx7625: add suspend / resume hooks") > > > > Signed-off-by: Pi-Hsun Shih > > > > --- > > > > An issue was found that the anx7625 driver won't power off when used as > > eDP bridge on Asurada board if suspend is entered via VT2. > > > > The reason is that in this case, anx7625_suspend won't power off anx7625 > > (since intp_irq is not set). And anx7625_bridge_disable is only called > > indirectly by other driver's (mediatek-drm) suspend. > > pm_runtime_put_sync won't do anything since it's already in system > > suspend. > > > > If not in VT2, the bridge disable is indirectly called when Chrome > > stops, so anx7625 will be powered off correctly. > > > > To fix the issue, the suspend resume hooks are changed to > > pm_runtime_force_{suspend,resume} to ensure the runtime suspend / resume > > is always called correctly when system suspend / resume. > > (Note that IRQ no longer needs to be disabled on suspend after commit > > f03ab6629c7b ("drm/bridge: anx7625: Make hpd workqueue freezable")) > > > > Since bridge disable is called indirectly by mediatek-drm driver's > > suspend, it might happens after anx7625 suspend is called. So a check > > if the driver is already suspended via pm_runtime_force_suspend is also > > added, to ensure that the anx7625_dp_stop won't be called when power > > is off. And also since bridge enable might happens before anx7625 resume > > is called, a check to that is also added, and would force resume the > > device in this case. > > > > I'm not sure if the approach to fix this is the most appropriate way, > > since using pm_runtime_force_resume in bridge enable kinda feels hacky > > to me. I'm open to any suggestions. > > I thought the real fix was to create device links between the bridge and > the other parts of the overall drm driver, so that the driver core can > resume devices in the right order. > > Unfortunately those device link patches haven't made it in yet. Quick > search on lore didn't find anything, maybe I was just dreaming, or maybe > the patches only existed for panels. > > Either way, this is a drm_bridge.c problem that needs to be fixed there, > not individually in each driver. > -Daniel Hi, Thanks for the response, I did find some discussion about this in 2018 for drm_panel (https://patchwork.kernel.org/project/dri-devel/patch/b53584fd988d045c13de22d81825395b0ae0aad7.1524727888.git.jsa...@ti.com/), which also mentioned drm_bridge. >From that thread it seems that linking all bridges with the previous one would break some drivers, and there was no conclusion on how this should be done. I have some ideas on how to solve this issue for the anx7625 driver without affecting other drivers, are patches that do one of the following acceptable? * Add some opt-in flag to drm_bridge which, if set, would create a stateless device link between the bridge and the encoder in drm_bridge_attach. And use the flag in anx7625 driver. * Add the stateless device link in the anx7625 driver inside anx7625_bridge_attach (We can remove the link if a general solution for drm_bridge comes out later). Or is it still preferred to have some general solution in drm_bridge without explicit opt-in? Regards, Pi-Hsun > > > > > --- > > drivers/gpu/drm/bridge/analogix/anx7625.c | 55 +-- > > 1 file changed, 20 insertions(+), 35 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > > b/drivers/gpu/drm/bridge/analogix/anx7625.c > > index a3d82377066b..9d0f5dc88b16 100644 > > --- a/drivers/gpu/drm/bridge/analogix/anx7625.c > > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > > @@ -1559,7 +1559,20 @@ static void anx7625_bridge_enable(struct drm_bridge > > *bridge) > > > > DRM_DEV_DEBUG_DRIVER(dev, "drm enable\n"); > > > > - pm_runtime_get_sync(dev); > > + /* > > + * The only case where pm_runtime is disabled here is when the > > function > > + * is called other driver's resume hook by > > + * drm_mode_config_helper_resume, but when the pm_runtime_force_resume > > + * hasn't been called on th
[PATCH] drm/bridge: anx7625: Add anx7625 port switching.
When output 2 lanes DP data, anx7625 can output to either TX1/RX1 or TX2/RX2. In typical usage, these two TX/RX pairs corresponds to two orientations of typec. On some board one anx7625 is used as DPI to DP converter for two typec ports. In this case, the TX1/RX1 and TX2/RX2 are connected to two usb muxes, which mux the DP data with the rest of the USB3 data, and connects to the two typec ports. This patch adds option for anx7625 to acts as a usb typec switch and switch output lanes based on the typec orientation, or acts as two usb typec mux and switch output lanes depending on whether the two ports currently has DP enabled. Signed-off-by: Pi-Hsun Shih This is an attempt to use typec framework with how we're using anx7625 on Chrome OS asurada board. An example of the dts for the two ports case can be found at https://crrev.com/c/2507199/6 Sending this as a RFC patch since I'm not sure about the best approach here. Should the logic of switching output lanes depends on ports be coupled inside anx7625 driver, or in another driver, or is there any existing way to accomplish this? --- drivers/gpu/drm/bridge/analogix/anx7625.c | 135 ++ drivers/gpu/drm/bridge/analogix/anx7625.h | 24 2 files changed, 159 insertions(+) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 65cc05982f82..75f35a197196 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -13,6 +13,9 @@ #include #include #include +#include +#include +#include #include #include @@ -1224,6 +1227,122 @@ static irqreturn_t anx7625_intr_hpd_isr(int irq, void *data) return IRQ_HANDLED; } +static void anx7625_set_crosspoint_switch(struct anx7625_data *ctx, + enum typec_orientation orientation) +{ + if (orientation == TYPEC_ORIENTATION_NORMAL) { + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0, + SW_SEL1_SSRX_B10_B11 | SW_SEL1_ML0_A10_A11); + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1, + SW_SEL2_SSTX_A2_A3 | SW_SEL2_ML1_B2_B3); + } else if (orientation == TYPEC_ORIENTATION_REVERSE) { + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0, + SW_SEL1_SSRX_A10_A11 | SW_SEL1_ML0_B10_B11); + anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1, + SW_SEL2_SSTX_B2_B3 | SW_SEL2_ML1_A2_A3); + } +} + +static int anx7625_usb_set_orientation(struct typec_switch *sw, + enum typec_orientation orientation) +{ + struct anx7625_data *ctx = typec_switch_get_drvdata(sw); + + anx7625_set_crosspoint_switch(ctx, orientation); + return 0; +} + +static int anx7625_register_usb(struct device *device, + struct anx7625_data *ctx) +{ + struct typec_switch_desc sw_desc = { }; + struct fwnode_handle *fwnode = of_fwnode_handle(device->of_node); + + sw_desc.fwnode = fwnode; + sw_desc.drvdata = ctx; + sw_desc.name = fwnode_get_name(fwnode); + sw_desc.set = anx7625_usb_set_orientation; + + ctx->typec_sw = typec_switch_register(device, &sw_desc); + if (IS_ERR(ctx->typec_sw)) + return PTR_ERR(ctx->typec_sw); + + return 0; +} + +static void anx7625_usb_two_ports_update(struct anx7625_data *ctx) +{ + if (ctx->typec_ports[0].has_dp && ctx->typec_ports[1].has_dp) + // Both ports available, do nothing to retain the current one. + return; + else if (ctx->typec_ports[0].has_dp) + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_NORMAL); + else if (ctx->typec_ports[1].has_dp) + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_REVERSE); +} + +static int anx7625_usb_mux_set(struct typec_mux *mux, + struct typec_mux_state *state) +{ + struct anx7625_port_data *data = typec_mux_get_drvdata(mux); + + if (state->alt && state->alt->svid == USB_TYPEC_DP_SID && + state->alt->mode == USB_TYPEC_DP_MODE) + data->has_dp = true; + else + data->has_dp = false; + + anx7625_usb_two_ports_update(data->ctx); + return 0; +} + +static int anx7625_register_usb_two_ports(struct device *device, + struct anx7625_data *ctx) +{ + struct typec_mux_desc mux_desc = { }; + struct fwnode_handle *fwnode; + struct anx7625_port_data *port_data; + u32 port_num; + int ret; + + device_for_each_child_node(device, fwnode) { +
Re: [PATCH] drm/bridge: anx7625: Add anx7625 port switching.
Hi Prashant, Please see inline reply as below. On Thu, Nov 12, 2020 at 4:59 PM Prashant Malani wrote: > > Hi Pi-Hsun, > > I haven't gone through the code, but did have a high-level comment > (kindly see inline) > > On Thu, Nov 12, 2020 at 02:40:40PM +0800, Pi-Hsun Shih wrote: > > When output 2 lanes DP data, anx7625 can output to either TX1/RX1 or > > TX2/RX2. In typical usage, these two TX/RX pairs corresponds to two > > orientations of typec. > > > > On some board one anx7625 is used as DPI to DP converter for two typec > > ports. In this case, the TX1/RX1 and TX2/RX2 are connected to two usb > > muxes, which mux the DP data with the rest of the USB3 data, and > > connects to the two typec ports. > > > > This patch adds option for anx7625 to acts as a usb typec switch and > > switch output lanes based on the typec orientation, or acts as two usb > > typec mux and switch output lanes depending on whether the two ports > > currently has DP enabled. > > > > Signed-off-by: Pi-Hsun Shih > > > > > > > > This is an attempt to use typec framework with how we're using anx7625 > > on Chrome OS asurada board. > > > > An example of the dts for the two ports case can be found at > > https://crrev.com/c/2507199/6 > > Do you plan on submitting DT schemas & bindings documentation for the > switch(es) > that are intended to be used? Yes I plan to submit corresponding DT schemas & bindings documentation changes if this change looks good. > > I would strongly recommend that for usb-c-connector since AFAIK they don't > exist, and > I don't believe there is explicit support for them in the Type C connector > class framework > (even . > > IMO this would be needed to ensure an implementation here doesn't break > in the event of modifications to the connector class framework (or Type > C port drivers like cros-ec-typec) in the future. I think some patches > were floated for this for orientation switch [1] so those might provide > some hints about how to proceed. > > I've CC-ed Heikki (Type C maintainer) in case he has additional comments > regarding this. > > > > > Sending this as a RFC patch since I'm not sure about the best approach > > here. Should the logic of switching output lanes depends on ports be > > coupled inside anx7625 driver, or in another driver, or is there > > any existing way to accomplish this? > > Might be good to add [RFC] as a tag instead of [PATCH] in case this > iteration is chiefly to solicit comments. Ah I did have [RFC] tag in some local .patch files before. I guess I somehow forgot it in later `git format-patch` runs... I'll add the tag in the next version, thanks for the comments. > > Best regards, > > -Prashant > > [1]: > https://lore.kernel.org/linux-usb/1604403610-16577-1-git-send-email-jun...@nxp.com/ > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver
Hi, On Tue, Oct 15, 2019 at 5:52 PM Xin Ji wrote: > > The ANX7625 is an ultra-low power 4K Mobile HD Transmitter designed > for portable device. It converts MIPI DSI/DPI to DisplayPort 1.3 4K. > > The ANX7625 can support both USB Type-C PD feature and MIPI DSI/DPI > to DP feature. This driver only enabled MIPI DSI/DPI to DP feature. > > Signed-off-by: Xin Ji > --- > drivers/gpu/drm/bridge/Makefile |2 +- > drivers/gpu/drm/bridge/analogix/Kconfig |6 + > drivers/gpu/drm/bridge/analogix/Makefile |1 + > drivers/gpu/drm/bridge/analogix/anx7625.c | 2043 > + > drivers/gpu/drm/bridge/analogix/anx7625.h | 406 ++ > 5 files changed, 2457 insertions(+), 1 deletion(-) > create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.c > create mode 100644 drivers/gpu/drm/bridge/analogix/anx7625.h > > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 4934fcf..bcd388a 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -12,8 +12,8 @@ obj-$(CONFIG_DRM_SII9234) += sii9234.o > obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o > obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o > obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o > -obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ > obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o > obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o > +obj-y += analogix/ > obj-y += synopsys/ > diff --git a/drivers/gpu/drm/bridge/analogix/Kconfig > b/drivers/gpu/drm/bridge/analogix/Kconfig > index e930ff9..b2f127e 100644 > --- a/drivers/gpu/drm/bridge/analogix/Kconfig > +++ b/drivers/gpu/drm/bridge/analogix/Kconfig > @@ -2,3 +2,9 @@ > config DRM_ANALOGIX_DP > tristate > depends on DRM > + > +config ANALOGIX_ANX7625 > + tristate "Analogix MIPI to DP interface support" > + help > + ANX7625 is an ultra-low power 4K mobile HD transmitter > designed > + for portable devices. It converts MIPI/DPI to DisplayPort1.3 > 4K. > diff --git a/drivers/gpu/drm/bridge/analogix/Makefile > b/drivers/gpu/drm/bridge/analogix/Makefile > index fdbf3fd..8a52867 100644 > --- a/drivers/gpu/drm/bridge/analogix/Makefile > +++ b/drivers/gpu/drm/bridge/analogix/Makefile > @@ -1,3 +1,4 @@ > # SPDX-License-Identifier: GPL-2.0-only > +obj-$(CONFIG_ANALOGIX_ANX7625) += anx7625.o > analogix_dp-objs := analogix_dp_core.o analogix_dp_reg.o > obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix_dp.o > diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c > b/drivers/gpu/drm/bridge/analogix/anx7625.c > new file mode 100644 > index 000..456b95c > --- /dev/null > +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c > @@ -0,0 +1,2043 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright(c) 2016, Analogix Semiconductor. All rights reserved. > + * > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#include "anx7625.h" > + > +/* > + * there is a sync issue while access I2C register between AP(CPU) and > + * internal firmware(OCM), to avoid the race condition, AP should access > + * the reserved slave address before slave address occurs changes. > + */ > +static int i2c_access_workaround(struct anx7625_data *ctx, > +struct i2c_client *client) > +{ > + u8 offset; > + struct device *dev = &client->dev; > + int ret; > + > + if (client == ctx->last_client) > + return 0; > + > + ctx->last_client = client; > + > + if (client == ctx->i2c.tcpc_client) > + offset = RSVD_00_ADDR; > + else if (client == ctx->i2c.tx_p0_client) > + offset = RSVD_D1_ADDR; > + else if (client == ctx->i2c.tx_p1_client) > + offset = RSVD_60_ADDR; > + else if (client == ctx->i2c.rx_p0_client) > + offset = RSVD_39_ADDR; > + else if (client == ctx->i2c.rx_p1_client) > + offset = RSVD_7F_ADDR; > + else > + offset = RSVD_00_ADDR; > + > + ret = i2c_smbus_write_byte_data(client, offset, 0x00); > + if (ret < 0) > + DRM_DEV_ERROR(dev, > + "failed to access i2c id=%x\n:%x", > + client->addr, offset); > + > + return ret; > +} > + > +static int anx7625_reg_read(struct anx7625_data *ctx, > + struct i2c_client *client, u8 reg_addr) > +{ > + int ret; > + struct device *dev = &client->dev; > + > + i2c_access_workaround(ctx, client); > + > + ret = i2c_smbus_read_byte_data(client, reg_addr); > + if (ret < 0) > +
[PATCH] drm/mediatek: Check return value of mtk_drm_ddp_comp_for_plane.
The mtk_drm_ddp_comp_for_plane can return NULL, but the usage doesn't check for it. Add check for it. Fixes: d6b53f68356f ("drm/mediatek: Add helper to get component for a plane") Signed-off-by: Pi-Hsun Shih --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index f80a8ba75977..4c4f976c994e 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -310,7 +310,9 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc) plane_state = to_mtk_plane_state(plane->state); comp = mtk_drm_ddp_comp_for_plane(crtc, plane, &local_layer); - mtk_ddp_comp_layer_config(comp, local_layer, plane_state); + if (comp) + mtk_ddp_comp_layer_config(comp, local_layer, + plane_state); } return 0; @@ -386,8 +388,9 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc) comp = mtk_drm_ddp_comp_for_plane(crtc, plane, &local_layer); - mtk_ddp_comp_layer_config(comp, local_layer, - plane_state); + if (comp) + mtk_ddp_comp_layer_config(comp, local_layer, + plane_state); plane_state->pending.config = false; } mtk_crtc->pending_planes = false; @@ -401,7 +404,9 @@ int mtk_drm_crtc_plane_check(struct drm_crtc *crtc, struct drm_plane *plane, struct mtk_ddp_comp *comp; comp = mtk_drm_ddp_comp_for_plane(crtc, plane, &local_layer); - return mtk_ddp_comp_layer_check(comp, local_layer, state); + if (comp) + return mtk_ddp_comp_layer_check(comp, local_layer, state); + return 0; } static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc, base-commit: 5a6fcbeabe3e20459ed8504690b2515dacc5246f -- 2.24.0.432.g9d3f5f5b63-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 3/3] drm/bridge: add it6505 driver
Hi allen, On Mon, Mar 9, 2020 at 2:32 PM allen wrote: > > From: Allen Chen > > This adds support for the iTE IT6505. > This device can convert DPI signal to DP output. > > Signed-off-by: Jitao Shi > Signed-off-by: Yilun Lin > Signed-off-by: Allen Chen > Signed-off-by: Pi-Hsun Shih > --- > drivers/gpu/drm/bridge/Kconfig | 11 +- > drivers/gpu/drm/bridge/Makefile |6 +- > drivers/gpu/drm/bridge/ite-it6505.c | 3022 > +++ > 3 files changed, 3035 insertions(+), 4 deletions(-) > create mode 100644 drivers/gpu/drm/bridge/ite-it6505.c > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index aaed234..ff81681 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -38,8 +38,15 @@ config DRM_DISPLAY_CONNECTOR > on ARM-based platforms. Saying Y here when this driver is not needed > will not cause any issue. > > -config DRM_LVDS_CODEC > - tristate "Transparent LVDS encoders and decoders support" > +config DRM_ITE_IT6505 > + tristate "ITE IT6505 DP bridge" > + depends on OF > + select DRM_KMS_HELPER > + help > + ITE IT6505 DP bridge chip driver. > + > +config DRM_LVDS_ENCODER > + tristate "Transparent parallel to LVDS encoder support" > depends on OF > select DRM_KMS_HELPER > select DRM_PANEL_BRIDGE > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 6fb062b..e6c80ab 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -1,7 +1,9 @@ > # SPDX-License-Identifier: GPL-2.0 > obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o > -obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o > -obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o > +obj-$(CONFIG_DRM_DUMB_VGA_DAC) += dumb-vga-dac.o > +obj-$(CONFIG_DRM_GENERIC_GPIO_MUX) += generic-gpio-mux.o > +obj-$(CONFIG_DRM_ITE_IT6505) += ite-it6505.o > +obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o > obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) += > megachips-stdp-ge-b850v3-fw.o > obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o > obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o There are unrelated changes to it6505 in the Makefile and Kconfig, please remove them. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/mediatek: Apply CMDQ control flow
Hi Bibby, On Fri, Aug 30, 2019 at 3:41 PM Bibby Hsieh wrote: > ... > +static void ddp_cmdq_cb(struct cmdq_cb_data data) > +{ > + > +#if IS_ENABLED(CONFIG_MTK_CMDQ) > + struct mtk_cmdq_cb_data *cb_data = data.data; > + struct drm_crtc_state *crtc_state = cb_data->state; > + struct drm_atomic_state *atomic_state = crtc_state->state; > + struct drm_crtc *crtc = crtc_state->crtc; > + struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > + > + DRM_DEBUG_DRIVER("%s\n", __func__); This debug message is printed about twice per second when enabled, which makes debugging other things that also use DRM_DEBUG_DRIVER harder. Can this be rate-limited or removed? > + > + if (mtk_crtc->pending_needs_vblank) { > + /* cmdq_vblank_event must be read after cmdq_needs_event */ > + smp_rmb(); > + > ... > +void mtk_drm_crtc_plane_update(struct drm_crtc *crtc, struct drm_plane > *plane, > + struct mtk_plane_state *state) > +{ > + struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > + struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[0]; > + struct drm_crtc_state *crtc_state = crtc->state; > + struct mtk_crtc_state *mtk_crtc_state = to_mtk_crtc_state(crtc_state); > + struct cmdq_pkt *cmdq_handle = mtk_crtc_state->cmdq_handle; > + unsigned int comp_layer_nr = mtk_ddp_comp_layer_nr(comp); > + unsigned int local_layer; > + unsigned int plane_index = plane - mtk_crtc->planes; > + > + DRM_DEBUG_DRIVER("%s\n", __func__); Same with this one. > + if (mtk_crtc->cmdq_client) { > + if (plane_index >= comp_layer_nr) { > + comp = mtk_crtc->ddp_comp[1]; > + local_layer = plane_index - comp_layer_nr; > ... > @@ -494,13 +599,29 @@ static void mtk_drm_crtc_atomic_flush(struct drm_crtc > *crtc, > struct drm_crtc_state *old_crtc_state) > { > struct drm_atomic_state *old_atomic_state = old_crtc_state->state; > + struct drm_crtc_state *crtc_state = crtc->state; > + struct mtk_crtc_state *state = to_mtk_crtc_state(crtc_state); > + struct cmdq_pkt *cmdq_handle = state->cmdq_handle; > struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); > struct mtk_drm_private *priv = crtc->dev->dev_private; > + struct mtk_cmdq_cb_data *cb_data; > unsigned int pending_planes = 0; > int i; > > - if (mtk_crtc->event) > - mtk_crtc->pending_needs_vblank = true; > + DRM_DEBUG_DRIVER("[CRTC:%u] [STATE:%p(%p)->%p(%p)]\n", crtc->base.id, > +old_crtc_state, old_crtc_state->state, > +crtc_state, crtc_state->state); Same with this one. > + > + if (IS_ENABLED(CONFIG_MTK_CMDQ) && mtk_crtc->cmdq_client) { > + drm_atomic_state_get(old_atomic_state); > + cb_data = kmalloc(sizeof(*cb_data), GFP_KERNEL); > + cb_data->state = old_crtc_state; > ...
[PATCH] drm/mediatek: Fix can't get component for external display plane.
From: Yongqiang Niu The original logic is ok for primary display, but will not find out component for external display. For example, plane->index is 6 for external display, but there are only 2 layer nr in external display, and this condition will never happen: if (plane->index < (count + mtk_ddp_comp_layer_nr(comp))) Fix this by using the offset of the plane to mtk_crtc->planes as index, instead of plane->index. Fixes: d6b53f68356f ("drm/mediatek: Add helper to get component for a plane") Signed-off-by: Yongqiang Niu Signed-off-by: Pi-Hsun Shih --- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c index f80a8ba75977..b34e7d70702a 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c @@ -215,11 +215,12 @@ struct mtk_ddp_comp *mtk_drm_ddp_comp_for_plane(struct drm_crtc *crtc, struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc); struct mtk_ddp_comp *comp; int i, count = 0; + unsigned int local_index = plane - mtk_crtc->planes; for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) { comp = mtk_crtc->ddp_comp[i]; - if (plane->index < (count + mtk_ddp_comp_layer_nr(comp))) { - *local_layer = plane->index - count; + if (local_index < (count + mtk_ddp_comp_layer_nr(comp))) { + *local_layer = local_index - count; return comp; } count += mtk_ddp_comp_layer_nr(comp); base-commit: 1875ff320f14afe21731a6e4c7b46dd33e45dfaa -- 2.24.0.393.g34dc348eaf-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/bridge: anx7625: Make hpd workqueue freezable
There were still a race condition between hpd work and suspend, since the workqueue work can still be run after anx7625 had powered off in suspend. Since we never want hpd work to run while suspending, and there's no harm to delay them to be run after resume, mark the workqueue as WQ_FREEZABLE so all works won't run while suspending. Fixes: 409776fa3c42 ("drm/bridge: anx7625: add suspend / resume hooks") Signed-off-by: Pi-Hsun Shih --- drivers/gpu/drm/bridge/analogix/anx7625.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index 7519b7a0f29d..e165be5a2067 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1730,7 +1730,6 @@ static int __maybe_unused anx7625_suspend(struct device *dev) if (!pm_runtime_enabled(dev) || !pm_runtime_suspended(dev)) { anx7625_runtime_pm_suspend(dev); disable_irq(ctx->pdata.intp_irq); - flush_workqueue(ctx->workqueue); } return 0; @@ -1790,7 +1789,8 @@ static int anx7625_i2c_probe(struct i2c_client *client, platform->pdata.intp_irq = client->irq; if (platform->pdata.intp_irq) { INIT_WORK(&platform->work, anx7625_work_func); - platform->workqueue = create_workqueue("anx7625_work"); + platform->workqueue = alloc_workqueue( + "anx7625_work", WQ_FREEZABLE | WQ_MEM_RECLAIM, 1); if (!platform->workqueue) { DRM_DEV_ERROR(dev, "fail to create work queue\n"); ret = -ENOMEM; base-commit: 25fe90f43fa312213b653dc1f12fd2d80f855883 -- 2.32.0.272.g935e593368-goog
Re: [PATCH v3] drm/bridge: add it6505 driver
On Fri, Sep 4, 2020 at 10:17 AM allen wrote: > > This adds support for the iTE IT6505. > This device can convert DPI signal to DP output. > > From: Allen Chen > Signed-off-by: Jitao Shi > Signed-off-by: Pi-Hsun Shih > Signed-off-by: Yilun Lin > Signed-off-by: Hermes Wu > Signed-off-by: Allen Chen > Reported-by: kernel test robot > --- > drivers/gpu/drm/bridge/Kconfig |7 + > drivers/gpu/drm/bridge/Makefile |1 + > drivers/gpu/drm/bridge/ite-it6505.c | 3338 +++ > 3 files changed, 3346 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/ite-it6505.c > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index 3e11af4e9f63e..f21dce3fabeb9 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -61,6 +61,13 @@ config DRM_LONTIUM_LT9611 > HDMI signals > Please say Y if you have such hardware. > > +config DRM_ITE_IT6505 > + tristate "ITE IT6505 DisplayPort bridge" > + depends on OF > + select DRM_KMS_HELPER > + help > + ITE IT6505 DisplayPort bridge chip driver. > + > config DRM_LVDS_CODEC > tristate "Transparent LVDS encoders and decoders support" > depends on OF > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index c589a6a7cbe1d..8a118fd901ad7 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -3,6 +3,7 @@ obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o > obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o > obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o > obj-$(CONFIG_DRM_LONTIUM_LT9611) += lontium-lt9611.o > +obj-$(CONFIG_DRM_ITE_IT6505) += ite-it6505.o > obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o > obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) += > megachips-stdp-ge-b850v3-fw.o > obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c > b/drivers/gpu/drm/bridge/ite-it6505.c > new file mode 100644 > index 0..0ed19673431ee > --- /dev/null > +++ b/drivers/gpu/drm/bridge/ite-it6505.c > ... > + > +static void __maybe_unused it6505_delayed_audio(struct work_struct *work) > +{ > + struct it6505 *it6505 = container_of(work, struct it6505, > +delayed_audio.work); > + > + DRM_DEV_DEBUG_DRIVER(&it6505->client->dev, "start"); > + > + if (!it6505->powered) > + return; > + > + if (!it6505->enable_drv_hold) > + it6505_enable_audio(it6505); > +} > + > +static int __maybe_unused it6505_audio_setup_hw_params(struct it6505 *it6505, > + struct hdmi_codec_params *params) > +{ > + struct device *dev = &it6505->client->dev; > + int i = 0; > + > + DRM_DEV_DEBUG_DRIVER(dev, "%s %d Hz, %d bit, %d channels\n", __func__, > +params->sample_rate, params->sample_width, > +params->cea.channels); > + > + if (!it6505->bridge.encoder) > + return -ENODEV; > + > + if (params->cea.channels <= 1 || params->cea.channels > 8) { > + DRM_DEV_DEBUG_DRIVER(dev, "channel number: %d not support", > +it6505->audio.channel_count); > + return -EINVAL; > + } > + > + it6505->audio.channel_count = params->cea.channels; > + > + while (i < ARRAY_SIZE(audio_sample_rate_map) && > + params->sample_rate != > + audio_sample_rate_map[i].sample_rate_value) { > + i++; > + } > + if (i == ARRAY_SIZE(audio_sample_rate_map)) { > + DRM_DEV_DEBUG_DRIVER(dev, "sample rate: %d Hz not support", > +params->sample_rate); > + return -EINVAL; > + } > + it6505->audio.sample_rate = audio_sample_rate_map[i].rate; > + > + switch (params->sample_width) { > + case 16: > + it6505->audio.word_length = WORD_LENGTH_16BIT; > + break; > + case 18: > + it6505->audio.word_length = WORD_LENGTH_18BIT; > + break; > + case 20: > + it6505->audio.word_length = WORD_LENGTH_20BIT; > + break; > + case 24: > + case 32: > + it6505->audio.word_length = WORD_LENGTH_24BIT; > + break