Hi,
This patchset includes cleanups, improvements, and bug fixes for Rockchip DRM driver and PSR support.
This new version is the same as before removing some of the patches already applied and fixing the Exynos issue due patch '[v4 15/38] drm/bridge: analogix_dp: Ensure edp is disabled when shutting down the panel' reported by Marek.
Regards, Enric
Changes in v5: - Removed the following patches as are already applied. [PATCH v4 01/38] drm/bridge: analogix_dp: set psr activate/deactivate when enable/disable bridge [PATCH v4 02/38] drm/rockchip: Don't use atomic constructs for psr - Add Mareks tested-tag and including the missing people. - [PATCH v4 15/38] move analogix_dp_set_analog_power_down() before phy_power_off() to fix Exynos issue.
Changes in v4: - Rebased all on top of drm-misc-next - Removed the following patches as are already applied. [PATCH v3 01/43] drm/rockchip: Get rid of unnecessary struct fields [PATCH v3 02/43] drm/rockchip: support prime import sg table [PATCH v3 03/43] drm/rockchip: Respect page offset for PRIME mmap calls - Removed the following patches as now are part of another patchset [PATCH v3 05/43] drm/bridge: analogix_dp: Don't power bridge in analogix_dp_bind [PATCH v3 33/43] drm/panel: simple: Change mode for Sharp lq123p1jx31
Changes in v3: - Addressed some of the comments from Sean on the v2
Changes in v2: - A few patches have been replaced by newer and cleaner versions from the ChromeOS kernel gerrit, especially about disallowing PSR for the whole atomic commit.
Douglas Anderson (4): drm/bridge: analogix_dp: Reorder plat_data->power_off to happen sooner drm/bridge: analogix_dp: Properly log AUX CH errors drm/bridge: analogix_dp: Properly disable aux chan retries on rockchip drm/bridge: analogix_dp: Split the platform-specific poweron in two parts
Kristian H. Kristensen (1): drm/rockchip: Disable PSR on input events
Lin Huang (6): drm/bridge: analogix_dp: Move enable video into config_video() drm/bridge: analogix_dp: Check AUX_EN status when doing AUX transfer drm/bridge: analogix_dp: Ensure edp is disabled when shutting down the panel drm/bridge: analogix_dp: Extend hpd check time to 100ms drm/bridge: analogix_dp: Check dpcd write/read status drm/bridge: analogix_dp: Reset aux channel if an error occurred
Mark Yao (1): drm/rockchip: pre dither down when output bpc is 8bit
Sean Paul (1): drm/rockchip: Remove analogix psr worker
Tomasz Figa (7): drm/rockchip: analogix_dp: Do not call Analogix code before bind drm/rockchip: Cancel PSR enable work before changing the state drm/rockchip: psr: Avoid redundant calls to .set() callback drm/rockchip: psr: Sanitize semantics of allow/disallow API drm/rockchip: Disable PSR from reboot notifier drm/rockchip: Disallow PSR for the whole atomic commit drm/rockchip: psr: Remove flush by CRTC
Yakir Yang (1): drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR
zain wang (15): drm/bridge: analogix_dp: Don't change psr while bridge is disabled drm/rockchip: add mutex vop lock drm/bridge: analogix_dp: add fast link train for eDP drm/rockchip: Only wait for panel ACK on PSR entry drm/bridge: analogix_dp: Don't use fast link training when panel just powered up drm/bridge: analogix_dp: Retry bridge enable when it failed drm/bridge: analogix_dp: Wait for HPD signal before configuring link drm/bridge: analogix_dp: Set PD_INC_BG first when powering up edp phy drm/bridge: analogix_dp: Fix incorrect usage of enhanced mode drm/bridge: analogix_dp: Fix AUX_PD bit for Rockchip drm/rockchip: Restore psr->state when enable/disable psr failed drm/bridge: analogix_dp: Don't use ANALOGIX_DP_PLL_CTL to control pll drm/bridge: analogix_dp: Fix timeout of video streamclk config drm/bridge: analogix_dp: Fix incorrect operations with register ANALOGIX_DP_FUNC_EN_1 drm/bridge: analogix_dp: Move fast link training detect to set_bridge
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 452 ++++++++++++++++----- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 14 +- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 274 ++++++++----- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 7 + drivers/gpu/drm/exynos/exynos_dp.c | 2 +- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 70 ++-- drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 61 ++- drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 309 +++++++++----- drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 7 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 43 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 + drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 + include/drm/bridge/analogix_dp.h | 5 +- 14 files changed, 879 insertions(+), 368 deletions(-)
From: Yakir Yang ykk@rock-chips.com
Make sure the request PSR state takes effect in analogix_dp_send_psr_spd() function, or print the sink PSR error state if we failed to apply the requested PSR setting.
Cc: 征增 王 wzz@rock-chips.com Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Yakir Yang ykk@rock-chips.com [seanpaul changed timeout loop to a readx poll] Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 ++-- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 6 ++-- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 35 +++++++++++++++++++--- 3 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index a693ab3078f0..e738aa6de1af 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -122,8 +122,7 @@ int analogix_dp_enable_psr(struct analogix_dp_device *dp) psr_vsc.DB0 = 0; psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
- analogix_dp_send_psr_spd(dp, &psr_vsc); - return 0; + return analogix_dp_send_psr_spd(dp, &psr_vsc); } EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
@@ -149,8 +148,7 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp) if (ret != 1) dev_err(dp->dev, "Failed to set DP Power0 %d\n", ret);
- analogix_dp_send_psr_spd(dp, &psr_vsc); - return 0; + return analogix_dp_send_psr_spd(dp, &psr_vsc); } EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index 5c6a28806129..b039b28d8fcc 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -20,6 +20,8 @@ #define MAX_CR_LOOP 5 #define MAX_EQ_LOOP 5
+#define DP_TIMEOUT_PSR_LOOP_MS 300 + /* DP_MAX_LANE_COUNT */ #define DPCD_ENHANCED_FRAME_CAP(x) (((x) >> 7) & 0x1) #define DPCD_MAX_LANE_COUNT(x) ((x) & 0x1f) @@ -247,8 +249,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp); void analogix_dp_enable_scrambling(struct analogix_dp_device *dp); void analogix_dp_disable_scrambling(struct analogix_dp_device *dp); void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp); -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp, - struct edp_vsc_psr *vsc); +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp, + struct edp_vsc_psr *vsc); ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, struct drm_dp_aux_msg *msg);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 303083ad28e3..005a3f7005d2 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -10,10 +10,11 @@ * option) any later version. */
-#include <linux/device.h> -#include <linux/io.h> #include <linux/delay.h> +#include <linux/device.h> #include <linux/gpio.h> +#include <linux/io.h> +#include <linux/iopoll.h>
#include <drm/bridge/analogix_dp.h>
@@ -992,10 +993,25 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp) writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON); }
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp, - struct edp_vsc_psr *vsc) +static ssize_t analogix_dp_get_psr_status(struct analogix_dp_device *dp) +{ + ssize_t val; + u8 status; + + val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &status); + if (val < 0) { + dev_err(dp->dev, "PSR_STATUS read failed ret=%zd", val); + return val; + } + return status; +} + +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp, + struct edp_vsc_psr *vsc) { unsigned int val; + int ret; + ssize_t psr_status;
/* don't send info frame */ val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL); @@ -1036,6 +1052,17 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp, val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL); val |= IF_EN; writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL); + + ret = readx_poll_timeout(analogix_dp_get_psr_status, dp, psr_status, + psr_status >= 0 && + ((vsc->DB1 && psr_status == DP_PSR_SINK_ACTIVE_RFB) || + (!vsc->DB1 && psr_status == DP_PSR_SINK_INACTIVE)), 1500, + DP_TIMEOUT_PSR_LOOP_MS * 1000); + if (ret) { + dev_warn(dp->dev, "Failed to apply PSR %d\n", ret); + return ret; + } + return 0; }
ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
Am Freitag, 9. März 2018, 23:22:52 CET schrieb Enric Balletbo i Serra:
From: Yakir Yang ykk@rock-chips.com
Make sure the request PSR state takes effect in analogix_dp_send_psr_spd() function, or print the sink PSR error state if we failed to apply the requested PSR setting.
Cc: 征增 王 wzz@rock-chips.com Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Yakir Yang ykk@rock-chips.com [seanpaul changed timeout loop to a readx poll] Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
Reviewed-by: Heiko Stuebner heiko@sntech.de
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
From: Yakir Yang ykk@rock-chips.com
Make sure the request PSR state takes effect in analogix_dp_send_psr_spd() function, or print the sink PSR error state if we failed to apply the requested PSR setting.
Reviewed-by: Archit Taneja architt@codeaurora.org
Cc: 征增 王 wzz@rock-chips.com Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Yakir Yang ykk@rock-chips.com [seanpaul changed timeout loop to a readx poll] Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 ++-- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 6 ++-- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 35 +++++++++++++++++++--- 3 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index a693ab3078f0..e738aa6de1af 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -122,8 +122,7 @@ int analogix_dp_enable_psr(struct analogix_dp_device *dp) psr_vsc.DB0 = 0; psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
- analogix_dp_send_psr_spd(dp, &psr_vsc);
- return 0;
- return analogix_dp_send_psr_spd(dp, &psr_vsc); } EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
@@ -149,8 +148,7 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp) if (ret != 1) dev_err(dp->dev, "Failed to set DP Power0 %d\n", ret);
- analogix_dp_send_psr_spd(dp, &psr_vsc);
- return 0;
- return analogix_dp_send_psr_spd(dp, &psr_vsc); } EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index 5c6a28806129..b039b28d8fcc 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -20,6 +20,8 @@ #define MAX_CR_LOOP 5 #define MAX_EQ_LOOP 5
+#define DP_TIMEOUT_PSR_LOOP_MS 300
- /* DP_MAX_LANE_COUNT */ #define DPCD_ENHANCED_FRAME_CAP(x) (((x) >> 7) & 0x1) #define DPCD_MAX_LANE_COUNT(x) ((x) & 0x1f)
@@ -247,8 +249,8 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp); void analogix_dp_enable_scrambling(struct analogix_dp_device *dp); void analogix_dp_disable_scrambling(struct analogix_dp_device *dp); void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp); -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
struct edp_vsc_psr *vsc);+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
struct edp_vsc_psr *vsc);diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 303083ad28e3..005a3f7005d2 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -10,10 +10,11 @@
- option) any later version.
*/
-#include <linux/device.h> -#include <linux/io.h> #include <linux/delay.h> +#include <linux/device.h> #include <linux/gpio.h> +#include <linux/io.h> +#include <linux/iopoll.h>
#include <drm/bridge/analogix_dp.h>
@@ -992,10 +993,25 @@ void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp) writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON); }
-void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
struct edp_vsc_psr *vsc)+static ssize_t analogix_dp_get_psr_status(struct analogix_dp_device *dp) +{
- ssize_t val;
- u8 status;
- val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &status);
- if (val < 0) {
dev_err(dp->dev, "PSR_STATUS read failed ret=%zd", val);return val;- }
- return status;
+}
+int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
struct edp_vsc_psr *vsc){ unsigned int val;
int ret;
ssize_t psr_status;
/* don't send info frame */ val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
@@ -1036,6 +1052,17 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp, val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL); val |= IF_EN; writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
ret = readx_poll_timeout(analogix_dp_get_psr_status, dp, psr_status,
psr_status >= 0 &&((vsc->DB1 && psr_status == DP_PSR_SINK_ACTIVE_RFB) ||(!vsc->DB1 && psr_status == DP_PSR_SINK_INACTIVE)), 1500,DP_TIMEOUT_PSR_LOOP_MS * 1000);if (ret) {
dev_warn(dp->dev, "Failed to apply PSR %d\n", ret);return ret;}
return 0; }
ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
Am Freitag, 9. März 2018, 23:22:52 CET schrieb Enric Balletbo i Serra:
From: Yakir Yang ykk@rock-chips.com
Make sure the request PSR state takes effect in analogix_dp_send_psr_spd() function, or print the sink PSR error state if we failed to apply the requested PSR setting.
Cc: 征增 王 wzz@rock-chips.com Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Yakir Yang ykk@rock-chips.com [seanpaul changed timeout loop to a readx poll] Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
applied to drm-misc-next
Thanks Heiko
From: Sean Paul seanpaul@chromium.org
Now that the spinlocks and timers are gone, we can remove the psr worker located in rockchip's analogix driver and do the enable/disable directly. This should simplify the code and remove races on disable.
Cc: 征增 王 wzz@rock-chips.com Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 31 ++----------------------- 1 file changed, 2 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 7d76ff47028d..36334839a3f8 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -71,10 +71,6 @@ struct rockchip_dp_device { struct regmap *grf; struct reset_control *rst;
- struct work_struct psr_work; - struct mutex psr_lock; - unsigned int psr_state; - const struct rockchip_dp_chip_data *data;
struct analogix_dp_device *adp; @@ -84,28 +80,13 @@ struct rockchip_dp_device { static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled) { struct rockchip_dp_device *dp = to_dp(encoder); + int ret;
if (!analogix_dp_psr_supported(dp->adp)) return;
DRM_DEV_DEBUG(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit");
- mutex_lock(&dp->psr_lock); - if (enabled) - dp->psr_state = EDP_VSC_PSR_STATE_ACTIVE; - else - dp->psr_state = ~EDP_VSC_PSR_STATE_ACTIVE; - - schedule_work(&dp->psr_work); - mutex_unlock(&dp->psr_lock); -} - -static void analogix_dp_psr_work(struct work_struct *work) -{ - struct rockchip_dp_device *dp = - container_of(work, typeof(*dp), psr_work); - int ret; - ret = rockchip_drm_wait_vact_end(dp->encoder.crtc, PSR_WAIT_LINE_FLAG_TIMEOUT_MS); if (ret) { @@ -113,12 +94,10 @@ static void analogix_dp_psr_work(struct work_struct *work) return; }
- mutex_lock(&dp->psr_lock); - if (dp->psr_state == EDP_VSC_PSR_STATE_ACTIVE) + if (enabled) analogix_dp_enable_psr(dp->adp); else analogix_dp_disable_psr(dp->adp); - mutex_unlock(&dp->psr_lock); }
static int rockchip_dp_pre_init(struct rockchip_dp_device *dp) @@ -135,8 +114,6 @@ static int rockchip_dp_poweron(struct analogix_dp_plat_data *plat_data) struct rockchip_dp_device *dp = to_dp(plat_data); int ret;
- cancel_work_sync(&dp->psr_work); - ret = clk_prepare_enable(dp->pclk); if (ret < 0) { DRM_DEV_ERROR(dp->dev, "failed to enable pclk %d\n", ret); @@ -355,10 +332,6 @@ static int rockchip_dp_bind(struct device *dev, struct device *master, dp->plat_data.power_off = rockchip_dp_powerdown; dp->plat_data.get_modes = rockchip_dp_get_modes;
- mutex_init(&dp->psr_lock); - dp->psr_state = ~EDP_VSC_PSR_STATE_ACTIVE; - INIT_WORK(&dp->psr_work, analogix_dp_psr_work); - ret = rockchip_drm_psr_register(&dp->encoder, analogix_dp_psr_set); if (ret < 0) goto err_cleanup_encoder;
Am Freitag, 9. März 2018, 23:22:53 CET schrieb Enric Balletbo i Serra:
From: Sean Paul seanpaul@chromium.org
Now that the spinlocks and timers are gone, we can remove the psr worker located in rockchip's analogix driver and do the enable/disable directly. This should simplify the code and remove races on disable.
Cc: 征增 王 wzz@rock-chips.com Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
Reviewed-by: Heiko Stuebner heiko@sntech.de
Am Freitag, 9. März 2018, 23:22:53 CET schrieb Enric Balletbo i Serra:
From: Sean Paul seanpaul@chromium.org
Now that the spinlocks and timers are gone, we can remove the psr worker located in rockchip's analogix driver and do the enable/disable directly. This should simplify the code and remove races on disable.
Cc: 征增 王 wzz@rock-chips.com Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
applied to drm-misc-next
Thanks Heiko
From: zain wang wzz@rock-chips.com
There is a race between AUX CH bring-up and enabling bridge which will cause link training to fail. To avoid hitting it, don't change psr state while enabling the bridge.
Cc: Tomeu Vizoso tomeu.vizoso@collabora.com Cc: Sean Paul seanpaul@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Caesar Wang wxt@rock-chips.com [seanpaul fixed up the commit message a bit and renamed *_supported to *_enabled] Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 15 ++++++++------- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 +- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 2 +- include/drm/bridge/analogix_dp.h | 2 +- 4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index e738aa6de1af..ee00d3d920e0 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -98,18 +98,18 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp) return 0; }
-int analogix_dp_psr_supported(struct analogix_dp_device *dp) +int analogix_dp_psr_enabled(struct analogix_dp_device *dp) {
- return dp->psr_support; + return dp->psr_enable; } -EXPORT_SYMBOL_GPL(analogix_dp_psr_supported); +EXPORT_SYMBOL_GPL(analogix_dp_psr_enabled);
int analogix_dp_enable_psr(struct analogix_dp_device *dp) { struct edp_vsc_psr psr_vsc;
- if (!dp->psr_support) + if (!dp->psr_enable) return 0;
/* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ @@ -131,7 +131,7 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp) struct edp_vsc_psr psr_vsc; int ret;
- if (!dp->psr_support) + if (!dp->psr_enable) return 0;
/* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ @@ -871,8 +871,8 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) /* Enable video */ analogix_dp_start_video(dp);
- dp->psr_support = analogix_dp_detect_sink_psr(dp); - if (dp->psr_support) + dp->psr_enable = analogix_dp_detect_sink_psr(dp); + if (dp->psr_enable) analogix_dp_enable_sink_psr(dp); }
@@ -1117,6 +1117,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge) if (ret) DRM_ERROR("failed to setup the panel ret = %d\n", ret);
+ dp->psr_enable = false; dp->dpms_mode = DRM_MODE_DPMS_OFF; }
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index b039b28d8fcc..e135a42cb19e 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -170,7 +170,7 @@ struct analogix_dp_device { int dpms_mode; int hpd_gpio; bool force_hpd; - bool psr_support; + bool psr_enable;
struct mutex panel_lock; bool panel_is_modeset; diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 36334839a3f8..3e8bf79bea58 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -82,7 +82,7 @@ static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled) struct rockchip_dp_device *dp = to_dp(encoder); int ret;
- if (!analogix_dp_psr_supported(dp->adp)) + if (!analogix_dp_psr_enabled(dp->adp)) return;
DRM_DEV_DEBUG(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit"); diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h index 711fff9b6803..e9a1116d2f8e 100644 --- a/include/drm/bridge/analogix_dp.h +++ b/include/drm/bridge/analogix_dp.h @@ -41,7 +41,7 @@ struct analogix_dp_plat_data { struct drm_connector *); };
-int analogix_dp_psr_supported(struct analogix_dp_device *dp); +int analogix_dp_psr_enabled(struct analogix_dp_device *dp); int analogix_dp_enable_psr(struct analogix_dp_device *dp); int analogix_dp_disable_psr(struct analogix_dp_device *dp);
Am Freitag, 9. März 2018, 23:22:54 CET schrieb Enric Balletbo i Serra:
From: zain wang wzz@rock-chips.com
There is a race between AUX CH bring-up and enabling bridge which will cause link training to fail. To avoid hitting it, don't change psr state while enabling the bridge.
Cc: Tomeu Vizoso tomeu.vizoso@collabora.com Cc: Sean Paul seanpaul@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Caesar Wang wxt@rock-chips.com [seanpaul fixed up the commit message a bit and renamed *_supported to *_enabled] Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
Reviewed-by: Heiko Stuebner heiko@sntech.de
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
There is a race between AUX CH bring-up and enabling bridge which will cause link training to fail. To avoid hitting it, don't change psr state while enabling the bridge.
Reviewed-by: Archit Taneja architt@codeaurora.org
Cc: Tomeu Vizoso tomeu.vizoso@collabora.com Cc: Sean Paul seanpaul@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Caesar Wang wxt@rock-chips.com [seanpaul fixed up the commit message a bit and renamed *_supported to *_enabled] Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 15 ++++++++------- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 +- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 2 +- include/drm/bridge/analogix_dp.h | 2 +- 4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index e738aa6de1af..ee00d3d920e0 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -98,18 +98,18 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp) return 0; }
-int analogix_dp_psr_supported(struct analogix_dp_device *dp) +int analogix_dp_psr_enabled(struct analogix_dp_device *dp) {
- return dp->psr_support;
- return dp->psr_enable; }
-EXPORT_SYMBOL_GPL(analogix_dp_psr_supported); +EXPORT_SYMBOL_GPL(analogix_dp_psr_enabled);
int analogix_dp_enable_psr(struct analogix_dp_device *dp) { struct edp_vsc_psr psr_vsc;
- if (!dp->psr_support)
if (!dp->psr_enable) return 0;
/* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */
@@ -131,7 +131,7 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp) struct edp_vsc_psr psr_vsc; int ret;
- if (!dp->psr_support)
if (!dp->psr_enable) return 0;
/* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */
@@ -871,8 +871,8 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) /* Enable video */ analogix_dp_start_video(dp);
- dp->psr_support = analogix_dp_detect_sink_psr(dp);
- if (dp->psr_support)
- dp->psr_enable = analogix_dp_detect_sink_psr(dp);
- if (dp->psr_enable) analogix_dp_enable_sink_psr(dp); }
@@ -1117,6 +1117,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge) if (ret) DRM_ERROR("failed to setup the panel ret = %d\n", ret);
- dp->psr_enable = false; dp->dpms_mode = DRM_MODE_DPMS_OFF; }
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index b039b28d8fcc..e135a42cb19e 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -170,7 +170,7 @@ struct analogix_dp_device { int dpms_mode; int hpd_gpio; bool force_hpd;
- bool psr_support;
bool psr_enable;
struct mutex panel_lock; bool panel_is_modeset;
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 36334839a3f8..3e8bf79bea58 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -82,7 +82,7 @@ static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled) struct rockchip_dp_device *dp = to_dp(encoder); int ret;
- if (!analogix_dp_psr_supported(dp->adp))
if (!analogix_dp_psr_enabled(dp->adp)) return;
DRM_DEV_DEBUG(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit");
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h index 711fff9b6803..e9a1116d2f8e 100644 --- a/include/drm/bridge/analogix_dp.h +++ b/include/drm/bridge/analogix_dp.h @@ -41,7 +41,7 @@ struct analogix_dp_plat_data { struct drm_connector *); };
-int analogix_dp_psr_supported(struct analogix_dp_device *dp); +int analogix_dp_psr_enabled(struct analogix_dp_device *dp); int analogix_dp_enable_psr(struct analogix_dp_device *dp); int analogix_dp_disable_psr(struct analogix_dp_device *dp);
Am Freitag, 9. März 2018, 23:22:54 CET schrieb Enric Balletbo i Serra:
From: zain wang wzz@rock-chips.com
There is a race between AUX CH bring-up and enabling bridge which will cause link training to fail. To avoid hitting it, don't change psr state while enabling the bridge.
Cc: Tomeu Vizoso tomeu.vizoso@collabora.com Cc: Sean Paul seanpaul@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Caesar Wang wxt@rock-chips.com [seanpaul fixed up the commit message a bit and renamed *_supported to *_enabled] Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
applied to drm-misc-next
Thanks Heiko
From: zain wang wzz@rock-chips.com
Add a lock to vop to avoid disabling the crtc while waiting for a line flag while enabling psr. If we disable in the middle of waiting for the line flag, we'll end up timing out or worse.
Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 30 +++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 158e79e5062e..b601c59e76a8 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -117,6 +117,8 @@ struct vop { spinlock_t reg_lock; /* lock vop irq reg */ spinlock_t irq_lock; + /* protects crtc enable/disable */ + struct mutex vop_lock;
unsigned int irq;
@@ -569,6 +571,7 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
WARN_ON(vop->event);
+ mutex_lock(&vop->vop_lock); drm_crtc_vblank_off(crtc);
/* @@ -604,6 +607,7 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc, clk_disable(vop->aclk); clk_disable(vop->hclk); pm_runtime_put(vop->dev); + mutex_unlock(&vop->vop_lock);
if (crtc->state->event && !crtc->state->active) { spin_lock_irq(&crtc->dev->event_lock); @@ -868,10 +872,13 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc, uint32_t pin_pol, val; int ret;
+ mutex_lock(&vop->vop_lock); + WARN_ON(vop->event);
ret = vop_enable(crtc); if (ret) { + mutex_unlock(&vop->vop_lock); DRM_DEV_ERROR(vop->dev, "Failed to enable vop (%d)\n", ret); return; } @@ -935,6 +942,7 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc, clk_set_rate(vop->dclk, adjusted_mode->clock * 1000);
VOP_REG_SET(vop, common, standby, 0); + mutex_unlock(&vop->vop_lock); }
static bool vop_fs_irq_is_pending(struct vop *vop) @@ -1473,15 +1481,21 @@ int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout) { struct vop *vop = to_vop(crtc); unsigned long jiffies_left; + int ret = 0;
if (!crtc || !vop->is_enabled) return -ENODEV;
- if (mstimeout <= 0) - return -EINVAL; + mutex_lock(&vop->vop_lock); + if (mstimeout <= 0) { + ret = -EINVAL; + goto out; + }
- if (vop_line_flag_irq_is_enabled(vop)) - return -EBUSY; + if (vop_line_flag_irq_is_enabled(vop)) { + ret = -EBUSY; + goto out; + }
reinit_completion(&vop->line_flag_completion); vop_line_flag_irq_enable(vop); @@ -1492,10 +1506,13 @@ int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout)
if (jiffies_left == 0) { DRM_DEV_ERROR(vop->dev, "Timeout waiting for IRQ\n"); - return -ETIMEDOUT; + ret = -ETIMEDOUT; + goto out; }
- return 0; +out: + mutex_unlock(&vop->vop_lock); + return ret; } EXPORT_SYMBOL(rockchip_drm_wait_vact_end);
@@ -1545,6 +1562,7 @@ static int vop_bind(struct device *dev, struct device *master, void *data)
spin_lock_init(&vop->reg_lock); spin_lock_init(&vop->irq_lock); + mutex_init(&vop->vop_lock);
ret = devm_request_irq(dev, vop->irq, vop_isr, IRQF_SHARED, dev_name(dev), vop);
Am Freitag, 9. März 2018, 23:22:55 CET schrieb Enric Balletbo i Serra:
From: zain wang wzz@rock-chips.com
Add a lock to vop to avoid disabling the crtc while waiting for a line flag while enabling psr. If we disable in the middle of waiting for the line flag, we'll end up timing out or worse.
Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
Reviewed-by: Heiko Stuebner heiko@sntech.de
Am Freitag, 9. März 2018, 23:22:55 CET schrieb Enric Balletbo i Serra:
From: zain wang wzz@rock-chips.com
Add a lock to vop to avoid disabling the crtc while waiting for a line flag while enabling psr. If we disable in the middle of waiting for the line flag, we'll end up timing out or worse.
Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
applied to drm-misc-next
Thanks Heiko
From: zain wang wzz@rock-chips.com
We would meet a short black screen when exit PSR with the full link training, In this case, we should use fast link train instead of full link training.
Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 142 ++++++++++++++++----- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 + 2 files changed, 114 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index ee00d3d920e0..806c3878b3d6 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -10,17 +10,18 @@ * option) any later version. */
-#include <linux/module.h> -#include <linux/platform_device.h> -#include <linux/err.h> #include <linux/clk.h> -#include <linux/io.h> +#include <linux/component.h> +#include <linux/err.h> +#include <linux/gpio.h> #include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/iopoll.h> +#include <linux/module.h> #include <linux/of.h> #include <linux/of_gpio.h> -#include <linux/gpio.h> -#include <linux/component.h> #include <linux/phy/phy.h> +#include <linux/platform_device.h>
#include <drm/drmP.h> #include <drm/drm_atomic_helper.h> @@ -35,6 +36,8 @@
#define to_dp(nm) container_of(nm, struct analogix_dp_device, nm)
+static const bool verify_fast_training; + struct bridge_init { struct i2c_client *client; struct device_node *node; @@ -528,7 +531,7 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) { int lane, lane_count, retval; u32 reg; - u8 link_align, link_status[2], adjust_request[2]; + u8 link_align, link_status[2], adjust_request[2], spread;
usleep_range(400, 401);
@@ -571,6 +574,20 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) dev_dbg(dp->dev, "final lane count = %.2x\n", dp->link_train.lane_count);
+ retval = drm_dp_dpcd_readb(&dp->aux, DP_MAX_DOWNSPREAD, + &spread); + if (retval != 1) { + dev_err(dp->dev, "failed to read downspread %d\n", + retval); + dp->fast_train_support = false; + } else { + dp->fast_train_support = + (spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING) ? + true : false; + } + dev_dbg(dp->dev, "fast link training %s\n", + dp->fast_train_support ? "supported" : "unsupported"); + /* set enhanced mode if available */ analogix_dp_set_enhanced_mode(dp); dp->link_train.lt_state = FINISHED; @@ -627,10 +644,12 @@ static void analogix_dp_get_max_rx_lane_count(struct analogix_dp_device *dp, *lane_count = DPCD_MAX_LANE_COUNT(data); }
-static void analogix_dp_init_training(struct analogix_dp_device *dp, - enum link_lane_count_type max_lane, - int max_rate) +static int analogix_dp_full_link_train(struct analogix_dp_device *dp, + u32 max_lanes, u32 max_rate) { + int retval = 0; + bool training_finished = false; + /* * MACRO_RST must be applied after the PLL_LOCK to avoid * the DP inter pair skew issue for at least 10 us @@ -656,18 +675,13 @@ static void analogix_dp_init_training(struct analogix_dp_device *dp, }
/* Setup TX lane count & rate */ - if (dp->link_train.lane_count > max_lane) - dp->link_train.lane_count = max_lane; + if (dp->link_train.lane_count > max_lanes) + dp->link_train.lane_count = max_lanes; if (dp->link_train.link_rate > max_rate) dp->link_train.link_rate = max_rate;
/* All DP analog module power up */ analogix_dp_set_analog_power_down(dp, POWER_ALL, 0); -} - -static int analogix_dp_sw_link_training(struct analogix_dp_device *dp) -{ - int retval = 0, training_finished = 0;
dp->link_train.lt_state = START;
@@ -702,22 +716,88 @@ static int analogix_dp_sw_link_training(struct analogix_dp_device *dp) return retval; }
-static int analogix_dp_set_link_train(struct analogix_dp_device *dp, - u32 count, u32 bwtype) +static int analogix_dp_fast_link_train(struct analogix_dp_device *dp) { - int i; - int retval; + int i, ret; + u8 link_align, link_status[2]; + enum pll_status status;
- for (i = 0; i < DP_TIMEOUT_LOOP_COUNT; i++) { - analogix_dp_init_training(dp, count, bwtype); - retval = analogix_dp_sw_link_training(dp); - if (retval == 0) - break; + analogix_dp_reset_macro(dp); + + analogix_dp_set_link_bandwidth(dp, dp->link_train.link_rate); + analogix_dp_set_lane_count(dp, dp->link_train.lane_count);
- usleep_range(100, 110); + for (i = 0; i < dp->link_train.lane_count; i++) { + analogix_dp_set_lane_link_training(dp, + dp->link_train.training_lane[i], i); }
- return retval; + ret = readx_poll_timeout(analogix_dp_get_pll_lock_status, dp, status, + status != PLL_UNLOCKED, 120, + 120 * DP_TIMEOUT_LOOP_COUNT); + if (ret) { + DRM_DEV_ERROR(dp->dev, "Wait for pll lock failed %d\n", ret); + return ret; + } + + /* source Set training pattern 1 */ + analogix_dp_set_training_pattern(dp, TRAINING_PTN1); + /* From DP spec, pattern must be on-screen for a minimum 500us */ + usleep_range(500, 600); + + analogix_dp_set_training_pattern(dp, TRAINING_PTN2); + /* From DP spec, pattern must be on-screen for a minimum 500us */ + usleep_range(500, 600); + + /* TODO: enhanced_mode?*/ + analogix_dp_set_training_pattern(dp, DP_NONE); + + /* + * Useful for debugging issues with fast link training, disable for more + * speed + */ + if (verify_fast_training) { + ret = drm_dp_dpcd_readb(&dp->aux, DP_LANE_ALIGN_STATUS_UPDATED, + &link_align); + if (ret < 0) { + DRM_DEV_ERROR(dp->dev, "Read align status failed %d\n", + ret); + return ret; + } + + ret = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status, + 2); + if (ret < 0) { + DRM_DEV_ERROR(dp->dev, "Read link status failed %d\n", + ret); + return ret; + } + + if (analogix_dp_clock_recovery_ok(link_status, + dp->link_train.lane_count)) { + DRM_DEV_ERROR(dp->dev, "Clock recovery failed\n"); + analogix_dp_reduce_link_rate(dp); + return -EIO; + } + + if (analogix_dp_channel_eq_ok(link_status, link_align, + dp->link_train.lane_count)) { + DRM_DEV_ERROR(dp->dev, "Channel EQ failed\n"); + analogix_dp_reduce_link_rate(dp); + return -EIO; + } + } + + return 0; +} + +static int analogix_dp_train_link(struct analogix_dp_device *dp) +{ + if (dp->fast_train_support) + return analogix_dp_fast_link_train(dp); + + return analogix_dp_full_link_train(dp, dp->video_info.max_lane_count, + dp->video_info.max_link_rate); }
static int analogix_dp_config_video(struct analogix_dp_device *dp) @@ -846,10 +926,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) DRM_ERROR("failed to disable the panel\n"); }
- ret = analogix_dp_set_link_train(dp, dp->video_info.max_lane_count, - dp->video_info.max_link_rate); + ret = readx_poll_timeout(analogix_dp_train_link, dp, ret, !ret, 100, + DP_TIMEOUT_TRAINING_US * 5); if (ret) { - dev_err(dp->dev, "unable to do link train\n"); + dev_err(dp->dev, "unable to do link train, ret=%d\n", ret); return; }
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index e135a42cb19e..920607d7eb3e 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -20,6 +20,8 @@ #define MAX_CR_LOOP 5 #define MAX_EQ_LOOP 5
+/* Training takes 22ms if AUX channel comm fails. Use this as retry interval */ +#define DP_TIMEOUT_TRAINING_US 22000 #define DP_TIMEOUT_PSR_LOOP_MS 300
/* DP_MAX_LANE_COUNT */ @@ -171,6 +173,7 @@ struct analogix_dp_device { int hpd_gpio; bool force_hpd; bool psr_enable; + bool fast_train_support;
struct mutex panel_lock; bool panel_is_modeset;
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
We would meet a short black screen when exit PSR with the full link training, In this case, we should use fast link train instead of full link training.
Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 142 ++++++++++++++++----- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 + 2 files changed, 114 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index ee00d3d920e0..806c3878b3d6 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -10,17 +10,18 @@
- option) any later version.
*/
-#include <linux/module.h> -#include <linux/platform_device.h> -#include <linux/err.h> #include <linux/clk.h> -#include <linux/io.h> +#include <linux/component.h> +#include <linux/err.h> +#include <linux/gpio.h> #include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/iopoll.h> +#include <linux/module.h> #include <linux/of.h> #include <linux/of_gpio.h> -#include <linux/gpio.h> -#include <linux/component.h> #include <linux/phy/phy.h> +#include <linux/platform_device.h>
This re-ordering doesn't seem like it should be a part of this patch, you can let it stay if it happens to cause conflicts with future patches. Other than that:
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
#include <drm/drmP.h> #include <drm/drm_atomic_helper.h> @@ -35,6 +36,8 @@
#define to_dp(nm) container_of(nm, struct analogix_dp_device, nm)
+static const bool verify_fast_training;
- struct bridge_init { struct i2c_client *client; struct device_node *node;
@@ -528,7 +531,7 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) { int lane, lane_count, retval; u32 reg;
- u8 link_align, link_status[2], adjust_request[2];
u8 link_align, link_status[2], adjust_request[2], spread;
usleep_range(400, 401);
@@ -571,6 +574,20 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) dev_dbg(dp->dev, "final lane count = %.2x\n", dp->link_train.lane_count);
retval = drm_dp_dpcd_readb(&dp->aux, DP_MAX_DOWNSPREAD,&spread);if (retval != 1) {dev_err(dp->dev, "failed to read downspread %d\n",retval);dp->fast_train_support = false;} else {dp->fast_train_support =(spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING) ?true : false;}dev_dbg(dp->dev, "fast link training %s\n",dp->fast_train_support ? "supported" : "unsupported");- /* set enhanced mode if available */ analogix_dp_set_enhanced_mode(dp); dp->link_train.lt_state = FINISHED;
@@ -627,10 +644,12 @@ static void analogix_dp_get_max_rx_lane_count(struct analogix_dp_device *dp, *lane_count = DPCD_MAX_LANE_COUNT(data); }
-static void analogix_dp_init_training(struct analogix_dp_device *dp,
enum link_lane_count_type max_lane,int max_rate)+static int analogix_dp_full_link_train(struct analogix_dp_device *dp,
u32 max_lanes, u32 max_rate)- int retval = 0;
- bool training_finished = false;
- /*
- MACRO_RST must be applied after the PLL_LOCK to avoid
- the DP inter pair skew issue for at least 10 us
@@ -656,18 +675,13 @@ static void analogix_dp_init_training(struct analogix_dp_device *dp, }
/* Setup TX lane count & rate */
- if (dp->link_train.lane_count > max_lane)
dp->link_train.lane_count = max_lane;
if (dp->link_train.lane_count > max_lanes)
dp->link_train.lane_count = max_lanes;if (dp->link_train.link_rate > max_rate) dp->link_train.link_rate = max_rate;
/* All DP analog module power up */ analogix_dp_set_analog_power_down(dp, POWER_ALL, 0);
-}
-static int analogix_dp_sw_link_training(struct analogix_dp_device *dp) -{
int retval = 0, training_finished = 0;
dp->link_train.lt_state = START;
@@ -702,22 +716,88 @@ static int analogix_dp_sw_link_training(struct analogix_dp_device *dp) return retval; }
-static int analogix_dp_set_link_train(struct analogix_dp_device *dp,
u32 count, u32 bwtype)+static int analogix_dp_fast_link_train(struct analogix_dp_device *dp) {
- int i;
- int retval;
- int i, ret;
- u8 link_align, link_status[2];
- enum pll_status status;
- for (i = 0; i < DP_TIMEOUT_LOOP_COUNT; i++) {
analogix_dp_init_training(dp, count, bwtype);retval = analogix_dp_sw_link_training(dp);if (retval == 0)break;
- analogix_dp_reset_macro(dp);
- analogix_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
- analogix_dp_set_lane_count(dp, dp->link_train.lane_count);
usleep_range(100, 110);
- for (i = 0; i < dp->link_train.lane_count; i++) {
analogix_dp_set_lane_link_training(dp,dp->link_train.training_lane[i], i);
- return retval;
- ret = readx_poll_timeout(analogix_dp_get_pll_lock_status, dp, status,
status != PLL_UNLOCKED, 120,120 * DP_TIMEOUT_LOOP_COUNT);- if (ret) {
DRM_DEV_ERROR(dp->dev, "Wait for pll lock failed %d\n", ret);return ret;- }
- /* source Set training pattern 1 */
- analogix_dp_set_training_pattern(dp, TRAINING_PTN1);
- /* From DP spec, pattern must be on-screen for a minimum 500us */
- usleep_range(500, 600);
- analogix_dp_set_training_pattern(dp, TRAINING_PTN2);
- /* From DP spec, pattern must be on-screen for a minimum 500us */
- usleep_range(500, 600);
- /* TODO: enhanced_mode?*/
- analogix_dp_set_training_pattern(dp, DP_NONE);
- /*
* Useful for debugging issues with fast link training, disable for more* speed*/- if (verify_fast_training) {
ret = drm_dp_dpcd_readb(&dp->aux, DP_LANE_ALIGN_STATUS_UPDATED,&link_align);if (ret < 0) {DRM_DEV_ERROR(dp->dev, "Read align status failed %d\n",ret);return ret;}ret = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status,2);if (ret < 0) {DRM_DEV_ERROR(dp->dev, "Read link status failed %d\n",ret);return ret;}if (analogix_dp_clock_recovery_ok(link_status,dp->link_train.lane_count)) {DRM_DEV_ERROR(dp->dev, "Clock recovery failed\n");analogix_dp_reduce_link_rate(dp);return -EIO;}if (analogix_dp_channel_eq_ok(link_status, link_align,dp->link_train.lane_count)) {DRM_DEV_ERROR(dp->dev, "Channel EQ failed\n");analogix_dp_reduce_link_rate(dp);return -EIO;}- }
- return 0;
+}
+static int analogix_dp_train_link(struct analogix_dp_device *dp) +{
if (dp->fast_train_support)
return analogix_dp_fast_link_train(dp);return analogix_dp_full_link_train(dp, dp->video_info.max_lane_count,
dp->video_info.max_link_rate);}
static int analogix_dp_config_video(struct analogix_dp_device *dp)
@@ -846,10 +926,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) DRM_ERROR("failed to disable the panel\n"); }
- ret = analogix_dp_set_link_train(dp, dp->video_info.max_lane_count,
dp->video_info.max_link_rate);
- ret = readx_poll_timeout(analogix_dp_train_link, dp, ret, !ret, 100,
DP_TIMEOUT_TRAINING_US * 5);
dev_err(dp->dev, "unable to do link train\n");
dev_err(dp->dev, "unable to do link train, ret=%d\n", ret);diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index e135a42cb19e..920607d7eb3e 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -20,6 +20,8 @@ #define MAX_CR_LOOP 5 #define MAX_EQ_LOOP 5
+/* Training takes 22ms if AUX channel comm fails. Use this as retry interval */ +#define DP_TIMEOUT_TRAINING_US 22000 #define DP_TIMEOUT_PSR_LOOP_MS 300
/* DP_MAX_LANE_COUNT */ @@ -171,6 +173,7 @@ struct analogix_dp_device { int hpd_gpio; bool force_hpd; bool psr_enable;
bool fast_train_support;
struct mutex panel_lock; bool panel_is_modeset;
Am Freitag, 9. März 2018, 23:22:56 CET schrieb Enric Balletbo i Serra:
From: zain wang wzz@rock-chips.com
We would meet a short black screen when exit PSR with the full link training, In this case, we should use fast link train instead of full link training.
Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
applied to drm-misc-next
Checking the whole series, there is nothing else that touches the reordered headers, so I've dropped that change, leaving only the newly added iopoll.h inclusion in [0].
Thanks Heiko
[0] https://cgit.freedesktop.org/drm/drm-misc/commit/?id=f9d5680596f2dac390918e6...
From: zain wang wzz@rock-chips.com
We currently wait for the panel to mirror our intended PSR state before continuing on both PSR enter and PSR exit. This is really only important to do when we're entering PSR, since we want to be sure the last frame we pushed is being served from the panel's internal fb before shutting down the soc blocks (vop/analogix).
This patch changes the behavior such that we only wait for the panel to complete the PSR transition when we're entering PSR, and to skip verification when we're exiting.
Cc: Stéphane Marchesin marcheu@chromium.org Cc: Sonny Rao sonnyrao@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 4 ++-- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 +- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 5 ++++- 3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 806c3878b3d6..5a2e35dc41e3 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -125,7 +125,7 @@ int analogix_dp_enable_psr(struct analogix_dp_device *dp) psr_vsc.DB0 = 0; psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
- return analogix_dp_send_psr_spd(dp, &psr_vsc); + return analogix_dp_send_psr_spd(dp, &psr_vsc, true); } EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
@@ -151,7 +151,7 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp) if (ret != 1) dev_err(dp->dev, "Failed to set DP Power0 %d\n", ret);
- return analogix_dp_send_psr_spd(dp, &psr_vsc); + return analogix_dp_send_psr_spd(dp, &psr_vsc, false); } EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index 920607d7eb3e..6a96ef7e6934 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -253,7 +253,7 @@ void analogix_dp_enable_scrambling(struct analogix_dp_device *dp); void analogix_dp_disable_scrambling(struct analogix_dp_device *dp); void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp); int analogix_dp_send_psr_spd(struct analogix_dp_device *dp, - struct edp_vsc_psr *vsc); + struct edp_vsc_psr *vsc, bool blocking); ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, struct drm_dp_aux_msg *msg);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 005a3f7005d2..9df2f3ef000c 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -1007,7 +1007,7 @@ static ssize_t analogix_dp_get_psr_status(struct analogix_dp_device *dp) }
int analogix_dp_send_psr_spd(struct analogix_dp_device *dp, - struct edp_vsc_psr *vsc) + struct edp_vsc_psr *vsc, bool blocking) { unsigned int val; int ret; @@ -1053,6 +1053,9 @@ int analogix_dp_send_psr_spd(struct analogix_dp_device *dp, val |= IF_EN; writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+ if (!blocking) + return 0; + ret = readx_poll_timeout(analogix_dp_get_psr_status, dp, psr_status, psr_status >= 0 && ((vsc->DB1 && psr_status == DP_PSR_SINK_ACTIVE_RFB) ||
Hi,
the subject is misleading I think, as this is touching only the generic bridge code and not anything Rockchip-related, so should probably be "drm/bridge"?
Am Freitag, 9. März 2018, 23:22:57 CET schrieb Enric Balletbo i Serra:
From: zain wang wzz@rock-chips.com
We currently wait for the panel to mirror our intended PSR state before continuing on both PSR enter and PSR exit. This is really only important to do when we're entering PSR, since we want to be sure the last frame we pushed is being served from the panel's internal fb before shutting down the soc blocks (vop/analogix).
This patch changes the behavior such that we only wait for the panel to complete the PSR transition when we're entering PSR, and to skip verification when we're exiting.
Cc: Stéphane Marchesin marcheu@chromium.org Cc: Sonny Rao sonnyrao@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
change itself looks good, so Reviewed-by: Heiko Stuebner heiko@sntech.de
On 12/03/18 17:58, Heiko Stübner wrote:
Hi,
the subject is misleading I think, as this is touching only the generic bridge code and not anything Rockchip-related, so should probably be "drm/bridge"?
Right, I'll fix this when I need to send another version, I'll wait, for now, a bit more for reviews on the other patches.
Thanks Enric
Am Freitag, 9. März 2018, 23:22:57 CET schrieb Enric Balletbo i Serra:
From: zain wang wzz@rock-chips.com
We currently wait for the panel to mirror our intended PSR state before continuing on both PSR enter and PSR exit. This is really only important to do when we're entering PSR, since we want to be sure the last frame we pushed is being served from the panel's internal fb before shutting down the soc blocks (vop/analogix).
This patch changes the behavior such that we only wait for the panel to complete the PSR transition when we're entering PSR, and to skip verification when we're exiting.
Cc: Stéphane Marchesin marcheu@chromium.org Cc: Sonny Rao sonnyrao@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
change itself looks good, so Reviewed-by: Heiko Stuebner heiko@sntech.de
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
We currently wait for the panel to mirror our intended PSR state before continuing on both PSR enter and PSR exit. This is really only important to do when we're entering PSR, since we want to be sure the last frame we pushed is being served from the panel's internal fb before shutting down the soc blocks (vop/analogix).
This patch changes the behavior such that we only wait for the panel to complete the PSR transition when we're entering PSR, and to skip verification when we're exiting.
With the subject fix:
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Stéphane Marchesin marcheu@chromium.org Cc: Sonny Rao sonnyrao@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 4 ++-- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 +- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 5 ++++- 3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 806c3878b3d6..5a2e35dc41e3 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -125,7 +125,7 @@ int analogix_dp_enable_psr(struct analogix_dp_device *dp) psr_vsc.DB0 = 0; psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
- return analogix_dp_send_psr_spd(dp, &psr_vsc);
- return analogix_dp_send_psr_spd(dp, &psr_vsc, true); } EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
@@ -151,7 +151,7 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp) if (ret != 1) dev_err(dp->dev, "Failed to set DP Power0 %d\n", ret);
- return analogix_dp_send_psr_spd(dp, &psr_vsc);
- return analogix_dp_send_psr_spd(dp, &psr_vsc, false); } EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index 920607d7eb3e..6a96ef7e6934 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -253,7 +253,7 @@ void analogix_dp_enable_scrambling(struct analogix_dp_device *dp); void analogix_dp_disable_scrambling(struct analogix_dp_device *dp); void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp); int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
struct edp_vsc_psr *vsc);
struct edp_vsc_psr *vsc, bool blocking);diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 005a3f7005d2..9df2f3ef000c 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -1007,7 +1007,7 @@ static ssize_t analogix_dp_get_psr_status(struct analogix_dp_device *dp) }
int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
struct edp_vsc_psr *vsc)
struct edp_vsc_psr *vsc, bool blocking)@@ -1053,6 +1053,9 @@ int analogix_dp_send_psr_spd(struct analogix_dp_device *dp, val |= IF_EN; writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
- if (!blocking)
return 0;- ret = readx_poll_timeout(analogix_dp_get_psr_status, dp, psr_status, psr_status >= 0 && ((vsc->DB1 && psr_status == DP_PSR_SINK_ACTIVE_RFB) ||
Am Freitag, 9. März 2018, 23:22:57 CET schrieb Enric Balletbo i Serra:
From: zain wang wzz@rock-chips.com
We currently wait for the panel to mirror our intended PSR state before continuing on both PSR enter and PSR exit. This is really only important to do when we're entering PSR, since we want to be sure the last frame we pushed is being served from the panel's internal fb before shutting down the soc blocks (vop/analogix).
This patch changes the behavior such that we only wait for the panel to complete the PSR transition when we're entering PSR, and to skip verification when we're exiting.
Cc: Stéphane Marchesin marcheu@chromium.org Cc: Sonny Rao sonnyrao@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
applied to drm-misc-next with the subject fixed.
Thanks Heiko
From: Lin Huang hl@rock-chips.com
We need to enable video before analogix_dp_is_video_stream_on(), so we can get the right video stream status.
Cc: 征增 王 wzz@rock-chips.com Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 5a2e35dc41e3..f9661b410cb9 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -819,11 +819,10 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp) if (analogix_dp_is_slave_video_stream_clock_on(dp) == 0) break; if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) { - dev_err(dp->dev, "Timeout of video streamclk ok\n"); + dev_err(dp->dev, "Timeout of slave video streamclk ok\n"); return -ETIMEDOUT; } - - usleep_range(1, 2); + usleep_range(1000, 1001); }
/* Set to use the register calculated M/N video */ @@ -838,6 +837,9 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp) /* Configure video slave mode */ analogix_dp_enable_video_master(dp, 0);
+ /* Enable video */ + analogix_dp_start_video(dp); + timeout_loop = 0;
for (;;) { @@ -948,9 +950,6 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) DRM_ERROR("failed to enable the panel\n"); }
- /* Enable video */ - analogix_dp_start_video(dp); - dp->psr_enable = analogix_dp_detect_sink_psr(dp); if (dp->psr_enable) analogix_dp_enable_sink_psr(dp);
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
From: Lin Huang hl@rock-chips.com
We need to enable video before analogix_dp_is_video_stream_on(), so we can get the right video stream status.
Cc: 征增 王 wzz@rock-chips.com Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 5a2e35dc41e3..f9661b410cb9 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -819,11 +819,10 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp) if (analogix_dp_is_slave_video_stream_clock_on(dp) == 0) break; if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
dev_err(dp->dev, "Timeout of video streamclk ok\n");
dev_err(dp->dev, "Timeout of slave video streamclk ok\n"); return -ETIMEDOUT;
usleep_range(1, 2);
usleep_range(1000, 1001);
Could we briefly explain in the commit message why we need to increase the delay in the timeout loop? Is it a consequence of calling analogix_dp_start_video() earlier, or is this the preferred time mentioned in the specs?
Thanks, Archit
}
/* Set to use the register calculated M/N video */ @@ -838,6 +837,9 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp) /* Configure video slave mode */ analogix_dp_enable_video_master(dp, 0);
/* Enable video */
analogix_dp_start_video(dp);
timeout_loop = 0;
for (;;) {
@@ -948,9 +950,6 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) DRM_ERROR("failed to enable the panel\n"); }
- /* Enable video */
- analogix_dp_start_video(dp);
- dp->psr_enable = analogix_dp_detect_sink_psr(dp); if (dp->psr_enable) analogix_dp_enable_sink_psr(dp);
Hi Archit,
Am Mittwoch, 14. März 2018, 06:59:59 CET schrieb Archit Taneja:
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
From: Lin Huang hl@rock-chips.com
We need to enable video before analogix_dp_is_video_stream_on(), so we can get the right video stream status.
Cc: 征增 王 wzz@rock-chips.com Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 5a2e35dc41e3..f9661b410cb9 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -819,11 +819,10 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp) if (analogix_dp_is_slave_video_stream_clock_on(dp) == 0) break; if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
dev_err(dp->dev, "Timeout of video streamclk ok\n");
dev_err(dp->dev, "Timeout of slave video streamclk ok\n"); return -ETIMEDOUT;
usleep_range(1, 2);
usleep_range(1000, 1001);Could we briefly explain in the commit message why we need to increase the delay in the timeout loop? Is it a consequence of calling analogix_dp_start_video() earlier, or is this the preferred time mentioned in the specs?
I asked Lin, the original author of this patch, response quoted below:
"There is random "Timeout of video streamclk ok" message happen when debug edp panel, So we extend this time, this time do not define in the spec."
So it looks like it was working by chance before and this move triggered some sort of timing issue.
Heiko
From: Lin Huang hl@rock-chips.com
We should check AUX_EN bit to confirm the AUX CH operation is completed.
Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 25 +++++++++++++---------- 1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 9df2f3ef000c..e78c861b9e06 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -1073,9 +1073,9 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, { u32 reg; u8 *buffer = msg->buffer; - int timeout_loop = 0; unsigned int i; int num_transferred = 0; + int ret;
/* Buffer size of AUX CH is 16 bytes */ if (WARN_ON(msg->size > 16)) @@ -1139,17 +1139,20 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
- /* Is AUX CH command reply received? */ + ret = readx_poll_timeout(readl, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2, + reg, !(reg & AUX_EN), 25, 500 * 1000); + if (ret) { + dev_err(dp->dev, "AUX CH enable timeout!\n"); + return -ETIMEDOUT; + } + /* TODO: Wait for an interrupt instead of looping? */ - reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA); - while (!(reg & RPLY_RECEIV)) { - timeout_loop++; - if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) { - dev_err(dp->dev, "AUX CH command reply failed!\n"); - return -ETIMEDOUT; - } - reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA); - usleep_range(10, 11); + /* Is AUX CH command reply received? */ + ret = readx_poll_timeout(readl, dp->reg_base + ANALOGIX_DP_INT_STA, + reg, reg & RPLY_RECEIV, 10, 20 * 1000); + if (ret) { + dev_err(dp->dev, "AUX CH cmd reply timeout!\n"); + return -ETIMEDOUT; }
/* Clear interrupt source for AUX CH command reply */
On Saturday 10 March 2018 03:52 AM, Enric Balletbo i Serra wrote:
From: Lin Huang hl@rock-chips.com
We should check AUX_EN bit to confirm the AUX CH operation is completed.
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 25 +++++++++++++---------- 1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 9df2f3ef000c..e78c861b9e06 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -1073,9 +1073,9 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, { u32 reg; u8 *buffer = msg->buffer;
- int timeout_loop = 0; unsigned int i; int num_transferred = 0;
int ret;
/* Buffer size of AUX CH is 16 bytes */ if (WARN_ON(msg->size > 16))
@@ -1139,17 +1139,20 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2);
- /* Is AUX CH command reply received? */
- ret = readx_poll_timeout(readl, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2,
reg, !(reg & AUX_EN), 25, 500 * 1000);- if (ret) {
dev_err(dp->dev, "AUX CH enable timeout!\n");return -ETIMEDOUT;- }
- /* TODO: Wait for an interrupt instead of looping? */
- reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
- while (!(reg & RPLY_RECEIV)) {
timeout_loop++;if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {dev_err(dp->dev, "AUX CH command reply failed!\n");return -ETIMEDOUT;}reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);usleep_range(10, 11);
/* Is AUX CH command reply received? */
ret = readx_poll_timeout(readl, dp->reg_base + ANALOGIX_DP_INT_STA,
reg, reg & RPLY_RECEIV, 10, 20 * 1000);if (ret) {
dev_err(dp->dev, "AUX CH cmd reply timeout!\n");return -ETIMEDOUT;}
/* Clear interrupt source for AUX CH command reply */
From: zain wang wzz@rock-chips.com
Panel would reset its setting when it powers down. It would forget the last succeeded link training setting. So we can't use the last successful link training setting to do fast link training. Let's reset fast_train_enable in analogix_dp_bridge_disable();
Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 9 +++++---- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index f9661b410cb9..ea7a80a989c6 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -579,14 +579,14 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) if (retval != 1) { dev_err(dp->dev, "failed to read downspread %d\n", retval); - dp->fast_train_support = false; + dp->fast_train_enable = false; } else { - dp->fast_train_support = + dp->fast_train_enable = (spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING) ? true : false; } dev_dbg(dp->dev, "fast link training %s\n", - dp->fast_train_support ? "supported" : "unsupported"); + dp->fast_train_enable ? "supported" : "unsupported");
/* set enhanced mode if available */ analogix_dp_set_enhanced_mode(dp); @@ -793,7 +793,7 @@ static int analogix_dp_fast_link_train(struct analogix_dp_device *dp)
static int analogix_dp_train_link(struct analogix_dp_device *dp) { - if (dp->fast_train_support) + if (dp->fast_train_enable) return analogix_dp_fast_link_train(dp);
return analogix_dp_full_link_train(dp, dp->video_info.max_lane_count, @@ -1197,6 +1197,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge) DRM_ERROR("failed to setup the panel ret = %d\n", ret);
dp->psr_enable = false; + dp->fast_train_enable = false; dp->dpms_mode = DRM_MODE_DPMS_OFF; }
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index 6a96ef7e6934..403ff853464b 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -173,7 +173,7 @@ struct analogix_dp_device { int hpd_gpio; bool force_hpd; bool psr_enable; - bool fast_train_support; + bool fast_train_enable;
struct mutex panel_lock; bool panel_is_modeset;
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
Panel would reset its setting when it powers down. It would forget the last succeeded link training setting. So we can't use the last successful link training setting to do fast link training. Let's reset fast_train_enable in analogix_dp_bridge_disable();
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 9 +++++---- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index f9661b410cb9..ea7a80a989c6 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -579,14 +579,14 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) if (retval != 1) { dev_err(dp->dev, "failed to read downspread %d\n", retval);
dp->fast_train_support = false;
dp->fast_train_enable = false;
dp->fast_train_support =
dp->fast_train_enable = (spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING) ? true : false;
dp->fast_train_support ? "supported" : "unsupported");
dp->fast_train_enable ? "supported" : "unsupported");/* set enhanced mode if available */ analogix_dp_set_enhanced_mode(dp);
@@ -793,7 +793,7 @@ static int analogix_dp_fast_link_train(struct analogix_dp_device *dp)
static int analogix_dp_train_link(struct analogix_dp_device *dp) {
- if (dp->fast_train_support)
if (dp->fast_train_enable) return analogix_dp_fast_link_train(dp);
return analogix_dp_full_link_train(dp, dp->video_info.max_lane_count,
@@ -1197,6 +1197,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge) DRM_ERROR("failed to setup the panel ret = %d\n", ret);
dp->psr_enable = false;
- dp->fast_train_enable = false; dp->dpms_mode = DRM_MODE_DPMS_OFF; }
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index 6a96ef7e6934..403ff853464b 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -173,7 +173,7 @@ struct analogix_dp_device { int hpd_gpio; bool force_hpd; bool psr_enable;
- bool fast_train_support;
bool fast_train_enable;
struct mutex panel_lock; bool panel_is_modeset;
From: zain wang wzz@rock-chips.com
When we enable bridge failed, we have to retry it, otherwise we would get the abnormal display.
Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 65 +++++++++++++++++----- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 +- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 5 +- 3 files changed, 56 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index ea7a80a989c6..c81733b8185e 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -43,8 +43,10 @@ struct bridge_init { struct device_node *node; };
-static void analogix_dp_init_dp(struct analogix_dp_device *dp) +static int analogix_dp_init_dp(struct analogix_dp_device *dp) { + int ret; + analogix_dp_reset(dp);
analogix_dp_swreset(dp); @@ -56,10 +58,13 @@ static void analogix_dp_init_dp(struct analogix_dp_device *dp) analogix_dp_enable_sw_function(dp);
analogix_dp_config_interrupt(dp); - analogix_dp_init_analog_func(dp); + ret = analogix_dp_init_analog_func(dp); + if (ret) + return ret;
analogix_dp_init_hpd(dp); analogix_dp_init_aux(dp); + return 0; }
static int analogix_dp_detect_hpd(struct analogix_dp_device *dp) @@ -918,7 +923,7 @@ static irqreturn_t analogix_dp_irq_thread(int irq, void *arg) return IRQ_HANDLED; }
-static void analogix_dp_commit(struct analogix_dp_device *dp) +static int analogix_dp_commit(struct analogix_dp_device *dp) { int ret;
@@ -928,11 +933,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) DRM_ERROR("failed to disable the panel\n"); }
- ret = readx_poll_timeout(analogix_dp_train_link, dp, ret, !ret, 100, - DP_TIMEOUT_TRAINING_US * 5); + ret = analogix_dp_train_link(dp); if (ret) { dev_err(dp->dev, "unable to do link train, ret=%d\n", ret); - return; + return ret; }
analogix_dp_enable_scramble(dp, 1); @@ -953,6 +957,7 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) dp->psr_enable = analogix_dp_detect_sink_psr(dp); if (dp->psr_enable) analogix_dp_enable_sink_psr(dp); + return 0; }
/* @@ -1149,12 +1154,9 @@ static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge) DRM_ERROR("failed to setup the panel ret = %d\n", ret); }
-static void analogix_dp_bridge_enable(struct drm_bridge *bridge) +static int analogix_dp_set_bridge(struct analogix_dp_device *dp) { - struct analogix_dp_device *dp = bridge->driver_private; - - if (dp->dpms_mode == DRM_MODE_DPMS_ON) - return; + int ret;
pm_runtime_get_sync(dp->dev);
@@ -1162,11 +1164,46 @@ static void analogix_dp_bridge_enable(struct drm_bridge *bridge) dp->plat_data->power_on(dp->plat_data);
phy_power_on(dp->phy); - analogix_dp_init_dp(dp); + + ret = analogix_dp_init_dp(dp); + if (ret) + goto out_dp_init; + + ret = analogix_dp_commit(dp); + if (ret) + goto out_dp_init; + enable_irq(dp->irq); - analogix_dp_commit(dp); + return 0;
- dp->dpms_mode = DRM_MODE_DPMS_ON; +out_dp_init: + phy_power_off(dp->phy); + if (dp->plat_data->power_off) + dp->plat_data->power_off(dp->plat_data); + pm_runtime_put_sync(dp->dev); + + return ret; +} + +static void analogix_dp_bridge_enable(struct drm_bridge *bridge) +{ + struct analogix_dp_device *dp = bridge->driver_private; + int timeout_loop = 0; + + if (dp->dpms_mode == DRM_MODE_DPMS_ON) + return; + + while (timeout_loop < MAX_PLL_LOCK_LOOP) { + if (analogix_dp_set_bridge(dp) == 0) { + dp->dpms_mode = DRM_MODE_DPMS_ON; + return; + } + dev_err(dp->dev, "failed to set bridge, retry: %d\n", + timeout_loop); + timeout_loop++; + usleep_range(10, 11); + } + dev_err(dp->dev, "too many times retry set bridge, give it up\n"); }
static void analogix_dp_bridge_disable(struct drm_bridge *bridge) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index 403ff853464b..769255dc6e99 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -19,6 +19,7 @@ #define DP_TIMEOUT_LOOP_COUNT 100 #define MAX_CR_LOOP 5 #define MAX_EQ_LOOP 5 +#define MAX_PLL_LOCK_LOOP 5
/* Training takes 22ms if AUX channel comm fails. Use this as retry interval */ #define DP_TIMEOUT_TRAINING_US 22000 @@ -197,7 +198,7 @@ void analogix_dp_set_pll_power_down(struct analogix_dp_device *dp, bool enable); void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp, enum analog_power_block block, bool enable); -void analogix_dp_init_analog_func(struct analogix_dp_device *dp); +int analogix_dp_init_analog_func(struct analogix_dp_device *dp); void analogix_dp_init_hpd(struct analogix_dp_device *dp); void analogix_dp_force_hpd(struct analogix_dp_device *dp); enum dp_irq_type analogix_dp_get_irq_type(struct analogix_dp_device *dp); diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index e78c861b9e06..b47c5af43560 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -333,7 +333,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp, } }
-void analogix_dp_init_analog_func(struct analogix_dp_device *dp) +int analogix_dp_init_analog_func(struct analogix_dp_device *dp) { u32 reg; int timeout_loop = 0; @@ -355,7 +355,7 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp) timeout_loop++; if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) { dev_err(dp->dev, "failed to get pll lock status\n"); - return; + return -ETIMEDOUT; } usleep_range(10, 20); } @@ -366,6 +366,7 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp) reg &= ~(SERDES_FIFO_FUNC_EN_N | LS_CLK_DOMAIN_FUNC_EN_N | AUX_FUNC_EN_N); writel(reg, dp->reg_base + ANALOGIX_DP_FUNC_EN_2); + return 0; }
void analogix_dp_clear_hotplug_interrupts(struct analogix_dp_device *dp)
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
When we enable bridge failed, we have to retry it, otherwise we would get the abnormal display.
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 65 +++++++++++++++++----- drivers/gpu/drm/bridge/analogix/analogix_dp_core.h | 3 +- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 5 +- 3 files changed, 56 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index ea7a80a989c6..c81733b8185e 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -43,8 +43,10 @@ struct bridge_init { struct device_node *node; };
-static void analogix_dp_init_dp(struct analogix_dp_device *dp) +static int analogix_dp_init_dp(struct analogix_dp_device *dp) {
int ret;
analogix_dp_reset(dp);
analogix_dp_swreset(dp);
@@ -56,10 +58,13 @@ static void analogix_dp_init_dp(struct analogix_dp_device *dp) analogix_dp_enable_sw_function(dp);
analogix_dp_config_interrupt(dp);
- analogix_dp_init_analog_func(dp);
ret = analogix_dp_init_analog_func(dp);
if (ret)
return ret;analogix_dp_init_hpd(dp); analogix_dp_init_aux(dp);
return 0; }
static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
@@ -918,7 +923,7 @@ static irqreturn_t analogix_dp_irq_thread(int irq, void *arg) return IRQ_HANDLED; }
-static void analogix_dp_commit(struct analogix_dp_device *dp) +static int analogix_dp_commit(struct analogix_dp_device *dp) { int ret;
@@ -928,11 +933,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) DRM_ERROR("failed to disable the panel\n"); }
- ret = readx_poll_timeout(analogix_dp_train_link, dp, ret, !ret, 100,
DP_TIMEOUT_TRAINING_US * 5);
- ret = analogix_dp_train_link(dp); if (ret) { dev_err(dp->dev, "unable to do link train, ret=%d\n", ret);
return;
return ret;}
analogix_dp_enable_scramble(dp, 1);
@@ -953,6 +957,7 @@ static void analogix_dp_commit(struct analogix_dp_device *dp) dp->psr_enable = analogix_dp_detect_sink_psr(dp); if (dp->psr_enable) analogix_dp_enable_sink_psr(dp);
return 0; }
/*
@@ -1149,12 +1154,9 @@ static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge) DRM_ERROR("failed to setup the panel ret = %d\n", ret); }
-static void analogix_dp_bridge_enable(struct drm_bridge *bridge) +static int analogix_dp_set_bridge(struct analogix_dp_device *dp) {
- struct analogix_dp_device *dp = bridge->driver_private;
- if (dp->dpms_mode == DRM_MODE_DPMS_ON)
return;
int ret;
pm_runtime_get_sync(dp->dev);
@@ -1162,11 +1164,46 @@ static void analogix_dp_bridge_enable(struct drm_bridge *bridge) dp->plat_data->power_on(dp->plat_data);
phy_power_on(dp->phy);
- analogix_dp_init_dp(dp);
- ret = analogix_dp_init_dp(dp);
- if (ret)
goto out_dp_init;- ret = analogix_dp_commit(dp);
- if (ret)
goto out_dp_init;- enable_irq(dp->irq);
- analogix_dp_commit(dp);
- return 0;
- dp->dpms_mode = DRM_MODE_DPMS_ON;
+out_dp_init:
- phy_power_off(dp->phy);
- if (dp->plat_data->power_off)
dp->plat_data->power_off(dp->plat_data);- pm_runtime_put_sync(dp->dev);
- return ret;
+}
+static void analogix_dp_bridge_enable(struct drm_bridge *bridge) +{
struct analogix_dp_device *dp = bridge->driver_private;
int timeout_loop = 0;
if (dp->dpms_mode == DRM_MODE_DPMS_ON)
return;while (timeout_loop < MAX_PLL_LOCK_LOOP) {
if (analogix_dp_set_bridge(dp) == 0) {dp->dpms_mode = DRM_MODE_DPMS_ON;return;}dev_err(dp->dev, "failed to set bridge, retry: %d\n",timeout_loop);timeout_loop++;usleep_range(10, 11);}
dev_err(dp->dev, "too many times retry set bridge, give it up\n"); }
static void analogix_dp_bridge_disable(struct drm_bridge *bridge)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index 403ff853464b..769255dc6e99 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -19,6 +19,7 @@ #define DP_TIMEOUT_LOOP_COUNT 100 #define MAX_CR_LOOP 5 #define MAX_EQ_LOOP 5 +#define MAX_PLL_LOCK_LOOP 5
/* Training takes 22ms if AUX channel comm fails. Use this as retry interval */ #define DP_TIMEOUT_TRAINING_US 22000 @@ -197,7 +198,7 @@ void analogix_dp_set_pll_power_down(struct analogix_dp_device *dp, bool enable); void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp, enum analog_power_block block, bool enable); -void analogix_dp_init_analog_func(struct analogix_dp_device *dp); +int analogix_dp_init_analog_func(struct analogix_dp_device *dp); void analogix_dp_init_hpd(struct analogix_dp_device *dp); void analogix_dp_force_hpd(struct analogix_dp_device *dp); enum dp_irq_type analogix_dp_get_irq_type(struct analogix_dp_device *dp); diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index e78c861b9e06..b47c5af43560 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -333,7 +333,7 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp, } }
-void analogix_dp_init_analog_func(struct analogix_dp_device *dp) +int analogix_dp_init_analog_func(struct analogix_dp_device *dp) { u32 reg; int timeout_loop = 0; @@ -355,7 +355,7 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp) timeout_loop++; if (DP_TIMEOUT_LOOP_COUNT < timeout_loop) { dev_err(dp->dev, "failed to get pll lock status\n");
return;
return -ETIMEDOUT; } usleep_range(10, 20);@@ -366,6 +366,7 @@ void analogix_dp_init_analog_func(struct analogix_dp_device *dp) reg &= ~(SERDES_FIFO_FUNC_EN_N | LS_CLK_DOMAIN_FUNC_EN_N | AUX_FUNC_EN_N); writel(reg, dp->reg_base + ANALOGIX_DP_FUNC_EN_2);
return 0; }
void analogix_dp_clear_hotplug_interrupts(struct analogix_dp_device *dp)
From: zain wang wzz@rock-chips.com
According to DP spec v1.3 chap 3.5.1.2 Link Training, Link Policy Maker must first detect that the HPD signal is asserted high by the Downstream Device before establishing a link with it.
Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index c81733b8185e..92fb9a072cb6 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1169,6 +1169,17 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp) if (ret) goto out_dp_init;
+ /* + * According to DP spec v1.3 chap 3.5.1.2 Link Training, + * We should first make sure the HPD signal is asserted high by device + * when we want to establish a link with it. + */ + ret = analogix_dp_detect_hpd(dp); + if (ret) { + DRM_ERROR("failed to get hpd single ret = %d\n", ret); + goto out_dp_init; + } + ret = analogix_dp_commit(dp); if (ret) goto out_dp_init;
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
According to DP spec v1.3 chap 3.5.1.2 Link Training, Link Policy Maker must first detect that the HPD signal is asserted high by the Downstream Device before establishing a link with it.
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index c81733b8185e..92fb9a072cb6 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1169,6 +1169,17 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp) if (ret) goto out_dp_init;
- /*
* According to DP spec v1.3 chap 3.5.1.2 Link Training,* We should first make sure the HPD signal is asserted high by device* when we want to establish a link with it.*/- ret = analogix_dp_detect_hpd(dp);
- if (ret) {
DRM_ERROR("failed to get hpd single ret = %d\n", ret);goto out_dp_init;- }
- ret = analogix_dp_commit(dp); if (ret) goto out_dp_init;
From: zain wang wzz@rock-chips.com
Following the correct power up sequence: dp_pd=ff => dp_pd=7f => wait 10us => dp_pd=00
Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 10 ++++++++-- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index b47c5af43560..bb72f8b0e603 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -321,10 +321,16 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp, break; case POWER_ALL: if (enable) { - reg = DP_PHY_PD | AUX_PD | CH3_PD | CH2_PD | - CH1_PD | CH0_PD; + reg = DP_ALL_PD; writel(reg, dp->reg_base + phy_pd_addr); } else { + reg = DP_ALL_PD; + writel(reg, dp->reg_base + phy_pd_addr); + usleep_range(10, 15); + reg &= ~DP_INC_BG; + writel(reg, dp->reg_base + phy_pd_addr); + usleep_range(10, 15); + writel(0x00, dp->reg_base + phy_pd_addr); } break; diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h index 40200c652533..9602668669f4 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h @@ -342,12 +342,15 @@ #define DP_PLL_REF_BIT_1_2500V (0x7 << 0)
/* ANALOGIX_DP_PHY_PD */ +#define DP_INC_BG (0x1 << 7) +#define DP_EXP_BG (0x1 << 6) #define DP_PHY_PD (0x1 << 5) #define AUX_PD (0x1 << 4) #define CH3_PD (0x1 << 3) #define CH2_PD (0x1 << 2) #define CH1_PD (0x1 << 1) #define CH0_PD (0x1 << 0) +#define DP_ALL_PD (0xff)
/* ANALOGIX_DP_PHY_TEST */ #define MACRO_RST (0x1 << 5)
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
Following the correct power up sequence: dp_pd=ff => dp_pd=7f => wait 10us => dp_pd=00
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 10 ++++++++-- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index b47c5af43560..bb72f8b0e603 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -321,10 +321,16 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp, break; case POWER_ALL: if (enable) {
reg = DP_PHY_PD | AUX_PD | CH3_PD | CH2_PD |CH1_PD | CH0_PD;
reg = DP_ALL_PD; writel(reg, dp->reg_base + phy_pd_addr);reg = DP_ALL_PD;writel(reg, dp->reg_base + phy_pd_addr);usleep_range(10, 15);reg &= ~DP_INC_BG;writel(reg, dp->reg_base + phy_pd_addr);usleep_range(10, 15);writel(0x00, dp->reg_base + phy_pd_addr);diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h index 40200c652533..9602668669f4 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h @@ -342,12 +342,15 @@ #define DP_PLL_REF_BIT_1_2500V (0x7 << 0)
/* ANALOGIX_DP_PHY_PD */ +#define DP_INC_BG (0x1 << 7) +#define DP_EXP_BG (0x1 << 6) #define DP_PHY_PD (0x1 << 5) #define AUX_PD (0x1 << 4) #define CH3_PD (0x1 << 3) #define CH2_PD (0x1 << 2) #define CH1_PD (0x1 << 1) #define CH0_PD (0x1 << 0) +#define DP_ALL_PD (0xff)
/* ANALOGIX_DP_PHY_TEST */ #define MACRO_RST (0x1 << 5)
From: Lin Huang hl@rock-chips.com
When panel is shut down, we should make sure edp can be disabled to avoid undefined behavior.
Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 92fb9a072cb6..3a222e7e46ee 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1160,6 +1160,12 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
pm_runtime_get_sync(dp->dev);
+ ret = clk_prepare_enable(dp->clock); + if (ret < 0) { + DRM_ERROR("Failed to prepare_enable the clock clk [%d]\n", ret); + goto out_dp_clk_pre; + } + if (dp->plat_data->power_on) dp->plat_data->power_on(dp->plat_data);
@@ -1191,6 +1197,8 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp) phy_power_off(dp->phy); if (dp->plat_data->power_off) dp->plat_data->power_off(dp->plat_data); + clk_disable_unprepare(dp->clock); +out_dp_clk_pre: pm_runtime_put_sync(dp->dev);
return ret; @@ -1233,11 +1241,14 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge) }
disable_irq(dp->irq); + analogix_dp_set_analog_power_down(dp, POWER_ALL, 1); phy_power_off(dp->phy);
if (dp->plat_data->power_off) dp->plat_data->power_off(dp->plat_data);
+ clk_disable_unprepare(dp->clock); + pm_runtime_put_sync(dp->dev);
ret = analogix_dp_prepare_panel(dp, false, true);
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: Lin Huang hl@rock-chips.com
When panel is shut down, we should make sure edp can be disabled to avoid undefined behavior.
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Stéphane Marchesin marcheu@chromium.org Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 92fb9a072cb6..3a222e7e46ee 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1160,6 +1160,12 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp)
pm_runtime_get_sync(dp->dev);
- ret = clk_prepare_enable(dp->clock);
- if (ret < 0) {
DRM_ERROR("Failed to prepare_enable the clock clk [%d]\n", ret);goto out_dp_clk_pre;- }
- if (dp->plat_data->power_on) dp->plat_data->power_on(dp->plat_data);
@@ -1191,6 +1197,8 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp) phy_power_off(dp->phy); if (dp->plat_data->power_off) dp->plat_data->power_off(dp->plat_data);
- clk_disable_unprepare(dp->clock);
+out_dp_clk_pre: pm_runtime_put_sync(dp->dev);
return ret; @@ -1233,11 +1241,14 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge) }
disable_irq(dp->irq);
analogix_dp_set_analog_power_down(dp, POWER_ALL, 1); phy_power_off(dp->phy);
if (dp->plat_data->power_off) dp->plat_data->power_off(dp->plat_data);
clk_disable_unprepare(dp->clock);
pm_runtime_put_sync(dp->dev);
ret = analogix_dp_prepare_panel(dp, false, true);
From: Lin Huang hl@rock-chips.com
There was a 1ms delay to detect the hpd signal, which is too short to detect a short pulse. This patch extends this delay to 100ms.
Cc: Stéphane Marchesin marcheu@chromium.org Cc: 征增 王 wzz@rock-chips.com Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 3a222e7e46ee..6cbde8473f58 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -76,7 +76,7 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp) return 0;
timeout_loop++; - usleep_range(10, 11); + usleep_range(1000, 1100); }
/*
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: Lin Huang hl@rock-chips.com
There was a 1ms delay to detect the hpd signal, which is too short to detect a short pulse. This patch extends this delay to 100ms.
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Stéphane Marchesin marcheu@chromium.org Cc: 征增 王 wzz@rock-chips.com Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 3a222e7e46ee..6cbde8473f58 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -76,7 +76,7 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp) return 0;
timeout_loop++;
usleep_range(10, 11);
usleep_range(1000, 1100);}
/*
From: zain wang wzz@rock-chips.com
Enhanced mode is required by the eDP 1.2 specification, and not doing it early could result in a period of time where we have a link transmitting idle packets without it. Since there is no reason to disable it, we just enable it at the beginning of link training and then keep it on all the time.
Cc: Tomasz Figa tfiga@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 6cbde8473f58..1eed35f9eb8d 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -281,6 +281,8 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp) retval = drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, buf, 2); if (retval < 0) return retval; + /* set enhanced mode if available */ + analogix_dp_set_enhanced_mode(dp);
/* Set TX pre-emphasis to minimum */ for (lane = 0; lane < lane_count; lane++) @@ -593,8 +595,6 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) dev_dbg(dp->dev, "fast link training %s\n", dp->fast_train_enable ? "supported" : "unsupported");
- /* set enhanced mode if available */ - analogix_dp_set_enhanced_mode(dp); dp->link_train.lt_state = FINISHED;
return 0; @@ -940,8 +940,6 @@ static int analogix_dp_commit(struct analogix_dp_device *dp) }
analogix_dp_enable_scramble(dp, 1); - analogix_dp_enable_rx_to_enhanced_mode(dp, 1); - analogix_dp_enable_enhanced_mode(dp, 1);
analogix_dp_init_video(dp); ret = analogix_dp_config_video(dp);
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
Enhanced mode is required by the eDP 1.2 specification, and not doing it early could result in a period of time where we have a link transmitting idle packets without it. Since there is no reason to disable it, we just enable it at the beginning of link training and then keep it on all the time.
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Tomasz Figa tfiga@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 6cbde8473f58..1eed35f9eb8d 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -281,6 +281,8 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp) retval = drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, buf, 2); if (retval < 0) return retval;
/* set enhanced mode if available */
analogix_dp_set_enhanced_mode(dp);
/* Set TX pre-emphasis to minimum */ for (lane = 0; lane < lane_count; lane++)
@@ -593,8 +595,6 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) dev_dbg(dp->dev, "fast link training %s\n", dp->fast_train_enable ? "supported" : "unsupported");
/* set enhanced mode if available */analogix_dp_set_enhanced_mode(dp);dp->link_train.lt_state = FINISHED;
return 0;
@@ -940,8 +940,6 @@ static int analogix_dp_commit(struct analogix_dp_device *dp) }
analogix_dp_enable_scramble(dp, 1);
analogix_dp_enable_rx_to_enhanced_mode(dp, 1);
analogix_dp_enable_enhanced_mode(dp, 1);
analogix_dp_init_video(dp); ret = analogix_dp_config_video(dp);
From: Lin Huang hl@rock-chips.com
We need to check the dpcd write/read return value to see whether the write/read was successful
Cc: Kristian H. Kristensen hoegsberg@chromium.org Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 169 ++++++++++++++++----- 1 file changed, 127 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 1eed35f9eb8d..be6eddd0d0a7 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -160,80 +160,137 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp) } EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
-static bool analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) +static int analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) { unsigned char psr_version; + int ret; + + ret = drm_dp_dpcd_readb(&dp->aux, DP_PSR_SUPPORT, &psr_version); + if (ret != 1) { + dev_err(dp->dev, "failed to get PSR version, disable it\n"); + return ret; + }
- drm_dp_dpcd_readb(&dp->aux, DP_PSR_SUPPORT, &psr_version); dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version);
- return (psr_version & DP_PSR_IS_SUPPORTED) ? true : false; + dp->psr_enable = (psr_version & DP_PSR_IS_SUPPORTED) ? true : false; + + return 0; }
-static void analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) +static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) { unsigned char psr_en; + int ret;
/* Disable psr function */ - drm_dp_dpcd_readb(&dp->aux, DP_PSR_EN_CFG, &psr_en); + ret = drm_dp_dpcd_readb(&dp->aux, DP_PSR_EN_CFG, &psr_en); + if (ret != 1) { + dev_err(dp->dev, "failed to get psr config\n"); + goto end; + } + psr_en &= ~DP_PSR_ENABLE; - drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en); + ret = drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en); + if (ret != 1) { + dev_err(dp->dev, "failed to disable panel psr\n"); + goto end; + }
/* Main-Link transmitter remains active during PSR active states */ psr_en = DP_PSR_MAIN_LINK_ACTIVE | DP_PSR_CRC_VERIFICATION; - drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en); + ret = drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en); + if (ret != 1) { + dev_err(dp->dev, "failed to set panel psr\n"); + goto end; + }
/* Enable psr function */ psr_en = DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE | DP_PSR_CRC_VERIFICATION; - drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en); + ret = drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en); + if (ret != 1) { + dev_err(dp->dev, "failed to set panel psr\n"); + goto end; + }
analogix_dp_enable_psr_crc(dp); + + return 0; +end: + dev_err(dp->dev, "enable psr fail, force to disable psr\n"); + dp->psr_enable = false; + + return ret; }
-static void +static int analogix_dp_enable_rx_to_enhanced_mode(struct analogix_dp_device *dp, bool enable) { u8 data; + int ret;
- drm_dp_dpcd_readb(&dp->aux, DP_LANE_COUNT_SET, &data); + ret = drm_dp_dpcd_readb(&dp->aux, DP_LANE_COUNT_SET, &data); + if (ret != 1) + return ret;
if (enable) - drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET, - DP_LANE_COUNT_ENHANCED_FRAME_EN | - DPCD_LANE_COUNT_SET(data)); + ret = drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET, + DP_LANE_COUNT_ENHANCED_FRAME_EN | + DPCD_LANE_COUNT_SET(data)); else - drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET, - DPCD_LANE_COUNT_SET(data)); + ret = drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET, + DPCD_LANE_COUNT_SET(data)); + + return ret < 0 ? ret : 0; }
-static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp) +static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp, + u8 *enhanced_mode_support) { u8 data; - int retval; + int ret;
- drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data); - retval = DPCD_ENHANCED_FRAME_CAP(data); + ret = drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data); + if (ret != 1) { + *enhanced_mode_support = 0; + return ret; + }
- return retval; + *enhanced_mode_support = DPCD_ENHANCED_FRAME_CAP(data); + + return 0; }
-static void analogix_dp_set_enhanced_mode(struct analogix_dp_device *dp) +static int analogix_dp_set_enhanced_mode(struct analogix_dp_device *dp) { u8 data; + int ret; + + ret = analogix_dp_is_enhanced_mode_available(dp, &data); + if (ret < 0) + return ret; + + ret = analogix_dp_enable_rx_to_enhanced_mode(dp, data); + if (ret < 0) + return ret;
- data = analogix_dp_is_enhanced_mode_available(dp); - analogix_dp_enable_rx_to_enhanced_mode(dp, data); analogix_dp_enable_enhanced_mode(dp, data); + + return 0; }
-static void analogix_dp_training_pattern_dis(struct analogix_dp_device *dp) +static int analogix_dp_training_pattern_dis(struct analogix_dp_device *dp) { + int ret; + analogix_dp_set_training_pattern(dp, DP_NONE);
- drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, - DP_TRAINING_PATTERN_DISABLE); + ret = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, + DP_TRAINING_PATTERN_DISABLE); + + return ret < 0 ? ret : 0; }
static void @@ -282,7 +339,11 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp) if (retval < 0) return retval; /* set enhanced mode if available */ - analogix_dp_set_enhanced_mode(dp); + retval = analogix_dp_set_enhanced_mode(dp); + if (retval < 0) { + dev_err(dp->dev, "failed to set enhance mode\n"); + return retval; + }
/* Set TX pre-emphasis to minimum */ for (lane = 0; lane < lane_count; lane++) @@ -567,10 +628,11 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
if (!analogix_dp_channel_eq_ok(link_status, link_align, lane_count)) { /* traing pattern Set to Normal */ - analogix_dp_training_pattern_dis(dp); + retval = analogix_dp_training_pattern_dis(dp); + if (retval < 0) + return retval;
dev_info(dp->dev, "Link Training success!\n"); - analogix_dp_get_link_bandwidth(dp, ®); dp->link_train.link_rate = reg; dev_dbg(dp->dev, "final bandwidth = %.2x\n", @@ -867,24 +929,32 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp) return 0; }
-static void analogix_dp_enable_scramble(struct analogix_dp_device *dp, - bool enable) +static int analogix_dp_enable_scramble(struct analogix_dp_device *dp, + bool enable) { u8 data; + int ret;
if (enable) { analogix_dp_enable_scrambling(dp);
- drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data); - drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, + ret = drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, + &data); + if (ret != 1) + return ret; + ret = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, (u8)(data & ~DP_LINK_SCRAMBLING_DISABLE)); } else { analogix_dp_disable_scrambling(dp);
- drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data); - drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, + ret = drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, + &data); + if (ret != 1) + return ret; + ret = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, (u8)(data | DP_LINK_SCRAMBLING_DISABLE)); } + return ret < 0 ? ret : 0; }
static irqreturn_t analogix_dp_hardirq(int irq, void *arg) @@ -939,23 +1009,36 @@ static int analogix_dp_commit(struct analogix_dp_device *dp) return ret; }
- analogix_dp_enable_scramble(dp, 1); + ret = analogix_dp_enable_scramble(dp, 1); + if (ret < 0) { + dev_err(dp->dev, "can not enable scramble\n"); + return ret; + }
analogix_dp_init_video(dp); ret = analogix_dp_config_video(dp); - if (ret) + if (ret) { dev_err(dp->dev, "unable to config video\n"); + return ret; + }
/* Safe to enable the panel now */ if (dp->plat_data->panel) { - if (drm_panel_enable(dp->plat_data->panel)) + ret = drm_panel_enable(dp->plat_data->panel); + if (ret) { DRM_ERROR("failed to enable the panel\n"); + return ret; + } }
- dp->psr_enable = analogix_dp_detect_sink_psr(dp); + ret = analogix_dp_detect_sink_psr(dp); + if (ret) + return ret; + if (dp->psr_enable) - analogix_dp_enable_sink_psr(dp); - return 0; + ret = analogix_dp_enable_sink_psr(dp); + + return ret; }
/* @@ -1185,8 +1268,10 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp) }
ret = analogix_dp_commit(dp); - if (ret) + if (ret) { + DRM_ERROR("dp commit error, ret = %d\n", ret); goto out_dp_init; + }
enable_irq(dp->irq); return 0;
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: Lin Huang hl@rock-chips.com
We need to check the dpcd write/read return value to see whether the write/read was successful
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Kristian H. Kristensen hoegsberg@chromium.org Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 169 ++++++++++++++++----- 1 file changed, 127 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 1eed35f9eb8d..be6eddd0d0a7 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -160,80 +160,137 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp) } EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
-static bool analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) +static int analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) { unsigned char psr_version;
- int ret;
- ret = drm_dp_dpcd_readb(&dp->aux, DP_PSR_SUPPORT, &psr_version);
- if (ret != 1) {
dev_err(dp->dev, "failed to get PSR version, disable it\n");return ret;- }
drm_dp_dpcd_readb(&dp->aux, DP_PSR_SUPPORT, &psr_version); dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version);
return (psr_version & DP_PSR_IS_SUPPORTED) ? true : false;
- dp->psr_enable = (psr_version & DP_PSR_IS_SUPPORTED) ? true : false;
- return 0; }
-static void analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) +static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) { unsigned char psr_en;
int ret;
/* Disable psr function */
- drm_dp_dpcd_readb(&dp->aux, DP_PSR_EN_CFG, &psr_en);
- ret = drm_dp_dpcd_readb(&dp->aux, DP_PSR_EN_CFG, &psr_en);
- if (ret != 1) {
dev_err(dp->dev, "failed to get psr config\n");goto end;- }
- psr_en &= ~DP_PSR_ENABLE;
- drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en);
ret = drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en);
if (ret != 1) {
dev_err(dp->dev, "failed to disable panel psr\n");goto end;}
/* Main-Link transmitter remains active during PSR active states */ psr_en = DP_PSR_MAIN_LINK_ACTIVE | DP_PSR_CRC_VERIFICATION;
- drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en);
ret = drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en);
if (ret != 1) {
dev_err(dp->dev, "failed to set panel psr\n");goto end;}
/* Enable psr function */ psr_en = DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE | DP_PSR_CRC_VERIFICATION;
- drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en);
ret = drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en);
if (ret != 1) {
dev_err(dp->dev, "failed to set panel psr\n");goto end;}
analogix_dp_enable_psr_crc(dp);
return 0;
+end:
- dev_err(dp->dev, "enable psr fail, force to disable psr\n");
- dp->psr_enable = false;
- return ret; }
-static void +static int analogix_dp_enable_rx_to_enhanced_mode(struct analogix_dp_device *dp, bool enable) { u8 data;
- int ret;
- drm_dp_dpcd_readb(&dp->aux, DP_LANE_COUNT_SET, &data);
ret = drm_dp_dpcd_readb(&dp->aux, DP_LANE_COUNT_SET, &data);
if (ret != 1)
return ret;if (enable)
drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET,DP_LANE_COUNT_ENHANCED_FRAME_EN |DPCD_LANE_COUNT_SET(data));
ret = drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET,DP_LANE_COUNT_ENHANCED_FRAME_EN |DPCD_LANE_COUNT_SET(data));
drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET,DPCD_LANE_COUNT_SET(data));
ret = drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET,DPCD_LANE_COUNT_SET(data));- return ret < 0 ? ret : 0; }
-static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp) +static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp,
u8 *enhanced_mode_support)
- int retval;
- int ret;
- drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data);
- retval = DPCD_ENHANCED_FRAME_CAP(data);
- ret = drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data);
- if (ret != 1) {
*enhanced_mode_support = 0;return ret;- }
- return retval;
- *enhanced_mode_support = DPCD_ENHANCED_FRAME_CAP(data);
- return 0; }
-static void analogix_dp_set_enhanced_mode(struct analogix_dp_device *dp) +static int analogix_dp_set_enhanced_mode(struct analogix_dp_device *dp) { u8 data;
- int ret;
- ret = analogix_dp_is_enhanced_mode_available(dp, &data);
- if (ret < 0)
return ret;- ret = analogix_dp_enable_rx_to_enhanced_mode(dp, data);
- if (ret < 0)
return ret;
- data = analogix_dp_is_enhanced_mode_available(dp);
- analogix_dp_enable_rx_to_enhanced_mode(dp, data); analogix_dp_enable_enhanced_mode(dp, data);
- return 0; }
-static void analogix_dp_training_pattern_dis(struct analogix_dp_device *dp) +static int analogix_dp_training_pattern_dis(struct analogix_dp_device *dp) {
- int ret;
- analogix_dp_set_training_pattern(dp, DP_NONE);
- drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
DP_TRAINING_PATTERN_DISABLE);
ret = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
DP_TRAINING_PATTERN_DISABLE);return ret < 0 ? ret : 0; }
static void
@@ -282,7 +339,11 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp) if (retval < 0) return retval; /* set enhanced mode if available */
- analogix_dp_set_enhanced_mode(dp);
retval = analogix_dp_set_enhanced_mode(dp);
if (retval < 0) {
dev_err(dp->dev, "failed to set enhance mode\n");return retval;}
/* Set TX pre-emphasis to minimum */ for (lane = 0; lane < lane_count; lane++)
@@ -567,10 +628,11 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp)
if (!analogix_dp_channel_eq_ok(link_status, link_align, lane_count)) { /* traing pattern Set to Normal */
analogix_dp_training_pattern_dis(dp);
retval = analogix_dp_training_pattern_dis(dp);if (retval < 0)return retval;dev_info(dp->dev, "Link Training success!\n");
- analogix_dp_get_link_bandwidth(dp, ®); dp->link_train.link_rate = reg; dev_dbg(dp->dev, "final bandwidth = %.2x\n",
@@ -867,24 +929,32 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp) return 0; }
-static void analogix_dp_enable_scramble(struct analogix_dp_device *dp,
bool enable)+static int analogix_dp_enable_scramble(struct analogix_dp_device *dp,
bool enable){ u8 data;
int ret;
if (enable) { analogix_dp_enable_scrambling(dp);
drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data);drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
ret = drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET,&data);if (ret != 1)return ret;ret = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, (u8)(data & ~DP_LINK_SCRAMBLING_DISABLE));
drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data);drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
ret = drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET,&data);if (ret != 1)return ret;ret = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, (u8)(data | DP_LINK_SCRAMBLING_DISABLE));}
return ret < 0 ? ret : 0; }
static irqreturn_t analogix_dp_hardirq(int irq, void *arg)
@@ -939,23 +1009,36 @@ static int analogix_dp_commit(struct analogix_dp_device *dp) return ret; }
- analogix_dp_enable_scramble(dp, 1);
ret = analogix_dp_enable_scramble(dp, 1);
if (ret < 0) {
dev_err(dp->dev, "can not enable scramble\n");return ret;}
analogix_dp_init_video(dp); ret = analogix_dp_config_video(dp);
- if (ret)
if (ret) { dev_err(dp->dev, "unable to config video\n");
return ret;}
/* Safe to enable the panel now */ if (dp->plat_data->panel) {
if (drm_panel_enable(dp->plat_data->panel))
ret = drm_panel_enable(dp->plat_data->panel);if (ret) { DRM_ERROR("failed to enable the panel\n");return ret;}
- dp->psr_enable = analogix_dp_detect_sink_psr(dp);
- ret = analogix_dp_detect_sink_psr(dp);
- if (ret)
return ret;- if (dp->psr_enable)
analogix_dp_enable_sink_psr(dp);- return 0;
ret = analogix_dp_enable_sink_psr(dp);return ret; }
/*
@@ -1185,8 +1268,10 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp) }
ret = analogix_dp_commit(dp);
- if (ret)
if (ret) {
DRM_ERROR("dp commit error, ret = %d\n", ret);goto out_dp_init;
}
enable_irq(dp->irq); return 0;
From: zain wang wzz@rock-chips.com
There are some different bits between Rockchip and Exynos in register "AUX_PD". This patch fixes the incorrect operations about it.
Cc: Douglas Anderson dianders@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 117 ++++++++++++---------- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 2 + 2 files changed, 65 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index bb72f8b0e603..dee1ba109b5f 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -248,76 +248,85 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp, { u32 reg; u32 phy_pd_addr = ANALOGIX_DP_PHY_PD; + u32 mask;
if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) phy_pd_addr = ANALOGIX_DP_PD;
switch (block) { case AUX_BLOCK: - if (enable) { - reg = readl(dp->reg_base + phy_pd_addr); - reg |= AUX_PD; - writel(reg, dp->reg_base + phy_pd_addr); - } else { - reg = readl(dp->reg_base + phy_pd_addr); - reg &= ~AUX_PD; - writel(reg, dp->reg_base + phy_pd_addr); - } + if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) + mask = RK_AUX_PD; + else + mask = AUX_PD; + + reg = readl(dp->reg_base + phy_pd_addr); + if (enable) + reg |= mask; + else + reg &= ~mask; + writel(reg, dp->reg_base + phy_pd_addr); break; case CH0_BLOCK: - if (enable) { - reg = readl(dp->reg_base + phy_pd_addr); - reg |= CH0_PD; - writel(reg, dp->reg_base + phy_pd_addr); - } else { - reg = readl(dp->reg_base + phy_pd_addr); - reg &= ~CH0_PD; - writel(reg, dp->reg_base + phy_pd_addr); - } + mask = CH0_PD; + reg = readl(dp->reg_base + phy_pd_addr); + + if (enable) + reg |= mask; + else + reg &= ~mask; + writel(reg, dp->reg_base + phy_pd_addr); break; case CH1_BLOCK: - if (enable) { - reg = readl(dp->reg_base + phy_pd_addr); - reg |= CH1_PD; - writel(reg, dp->reg_base + phy_pd_addr); - } else { - reg = readl(dp->reg_base + phy_pd_addr); - reg &= ~CH1_PD; - writel(reg, dp->reg_base + phy_pd_addr); - } + mask = CH1_PD; + reg = readl(dp->reg_base + phy_pd_addr); + + if (enable) + reg |= mask; + else + reg &= ~mask; + writel(reg, dp->reg_base + phy_pd_addr); break; case CH2_BLOCK: - if (enable) { - reg = readl(dp->reg_base + phy_pd_addr); - reg |= CH2_PD; - writel(reg, dp->reg_base + phy_pd_addr); - } else { - reg = readl(dp->reg_base + phy_pd_addr); - reg &= ~CH2_PD; - writel(reg, dp->reg_base + phy_pd_addr); - } + mask = CH2_PD; + reg = readl(dp->reg_base + phy_pd_addr); + + if (enable) + reg |= mask; + else + reg &= ~mask; + writel(reg, dp->reg_base + phy_pd_addr); break; case CH3_BLOCK: - if (enable) { - reg = readl(dp->reg_base + phy_pd_addr); - reg |= CH3_PD; - writel(reg, dp->reg_base + phy_pd_addr); - } else { - reg = readl(dp->reg_base + phy_pd_addr); - reg &= ~CH3_PD; - writel(reg, dp->reg_base + phy_pd_addr); - } + mask = CH3_PD; + reg = readl(dp->reg_base + phy_pd_addr); + + if (enable) + reg |= mask; + else + reg &= ~mask; + writel(reg, dp->reg_base + phy_pd_addr); break; case ANALOG_TOTAL: - if (enable) { - reg = readl(dp->reg_base + phy_pd_addr); - reg |= DP_PHY_PD; - writel(reg, dp->reg_base + phy_pd_addr); - } else { - reg = readl(dp->reg_base + phy_pd_addr); - reg &= ~DP_PHY_PD; - writel(reg, dp->reg_base + phy_pd_addr); - } + /* + * There is no bit named DP_PHY_PD, so We used DP_INC_BG + * to power off everything instead of DP_PHY_PD in + * Rockchip + */ + if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) + mask = DP_INC_BG; + else + mask = DP_PHY_PD; + + reg = readl(dp->reg_base + phy_pd_addr); + if (enable) + reg |= mask; + else + reg &= ~mask; + + writel(reg, dp->reg_base + phy_pd_addr); + if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) + usleep_range(10, 15); break; case POWER_ALL: if (enable) { diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h index 9602668669f4..b633a4a5082a 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h @@ -345,7 +345,9 @@ #define DP_INC_BG (0x1 << 7) #define DP_EXP_BG (0x1 << 6) #define DP_PHY_PD (0x1 << 5) +#define RK_AUX_PD (0x1 << 5) #define AUX_PD (0x1 << 4) +#define RK_PLL_PD (0x1 << 4) #define CH3_PD (0x1 << 3) #define CH2_PD (0x1 << 2) #define CH1_PD (0x1 << 1)
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
There are some different bits between Rockchip and Exynos in register "AUX_PD". This patch fixes the incorrect operations about it.
You mean the register ANALOGIX_DP_PHY_PD/ANALOGIX_DP_PD, right? AUX_PD sounds like just one of the fields of the register.
With that,
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Douglas Anderson dianders@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 117 ++++++++++++---------- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 2 + 2 files changed, 65 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index bb72f8b0e603..dee1ba109b5f 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -248,76 +248,85 @@ void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp, { u32 reg; u32 phy_pd_addr = ANALOGIX_DP_PHY_PD;
u32 mask;
if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) phy_pd_addr = ANALOGIX_DP_PD;
switch (block) { case AUX_BLOCK:
if (enable) {reg = readl(dp->reg_base + phy_pd_addr);reg |= AUX_PD;writel(reg, dp->reg_base + phy_pd_addr);} else {reg = readl(dp->reg_base + phy_pd_addr);reg &= ~AUX_PD;writel(reg, dp->reg_base + phy_pd_addr);}
if (dp->plat_data && is_rockchip(dp->plat_data->dev_type))mask = RK_AUX_PD;elsemask = AUX_PD;reg = readl(dp->reg_base + phy_pd_addr);if (enable)reg |= mask;elsereg &= ~mask;writel(reg, dp->reg_base + phy_pd_addr);
if (enable) {reg = readl(dp->reg_base + phy_pd_addr);reg |= CH0_PD;writel(reg, dp->reg_base + phy_pd_addr);} else {reg = readl(dp->reg_base + phy_pd_addr);reg &= ~CH0_PD;writel(reg, dp->reg_base + phy_pd_addr);}
mask = CH0_PD;reg = readl(dp->reg_base + phy_pd_addr);if (enable)reg |= mask;elsereg &= ~mask;writel(reg, dp->reg_base + phy_pd_addr);
if (enable) {reg = readl(dp->reg_base + phy_pd_addr);reg |= CH1_PD;writel(reg, dp->reg_base + phy_pd_addr);} else {reg = readl(dp->reg_base + phy_pd_addr);reg &= ~CH1_PD;writel(reg, dp->reg_base + phy_pd_addr);}
mask = CH1_PD;reg = readl(dp->reg_base + phy_pd_addr);if (enable)reg |= mask;elsereg &= ~mask;writel(reg, dp->reg_base + phy_pd_addr);
if (enable) {reg = readl(dp->reg_base + phy_pd_addr);reg |= CH2_PD;writel(reg, dp->reg_base + phy_pd_addr);} else {reg = readl(dp->reg_base + phy_pd_addr);reg &= ~CH2_PD;writel(reg, dp->reg_base + phy_pd_addr);}
mask = CH2_PD;reg = readl(dp->reg_base + phy_pd_addr);if (enable)reg |= mask;elsereg &= ~mask;writel(reg, dp->reg_base + phy_pd_addr);
if (enable) {reg = readl(dp->reg_base + phy_pd_addr);reg |= CH3_PD;writel(reg, dp->reg_base + phy_pd_addr);} else {reg = readl(dp->reg_base + phy_pd_addr);reg &= ~CH3_PD;writel(reg, dp->reg_base + phy_pd_addr);}
mask = CH3_PD;reg = readl(dp->reg_base + phy_pd_addr);if (enable)reg |= mask;elsereg &= ~mask;writel(reg, dp->reg_base + phy_pd_addr);
if (enable) {reg = readl(dp->reg_base + phy_pd_addr);reg |= DP_PHY_PD;writel(reg, dp->reg_base + phy_pd_addr);} else {reg = readl(dp->reg_base + phy_pd_addr);reg &= ~DP_PHY_PD;writel(reg, dp->reg_base + phy_pd_addr);}
/** There is no bit named DP_PHY_PD, so We used DP_INC_BG* to power off everything instead of DP_PHY_PD in* Rockchip*/if (dp->plat_data && is_rockchip(dp->plat_data->dev_type))mask = DP_INC_BG;elsemask = DP_PHY_PD;reg = readl(dp->reg_base + phy_pd_addr);if (enable)reg |= mask;elsereg &= ~mask;writel(reg, dp->reg_base + phy_pd_addr);if (dp->plat_data && is_rockchip(dp->plat_data->dev_type))usleep_range(10, 15);diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h index 9602668669f4..b633a4a5082a 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h @@ -345,7 +345,9 @@ #define DP_INC_BG (0x1 << 7) #define DP_EXP_BG (0x1 << 6) #define DP_PHY_PD (0x1 << 5) +#define RK_AUX_PD (0x1 << 5) #define AUX_PD (0x1 << 4) +#define RK_PLL_PD (0x1 << 4) #define CH3_PD (0x1 << 3) #define CH2_PD (0x1 << 2) #define CH1_PD (0x1 << 1)
From: Lin Huang hl@rock-chips.com
AUX errors are caused by many different reasons. We may not know what happened in aux channel on failure, so let's reset aux channel if some errors occurred.
Cc: 征增 王 wzz@rock-chips.com Cc: Douglas Anderson dianders@chromium.org Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index dee1ba109b5f..7b7fd227e1f9 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -466,6 +466,10 @@ void analogix_dp_init_aux(struct analogix_dp_device *dp) reg = RPLY_RECEIV | AUX_ERR; writel(reg, dp->reg_base + ANALOGIX_DP_INT_STA);
+ analogix_dp_set_analog_power_down(dp, AUX_BLOCK, true); + usleep_range(10, 11); + analogix_dp_set_analog_power_down(dp, AUX_BLOCK, false); + analogix_dp_reset_aux(dp);
/* Disable AUX transaction H/W retry */ @@ -1159,7 +1163,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, reg, !(reg & AUX_EN), 25, 500 * 1000); if (ret) { dev_err(dp->dev, "AUX CH enable timeout!\n"); - return -ETIMEDOUT; + goto aux_error; }
/* TODO: Wait for an interrupt instead of looping? */ @@ -1168,7 +1172,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, reg, reg & RPLY_RECEIV, 10, 20 * 1000); if (ret) { dev_err(dp->dev, "AUX CH cmd reply timeout!\n"); - return -ETIMEDOUT; + goto aux_error; }
/* Clear interrupt source for AUX CH command reply */ @@ -1178,7 +1182,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA); if (reg & AUX_ERR) { writel(AUX_ERR, dp->reg_base + ANALOGIX_DP_INT_STA); - return -EREMOTEIO; + goto aux_error; }
/* Check AUX CH error access status */ @@ -1186,7 +1190,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, if ((reg & AUX_STATUS_MASK)) { dev_err(dp->dev, "AUX CH error happened: %d\n\n", reg & AUX_STATUS_MASK); - return -EREMOTEIO; + goto aux_error; }
if (msg->request & DP_AUX_I2C_READ) { @@ -1212,4 +1216,10 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, msg->reply = DP_AUX_NATIVE_REPLY_ACK;
return num_transferred > 0 ? num_transferred : -EBUSY; + +aux_error: + /* if aux err happen, reset aux */ + analogix_dp_init_aux(dp); + + return -EREMOTEIO; }
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: Lin Huang hl@rock-chips.com
AUX errors are caused by many different reasons. We may not know what happened in aux channel on failure, so let's reset aux channel if some errors occurred.
Cc: 征增 王 wzz@rock-chips.com Cc: Douglas Anderson dianders@chromium.org Signed-off-by: Lin Huang hl@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index dee1ba109b5f..7b7fd227e1f9 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -466,6 +466,10 @@ void analogix_dp_init_aux(struct analogix_dp_device *dp) reg = RPLY_RECEIV | AUX_ERR; writel(reg, dp->reg_base + ANALOGIX_DP_INT_STA);
- analogix_dp_set_analog_power_down(dp, AUX_BLOCK, true);
- usleep_range(10, 11);
- analogix_dp_set_analog_power_down(dp, AUX_BLOCK, false);
Is this additional power on/off the analog power to the AUX block required to reset it correctly? If so, could you mention that in the commit message?
analogix_dp_reset_aux(dp);
/* Disable AUX transaction H/W retry */ @@ -1159,7 +1163,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, reg, !(reg & AUX_EN), 25, 500 * 1000); if (ret) { dev_err(dp->dev, "AUX CH enable timeout!\n");
return -ETIMEDOUT;
goto aux_error;}
/* TODO: Wait for an interrupt instead of looping? */
@@ -1168,7 +1172,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, reg, reg & RPLY_RECEIV, 10, 20 * 1000); if (ret) { dev_err(dp->dev, "AUX CH cmd reply timeout!\n");
return -ETIMEDOUT;
goto aux_error;}
/* Clear interrupt source for AUX CH command reply */
@@ -1178,7 +1182,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA); if (reg & AUX_ERR) { writel(AUX_ERR, dp->reg_base + ANALOGIX_DP_INT_STA);
return -EREMOTEIO;
goto aux_error;}
/* Check AUX CH error access status */
@@ -1186,7 +1190,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, if ((reg & AUX_STATUS_MASK)) { dev_err(dp->dev, "AUX CH error happened: %d\n\n", reg & AUX_STATUS_MASK);
return -EREMOTEIO;
goto aux_error;}
if (msg->request & DP_AUX_I2C_READ) {
@@ -1212,4 +1216,10 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, msg->reply = DP_AUX_NATIVE_REPLY_ACK;
return num_transferred > 0 ? num_transferred : -EBUSY;
+aux_error:
- /* if aux err happen, reset aux */
- analogix_dp_init_aux(dp);
- return -EREMOTEIO;
A couple of ETIMEDOUTs have been replaced with EREMOTEIOs after this change. Maybe we set it the error no in ret and return ret?
With those changes,
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
}
From: zain wang wzz@rock-chips.com
If we failed disable psr, it would hang the display until next psr cycle coming. So we should restore psr->state when it failed.
Cc: Tomasz Figa tfiga@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 4 +++- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 10 +++++----- drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 20 +++++++++++++------- drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 2 +- 4 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index be6eddd0d0a7..1f1cb624414d 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -153,8 +153,10 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp) psr_vsc.DB1 = 0;
ret = drm_dp_dpcd_writeb(&dp->aux, DP_SET_POWER, DP_SET_POWER_D0); - if (ret != 1) + if (ret != 1) { dev_err(dp->dev, "Failed to set DP Power0 %d\n", ret); + return ret; + }
return analogix_dp_send_psr_spd(dp, &psr_vsc, false); } diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 3e8bf79bea58..8c884f9ce713 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -77,13 +77,13 @@ struct rockchip_dp_device { struct analogix_dp_plat_data plat_data; };
-static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled) +static int analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled) { struct rockchip_dp_device *dp = to_dp(encoder); int ret;
if (!analogix_dp_psr_enabled(dp->adp)) - return; + return 0;
DRM_DEV_DEBUG(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit");
@@ -91,13 +91,13 @@ static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled) PSR_WAIT_LINE_FLAG_TIMEOUT_MS); if (ret) { DRM_DEV_ERROR(dp->dev, "line flag interrupt did not arrive\n"); - return; + return -ETIMEDOUT; }
if (enabled) - analogix_dp_enable_psr(dp->adp); + return analogix_dp_enable_psr(dp->adp); else - analogix_dp_disable_psr(dp->adp); + return analogix_dp_disable_psr(dp->adp); }
static int rockchip_dp_pre_init(struct rockchip_dp_device *dp) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c index b339ca943139..9376f4396b6b 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c @@ -36,7 +36,7 @@ struct psr_drv {
struct delayed_work flush_work;
- void (*set)(struct drm_encoder *encoder, bool enable); + int (*set)(struct drm_encoder *encoder, bool enable); };
static struct psr_drv *find_psr_by_crtc(struct drm_crtc *crtc) @@ -93,19 +93,25 @@ static void psr_set_state_locked(struct psr_drv *psr, enum psr_state state) return; }
- psr->state = state; - /* Actually commit the state change to hardware */ - switch (psr->state) { + switch (state) { case PSR_ENABLE: - psr->set(psr->encoder, true); + if (psr->set(psr->encoder, true)) + return; break;
case PSR_DISABLE: case PSR_FLUSH: - psr->set(psr->encoder, false); + if (psr->set(psr->encoder, false)) + return; break; + + default: + pr_err("%s: Unknown state %d\n", __func__, state); + return; } + + psr->state = state; }
static void psr_set_state(struct psr_drv *psr, enum psr_state state) @@ -229,7 +235,7 @@ EXPORT_SYMBOL(rockchip_drm_psr_flush_all); * Zero on success, negative errno on failure. */ int rockchip_drm_psr_register(struct drm_encoder *encoder, - void (*psr_set)(struct drm_encoder *, bool enable)) + int (*psr_set)(struct drm_encoder *, bool enable)) { struct rockchip_drm_private *drm_drv = encoder->dev->dev_private; struct psr_drv *psr; diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h index b1ea0155e57c..06537ee27565 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h @@ -22,7 +22,7 @@ int rockchip_drm_psr_activate(struct drm_encoder *encoder); int rockchip_drm_psr_deactivate(struct drm_encoder *encoder);
int rockchip_drm_psr_register(struct drm_encoder *encoder, - void (*psr_set)(struct drm_encoder *, bool enable)); + int (*psr_set)(struct drm_encoder *, bool enable)); void rockchip_drm_psr_unregister(struct drm_encoder *encoder);
#endif /* __ROCKCHIP_DRM_PSR__ */
Am Freitag, 9. März 2018, 23:23:10 CET schrieb Enric Balletbo i Serra:
From: zain wang wzz@rock-chips.com
If we failed disable psr, it would hang the display until next psr cycle coming. So we should restore psr->state when it failed.
Cc: Tomasz Figa tfiga@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
Reviewed-by: Heiko Stuebner heiko@sntech.de
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
If we failed disable psr, it would hang the display until next psr cycle coming. So we should restore psr->state when it failed.
For the bridge part,
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Tomasz Figa tfiga@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 4 +++- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 10 +++++----- drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 20 +++++++++++++------- drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 2 +- 4 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index be6eddd0d0a7..1f1cb624414d 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -153,8 +153,10 @@ int analogix_dp_disable_psr(struct analogix_dp_device *dp) psr_vsc.DB1 = 0;
ret = drm_dp_dpcd_writeb(&dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
- if (ret != 1)
if (ret != 1) { dev_err(dp->dev, "Failed to set DP Power0 %d\n", ret);
return ret;}
return analogix_dp_send_psr_spd(dp, &psr_vsc, false); }
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 3e8bf79bea58..8c884f9ce713 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -77,13 +77,13 @@ struct rockchip_dp_device { struct analogix_dp_plat_data plat_data; };
-static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled) +static int analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled) { struct rockchip_dp_device *dp = to_dp(encoder); int ret;
if (!analogix_dp_psr_enabled(dp->adp))
return;
return 0;DRM_DEV_DEBUG(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit");
@@ -91,13 +91,13 @@ static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled) PSR_WAIT_LINE_FLAG_TIMEOUT_MS); if (ret) { DRM_DEV_ERROR(dp->dev, "line flag interrupt did not arrive\n");
return;
return -ETIMEDOUT;}
if (enabled)
analogix_dp_enable_psr(dp->adp);
return analogix_dp_enable_psr(dp->adp);
analogix_dp_disable_psr(dp->adp);
return analogix_dp_disable_psr(dp->adp);}
static int rockchip_dp_pre_init(struct rockchip_dp_device *dp)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c index b339ca943139..9376f4396b6b 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c @@ -36,7 +36,7 @@ struct psr_drv {
struct delayed_work flush_work;
- void (*set)(struct drm_encoder *encoder, bool enable);
int (*set)(struct drm_encoder *encoder, bool enable); };
static struct psr_drv *find_psr_by_crtc(struct drm_crtc *crtc)
@@ -93,19 +93,25 @@ static void psr_set_state_locked(struct psr_drv *psr, enum psr_state state) return; }
- psr->state = state;
- /* Actually commit the state change to hardware */
- switch (psr->state) {
- switch (state) { case PSR_ENABLE:
psr->set(psr->encoder, true);
if (psr->set(psr->encoder, true))return;break;
case PSR_DISABLE: case PSR_FLUSH:
psr->set(psr->encoder, false);
if (psr->set(psr->encoder, false))return;break;
default:
pr_err("%s: Unknown state %d\n", __func__, state);return;}
psr->state = state; }
static void psr_set_state(struct psr_drv *psr, enum psr_state state)
@@ -229,7 +235,7 @@ EXPORT_SYMBOL(rockchip_drm_psr_flush_all);
- Zero on success, negative errno on failure.
*/ int rockchip_drm_psr_register(struct drm_encoder *encoder,
void (*psr_set)(struct drm_encoder *, bool enable))
int (*psr_set)(struct drm_encoder *, bool enable))diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h index b1ea0155e57c..06537ee27565 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h @@ -22,7 +22,7 @@ int rockchip_drm_psr_activate(struct drm_encoder *encoder); int rockchip_drm_psr_deactivate(struct drm_encoder *encoder);
int rockchip_drm_psr_register(struct drm_encoder *encoder,
void (*psr_set)(struct drm_encoder *, bool enable));
int (*psr_set)(struct drm_encoder *, bool enable));void rockchip_drm_psr_unregister(struct drm_encoder *encoder);
#endif /* __ROCKCHIP_DRM_PSR__ */
From: zain wang wzz@rock-chips.com
There is no register named ANALOGIX_DP_PLL_CTL in Rockchip edp phy reg list. We should use BIT_4 in ANALOGIX_DP_PD to control the pll power instead of ANALOGIX_DP_PLL_CTL.
Cc: Douglas Anderson dianders@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 7b7fd227e1f9..02ab1aaa9993 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -230,16 +230,20 @@ enum pll_status analogix_dp_get_pll_lock_status(struct analogix_dp_device *dp) void analogix_dp_set_pll_power_down(struct analogix_dp_device *dp, bool enable) { u32 reg; + u32 mask = DP_PLL_PD; + u32 pd_addr = ANALOGIX_DP_PLL_CTL;
- if (enable) { - reg = readl(dp->reg_base + ANALOGIX_DP_PLL_CTL); - reg |= DP_PLL_PD; - writel(reg, dp->reg_base + ANALOGIX_DP_PLL_CTL); - } else { - reg = readl(dp->reg_base + ANALOGIX_DP_PLL_CTL); - reg &= ~DP_PLL_PD; - writel(reg, dp->reg_base + ANALOGIX_DP_PLL_CTL); + if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) { + pd_addr = ANALOGIX_DP_PD; + mask = RK_PLL_PD; } + + reg = readl(dp->reg_base + pd_addr); + if (enable) + reg |= mask; + else + reg &= ~mask; + writel(reg, dp->reg_base + pd_addr); }
void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
There is no register named ANALOGIX_DP_PLL_CTL in Rockchip edp phy reg list. We should use BIT_4 in ANALOGIX_DP_PD to control the pll power instead of ANALOGIX_DP_PLL_CTL.
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Douglas Anderson dianders@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 7b7fd227e1f9..02ab1aaa9993 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -230,16 +230,20 @@ enum pll_status analogix_dp_get_pll_lock_status(struct analogix_dp_device *dp) void analogix_dp_set_pll_power_down(struct analogix_dp_device *dp, bool enable) { u32 reg;
- u32 mask = DP_PLL_PD;
- u32 pd_addr = ANALOGIX_DP_PLL_CTL;
- if (enable) {
reg = readl(dp->reg_base + ANALOGIX_DP_PLL_CTL);reg |= DP_PLL_PD;writel(reg, dp->reg_base + ANALOGIX_DP_PLL_CTL);- } else {
reg = readl(dp->reg_base + ANALOGIX_DP_PLL_CTL);reg &= ~DP_PLL_PD;writel(reg, dp->reg_base + ANALOGIX_DP_PLL_CTL);
if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) {
pd_addr = ANALOGIX_DP_PD;mask = RK_PLL_PD;}
reg = readl(dp->reg_base + pd_addr);
if (enable)
reg |= mask;else
reg &= ~mask;writel(reg, dp->reg_base + pd_addr); }
void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
From: zain wang wzz@rock-chips.com
The STRM_VALID bit in register ANALOGIX_DP_SYS_CTL_3 may be unstable, so we may hit the error log "Timeout of video streamclk ok" since checked this unstable bit. In fact, we can go continue and the streamclk is ok if we wait enough time, it does no effect on display. Let's change this error to warn.
Cc: Douglas Anderson dianders@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 1f1cb624414d..d76e1652b1fd 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -921,8 +921,9 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp) done_count = 0; } if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) { - dev_err(dp->dev, "Timeout of video streamclk ok\n"); - return -ETIMEDOUT; + dev_warn(dp->dev, + "Ignoring timeout of video streamclk ok\n"); + break; }
usleep_range(1000, 1001);
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
The STRM_VALID bit in register ANALOGIX_DP_SYS_CTL_3 may be unstable, so we may hit the error log "Timeout of video streamclk ok" since checked this unstable bit. In fact, we can go continue and the streamclk is ok if we wait enough time, it does no effect on display. Let's change this error to warn.
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Douglas Anderson dianders@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 1f1cb624414d..d76e1652b1fd 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -921,8 +921,9 @@ static int analogix_dp_config_video(struct analogix_dp_device *dp) done_count = 0; } if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) {
dev_err(dp->dev, "Timeout of video streamclk ok\n");return -ETIMEDOUT;
dev_warn(dp->dev,"Ignoring timeout of video streamclk ok\n");break;}
usleep_range(1000, 1001);
From: zain wang wzz@rock-chips.com
Register ANALOGIX_DP_FUNC_EN_1(offset 0x18), Rockchip is different to Exynos:
on Exynos edp phy, BIT 7 MASTER_VID_FUNC_EN_N BIT 6 reserved BIT 5 SLAVE_VID_FUNC_EN_N
on Rockchip edp phy, BIT 7 reserved BIT 6 RK_VID_CAP_FUNC_EN_N BIT 5 RK_VID_FIFO_FUNC_EN_N
So, we should do some private operations to Rockchip.
Cc: Tomasz Figa tfiga@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 19 ++++++++++++++----- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 2 ++ 2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 02ab1aaa9993..4eae206ec31b 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -126,9 +126,14 @@ void analogix_dp_reset(struct analogix_dp_device *dp) analogix_dp_stop_video(dp); analogix_dp_enable_video_mute(dp, 0);
- reg = MASTER_VID_FUNC_EN_N | SLAVE_VID_FUNC_EN_N | - AUD_FIFO_FUNC_EN_N | AUD_FUNC_EN_N | - HDCP_FUNC_EN_N | SW_FUNC_EN_N; + if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) + reg = RK_VID_CAP_FUNC_EN_N | RK_VID_FIFO_FUNC_EN_N | + SW_FUNC_EN_N; + else + reg = MASTER_VID_FUNC_EN_N | SLAVE_VID_FUNC_EN_N | + AUD_FIFO_FUNC_EN_N | AUD_FUNC_EN_N | + HDCP_FUNC_EN_N | SW_FUNC_EN_N; + writel(reg, dp->reg_base + ANALOGIX_DP_FUNC_EN_1);
reg = SSC_FUNC_EN_N | AUX_FUNC_EN_N | @@ -971,8 +976,12 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp) u32 reg;
reg = readl(dp->reg_base + ANALOGIX_DP_FUNC_EN_1); - reg &= ~(MASTER_VID_FUNC_EN_N | SLAVE_VID_FUNC_EN_N); - reg |= MASTER_VID_FUNC_EN_N; + if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) { + reg &= ~(RK_VID_CAP_FUNC_EN_N | RK_VID_FIFO_FUNC_EN_N); + } else { + reg &= ~(MASTER_VID_FUNC_EN_N | SLAVE_VID_FUNC_EN_N); + reg |= MASTER_VID_FUNC_EN_N; + } writel(reg, dp->reg_base + ANALOGIX_DP_FUNC_EN_1);
reg = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_10); diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h index b633a4a5082a..0cf27c731727 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h @@ -127,7 +127,9 @@
/* ANALOGIX_DP_FUNC_EN_1 */ #define MASTER_VID_FUNC_EN_N (0x1 << 7) +#define RK_VID_CAP_FUNC_EN_N (0x1 << 6) #define SLAVE_VID_FUNC_EN_N (0x1 << 5) +#define RK_VID_FIFO_FUNC_EN_N (0x1 << 5) #define AUD_FIFO_FUNC_EN_N (0x1 << 4) #define AUD_FUNC_EN_N (0x1 << 3) #define HDCP_FUNC_EN_N (0x1 << 2)
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
Register ANALOGIX_DP_FUNC_EN_1(offset 0x18), Rockchip is different to Exynos:
on Exynos edp phy, BIT 7 MASTER_VID_FUNC_EN_N BIT 6 reserved BIT 5 SLAVE_VID_FUNC_EN_N
on Rockchip edp phy, BIT 7 reserved BIT 6 RK_VID_CAP_FUNC_EN_N BIT 5 RK_VID_FIFO_FUNC_EN_N
So, we should do some private operations to Rockchip.
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Tomasz Figa tfiga@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 19 ++++++++++++++----- drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h | 2 ++ 2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 02ab1aaa9993..4eae206ec31b 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -126,9 +126,14 @@ void analogix_dp_reset(struct analogix_dp_device *dp) analogix_dp_stop_video(dp); analogix_dp_enable_video_mute(dp, 0);
- reg = MASTER_VID_FUNC_EN_N | SLAVE_VID_FUNC_EN_N |
AUD_FIFO_FUNC_EN_N | AUD_FUNC_EN_N |HDCP_FUNC_EN_N | SW_FUNC_EN_N;
if (dp->plat_data && is_rockchip(dp->plat_data->dev_type))
reg = RK_VID_CAP_FUNC_EN_N | RK_VID_FIFO_FUNC_EN_N |SW_FUNC_EN_N;else
reg = MASTER_VID_FUNC_EN_N | SLAVE_VID_FUNC_EN_N |AUD_FIFO_FUNC_EN_N | AUD_FUNC_EN_N |HDCP_FUNC_EN_N | SW_FUNC_EN_N;writel(reg, dp->reg_base + ANALOGIX_DP_FUNC_EN_1);
reg = SSC_FUNC_EN_N | AUX_FUNC_EN_N |
@@ -971,8 +976,12 @@ void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp) u32 reg;
reg = readl(dp->reg_base + ANALOGIX_DP_FUNC_EN_1);
- reg &= ~(MASTER_VID_FUNC_EN_N | SLAVE_VID_FUNC_EN_N);
- reg |= MASTER_VID_FUNC_EN_N;
if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) {
reg &= ~(RK_VID_CAP_FUNC_EN_N | RK_VID_FIFO_FUNC_EN_N);} else {
reg &= ~(MASTER_VID_FUNC_EN_N | SLAVE_VID_FUNC_EN_N);reg |= MASTER_VID_FUNC_EN_N;} writel(reg, dp->reg_base + ANALOGIX_DP_FUNC_EN_1);
reg = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_10);
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h index b633a4a5082a..0cf27c731727 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h @@ -127,7 +127,9 @@
/* ANALOGIX_DP_FUNC_EN_1 */ #define MASTER_VID_FUNC_EN_N (0x1 << 7) +#define RK_VID_CAP_FUNC_EN_N (0x1 << 6) #define SLAVE_VID_FUNC_EN_N (0x1 << 5) +#define RK_VID_FIFO_FUNC_EN_N (0x1 << 5) #define AUD_FIFO_FUNC_EN_N (0x1 << 4) #define AUD_FUNC_EN_N (0x1 << 3) #define HDCP_FUNC_EN_N (0x1 << 2)
From: zain wang wzz@rock-chips.com
It's too early to detect fast link training, if other step after it failed, we will set fast_link flag to 1, and retry set_bridge again. In this case we will power down and power up panel power supply, and we will do fast link training since we have set fast_link flag to 1. In fact, we should do full link training now, not the fast link training. So we should move the fast link detection at the end of set_bridge.
Cc: Tomasz Figa tfiga@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 42 +++++++++++++--------- 1 file changed, 26 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index d76e1652b1fd..37b16643f14c 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -601,7 +601,7 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) { int lane, lane_count, retval; u32 reg; - u8 link_align, link_status[2], adjust_request[2], spread; + u8 link_align, link_status[2], adjust_request[2];
usleep_range(400, 401);
@@ -645,20 +645,6 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) dev_dbg(dp->dev, "final lane count = %.2x\n", dp->link_train.lane_count);
- retval = drm_dp_dpcd_readb(&dp->aux, DP_MAX_DOWNSPREAD, - &spread); - if (retval != 1) { - dev_err(dp->dev, "failed to read downspread %d\n", - retval); - dp->fast_train_enable = false; - } else { - dp->fast_train_enable = - (spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING) ? - true : false; - } - dev_dbg(dp->dev, "fast link training %s\n", - dp->fast_train_enable ? "supported" : "unsupported"); - dp->link_train.lt_state = FINISHED;
return 0; @@ -996,6 +982,22 @@ static irqreturn_t analogix_dp_irq_thread(int irq, void *arg) return IRQ_HANDLED; }
+static int analogix_dp_fast_link_train_detection(struct analogix_dp_device *dp) +{ + int ret; + u8 spread; + + ret = drm_dp_dpcd_readb(&dp->aux, DP_MAX_DOWNSPREAD, &spread); + if (ret != 1) { + dev_err(dp->dev, "failed to read downspread %d\n", ret); + return ret; + } + dp->fast_train_enable = !!(spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING); + dev_dbg(dp->dev, "fast link training %s\n", + dp->fast_train_enable ? "supported" : "unsupported"); + return 0; +} + static int analogix_dp_commit(struct analogix_dp_device *dp) { int ret; @@ -1038,8 +1040,16 @@ static int analogix_dp_commit(struct analogix_dp_device *dp) if (ret) return ret;
- if (dp->psr_enable) + if (dp->psr_enable) { ret = analogix_dp_enable_sink_psr(dp); + if (ret) + return ret; + } + + /* Check whether panel supports fast training */ + ret = analogix_dp_fast_link_train_detection(dp); + if (ret) + dp->psr_enable = false;
return ret; }
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: zain wang wzz@rock-chips.com
It's too early to detect fast link training, if other step after it failed, we will set fast_link flag to 1, and retry set_bridge again. In this case we will power down and power up panel power supply, and we will do fast link training since we have set fast_link flag to 1. In fact, we should do full link training now, not the fast link training. So we should move the fast link detection at the end of set_bridge.
Is it possible to re-write this commit message? It's a bit hard to follow.
Thanks, Archit
Cc: Tomasz Figa tfiga@chromium.org Signed-off-by: zain wang wzz@rock-chips.com Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 42 +++++++++++++--------- 1 file changed, 26 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index d76e1652b1fd..37b16643f14c 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -601,7 +601,7 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) { int lane, lane_count, retval; u32 reg;
- u8 link_align, link_status[2], adjust_request[2], spread;
u8 link_align, link_status[2], adjust_request[2];
usleep_range(400, 401);
@@ -645,20 +645,6 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) dev_dbg(dp->dev, "final lane count = %.2x\n", dp->link_train.lane_count);
retval = drm_dp_dpcd_readb(&dp->aux, DP_MAX_DOWNSPREAD,&spread);if (retval != 1) {dev_err(dp->dev, "failed to read downspread %d\n",retval);dp->fast_train_enable = false;} else {dp->fast_train_enable =(spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING) ?true : false;}dev_dbg(dp->dev, "fast link training %s\n",dp->fast_train_enable ? "supported" : "unsupported");dp->link_train.lt_state = FINISHED;
return 0;
@@ -996,6 +982,22 @@ static irqreturn_t analogix_dp_irq_thread(int irq, void *arg) return IRQ_HANDLED; }
+static int analogix_dp_fast_link_train_detection(struct analogix_dp_device *dp) +{
- int ret;
- u8 spread;
- ret = drm_dp_dpcd_readb(&dp->aux, DP_MAX_DOWNSPREAD, &spread);
- if (ret != 1) {
dev_err(dp->dev, "failed to read downspread %d\n", ret);return ret;- }
- dp->fast_train_enable = !!(spread & DP_NO_AUX_HANDSHAKE_LINK_TRAINING);
- dev_dbg(dp->dev, "fast link training %s\n",
dp->fast_train_enable ? "supported" : "unsupported");- return 0;
+}
- static int analogix_dp_commit(struct analogix_dp_device *dp) { int ret;
@@ -1038,8 +1040,16 @@ static int analogix_dp_commit(struct analogix_dp_device *dp) if (ret) return ret;
- if (dp->psr_enable)
if (dp->psr_enable) { ret = analogix_dp_enable_sink_psr(dp);
if (ret)return ret;}
/* Check whether panel supports fast training */
ret = analogix_dp_fast_link_train_detection(dp);
if (ret)
dp->psr_enable = false;return ret; }
From: Douglas Anderson dianders@chromium.org
The current user of the analogix power_off is "analogix_dp-rockchip". That driver does this: - deactivate PSR - turn off a clock
Both of these things (especially deactive PSR) should be done before we turn the PHY power off and turn off analog power. Let's move the callback up.
Note that without this patch (and with https://patchwork.kernel.org/patch/9553349/ [seanpaul: this patch was not applied, but it seems like the race can still occur]), I experienced an error in reboot testing where one thread was at:
rockchip_drm_psr_deactivate rockchip_dp_powerdown analogix_dp_bridge_disable drm_bridge_disable
...and the other thread was at:
analogix_dp_send_psr_spd analogix_dp_enable_psr analogix_dp_psr_set psr_flush_handler
The flush handler thread was finding AUX channel errors and eventually reported "Failed to apply PSR", where I had a kgdb breakpoint. Presumably the device would have eventually given up and shut down anyway, but it seems better to fix the order to be more correct.
Cc: Kristian H. Kristensen hoegsberg@chromium.org Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 37b16643f14c..eaf93cbd47a8 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1337,12 +1337,13 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge) }
disable_irq(dp->irq); - analogix_dp_set_analog_power_down(dp, POWER_ALL, 1); - phy_power_off(dp->phy);
if (dp->plat_data->power_off) dp->plat_data->power_off(dp->plat_data);
+ analogix_dp_set_analog_power_down(dp, POWER_ALL, 1); + phy_power_off(dp->phy); + clk_disable_unprepare(dp->clock);
pm_runtime_put_sync(dp->dev);
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: Douglas Anderson dianders@chromium.org
The current user of the analogix power_off is "analogix_dp-rockchip". That driver does this:
- deactivate PSR
- turn off a clock
Both of these things (especially deactive PSR) should be done before we turn the PHY power off and turn off analog power. Let's move the callback up.
Note that without this patch (and with https://patchwork.kernel.org/patch/9553349/ [seanpaul: this patch was not applied, but it seems like the race can still occur]), I experienced an error in reboot testing where one thread was at:
rockchip_drm_psr_deactivate rockchip_dp_powerdown analogix_dp_bridge_disable drm_bridge_disable
...and the other thread was at:
analogix_dp_send_psr_spd analogix_dp_enable_psr analogix_dp_psr_set psr_flush_handler
The flush handler thread was finding AUX channel errors and eventually reported "Failed to apply PSR", where I had a kgdb breakpoint. Presumably the device would have eventually given up and shut down anyway, but it seems better to fix the order to be more correct.
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Kristian H. Kristensen hoegsberg@chromium.org Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index 37b16643f14c..eaf93cbd47a8 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1337,12 +1337,13 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge) }
disable_irq(dp->irq);
analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
phy_power_off(dp->phy);
if (dp->plat_data->power_off) dp->plat_data->power_off(dp->plat_data);
analogix_dp_set_analog_power_down(dp, POWER_ALL, 1);
phy_power_off(dp->phy);
clk_disable_unprepare(dp->clock);
pm_runtime_put_sync(dp->dev);
From: Douglas Anderson dianders@chromium.org
The code in analogix_dp_transfer() that was supposed to print out: AUX CH error happened
Was actually dead code. That's because the previous check (whether the interrupt status indicated any errors) would have hit for all errors anyway.
Let's combine the two error checks so we can actually see AUX CH errors. We'll also downgrade the message to a warning since some of these types of errors might be expected for some displays. If this gets too noisy we can downgrade again to debug.
Cc: 征增 王 wzz@rock-chips.com Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 4eae206ec31b..58e8a28e99aa 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -1105,6 +1105,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, struct drm_dp_aux_msg *msg) { u32 reg; + u32 status_reg; u8 *buffer = msg->buffer; unsigned int i; int num_transferred = 0; @@ -1193,16 +1194,12 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
/* Clear interrupt source for AUX CH access error */ reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA); - if (reg & AUX_ERR) { + status_reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_STA); + if ((reg & AUX_ERR) || (status_reg & AUX_STATUS_MASK)) { writel(AUX_ERR, dp->reg_base + ANALOGIX_DP_INT_STA); - goto aux_error; - }
- /* Check AUX CH error access status */ - reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_STA); - if ((reg & AUX_STATUS_MASK)) { - dev_err(dp->dev, "AUX CH error happened: %d\n\n", - reg & AUX_STATUS_MASK); + dev_warn(dp->dev, "AUX CH error happened: %#x (%d)\n", + status_reg & AUX_STATUS_MASK, !!(reg & AUX_ERR)); goto aux_error; }
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: Douglas Anderson dianders@chromium.org
The code in analogix_dp_transfer() that was supposed to print out: AUX CH error happened
Was actually dead code. That's because the previous check (whether the interrupt status indicated any errors) would have hit for all errors anyway.
Let's combine the two error checks so we can actually see AUX CH errors. We'll also downgrade the message to a warning since some of these types of errors might be expected for some displays. If this gets too noisy we can downgrade again to debug.
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: 征增 王 wzz@rock-chips.com Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 4eae206ec31b..58e8a28e99aa 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -1105,6 +1105,7 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, struct drm_dp_aux_msg *msg) { u32 reg;
- u32 status_reg; u8 *buffer = msg->buffer; unsigned int i; int num_transferred = 0;
@@ -1193,16 +1194,12 @@ ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
/* Clear interrupt source for AUX CH access error */ reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA);
- if (reg & AUX_ERR) {
- status_reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_STA);
- if ((reg & AUX_ERR) || (status_reg & AUX_STATUS_MASK)) { writel(AUX_ERR, dp->reg_base + ANALOGIX_DP_INT_STA);
goto aux_error;}
/* Check AUX CH error access status */
reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_STA);
if ((reg & AUX_STATUS_MASK)) {
dev_err(dp->dev, "AUX CH error happened: %d\n\n",reg & AUX_STATUS_MASK);
dev_warn(dp->dev, "AUX CH error happened: %#x (%d)\n",status_reg & AUX_STATUS_MASK, !!(reg & AUX_ERR));
From: Douglas Anderson dianders@chromium.org
The comments in analogix_dp_init_aux() claim that we're disabling aux channel retries, but then right below it for Rockchip it sets them to 3. If we actually need 3 retries for Rockchip then we could adjust the comment, but it seems more likely that we want the same retry behavior across all platforms.
Cc: Stéphane Marchesin marcheu@chromium.org Cc: 征增 王 wzz@rock-chips.com Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 58e8a28e99aa..a5f2763d72e4 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -481,15 +481,16 @@ void analogix_dp_init_aux(struct analogix_dp_device *dp)
analogix_dp_reset_aux(dp);
- /* Disable AUX transaction H/W retry */ + /* AUX_BIT_PERIOD_EXPECTED_DELAY doesn't apply to Rockchip IP */ if (dp->plat_data && is_rockchip(dp->plat_data->dev_type)) - reg = AUX_BIT_PERIOD_EXPECTED_DELAY(0) | - AUX_HW_RETRY_COUNT_SEL(3) | - AUX_HW_RETRY_INTERVAL_600_MICROSECONDS; + reg = 0; else - reg = AUX_BIT_PERIOD_EXPECTED_DELAY(3) | - AUX_HW_RETRY_COUNT_SEL(0) | - AUX_HW_RETRY_INTERVAL_600_MICROSECONDS; + reg = AUX_BIT_PERIOD_EXPECTED_DELAY(3); + + /* Disable AUX transaction H/W retry */ + reg |= AUX_HW_RETRY_COUNT_SEL(0) | + AUX_HW_RETRY_INTERVAL_600_MICROSECONDS; + writel(reg, dp->reg_base + ANALOGIX_DP_AUX_HW_RETRY_CTL);
/* Receive AUX Channel DEFER commands equal to DEFFER_COUNT*64 */
On Saturday 10 March 2018 03:53 AM, Enric Balletbo i Serra wrote:
From: Douglas Anderson dianders@chromium.org
The comments in analogix_dp_init_aux() claim that we're disabling aux channel retries, but then right below it for Rockchip it sets them to 3. If we actually need 3 retries for Rockchip then we could adjust the comment, but it seems more likely that we want the same retry behavior across all platforms.
Reviewed-by: Archit Taneja architt@codeaurora.org
Thanks, Archit
Cc: Stéphane Marchesin marcheu@chromium.org Cc: 征增 王 wzz@rock-chips.com Signed-off-by: Douglas Anderson dianders@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com
drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index 58e8a28e99aa..a5f2763d72e4 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -481,15 +481,16 @@ void analogix_dp_init_aux(struct analogix_dp_device *dp)
analogix_dp_reset_aux(dp);
- /* Disable AUX transaction H/W retry */
- /* AUX_BIT_PERIOD_EXPECTED_DELAY doesn't apply to Rockchip IP */ if (dp->plat_data && is_rockchip(dp->plat_data->dev_type))
reg = AUX_BIT_PERIOD_EXPECTED_DELAY(0) |AUX_HW_RETRY_COUNT_SEL(3) |AUX_HW_RETRY_INTERVAL_600_MICROSECONDS;
reg = 0;
reg = AUX_BIT_PERIOD_EXPECTED_DELAY(3) |AUX_HW_RETRY_COUNT_SEL(0) |AUX_HW_RETRY_INTERVAL_600_MICROSECONDS;
reg = AUX_BIT_PERIOD_EXPECTED_DELAY(3);/* Disable AUX transaction H/W retry */
reg |= AUX_HW_RETRY_COUNT_SEL(0) |
AUX_HW_RETRY_INTERVAL_600_MICROSECONDS;writel(reg, dp->reg_base + ANALOGIX_DP_AUX_HW_RETRY_CTL);
/* Receive AUX Channel DEFER commands equal to DEFFER_COUNT*64 */
From: Mark Yao mark.yao@rock-chips.com
Some encoder have a crc verification check, crc check fail if input and output data is not equal.
That means encoder input and output need use same color depth, vop can output 10bit data to encoder, but some panel only support 8bit depth, that would make crc check die.
So pre dither down vop data to 8bit if panel's bpc is 8.
Signed-off-by: Mark Yao mark.yao@rock-chips.com [seanpaul resolved conflict in rockchip_drm_vop.c] Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 2 ++ drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 ++++++ drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 + drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 + 5 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 8c884f9ce713..b3f46ed24cdc 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -218,6 +218,7 @@ rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder, struct drm_connector_state *conn_state) { struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state); + struct drm_display_info *di = &conn_state->connector->display_info;
/* * The hardware IC designed that VOP must output the RGB10 video @@ -229,6 +230,7 @@ rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder,
s->output_mode = ROCKCHIP_OUT_MODE_AAAA; s->output_type = DRM_MODE_CONNECTOR_eDP; + s->output_bpc = di->bpc;
return 0; } diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index 9c064a40458b..3a6ebfc26036 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h @@ -36,6 +36,7 @@ struct rockchip_crtc_state { struct drm_crtc_state base; int output_type; int output_mode; + int output_bpc; }; #define to_rockchip_crtc_state(s) \ container_of(s, struct rockchip_crtc_state, base) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index b601c59e76a8..e2c9ee3204f4 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -923,6 +923,12 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc, if (s->output_mode == ROCKCHIP_OUT_MODE_AAAA && !(vop_data->feature & VOP_FEATURE_OUTPUT_RGB10)) s->output_mode = ROCKCHIP_OUT_MODE_P888; + + if (s->output_mode == ROCKCHIP_OUT_MODE_AAAA && s->output_bpc == 8) + VOP_REG_SET(vop, common, pre_dither_down, 1); + else + VOP_REG_SET(vop, common, pre_dither_down, 0); + VOP_REG_SET(vop, common, out_mode, s->output_mode);
VOP_REG_SET(vop, modeset, htotal_pw, (htotal << 16) | hsync_len); diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h index 56bbd2e2a8ef..084acdd0019a 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h @@ -67,6 +67,7 @@ struct vop_common { struct vop_reg cfg_done; struct vop_reg dsp_blank; struct vop_reg data_blank; + struct vop_reg pre_dither_down; struct vop_reg dither_down; struct vop_reg dither_up; struct vop_reg gate_en; diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c index 2e4eea3459fe..08023d3ecb76 100644 --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c @@ -264,6 +264,7 @@ static const struct vop_common rk3288_common = { .standby = VOP_REG_SYNC(RK3288_SYS_CTRL, 0x1, 22), .gate_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 23), .mmu_en = VOP_REG(RK3288_SYS_CTRL, 0x1, 20), + .pre_dither_down = VOP_REG(RK3288_DSP_CTRL1, 0x1, 1), .dither_down = VOP_REG(RK3288_DSP_CTRL1, 0xf, 1), .dither_up = VOP_REG(RK3288_DSP_CTRL1, 0x1, 6), .data_blank = VOP_REG(RK3288_DSP_CTRL0, 0x1, 19),
From: Douglas Anderson dianders@chromium.org
Some of the platform-specific stuff in rockchip_dp_poweron() needs to happen before the generic code. Some needs to happen after. Let's split the callback in two.
Specifically we can't start doing PSR work until _after_ the whole controller is up, so don't set the enable until the end.
Cc: Kristian H. Kristensen hoegsberg@chromium.org Signed-off-by: Douglas Anderson dianders@chromium.org [seanpaul added exynos change] Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Reviewed-by: Andrzej Hajda a.hajda@samsung.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 7 +++++-- drivers/gpu/drm/exynos/exynos_dp.c | 2 +- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 12 ++++++++++-- include/drm/bridge/analogix_dp.h | 3 ++- 4 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index eaf93cbd47a8..c527019daaa6 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -1260,8 +1260,8 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp) goto out_dp_clk_pre; }
- if (dp->plat_data->power_on) - dp->plat_data->power_on(dp->plat_data); + if (dp->plat_data->power_on_start) + dp->plat_data->power_on_start(dp->plat_data);
phy_power_on(dp->phy);
@@ -1286,6 +1286,9 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp) goto out_dp_init; }
+ if (dp->plat_data->power_on_end) + dp->plat_data->power_on_end(dp->plat_data); + enable_irq(dp->irq); return 0;
diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c index 964831dab102..86330f396784 100644 --- a/drivers/gpu/drm/exynos/exynos_dp.c +++ b/drivers/gpu/drm/exynos/exynos_dp.c @@ -162,7 +162,7 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) dp->drm_dev = drm_dev;
dp->plat_data.dev_type = EXYNOS_DP; - dp->plat_data.power_on = exynos_dp_poweron; + dp->plat_data.power_on_start = exynos_dp_poweron; dp->plat_data.power_off = exynos_dp_poweroff; dp->plat_data.attach = exynos_dp_bridge_attach; dp->plat_data.get_modes = exynos_dp_get_modes; diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index b3f46ed24cdc..23317a2269e1 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -109,7 +109,7 @@ static int rockchip_dp_pre_init(struct rockchip_dp_device *dp) return 0; }
-static int rockchip_dp_poweron(struct analogix_dp_plat_data *plat_data) +static int rockchip_dp_poweron_start(struct analogix_dp_plat_data *plat_data) { struct rockchip_dp_device *dp = to_dp(plat_data); int ret; @@ -127,6 +127,13 @@ static int rockchip_dp_poweron(struct analogix_dp_plat_data *plat_data) return ret; }
+ return ret; +} + +static int rockchip_dp_poweron_end(struct analogix_dp_plat_data *plat_data) +{ + struct rockchip_dp_device *dp = to_dp(plat_data); + return rockchip_drm_psr_activate(&dp->encoder); }
@@ -330,7 +337,8 @@ static int rockchip_dp_bind(struct device *dev, struct device *master, dp->plat_data.encoder = &dp->encoder;
dp->plat_data.dev_type = dp->data->chip_type; - dp->plat_data.power_on = rockchip_dp_poweron; + dp->plat_data.power_on_start = rockchip_dp_poweron_start; + dp->plat_data.power_on_end = rockchip_dp_poweron_end; dp->plat_data.power_off = rockchip_dp_powerdown; dp->plat_data.get_modes = rockchip_dp_get_modes;
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h index e9a1116d2f8e..475b706b49de 100644 --- a/include/drm/bridge/analogix_dp.h +++ b/include/drm/bridge/analogix_dp.h @@ -33,7 +33,8 @@ struct analogix_dp_plat_data { struct drm_connector *connector; bool skip_connector;
- int (*power_on)(struct analogix_dp_plat_data *); + int (*power_on_start)(struct analogix_dp_plat_data *); + int (*power_on_end)(struct analogix_dp_plat_data *); int (*power_off)(struct analogix_dp_plat_data *); int (*attach)(struct analogix_dp_plat_data *, struct drm_bridge *, struct drm_connector *);
From: Tomasz Figa tfiga@chromium.org
Driver callbacks, such as system suspend or resume can be called any time, specifically they can be called before the component bind callback. Let's use dp->adp pointer as a safeguard and skip calling Analogix entry points if it is an ERR_PTR().
Signed-off-by: Tomasz Figa tfiga@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 23317a2269e1..6d45d62466b3 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -368,6 +368,8 @@ static void rockchip_dp_unbind(struct device *dev, struct device *master, analogix_dp_unbind(dp->adp); rockchip_drm_psr_unregister(&dp->encoder); dp->encoder.funcs->destroy(&dp->encoder); + + dp->adp = ERR_PTR(-ENODEV); }
static const struct component_ops rockchip_dp_component_ops = { @@ -391,6 +393,7 @@ static int rockchip_dp_probe(struct platform_device *pdev) return -ENOMEM;
dp->dev = dev; + dp->adp = ERR_PTR(-ENODEV); dp->plat_data.panel = panel;
ret = rockchip_dp_of_probe(dp); @@ -414,6 +417,9 @@ static int rockchip_dp_suspend(struct device *dev) { struct rockchip_dp_device *dp = dev_get_drvdata(dev);
+ if (IS_ERR(dp->adp)) + return 0; + return analogix_dp_suspend(dp->adp); }
@@ -421,6 +427,9 @@ static int rockchip_dp_resume(struct device *dev) { struct rockchip_dp_device *dp = dev_get_drvdata(dev);
+ if (IS_ERR(dp->adp)) + return 0; + return analogix_dp_resume(dp->adp); } #endif
From: "Kristian H. Kristensen" hoegsberg@google.com
To improve PSR exit latency, we speculatively start exiting when we receive input events. Occasionally, this may lead to false positives, but most of the time we get a head start on coming out of PSR. Depending on how userspace takes to produce a new frame in response to the event, this can completely hide the exit latency. In case of Chrome OS, we typically get the input notifier 50ms or more before the dirty_fb triggered exit.
Signed-off-by: Kristian H. Kristensen hoegsberg@google.com Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 134 ++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c index 9376f4396b6b..a107845ba97c 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c @@ -12,6 +12,8 @@ * GNU General Public License for more details. */
+#include <linux/input.h> + #include <drm/drmP.h> #include <drm/drm_crtc_helper.h>
@@ -35,6 +37,9 @@ struct psr_drv { enum psr_state state;
struct delayed_work flush_work; + struct work_struct disable_work; + + struct input_handler input_handler;
int (*set)(struct drm_encoder *encoder, bool enable); }; @@ -133,6 +138,18 @@ static void psr_flush_handler(struct work_struct *work) mutex_unlock(&psr->lock); }
+static void psr_disable_handler(struct work_struct *work) +{ + struct psr_drv *psr = container_of(work, struct psr_drv, disable_work); + + /* If the state has changed since we initiated the flush, do nothing */ + mutex_lock(&psr->lock); + if (psr->state == PSR_ENABLE) + psr_set_state_locked(psr, PSR_FLUSH); + mutex_unlock(&psr->lock); + mod_delayed_work(system_wq, &psr->flush_work, PSR_FLUSH_TIMEOUT_MS); +} + /** * rockchip_drm_psr_activate - activate PSR on the given pipe * @encoder: encoder to obtain the PSR encoder @@ -173,6 +190,7 @@ int rockchip_drm_psr_deactivate(struct drm_encoder *encoder) psr->active = false; mutex_unlock(&psr->lock); cancel_delayed_work_sync(&psr->flush_work); + cancel_work_sync(&psr->disable_work);
return 0; } @@ -226,6 +244,95 @@ void rockchip_drm_psr_flush_all(struct drm_device *dev) } EXPORT_SYMBOL(rockchip_drm_psr_flush_all);
+static void psr_input_event(struct input_handle *handle, + unsigned int type, unsigned int code, + int value) +{ + struct psr_drv *psr = handle->handler->private; + + schedule_work(&psr->disable_work); +} + +static int psr_input_connect(struct input_handler *handler, + struct input_dev *dev, + const struct input_device_id *id) +{ + struct input_handle *handle; + int error; + + handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL); + if (!handle) + return -ENOMEM; + + handle->dev = dev; + handle->handler = handler; + handle->name = "rockchip-psr"; + + error = input_register_handle(handle); + if (error) + goto err2; + + error = input_open_device(handle); + if (error) + goto err1; + + return 0; + +err1: + input_unregister_handle(handle); +err2: + kfree(handle); + return error; +} + +static void psr_input_disconnect(struct input_handle *handle) +{ + input_close_device(handle); + input_unregister_handle(handle); + kfree(handle); +} + +/* Same device ids as cpu-boost */ +static const struct input_device_id psr_ids[] = { + { + .flags = INPUT_DEVICE_ID_MATCH_EVBIT | + INPUT_DEVICE_ID_MATCH_ABSBIT, + .evbit = { BIT_MASK(EV_ABS) }, + .absbit = { [BIT_WORD(ABS_MT_POSITION_X)] = + BIT_MASK(ABS_MT_POSITION_X) | + BIT_MASK(ABS_MT_POSITION_Y) }, + }, /* multi-touch touchscreen */ + { + .flags = INPUT_DEVICE_ID_MATCH_EVBIT, + .evbit = { BIT_MASK(EV_ABS) }, + .absbit = { [BIT_WORD(ABS_X)] = BIT_MASK(ABS_X) } + + }, /* stylus or joystick device */ + { + .flags = INPUT_DEVICE_ID_MATCH_EVBIT, + .evbit = { BIT_MASK(EV_KEY) }, + .keybit = { [BIT_WORD(BTN_LEFT)] = BIT_MASK(BTN_LEFT) }, + }, /* pointer (e.g. trackpad, mouse) */ + { + .flags = INPUT_DEVICE_ID_MATCH_EVBIT, + .evbit = { BIT_MASK(EV_KEY) }, + .keybit = { [BIT_WORD(KEY_ESC)] = BIT_MASK(KEY_ESC) }, + }, /* keyboard */ + { + .flags = INPUT_DEVICE_ID_MATCH_EVBIT | + INPUT_DEVICE_ID_MATCH_KEYBIT, + .evbit = { BIT_MASK(EV_KEY) }, + .keybit = {[BIT_WORD(BTN_JOYSTICK)] = BIT_MASK(BTN_JOYSTICK) }, + }, /* joysticks not caught by ABS_X above */ + { + .flags = INPUT_DEVICE_ID_MATCH_EVBIT | + INPUT_DEVICE_ID_MATCH_KEYBIT, + .evbit = { BIT_MASK(EV_KEY) }, + .keybit = { [BIT_WORD(BTN_GAMEPAD)] = BIT_MASK(BTN_GAMEPAD) }, + }, /* gamepad */ + { }, +}; + /** * rockchip_drm_psr_register - register encoder to psr driver * @encoder: encoder that obtain the PSR function @@ -239,6 +346,7 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder, { struct rockchip_drm_private *drm_drv = encoder->dev->dev_private; struct psr_drv *psr; + int error;
if (!encoder || !psr_set) return -EINVAL; @@ -248,6 +356,7 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder, return -ENOMEM;
INIT_DELAYED_WORK(&psr->flush_work, psr_flush_handler); + INIT_WORK(&psr->disable_work, psr_disable_handler); mutex_init(&psr->lock);
psr->active = true; @@ -255,11 +364,33 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder, psr->encoder = encoder; psr->set = psr_set;
+ psr->input_handler.event = psr_input_event; + psr->input_handler.connect = psr_input_connect; + psr->input_handler.disconnect = psr_input_disconnect; + psr->input_handler.name = + kasprintf(GFP_KERNEL, "rockchip-psr-%s", encoder->name); + if (!psr->input_handler.name) { + error = -ENOMEM; + goto err2; + } + psr->input_handler.id_table = psr_ids; + psr->input_handler.private = psr; + + error = input_register_handler(&psr->input_handler); + if (error) + goto err1; + mutex_lock(&drm_drv->psr_list_lock); list_add_tail(&psr->list, &drm_drv->psr_list); mutex_unlock(&drm_drv->psr_list_lock);
return 0; + + err1: + kfree(psr->input_handler.name); + err2: + kfree(psr); + return error; } EXPORT_SYMBOL(rockchip_drm_psr_register);
@@ -279,8 +410,11 @@ void rockchip_drm_psr_unregister(struct drm_encoder *encoder) mutex_lock(&drm_drv->psr_list_lock); list_for_each_entry_safe(psr, n, &drm_drv->psr_list, list) { if (psr->encoder == encoder) { + input_unregister_handler(&psr->input_handler); cancel_delayed_work_sync(&psr->flush_work); + cancel_work_sync(&psr->disable_work); list_del(&psr->list); + kfree(psr->input_handler.name); kfree(psr); } }
From: Tomasz Figa tfiga@chromium.org
If we change the state first and reschedule later, we might have the work executed according to previous scheduled time and end up with PSR re-enabled instantly. Let's cancel the work before changing the state.
While at it, consolidate psr_disable_handler() to just call rockchip_drm_do_flush(), as they are both supposed to do the same.
Signed-off-by: Tomasz Figa tfiga@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c index a107845ba97c..c8655e625ba2 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c @@ -138,18 +138,6 @@ static void psr_flush_handler(struct work_struct *work) mutex_unlock(&psr->lock); }
-static void psr_disable_handler(struct work_struct *work) -{ - struct psr_drv *psr = container_of(work, struct psr_drv, disable_work); - - /* If the state has changed since we initiated the flush, do nothing */ - mutex_lock(&psr->lock); - if (psr->state == PSR_ENABLE) - psr_set_state_locked(psr, PSR_FLUSH); - mutex_unlock(&psr->lock); - mod_delayed_work(system_wq, &psr->flush_work, PSR_FLUSH_TIMEOUT_MS); -} - /** * rockchip_drm_psr_activate - activate PSR on the given pipe * @encoder: encoder to obtain the PSR encoder @@ -198,6 +186,7 @@ EXPORT_SYMBOL(rockchip_drm_psr_deactivate);
static void rockchip_drm_do_flush(struct psr_drv *psr) { + cancel_delayed_work_sync(&psr->flush_work); psr_set_state(psr, PSR_FLUSH); mod_delayed_work(system_wq, &psr->flush_work, PSR_FLUSH_TIMEOUT_MS); } @@ -244,6 +233,13 @@ void rockchip_drm_psr_flush_all(struct drm_device *dev) } EXPORT_SYMBOL(rockchip_drm_psr_flush_all);
+static void psr_disable_handler(struct work_struct *work) +{ + struct psr_drv *psr = container_of(work, struct psr_drv, disable_work); + + rockchip_drm_do_flush(psr); +} + static void psr_input_event(struct input_handle *handle, unsigned int type, unsigned int code, int value)
From: Tomasz Figa tfiga@chromium.org
The first time after we call rockchip_drm_do_flush() after rockchip_drm_psr_register(), we go from PSR_DISABLE to PSR_FLUSH. The difference between PSR_DISABLE and PSR_FLUSH is whether or not we have a delayed work pending - PSR is off in either state. However psr_set_state() only catches the transition from PSR_FLUSH to PSR_DISABLE (which never happens), while going from PSR_DISABLE to PSR_FLUSH triggers a call to psr->set() to disable PSR while it's already disabled. This triggers the eDP PHY power-on sequence without being shut down first and this seems to occasionally leave the encoder unable to later enable PSR. Let's just simplify the state machine and simply consider PSR_DISABLE and PSR_FLUSH the same state. This lets us represent the hardware state by a simple boolean called "enabled" and, while at it, rename the misleading "active" boolean to "inhibit", which represents the purpose much better.
Also, userspace can (and does) trigger the rockchip_drm_do_flush() path from drmModeDirtyFB() at any time, whether or the encoder is active. If no mode is set, we call into analogix_dp_psr_set() which returns -EINVAL because encoder->crtc is NULL. Avoid this by starting out with psr->allowed set to false.
Signed-off-by: Kristian H. Kristensen hoegsberg@chromium.org Signed-off-by: Tomasz Figa tfiga@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 79 +++++++++-------------------- 1 file changed, 23 insertions(+), 56 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c index c8655e625ba2..448c5fde241c 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c @@ -22,19 +22,13 @@
#define PSR_FLUSH_TIMEOUT_MS 100
-enum psr_state { - PSR_FLUSH, - PSR_ENABLE, - PSR_DISABLE, -}; - struct psr_drv { struct list_head list; struct drm_encoder *encoder;
struct mutex lock; bool active; - enum psr_state state; + bool enabled;
struct delayed_work flush_work; struct work_struct disable_work; @@ -78,52 +72,22 @@ static struct psr_drv *find_psr_by_encoder(struct drm_encoder *encoder) return psr; }
-static void psr_set_state_locked(struct psr_drv *psr, enum psr_state state) +static int psr_set_state_locked(struct psr_drv *psr, bool enable) { - /* - * Allowed finite state machine: - * - * PSR_ENABLE < = = = = = > PSR_FLUSH - * | ^ | - * | | | - * v | | - * PSR_DISABLE < - - - - - - - - - - */ - if (state == psr->state || !psr->active) - return; - - /* Already disabled in flush, change the state, but not the hardware */ - if (state == PSR_DISABLE && psr->state == PSR_FLUSH) { - psr->state = state; - return; - } + int ret;
- /* Actually commit the state change to hardware */ - switch (state) { - case PSR_ENABLE: - if (psr->set(psr->encoder, true)) - return; - break; - - case PSR_DISABLE: - case PSR_FLUSH: - if (psr->set(psr->encoder, false)) - return; - break; - - default: - pr_err("%s: Unknown state %d\n", __func__, state); - return; - } + if (!psr->active) + return -EINVAL;
- psr->state = state; -} + if (enable == psr->enabled) + return 0;
-static void psr_set_state(struct psr_drv *psr, enum psr_state state) -{ - mutex_lock(&psr->lock); - psr_set_state_locked(psr, state); - mutex_unlock(&psr->lock); + ret = psr->set(psr->encoder, enable); + if (ret) + return ret; + + psr->enabled = enable; + return 0; }
static void psr_flush_handler(struct work_struct *work) @@ -131,10 +95,8 @@ static void psr_flush_handler(struct work_struct *work) struct psr_drv *psr = container_of(to_delayed_work(work), struct psr_drv, flush_work);
- /* If the state has changed since we initiated the flush, do nothing */ mutex_lock(&psr->lock); - if (psr->state == PSR_FLUSH) - psr_set_state_locked(psr, PSR_ENABLE); + psr_set_state_locked(psr, true); mutex_unlock(&psr->lock); }
@@ -176,6 +138,7 @@ int rockchip_drm_psr_deactivate(struct drm_encoder *encoder)
mutex_lock(&psr->lock); psr->active = false; + psr->enabled = false; mutex_unlock(&psr->lock); cancel_delayed_work_sync(&psr->flush_work); cancel_work_sync(&psr->disable_work); @@ -187,8 +150,12 @@ EXPORT_SYMBOL(rockchip_drm_psr_deactivate); static void rockchip_drm_do_flush(struct psr_drv *psr) { cancel_delayed_work_sync(&psr->flush_work); - psr_set_state(psr, PSR_FLUSH); - mod_delayed_work(system_wq, &psr->flush_work, PSR_FLUSH_TIMEOUT_MS); + + mutex_lock(&psr->lock); + if (!psr_set_state_locked(psr, false)) + mod_delayed_work(system_wq, &psr->flush_work, + PSR_FLUSH_TIMEOUT_MS); + mutex_unlock(&psr->lock); }
/** @@ -355,8 +322,8 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder, INIT_WORK(&psr->disable_work, psr_disable_handler); mutex_init(&psr->lock);
- psr->active = true; - psr->state = PSR_DISABLE; + psr->active = false; + psr->enabled = false; psr->encoder = encoder; psr->set = psr_set;
From: Tomasz Figa tfiga@chromium.org
Currently both rockchip_drm_psr_activate() and _deactivate() only set the boolean "active" flag without actually making sure that hardware state complies with it.
Since we are going to extend the usage of this API to properly lock PSR for the duration of atomic commits, we change the semantics in following way: - a counter is used to track the number of disallow requests, - PSR is actually disabled in hardware on first disallow request, - PSR enable work is scheduled on last disallow request.
The above allows using the API as a way to deterministically synchronize PSR state changes with other DRM events, i.e. atomic commits and cursor updates. As a nice side effect, the naming is sorted out and we have "inhibit" for stopping the software logic and "enable" for hardware state.
Signed-off-by: Tomasz Figa tfiga@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 4 +- drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 57 ++++++++++++++++++------- drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 4 +- 3 files changed, 46 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 6d45d62466b3..080f05352195 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -134,7 +134,7 @@ static int rockchip_dp_poweron_end(struct analogix_dp_plat_data *plat_data) { struct rockchip_dp_device *dp = to_dp(plat_data);
- return rockchip_drm_psr_activate(&dp->encoder); + return rockchip_drm_psr_inhibit_put(&dp->encoder); }
static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data) @@ -142,7 +142,7 @@ static int rockchip_dp_powerdown(struct analogix_dp_plat_data *plat_data) struct rockchip_dp_device *dp = to_dp(plat_data); int ret;
- ret = rockchip_drm_psr_deactivate(&dp->encoder); + ret = rockchip_drm_psr_inhibit_get(&dp->encoder); if (ret != 0) return ret;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c index 448c5fde241c..e7e16d92d5a1 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c @@ -27,7 +27,7 @@ struct psr_drv { struct drm_encoder *encoder;
struct mutex lock; - bool active; + int inhibit_count; bool enabled;
struct delayed_work flush_work; @@ -76,7 +76,7 @@ static int psr_set_state_locked(struct psr_drv *psr, bool enable) { int ret;
- if (!psr->active) + if (psr->inhibit_count > 0) return -EINVAL;
if (enable == psr->enabled) @@ -101,13 +101,18 @@ static void psr_flush_handler(struct work_struct *work) }
/** - * rockchip_drm_psr_activate - activate PSR on the given pipe + * rockchip_drm_psr_inhibit_put - release PSR inhibit on given encoder * @encoder: encoder to obtain the PSR encoder * + * Decrements PSR inhibit count on given encoder. Should be called only + * for a PSR inhibit count increment done before. If PSR inhibit counter + * reaches zero, PSR flush work is scheduled to make the hardware enter + * PSR mode in PSR_FLUSH_TIMEOUT_MS. + * * Returns: * Zero on success, negative errno on failure. */ -int rockchip_drm_psr_activate(struct drm_encoder *encoder) +int rockchip_drm_psr_inhibit_put(struct drm_encoder *encoder) { struct psr_drv *psr = find_psr_by_encoder(encoder);
@@ -115,21 +120,29 @@ int rockchip_drm_psr_activate(struct drm_encoder *encoder) return PTR_ERR(psr);
mutex_lock(&psr->lock); - psr->active = true; + --psr->inhibit_count; + if (!psr->inhibit_count) + mod_delayed_work(system_wq, &psr->flush_work, + PSR_FLUSH_TIMEOUT_MS); mutex_unlock(&psr->lock);
return 0; } -EXPORT_SYMBOL(rockchip_drm_psr_activate); +EXPORT_SYMBOL(rockchip_drm_psr_inhibit_put);
/** - * rockchip_drm_psr_deactivate - deactivate PSR on the given pipe + * rockchip_drm_psr_inhibit_get - acquire PSR inhibit on given encoder * @encoder: encoder to obtain the PSR encoder * + * Increments PSR inhibit count on given encoder. This function guarantees + * that after it returns PSR is turned off on given encoder and no PSR-related + * hardware state change occurs at least until a matching call to + * rockchip_drm_psr_inhibit_put() is done. + * * Returns: * Zero on success, negative errno on failure. */ -int rockchip_drm_psr_deactivate(struct drm_encoder *encoder) +int rockchip_drm_psr_inhibit_get(struct drm_encoder *encoder) { struct psr_drv *psr = find_psr_by_encoder(encoder);
@@ -137,15 +150,15 @@ int rockchip_drm_psr_deactivate(struct drm_encoder *encoder) return PTR_ERR(psr);
mutex_lock(&psr->lock); - psr->active = false; - psr->enabled = false; + psr_set_state_locked(psr, false); + ++psr->inhibit_count; mutex_unlock(&psr->lock); cancel_delayed_work_sync(&psr->flush_work); cancel_work_sync(&psr->disable_work);
return 0; } -EXPORT_SYMBOL(rockchip_drm_psr_deactivate); +EXPORT_SYMBOL(rockchip_drm_psr_inhibit_get);
static void rockchip_drm_do_flush(struct psr_drv *psr) { @@ -301,6 +314,11 @@ static const struct input_device_id psr_ids[] = { * @encoder: encoder that obtain the PSR function * @psr_set: call back to set PSR state * + * The function returns with PSR inhibit counter initialized with one + * and the caller (typically encoder driver) needs to call + * rockchip_drm_psr_inhibit_put() when it becomes ready to accept PSR + * enable request. + * * Returns: * Zero on success, negative errno on failure. */ @@ -322,7 +340,7 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder, INIT_WORK(&psr->disable_work, psr_disable_handler); mutex_init(&psr->lock);
- psr->active = false; + psr->inhibit_count = 1; psr->enabled = false; psr->encoder = encoder; psr->set = psr_set; @@ -362,6 +380,11 @@ EXPORT_SYMBOL(rockchip_drm_psr_register); * @encoder: encoder that obtain the PSR function * @psr_set: call back to set PSR state * + * It is expected that the PSR inhibit counter is 1 when this function is + * called, which corresponds to a state when related encoder has been + * disconnected from any CRTCs and its driver called + * rockchip_drm_psr_inhibit_get() to stop the PSR logic. + * * Returns: * Zero on success, negative errno on failure. */ @@ -373,10 +396,14 @@ void rockchip_drm_psr_unregister(struct drm_encoder *encoder) mutex_lock(&drm_drv->psr_list_lock); list_for_each_entry_safe(psr, n, &drm_drv->psr_list, list) { if (psr->encoder == encoder) { - input_unregister_handler(&psr->input_handler); - cancel_delayed_work_sync(&psr->flush_work); - cancel_work_sync(&psr->disable_work); + /* + * Any other value would mean that the encoder + * is still in use. + */ + WARN_ON(psr->inhibit_count != 1); + list_del(&psr->list); + input_unregister_handler(&psr->input_handler); kfree(psr->input_handler.name); kfree(psr); } diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h index 06537ee27565..40e026c14168 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h @@ -18,8 +18,8 @@ void rockchip_drm_psr_flush_all(struct drm_device *dev); int rockchip_drm_psr_flush(struct drm_crtc *crtc);
-int rockchip_drm_psr_activate(struct drm_encoder *encoder); -int rockchip_drm_psr_deactivate(struct drm_encoder *encoder); +int rockchip_drm_psr_inhibit_put(struct drm_encoder *encoder); +int rockchip_drm_psr_inhibit_get(struct drm_encoder *encoder);
int rockchip_drm_psr_register(struct drm_encoder *encoder, int (*psr_set)(struct drm_encoder *, bool enable));
From: Tomasz Figa tfiga@chromium.org
It looks like the driver subsystem detaches devices from power domains at shutdown without consent of the drivers. This means that we might have our power domain turned off behind our back and the only way to avoid problems is to stop doing any hardware programming at some point before the power is cut. A reboot notifier, despite being a misnomer and handling shutdowns as well, is a good place to do it.
Signed-off-by: Tomasz Figa tfiga@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c index e7e16d92d5a1..1bf5cba9a64d 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c @@ -13,6 +13,7 @@ */
#include <linux/input.h> +#include <linux/reboot.h>
#include <drm/drmP.h> #include <drm/drm_crtc_helper.h> @@ -33,6 +34,7 @@ struct psr_drv { struct delayed_work flush_work; struct work_struct disable_work;
+ struct notifier_block reboot_nb; struct input_handler input_handler;
int (*set)(struct drm_encoder *encoder, bool enable); @@ -309,6 +311,24 @@ static const struct input_device_id psr_ids[] = { { }, };
+static int rockchip_drm_psr_reboot_notifier(struct notifier_block *nb, + unsigned long action, void *data) +{ + struct psr_drv *psr = container_of(nb, struct psr_drv, reboot_nb); + + /* + * It looks like the driver subsystem detaches devices from power + * domains at shutdown without consent of the drivers. This means + * that we might have our power domain turned off behind our back + * and the only way to avoid problems is to stop doing any hardware + * programming after this point, which is achieved by the unbalanced + * call below. + */ + rockchip_drm_psr_inhibit_get(psr->encoder); + + return 0; +} + /** * rockchip_drm_psr_register - register encoder to psr driver * @encoder: encoder that obtain the PSR function @@ -361,6 +381,9 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder, if (error) goto err1;
+ psr->reboot_nb.notifier_call = rockchip_drm_psr_reboot_notifier; + register_reboot_notifier(&psr->reboot_nb); + mutex_lock(&drm_drv->psr_list_lock); list_add_tail(&psr->list, &drm_drv->psr_list); mutex_unlock(&drm_drv->psr_list_lock); @@ -403,6 +426,7 @@ void rockchip_drm_psr_unregister(struct drm_encoder *encoder) WARN_ON(psr->inhibit_count != 1);
list_del(&psr->list); + unregister_reboot_notifier(&psr->reboot_nb); input_unregister_handler(&psr->input_handler); kfree(psr->input_handler.name); kfree(psr);
From: Tomasz Figa tfiga@chromium.org
Currently PSR flush is triggered from CRTC's .atomic_begin() callback, which is executed after modeset disables and enables and before plane updates are committed. Since PSR flush and re-enable can be triggered asynchronously by external sources (input event, delayed work), it can race with hardware programming done in the aforementioned stages.
This patch blocks the PSR completely before hardware programming part begins and unblock after it ends. This relies on reference counted PSR disable introduced with previous patch.
Cc: Kristian H. Kristensen hoegsberg@chromium.org Signed-off-by: Tomasz Figa tfiga@chromium.org Signed-off-by: Sean Paul seanpaul@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 61 ++++++++++++++++++++++++++++- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 7 ---- 2 files changed, 60 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c index e266539e04e5..d4f4118b482d 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c @@ -167,8 +167,67 @@ rockchip_user_fb_create(struct drm_device *dev, struct drm_file *file_priv, return ERR_PTR(ret); }
+static void +rockchip_drm_psr_inhibit_get_state(struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + struct drm_encoder *encoder; + u32 encoder_mask = 0; + int i; + + for_each_old_crtc_in_state(state, crtc, crtc_state, i) { + encoder_mask |= crtc_state->encoder_mask; + encoder_mask |= crtc->state->encoder_mask; + } + + drm_for_each_encoder_mask(encoder, state->dev, encoder_mask) + rockchip_drm_psr_inhibit_get(encoder); +} + +static void +rockchip_drm_psr_inhibit_put_state(struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + struct drm_encoder *encoder; + u32 encoder_mask = 0; + int i; + + for_each_old_crtc_in_state(state, crtc, crtc_state, i) { + encoder_mask |= crtc_state->encoder_mask; + encoder_mask |= crtc->state->encoder_mask; + } + + drm_for_each_encoder_mask(encoder, state->dev, encoder_mask) + rockchip_drm_psr_inhibit_put(encoder); +} + +static void +rockchip_atomic_helper_commit_tail_rpm(struct drm_atomic_state *old_state) +{ + struct drm_device *dev = old_state->dev; + + rockchip_drm_psr_inhibit_get_state(old_state); + + drm_atomic_helper_commit_modeset_disables(dev, old_state); + + drm_atomic_helper_commit_modeset_enables(dev, old_state); + + drm_atomic_helper_commit_planes(dev, old_state, + DRM_PLANE_COMMIT_ACTIVE_ONLY); + + rockchip_drm_psr_inhibit_put_state(old_state); + + drm_atomic_helper_commit_hw_done(old_state); + + drm_atomic_helper_wait_for_vblanks(dev, old_state); + + drm_atomic_helper_cleanup_planes(dev, old_state); +} + static const struct drm_mode_config_helper_funcs rockchip_mode_config_helpers = { - .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm, + .atomic_commit_tail = rockchip_atomic_helper_commit_tail_rpm, };
static const struct drm_mode_config_funcs rockchip_drm_mode_config_funcs = { diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index e2c9ee3204f4..27e37b653b61 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -1027,16 +1027,9 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc, } }
-static void vop_crtc_atomic_begin(struct drm_crtc *crtc, - struct drm_crtc_state *old_crtc_state) -{ - rockchip_drm_psr_flush(crtc); -} - static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = { .mode_fixup = vop_crtc_mode_fixup, .atomic_flush = vop_crtc_atomic_flush, - .atomic_begin = vop_crtc_atomic_begin, .atomic_enable = vop_crtc_atomic_enable, .atomic_disable = vop_crtc_atomic_disable, };
From: Tomasz Figa tfiga@chromium.org
It is not used anymore after last changes and it was not even correct to begin with as it assumed a 1:1 relation between a CRTC and encoder, while in fact a CRTC can be attached to multiple encoders.
Signed-off-by: Tomasz Figa tfiga@chromium.org Signed-off-by: Thierry Escande thierry.escande@collabora.com Signed-off-by: Enric Balletbo i Serra enric.balletbo@collabora.com Tested-by: Marek Szyprowski m.szyprowski@samsung.com ---
drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 35 ----------------------------- drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 1 - 2 files changed, 36 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c index 1bf5cba9a64d..b1988ac758d5 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c @@ -40,23 +40,6 @@ struct psr_drv { int (*set)(struct drm_encoder *encoder, bool enable); };
-static struct psr_drv *find_psr_by_crtc(struct drm_crtc *crtc) -{ - struct rockchip_drm_private *drm_drv = crtc->dev->dev_private; - struct psr_drv *psr; - - mutex_lock(&drm_drv->psr_list_lock); - list_for_each_entry(psr, &drm_drv->psr_list, list) { - if (psr->encoder->crtc == crtc) - goto out; - } - psr = ERR_PTR(-ENODEV); - -out: - mutex_unlock(&drm_drv->psr_list_lock); - return psr; -} - static struct psr_drv *find_psr_by_encoder(struct drm_encoder *encoder) { struct rockchip_drm_private *drm_drv = encoder->dev->dev_private; @@ -173,24 +156,6 @@ static void rockchip_drm_do_flush(struct psr_drv *psr) mutex_unlock(&psr->lock); }
-/** - * rockchip_drm_psr_flush - flush a single pipe - * @crtc: CRTC of the pipe to flush - * - * Returns: - * 0 on success, -errno on fail - */ -int rockchip_drm_psr_flush(struct drm_crtc *crtc) -{ - struct psr_drv *psr = find_psr_by_crtc(crtc); - if (IS_ERR(psr)) - return PTR_ERR(psr); - - rockchip_drm_do_flush(psr); - return 0; -} -EXPORT_SYMBOL(rockchip_drm_psr_flush); - /** * rockchip_drm_psr_flush_all - force to flush all registered PSR encoders * @dev: drm device diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h index 40e026c14168..860c62494496 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h @@ -16,7 +16,6 @@ #define __ROCKCHIP_DRM_PSR___
void rockchip_drm_psr_flush_all(struct drm_device *dev); -int rockchip_drm_psr_flush(struct drm_crtc *crtc);
int rockchip_drm_psr_inhibit_put(struct drm_encoder *encoder); int rockchip_drm_psr_inhibit_get(struct drm_encoder *encoder);
Am Freitag, 9. März 2018, 23:22:51 CET schrieb Enric Balletbo i Serra:
Hi,
This patchset includes cleanups, improvements, and bug fixes for Rockchip DRM driver and PSR support.
This new version is the same as before removing some of the patches already applied and fixing the Exynos issue due patch '[v4 15/38] drm/bridge: analogix_dp: Ensure edp is disabled when shutting down the panel' reported by Marek.
The series on both rk3288 (without psr) and rk3399 (with psr) Tested-by: Heiko Stuebner heiko@sntech.de
I've tried reviewing at least the Rockchip parts, so if bridge- maintainers are going to apply patches to drm-misc they could pick up the interlinked Rockchip patches as well.
I don't think it's necessary to wait for the full series to be mergeable and before I just picked things from the beginning that looked ready and worked, so I guess continuing this way could help making that chunk of patches smaller :-) .
Heiko
dri-devel@lists.freedesktop.org
-
Archit Taneja -
Enric Balletbo i Serra -
Heiko Stuebner -
Heiko Stübner