From b883c9ba18a4d527fddc952a35565e34a5d22e15 Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Thu, 18 Aug 2016 12:01:46 -0700 Subject: [PATCH 01/13] drm/rockchip: Don't key off vblank for psr Instead of keying off vblank for psr, just flush every time we get an atomic update. This ensures that cursor updates will properly disable psr (without turning vblank on/off), and unifies the paths between fb_dirty and atomic psr enable/disable. Reviewed-by: Yakir Yang Signed-off-by: Sean Paul --- drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +- drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 72 ++++++++++++++------- drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 8 ++- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 10 +-- 4 files changed, 62 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c index 60bcc48f84b9..9890eccadd3b 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c @@ -70,7 +70,7 @@ static int rockchip_drm_fb_dirty(struct drm_framebuffer *fb, struct drm_clip_rect *clips, unsigned int num_clips) { - rockchip_drm_psr_flush(fb->dev); + rockchip_drm_psr_flush_all(fb->dev); return 0; } diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c index c6ac5d01c8e4..de6252f6cb89 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c @@ -31,6 +31,7 @@ struct psr_drv { struct drm_encoder *encoder; spinlock_t lock; + bool active; enum psr_state state; struct timer_list flush_timer; @@ -67,11 +68,7 @@ static void psr_set_state_locked(struct psr_drv *psr, enum psr_state state) * v | | * PSR_DISABLE < - - - - - - - - - */ - if (state == psr->state) - return; - - /* Requesting a flush when disabled is a noop */ - if (state == PSR_FLUSH && psr->state == PSR_DISABLE) + if (state == psr->state || !psr->active) return; psr->state = state; @@ -115,45 +112,79 @@ static void psr_flush_handler(unsigned long data) } /** - * rockchip_drm_psr_enable - enable the encoder PSR which bind to given CRTC + * rockchip_drm_psr_activate - activate PSR on the given pipe * @crtc: CRTC to obtain the PSR encoder * * Returns: * Zero on success, negative errno on failure. */ -int rockchip_drm_psr_enable(struct drm_crtc *crtc) +int rockchip_drm_psr_activate(struct drm_crtc *crtc) { struct psr_drv *psr = find_psr_by_crtc(crtc); + unsigned long flags; if (IS_ERR(psr)) return PTR_ERR(psr); - psr_set_state(psr, PSR_ENABLE); + spin_lock_irqsave(&psr->lock, flags); + psr->active = true; + spin_unlock_irqrestore(&psr->lock, flags); + return 0; } -EXPORT_SYMBOL(rockchip_drm_psr_enable); +EXPORT_SYMBOL(rockchip_drm_psr_activate); /** - * rockchip_drm_psr_disable - disable the encoder PSR which bind to given CRTC + * rockchip_drm_psr_deactivate - deactivate PSR on the given pipe * @crtc: CRTC to obtain the PSR encoder * * Returns: * Zero on success, negative errno on failure. */ -int rockchip_drm_psr_disable(struct drm_crtc *crtc) +int rockchip_drm_psr_deactivate(struct drm_crtc *crtc) { struct psr_drv *psr = find_psr_by_crtc(crtc); + unsigned long flags; if (IS_ERR(psr)) return PTR_ERR(psr); - psr_set_state(psr, PSR_DISABLE); + spin_lock_irqsave(&psr->lock, flags); + psr->active = false; + spin_unlock_irqrestore(&psr->lock, flags); + del_timer_sync(&psr->flush_timer); + return 0; } -EXPORT_SYMBOL(rockchip_drm_psr_disable); +EXPORT_SYMBOL(rockchip_drm_psr_deactivate); + +static void rockchip_drm_do_flush(struct psr_drv *psr) +{ + mod_timer(&psr->flush_timer, + round_jiffies_up(jiffies + PSR_FLUSH_TIMEOUT)); + psr_set_state(psr, PSR_FLUSH); +} /** - * rockchip_drm_psr_flush - force to flush all registered PSR encoders + * rockchip_drm_psr_flush - flush a single pipe + * @crtc: CRTC of the pipe to flush + * + * Returns: + * 0 on success, -errno on fail + */ +int rockchip_drm_psr_flush(struct drm_crtc *crtc) +{ + struct psr_drv *psr = find_psr_by_crtc(crtc); + if (IS_ERR(psr)) + return PTR_ERR(psr); + + rockchip_drm_do_flush(psr); + return 0; +} +EXPORT_SYMBOL(rockchip_drm_psr_flush); + +/** + * rockchip_drm_psr_flush_all - force to flush all registered PSR encoders * @dev: drm device * * Disable the PSR function for all registered encoders, and then enable the @@ -164,22 +195,18 @@ EXPORT_SYMBOL(rockchip_drm_psr_disable); * Returns: * Zero on success, negative errno on failure. */ -void rockchip_drm_psr_flush(struct drm_device *dev) +void rockchip_drm_psr_flush_all(struct drm_device *dev) { struct rockchip_drm_private *drm_drv = dev->dev_private; struct psr_drv *psr; unsigned long flags; spin_lock_irqsave(&drm_drv->psr_list_lock, flags); - list_for_each_entry(psr, &drm_drv->psr_list, list) { - mod_timer(&psr->flush_timer, - round_jiffies_up(jiffies + PSR_FLUSH_TIMEOUT)); - - psr_set_state(psr, PSR_FLUSH); - } + list_for_each_entry(psr, &drm_drv->psr_list, list) + rockchip_drm_do_flush(psr); spin_unlock_irqrestore(&drm_drv->psr_list_lock, flags); } -EXPORT_SYMBOL(rockchip_drm_psr_flush); +EXPORT_SYMBOL(rockchip_drm_psr_flush_all); /** * rockchip_drm_psr_register - register encoder to psr driver @@ -206,6 +233,7 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder, setup_timer(&psr->flush_timer, psr_flush_handler, (unsigned long)psr); spin_lock_init(&psr->lock); + psr->active = true; psr->state = PSR_DISABLE; psr->encoder = encoder; psr->set = psr_set; diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h index c35b68873a30..b420cf1bf902 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h @@ -15,9 +15,11 @@ #ifndef __ROCKCHIP_DRM_PSR___ #define __ROCKCHIP_DRM_PSR___ -void rockchip_drm_psr_flush(struct drm_device *dev); -int rockchip_drm_psr_enable(struct drm_crtc *crtc); -int rockchip_drm_psr_disable(struct drm_crtc *crtc); +void rockchip_drm_psr_flush_all(struct drm_device *dev); +int rockchip_drm_psr_flush(struct drm_crtc *crtc); + +int rockchip_drm_psr_activate(struct drm_crtc *crtc); +int rockchip_drm_psr_deactivate(struct drm_crtc *crtc); int rockchip_drm_psr_register(struct drm_encoder *encoder, void (*psr_set)(struct drm_encoder *, bool enable)); diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index d486049f9722..efb216005baa 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -569,6 +569,8 @@ static void vop_crtc_disable(struct drm_crtc *crtc) WARN_ON(vop->event); + rockchip_drm_psr_deactivate(&vop->crtc); + /* * We need to make sure that all windows are disabled before we * disable that crtc. Otherwise we might try to scan from a destroyed @@ -919,8 +921,6 @@ static int vop_crtc_enable_vblank(struct drm_crtc *crtc) spin_unlock_irqrestore(&vop->irq_lock, flags); - rockchip_drm_psr_disable(&vop->crtc); - return 0; } @@ -937,8 +937,6 @@ static void vop_crtc_disable_vblank(struct drm_crtc *crtc) VOP_INTR_SET_TYPE(vop, enable, FS_INTR, 0); spin_unlock_irqrestore(&vop->irq_lock, flags); - - rockchip_drm_psr_enable(&vop->crtc); } static void vop_crtc_wait_for_update(struct drm_crtc *crtc) @@ -1072,6 +1070,8 @@ static void vop_crtc_enable(struct drm_crtc *crtc) clk_set_rate(vop->dclk, adjusted_mode->clock * 1000); VOP_CTRL_SET(vop, standby, 0); + + rockchip_drm_psr_activate(&vop->crtc); } static void vop_crtc_atomic_flush(struct drm_crtc *crtc, @@ -1094,6 +1094,8 @@ static void vop_crtc_atomic_begin(struct drm_crtc *crtc, { struct vop *vop = to_vop(crtc); + rockchip_drm_psr_flush(crtc); + spin_lock_irq(&crtc->dev->event_lock); vop->vblank_active = true; WARN_ON(drm_crtc_vblank_get(crtc) != 0); From 604bac48a7d97b305b126731c443e79a78253582 Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Thu, 18 Aug 2016 12:03:22 -0700 Subject: [PATCH 02/13] drm/rockchip: Reduce psr flush time to 100ms 3 seconds is a bit too conservative, drop this to 100ms for better power savings. Reviewed-by: Yakir Yang Signed-off-by: Sean Paul --- drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c index de6252f6cb89..2cdd6eb359f2 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c @@ -18,7 +18,7 @@ #include "rockchip_drm_drv.h" #include "rockchip_drm_psr.h" -#define PSR_FLUSH_TIMEOUT msecs_to_jiffies(3000) /* 3 seconds */ +#define PSR_FLUSH_TIMEOUT msecs_to_jiffies(100) enum psr_state { PSR_FLUSH, From d2f12adc004dd83ce700ecaebabd4f802a586370 Mon Sep 17 00:00:00 2001 From: Tomeu Vizoso Date: Fri, 5 Aug 2016 14:59:03 +0200 Subject: [PATCH 03/13] drm/bridge: analogix_dp: Remove duplicated code Remove code for reading the EDID and DPCD fields and use the helpers instead. Besides the obvious code reduction, other helpers are being added to the core that could be used in this driver and will be good to be able to use them instead of duplicating them. Signed-off-by: Tomeu Vizoso Tested-by: Javier Martinez Canillas Tested-by: Sean Paul Reviewed-by: Sean Paul Reviewed-by: Yakir Yang Signed-off-by: Sean Paul Cc: Javier Martinez Canillas Cc: Mika Kahola Cc: Yakir Yang Cc: Daniel Vetter Cc: Archit Taneja --- .../drm/bridge/analogix/analogix_dp_core.c | 274 ++++------- .../drm/bridge/analogix/analogix_dp_core.h | 40 +- .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 451 +++++------------- 3 files changed, 210 insertions(+), 555 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c index efac8aba6776..774cc7963e10 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c @@ -31,6 +31,7 @@ #include #include "analogix_dp_core.h" +#include "analogix_dp_reg.h" #define to_dp(nm) container_of(nm, struct analogix_dp_device, nm) @@ -147,7 +148,7 @@ static bool analogix_dp_detect_sink_psr(struct analogix_dp_device *dp) { unsigned char psr_version; - analogix_dp_read_byte_from_dpcd(dp, DP_PSR_SUPPORT, &psr_version); + drm_dp_dpcd_readb(&dp->aux, DP_PSR_SUPPORT, &psr_version); dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version); return (psr_version & DP_PSR_IS_SUPPORTED) ? true : false; @@ -158,166 +159,37 @@ static void analogix_dp_enable_sink_psr(struct analogix_dp_device *dp) unsigned char psr_en; /* Disable psr function */ - analogix_dp_read_byte_from_dpcd(dp, DP_PSR_EN_CFG, &psr_en); + drm_dp_dpcd_readb(&dp->aux, DP_PSR_EN_CFG, &psr_en); psr_en &= ~DP_PSR_ENABLE; - analogix_dp_write_byte_to_dpcd(dp, DP_PSR_EN_CFG, psr_en); + drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en); /* Main-Link transmitter remains active during PSR active states */ psr_en = DP_PSR_MAIN_LINK_ACTIVE | DP_PSR_CRC_VERIFICATION; - analogix_dp_write_byte_to_dpcd(dp, DP_PSR_EN_CFG, psr_en); + drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en); /* Enable psr function */ psr_en = DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE | DP_PSR_CRC_VERIFICATION; - analogix_dp_write_byte_to_dpcd(dp, DP_PSR_EN_CFG, psr_en); + drm_dp_dpcd_writeb(&dp->aux, DP_PSR_EN_CFG, psr_en); analogix_dp_enable_psr_crc(dp); } -static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data) -{ - int i; - unsigned char sum = 0; - - for (i = 0; i < EDID_BLOCK_LENGTH; i++) - sum = sum + edid_data[i]; - - return sum; -} - -static int analogix_dp_read_edid(struct analogix_dp_device *dp) -{ - unsigned char *edid = dp->edid; - unsigned int extend_block = 0; - unsigned char sum; - unsigned char test_vector; - int retval; - - /* - * EDID device address is 0x50. - * However, if necessary, you must have set upper address - * into E-EDID in I2C device, 0x30. - */ - - /* Read Extension Flag, Number of 128-byte EDID extension blocks */ - retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR, - EDID_EXTENSION_FLAG, - &extend_block); - if (retval) - return retval; - - if (extend_block > 0) { - dev_dbg(dp->dev, "EDID data includes a single extension!\n"); - - /* Read EDID data */ - retval = analogix_dp_read_bytes_from_i2c(dp, - I2C_EDID_DEVICE_ADDR, - EDID_HEADER_PATTERN, - EDID_BLOCK_LENGTH, - &edid[EDID_HEADER_PATTERN]); - if (retval != 0) { - dev_err(dp->dev, "EDID Read failed!\n"); - return -EIO; - } - sum = analogix_dp_calc_edid_check_sum(edid); - if (sum != 0) { - dev_err(dp->dev, "EDID bad checksum!\n"); - return -EIO; - } - - /* Read additional EDID data */ - retval = analogix_dp_read_bytes_from_i2c(dp, - I2C_EDID_DEVICE_ADDR, - EDID_BLOCK_LENGTH, - EDID_BLOCK_LENGTH, - &edid[EDID_BLOCK_LENGTH]); - if (retval != 0) { - dev_err(dp->dev, "EDID Read failed!\n"); - return -EIO; - } - sum = analogix_dp_calc_edid_check_sum(&edid[EDID_BLOCK_LENGTH]); - if (sum != 0) { - dev_err(dp->dev, "EDID bad checksum!\n"); - return -EIO; - } - - analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST, - &test_vector); - if (test_vector & DP_TEST_LINK_EDID_READ) { - analogix_dp_write_byte_to_dpcd(dp, - DP_TEST_EDID_CHECKSUM, - edid[EDID_BLOCK_LENGTH + EDID_CHECKSUM]); - analogix_dp_write_byte_to_dpcd(dp, - DP_TEST_RESPONSE, - DP_TEST_EDID_CHECKSUM_WRITE); - } - } else { - dev_info(dp->dev, "EDID data does not include any extensions.\n"); - - /* Read EDID data */ - retval = analogix_dp_read_bytes_from_i2c(dp, - I2C_EDID_DEVICE_ADDR, EDID_HEADER_PATTERN, - EDID_BLOCK_LENGTH, &edid[EDID_HEADER_PATTERN]); - if (retval != 0) { - dev_err(dp->dev, "EDID Read failed!\n"); - return -EIO; - } - sum = analogix_dp_calc_edid_check_sum(edid); - if (sum != 0) { - dev_err(dp->dev, "EDID bad checksum!\n"); - return -EIO; - } - - analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST, - &test_vector); - if (test_vector & DP_TEST_LINK_EDID_READ) { - analogix_dp_write_byte_to_dpcd(dp, - DP_TEST_EDID_CHECKSUM, edid[EDID_CHECKSUM]); - analogix_dp_write_byte_to_dpcd(dp, - DP_TEST_RESPONSE, DP_TEST_EDID_CHECKSUM_WRITE); - } - } - - dev_dbg(dp->dev, "EDID Read success!\n"); - return 0; -} - -static int analogix_dp_handle_edid(struct analogix_dp_device *dp) -{ - u8 buf[12]; - int i; - int retval; - - /* Read DPCD DP_DPCD_REV~RECEIVE_PORT1_CAP_1 */ - retval = analogix_dp_read_bytes_from_dpcd(dp, DP_DPCD_REV, 12, buf); - if (retval) - return retval; - - /* Read EDID */ - for (i = 0; i < 3; i++) { - retval = analogix_dp_read_edid(dp); - if (!retval) - break; - } - - return retval; -} - static void analogix_dp_enable_rx_to_enhanced_mode(struct analogix_dp_device *dp, bool enable) { u8 data; - analogix_dp_read_byte_from_dpcd(dp, DP_LANE_COUNT_SET, &data); + drm_dp_dpcd_readb(&dp->aux, DP_LANE_COUNT_SET, &data); if (enable) - analogix_dp_write_byte_to_dpcd(dp, DP_LANE_COUNT_SET, - DP_LANE_COUNT_ENHANCED_FRAME_EN | - DPCD_LANE_COUNT_SET(data)); + drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET, + DP_LANE_COUNT_ENHANCED_FRAME_EN | + DPCD_LANE_COUNT_SET(data)); else - analogix_dp_write_byte_to_dpcd(dp, DP_LANE_COUNT_SET, - DPCD_LANE_COUNT_SET(data)); + drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET, + DPCD_LANE_COUNT_SET(data)); } static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp) @@ -325,7 +197,7 @@ static int analogix_dp_is_enhanced_mode_available(struct analogix_dp_device *dp) u8 data; int retval; - analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LANE_COUNT, &data); + drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data); retval = DPCD_ENHANCED_FRAME_CAP(data); return retval; @@ -344,8 +216,8 @@ static void analogix_dp_training_pattern_dis(struct analogix_dp_device *dp) { analogix_dp_set_training_pattern(dp, DP_NONE); - analogix_dp_write_byte_to_dpcd(dp, DP_TRAINING_PATTERN_SET, - DP_TRAINING_PATTERN_DISABLE); + drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, + DP_TRAINING_PATTERN_DISABLE); } static void @@ -390,8 +262,8 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp) /* Setup RX configuration */ buf[0] = dp->link_train.link_rate; buf[1] = dp->link_train.lane_count; - retval = analogix_dp_write_bytes_to_dpcd(dp, DP_LINK_BW_SET, 2, buf); - if (retval) + retval = drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, buf, 2); + if (retval < 0) return retval; /* Set TX pre-emphasis to minimum */ @@ -415,20 +287,22 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp) analogix_dp_set_training_pattern(dp, TRAINING_PTN1); /* Set RX training pattern */ - retval = analogix_dp_write_byte_to_dpcd(dp, - DP_TRAINING_PATTERN_SET, - DP_LINK_SCRAMBLING_DISABLE | DP_TRAINING_PATTERN_1); - if (retval) + retval = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, + DP_LINK_SCRAMBLING_DISABLE | + DP_TRAINING_PATTERN_1); + if (retval < 0) return retval; for (lane = 0; lane < lane_count; lane++) buf[lane] = DP_TRAIN_PRE_EMPH_LEVEL_0 | DP_TRAIN_VOLTAGE_SWING_LEVEL_0; - retval = analogix_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET, - lane_count, buf); + retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, buf, + lane_count); + if (retval < 0) + return retval; - return retval; + return 0; } static unsigned char analogix_dp_get_lane_status(u8 link_status[2], int lane) @@ -580,25 +454,23 @@ static int analogix_dp_process_clock_recovery(struct analogix_dp_device *dp) lane_count = dp->link_train.lane_count; - retval = analogix_dp_read_bytes_from_dpcd(dp, - DP_LANE0_1_STATUS, 2, link_status); - if (retval) + retval = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status, 2); + if (retval < 0) return retval; - retval = analogix_dp_read_bytes_from_dpcd(dp, - DP_ADJUST_REQUEST_LANE0_1, 2, adjust_request); - if (retval) + retval = drm_dp_dpcd_read(&dp->aux, DP_ADJUST_REQUEST_LANE0_1, + adjust_request, 2); + if (retval < 0) return retval; if (analogix_dp_clock_recovery_ok(link_status, lane_count) == 0) { /* set training pattern 2 for EQ */ analogix_dp_set_training_pattern(dp, TRAINING_PTN2); - retval = analogix_dp_write_byte_to_dpcd(dp, - DP_TRAINING_PATTERN_SET, - DP_LINK_SCRAMBLING_DISABLE | - DP_TRAINING_PATTERN_2); - if (retval) + retval = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, + DP_LINK_SCRAMBLING_DISABLE | + DP_TRAINING_PATTERN_2); + if (retval < 0) return retval; dev_info(dp->dev, "Link Training Clock Recovery success\n"); @@ -636,13 +508,12 @@ static int analogix_dp_process_clock_recovery(struct analogix_dp_device *dp) analogix_dp_set_lane_link_training(dp, dp->link_train.training_lane[lane], lane); - retval = analogix_dp_write_bytes_to_dpcd(dp, - DP_TRAINING_LANE0_SET, lane_count, - dp->link_train.training_lane); - if (retval) + retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, + dp->link_train.training_lane, lane_count); + if (retval < 0) return retval; - return retval; + return 0; } static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) @@ -655,9 +526,8 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) lane_count = dp->link_train.lane_count; - retval = analogix_dp_read_bytes_from_dpcd(dp, - DP_LANE0_1_STATUS, 2, link_status); - if (retval) + retval = drm_dp_dpcd_read(&dp->aux, DP_LANE0_1_STATUS, link_status, 2); + if (retval < 0) return retval; if (analogix_dp_clock_recovery_ok(link_status, lane_count)) { @@ -665,14 +535,14 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) return -EIO; } - retval = analogix_dp_read_bytes_from_dpcd(dp, - DP_ADJUST_REQUEST_LANE0_1, 2, adjust_request); - if (retval) + retval = drm_dp_dpcd_read(&dp->aux, DP_ADJUST_REQUEST_LANE0_1, + adjust_request, 2); + if (retval < 0) return retval; - retval = analogix_dp_read_byte_from_dpcd(dp, - DP_LANE_ALIGN_STATUS_UPDATED, &link_align); - if (retval) + retval = drm_dp_dpcd_readb(&dp->aux, DP_LANE_ALIGN_STATUS_UPDATED, + &link_align); + if (retval < 0) return retval; analogix_dp_get_adjust_training_lane(dp, adjust_request); @@ -713,10 +583,12 @@ static int analogix_dp_process_equalizer_training(struct analogix_dp_device *dp) analogix_dp_set_lane_link_training(dp, dp->link_train.training_lane[lane], lane); - retval = analogix_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET, - lane_count, dp->link_train.training_lane); + retval = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, + dp->link_train.training_lane, lane_count); + if (retval < 0) + return retval; - return retval; + return 0; } static void analogix_dp_get_max_rx_bandwidth(struct analogix_dp_device *dp, @@ -730,7 +602,7 @@ static void analogix_dp_get_max_rx_bandwidth(struct analogix_dp_device *dp, * For DP rev.1.2, Maximum link rate of Main Link lanes * 0x06 = 1.62 Gbps, 0x0a = 2.7 Gbps, 0x14 = 5.4Gbps */ - analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LINK_RATE, &data); + drm_dp_dpcd_readb(&dp->aux, DP_MAX_LINK_RATE, &data); *bandwidth = data; } @@ -743,7 +615,7 @@ static void analogix_dp_get_max_rx_lane_count(struct analogix_dp_device *dp, * For DP rev.1.1, Maximum number of Main Link lanes * 0x01 = 1 lane, 0x02 = 2 lanes, 0x04 = 4 lanes */ - analogix_dp_read_byte_from_dpcd(dp, DP_MAX_LANE_COUNT, &data); + drm_dp_dpcd_readb(&dp->aux, DP_MAX_LANE_COUNT, &data); *lane_count = DPCD_MAX_LANE_COUNT(data); } @@ -912,19 +784,15 @@ static void analogix_dp_enable_scramble(struct analogix_dp_device *dp, if (enable) { analogix_dp_enable_scrambling(dp); - analogix_dp_read_byte_from_dpcd(dp, DP_TRAINING_PATTERN_SET, - &data); - analogix_dp_write_byte_to_dpcd(dp, - DP_TRAINING_PATTERN_SET, - (u8)(data & ~DP_LINK_SCRAMBLING_DISABLE)); + drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data); + drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, + (u8)(data & ~DP_LINK_SCRAMBLING_DISABLE)); } else { analogix_dp_disable_scrambling(dp); - analogix_dp_read_byte_from_dpcd(dp, DP_TRAINING_PATTERN_SET, - &data); - analogix_dp_write_byte_to_dpcd(dp, - DP_TRAINING_PATTERN_SET, - (u8)(data | DP_LINK_SCRAMBLING_DISABLE)); + drm_dp_dpcd_readb(&dp->aux, DP_TRAINING_PATTERN_SET, &data); + drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, + (u8)(data | DP_LINK_SCRAMBLING_DISABLE)); } } @@ -1053,7 +921,7 @@ out: int analogix_dp_get_modes(struct drm_connector *connector) { struct analogix_dp_device *dp = to_dp(connector); - struct edid *edid = (struct edid *)dp->edid; + struct edid *edid; int ret, num_modes = 0; ret = analogix_dp_prepare_panel(dp, true, false); @@ -1062,9 +930,11 @@ int analogix_dp_get_modes(struct drm_connector *connector) return 0; } - if (analogix_dp_handle_edid(dp) == 0) { + edid = drm_get_edid(connector, &dp->aux.ddc); + if (edid) { drm_mode_connector_update_edid_property(&dp->connector, edid); num_modes += drm_add_edid_modes(&dp->connector, edid); + kfree(edid); } if (dp->plat_data->panel) @@ -1395,6 +1265,14 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp) return 0; } +static ssize_t analogix_dpaux_transfer(struct drm_dp_aux *aux, + struct drm_dp_aux_msg *msg) +{ + struct analogix_dp_device *dp = to_dp(aux); + + return analogix_dp_transfer(dp, msg); +} + int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, struct analogix_dp_plat_data *plat_data) { @@ -1515,6 +1393,14 @@ int analogix_dp_bind(struct device *dev, struct drm_device *drm_dev, dp->drm_dev = drm_dev; dp->encoder = dp->plat_data->encoder; + dp->aux.name = "DP-AUX"; + dp->aux.transfer = analogix_dpaux_transfer; + dp->aux.dev = &pdev->dev; + + ret = drm_dp_aux_register(&dp->aux); + if (ret) + goto err_disable_pm_runtime; + ret = analogix_dp_create_bridge(drm_dev, dp); if (ret) { DRM_ERROR("failed to create bridge (%d)\n", ret); diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h index 473b9802b2d6..5c6a28806129 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h @@ -20,15 +20,6 @@ #define MAX_CR_LOOP 5 #define MAX_EQ_LOOP 5 -/* I2C EDID Chip ID, Slave Address */ -#define I2C_EDID_DEVICE_ADDR 0x50 -#define I2C_E_EDID_DEVICE_ADDR 0x30 - -#define EDID_BLOCK_LENGTH 0x80 -#define EDID_HEADER_PATTERN 0x00 -#define EDID_EXTENSION_FLAG 0x7e -#define EDID_CHECKSUM 0x7f - /* DP_MAX_LANE_COUNT */ #define DPCD_ENHANCED_FRAME_CAP(x) (((x) >> 7) & 0x1) #define DPCD_MAX_LANE_COUNT(x) ((x) & 0x1f) @@ -166,6 +157,7 @@ struct analogix_dp_device { struct drm_device *drm_dev; struct drm_connector connector; struct drm_bridge *bridge; + struct drm_dp_aux aux; struct clk *clock; unsigned int irq; void __iomem *reg_base; @@ -176,7 +168,6 @@ struct analogix_dp_device { int dpms_mode; int hpd_gpio; bool force_hpd; - unsigned char edid[EDID_BLOCK_LENGTH * 2]; bool psr_support; struct mutex panel_lock; @@ -210,33 +201,6 @@ void analogix_dp_reset_aux(struct analogix_dp_device *dp); void analogix_dp_init_aux(struct analogix_dp_device *dp); int analogix_dp_get_plug_in_status(struct analogix_dp_device *dp); void analogix_dp_enable_sw_function(struct analogix_dp_device *dp); -int analogix_dp_start_aux_transaction(struct analogix_dp_device *dp); -int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp, - unsigned int reg_addr, - unsigned char data); -int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp, - unsigned int reg_addr, - unsigned char *data); -int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp, - unsigned int reg_addr, - unsigned int count, - unsigned char data[]); -int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp, - unsigned int reg_addr, - unsigned int count, - unsigned char data[]); -int analogix_dp_select_i2c_device(struct analogix_dp_device *dp, - unsigned int device_addr, - unsigned int reg_addr); -int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp, - unsigned int device_addr, - unsigned int reg_addr, - unsigned int *data); -int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp, - unsigned int device_addr, - unsigned int reg_addr, - unsigned int count, - unsigned char edid[]); void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype); void analogix_dp_get_link_bandwidth(struct analogix_dp_device *dp, u32 *bwtype); void analogix_dp_set_lane_count(struct analogix_dp_device *dp, u32 count); @@ -285,5 +249,7 @@ void analogix_dp_disable_scrambling(struct analogix_dp_device *dp); void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp); void analogix_dp_send_psr_spd(struct analogix_dp_device *dp, struct edp_vsc_psr *vsc); +ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, + struct drm_dp_aux_msg *msg); #endif /* _ANALOGIX_DP_CORE_H */ diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c index fae0293d509a..cd37ac058675 100644 --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c @@ -585,330 +585,6 @@ int analogix_dp_write_byte_to_dpcd(struct analogix_dp_device *dp, return retval; } -int analogix_dp_read_byte_from_dpcd(struct analogix_dp_device *dp, - unsigned int reg_addr, - unsigned char *data) -{ - u32 reg; - int i; - int retval; - - for (i = 0; i < 3; i++) { - /* Clear AUX CH data buffer */ - reg = BUF_CLR; - writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL); - - /* Select DPCD device address */ - reg = AUX_ADDR_7_0(reg_addr); - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0); - reg = AUX_ADDR_15_8(reg_addr); - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8); - reg = AUX_ADDR_19_16(reg_addr); - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16); - - /* - * Set DisplayPort transaction and read 1 byte - * If bit 3 is 1, DisplayPort transaction. - * If Bit 3 is 0, I2C transaction. - */ - reg = AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_READ; - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1); - - /* Start AUX transaction */ - retval = analogix_dp_start_aux_transaction(dp); - if (retval == 0) - break; - - dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__); - } - - /* Read data buffer */ - reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0); - *data = (unsigned char)(reg & 0xff); - - return retval; -} - -int analogix_dp_write_bytes_to_dpcd(struct analogix_dp_device *dp, - unsigned int reg_addr, - unsigned int count, - unsigned char data[]) -{ - u32 reg; - unsigned int start_offset; - unsigned int cur_data_count; - unsigned int cur_data_idx; - int i; - int retval = 0; - - /* Clear AUX CH data buffer */ - reg = BUF_CLR; - writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL); - - start_offset = 0; - while (start_offset < count) { - /* Buffer size of AUX CH is 16 * 4bytes */ - if ((count - start_offset) > 16) - cur_data_count = 16; - else - cur_data_count = count - start_offset; - - for (i = 0; i < 3; i++) { - /* Select DPCD device address */ - reg = AUX_ADDR_7_0(reg_addr + start_offset); - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0); - reg = AUX_ADDR_15_8(reg_addr + start_offset); - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8); - reg = AUX_ADDR_19_16(reg_addr + start_offset); - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16); - - for (cur_data_idx = 0; cur_data_idx < cur_data_count; - cur_data_idx++) { - reg = data[start_offset + cur_data_idx]; - writel(reg, dp->reg_base + - ANALOGIX_DP_BUF_DATA_0 + - 4 * cur_data_idx); - } - - /* - * Set DisplayPort transaction and write - * If bit 3 is 1, DisplayPort transaction. - * If Bit 3 is 0, I2C transaction. - */ - reg = AUX_LENGTH(cur_data_count) | - AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_WRITE; - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1); - - /* Start AUX transaction */ - retval = analogix_dp_start_aux_transaction(dp); - if (retval == 0) - break; - - dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", - __func__); - } - - start_offset += cur_data_count; - } - - return retval; -} - -int analogix_dp_read_bytes_from_dpcd(struct analogix_dp_device *dp, - unsigned int reg_addr, - unsigned int count, - unsigned char data[]) -{ - u32 reg; - unsigned int start_offset; - unsigned int cur_data_count; - unsigned int cur_data_idx; - int i; - int retval = 0; - - /* Clear AUX CH data buffer */ - reg = BUF_CLR; - writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL); - - start_offset = 0; - while (start_offset < count) { - /* Buffer size of AUX CH is 16 * 4bytes */ - if ((count - start_offset) > 16) - cur_data_count = 16; - else - cur_data_count = count - start_offset; - - /* AUX CH Request Transaction process */ - for (i = 0; i < 3; i++) { - /* Select DPCD device address */ - reg = AUX_ADDR_7_0(reg_addr + start_offset); - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0); - reg = AUX_ADDR_15_8(reg_addr + start_offset); - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8); - reg = AUX_ADDR_19_16(reg_addr + start_offset); - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16); - - /* - * Set DisplayPort transaction and read - * If bit 3 is 1, DisplayPort transaction. - * If Bit 3 is 0, I2C transaction. - */ - reg = AUX_LENGTH(cur_data_count) | - AUX_TX_COMM_DP_TRANSACTION | AUX_TX_COMM_READ; - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1); - - /* Start AUX transaction */ - retval = analogix_dp_start_aux_transaction(dp); - if (retval == 0) - break; - - dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", - __func__); - } - - for (cur_data_idx = 0; cur_data_idx < cur_data_count; - cur_data_idx++) { - reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0 - + 4 * cur_data_idx); - data[start_offset + cur_data_idx] = - (unsigned char)reg; - } - - start_offset += cur_data_count; - } - - return retval; -} - -int analogix_dp_select_i2c_device(struct analogix_dp_device *dp, - unsigned int device_addr, - unsigned int reg_addr) -{ - u32 reg; - int retval; - - /* Set EDID device address */ - reg = device_addr; - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0); - writel(0x0, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8); - writel(0x0, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16); - - /* Set offset from base address of EDID device */ - writel(reg_addr, dp->reg_base + ANALOGIX_DP_BUF_DATA_0); - - /* - * Set I2C transaction and write address - * If bit 3 is 1, DisplayPort transaction. - * If Bit 3 is 0, I2C transaction. - */ - reg = AUX_TX_COMM_I2C_TRANSACTION | AUX_TX_COMM_MOT | - AUX_TX_COMM_WRITE; - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1); - - /* Start AUX transaction */ - retval = analogix_dp_start_aux_transaction(dp); - if (retval != 0) - dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__); - - return retval; -} - -int analogix_dp_read_byte_from_i2c(struct analogix_dp_device *dp, - unsigned int device_addr, - unsigned int reg_addr, - unsigned int *data) -{ - u32 reg; - int i; - int retval; - - for (i = 0; i < 3; i++) { - /* Clear AUX CH data buffer */ - reg = BUF_CLR; - writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL); - - /* Select EDID device */ - retval = analogix_dp_select_i2c_device(dp, device_addr, - reg_addr); - if (retval != 0) - continue; - - /* - * Set I2C transaction and read data - * If bit 3 is 1, DisplayPort transaction. - * If Bit 3 is 0, I2C transaction. - */ - reg = AUX_TX_COMM_I2C_TRANSACTION | - AUX_TX_COMM_READ; - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1); - - /* Start AUX transaction */ - retval = analogix_dp_start_aux_transaction(dp); - if (retval == 0) - break; - - dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", __func__); - } - - /* Read data */ - if (retval == 0) - *data = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0); - - return retval; -} - -int analogix_dp_read_bytes_from_i2c(struct analogix_dp_device *dp, - unsigned int device_addr, - unsigned int reg_addr, - unsigned int count, - unsigned char edid[]) -{ - u32 reg; - unsigned int i, j; - unsigned int cur_data_idx; - unsigned int defer = 0; - int retval = 0; - - for (i = 0; i < count; i += 16) { - for (j = 0; j < 3; j++) { - /* Clear AUX CH data buffer */ - reg = BUF_CLR; - writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL); - - /* Set normal AUX CH command */ - reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2); - reg &= ~ADDR_ONLY; - writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2); - - /* - * If Rx sends defer, Tx sends only reads - * request without sending address - */ - if (!defer) - retval = analogix_dp_select_i2c_device(dp, - device_addr, reg_addr + i); - else - defer = 0; - - if (retval == 0) { - /* - * Set I2C transaction and write data - * If bit 3 is 1, DisplayPort transaction. - * If Bit 3 is 0, I2C transaction. - */ - reg = AUX_LENGTH(16) | - AUX_TX_COMM_I2C_TRANSACTION | - AUX_TX_COMM_READ; - writel(reg, dp->reg_base + - ANALOGIX_DP_AUX_CH_CTL_1); - - /* Start AUX transaction */ - retval = analogix_dp_start_aux_transaction(dp); - if (retval == 0) - break; - - dev_dbg(dp->dev, "%s: Aux Transaction fail!\n", - __func__); - } - /* Check if Rx sends defer */ - reg = readl(dp->reg_base + ANALOGIX_DP_AUX_RX_COMM); - if (reg == AUX_RX_COMM_AUX_DEFER || - reg == AUX_RX_COMM_I2C_DEFER) { - dev_err(dp->dev, "Defer: %d\n\n", reg); - defer = 1; - } - } - - for (cur_data_idx = 0; cur_data_idx < 16; cur_data_idx++) { - reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0 - + 4 * cur_data_idx); - edid[i + cur_data_idx] = (unsigned char)reg; - } - } - - return retval; -} - void analogix_dp_set_link_bandwidth(struct analogix_dp_device *dp, u32 bwtype) { u32 reg; @@ -1361,3 +1037,130 @@ void analogix_dp_send_psr_spd(struct analogix_dp_device *dp, val |= IF_EN; writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL); } + +ssize_t analogix_dp_transfer(struct analogix_dp_device *dp, + struct drm_dp_aux_msg *msg) +{ + u32 reg; + u8 *buffer = msg->buffer; + int timeout_loop = 0; + unsigned int i; + int num_transferred = 0; + + /* Buffer size of AUX CH is 16 bytes */ + if (WARN_ON(msg->size > 16)) + return -E2BIG; + + /* Clear AUX CH data buffer */ + reg = BUF_CLR; + writel(reg, dp->reg_base + ANALOGIX_DP_BUFFER_DATA_CTL); + + switch (msg->request & ~DP_AUX_I2C_MOT) { + case DP_AUX_I2C_WRITE: + reg = AUX_TX_COMM_WRITE | AUX_TX_COMM_I2C_TRANSACTION; + if (msg->request & DP_AUX_I2C_MOT) + reg |= AUX_TX_COMM_MOT; + break; + + case DP_AUX_I2C_READ: + reg = AUX_TX_COMM_READ | AUX_TX_COMM_I2C_TRANSACTION; + if (msg->request & DP_AUX_I2C_MOT) + reg |= AUX_TX_COMM_MOT; + break; + + case DP_AUX_NATIVE_WRITE: + reg = AUX_TX_COMM_WRITE | AUX_TX_COMM_DP_TRANSACTION; + break; + + case DP_AUX_NATIVE_READ: + reg = AUX_TX_COMM_READ | AUX_TX_COMM_DP_TRANSACTION; + break; + + default: + return -EINVAL; + } + + reg |= AUX_LENGTH(msg->size); + writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_1); + + /* Select DPCD device address */ + reg = AUX_ADDR_7_0(msg->address); + writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_7_0); + reg = AUX_ADDR_15_8(msg->address); + writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_15_8); + reg = AUX_ADDR_19_16(msg->address); + writel(reg, dp->reg_base + ANALOGIX_DP_AUX_ADDR_19_16); + + if (!(msg->request & DP_AUX_I2C_READ)) { + for (i = 0; i < msg->size; i++) { + reg = buffer[i]; + writel(reg, dp->reg_base + ANALOGIX_DP_BUF_DATA_0 + + 4 * i); + num_transferred++; + } + } + + /* Enable AUX CH operation */ + reg = AUX_EN; + + /* Zero-sized messages specify address-only transactions. */ + if (msg->size < 1) + reg |= ADDR_ONLY; + + writel(reg, dp->reg_base + ANALOGIX_DP_AUX_CH_CTL_2); + + /* Is AUX CH command reply received? */ + /* TODO: Wait for an interrupt instead of looping? */ + reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA); + while (!(reg & RPLY_RECEIV)) { + timeout_loop++; + if (timeout_loop > DP_TIMEOUT_LOOP_COUNT) { + dev_err(dp->dev, "AUX CH command reply failed!\n"); + return -ETIMEDOUT; + } + reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA); + usleep_range(10, 11); + } + + /* Clear interrupt source for AUX CH command reply */ + writel(RPLY_RECEIV, dp->reg_base + ANALOGIX_DP_INT_STA); + + /* Clear interrupt source for AUX CH access error */ + reg = readl(dp->reg_base + ANALOGIX_DP_INT_STA); + if (reg & AUX_ERR) { + writel(AUX_ERR, dp->reg_base + ANALOGIX_DP_INT_STA); + return -EREMOTEIO; + } + + /* Check AUX CH error access status */ + reg = readl(dp->reg_base + ANALOGIX_DP_AUX_CH_STA); + if ((reg & AUX_STATUS_MASK)) { + dev_err(dp->dev, "AUX CH error happened: %d\n\n", + reg & AUX_STATUS_MASK); + return -EREMOTEIO; + } + + if (msg->request & DP_AUX_I2C_READ) { + for (i = 0; i < msg->size; i++) { + reg = readl(dp->reg_base + ANALOGIX_DP_BUF_DATA_0 + + 4 * i); + buffer[i] = (unsigned char)reg; + num_transferred++; + } + } + + /* Check if Rx sends defer */ + reg = readl(dp->reg_base + ANALOGIX_DP_AUX_RX_COMM); + if (reg == AUX_RX_COMM_AUX_DEFER) + msg->reply = DP_AUX_NATIVE_REPLY_DEFER; + else if (reg == AUX_RX_COMM_I2C_DEFER) + msg->reply = DP_AUX_I2C_REPLY_DEFER; + else if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE || + (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_I2C_READ) + msg->reply = DP_AUX_I2C_REPLY_ACK; + else if ((msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_WRITE || + (msg->request & ~DP_AUX_I2C_MOT) == DP_AUX_NATIVE_READ) + msg->reply = DP_AUX_NATIVE_REPLY_ACK; + + return num_transferred; +} From be91a983eaf272ed0a982e7c92b47e78d7ccc51a Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Tue, 6 Sep 2016 14:11:53 -0400 Subject: [PATCH 04/13] drm/rockchip: Fix up bug in psr state machine The ->set() callback would always be called when transitioning from FLUSH->DISABLE since we assign state to psr->state right above the skip condition. Reported-by: Daniel Kurtz Reviewed-by: Daniel Kurtz Signed-off-by: Sean Paul --- drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c index 2cdd6eb359f2..a553e182ff53 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c @@ -71,11 +71,13 @@ static void psr_set_state_locked(struct psr_drv *psr, enum psr_state state) if (state == psr->state || !psr->active) return; - psr->state = state; - /* Already disabled in flush, change the state, but not the hardware */ - if (state == PSR_DISABLE && psr->state == PSR_FLUSH) + if (state == PSR_DISABLE && psr->state == PSR_FLUSH) { + psr->state = state; return; + } + + psr->state = state; /* Actually commit the state change to hardware */ switch (psr->state) { From fa374107c1c7c483263e360cba68d078231eeb6a Mon Sep 17 00:00:00 2001 From: Tomasz Figa Date: Wed, 14 Sep 2016 21:54:54 +0900 Subject: [PATCH 05/13] drm/rockchip: Clear interrupt status bits before enabling The enable register only masks the raw status bits to signal CPU interrupt only for enabled interrupts. The status bits are activated regardless of the enable register. This means that we might have an old interrupt event queued, which we are not interested in. To avoid getting a spurious interrupt signalled, we have to clear the old bit before we update the enable register. Signed-off-by: Tomasz Figa --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index efb216005baa..48fe554fa620 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -414,6 +414,7 @@ static void vop_dsp_hold_valid_irq_enable(struct vop *vop) spin_lock_irqsave(&vop->irq_lock, flags); + VOP_INTR_SET_TYPE(vop, clear, DSP_HOLD_VALID_INTR, 1); VOP_INTR_SET_TYPE(vop, enable, DSP_HOLD_VALID_INTR, 1); spin_unlock_irqrestore(&vop->irq_lock, flags); @@ -479,6 +480,7 @@ static void vop_line_flag_irq_enable(struct vop *vop, int line_num) spin_lock_irqsave(&vop->irq_lock, flags); VOP_CTRL_SET(vop, line_flag_num[0], line_num); + VOP_INTR_SET_TYPE(vop, clear, LINE_FLAG_INTR, 1); VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1); spin_unlock_irqrestore(&vop->irq_lock, flags); @@ -917,6 +919,7 @@ static int vop_crtc_enable_vblank(struct drm_crtc *crtc) spin_lock_irqsave(&vop->irq_lock, flags); + VOP_INTR_SET_TYPE(vop, clear, FS_INTR, 1); VOP_INTR_SET_TYPE(vop, enable, FS_INTR, 1); spin_unlock_irqrestore(&vop->irq_lock, flags); From 65bcb6bcc8b791ed9fa97603c2ed8063d41cedbe Mon Sep 17 00:00:00 2001 From: Tomasz Figa Date: Wed, 14 Sep 2016 21:54:55 +0900 Subject: [PATCH 06/13] drm/rockchip: Get rid of some unnecessary code Current code implements prepare_fb and cleanup_fb callbacks only to grab/release fb references, which is already done by atomic framework when creating/destryoing plane state. Let's remove these unused bits. Signed-off-by: Tomasz Figa --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 48fe554fa620..ba30c6a51bf7 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -637,22 +637,6 @@ static void vop_plane_destroy(struct drm_plane *plane) drm_plane_cleanup(plane); } -static int vop_plane_prepare_fb(struct drm_plane *plane, - struct drm_plane_state *new_state) -{ - if (plane->state->fb) - drm_framebuffer_reference(plane->state->fb); - - return 0; -} - -static void vop_plane_cleanup_fb(struct drm_plane *plane, - struct drm_plane_state *old_state) -{ - if (old_state->fb) - drm_framebuffer_unreference(old_state->fb); -} - static int vop_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) { @@ -845,8 +829,6 @@ static void vop_plane_atomic_update(struct drm_plane *plane, } static const struct drm_plane_helper_funcs plane_helper_funcs = { - .prepare_fb = vop_plane_prepare_fb, - .cleanup_fb = vop_plane_cleanup_fb, .atomic_check = vop_plane_atomic_check, .atomic_update = vop_plane_atomic_update, .atomic_disable = vop_plane_atomic_disable, From 7caecdbec1128a934ee628f399110dff8a50c9a1 Mon Sep 17 00:00:00 2001 From: Tomasz Figa Date: Wed, 14 Sep 2016 21:54:56 +0900 Subject: [PATCH 07/13] drm/rockchip: Avoid race with vblank count increment Since VOP does not have a hardware vblank count register, the ongoing commit might be racing with a requested vblank interrupt, which would increment the software vblank counter before the changes being committed actually happen. To avoid this, we can extend .atomic_flush(), so after it sets cfg_done bit, it polls the vblank interrupt bit until it's inactive to make sure that any old vblank interrupt gets to the handler and then uses synchronize_irq(vop->irq) to make sure the handler finishes running. The polling case should happen very rarely, but even if, the total wait time should be relatively low and in practice almost equal to the vop hardirq handler running time. Signed-off-by: Tomasz Figa --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 34 +++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index ba30c6a51bf7..0c0dc808453f 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -1059,6 +1060,32 @@ static void vop_crtc_enable(struct drm_crtc *crtc) rockchip_drm_psr_activate(&vop->crtc); } +static bool vop_fs_irq_is_pending(struct vop *vop) +{ + return VOP_INTR_GET_TYPE(vop, status, FS_INTR); +} + +static void vop_wait_for_irq_handler(struct vop *vop) +{ + bool pending; + int ret; + + /* + * Spin until frame start interrupt status bit goes low, which means + * that interrupt handler was invoked and cleared it. The timeout of + * 10 msecs is really too long, but it is just a safety measure if + * something goes really wrong. The wait will only happen in the very + * unlikely case of a vblank happening exactly at the same time and + * shouldn't exceed microseconds range. + */ + ret = readx_poll_timeout_atomic(vop_fs_irq_is_pending, vop, pending, + !pending, 0, 10 * 1000); + if (ret) + DRM_DEV_ERROR(vop->dev, "VOP vblank IRQ stuck for 10 ms\n"); + + synchronize_irq(vop->irq); +} + static void vop_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) { @@ -1072,6 +1099,13 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc, vop_cfg_done(vop); spin_unlock(&vop->reg_lock); + + /* + * There is a (rather unlikely) possiblity that a vblank interrupt + * fired before we set the cfg_done bit. To avoid spuriously + * signalling flip completion we need to wait for it to finish. + */ + vop_wait_for_irq_handler(vop); } static void vop_crtc_atomic_begin(struct drm_crtc *crtc, From 47a7eb4597775ecdc29d2630d875a991f0449bf3 Mon Sep 17 00:00:00 2001 From: Tomasz Figa Date: Wed, 14 Sep 2016 21:54:57 +0900 Subject: [PATCH 08/13] drm/rockchip: Unreference framebuffers from flip work Currently the driver waits for vblank and then unreferences old framebuffers from atomic commit code path. This is however breaking the legacy cursor API, which requires the updates to be fully asynchronous. Instead of just adding a special case for cursor, we can have actually smaller amount of code to unreference any changed framebuffer from a flip work. Signed-off-by: Tomasz Figa --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 41 +++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 0c0dc808453f..82dafcdd9056 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -89,6 +90,10 @@ #define to_vop_win(x) container_of(x, struct vop_win, base) #define to_vop_plane_state(x) container_of(x, struct vop_plane_state, base) +enum vop_pending { + VOP_PENDING_FB_UNREF, +}; + struct vop_plane_state { struct drm_plane_state base; int format; @@ -122,6 +127,9 @@ struct vop { /* protected by dev->event_lock */ struct drm_pending_vblank_event *event; + struct drm_flip_work fb_unref_work; + unsigned long pending; + struct completion line_flag_completion; const struct vop_data *data; @@ -1089,7 +1097,11 @@ static void vop_wait_for_irq_handler(struct vop *vop) static void vop_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) { + struct drm_atomic_state *old_state = old_crtc_state->state; + struct drm_plane_state *old_plane_state; struct vop *vop = to_vop(crtc); + struct drm_plane *plane; + int i; if (WARN_ON(!vop->is_enabled)) return; @@ -1106,6 +1118,19 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc, * signalling flip completion we need to wait for it to finish. */ vop_wait_for_irq_handler(vop); + + for_each_plane_in_state(old_state, plane, old_plane_state, i) { + if (!old_plane_state->fb) + continue; + + if (old_plane_state->fb == plane->state->fb) + continue; + + drm_framebuffer_reference(old_plane_state->fb); + drm_flip_work_queue(&vop->fb_unref_work, old_plane_state->fb); + set_bit(VOP_PENDING_FB_UNREF, &vop->pending); + WARN_ON(drm_crtc_vblank_get(crtc) != 0); + } } static void vop_crtc_atomic_begin(struct drm_crtc *crtc, @@ -1181,6 +1206,15 @@ static const struct drm_crtc_funcs vop_crtc_funcs = { .atomic_destroy_state = vop_crtc_destroy_state, }; +static void vop_fb_unref_worker(struct drm_flip_work *work, void *val) +{ + struct vop *vop = container_of(work, struct vop, fb_unref_work); + struct drm_framebuffer *fb = val; + + drm_crtc_vblank_put(&vop->crtc); + drm_framebuffer_unreference(fb); +} + static bool vop_win_pending_is_complete(struct vop_win *vop_win) { dma_addr_t yrgb_mst; @@ -1219,6 +1253,9 @@ static void vop_handle_vblank(struct vop *vop) if (!completion_done(&vop->wait_update_complete)) complete(&vop->wait_update_complete); + + if (test_and_clear_bit(VOP_PENDING_FB_UNREF, &vop->pending)) + drm_flip_work_commit(&vop->fb_unref_work, system_unbound_wq); } static irqreturn_t vop_isr(int irq, void *data) @@ -1357,6 +1394,9 @@ static int vop_create_crtc(struct vop *vop) goto err_cleanup_crtc; } + drm_flip_work_init(&vop->fb_unref_work, "fb_unref", + vop_fb_unref_worker); + init_completion(&vop->dsp_hold_completion); init_completion(&vop->wait_update_complete); init_completion(&vop->line_flag_completion); @@ -1400,6 +1440,7 @@ static void vop_destroy_crtc(struct vop *vop) * references the CRTC. */ drm_crtc_cleanup(crtc); + drm_flip_work_cleanup(&vop->fb_unref_work); } static int vop_initial(struct vop *vop) From 81c248f75a130c1ce46c67e8b05b37e8ffbbb33e Mon Sep 17 00:00:00 2001 From: Tomasz Figa Date: Wed, 14 Sep 2016 21:54:58 +0900 Subject: [PATCH 09/13] drm/rockchip: Replace custom wait_for_vblanks with helper Currently the driver uses a custom function to wait for flip to complete after an atomic commit. It was needed before because of two problems: - there is no hardware vblank counter, so the original helper would have a race condition with the vblank interrupt, - the driver didn't support unreferencing cursor framebuffers asynchronously to the commit, which was what the helper expected. Since both problems have been solved by previous patches, we can now make the driver use the generic helper and remove custom waiting code. Signed-off-by: Tomasz Figa --- drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 - drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 64 +-------------------- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 14 ----- 3 files changed, 1 insertion(+), 78 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h index 5c698456aa1c..fb6226cf84b7 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h @@ -39,7 +39,6 @@ struct drm_connector; struct rockchip_crtc_funcs { int (*enable_vblank)(struct drm_crtc *crtc); void (*disable_vblank)(struct drm_crtc *crtc); - void (*wait_for_update)(struct drm_crtc *crtc); }; struct rockchip_crtc_state { diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c index 9890eccadd3b..0f6eda023bd0 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c @@ -174,68 +174,6 @@ static void rockchip_drm_output_poll_changed(struct drm_device *dev) drm_fb_helper_hotplug_event(fb_helper); } -static void rockchip_crtc_wait_for_update(struct drm_crtc *crtc) -{ - struct rockchip_drm_private *priv = crtc->dev->dev_private; - int pipe = drm_crtc_index(crtc); - const struct rockchip_crtc_funcs *crtc_funcs = priv->crtc_funcs[pipe]; - - if (crtc_funcs && crtc_funcs->wait_for_update) - crtc_funcs->wait_for_update(crtc); -} - -/* - * We can't use drm_atomic_helper_wait_for_vblanks() because rk3288 and rk3066 - * have hardware counters for neither vblanks nor scanlines, which results in - * a race where: - * | <-- HW vsync irq and reg take effect - * plane_commit --> | - * get_vblank and wait --> | - * | <-- handle_vblank, vblank->count + 1 - * cleanup_fb --> | - * iommu crash --> | - * | <-- HW vsync irq and reg take effect - * - * This function is equivalent but uses rockchip_crtc_wait_for_update() instead - * of waiting for vblank_count to change. - */ -static void -rockchip_atomic_wait_for_complete(struct drm_device *dev, struct drm_atomic_state *old_state) -{ - struct drm_crtc_state *old_crtc_state; - struct drm_crtc *crtc; - int i, ret; - - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { - /* No one cares about the old state, so abuse it for tracking - * and store whether we hold a vblank reference (and should do a - * vblank wait) in the ->enable boolean. - */ - old_crtc_state->enable = false; - - if (!crtc->state->active) - continue; - - if (!drm_atomic_helper_framebuffer_changed(dev, - old_state, crtc)) - continue; - - ret = drm_crtc_vblank_get(crtc); - if (ret != 0) - continue; - - old_crtc_state->enable = true; - } - - for_each_crtc_in_state(old_state, crtc, old_crtc_state, i) { - if (!old_crtc_state->enable) - continue; - - rockchip_crtc_wait_for_update(crtc); - drm_crtc_vblank_put(crtc); - } -} - static void rockchip_atomic_commit_tail(struct drm_atomic_state *state) { @@ -250,7 +188,7 @@ rockchip_atomic_commit_tail(struct drm_atomic_state *state) drm_atomic_helper_commit_hw_done(state); - rockchip_atomic_wait_for_complete(dev, state); + drm_atomic_helper_wait_for_vblanks(dev, state); drm_atomic_helper_cleanup_planes(dev, state); } diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 82dafcdd9056..b6d47dda691a 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -122,7 +122,6 @@ struct vop { struct mutex vsync_mutex; bool vsync_work_pending; struct completion dsp_hold_completion; - struct completion wait_update_complete; /* protected by dev->event_lock */ struct drm_pending_vblank_event *event; @@ -933,18 +932,9 @@ static void vop_crtc_disable_vblank(struct drm_crtc *crtc) spin_unlock_irqrestore(&vop->irq_lock, flags); } -static void vop_crtc_wait_for_update(struct drm_crtc *crtc) -{ - struct vop *vop = to_vop(crtc); - - reinit_completion(&vop->wait_update_complete); - WARN_ON(!wait_for_completion_timeout(&vop->wait_update_complete, 100)); -} - static const struct rockchip_crtc_funcs private_crtc_funcs = { .enable_vblank = vop_crtc_enable_vblank, .disable_vblank = vop_crtc_disable_vblank, - .wait_for_update = vop_crtc_wait_for_update, }; static bool vop_crtc_mode_fixup(struct drm_crtc *crtc, @@ -1251,9 +1241,6 @@ static void vop_handle_vblank(struct vop *vop) } spin_unlock_irqrestore(&drm->event_lock, flags); - if (!completion_done(&vop->wait_update_complete)) - complete(&vop->wait_update_complete); - if (test_and_clear_bit(VOP_PENDING_FB_UNREF, &vop->pending)) drm_flip_work_commit(&vop->fb_unref_work, system_unbound_wq); } @@ -1398,7 +1385,6 @@ static int vop_create_crtc(struct vop *vop) vop_fb_unref_worker); init_completion(&vop->dsp_hold_completion); - init_completion(&vop->wait_update_complete); init_completion(&vop->line_flag_completion); crtc->port = port; rockchip_register_crtc_funcs(crtc, &private_crtc_funcs); From 646ec68718067a3e6b029b72a9a2919178f0f201 Mon Sep 17 00:00:00 2001 From: Tomasz Figa Date: Wed, 14 Sep 2016 21:54:59 +0900 Subject: [PATCH 10/13] drm/rockchip: Do not enable vblank without event Originally we needed to enable vblank for any atomic commit to kick the PSR machine, but that was changed and we no longer need to do so from a vblank interrupt. Let's return to original behavior of enabling vblank only if it is really necessary. This essentially reverts commit 5b6804034ae9 ("drm/rockchip: Enable vblank without event"). Signed-off-by: Tomasz Figa --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index b6d47dda691a..d3237f84cbdb 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -116,7 +116,6 @@ struct vop { struct device *dev; struct drm_device *drm_dev; bool is_enabled; - bool vblank_active; /* mutex vsync_ work */ struct mutex vsync_mutex; @@ -1131,11 +1130,10 @@ static void vop_crtc_atomic_begin(struct drm_crtc *crtc, rockchip_drm_psr_flush(crtc); spin_lock_irq(&crtc->dev->event_lock); - vop->vblank_active = true; - WARN_ON(drm_crtc_vblank_get(crtc) != 0); - WARN_ON(vop->event); - if (crtc->state->event) { + WARN_ON(drm_crtc_vblank_get(crtc) != 0); + WARN_ON(vop->event); + vop->event = crtc->state->event; crtc->state->event = NULL; } @@ -1232,12 +1230,8 @@ static void vop_handle_vblank(struct vop *vop) spin_lock_irqsave(&drm->event_lock, flags); if (vop->event) { drm_crtc_send_vblank_event(crtc, vop->event); - vop->event = NULL; - - } - if (vop->vblank_active) { - vop->vblank_active = false; drm_crtc_vblank_put(crtc); + vop->event = NULL; } spin_unlock_irqrestore(&drm->event_lock, flags); @@ -1514,7 +1508,6 @@ static int vop_initial(struct vop *vop) clk_disable(vop->aclk); vop->is_enabled = false; - vop->vblank_active = false; return 0; From 41ee436700f41260663d0b201585551745b623d0 Mon Sep 17 00:00:00 2001 From: Tomasz Figa Date: Wed, 14 Sep 2016 21:55:00 +0900 Subject: [PATCH 11/13] drm/rockchip: Always signal event in next vblank after cfg_done This patch makes the driver send the pending vblank event in next vblank following the commit, relying on vblank signalling improvements done in previous patches. This gives us vblank events that always represent the real moment of changes hitting on the screen (which was the case only for complete FB changes before) and lets us remove the manual window update check. Signed-off-by: Tomasz Figa --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 54 ++++----------------- 1 file changed, 10 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index d3237f84cbdb..1d8c942b5ce9 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -105,10 +105,6 @@ struct vop_win { struct drm_plane base; const struct vop_win_data *data; struct vop *vop; - - /* protected by dev->event_lock */ - bool enable; - dma_addr_t yrgb_mst; }; struct vop { @@ -712,11 +708,6 @@ static void vop_plane_atomic_disable(struct drm_plane *plane, if (!old_state->crtc) return; - spin_lock_irq(&plane->dev->event_lock); - vop_win->enable = false; - vop_win->yrgb_mst = 0; - spin_unlock_irq(&plane->dev->event_lock); - spin_lock(&vop->reg_lock); VOP_WIN_SET(vop, win, enable, 0); @@ -780,11 +771,6 @@ static void vop_plane_atomic_update(struct drm_plane *plane, offset += (src->y1 >> 16) * fb->pitches[0]; vop_plane_state->yrgb_mst = rk_obj->dma_addr + offset + fb->offsets[0]; - spin_lock_irq(&plane->dev->event_lock); - vop_win->enable = true; - vop_win->yrgb_mst = vop_plane_state->yrgb_mst; - spin_unlock_irq(&plane->dev->event_lock); - spin_lock(&vop->reg_lock); VOP_WIN_SET(vop, win, format, vop_plane_state->format); @@ -1108,6 +1094,16 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc, */ vop_wait_for_irq_handler(vop); + spin_lock_irq(&crtc->dev->event_lock); + if (crtc->state->event) { + WARN_ON(drm_crtc_vblank_get(crtc) != 0); + WARN_ON(vop->event); + + vop->event = crtc->state->event; + crtc->state->event = NULL; + } + spin_unlock_irq(&crtc->dev->event_lock); + for_each_plane_in_state(old_state, plane, old_plane_state, i) { if (!old_plane_state->fb) continue; @@ -1125,19 +1121,7 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc, static void vop_crtc_atomic_begin(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state) { - struct vop *vop = to_vop(crtc); - rockchip_drm_psr_flush(crtc); - - spin_lock_irq(&crtc->dev->event_lock); - if (crtc->state->event) { - WARN_ON(drm_crtc_vblank_get(crtc) != 0); - WARN_ON(vop->event); - - vop->event = crtc->state->event; - crtc->state->event = NULL; - } - spin_unlock_irq(&crtc->dev->event_lock); } static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = { @@ -1203,29 +1187,11 @@ static void vop_fb_unref_worker(struct drm_flip_work *work, void *val) drm_framebuffer_unreference(fb); } -static bool vop_win_pending_is_complete(struct vop_win *vop_win) -{ - dma_addr_t yrgb_mst; - - if (!vop_win->enable) - return VOP_WIN_GET(vop_win->vop, vop_win->data, enable) == 0; - - yrgb_mst = VOP_WIN_GET_YRGBADDR(vop_win->vop, vop_win->data); - - return yrgb_mst == vop_win->yrgb_mst; -} - static void vop_handle_vblank(struct vop *vop) { struct drm_device *drm = vop->drm_dev; struct drm_crtc *crtc = &vop->crtc; unsigned long flags; - int i; - - for (i = 0; i < vop->data->win_size; i++) { - if (!vop_win_pending_is_complete(&vop->win[i])) - return; - } spin_lock_irqsave(&drm->event_lock, flags); if (vop->event) { From d47a7246bb5f2a36f7c26672966e9febeb78bc82 Mon Sep 17 00:00:00 2001 From: Tomasz Figa Date: Wed, 14 Sep 2016 21:55:01 +0900 Subject: [PATCH 12/13] drm/rockchip: Kill vop_plane_state After changes introduced by last patches, there is no useful data stored in vop_plane_state struct. Let's remove it and make the driver use generic plane state alone. Signed-off-by: Tomasz Figa --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 94 ++++----------------- 1 file changed, 15 insertions(+), 79 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 1d8c942b5ce9..37e6ba92b8fa 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -88,19 +88,11 @@ #define to_vop(x) container_of(x, struct vop, crtc) #define to_vop_win(x) container_of(x, struct vop_win, base) -#define to_vop_plane_state(x) container_of(x, struct vop_plane_state, base) enum vop_pending { VOP_PENDING_FB_UNREF, }; -struct vop_plane_state { - struct drm_plane_state base; - int format; - dma_addr_t yrgb_mst; - bool enable; -}; - struct vop_win { struct drm_plane base; const struct vop_win_data *data; @@ -647,7 +639,6 @@ static int vop_plane_atomic_check(struct drm_plane *plane, struct drm_crtc_state *crtc_state; struct drm_framebuffer *fb = state->fb; struct vop_win *vop_win = to_vop_win(plane); - struct vop_plane_state *vop_plane_state = to_vop_plane_state(state); const struct vop_win_data *win = vop_win->data; int ret; struct drm_rect clip; @@ -657,7 +648,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane, DRM_PLANE_HELPER_NO_SCALING; if (!crtc || !fb) - goto out_disable; + return 0; crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc); if (WARN_ON(!crtc_state)) @@ -675,11 +666,11 @@ static int vop_plane_atomic_check(struct drm_plane *plane, return ret; if (!state->visible) - goto out_disable; + return 0; - vop_plane_state->format = vop_convert_format(fb->pixel_format); - if (vop_plane_state->format < 0) - return vop_plane_state->format; + ret = vop_convert_format(fb->pixel_format); + if (ret < 0) + return ret; /* * Src.x1 can be odd when do clip, but yuv plane start point @@ -688,19 +679,12 @@ static int vop_plane_atomic_check(struct drm_plane *plane, if (is_yuv_support(fb->pixel_format) && ((state->src.x1 >> 16) % 2)) return -EINVAL; - vop_plane_state->enable = true; - - return 0; - -out_disable: - vop_plane_state->enable = false; return 0; } static void vop_plane_atomic_disable(struct drm_plane *plane, struct drm_plane_state *old_state) { - struct vop_plane_state *vop_plane_state = to_vop_plane_state(old_state); struct vop_win *vop_win = to_vop_win(plane); const struct vop_win_data *win = vop_win->data; struct vop *vop = to_vop(old_state->crtc); @@ -713,8 +697,6 @@ static void vop_plane_atomic_disable(struct drm_plane *plane, VOP_WIN_SET(vop, win, enable, 0); spin_unlock(&vop->reg_lock); - - vop_plane_state->enable = false; } static void vop_plane_atomic_update(struct drm_plane *plane, @@ -723,7 +705,6 @@ static void vop_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *state = plane->state; struct drm_crtc *crtc = state->crtc; struct vop_win *vop_win = to_vop_win(plane); - struct vop_plane_state *vop_plane_state = to_vop_plane_state(state); const struct vop_win_data *win = vop_win->data; struct vop *vop = to_vop(state->crtc); struct drm_framebuffer *fb = state->fb; @@ -738,6 +719,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane, dma_addr_t dma_addr; uint32_t val; bool rb_swap; + int format; /* * can't update plane when vop is disabled. @@ -748,7 +730,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane, if (WARN_ON(!vop->is_enabled)) return; - if (!vop_plane_state->enable) { + if (!state->visible) { vop_plane_atomic_disable(plane, old_state); return; } @@ -769,13 +751,15 @@ static void vop_plane_atomic_update(struct drm_plane *plane, offset = (src->x1 >> 16) * drm_format_plane_cpp(fb->pixel_format, 0); offset += (src->y1 >> 16) * fb->pitches[0]; - vop_plane_state->yrgb_mst = rk_obj->dma_addr + offset + fb->offsets[0]; + dma_addr = rk_obj->dma_addr + offset + fb->offsets[0]; + + format = vop_convert_format(fb->pixel_format); spin_lock(&vop->reg_lock); - VOP_WIN_SET(vop, win, format, vop_plane_state->format); + VOP_WIN_SET(vop, win, format, format); VOP_WIN_SET(vop, win, yrgb_vir, fb->pitches[0] >> 2); - VOP_WIN_SET(vop, win, yrgb_mst, vop_plane_state->yrgb_mst); + VOP_WIN_SET(vop, win, yrgb_mst, dma_addr); if (is_yuv_support(fb->pixel_format)) { int hsub = drm_format_horz_chroma_subsampling(fb->pixel_format); int vsub = drm_format_vert_chroma_subsampling(fb->pixel_format); @@ -827,61 +811,13 @@ static const struct drm_plane_helper_funcs plane_helper_funcs = { .atomic_disable = vop_plane_atomic_disable, }; -static void vop_atomic_plane_reset(struct drm_plane *plane) -{ - struct vop_plane_state *vop_plane_state = - to_vop_plane_state(plane->state); - - if (plane->state && plane->state->fb) - drm_framebuffer_unreference(plane->state->fb); - - kfree(vop_plane_state); - vop_plane_state = kzalloc(sizeof(*vop_plane_state), GFP_KERNEL); - if (!vop_plane_state) - return; - - plane->state = &vop_plane_state->base; - plane->state->plane = plane; -} - -static struct drm_plane_state * -vop_atomic_plane_duplicate_state(struct drm_plane *plane) -{ - struct vop_plane_state *old_vop_plane_state; - struct vop_plane_state *vop_plane_state; - - if (WARN_ON(!plane->state)) - return NULL; - - old_vop_plane_state = to_vop_plane_state(plane->state); - vop_plane_state = kmemdup(old_vop_plane_state, - sizeof(*vop_plane_state), GFP_KERNEL); - if (!vop_plane_state) - return NULL; - - __drm_atomic_helper_plane_duplicate_state(plane, - &vop_plane_state->base); - - return &vop_plane_state->base; -} - -static void vop_atomic_plane_destroy_state(struct drm_plane *plane, - struct drm_plane_state *state) -{ - struct vop_plane_state *vop_state = to_vop_plane_state(state); - - __drm_atomic_helper_plane_destroy_state(state); - - kfree(vop_state); -} - static const struct drm_plane_funcs vop_plane_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane, .destroy = vop_plane_destroy, - .reset = vop_atomic_plane_reset, - .atomic_duplicate_state = vop_atomic_plane_duplicate_state, - .atomic_destroy_state = vop_atomic_plane_destroy_state, + .reset = drm_atomic_helper_plane_reset, + .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state, }; static int vop_crtc_enable_vblank(struct drm_crtc *crtc) From 8c763c9b1072c91aac90e5bc0a2ac2220a6980d7 Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Fri, 16 Sep 2016 14:22:03 -0400 Subject: [PATCH 13/13] drm/rockchip: Balance irq refcount on failure If create_crtc fails in vop bind, ensure the irq refcount is zeroed back out before exiting. Reviewed-by: Daniel Kurtz Signed-off-by: Sean Paul --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 37e6ba92b8fa..c7eba305c488 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -1547,11 +1547,15 @@ static int vop_bind(struct device *dev, struct device *master, void *data) ret = vop_create_crtc(vop); if (ret) - return ret; + goto err_enable_irq; pm_runtime_enable(&pdev->dev); return 0; + +err_enable_irq: + enable_irq(vop->irq); /* To balance out the disable_irq above */ + return ret; } static void vop_unbind(struct device *dev, struct device *master, void *data)