Message ID | 20180423105003.9004-1-enric.balletbo@collabora.com |
---|---|
Headers | show |
Series | DRM Rockchip rk3399 (Kevin) | expand |
On Monday, April 23, 2018 6:50 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. > > 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> > Reviewed-by: Archit Taneja <architt@codeaurora.org> Acked-by: Jingoo Han <jingoohan1@gmail.com> Best regards, Jingoo Han > --- > > .../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, > -- > 2.17.0
On Monday, April 23, 2018 6:50 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. > > 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> > Reviewed-by: Archit Taneja <architt@codeaurora.org> Acked-by: Jingoo Han <jingoohan1@gmail.com> Best regards, Jingoo Han > --- > > .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 19 ++++++++++++++----- > .../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) > -- > 2.17.0
On 23.04.2018 12:49, Enric Balletbo i Serra wrote: > Hi Andrzej, > > This is the rebased version of v6 series. This patchset includes cleanups, > improvements, and bug fixes for Rockchip DRM driver and PSR support. > > This version is a RESEND of v6 with few changes. The following two > patches are not included and will be send in a separate patchset for > further discussion. > > - [PATCH v6 24/30] drm/rockchip: Disable PSR on input events > - [PATCH v6 25/30] drm/rockchip: Cancel PSR enable work before changing > the state > > The following patch was remove as is not needed. > > - [PATCH v6 28/30] drm/rockchip: Disable PSR from reboot notifier > > I think that the other patches have the required tags and are ready to > be merged. Queued to drm-misc-next. Regards Andrzej > > Regards, > Enric > > Changes in v6: > - Removed the following patches as are already applied. > [PATCH v5 01/36] drm/bridge: analogix_dp: detect Sink PSR state after > configuring the PSR > [PATCH v5 02/36] drm/rockchip: Remove analogix psr worker > [PATCH v5 03/36] drm/bridge: analogix_dp: Don't change psr while > bridge is disabled > - Explain in the commit message why we need to increase > the delay in the timeout loop in > [PATCH v5 07/36] drm/bridge: analogix_dp: Move enable video into > config_video() > - Add Reviewed-by: Archit Taneja <architt@codeaurora.org> for the > drm/bridge parts > - Add Reviewed-by: Heiko Stuebner <heiko@sntech.de> for > [PATCH v5 19/36] drm/rockchip: Restore psr->state when enable/disable > psr failed > > 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 > > 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 > > Tomasz Figa (5): > drm/rockchip: analogix_dp: Do not call Analogix code before bind > drm/rockchip: psr: Avoid redundant calls to .set() callback > drm/rockchip: psr: Sanitize semantics of allow/inhibit API > drm/rockchip: Disallow PSR for the whole atomic commit > drm/rockchip: psr: Remove flush by CRTC > > zain wang (11): > 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 > > .../drm/bridge/analogix/analogix_dp_core.c | 331 +++++++++++++----- > .../drm/bridge/analogix/analogix_dp_core.h | 5 +- > .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 236 +++++++------ > .../gpu/drm/bridge/analogix/analogix_dp_reg.h | 7 + > drivers/gpu/drm/exynos/exynos_dp.c | 2 +- > .../gpu/drm/rockchip/analogix_dp-rockchip.c | 37 +- > 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 | 158 ++++----- > drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 7 +- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 13 +- > drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 + > drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 1 + > include/drm/bridge/analogix_dp.h | 3 +- > 14 files changed, 559 insertions(+), 304 deletions(-) >
On Monday, April 23, 2018 6:50 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 > > 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> > Reviewed-by: Archit Taneja <architt@codeaurora.org> Acked-by: Jingoo Han <jingoohan1@gmail.com> Best regards, Jingoo Han > --- > > .../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 1e1743b59c77..75e61ebf6722 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; > -- > 2.17.0
On Monday, April 23, 2018 6:50 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. > > 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> > Reviewed-by: Archit Taneja <architt@codeaurora.org> Acked-by: Jingoo Han <jingoohan1@gmail.com> Best regards, Jingoo Han > --- > > .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 117 ++++++++++-------- > .../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) > -- > 2.17.0
On Monday, April 23, 2018 6:50 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. > > 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> > Reviewed-by: Archit Taneja <architt@codeaurora.org> Acked-by: Jingoo Han <jingoohan1@gmail.com> Best regards, Jingoo Han > --- > > .../drm/bridge/analogix/analogix_dp_core.c | 4 +++- > .../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 75e61ebf6722..5540e2dfc2ec 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__ */ > -- > 2.17.0
On Monday, April 23, 2018 6:50 AM, Enric Balletbo i Serra wrote: > > 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> > Reviewed-by: Archit Taneja <architt@codeaurora.org> Acked-by: Jingoo Han <jingoohan1@gmail.com> Best regards, Jingoo Han > --- > > 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 a260de4f0bd8..2bcbfadb6ac5 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 *); > -- > 2.17.0