Message ID | 20190326204509.96515-3-sean@poorly.run (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2,1/5] drm: Add helpers to kick off self refresh mode in drivers | expand |
Hi, Am Dienstag, 26. März 2019, 21:44:56 CET schrieb Sean Paul: > From: Sean Paul <seanpaul@chromium.org> > > Instead of rolling our own implementation for tracking when PSR should > be [in]active, use the new self refresh helpers to do the heavy lifting. I only got patches 3-5 and had to pull 1+2 from patchwork, the following applies to the whole series though. While my Kevin display does still generally work with these 5 patches applied functionality has regressed somehow. Environment is a standard Debian with framebuffer console and sddm login manager. With this patchset applied, when sddm starts I only get the see the login screen after I move the mouse, similarly the blinking cursor in the password field also only blinks when I actively move the cursor and stops when there is no mouse movement. With all 5 patches reverted this goes back to normal. So it looks like the PSR gets a bit too agressive with these? Heiko > Changes in v2: > - updated to reflect changes made in the helpers > > Link to v1: https://patchwork.freedesktop.org/patch/msgid/20190228210939.83386-4-sean@poorly.run > > Cc: Zain Wang <wzz@rock-chips.com> > Cc: Tomasz Figa <tfiga@chromium.org> > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- > .../drm/bridge/analogix/analogix_dp_core.c | 213 ++++++++----- > .../drm/bridge/analogix/analogix_dp_core.h | 2 +- > drivers/gpu/drm/rockchip/Makefile | 3 +- > .../gpu/drm/rockchip/analogix_dp-rockchip.c | 70 ++--- > drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 16 - > drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 290 ------------------ > drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 30 -- > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 33 +- > include/drm/bridge/analogix_dp.h | 4 +- > 9 files changed, 185 insertions(+), 476 deletions(-) > delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.c > delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.h > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > index af34554a5a02..c503fe3bc3b3 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > @@ -106,63 +106,13 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp) > return 0; > } > > -int analogix_dp_psr_enabled(struct analogix_dp_device *dp) > +bool analogix_dp_self_refresh_changed(struct analogix_dp_device *dp) > { > - > - return dp->psr_enable; > + return dp->connector.state->self_refresh_changed; > } > -EXPORT_SYMBOL_GPL(analogix_dp_psr_enabled); > +EXPORT_SYMBOL_GPL(analogix_dp_self_refresh_changed); > > -int analogix_dp_enable_psr(struct analogix_dp_device *dp) > -{ > - struct edp_vsc_psr psr_vsc; > - > - if (!dp->psr_enable) > - return 0; > - > - /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ > - memset(&psr_vsc, 0, sizeof(psr_vsc)); > - psr_vsc.sdp_header.HB0 = 0; > - psr_vsc.sdp_header.HB1 = 0x7; > - psr_vsc.sdp_header.HB2 = 0x2; > - psr_vsc.sdp_header.HB3 = 0x8; > - > - 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, true); > -} > -EXPORT_SYMBOL_GPL(analogix_dp_enable_psr); > - > -int analogix_dp_disable_psr(struct analogix_dp_device *dp) > -{ > - struct edp_vsc_psr psr_vsc; > - int ret; > - > - if (!dp->psr_enable) > - return 0; > - > - /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ > - memset(&psr_vsc, 0, sizeof(psr_vsc)); > - psr_vsc.sdp_header.HB0 = 0; > - psr_vsc.sdp_header.HB1 = 0x7; > - psr_vsc.sdp_header.HB2 = 0x2; > - psr_vsc.sdp_header.HB3 = 0x8; > - > - psr_vsc.DB0 = 0; > - psr_vsc.DB1 = 0; > - > - ret = drm_dp_dpcd_writeb(&dp->aux, DP_SET_POWER, DP_SET_POWER_D0); > - 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); > -} > -EXPORT_SYMBOL_GPL(analogix_dp_disable_psr); > - > -static int analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) > +static bool analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) > { > unsigned char psr_version; > int ret; > @@ -170,14 +120,11 @@ static int analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) > 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; > + return false; > } > > dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version); > - > - dp->psr_enable = (psr_version & DP_PSR_IS_SUPPORTED) ? true : false; > - > - return 0; > + return psr_version & DP_PSR_IS_SUPPORTED; > } > > static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) > @@ -200,7 +147,7 @@ static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) > } > > /* Main-Link transmitter remains active during PSR active states */ > - psr_en = DP_PSR_MAIN_LINK_ACTIVE | DP_PSR_CRC_VERIFICATION; > + psr_en = DP_PSR_CRC_VERIFICATION; > 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"); > @@ -208,8 +155,7 @@ static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) > } > > /* Enable psr function */ > - psr_en = DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE | > - DP_PSR_CRC_VERIFICATION; > + psr_en = DP_PSR_ENABLE | DP_PSR_CRC_VERIFICATION; > 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"); > @@ -218,10 +164,11 @@ static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) > > analogix_dp_enable_psr_crc(dp); > > + dp->psr_supported = true; > + > return 0; > end: > dev_err(dp->dev, "enable psr fail, force to disable psr\n"); > - dp->psr_enable = false; > > return ret; > } > @@ -1036,25 +983,90 @@ static int analogix_dp_commit(struct analogix_dp_device *dp) > } > } > > - ret = analogix_dp_detect_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; > > - if (dp->psr_enable) { > + if (analogix_dp_detect_sink_psr(dp)) { > ret = analogix_dp_enable_sink_psr(dp); > if (ret) > return ret; > } > > + return ret; > +} > + > +static int analogix_dp_enable_psr(struct analogix_dp_device *dp) > +{ > + struct edp_vsc_psr psr_vsc; > + int ret; > + u8 sink; > + > + ret = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink); > + if (ret != 1) > + DRM_DEV_ERROR(dp->dev, "Failed to read psr status %d\n", ret); > + else if (sink == DP_PSR_SINK_ACTIVE_RFB) > + return 0; > + > + /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ > + memset(&psr_vsc, 0, sizeof(psr_vsc)); > + psr_vsc.sdp_header.HB0 = 0; > + psr_vsc.sdp_header.HB1 = 0x7; > + psr_vsc.sdp_header.HB2 = 0x2; > + psr_vsc.sdp_header.HB3 = 0x8; > + psr_vsc.DB0 = 0; > + psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID; > + > + ret = analogix_dp_send_psr_spd(dp, &psr_vsc, true); > + if (!ret) > + analogix_dp_set_analog_power_down(dp, POWER_ALL, true); > > return ret; > } > > +static int analogix_dp_disable_psr(struct analogix_dp_device *dp) > +{ > + struct edp_vsc_psr psr_vsc; > + int ret; > + u8 sink; > + > + analogix_dp_set_analog_power_down(dp, POWER_ALL, false); > + > + ret = drm_dp_dpcd_writeb(&dp->aux, DP_SET_POWER, DP_SET_POWER_D0); > + if (ret != 1) { > + DRM_DEV_ERROR(dp->dev, "Failed to set DP Power0 %d\n", ret); > + return ret; > + } > + > + ret = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink); > + if (ret != 1) { > + DRM_DEV_ERROR(dp->dev, "Failed to read psr status %d\n", ret); > + return ret; > + } else if (sink == DP_PSR_SINK_INACTIVE) { > + DRM_DEV_ERROR(dp->dev, "sink inactive, skip disable psr"); > + return 0; > + } > + > + ret = analogix_dp_train_link(dp); > + if (ret) { > + DRM_DEV_ERROR(dp->dev, "Failed to train the link %d\n", ret); > + return ret; > + } > + > + /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ > + memset(&psr_vsc, 0, sizeof(psr_vsc)); > + psr_vsc.sdp_header.HB0 = 0; > + psr_vsc.sdp_header.HB1 = 0x7; > + psr_vsc.sdp_header.HB2 = 0x2; > + psr_vsc.sdp_header.HB3 = 0x8; > + > + psr_vsc.DB0 = 0; > + psr_vsc.DB1 = 0; > + > + return analogix_dp_send_psr_spd(dp, &psr_vsc, true); > +} > + > /* > * This function is a bit of a catch-all for panel preparation, hopefully > * simplifying the logic of functions that need to prepare/unprepare the panel > @@ -1145,9 +1157,23 @@ analogix_dp_best_encoder(struct drm_connector *connector) > return dp->encoder; > } > > + > +int analogix_dp_atomic_check(struct drm_connector *connector, > + struct drm_connector_state *state) > +{ > + struct analogix_dp_device *dp = to_dp(connector); > + > + state->self_refresh_aware = true; > + if (state->self_refresh_active && !dp->psr_supported) > + return -EINVAL; > + > + return 0; > +} > + > static const struct drm_connector_helper_funcs analogix_dp_connector_helper_funcs = { > .get_modes = analogix_dp_get_modes, > .best_encoder = analogix_dp_best_encoder, > + .atomic_check = analogix_dp_atomic_check, > }; > > static enum drm_connector_status > @@ -1244,6 +1270,10 @@ static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge) > struct analogix_dp_device *dp = bridge->driver_private; > int ret; > > + /* Don't touch the panel if we're coming back from PSR */ > + if (dp->connector.state->self_refresh_changed) > + return; > + > ret = analogix_dp_prepare_panel(dp, true, true); > if (ret) > DRM_ERROR("failed to setup the panel ret = %d\n", ret); > @@ -1309,6 +1339,14 @@ static void analogix_dp_bridge_enable(struct drm_bridge *bridge) > struct analogix_dp_device *dp = bridge->driver_private; > int timeout_loop = 0; > > + /* Not a full enable, just disable PSR and continue */ > + if (dp->connector.state->self_refresh_changed) { > + int ret = analogix_dp_disable_psr(dp); > + if (ret) > + DRM_ERROR("Failed to disable psr %d\n", ret); > + return; > + } > + > if (dp->dpms_mode == DRM_MODE_DPMS_ON) > return; > > @@ -1356,11 +1394,37 @@ 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->fast_train_enable = false; > + dp->psr_supported = false; > dp->dpms_mode = DRM_MODE_DPMS_OFF; > } > > +static void analogix_dp_bridge_atomic_disable(struct drm_bridge *bridge) > +{ > + struct analogix_dp_device *dp = bridge->driver_private; > + struct drm_connector_state *conn_state = dp->connector.state; > + > + /* Don't do a full disable on PSR transitions */ > + if (conn_state->crtc && conn_state->self_refresh_changed) > + return; > + > + analogix_dp_bridge_disable(bridge); > +} > + > +static void analogix_dp_bridge_post_disable(struct drm_bridge *bridge) > +{ > + struct analogix_dp_device *dp = bridge->driver_private; > + struct drm_connector_state *conn_state = dp->connector.state; > + int ret; > + > + if (conn_state->self_refresh_active) { > + ret = analogix_dp_enable_psr(dp); > + if (ret) > + DRM_ERROR("Failed to enable psr (%d)\n", ret); > + return; > + } > +} > + > static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge, > const struct drm_display_mode *orig_mode, > const struct drm_display_mode *mode) > @@ -1440,16 +1504,11 @@ static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge, > video->interlaced = true; > } > > -static void analogix_dp_bridge_nop(struct drm_bridge *bridge) > -{ > - /* do nothing */ > -} > - > static const struct drm_bridge_funcs analogix_dp_bridge_funcs = { > .pre_enable = analogix_dp_bridge_pre_enable, > .enable = analogix_dp_bridge_enable, > - .disable = analogix_dp_bridge_disable, > - .post_disable = analogix_dp_bridge_nop, > + .disable = analogix_dp_bridge_atomic_disable, > + .post_disable = analogix_dp_bridge_post_disable, > .mode_set = analogix_dp_bridge_mode_set, > .attach = analogix_dp_bridge_attach, > }; > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h > index 769255dc6e99..d937fa3d9ee8 100644 > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h > @@ -173,8 +173,8 @@ struct analogix_dp_device { > int dpms_mode; > int hpd_gpio; > bool force_hpd; > - bool psr_enable; > bool fast_train_enable; > + bool psr_supported; > > struct mutex panel_lock; > bool panel_is_modeset; > diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile > index f6fc9d5dd0ad..a777726c23cb 100644 > --- a/drivers/gpu/drm/rockchip/Makefile > +++ b/drivers/gpu/drm/rockchip/Makefile > @@ -4,8 +4,7 @@ > # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. > > rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ > - rockchip_drm_gem.o rockchip_drm_psr.o \ > - rockchip_drm_vop.o rockchip_vop_reg.o > + rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o > rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o > > rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o > diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > index bc4423624209..8f42c38848db 100644 > --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > @@ -32,7 +32,6 @@ > #include <drm/bridge/analogix_dp.h> > > #include "rockchip_drm_drv.h" > -#include "rockchip_drm_psr.h" > #include "rockchip_drm_vop.h" > > #define RK3288_GRF_SOC_CON6 0x25c > @@ -77,29 +76,6 @@ struct rockchip_dp_device { > struct analogix_dp_plat_data plat_data; > }; > > -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 0; > - > - DRM_DEV_DEBUG(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit"); > - > - ret = rockchip_drm_wait_vact_end(dp->encoder.crtc, > - PSR_WAIT_LINE_FLAG_TIMEOUT_MS); > - if (ret) { > - DRM_DEV_ERROR(dp->dev, "line flag interrupt did not arrive\n"); > - return -ETIMEDOUT; > - } > - > - if (enabled) > - return analogix_dp_enable_psr(dp->adp); > - else > - return analogix_dp_disable_psr(dp->adp); > -} > - > static int rockchip_dp_pre_init(struct rockchip_dp_device *dp) > { > reset_control_assert(dp->rst); > @@ -130,21 +106,9 @@ static int rockchip_dp_poweron_start(struct analogix_dp_plat_data *plat_data) > 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_inhibit_put(&dp->encoder); > -} > - > 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_inhibit_get(&dp->encoder); > - if (ret != 0) > - return ret; > > clk_disable_unprepare(dp->pclk); > > @@ -190,6 +154,9 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder) > int ret; > u32 val; > > + if (analogix_dp_self_refresh_changed(dp->adp)) > + return; > + > ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder); > if (ret < 0) > return; > @@ -214,9 +181,24 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder) > clk_disable_unprepare(dp->grfclk); > } > > -static void rockchip_dp_drm_encoder_nop(struct drm_encoder *encoder) > +static void rockchip_dp_drm_encoder_disable(struct drm_encoder *encoder) > { > - /* do nothing */ > + struct rockchip_dp_device *dp = to_dp(encoder); > + struct drm_crtc *crtc; > + int ret; > + > + drm_for_each_crtc(crtc, dp->drm_dev) { > + if (!(crtc->state->encoder_mask & drm_encoder_mask(encoder))) > + continue; > + > + if (!crtc->state->self_refresh_active) > + continue; > + > + ret = rockchip_drm_wait_vact_end(crtc, > + PSR_WAIT_LINE_FLAG_TIMEOUT_MS); > + if (ret) > + DRM_DEV_ERROR(dp->dev, "line flag irq timed out\n"); > + } > } > > static int > @@ -246,7 +228,7 @@ static struct drm_encoder_helper_funcs rockchip_dp_encoder_helper_funcs = { > .mode_fixup = rockchip_dp_drm_encoder_mode_fixup, > .mode_set = rockchip_dp_drm_encoder_mode_set, > .enable = rockchip_dp_drm_encoder_enable, > - .disable = rockchip_dp_drm_encoder_nop, > + .disable = rockchip_dp_drm_encoder_disable, > .atomic_check = rockchip_dp_drm_encoder_atomic_check, > }; > > @@ -338,23 +320,16 @@ static int rockchip_dp_bind(struct device *dev, struct device *master, > > dp->plat_data.dev_type = dp->data->chip_type; > 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; > > - ret = rockchip_drm_psr_register(&dp->encoder, analogix_dp_psr_set); > - if (ret < 0) > - goto err_cleanup_encoder; > - > dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data); > if (IS_ERR(dp->adp)) { > ret = PTR_ERR(dp->adp); > - goto err_unreg_psr; > + goto err_cleanup_encoder; > } > > return 0; > -err_unreg_psr: > - rockchip_drm_psr_unregister(&dp->encoder); > err_cleanup_encoder: > dp->encoder.funcs->destroy(&dp->encoder); > return ret; > @@ -366,7 +341,6 @@ static void rockchip_dp_unbind(struct device *dev, struct device *master, > struct rockchip_dp_device *dp = dev_get_drvdata(dev); > > analogix_dp_unbind(dp->adp); > - rockchip_drm_psr_unregister(&dp->encoder); > dp->encoder.funcs->destroy(&dp->encoder); > > dp->adp = ERR_PTR(-ENODEV); > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > index 97438bbbe389..7e121875e3c9 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > @@ -23,22 +23,10 @@ > #include "rockchip_drm_drv.h" > #include "rockchip_drm_fb.h" > #include "rockchip_drm_gem.h" > -#include "rockchip_drm_psr.h" > - > -static int rockchip_drm_fb_dirty(struct drm_framebuffer *fb, > - struct drm_file *file, > - unsigned int flags, unsigned int color, > - struct drm_clip_rect *clips, > - unsigned int num_clips) > -{ > - rockchip_drm_psr_flush_all(fb->dev); > - return 0; > -} > > static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = { > .destroy = drm_gem_fb_destroy, > .create_handle = drm_gem_fb_create_handle, > - .dirty = rockchip_drm_fb_dirty, > }; > > static struct drm_framebuffer * > @@ -132,8 +120,6 @@ 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); > @@ -141,8 +127,6 @@ rockchip_atomic_helper_commit_tail_rpm(struct drm_atomic_state *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); > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c > deleted file mode 100644 > index a0c8bd235b67..000000000000 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c > +++ /dev/null > @@ -1,290 +0,0 @@ > -/* > - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd > - * Author: Yakir Yang <ykk@rock-chips.com> > - * > - * This software is licensed under the terms of the GNU General Public > - * License version 2, as published by the Free Software Foundation, and > - * may be copied, distributed, and modified under those terms. > - * > - * This program is distributed in the hope that it will be useful, > - * but WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > - * GNU General Public License for more details. > - */ > - > -#include <drm/drmP.h> > -#include <drm/drm_atomic.h> > -#include <drm/drm_probe_helper.h> > - > -#include "rockchip_drm_drv.h" > -#include "rockchip_drm_psr.h" > - > -#define PSR_FLUSH_TIMEOUT_MS 100 > - > -struct psr_drv { > - struct list_head list; > - struct drm_encoder *encoder; > - > - struct mutex lock; > - int inhibit_count; > - bool enabled; > - > - struct delayed_work flush_work; > - > - int (*set)(struct drm_encoder *encoder, bool enable); > -}; > - > -static struct psr_drv *find_psr_by_encoder(struct drm_encoder *encoder) > -{ > - struct rockchip_drm_private *drm_drv = encoder->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 == encoder) > - goto out; > - } > - psr = ERR_PTR(-ENODEV); > - > -out: > - mutex_unlock(&drm_drv->psr_list_lock); > - return psr; > -} > - > -static int psr_set_state_locked(struct psr_drv *psr, bool enable) > -{ > - int ret; > - > - if (psr->inhibit_count > 0) > - return -EINVAL; > - > - if (enable == psr->enabled) > - return 0; > - > - ret = psr->set(psr->encoder, enable); > - if (ret) > - return ret; > - > - psr->enabled = enable; > - return 0; > -} > - > -static void psr_flush_handler(struct work_struct *work) > -{ > - struct psr_drv *psr = container_of(to_delayed_work(work), > - struct psr_drv, flush_work); > - > - mutex_lock(&psr->lock); > - psr_set_state_locked(psr, true); > - mutex_unlock(&psr->lock); > -} > - > -/** > - * 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_inhibit_put(struct drm_encoder *encoder) > -{ > - struct psr_drv *psr = find_psr_by_encoder(encoder); > - > - if (IS_ERR(psr)) > - return PTR_ERR(psr); > - > - mutex_lock(&psr->lock); > - --psr->inhibit_count; > - WARN_ON(psr->inhibit_count < 0); > - 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_inhibit_put); > - > -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); > -} > -EXPORT_SYMBOL(rockchip_drm_psr_inhibit_get_state); > - > -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); > -} > -EXPORT_SYMBOL(rockchip_drm_psr_inhibit_put_state); > - > -/** > - * 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_inhibit_get(struct drm_encoder *encoder) > -{ > - struct psr_drv *psr = find_psr_by_encoder(encoder); > - > - if (IS_ERR(psr)) > - return PTR_ERR(psr); > - > - mutex_lock(&psr->lock); > - psr_set_state_locked(psr, false); > - ++psr->inhibit_count; > - mutex_unlock(&psr->lock); > - cancel_delayed_work_sync(&psr->flush_work); > - > - return 0; > -} > -EXPORT_SYMBOL(rockchip_drm_psr_inhibit_get); > - > -static void rockchip_drm_do_flush(struct psr_drv *psr) > -{ > - cancel_delayed_work_sync(&psr->flush_work); > - > - 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); > -} > - > -/** > - * rockchip_drm_psr_flush_all - force to flush all registered PSR encoders > - * @dev: drm device > - * > - * Disable the PSR function for all registered encoders, and then enable the > - * PSR function back after PSR_FLUSH_TIMEOUT. If encoder PSR state have been > - * changed during flush time, then keep the state no change after flush > - * timeout. > - * > - * Returns: > - * Zero on success, negative errno on failure. > - */ > -void rockchip_drm_psr_flush_all(struct drm_device *dev) > -{ > - struct rockchip_drm_private *drm_drv = dev->dev_private; > - struct psr_drv *psr; > - > - mutex_lock(&drm_drv->psr_list_lock); > - list_for_each_entry(psr, &drm_drv->psr_list, list) > - rockchip_drm_do_flush(psr); > - mutex_unlock(&drm_drv->psr_list_lock); > -} > -EXPORT_SYMBOL(rockchip_drm_psr_flush_all); > - > -/** > - * rockchip_drm_psr_register - register encoder to psr driver > - * @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. > - */ > -int rockchip_drm_psr_register(struct drm_encoder *encoder, > - int (*psr_set)(struct drm_encoder *, bool enable)) > -{ > - struct rockchip_drm_private *drm_drv; > - struct psr_drv *psr; > - > - if (!encoder || !psr_set) > - return -EINVAL; > - > - drm_drv = encoder->dev->dev_private; > - > - psr = kzalloc(sizeof(struct psr_drv), GFP_KERNEL); > - if (!psr) > - return -ENOMEM; > - > - INIT_DELAYED_WORK(&psr->flush_work, psr_flush_handler); > - mutex_init(&psr->lock); > - > - psr->inhibit_count = 1; > - psr->enabled = false; > - psr->encoder = encoder; > - psr->set = psr_set; > - > - 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; > -} > -EXPORT_SYMBOL(rockchip_drm_psr_register); > - > -/** > - * rockchip_drm_psr_unregister - unregister encoder to psr driver > - * @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. > - */ > -void rockchip_drm_psr_unregister(struct drm_encoder *encoder) > -{ > - struct rockchip_drm_private *drm_drv = encoder->dev->dev_private; > - struct psr_drv *psr, *n; > - > - mutex_lock(&drm_drv->psr_list_lock); > - list_for_each_entry_safe(psr, n, &drm_drv->psr_list, list) { > - if (psr->encoder == encoder) { > - /* > - * Any other value would mean that the encoder > - * is still in use. > - */ > - WARN_ON(psr->inhibit_count != 1); > - > - list_del(&psr->list); > - kfree(psr); > - } > - } > - mutex_unlock(&drm_drv->psr_list_lock); > -} > -EXPORT_SYMBOL(rockchip_drm_psr_unregister); > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h > deleted file mode 100644 > index 25350ba3237b..000000000000 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h > +++ /dev/null > @@ -1,30 +0,0 @@ > -/* > - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd > - * Author: Yakir Yang <ykk@rock-chips.com> > - * > - * This software is licensed under the terms of the GNU General Public > - * License version 2, as published by the Free Software Foundation, and > - * may be copied, distributed, and modified under those terms. > - * > - * This program is distributed in the hope that it will be useful, > - * but WITHOUT ANY WARRANTY; without even the implied warranty of > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > - * GNU General Public License for more details. > - */ > - > -#ifndef __ROCKCHIP_DRM_PSR___ > -#define __ROCKCHIP_DRM_PSR___ > - > -void rockchip_drm_psr_flush_all(struct drm_device *dev); > - > -int rockchip_drm_psr_inhibit_put(struct drm_encoder *encoder); > -int rockchip_drm_psr_inhibit_get(struct drm_encoder *encoder); > - > -void rockchip_drm_psr_inhibit_get_state(struct drm_atomic_state *state); > -void rockchip_drm_psr_inhibit_put_state(struct drm_atomic_state *state); > - > -int rockchip_drm_psr_register(struct drm_encoder *encoder, > - int (*psr_set)(struct drm_encoder *, bool enable)); > -void rockchip_drm_psr_unregister(struct drm_encoder *encoder); > - > -#endif /* __ROCKCHIP_DRM_PSR__ */ > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index c7d4c6073ea5..21fccda70356 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -21,6 +21,7 @@ > #include <drm/drm_gem_framebuffer_helper.h> > #include <drm/drm_plane_helper.h> > #include <drm/drm_probe_helper.h> > +#include <drm/drm_self_refresh_helper.h> > #ifdef CONFIG_DRM_ANALOGIX_DP > #include <drm/bridge/analogix_dp.h> > #endif > @@ -42,10 +43,11 @@ > #include "rockchip_drm_drv.h" > #include "rockchip_drm_gem.h" > #include "rockchip_drm_fb.h" > -#include "rockchip_drm_psr.h" > #include "rockchip_drm_vop.h" > #include "rockchip_rgb.h" > > +#define VOP_SELF_REFRESH_ENTRY_DELAY_MS 100 > + > #define VOP_WIN_SET(vop, win, name, v) \ > vop_reg_set(vop, &win->phy->name, win->base, ~0, v, #name) > #define VOP_SCL_SET(vop, win, name, v) \ > @@ -541,7 +543,7 @@ static void vop_core_clks_disable(struct vop *vop) > clk_disable(vop->hclk); > } > > -static int vop_enable(struct drm_crtc *crtc) > +static int vop_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) > { > struct vop *vop = to_vop(crtc); > int ret, i; > @@ -581,12 +583,18 @@ static int vop_enable(struct drm_crtc *crtc) > * We need to make sure that all windows are disabled before we > * enable the crtc. Otherwise we might try to scan from a destroyed > * buffer later. > + * > + * In the case of enable-after-PSR, we don't need to worry about this > + * case since the buffer is guaranteed to be valid and disabling the > + * window will result in screen glitches on PSR exit. > */ > - for (i = 0; i < vop->data->win_size; i++) { > - struct vop_win *vop_win = &vop->win[i]; > - const struct vop_win_data *win = vop_win->data; > + if (!old_state || !old_state->self_refresh_active) { > + for (i = 0; i < vop->data->win_size; i++) { > + struct vop_win *vop_win = &vop->win[i]; > + const struct vop_win_data *win = vop_win->data; > > - VOP_WIN_SET(vop, win, enable, 0); > + VOP_WIN_SET(vop, win, enable, 0); > + } > } > spin_unlock(&vop->reg_lock); > > @@ -937,12 +945,10 @@ static void vop_plane_atomic_async_update(struct drm_plane *plane, > } > > if (vop->is_enabled) { > - rockchip_drm_psr_inhibit_get_state(new_state->state); > vop_plane_atomic_update(plane, plane->state); > spin_lock(&vop->reg_lock); > vop_cfg_done(vop); > spin_unlock(&vop->reg_lock); > - rockchip_drm_psr_inhibit_put_state(new_state->state); > } > > plane->funcs->atomic_destroy_state(plane, plane_state); > @@ -1035,7 +1041,7 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc, > > WARN_ON(vop->event); > > - ret = vop_enable(crtc); > + ret = vop_enable(crtc, old_state); > if (ret) { > mutex_unlock(&vop->vop_lock); > DRM_DEV_ERROR(vop->dev, "Failed to enable vop (%d)\n", ret); > @@ -1509,6 +1515,13 @@ static int vop_create_crtc(struct vop *vop) > init_completion(&vop->line_flag_completion); > crtc->port = port; > > + ret = drm_self_refresh_helper_register(crtc, > + VOP_SELF_REFRESH_ENTRY_DELAY_MS); > + if (ret) > + DRM_DEV_DEBUG_KMS(vop->dev, > + "Failed to register %s with SR helpers %d, ignoring\n", > + crtc->name, ret); > + > return 0; > > err_cleanup_crtc: > @@ -1526,6 +1539,8 @@ static void vop_destroy_crtc(struct vop *vop) > struct drm_device *drm_dev = vop->drm_dev; > struct drm_plane *plane, *tmp; > > + drm_self_refresh_helper_unregister(crtc); > + > of_node_put(crtc->port); > > /* > diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h > index 475b706b49de..b6cc654143d7 100644 > --- a/include/drm/bridge/analogix_dp.h > +++ b/include/drm/bridge/analogix_dp.h > @@ -42,9 +42,7 @@ struct analogix_dp_plat_data { > struct drm_connector *); > }; > > -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); > +bool analogix_dp_self_refresh_changed(struct analogix_dp_device *dp); > > int analogix_dp_resume(struct analogix_dp_device *dp); > int analogix_dp_suspend(struct analogix_dp_device *dp); >
On Fri, Mar 29, 2019 at 07:51:51PM +0100, Heiko Stübner wrote: > Hi, > > Am Dienstag, 26. März 2019, 21:44:56 CET schrieb Sean Paul: > > From: Sean Paul <seanpaul@chromium.org> > > > > Instead of rolling our own implementation for tracking when PSR should > > be [in]active, use the new self refresh helpers to do the heavy lifting. > > I only got patches 3-5 and had to pull 1+2 from patchwork, the following > applies to the whole series though. > > While my Kevin display does still generally work with these 5 patches > applied functionality has regressed somehow. > > Environment is a standard Debian with framebuffer console and sddm > login manager. Ahh, this is probably b/c fb_dirty is not triggering PSR exit. I had it working in v1, but removed it in v2 since Daniel suggested we require the fb_dirty helpers. I neglected to add support for the fb_dirty helpers, so fbcon updates won't cause the screen to refresh. I'll add the helpers in v3 and make sure I test it out with fbcon. Thanks for reporting this! Sean > > With this patchset applied, when sddm starts I only get the see the > login screen after I move the mouse, similarly the blinking cursor in > the password field also only blinks when I actively move the cursor > and stops when there is no mouse movement. > > With all 5 patches reverted this goes back to normal. > > So it looks like the PSR gets a bit too agressive with these? > > > Heiko > > > Changes in v2: > > - updated to reflect changes made in the helpers > > > > Link to v1: https://patchwork.freedesktop.org/patch/msgid/20190228210939.83386-4-sean@poorly.run > > > > Cc: Zain Wang <wzz@rock-chips.com> > > Cc: Tomasz Figa <tfiga@chromium.org> > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > > --- > > .../drm/bridge/analogix/analogix_dp_core.c | 213 ++++++++----- > > .../drm/bridge/analogix/analogix_dp_core.h | 2 +- > > drivers/gpu/drm/rockchip/Makefile | 3 +- > > .../gpu/drm/rockchip/analogix_dp-rockchip.c | 70 ++--- > > drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 16 - > > drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 290 ------------------ > > drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 30 -- > > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 33 +- > > include/drm/bridge/analogix_dp.h | 4 +- > > 9 files changed, 185 insertions(+), 476 deletions(-) > > delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.c > > delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.h > > > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > index af34554a5a02..c503fe3bc3b3 100644 > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c > > @@ -106,63 +106,13 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp) > > return 0; > > } > > > > -int analogix_dp_psr_enabled(struct analogix_dp_device *dp) > > +bool analogix_dp_self_refresh_changed(struct analogix_dp_device *dp) > > { > > - > > - return dp->psr_enable; > > + return dp->connector.state->self_refresh_changed; > > } > > -EXPORT_SYMBOL_GPL(analogix_dp_psr_enabled); > > +EXPORT_SYMBOL_GPL(analogix_dp_self_refresh_changed); > > > > -int analogix_dp_enable_psr(struct analogix_dp_device *dp) > > -{ > > - struct edp_vsc_psr psr_vsc; > > - > > - if (!dp->psr_enable) > > - return 0; > > - > > - /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ > > - memset(&psr_vsc, 0, sizeof(psr_vsc)); > > - psr_vsc.sdp_header.HB0 = 0; > > - psr_vsc.sdp_header.HB1 = 0x7; > > - psr_vsc.sdp_header.HB2 = 0x2; > > - psr_vsc.sdp_header.HB3 = 0x8; > > - > > - 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, true); > > -} > > -EXPORT_SYMBOL_GPL(analogix_dp_enable_psr); > > - > > -int analogix_dp_disable_psr(struct analogix_dp_device *dp) > > -{ > > - struct edp_vsc_psr psr_vsc; > > - int ret; > > - > > - if (!dp->psr_enable) > > - return 0; > > - > > - /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ > > - memset(&psr_vsc, 0, sizeof(psr_vsc)); > > - psr_vsc.sdp_header.HB0 = 0; > > - psr_vsc.sdp_header.HB1 = 0x7; > > - psr_vsc.sdp_header.HB2 = 0x2; > > - psr_vsc.sdp_header.HB3 = 0x8; > > - > > - psr_vsc.DB0 = 0; > > - psr_vsc.DB1 = 0; > > - > > - ret = drm_dp_dpcd_writeb(&dp->aux, DP_SET_POWER, DP_SET_POWER_D0); > > - 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); > > -} > > -EXPORT_SYMBOL_GPL(analogix_dp_disable_psr); > > - > > -static int analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) > > +static bool analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) > > { > > unsigned char psr_version; > > int ret; > > @@ -170,14 +120,11 @@ static int analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) > > 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; > > + return false; > > } > > > > dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version); > > - > > - dp->psr_enable = (psr_version & DP_PSR_IS_SUPPORTED) ? true : false; > > - > > - return 0; > > + return psr_version & DP_PSR_IS_SUPPORTED; > > } > > > > static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) > > @@ -200,7 +147,7 @@ static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) > > } > > > > /* Main-Link transmitter remains active during PSR active states */ > > - psr_en = DP_PSR_MAIN_LINK_ACTIVE | DP_PSR_CRC_VERIFICATION; > > + psr_en = DP_PSR_CRC_VERIFICATION; > > 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"); > > @@ -208,8 +155,7 @@ static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) > > } > > > > /* Enable psr function */ > > - psr_en = DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE | > > - DP_PSR_CRC_VERIFICATION; > > + psr_en = DP_PSR_ENABLE | DP_PSR_CRC_VERIFICATION; > > 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"); > > @@ -218,10 +164,11 @@ static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) > > > > analogix_dp_enable_psr_crc(dp); > > > > + dp->psr_supported = true; > > + > > return 0; > > end: > > dev_err(dp->dev, "enable psr fail, force to disable psr\n"); > > - dp->psr_enable = false; > > > > return ret; > > } > > @@ -1036,25 +983,90 @@ static int analogix_dp_commit(struct analogix_dp_device *dp) > > } > > } > > > > - ret = analogix_dp_detect_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; > > > > - if (dp->psr_enable) { > > + if (analogix_dp_detect_sink_psr(dp)) { > > ret = analogix_dp_enable_sink_psr(dp); > > if (ret) > > return ret; > > } > > > > + return ret; > > +} > > + > > +static int analogix_dp_enable_psr(struct analogix_dp_device *dp) > > +{ > > + struct edp_vsc_psr psr_vsc; > > + int ret; > > + u8 sink; > > + > > + ret = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink); > > + if (ret != 1) > > + DRM_DEV_ERROR(dp->dev, "Failed to read psr status %d\n", ret); > > + else if (sink == DP_PSR_SINK_ACTIVE_RFB) > > + return 0; > > + > > + /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ > > + memset(&psr_vsc, 0, sizeof(psr_vsc)); > > + psr_vsc.sdp_header.HB0 = 0; > > + psr_vsc.sdp_header.HB1 = 0x7; > > + psr_vsc.sdp_header.HB2 = 0x2; > > + psr_vsc.sdp_header.HB3 = 0x8; > > + psr_vsc.DB0 = 0; > > + psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID; > > + > > + ret = analogix_dp_send_psr_spd(dp, &psr_vsc, true); > > + if (!ret) > > + analogix_dp_set_analog_power_down(dp, POWER_ALL, true); > > > > return ret; > > } > > > > +static int analogix_dp_disable_psr(struct analogix_dp_device *dp) > > +{ > > + struct edp_vsc_psr psr_vsc; > > + int ret; > > + u8 sink; > > + > > + analogix_dp_set_analog_power_down(dp, POWER_ALL, false); > > + > > + ret = drm_dp_dpcd_writeb(&dp->aux, DP_SET_POWER, DP_SET_POWER_D0); > > + if (ret != 1) { > > + DRM_DEV_ERROR(dp->dev, "Failed to set DP Power0 %d\n", ret); > > + return ret; > > + } > > + > > + ret = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink); > > + if (ret != 1) { > > + DRM_DEV_ERROR(dp->dev, "Failed to read psr status %d\n", ret); > > + return ret; > > + } else if (sink == DP_PSR_SINK_INACTIVE) { > > + DRM_DEV_ERROR(dp->dev, "sink inactive, skip disable psr"); > > + return 0; > > + } > > + > > + ret = analogix_dp_train_link(dp); > > + if (ret) { > > + DRM_DEV_ERROR(dp->dev, "Failed to train the link %d\n", ret); > > + return ret; > > + } > > + > > + /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ > > + memset(&psr_vsc, 0, sizeof(psr_vsc)); > > + psr_vsc.sdp_header.HB0 = 0; > > + psr_vsc.sdp_header.HB1 = 0x7; > > + psr_vsc.sdp_header.HB2 = 0x2; > > + psr_vsc.sdp_header.HB3 = 0x8; > > + > > + psr_vsc.DB0 = 0; > > + psr_vsc.DB1 = 0; > > + > > + return analogix_dp_send_psr_spd(dp, &psr_vsc, true); > > +} > > + > > /* > > * This function is a bit of a catch-all for panel preparation, hopefully > > * simplifying the logic of functions that need to prepare/unprepare the panel > > @@ -1145,9 +1157,23 @@ analogix_dp_best_encoder(struct drm_connector *connector) > > return dp->encoder; > > } > > > > + > > +int analogix_dp_atomic_check(struct drm_connector *connector, > > + struct drm_connector_state *state) > > +{ > > + struct analogix_dp_device *dp = to_dp(connector); > > + > > + state->self_refresh_aware = true; > > + if (state->self_refresh_active && !dp->psr_supported) > > + return -EINVAL; > > + > > + return 0; > > +} > > + > > static const struct drm_connector_helper_funcs analogix_dp_connector_helper_funcs = { > > .get_modes = analogix_dp_get_modes, > > .best_encoder = analogix_dp_best_encoder, > > + .atomic_check = analogix_dp_atomic_check, > > }; > > > > static enum drm_connector_status > > @@ -1244,6 +1270,10 @@ static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge) > > struct analogix_dp_device *dp = bridge->driver_private; > > int ret; > > > > + /* Don't touch the panel if we're coming back from PSR */ > > + if (dp->connector.state->self_refresh_changed) > > + return; > > + > > ret = analogix_dp_prepare_panel(dp, true, true); > > if (ret) > > DRM_ERROR("failed to setup the panel ret = %d\n", ret); > > @@ -1309,6 +1339,14 @@ static void analogix_dp_bridge_enable(struct drm_bridge *bridge) > > struct analogix_dp_device *dp = bridge->driver_private; > > int timeout_loop = 0; > > > > + /* Not a full enable, just disable PSR and continue */ > > + if (dp->connector.state->self_refresh_changed) { > > + int ret = analogix_dp_disable_psr(dp); > > + if (ret) > > + DRM_ERROR("Failed to disable psr %d\n", ret); > > + return; > > + } > > + > > if (dp->dpms_mode == DRM_MODE_DPMS_ON) > > return; > > > > @@ -1356,11 +1394,37 @@ 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->fast_train_enable = false; > > + dp->psr_supported = false; > > dp->dpms_mode = DRM_MODE_DPMS_OFF; > > } > > > > +static void analogix_dp_bridge_atomic_disable(struct drm_bridge *bridge) > > +{ > > + struct analogix_dp_device *dp = bridge->driver_private; > > + struct drm_connector_state *conn_state = dp->connector.state; > > + > > + /* Don't do a full disable on PSR transitions */ > > + if (conn_state->crtc && conn_state->self_refresh_changed) > > + return; > > + > > + analogix_dp_bridge_disable(bridge); > > +} > > + > > +static void analogix_dp_bridge_post_disable(struct drm_bridge *bridge) > > +{ > > + struct analogix_dp_device *dp = bridge->driver_private; > > + struct drm_connector_state *conn_state = dp->connector.state; > > + int ret; > > + > > + if (conn_state->self_refresh_active) { > > + ret = analogix_dp_enable_psr(dp); > > + if (ret) > > + DRM_ERROR("Failed to enable psr (%d)\n", ret); > > + return; > > + } > > +} > > + > > static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge, > > const struct drm_display_mode *orig_mode, > > const struct drm_display_mode *mode) > > @@ -1440,16 +1504,11 @@ static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge, > > video->interlaced = true; > > } > > > > -static void analogix_dp_bridge_nop(struct drm_bridge *bridge) > > -{ > > - /* do nothing */ > > -} > > - > > static const struct drm_bridge_funcs analogix_dp_bridge_funcs = { > > .pre_enable = analogix_dp_bridge_pre_enable, > > .enable = analogix_dp_bridge_enable, > > - .disable = analogix_dp_bridge_disable, > > - .post_disable = analogix_dp_bridge_nop, > > + .disable = analogix_dp_bridge_atomic_disable, > > + .post_disable = analogix_dp_bridge_post_disable, > > .mode_set = analogix_dp_bridge_mode_set, > > .attach = analogix_dp_bridge_attach, > > }; > > diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h > > index 769255dc6e99..d937fa3d9ee8 100644 > > --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h > > +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h > > @@ -173,8 +173,8 @@ struct analogix_dp_device { > > int dpms_mode; > > int hpd_gpio; > > bool force_hpd; > > - bool psr_enable; > > bool fast_train_enable; > > + bool psr_supported; > > > > struct mutex panel_lock; > > bool panel_is_modeset; > > diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile > > index f6fc9d5dd0ad..a777726c23cb 100644 > > --- a/drivers/gpu/drm/rockchip/Makefile > > +++ b/drivers/gpu/drm/rockchip/Makefile > > @@ -4,8 +4,7 @@ > > # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. > > > > rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ > > - rockchip_drm_gem.o rockchip_drm_psr.o \ > > - rockchip_drm_vop.o rockchip_vop_reg.o > > + rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o > > rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o > > > > rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o > > diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > > index bc4423624209..8f42c38848db 100644 > > --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > > +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c > > @@ -32,7 +32,6 @@ > > #include <drm/bridge/analogix_dp.h> > > > > #include "rockchip_drm_drv.h" > > -#include "rockchip_drm_psr.h" > > #include "rockchip_drm_vop.h" > > > > #define RK3288_GRF_SOC_CON6 0x25c > > @@ -77,29 +76,6 @@ struct rockchip_dp_device { > > struct analogix_dp_plat_data plat_data; > > }; > > > > -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 0; > > - > > - DRM_DEV_DEBUG(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit"); > > - > > - ret = rockchip_drm_wait_vact_end(dp->encoder.crtc, > > - PSR_WAIT_LINE_FLAG_TIMEOUT_MS); > > - if (ret) { > > - DRM_DEV_ERROR(dp->dev, "line flag interrupt did not arrive\n"); > > - return -ETIMEDOUT; > > - } > > - > > - if (enabled) > > - return analogix_dp_enable_psr(dp->adp); > > - else > > - return analogix_dp_disable_psr(dp->adp); > > -} > > - > > static int rockchip_dp_pre_init(struct rockchip_dp_device *dp) > > { > > reset_control_assert(dp->rst); > > @@ -130,21 +106,9 @@ static int rockchip_dp_poweron_start(struct analogix_dp_plat_data *plat_data) > > 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_inhibit_put(&dp->encoder); > > -} > > - > > 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_inhibit_get(&dp->encoder); > > - if (ret != 0) > > - return ret; > > > > clk_disable_unprepare(dp->pclk); > > > > @@ -190,6 +154,9 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder) > > int ret; > > u32 val; > > > > + if (analogix_dp_self_refresh_changed(dp->adp)) > > + return; > > + > > ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder); > > if (ret < 0) > > return; > > @@ -214,9 +181,24 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder) > > clk_disable_unprepare(dp->grfclk); > > } > > > > -static void rockchip_dp_drm_encoder_nop(struct drm_encoder *encoder) > > +static void rockchip_dp_drm_encoder_disable(struct drm_encoder *encoder) > > { > > - /* do nothing */ > > + struct rockchip_dp_device *dp = to_dp(encoder); > > + struct drm_crtc *crtc; > > + int ret; > > + > > + drm_for_each_crtc(crtc, dp->drm_dev) { > > + if (!(crtc->state->encoder_mask & drm_encoder_mask(encoder))) > > + continue; > > + > > + if (!crtc->state->self_refresh_active) > > + continue; > > + > > + ret = rockchip_drm_wait_vact_end(crtc, > > + PSR_WAIT_LINE_FLAG_TIMEOUT_MS); > > + if (ret) > > + DRM_DEV_ERROR(dp->dev, "line flag irq timed out\n"); > > + } > > } > > > > static int > > @@ -246,7 +228,7 @@ static struct drm_encoder_helper_funcs rockchip_dp_encoder_helper_funcs = { > > .mode_fixup = rockchip_dp_drm_encoder_mode_fixup, > > .mode_set = rockchip_dp_drm_encoder_mode_set, > > .enable = rockchip_dp_drm_encoder_enable, > > - .disable = rockchip_dp_drm_encoder_nop, > > + .disable = rockchip_dp_drm_encoder_disable, > > .atomic_check = rockchip_dp_drm_encoder_atomic_check, > > }; > > > > @@ -338,23 +320,16 @@ static int rockchip_dp_bind(struct device *dev, struct device *master, > > > > dp->plat_data.dev_type = dp->data->chip_type; > > 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; > > > > - ret = rockchip_drm_psr_register(&dp->encoder, analogix_dp_psr_set); > > - if (ret < 0) > > - goto err_cleanup_encoder; > > - > > dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data); > > if (IS_ERR(dp->adp)) { > > ret = PTR_ERR(dp->adp); > > - goto err_unreg_psr; > > + goto err_cleanup_encoder; > > } > > > > return 0; > > -err_unreg_psr: > > - rockchip_drm_psr_unregister(&dp->encoder); > > err_cleanup_encoder: > > dp->encoder.funcs->destroy(&dp->encoder); > > return ret; > > @@ -366,7 +341,6 @@ static void rockchip_dp_unbind(struct device *dev, struct device *master, > > struct rockchip_dp_device *dp = dev_get_drvdata(dev); > > > > analogix_dp_unbind(dp->adp); > > - rockchip_drm_psr_unregister(&dp->encoder); > > dp->encoder.funcs->destroy(&dp->encoder); > > > > dp->adp = ERR_PTR(-ENODEV); > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > > index 97438bbbe389..7e121875e3c9 100644 > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c > > @@ -23,22 +23,10 @@ > > #include "rockchip_drm_drv.h" > > #include "rockchip_drm_fb.h" > > #include "rockchip_drm_gem.h" > > -#include "rockchip_drm_psr.h" > > - > > -static int rockchip_drm_fb_dirty(struct drm_framebuffer *fb, > > - struct drm_file *file, > > - unsigned int flags, unsigned int color, > > - struct drm_clip_rect *clips, > > - unsigned int num_clips) > > -{ > > - rockchip_drm_psr_flush_all(fb->dev); > > - return 0; > > -} > > > > static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = { > > .destroy = drm_gem_fb_destroy, > > .create_handle = drm_gem_fb_create_handle, > > - .dirty = rockchip_drm_fb_dirty, > > }; > > > > static struct drm_framebuffer * > > @@ -132,8 +120,6 @@ 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); > > @@ -141,8 +127,6 @@ rockchip_atomic_helper_commit_tail_rpm(struct drm_atomic_state *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); > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c > > deleted file mode 100644 > > index a0c8bd235b67..000000000000 > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c > > +++ /dev/null > > @@ -1,290 +0,0 @@ > > -/* > > - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd > > - * Author: Yakir Yang <ykk@rock-chips.com> > > - * > > - * This software is licensed under the terms of the GNU General Public > > - * License version 2, as published by the Free Software Foundation, and > > - * may be copied, distributed, and modified under those terms. > > - * > > - * This program is distributed in the hope that it will be useful, > > - * but WITHOUT ANY WARRANTY; without even the implied warranty of > > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > - * GNU General Public License for more details. > > - */ > > - > > -#include <drm/drmP.h> > > -#include <drm/drm_atomic.h> > > -#include <drm/drm_probe_helper.h> > > - > > -#include "rockchip_drm_drv.h" > > -#include "rockchip_drm_psr.h" > > - > > -#define PSR_FLUSH_TIMEOUT_MS 100 > > - > > -struct psr_drv { > > - struct list_head list; > > - struct drm_encoder *encoder; > > - > > - struct mutex lock; > > - int inhibit_count; > > - bool enabled; > > - > > - struct delayed_work flush_work; > > - > > - int (*set)(struct drm_encoder *encoder, bool enable); > > -}; > > - > > -static struct psr_drv *find_psr_by_encoder(struct drm_encoder *encoder) > > -{ > > - struct rockchip_drm_private *drm_drv = encoder->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 == encoder) > > - goto out; > > - } > > - psr = ERR_PTR(-ENODEV); > > - > > -out: > > - mutex_unlock(&drm_drv->psr_list_lock); > > - return psr; > > -} > > - > > -static int psr_set_state_locked(struct psr_drv *psr, bool enable) > > -{ > > - int ret; > > - > > - if (psr->inhibit_count > 0) > > - return -EINVAL; > > - > > - if (enable == psr->enabled) > > - return 0; > > - > > - ret = psr->set(psr->encoder, enable); > > - if (ret) > > - return ret; > > - > > - psr->enabled = enable; > > - return 0; > > -} > > - > > -static void psr_flush_handler(struct work_struct *work) > > -{ > > - struct psr_drv *psr = container_of(to_delayed_work(work), > > - struct psr_drv, flush_work); > > - > > - mutex_lock(&psr->lock); > > - psr_set_state_locked(psr, true); > > - mutex_unlock(&psr->lock); > > -} > > - > > -/** > > - * 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_inhibit_put(struct drm_encoder *encoder) > > -{ > > - struct psr_drv *psr = find_psr_by_encoder(encoder); > > - > > - if (IS_ERR(psr)) > > - return PTR_ERR(psr); > > - > > - mutex_lock(&psr->lock); > > - --psr->inhibit_count; > > - WARN_ON(psr->inhibit_count < 0); > > - 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_inhibit_put); > > - > > -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); > > -} > > -EXPORT_SYMBOL(rockchip_drm_psr_inhibit_get_state); > > - > > -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); > > -} > > -EXPORT_SYMBOL(rockchip_drm_psr_inhibit_put_state); > > - > > -/** > > - * 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_inhibit_get(struct drm_encoder *encoder) > > -{ > > - struct psr_drv *psr = find_psr_by_encoder(encoder); > > - > > - if (IS_ERR(psr)) > > - return PTR_ERR(psr); > > - > > - mutex_lock(&psr->lock); > > - psr_set_state_locked(psr, false); > > - ++psr->inhibit_count; > > - mutex_unlock(&psr->lock); > > - cancel_delayed_work_sync(&psr->flush_work); > > - > > - return 0; > > -} > > -EXPORT_SYMBOL(rockchip_drm_psr_inhibit_get); > > - > > -static void rockchip_drm_do_flush(struct psr_drv *psr) > > -{ > > - cancel_delayed_work_sync(&psr->flush_work); > > - > > - 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); > > -} > > - > > -/** > > - * rockchip_drm_psr_flush_all - force to flush all registered PSR encoders > > - * @dev: drm device > > - * > > - * Disable the PSR function for all registered encoders, and then enable the > > - * PSR function back after PSR_FLUSH_TIMEOUT. If encoder PSR state have been > > - * changed during flush time, then keep the state no change after flush > > - * timeout. > > - * > > - * Returns: > > - * Zero on success, negative errno on failure. > > - */ > > -void rockchip_drm_psr_flush_all(struct drm_device *dev) > > -{ > > - struct rockchip_drm_private *drm_drv = dev->dev_private; > > - struct psr_drv *psr; > > - > > - mutex_lock(&drm_drv->psr_list_lock); > > - list_for_each_entry(psr, &drm_drv->psr_list, list) > > - rockchip_drm_do_flush(psr); > > - mutex_unlock(&drm_drv->psr_list_lock); > > -} > > -EXPORT_SYMBOL(rockchip_drm_psr_flush_all); > > - > > -/** > > - * rockchip_drm_psr_register - register encoder to psr driver > > - * @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. > > - */ > > -int rockchip_drm_psr_register(struct drm_encoder *encoder, > > - int (*psr_set)(struct drm_encoder *, bool enable)) > > -{ > > - struct rockchip_drm_private *drm_drv; > > - struct psr_drv *psr; > > - > > - if (!encoder || !psr_set) > > - return -EINVAL; > > - > > - drm_drv = encoder->dev->dev_private; > > - > > - psr = kzalloc(sizeof(struct psr_drv), GFP_KERNEL); > > - if (!psr) > > - return -ENOMEM; > > - > > - INIT_DELAYED_WORK(&psr->flush_work, psr_flush_handler); > > - mutex_init(&psr->lock); > > - > > - psr->inhibit_count = 1; > > - psr->enabled = false; > > - psr->encoder = encoder; > > - psr->set = psr_set; > > - > > - 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; > > -} > > -EXPORT_SYMBOL(rockchip_drm_psr_register); > > - > > -/** > > - * rockchip_drm_psr_unregister - unregister encoder to psr driver > > - * @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. > > - */ > > -void rockchip_drm_psr_unregister(struct drm_encoder *encoder) > > -{ > > - struct rockchip_drm_private *drm_drv = encoder->dev->dev_private; > > - struct psr_drv *psr, *n; > > - > > - mutex_lock(&drm_drv->psr_list_lock); > > - list_for_each_entry_safe(psr, n, &drm_drv->psr_list, list) { > > - if (psr->encoder == encoder) { > > - /* > > - * Any other value would mean that the encoder > > - * is still in use. > > - */ > > - WARN_ON(psr->inhibit_count != 1); > > - > > - list_del(&psr->list); > > - kfree(psr); > > - } > > - } > > - mutex_unlock(&drm_drv->psr_list_lock); > > -} > > -EXPORT_SYMBOL(rockchip_drm_psr_unregister); > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h > > deleted file mode 100644 > > index 25350ba3237b..000000000000 > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h > > +++ /dev/null > > @@ -1,30 +0,0 @@ > > -/* > > - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd > > - * Author: Yakir Yang <ykk@rock-chips.com> > > - * > > - * This software is licensed under the terms of the GNU General Public > > - * License version 2, as published by the Free Software Foundation, and > > - * may be copied, distributed, and modified under those terms. > > - * > > - * This program is distributed in the hope that it will be useful, > > - * but WITHOUT ANY WARRANTY; without even the implied warranty of > > - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > - * GNU General Public License for more details. > > - */ > > - > > -#ifndef __ROCKCHIP_DRM_PSR___ > > -#define __ROCKCHIP_DRM_PSR___ > > - > > -void rockchip_drm_psr_flush_all(struct drm_device *dev); > > - > > -int rockchip_drm_psr_inhibit_put(struct drm_encoder *encoder); > > -int rockchip_drm_psr_inhibit_get(struct drm_encoder *encoder); > > - > > -void rockchip_drm_psr_inhibit_get_state(struct drm_atomic_state *state); > > -void rockchip_drm_psr_inhibit_put_state(struct drm_atomic_state *state); > > - > > -int rockchip_drm_psr_register(struct drm_encoder *encoder, > > - int (*psr_set)(struct drm_encoder *, bool enable)); > > -void rockchip_drm_psr_unregister(struct drm_encoder *encoder); > > - > > -#endif /* __ROCKCHIP_DRM_PSR__ */ > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > index c7d4c6073ea5..21fccda70356 100644 > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > @@ -21,6 +21,7 @@ > > #include <drm/drm_gem_framebuffer_helper.h> > > #include <drm/drm_plane_helper.h> > > #include <drm/drm_probe_helper.h> > > +#include <drm/drm_self_refresh_helper.h> > > #ifdef CONFIG_DRM_ANALOGIX_DP > > #include <drm/bridge/analogix_dp.h> > > #endif > > @@ -42,10 +43,11 @@ > > #include "rockchip_drm_drv.h" > > #include "rockchip_drm_gem.h" > > #include "rockchip_drm_fb.h" > > -#include "rockchip_drm_psr.h" > > #include "rockchip_drm_vop.h" > > #include "rockchip_rgb.h" > > > > +#define VOP_SELF_REFRESH_ENTRY_DELAY_MS 100 > > + > > #define VOP_WIN_SET(vop, win, name, v) \ > > vop_reg_set(vop, &win->phy->name, win->base, ~0, v, #name) > > #define VOP_SCL_SET(vop, win, name, v) \ > > @@ -541,7 +543,7 @@ static void vop_core_clks_disable(struct vop *vop) > > clk_disable(vop->hclk); > > } > > > > -static int vop_enable(struct drm_crtc *crtc) > > +static int vop_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) > > { > > struct vop *vop = to_vop(crtc); > > int ret, i; > > @@ -581,12 +583,18 @@ static int vop_enable(struct drm_crtc *crtc) > > * We need to make sure that all windows are disabled before we > > * enable the crtc. Otherwise we might try to scan from a destroyed > > * buffer later. > > + * > > + * In the case of enable-after-PSR, we don't need to worry about this > > + * case since the buffer is guaranteed to be valid and disabling the > > + * window will result in screen glitches on PSR exit. > > */ > > - for (i = 0; i < vop->data->win_size; i++) { > > - struct vop_win *vop_win = &vop->win[i]; > > - const struct vop_win_data *win = vop_win->data; > > + if (!old_state || !old_state->self_refresh_active) { > > + for (i = 0; i < vop->data->win_size; i++) { > > + struct vop_win *vop_win = &vop->win[i]; > > + const struct vop_win_data *win = vop_win->data; > > > > - VOP_WIN_SET(vop, win, enable, 0); > > + VOP_WIN_SET(vop, win, enable, 0); > > + } > > } > > spin_unlock(&vop->reg_lock); > > > > @@ -937,12 +945,10 @@ static void vop_plane_atomic_async_update(struct drm_plane *plane, > > } > > > > if (vop->is_enabled) { > > - rockchip_drm_psr_inhibit_get_state(new_state->state); > > vop_plane_atomic_update(plane, plane->state); > > spin_lock(&vop->reg_lock); > > vop_cfg_done(vop); > > spin_unlock(&vop->reg_lock); > > - rockchip_drm_psr_inhibit_put_state(new_state->state); > > } > > > > plane->funcs->atomic_destroy_state(plane, plane_state); > > @@ -1035,7 +1041,7 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc, > > > > WARN_ON(vop->event); > > > > - ret = vop_enable(crtc); > > + ret = vop_enable(crtc, old_state); > > if (ret) { > > mutex_unlock(&vop->vop_lock); > > DRM_DEV_ERROR(vop->dev, "Failed to enable vop (%d)\n", ret); > > @@ -1509,6 +1515,13 @@ static int vop_create_crtc(struct vop *vop) > > init_completion(&vop->line_flag_completion); > > crtc->port = port; > > > > + ret = drm_self_refresh_helper_register(crtc, > > + VOP_SELF_REFRESH_ENTRY_DELAY_MS); > > + if (ret) > > + DRM_DEV_DEBUG_KMS(vop->dev, > > + "Failed to register %s with SR helpers %d, ignoring\n", > > + crtc->name, ret); > > + > > return 0; > > > > err_cleanup_crtc: > > @@ -1526,6 +1539,8 @@ static void vop_destroy_crtc(struct vop *vop) > > struct drm_device *drm_dev = vop->drm_dev; > > struct drm_plane *plane, *tmp; > > > > + drm_self_refresh_helper_unregister(crtc); > > + > > of_node_put(crtc->port); > > > > /* > > diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h > > index 475b706b49de..b6cc654143d7 100644 > > --- a/include/drm/bridge/analogix_dp.h > > +++ b/include/drm/bridge/analogix_dp.h > > @@ -42,9 +42,7 @@ struct analogix_dp_plat_data { > > struct drm_connector *); > > }; > > > > -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); > > +bool analogix_dp_self_refresh_changed(struct analogix_dp_device *dp); > > > > int analogix_dp_resume(struct analogix_dp_device *dp); > > int analogix_dp_suspend(struct analogix_dp_device *dp); > > > > > >
Am Freitag, 29. März 2019, 20:00:10 CET schrieb Sean Paul: > On Fri, Mar 29, 2019 at 07:51:51PM +0100, Heiko Stübner wrote: > > Hi, > > > > Am Dienstag, 26. März 2019, 21:44:56 CET schrieb Sean Paul: > > > From: Sean Paul <seanpaul@chromium.org> > > > > > > Instead of rolling our own implementation for tracking when PSR should > > > be [in]active, use the new self refresh helpers to do the heavy lifting. > > > > I only got patches 3-5 and had to pull 1+2 from patchwork, the following > > applies to the whole series though. > > > > While my Kevin display does still generally work with these 5 patches > > applied functionality has regressed somehow. > > > > Environment is a standard Debian with framebuffer console and sddm > > login manager. > > Ahh, this is probably b/c fb_dirty is not triggering PSR exit. I had it > working in v1, but removed it in v2 since Daniel suggested we require the > fb_dirty helpers. I neglected to add support for the fb_dirty helpers, so fbcon > updates won't cause the screen to refresh. I'll add the helpers in v3 and make > sure I test it out with fbcon. > > Thanks for reporting this! The cursor-not-blinking case above was X11+modesetting+sddm though, so not directly fbcon-related probably ... 2nd issue? Heiko
On Fri, Mar 29, 2019 at 08:02:17PM +0100, Heiko Stübner wrote: > Am Freitag, 29. März 2019, 20:00:10 CET schrieb Sean Paul: > > On Fri, Mar 29, 2019 at 07:51:51PM +0100, Heiko Stübner wrote: > > > Hi, > > > > > > Am Dienstag, 26. März 2019, 21:44:56 CET schrieb Sean Paul: > > > > From: Sean Paul <seanpaul@chromium.org> > > > > > > > > Instead of rolling our own implementation for tracking when PSR should > > > > be [in]active, use the new self refresh helpers to do the heavy lifting. > > > > > > I only got patches 3-5 and had to pull 1+2 from patchwork, the following > > > applies to the whole series though. > > > > > > While my Kevin display does still generally work with these 5 patches > > > applied functionality has regressed somehow. > > > > > > Environment is a standard Debian with framebuffer console and sddm > > > login manager. > > > > Ahh, this is probably b/c fb_dirty is not triggering PSR exit. I had it > > working in v1, but removed it in v2 since Daniel suggested we require the > > fb_dirty helpers. I neglected to add support for the fb_dirty helpers, so fbcon > > updates won't cause the screen to refresh. I'll add the helpers in v3 and make > > sure I test it out with fbcon. > > > > Thanks for reporting this! > > The cursor-not-blinking case above was X11+modesetting+sddm though, so > not directly fbcon-related probably ... 2nd issue? Yep, definitely could be. If you have the chance, could you collect some DRM_DEBUG_KMS logs for the second issue to see what's going on? My guess is there's a legacy path (update_plane?) that's somehow not triggering psr exit. Sean > > Heiko > >
On Fri, Mar 29, 2019 at 8:12 PM Sean Paul <sean@poorly.run> wrote: > > On Fri, Mar 29, 2019 at 08:02:17PM +0100, Heiko Stübner wrote: > > Am Freitag, 29. März 2019, 20:00:10 CET schrieb Sean Paul: > > > On Fri, Mar 29, 2019 at 07:51:51PM +0100, Heiko Stübner wrote: > > > > Hi, > > > > > > > > Am Dienstag, 26. März 2019, 21:44:56 CET schrieb Sean Paul: > > > > > From: Sean Paul <seanpaul@chromium.org> > > > > > > > > > > Instead of rolling our own implementation for tracking when PSR should > > > > > be [in]active, use the new self refresh helpers to do the heavy lifting. > > > > > > > > I only got patches 3-5 and had to pull 1+2 from patchwork, the following > > > > applies to the whole series though. > > > > > > > > While my Kevin display does still generally work with these 5 patches > > > > applied functionality has regressed somehow. > > > > > > > > Environment is a standard Debian with framebuffer console and sddm > > > > login manager. > > > > > > Ahh, this is probably b/c fb_dirty is not triggering PSR exit. I had it > > > working in v1, but removed it in v2 since Daniel suggested we require the > > > fb_dirty helpers. I neglected to add support for the fb_dirty helpers, so fbcon > > > updates won't cause the screen to refresh. I'll add the helpers in v3 and make > > > sure I test it out with fbcon. > > > > > > Thanks for reporting this! > > > > The cursor-not-blinking case above was X11+modesetting+sddm though, so > > not directly fbcon-related probably ... 2nd issue? > > Yep, definitely could be. If you have the chance, could you collect some > DRM_DEBUG_KMS logs for the second issue to see what's going on? My guess is > there's a legacy path (update_plane?) that's somehow not triggering psr exit. fb_dirty helper is also for userspace kms. It's just that you get a nice combo with the helpers that also takes care of fbcon (which is a separate path if you don't bother with the dirty helpers but roll your own for everything). And yeah the v2 design is a package, if you handroll your own fb_dirty the magic won't work :-) -Daniel
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index af34554a5a02..c503fe3bc3b3 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -106,63 +106,13 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp) return 0; } -int analogix_dp_psr_enabled(struct analogix_dp_device *dp) +bool analogix_dp_self_refresh_changed(struct analogix_dp_device *dp) { - - return dp->psr_enable; + return dp->connector.state->self_refresh_changed; } -EXPORT_SYMBOL_GPL(analogix_dp_psr_enabled); +EXPORT_SYMBOL_GPL(analogix_dp_self_refresh_changed); -int analogix_dp_enable_psr(struct analogix_dp_device *dp) -{ - struct edp_vsc_psr psr_vsc; - - if (!dp->psr_enable) - return 0; - - /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ - memset(&psr_vsc, 0, sizeof(psr_vsc)); - psr_vsc.sdp_header.HB0 = 0; - psr_vsc.sdp_header.HB1 = 0x7; - psr_vsc.sdp_header.HB2 = 0x2; - psr_vsc.sdp_header.HB3 = 0x8; - - 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, true); -} -EXPORT_SYMBOL_GPL(analogix_dp_enable_psr); - -int analogix_dp_disable_psr(struct analogix_dp_device *dp) -{ - struct edp_vsc_psr psr_vsc; - int ret; - - if (!dp->psr_enable) - return 0; - - /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ - memset(&psr_vsc, 0, sizeof(psr_vsc)); - psr_vsc.sdp_header.HB0 = 0; - psr_vsc.sdp_header.HB1 = 0x7; - psr_vsc.sdp_header.HB2 = 0x2; - psr_vsc.sdp_header.HB3 = 0x8; - - psr_vsc.DB0 = 0; - psr_vsc.DB1 = 0; - - ret = drm_dp_dpcd_writeb(&dp->aux, DP_SET_POWER, DP_SET_POWER_D0); - 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); -} -EXPORT_SYMBOL_GPL(analogix_dp_disable_psr); - -static int analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) +static bool analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) { unsigned char psr_version; int ret; @@ -170,14 +120,11 @@ static int analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) 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; + return false; } dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version); - - dp->psr_enable = (psr_version & DP_PSR_IS_SUPPORTED) ? true : false; - - return 0; + return psr_version & DP_PSR_IS_SUPPORTED; } static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) @@ -200,7 +147,7 @@ static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) } /* Main-Link transmitter remains active during PSR active states */ - psr_en = DP_PSR_MAIN_LINK_ACTIVE | DP_PSR_CRC_VERIFICATION; + psr_en = DP_PSR_CRC_VERIFICATION; 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"); @@ -208,8 +155,7 @@ static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) } /* Enable psr function */ - psr_en = DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE | - DP_PSR_CRC_VERIFICATION; + psr_en = DP_PSR_ENABLE | DP_PSR_CRC_VERIFICATION; 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"); @@ -218,10 +164,11 @@ static int analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) analogix_dp_enable_psr_crc(dp); + dp->psr_supported = true; + return 0; end: dev_err(dp->dev, "enable psr fail, force to disable psr\n"); - dp->psr_enable = false; return ret; } @@ -1036,25 +983,90 @@ static int analogix_dp_commit(struct analogix_dp_device *dp) } } - ret = analogix_dp_detect_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; - if (dp->psr_enable) { + if (analogix_dp_detect_sink_psr(dp)) { ret = analogix_dp_enable_sink_psr(dp); if (ret) return ret; } + return ret; +} + +static int analogix_dp_enable_psr(struct analogix_dp_device *dp) +{ + struct edp_vsc_psr psr_vsc; + int ret; + u8 sink; + + ret = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink); + if (ret != 1) + DRM_DEV_ERROR(dp->dev, "Failed to read psr status %d\n", ret); + else if (sink == DP_PSR_SINK_ACTIVE_RFB) + return 0; + + /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ + memset(&psr_vsc, 0, sizeof(psr_vsc)); + psr_vsc.sdp_header.HB0 = 0; + psr_vsc.sdp_header.HB1 = 0x7; + psr_vsc.sdp_header.HB2 = 0x2; + psr_vsc.sdp_header.HB3 = 0x8; + psr_vsc.DB0 = 0; + psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID; + + ret = analogix_dp_send_psr_spd(dp, &psr_vsc, true); + if (!ret) + analogix_dp_set_analog_power_down(dp, POWER_ALL, true); return ret; } +static int analogix_dp_disable_psr(struct analogix_dp_device *dp) +{ + struct edp_vsc_psr psr_vsc; + int ret; + u8 sink; + + analogix_dp_set_analog_power_down(dp, POWER_ALL, false); + + ret = drm_dp_dpcd_writeb(&dp->aux, DP_SET_POWER, DP_SET_POWER_D0); + if (ret != 1) { + DRM_DEV_ERROR(dp->dev, "Failed to set DP Power0 %d\n", ret); + return ret; + } + + ret = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink); + if (ret != 1) { + DRM_DEV_ERROR(dp->dev, "Failed to read psr status %d\n", ret); + return ret; + } else if (sink == DP_PSR_SINK_INACTIVE) { + DRM_DEV_ERROR(dp->dev, "sink inactive, skip disable psr"); + return 0; + } + + ret = analogix_dp_train_link(dp); + if (ret) { + DRM_DEV_ERROR(dp->dev, "Failed to train the link %d\n", ret); + return ret; + } + + /* Prepare VSC packet as per EDP 1.4 spec, Table 6.9 */ + memset(&psr_vsc, 0, sizeof(psr_vsc)); + psr_vsc.sdp_header.HB0 = 0; + psr_vsc.sdp_header.HB1 = 0x7; + psr_vsc.sdp_header.HB2 = 0x2; + psr_vsc.sdp_header.HB3 = 0x8; + + psr_vsc.DB0 = 0; + psr_vsc.DB1 = 0; + + return analogix_dp_send_psr_spd(dp, &psr_vsc, true); +} + /* * This function is a bit of a catch-all for panel preparation, hopefully * simplifying the logic of functions that need to prepare/unprepare the panel @@ -1145,9 +1157,23 @@ analogix_dp_best_encoder(struct drm_connector *connector) return dp->encoder; } + +int analogix_dp_atomic_check(struct drm_connector *connector, + struct drm_connector_state *state) +{ + struct analogix_dp_device *dp = to_dp(connector); + + state->self_refresh_aware = true; + if (state->self_refresh_active && !dp->psr_supported) + return -EINVAL; + + return 0; +} + static const struct drm_connector_helper_funcs analogix_dp_connector_helper_funcs = { .get_modes = analogix_dp_get_modes, .best_encoder = analogix_dp_best_encoder, + .atomic_check = analogix_dp_atomic_check, }; static enum drm_connector_status @@ -1244,6 +1270,10 @@ static void analogix_dp_bridge_pre_enable(struct drm_bridge *bridge) struct analogix_dp_device *dp = bridge->driver_private; int ret; + /* Don't touch the panel if we're coming back from PSR */ + if (dp->connector.state->self_refresh_changed) + return; + ret = analogix_dp_prepare_panel(dp, true, true); if (ret) DRM_ERROR("failed to setup the panel ret = %d\n", ret); @@ -1309,6 +1339,14 @@ static void analogix_dp_bridge_enable(struct drm_bridge *bridge) struct analogix_dp_device *dp = bridge->driver_private; int timeout_loop = 0; + /* Not a full enable, just disable PSR and continue */ + if (dp->connector.state->self_refresh_changed) { + int ret = analogix_dp_disable_psr(dp); + if (ret) + DRM_ERROR("Failed to disable psr %d\n", ret); + return; + } + if (dp->dpms_mode == DRM_MODE_DPMS_ON) return; @@ -1356,11 +1394,37 @@ 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->fast_train_enable = false; + dp->psr_supported = false; dp->dpms_mode = DRM_MODE_DPMS_OFF; } +static void analogix_dp_bridge_atomic_disable(struct drm_bridge *bridge) +{ + struct analogix_dp_device *dp = bridge->driver_private; + struct drm_connector_state *conn_state = dp->connector.state; + + /* Don't do a full disable on PSR transitions */ + if (conn_state->crtc && conn_state->self_refresh_changed) + return; + + analogix_dp_bridge_disable(bridge); +} + +static void analogix_dp_bridge_post_disable(struct drm_bridge *bridge) +{ + struct analogix_dp_device *dp = bridge->driver_private; + struct drm_connector_state *conn_state = dp->connector.state; + int ret; + + if (conn_state->self_refresh_active) { + ret = analogix_dp_enable_psr(dp); + if (ret) + DRM_ERROR("Failed to enable psr (%d)\n", ret); + return; + } +} + static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge, const struct drm_display_mode *orig_mode, const struct drm_display_mode *mode) @@ -1440,16 +1504,11 @@ static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge, video->interlaced = true; } -static void analogix_dp_bridge_nop(struct drm_bridge *bridge) -{ - /* do nothing */ -} - static const struct drm_bridge_funcs analogix_dp_bridge_funcs = { .pre_enable = analogix_dp_bridge_pre_enable, .enable = analogix_dp_bridge_enable, - .disable = analogix_dp_bridge_disable, - .post_disable = analogix_dp_bridge_nop, + .disable = analogix_dp_bridge_atomic_disable, + .post_disable = analogix_dp_bridge_post_disable, .mode_set = analogix_dp_bridge_mode_set, .attach = analogix_dp_bridge_attach, }; diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index 769255dc6e99..d937fa3d9ee8 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -173,8 +173,8 @@ struct analogix_dp_device { int dpms_mode; int hpd_gpio; bool force_hpd; - bool psr_enable; bool fast_train_enable; + bool psr_supported; struct mutex panel_lock; bool panel_is_modeset; diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile index f6fc9d5dd0ad..a777726c23cb 100644 --- a/drivers/gpu/drm/rockchip/Makefile +++ b/drivers/gpu/drm/rockchip/Makefile @@ -4,8 +4,7 @@ # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher. rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ - rockchip_drm_gem.o rockchip_drm_psr.o \ - rockchip_drm_vop.o rockchip_vop_reg.o + rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index bc4423624209..8f42c38848db 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -32,7 +32,6 @@ #include <drm/bridge/analogix_dp.h> #include "rockchip_drm_drv.h" -#include "rockchip_drm_psr.h" #include "rockchip_drm_vop.h" #define RK3288_GRF_SOC_CON6 0x25c @@ -77,29 +76,6 @@ struct rockchip_dp_device { struct analogix_dp_plat_data plat_data; }; -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 0; - - DRM_DEV_DEBUG(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit"); - - ret = rockchip_drm_wait_vact_end(dp->encoder.crtc, - PSR_WAIT_LINE_FLAG_TIMEOUT_MS); - if (ret) { - DRM_DEV_ERROR(dp->dev, "line flag interrupt did not arrive\n"); - return -ETIMEDOUT; - } - - if (enabled) - return analogix_dp_enable_psr(dp->adp); - else - return analogix_dp_disable_psr(dp->adp); -} - static int rockchip_dp_pre_init(struct rockchip_dp_device *dp) { reset_control_assert(dp->rst); @@ -130,21 +106,9 @@ static int rockchip_dp_poweron_start(struct analogix_dp_plat_data *plat_data) 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_inhibit_put(&dp->encoder); -} - 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_inhibit_get(&dp->encoder); - if (ret != 0) - return ret; clk_disable_unprepare(dp->pclk); @@ -190,6 +154,9 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder) int ret; u32 val; + if (analogix_dp_self_refresh_changed(dp->adp)) + return; + ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder); if (ret < 0) return; @@ -214,9 +181,24 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder) clk_disable_unprepare(dp->grfclk); } -static void rockchip_dp_drm_encoder_nop(struct drm_encoder *encoder) +static void rockchip_dp_drm_encoder_disable(struct drm_encoder *encoder) { - /* do nothing */ + struct rockchip_dp_device *dp = to_dp(encoder); + struct drm_crtc *crtc; + int ret; + + drm_for_each_crtc(crtc, dp->drm_dev) { + if (!(crtc->state->encoder_mask & drm_encoder_mask(encoder))) + continue; + + if (!crtc->state->self_refresh_active) + continue; + + ret = rockchip_drm_wait_vact_end(crtc, + PSR_WAIT_LINE_FLAG_TIMEOUT_MS); + if (ret) + DRM_DEV_ERROR(dp->dev, "line flag irq timed out\n"); + } } static int @@ -246,7 +228,7 @@ static struct drm_encoder_helper_funcs rockchip_dp_encoder_helper_funcs = { .mode_fixup = rockchip_dp_drm_encoder_mode_fixup, .mode_set = rockchip_dp_drm_encoder_mode_set, .enable = rockchip_dp_drm_encoder_enable, - .disable = rockchip_dp_drm_encoder_nop, + .disable = rockchip_dp_drm_encoder_disable, .atomic_check = rockchip_dp_drm_encoder_atomic_check, }; @@ -338,23 +320,16 @@ static int rockchip_dp_bind(struct device *dev, struct device *master, dp->plat_data.dev_type = dp->data->chip_type; 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; - ret = rockchip_drm_psr_register(&dp->encoder, analogix_dp_psr_set); - if (ret < 0) - goto err_cleanup_encoder; - dp->adp = analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data); if (IS_ERR(dp->adp)) { ret = PTR_ERR(dp->adp); - goto err_unreg_psr; + goto err_cleanup_encoder; } return 0; -err_unreg_psr: - rockchip_drm_psr_unregister(&dp->encoder); err_cleanup_encoder: dp->encoder.funcs->destroy(&dp->encoder); return ret; @@ -366,7 +341,6 @@ static void rockchip_dp_unbind(struct device *dev, struct device *master, struct rockchip_dp_device *dp = dev_get_drvdata(dev); analogix_dp_unbind(dp->adp); - rockchip_drm_psr_unregister(&dp->encoder); dp->encoder.funcs->destroy(&dp->encoder); dp->adp = ERR_PTR(-ENODEV); diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c index 97438bbbe389..7e121875e3c9 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c @@ -23,22 +23,10 @@ #include "rockchip_drm_drv.h" #include "rockchip_drm_fb.h" #include "rockchip_drm_gem.h" -#include "rockchip_drm_psr.h" - -static int rockchip_drm_fb_dirty(struct drm_framebuffer *fb, - struct drm_file *file, - unsigned int flags, unsigned int color, - struct drm_clip_rect *clips, - unsigned int num_clips) -{ - rockchip_drm_psr_flush_all(fb->dev); - return 0; -} static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = { .destroy = drm_gem_fb_destroy, .create_handle = drm_gem_fb_create_handle, - .dirty = rockchip_drm_fb_dirty, }; static struct drm_framebuffer * @@ -132,8 +120,6 @@ 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); @@ -141,8 +127,6 @@ rockchip_atomic_helper_commit_tail_rpm(struct drm_atomic_state *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); diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c deleted file mode 100644 index a0c8bd235b67..000000000000 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c +++ /dev/null @@ -1,290 +0,0 @@ -/* - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd - * Author: Yakir Yang <ykk@rock-chips.com> - * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#include <drm/drmP.h> -#include <drm/drm_atomic.h> -#include <drm/drm_probe_helper.h> - -#include "rockchip_drm_drv.h" -#include "rockchip_drm_psr.h" - -#define PSR_FLUSH_TIMEOUT_MS 100 - -struct psr_drv { - struct list_head list; - struct drm_encoder *encoder; - - struct mutex lock; - int inhibit_count; - bool enabled; - - struct delayed_work flush_work; - - int (*set)(struct drm_encoder *encoder, bool enable); -}; - -static struct psr_drv *find_psr_by_encoder(struct drm_encoder *encoder) -{ - struct rockchip_drm_private *drm_drv = encoder->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 == encoder) - goto out; - } - psr = ERR_PTR(-ENODEV); - -out: - mutex_unlock(&drm_drv->psr_list_lock); - return psr; -} - -static int psr_set_state_locked(struct psr_drv *psr, bool enable) -{ - int ret; - - if (psr->inhibit_count > 0) - return -EINVAL; - - if (enable == psr->enabled) - return 0; - - ret = psr->set(psr->encoder, enable); - if (ret) - return ret; - - psr->enabled = enable; - return 0; -} - -static void psr_flush_handler(struct work_struct *work) -{ - struct psr_drv *psr = container_of(to_delayed_work(work), - struct psr_drv, flush_work); - - mutex_lock(&psr->lock); - psr_set_state_locked(psr, true); - mutex_unlock(&psr->lock); -} - -/** - * 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_inhibit_put(struct drm_encoder *encoder) -{ - struct psr_drv *psr = find_psr_by_encoder(encoder); - - if (IS_ERR(psr)) - return PTR_ERR(psr); - - mutex_lock(&psr->lock); - --psr->inhibit_count; - WARN_ON(psr->inhibit_count < 0); - 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_inhibit_put); - -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); -} -EXPORT_SYMBOL(rockchip_drm_psr_inhibit_get_state); - -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); -} -EXPORT_SYMBOL(rockchip_drm_psr_inhibit_put_state); - -/** - * 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_inhibit_get(struct drm_encoder *encoder) -{ - struct psr_drv *psr = find_psr_by_encoder(encoder); - - if (IS_ERR(psr)) - return PTR_ERR(psr); - - mutex_lock(&psr->lock); - psr_set_state_locked(psr, false); - ++psr->inhibit_count; - mutex_unlock(&psr->lock); - cancel_delayed_work_sync(&psr->flush_work); - - return 0; -} -EXPORT_SYMBOL(rockchip_drm_psr_inhibit_get); - -static void rockchip_drm_do_flush(struct psr_drv *psr) -{ - cancel_delayed_work_sync(&psr->flush_work); - - 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); -} - -/** - * rockchip_drm_psr_flush_all - force to flush all registered PSR encoders - * @dev: drm device - * - * Disable the PSR function for all registered encoders, and then enable the - * PSR function back after PSR_FLUSH_TIMEOUT. If encoder PSR state have been - * changed during flush time, then keep the state no change after flush - * timeout. - * - * Returns: - * Zero on success, negative errno on failure. - */ -void rockchip_drm_psr_flush_all(struct drm_device *dev) -{ - struct rockchip_drm_private *drm_drv = dev->dev_private; - struct psr_drv *psr; - - mutex_lock(&drm_drv->psr_list_lock); - list_for_each_entry(psr, &drm_drv->psr_list, list) - rockchip_drm_do_flush(psr); - mutex_unlock(&drm_drv->psr_list_lock); -} -EXPORT_SYMBOL(rockchip_drm_psr_flush_all); - -/** - * rockchip_drm_psr_register - register encoder to psr driver - * @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. - */ -int rockchip_drm_psr_register(struct drm_encoder *encoder, - int (*psr_set)(struct drm_encoder *, bool enable)) -{ - struct rockchip_drm_private *drm_drv; - struct psr_drv *psr; - - if (!encoder || !psr_set) - return -EINVAL; - - drm_drv = encoder->dev->dev_private; - - psr = kzalloc(sizeof(struct psr_drv), GFP_KERNEL); - if (!psr) - return -ENOMEM; - - INIT_DELAYED_WORK(&psr->flush_work, psr_flush_handler); - mutex_init(&psr->lock); - - psr->inhibit_count = 1; - psr->enabled = false; - psr->encoder = encoder; - psr->set = psr_set; - - 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; -} -EXPORT_SYMBOL(rockchip_drm_psr_register); - -/** - * rockchip_drm_psr_unregister - unregister encoder to psr driver - * @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. - */ -void rockchip_drm_psr_unregister(struct drm_encoder *encoder) -{ - struct rockchip_drm_private *drm_drv = encoder->dev->dev_private; - struct psr_drv *psr, *n; - - mutex_lock(&drm_drv->psr_list_lock); - list_for_each_entry_safe(psr, n, &drm_drv->psr_list, list) { - if (psr->encoder == encoder) { - /* - * Any other value would mean that the encoder - * is still in use. - */ - WARN_ON(psr->inhibit_count != 1); - - list_del(&psr->list); - kfree(psr); - } - } - mutex_unlock(&drm_drv->psr_list_lock); -} -EXPORT_SYMBOL(rockchip_drm_psr_unregister); diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h deleted file mode 100644 index 25350ba3237b..000000000000 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd - * Author: Yakir Yang <ykk@rock-chips.com> - * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#ifndef __ROCKCHIP_DRM_PSR___ -#define __ROCKCHIP_DRM_PSR___ - -void rockchip_drm_psr_flush_all(struct drm_device *dev); - -int rockchip_drm_psr_inhibit_put(struct drm_encoder *encoder); -int rockchip_drm_psr_inhibit_get(struct drm_encoder *encoder); - -void rockchip_drm_psr_inhibit_get_state(struct drm_atomic_state *state); -void rockchip_drm_psr_inhibit_put_state(struct drm_atomic_state *state); - -int rockchip_drm_psr_register(struct drm_encoder *encoder, - int (*psr_set)(struct drm_encoder *, bool enable)); -void rockchip_drm_psr_unregister(struct drm_encoder *encoder); - -#endif /* __ROCKCHIP_DRM_PSR__ */ diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index c7d4c6073ea5..21fccda70356 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -21,6 +21,7 @@ #include <drm/drm_gem_framebuffer_helper.h> #include <drm/drm_plane_helper.h> #include <drm/drm_probe_helper.h> +#include <drm/drm_self_refresh_helper.h> #ifdef CONFIG_DRM_ANALOGIX_DP #include <drm/bridge/analogix_dp.h> #endif @@ -42,10 +43,11 @@ #include "rockchip_drm_drv.h" #include "rockchip_drm_gem.h" #include "rockchip_drm_fb.h" -#include "rockchip_drm_psr.h" #include "rockchip_drm_vop.h" #include "rockchip_rgb.h" +#define VOP_SELF_REFRESH_ENTRY_DELAY_MS 100 + #define VOP_WIN_SET(vop, win, name, v) \ vop_reg_set(vop, &win->phy->name, win->base, ~0, v, #name) #define VOP_SCL_SET(vop, win, name, v) \ @@ -541,7 +543,7 @@ static void vop_core_clks_disable(struct vop *vop) clk_disable(vop->hclk); } -static int vop_enable(struct drm_crtc *crtc) +static int vop_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { struct vop *vop = to_vop(crtc); int ret, i; @@ -581,12 +583,18 @@ static int vop_enable(struct drm_crtc *crtc) * We need to make sure that all windows are disabled before we * enable the crtc. Otherwise we might try to scan from a destroyed * buffer later. + * + * In the case of enable-after-PSR, we don't need to worry about this + * case since the buffer is guaranteed to be valid and disabling the + * window will result in screen glitches on PSR exit. */ - for (i = 0; i < vop->data->win_size; i++) { - struct vop_win *vop_win = &vop->win[i]; - const struct vop_win_data *win = vop_win->data; + if (!old_state || !old_state->self_refresh_active) { + for (i = 0; i < vop->data->win_size; i++) { + struct vop_win *vop_win = &vop->win[i]; + const struct vop_win_data *win = vop_win->data; - VOP_WIN_SET(vop, win, enable, 0); + VOP_WIN_SET(vop, win, enable, 0); + } } spin_unlock(&vop->reg_lock); @@ -937,12 +945,10 @@ static void vop_plane_atomic_async_update(struct drm_plane *plane, } if (vop->is_enabled) { - rockchip_drm_psr_inhibit_get_state(new_state->state); vop_plane_atomic_update(plane, plane->state); spin_lock(&vop->reg_lock); vop_cfg_done(vop); spin_unlock(&vop->reg_lock); - rockchip_drm_psr_inhibit_put_state(new_state->state); } plane->funcs->atomic_destroy_state(plane, plane_state); @@ -1035,7 +1041,7 @@ static void vop_crtc_atomic_enable(struct drm_crtc *crtc, WARN_ON(vop->event); - ret = vop_enable(crtc); + ret = vop_enable(crtc, old_state); if (ret) { mutex_unlock(&vop->vop_lock); DRM_DEV_ERROR(vop->dev, "Failed to enable vop (%d)\n", ret); @@ -1509,6 +1515,13 @@ static int vop_create_crtc(struct vop *vop) init_completion(&vop->line_flag_completion); crtc->port = port; + ret = drm_self_refresh_helper_register(crtc, + VOP_SELF_REFRESH_ENTRY_DELAY_MS); + if (ret) + DRM_DEV_DEBUG_KMS(vop->dev, + "Failed to register %s with SR helpers %d, ignoring\n", + crtc->name, ret); + return 0; err_cleanup_crtc: @@ -1526,6 +1539,8 @@ static void vop_destroy_crtc(struct vop *vop) struct drm_device *drm_dev = vop->drm_dev; struct drm_plane *plane, *tmp; + drm_self_refresh_helper_unregister(crtc); + of_node_put(crtc->port); /* diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h index 475b706b49de..b6cc654143d7 100644 --- a/include/drm/bridge/analogix_dp.h +++ b/include/drm/bridge/analogix_dp.h @@ -42,9 +42,7 @@ struct analogix_dp_plat_data { struct drm_connector *); }; -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); +bool analogix_dp_self_refresh_changed(struct analogix_dp_device *dp); int analogix_dp_resume(struct analogix_dp_device *dp); int analogix_dp_suspend(struct analogix_dp_device *dp);