mbox series

[RESEND,v6,00/27] DRM Rockchip rk3399 (Kevin)

Message ID 20180423105003.9004-1-enric.balletbo@collabora.com
Headers show
Series DRM Rockchip rk3399 (Kevin) | expand

Message

Enric Balletbo i Serra April 23, 2018, 10:49 a.m. UTC
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.

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(-)

Comments

Han Jingoo April 24, 2018, 1:25 a.m. UTC | #1
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
Han Jingoo April 24, 2018, 1:29 a.m. UTC | #2
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
Andrzej Hajda April 24, 2018, 6:43 a.m. UTC | #3
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(-)
>
Han Jingoo April 24, 2018, 1:54 p.m. UTC | #4
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, &reg);
>  		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
Han Jingoo April 24, 2018, 1:57 p.m. UTC | #5
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
Han Jingoo April 24, 2018, 1:58 p.m. UTC | #6
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
Han Jingoo April 24, 2018, 2:02 p.m. UTC | #7
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