linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] Add PSR function support for Analogix/Rockchip DP
@ 2016-07-14  4:15 Yakir Yang
  2016-07-14  4:15 ` [PATCH v4 1/4] drm/rockchip: vop: export line flag function Yakir Yang
                   ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Yakir Yang @ 2016-07-14  4:15 UTC (permalink / raw)
  To: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner
  Cc: Javier Martinez Canillas, Stéphane Marchesin, Sean Paul,
	Tomasz Figa, David Airlie, daniel.vetter, Thierry Reding,
	dianders, Krzysztof Kozlowski, emil.l.velikov, Dan Carpenter,
	Yakir Yang, linux-kernel, dri-devel, linux-samsung-soc,
	linux-rockchip


The full name of PSR is Panel Self Refresh, panel device could refresh
itself with the hardware framebuffer in panel, this would make a lots
of sense to save the power consumption.

This v3 version have splited an common PSR driver for Rockchip, which is
biggest changes from v2.

This thread is based on Mark's RK3399 VOP thread[0] and my RK3399 eDP
thread[1].

[0]: https://patchwork.kernel.org/patch/8886041/
[1]: https://patchwork.kernel.org/patch/9204497/


Changes in v4:
- Avoid the weird behavior in rockchip_drm_wait_line_flag(). (Sean)
- Make line_flag_num_x to an array. (Sean)
- Remove the unused vop_cfg_done() in vop_line_flag_irq_enable(). (Stephane, reviewed in Google gerrit)
    [https://chromium-review.googlesource.com/#/c/349084/33/drivers/gpu/drm/rockchip/rockchip_drm_vop.c@466]
- Tuck the global "psr_list" & "psr_list_mutex" in struct rockchip_drm_private. (Sean)
- Move the access of "psr->state" under "psr->state_mutex"'s protect. (Sean)
- Let "psr->state = PSR_FLUSH" under "psr->state_mutex"'s protect. (Sean)
- Collect psr_enable() and psr_disable() into psr_set_state()
- s/5\ second/PSR_FLUSH_TIMEOUT/ (Sean)
- Flush the psr callback in vop_crtc_disable(). (Stéphane, reviewed in Google gerrit)
    [https://chromium-review.googlesource.com/#/c/349084/6/drivers/gpu/drm/rockchip/rockchip_drm_vop.c@475]
- Add the missing file head with license. (Stéphane, reviewed in Google gerrit)
    [https://chromium-review.googlesource.com/#/c/357563/1/drivers/gpu/drm/rockchip/rockchip_drm_psr.h@3]
- Downgrade the PSR version print message to debug level. (Sean)
- Return 'void' instead of 'int' in analogix_dp_enable_sink_psr(). (Sean)
- Delete the unused read dpcd operations in analogix_dp_enable_sink_psr(). (Sean)
- Delete the arbitrary usleep_range in analogix_dp_enable_psr_crc. (Sean).
- Clean up the hardcoded values in analogix_dp_send_psr_spd(). (Sean)
- Rename "active/inactive" to "enable/disable". (Sean, Dominik)
- Keep set the PSR_VID_CRC_FLUSH gate in analogix_dp_enable_psr_crc().
- Return 'void' instead of 'int' in analogix_dp_psr_set(). (Sean)
- Pull the 10ms delay time out into a #define. (Sean)
- Improved the code of analogix_dp_psr_work(). (Sean)
- Indented with spaces for new numbers in rockchip_dp_device struct. (Stéphane, reviewed at Google gerrit)
    [https://chromium-review.googlesource.com/#/c/349085/33/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c@83]

Changes in v3:
- Export the 'rockchip_drm_wait_line_flag' symbol, and document it.
- Add 'line_flag_num_0' for RK3288/RK3036
- Remove the notify for waiting line_flag event (Daniel)
- split the psr flow into an common abstracted PSR driver
- implement the 'fb->dirty' callback function (Daniel)
- avoid to use notify to acqiure for vact event (Daniel)
- remove psr_active() callback which introduce in v2
- split analogix_dp_enable_psr(), make it more clearly
    analogix_dp_detect_sink_psr()
    analogix_dp_enable_sink_psr()
- remove some nosie register setting comments
- split the common psr logic into a seperate driver, make this to a
  simple sub-psr device driver.

Changes in v2:
- Introduce in v2, split VOP line flag changes out
- introduce in v2, splite the common Analogix DP changes out
- remove vblank notify out (Daniel)
- create a psr_active() callback in vop data struct.

Yakir Yang (4):
  drm/rockchip: vop: export line flag function
  drm/rockchip: add an common abstracted PSR driver
  drm/bridge: analogix_dp: add the PSR function support
  drm/rockchip: analogix_dp: implement PSR function

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  60 ++++++
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |   4 +
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  |  49 +++++
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h  |  28 +++
 drivers/gpu/drm/rockchip/Makefile                  |   2 +-
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c    |  57 ++++++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c        |   4 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h        |   6 +
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c         |  12 ++
 drivers/gpu/drm/rockchip/rockchip_drm_psr.c        | 223 +++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_psr.h        |  26 +++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c        | 147 ++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h        |   2 +
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c        |   4 +
 include/drm/bridge/analogix_dp.h                   |   3 +
 15 files changed, 626 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.h

-- 
1.9.1

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 1/4] drm/rockchip: vop: export line flag function
  2016-07-14  4:15 [PATCH v4 0/4] Add PSR function support for Analogix/Rockchip DP Yakir Yang
@ 2016-07-14  4:15 ` Yakir Yang
  2016-07-14 14:46   ` Sean Paul
  2016-07-14  4:15 ` [PATCH v4 2/4] drm/rockchip: add an common abstracted PSR driver Yakir Yang
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Yakir Yang @ 2016-07-14  4:15 UTC (permalink / raw)
  To: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner
  Cc: Javier Martinez Canillas, Stéphane Marchesin, Sean Paul,
	Tomasz Figa, David Airlie, daniel.vetter, Thierry Reding,
	dianders, Krzysztof Kozlowski, emil.l.velikov, Dan Carpenter,
	Yakir Yang, linux-kernel, dri-devel, linux-samsung-soc,
	linux-rockchip

VOP have integrated a hardware counter which indicate the exact display
line that vop is scanning. And if we're interested in a specific line,
we can set the line number to vop line_flag register, and then vop would
generate a line_flag interrupt for it.

For example eDP PSR function is interested in the vertical blanking
period, then driver could set the line number to zero.

This patch have exported a symbol that allow other driver to listen the
line flag event with given timeout limit:
-  rockchip_drm_wait_line_flag()

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v4:
- Avoid the weird behavior in rockchip_drm_wait_line_flag(). (Sean)
- Make line_flag_num_x to an array. (Sean)
- Remove the unused vop_cfg_done() in vop_line_flag_irq_enable(). (Stephane, reviewed in Google gerrit)
    [https://chromium-review.googlesource.com/#/c/349084/33/drivers/gpu/drm/rockchip/rockchip_drm_vop.c@466]

Changes in v3:
- Export the 'rockchip_drm_wait_line_flag' symbol, and document it.
- Add 'line_flag_num_0' for RK3288/RK3036
- Remove the notify for waiting line_flag event (Daniel)

Changes in v2:
- Introduce in v2, split VOP line flag changes out

 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   3 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 118 ++++++++++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h |   2 +
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   4 +
 4 files changed, 127 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index ea39329..239b830 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -70,4 +70,7 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
 				   struct device *dev);
 void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 				    struct device *dev);
+int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
+				unsigned int mstimeout);
+
 #endif /* _ROCKCHIP_DRM_DRV_H_ */
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index c8a62a8..69d32f1 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -121,6 +121,8 @@ struct vop {
 	/* protected by dev->event_lock */
 	struct drm_pending_vblank_event *event;
 
+	struct completion line_flag_completion;
+
 	const struct vop_data *data;
 
 	uint32_t *regsbak;
@@ -431,6 +433,71 @@ static void vop_dsp_hold_valid_irq_disable(struct vop *vop)
 	spin_unlock_irqrestore(&vop->irq_lock, flags);
 }
 
+/*
+ * (1) each frame starts at the start of the Vsync pulse which is signaled by
+ *     the "FRAME_SYNC" interrupt.
+ * (2) the active data region of each frame ends at dsp_vact_end
+ * (3) we should program this same number (dsp_vact_end) into dsp_line_frag_num,
+ *      to get "LINE_FLAG" interrupt at the end of the active on screen data.
+ *
+ * VOP_INTR_CTRL0.dsp_line_frag_num = VOP_DSP_VACT_ST_END.dsp_vact_end
+ * Interrupts
+ * LINE_FLAG -------------------------------+
+ * FRAME_SYNC ----+                         |
+ *                |                         |
+ *                v                         v
+ *                | Vsync | Vbp |  Vactive  | Vfp |
+ *                        ^     ^           ^     ^
+ *                        |     |           |     |
+ *                        |     |           |     |
+ * dsp_vs_end ------------+     |           |     |   VOP_DSP_VTOTAL_VS_END
+ * dsp_vact_start --------------+           |     |   VOP_DSP_VACT_ST_END
+ * dsp_vact_end ----------------------------+     |   VOP_DSP_VACT_ST_END
+ * dsp_total -------------------------------------+   VOP_DSP_VTOTAL_VS_END
+ */
+static bool vop_line_flag_irq_is_enabled(struct vop *vop)
+{
+	uint32_t line_flag_irq;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vop->irq_lock, flags);
+
+	line_flag_irq = VOP_INTR_GET_TYPE(vop, enable, LINE_FLAG_INTR);
+
+	spin_unlock_irqrestore(&vop->irq_lock, flags);
+
+	return !!line_flag_irq;
+}
+
+static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
+{
+	unsigned long flags;
+
+	if (WARN_ON(!vop->is_enabled))
+		return;
+
+	spin_lock_irqsave(&vop->irq_lock, flags);
+
+	VOP_CTRL_SET(vop, line_flag_num[0], line_num);
+	VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
+
+	spin_unlock_irqrestore(&vop->irq_lock, flags);
+}
+
+static void vop_line_flag_irq_disable(struct vop *vop)
+{
+	unsigned long flags;
+
+	if (WARN_ON(!vop->is_enabled))
+		return;
+
+	spin_lock_irqsave(&vop->irq_lock, flags);
+
+	VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 0);
+
+	spin_unlock_irqrestore(&vop->irq_lock, flags);
+}
+
 static void vop_enable(struct drm_crtc *crtc)
 {
 	struct vop *vop = to_vop(crtc);
@@ -1157,6 +1224,13 @@ static irqreturn_t vop_isr(int irq, void *data)
 		ret = IRQ_HANDLED;
 	}
 
+	if (active_irqs & LINE_FLAG_INTR) {
+		if (!completion_done(&vop->line_flag_completion))
+			complete(&vop->line_flag_completion);
+		active_irqs &= ~LINE_FLAG_INTR;
+		ret = IRQ_HANDLED;
+	}
+
 	if (active_irqs & FS_INTR) {
 		drm_crtc_handle_vblank(crtc);
 		vop_handle_vblank(vop);
@@ -1255,6 +1329,7 @@ static int vop_create_crtc(struct vop *vop)
 
 	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);
 
@@ -1411,6 +1486,49 @@ static void vop_win_init(struct vop *vop)
 	}
 }
 
+/**
+ * rockchip_drm_wait_line_flag - acqiure the give line flag event
+ * @crtc: CRTC to enable line flag
+ * @line_num: interested line number
+ * @mstimeout: millisecond for timeout
+ *
+ * Driver would hold here until the interested line flag interrupt have
+ * happened or timeout to wait.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
+				unsigned int mstimeout)
+{
+	struct vop *vop = to_vop(crtc);
+	unsigned long jiffies_left;
+
+	if (!crtc || !vop->is_enabled)
+		return -ENODEV;
+
+	if (line_num > crtc->mode.vtotal || mstimeout <= 0)
+		return -EINVAL;
+
+	if (vop_line_flag_irq_is_enabled(vop))
+		return -EBUSY;
+
+	reinit_completion(&vop->line_flag_completion);
+	vop_line_flag_irq_enable(vop, line_num);
+
+	jiffies_left = wait_for_completion_timeout(&vop->line_flag_completion,
+						   msecs_to_jiffies(mstimeout));
+	vop_line_flag_irq_disable(vop);
+
+	if (jiffies_left == 0) {
+		dev_err(vop->dev, "Timeout waiting for IRQ\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(rockchip_drm_wait_line_flag);
+
 static int vop_bind(struct device *dev, struct device *master, void *data)
 {
 	struct platform_device *pdev = to_platform_device(dev);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index ff4f52e..1dbc526 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -61,6 +61,8 @@ struct vop_ctrl {
 	struct vop_reg hpost_st_end;
 	struct vop_reg vpost_st_end;
 
+	struct vop_reg line_flag_num[2];
+
 	struct vop_reg cfg_done;
 };
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 6f42e56..eea8427 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -122,6 +122,7 @@ static const struct vop_ctrl rk3036_ctrl_data = {
 	.hact_st_end = VOP_REG(RK3036_DSP_HACT_ST_END, 0x1fff1fff, 0),
 	.vtotal_pw = VOP_REG(RK3036_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
 	.vact_st_end = VOP_REG(RK3036_DSP_VACT_ST_END, 0x1fff1fff, 0),
+	.line_flag_num[0] = VOP_REG(RK3036_INT_STATUS, 0xfff, 12),
 	.cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
 };
 
@@ -221,6 +222,7 @@ static const struct vop_ctrl rk3288_ctrl_data = {
 	.vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
 	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
 	.vpost_st_end = VOP_REG(RK3288_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
+	.line_flag_num[0] = VOP_REG(RK3288_INTR_CTRL0, 0x1fff, 12),
 	.cfg_done = VOP_REG(RK3288_REG_CFG_DONE, 0x1, 0),
 };
 
@@ -299,6 +301,8 @@ static const struct vop_ctrl rk3399_ctrl_data = {
 	.vact_st_end = VOP_REG(RK3399_DSP_VACT_ST_END, 0x1fff1fff, 0),
 	.hpost_st_end = VOP_REG(RK3399_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
 	.vpost_st_end = VOP_REG(RK3399_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
+	.line_flag_num[0] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 0),
+	.line_flag_num[1] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 16),
 	.cfg_done = VOP_REG_MASK(RK3399_REG_CFG_DONE, 0x1, 0),
 };
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 2/4] drm/rockchip: add an common abstracted PSR driver
  2016-07-14  4:15 [PATCH v4 0/4] Add PSR function support for Analogix/Rockchip DP Yakir Yang
  2016-07-14  4:15 ` [PATCH v4 1/4] drm/rockchip: vop: export line flag function Yakir Yang
@ 2016-07-14  4:15 ` Yakir Yang
  2016-07-14 15:14   ` Sean Paul
  2016-07-23  4:04   ` Doug Anderson
  2016-07-14  4:15 ` [PATCH v4 3/4] drm/bridge: analogix_dp: add the PSR function support Yakir Yang
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 21+ messages in thread
From: Yakir Yang @ 2016-07-14  4:15 UTC (permalink / raw)
  To: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner
  Cc: Javier Martinez Canillas, Stéphane Marchesin, Sean Paul,
	Tomasz Figa, David Airlie, daniel.vetter, Thierry Reding,
	dianders, Krzysztof Kozlowski, emil.l.velikov, Dan Carpenter,
	Yakir Yang, linux-kernel, dri-devel, linux-samsung-soc,
	linux-rockchip

The PSR driver have exported four symbols for specific device driver:
- rockchip_drm_psr_register()
- rockchip_drm_psr_unregister()
- rockchip_drm_psr_enable()
- rockchip_drm_psr_disable()
- rockchip_drm_psr_flush()

Encoder driver should call the register/unregister interfaces to hook
itself into common PSR driver, encoder have implement the 'psr_set'
callback which use the set PSR state in hardware side.

Crtc driver would call the enable/disable interfaces when vblank is
enable/disable, after that the common PSR driver would call the encoder
registered callback to set the PSR state.

Fb driver would call the flush interface in 'fb->dirty' callback, this
helper function would force all PSR enabled encoders to exit from PSR
for 3 seconds.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v4:
- Tuck the global "psr_list" & "psr_list_mutex" in struct rockchip_drm_private. (Sean)
- Move the access of "psr->state" under "psr->state_mutex"'s protect. (Sean)
- Let "psr->state = PSR_FLUSH" under "psr->state_mutex"'s protect. (Sean)
- Collect psr_enable() and psr_disable() into psr_set_state()
- s/5\ second/PSR_FLUSH_TIMEOUT/ (Sean)
- Flush the psr callback in vop_crtc_disable(). (Stéphane, reviewed in Google gerrit)
    [https://chromium-review.googlesource.com/#/c/349084/6/drivers/gpu/drm/rockchip/rockchip_drm_vop.c@475]
- Add the missing file head with license. (Stéphane, reviewed in Google gerrit)
    [https://chromium-review.googlesource.com/#/c/357563/1/drivers/gpu/drm/rockchip/rockchip_drm_psr.h@3]

Changes in v3:
- split the psr flow into an common abstracted PSR driver
- implement the 'fb->dirty' callback function (Daniel)
- avoid to use notify to acqiure for vact event (Daniel)
- remove psr_active() callback which introduce in v2

Changes in v2: None

 drivers/gpu/drm/rockchip/Makefile           |   2 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c |   4 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   3 +
 drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  12 ++
 drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 223 ++++++++++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_psr.h |  26 ++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  29 ++++
 7 files changed, 298 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.h

diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
index 05d0713..9746365 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -3,7 +3,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_vop.o
+		rockchip_drm_gem.o rockchip_drm_psr.o rockchip_drm_vop.o
 rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
 
 obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index d665fb0..26c12b3 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -156,6 +156,9 @@ static int rockchip_drm_bind(struct device *dev)
 
 	drm_dev->dev_private = private;
 
+	INIT_LIST_HEAD(&private->psr_list);
+	mutex_init(&private->psr_list_mutex);
+
 	drm_mode_config_init(drm_dev);
 
 	rockchip_drm_mode_config_init(drm_dev);
@@ -218,6 +221,7 @@ static int rockchip_drm_bind(struct device *dev)
 
 	if (is_support_iommu)
 		arm_iommu_release_mapping(mapping);
+
 	return 0;
 err_fbdev_fini:
 	rockchip_drm_fbdev_fini(drm_dev);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index 239b830..9c34c9e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -61,6 +61,9 @@ struct rockchip_drm_private {
 	struct drm_gem_object *fbdev_bo;
 	const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
 	struct drm_atomic_state *state;
+
+	struct list_head psr_list;
+	struct mutex psr_list_mutex;
 };
 
 int rockchip_register_crtc_funcs(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index 20f12bc..36afd9c 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -21,6 +21,7 @@
 
 #include "rockchip_drm_drv.h"
 #include "rockchip_drm_gem.h"
+#include "rockchip_drm_psr.h"
 
 #define to_rockchip_fb(x) container_of(x, struct rockchip_drm_fb, fb)
 
@@ -66,9 +67,20 @@ static int rockchip_drm_fb_create_handle(struct drm_framebuffer *fb,
 				     rockchip_fb->obj[0], handle);
 }
 
+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(fb->dev);
+	return 0;
+}
+
 static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
 	.destroy	= rockchip_drm_fb_destroy,
 	.create_handle	= rockchip_drm_fb_create_handle,
+	.dirty		= rockchip_drm_fb_dirty,
 };
 
 static struct rockchip_drm_fb *
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
new file mode 100644
index 0000000..e79ea18
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
@@ -0,0 +1,223 @@
+/*
+ * 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_crtc_helper.h>
+
+#include "rockchip_drm_drv.h"
+#include "rockchip_drm_psr.h"
+
+#define PSR_FLUSH_TIMEOUT	msecs_to_jiffies(3000) /* 3 seconds */
+
+static LIST_HEAD(psr_list);
+static DEFINE_MUTEX(psr_list_mutex);
+
+enum psr_state {
+	PSR_FLUSH,
+	PSR_ENABLE,
+	PSR_DISABLE,
+};
+
+struct psr_drv {
+	struct list_head list;
+	enum psr_state state;
+	struct mutex state_mutex;
+
+	struct timer_list flush_timer;
+
+	struct drm_encoder *encoder;
+	void (*set)(struct drm_encoder *encoder, bool enable);
+};
+
+static struct psr_drv *find_psr_by_crtc(struct drm_crtc *crtc)
+{
+	struct rockchip_drm_private *drm_drv = crtc->dev->dev_private;
+	struct psr_drv *psr;
+
+	mutex_lock(&drm_drv->psr_list_mutex);
+	list_for_each_entry(psr, &drm_drv->psr_list, list) {
+		if (psr->encoder->crtc == crtc) {
+			mutex_unlock(&drm_drv->psr_list_mutex);
+			return psr;
+		}
+	}
+	mutex_unlock(&drm_drv->psr_list_mutex);
+
+	return ERR_PTR(-ENODEV);
+}
+
+static void psr_set_state(struct psr_drv *psr, enum psr_state state)
+{
+	mutex_lock(&psr->state_mutex);
+
+	if (psr->state == state) {
+		mutex_unlock(&psr->state_mutex);
+		return;
+	}
+
+	psr->state = state;
+	switch (state) {
+	case PSR_ENABLE:
+		psr->set(psr->encoder, true);
+		break;
+
+	case PSR_DISABLE:
+	case PSR_FLUSH:
+		psr->set(psr->encoder, false);
+		break;
+	};
+
+	mutex_unlock(&psr->state_mutex);
+}
+
+static void psr_flush_handler(unsigned long data)
+{
+	struct psr_drv *psr = (struct psr_drv *)data;
+
+	if (!psr || psr->state != PSR_FLUSH)
+		return;
+
+	psr_set_state(psr, PSR_ENABLE);
+}
+
+/**
+ * rockchip_drm_psr_enable - enable the encoder PSR which bind to given CRTC
+ * @crtc: CRTC to obtain the PSR encoder
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int rockchip_drm_psr_enable(struct drm_crtc *crtc)
+{
+	struct psr_drv *psr = find_psr_by_crtc(crtc);
+
+	if (IS_ERR(psr))
+		return PTR_ERR(psr);
+
+	psr_set_state(psr, PSR_ENABLE);
+	return 0;
+}
+EXPORT_SYMBOL(rockchip_drm_psr_enable);
+
+/**
+ * rockchip_drm_psr_disable - disable the encoder PSR which bind to given CRTC
+ * @crtc: CRTC to obtain the PSR encoder
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int rockchip_drm_psr_disable(struct drm_crtc *crtc)
+{
+	struct psr_drv *psr = find_psr_by_crtc(crtc);
+
+	if (IS_ERR(psr))
+		return PTR_ERR(psr);
+
+	psr_set_state(psr, PSR_DISABLE);
+	return 0;
+}
+EXPORT_SYMBOL(rockchip_drm_psr_disable);
+
+/**
+ * rockchip_drm_psr_flush - 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(struct drm_device *dev)
+{
+	struct rockchip_drm_private *drm_drv = dev->dev_private;
+	struct psr_drv *psr;
+
+	mutex_lock(&drm_drv->psr_list_mutex);
+	list_for_each_entry(psr, &drm_drv->psr_list, list) {
+		if (psr->state == PSR_DISABLE)
+			continue;
+
+		mod_timer(&psr->flush_timer,
+			  round_jiffies_up(jiffies + PSR_FLUSH_TIMEOUT));
+
+		psr_set_state(psr, PSR_FLUSH);
+	}
+	mutex_unlock(&drm_drv->psr_list_mutex);
+}
+EXPORT_SYMBOL(rockchip_drm_psr_flush);
+
+/**
+ * rockchip_drm_psr_register - register encoder to psr driver
+ * @encoder: encoder that obtain the PSR function
+ * @psr_set: call back to set PSR state
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int rockchip_drm_psr_register(struct drm_encoder *encoder,
+			void (*psr_set)(struct drm_encoder *, bool enable))
+{
+	struct rockchip_drm_private *drm_drv = encoder->dev->dev_private;
+	struct psr_drv *psr;
+
+	if (!encoder || !psr_set)
+		return -EINVAL;
+
+	psr = kzalloc(sizeof(struct psr_drv), GFP_KERNEL);
+	if (!psr)
+		return -ENOMEM;
+
+	setup_timer(&psr->flush_timer, psr_flush_handler, (unsigned long)psr);
+
+	mutex_init(&psr->state_mutex);
+
+	psr->state = PSR_DISABLE;
+	psr->encoder = encoder;
+	psr->set = psr_set;
+
+	mutex_lock(&drm_drv->psr_list_mutex);
+	list_add_tail(&psr->list, &drm_drv->psr_list);
+	mutex_unlock(&drm_drv->psr_list_mutex);
+
+	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
+ *
+ * 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;
+
+	mutex_lock(&drm_drv->psr_list_mutex);
+	list_for_each_entry(psr, &drm_drv->psr_list, list) {
+		if (psr->encoder == encoder) {
+			del_timer(&psr->flush_timer);
+			list_del(&psr->list);
+			kfree(psr);
+		}
+	}
+	mutex_unlock(&drm_drv->psr_list_mutex);
+}
+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
new file mode 100644
index 0000000..c35b688
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h
@@ -0,0 +1,26 @@
+/*
+ * 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(struct drm_device *dev);
+int rockchip_drm_psr_enable(struct drm_crtc *crtc);
+int rockchip_drm_psr_disable(struct drm_crtc *crtc);
+
+int rockchip_drm_psr_register(struct drm_encoder *encoder,
+			void (*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 69d32f1..e702fb3 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -34,6 +34,7 @@
 #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"
 
 #define __REG_SET_RELAXED(x, off, mask, shift, v, write_mask) \
@@ -87,6 +88,8 @@
 #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)
 
+#define VOP_PSR_SET_DELAY_TIME		msecs_to_jiffies(10)
+
 struct vop_plane_state {
 	struct drm_plane_state base;
 	int format;
@@ -121,6 +124,9 @@ struct vop {
 	/* protected by dev->event_lock */
 	struct drm_pending_vblank_event *event;
 
+	bool psr_enabled;
+	struct delayed_work psr_work;
+
 	struct completion line_flag_completion;
 
 	const struct vop_data *data;
@@ -629,6 +635,9 @@ static void vop_crtc_disable(struct drm_crtc *crtc)
 
 		crtc->state->event = NULL;
 	}
+
+	vop->psr_enabled = false;
+	INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
 }
 
 static void vop_plane_destroy(struct drm_plane *plane)
@@ -923,6 +932,16 @@ static const struct drm_plane_funcs vop_plane_funcs = {
 	.atomic_destroy_state = vop_atomic_plane_destroy_state,
 };
 
+static void vop_psr_work(struct work_struct *work)
+{
+	struct vop *vop = container_of(work, typeof(*vop), psr_work.work);
+
+	if (vop->psr_enabled)
+		rockchip_drm_psr_enable(&vop->crtc);
+	else
+		rockchip_drm_psr_disable(&vop->crtc);
+}
+
 static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
 {
 	struct vop *vop = to_vop(crtc);
@@ -937,6 +956,9 @@ static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
 
 	spin_unlock_irqrestore(&vop->irq_lock, flags);
 
+	vop->psr_enabled = false;
+	schedule_delayed_work(&vop->psr_work, VOP_PSR_SET_DELAY_TIME);
+
 	return 0;
 }
 
@@ -953,6 +975,9 @@ 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);
+
+	vop->psr_enabled = true;
+	schedule_delayed_work(&vop->psr_work, VOP_PSR_SET_DELAY_TIME);
 }
 
 static void vop_crtc_wait_for_update(struct drm_crtc *crtc)
@@ -1597,6 +1622,10 @@ static int vop_bind(struct device *dev, struct device *master, void *data)
 		return ret;
 
 	pm_runtime_enable(&pdev->dev);
+
+	vop->psr_enabled = false;
+	INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
+
 	return 0;
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 3/4] drm/bridge: analogix_dp: add the PSR function support
  2016-07-14  4:15 [PATCH v4 0/4] Add PSR function support for Analogix/Rockchip DP Yakir Yang
  2016-07-14  4:15 ` [PATCH v4 1/4] drm/rockchip: vop: export line flag function Yakir Yang
  2016-07-14  4:15 ` [PATCH v4 2/4] drm/rockchip: add an common abstracted PSR driver Yakir Yang
@ 2016-07-14  4:15 ` Yakir Yang
  2016-07-14 15:23   ` Sean Paul
  2016-07-14  4:15 ` [PATCH v4 4/4] drm/rockchip: analogix_dp: implement PSR function Yakir Yang
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Yakir Yang @ 2016-07-14  4:15 UTC (permalink / raw)
  To: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner
  Cc: Javier Martinez Canillas, Stéphane Marchesin, Sean Paul,
	Tomasz Figa, David Airlie, daniel.vetter, Thierry Reding,
	dianders, Krzysztof Kozlowski, emil.l.velikov, Dan Carpenter,
	Yakir Yang, linux-kernel, dri-devel, linux-samsung-soc,
	linux-rockchip

The full name of PSR is Panel Self Refresh, panel device could refresh
itself with the hardware framebuffer in panel, this would make lots of
sense to save the power consumption.

This patch have exported two symbols for platform driver to implement
the PSR function in hardware side:
- analogix_dp_active_psr()
- analogix_dp_inactive_psr()

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v4:
- Downgrade the PSR version print message to debug level. (Sean)
- Return 'void' instead of 'int' in analogix_dp_enable_sink_psr(). (Sean)
- Delete the unused read dpcd operations in analogix_dp_enable_sink_psr(). (Sean)
- Delete the arbitrary usleep_range in analogix_dp_enable_psr_crc. (Sean).
- Clean up the hardcoded values in analogix_dp_send_psr_spd(). (Sean)
- Rename "active/inactive" to "enable/disable". (Sean, Dominik)
- Keep set the PSR_VID_CRC_FLUSH gate in analogix_dp_enable_psr_crc().

Changes in v3:
- split analogix_dp_enable_psr(), make it more clearly
    analogix_dp_detect_sink_psr()
    analogix_dp_enable_sink_psr()
- remove some nosie register setting comments

Changes in v2:
- introduce in v2, splite the common Analogix DP changes out

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 60 ++++++++++++++++++++++
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  4 ++
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 49 ++++++++++++++++++
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h  | 28 ++++++++++
 include/drm/bridge/analogix_dp.h                   |  3 ++
 5 files changed, 144 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 32715da..1fec91a 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -97,6 +97,62 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
 	return 0;
 }
 
+int analogix_dp_enable_psr(struct device *dev)
+{
+	struct analogix_dp_device *dp = dev_get_drvdata(dev);
+
+	if (!dp->psr_support)
+		return -EINVAL;
+
+	analogix_dp_send_psr_spd(dp, EDP_VSC_PSR_STATE_ACTIVE |
+				 EDP_VSC_PSR_CRC_VALUES_VALID);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
+
+int analogix_dp_disable_psr(struct device *dev)
+{
+	struct analogix_dp_device *dp = dev_get_drvdata(dev);
+
+	if (!dp->psr_support)
+		return -EINVAL;
+
+	analogix_dp_send_psr_spd(dp, 0);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
+
+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);
+	dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version);
+
+	return (psr_version & DP_PSR_IS_SUPPORTED) ? true : false;
+}
+
+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);
+	psr_en &= ~DP_PSR_ENABLE;
+	analogix_dp_write_byte_to_dpcd(dp, 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);
+
+	/* 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);
+
+	analogix_dp_enable_psr_crc(dp);
+}
+
 static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
 {
 	int i;
@@ -921,6 +977,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
 
 	/* Enable video */
 	analogix_dp_start_video(dp);
+
+	dp->psr_support = analogix_dp_detect_sink_psr(dp);
+	if (dp->psr_support)
+		analogix_dp_enable_sink_psr(dp);
 }
 
 int analogix_dp_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index b456380..6ca5dde 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -177,6 +177,7 @@ struct analogix_dp_device {
 	int			hpd_gpio;
 	bool                    force_hpd;
 	unsigned char           edid[EDID_BLOCK_LENGTH * 2];
+	bool			psr_support;
 
 	struct analogix_dp_plat_data *plat_data;
 };
@@ -278,4 +279,7 @@ int analogix_dp_is_video_stream_on(struct analogix_dp_device *dp);
 void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
 void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
 void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
+void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
+void analogix_dp_send_psr_spd(struct analogix_dp_device *dp, int db1);
+
 #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 48030f0..26446ae 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -26,6 +26,9 @@
 #define COMMON_INT_MASK_4	(HOTPLUG_CHG | HPD_LOST | PLUG)
 #define INT_STA_MASK		INT_HPD
 
+static u32 psr_spd_hbx[] = { 0x00, 0x07, 0x02, 0x08 };
+static u32 psr_spd_pbx[] = { 0x00, 0x16, 0xCE, 0x5D };
+
 void analogix_dp_enable_video_mute(struct analogix_dp_device *dp, bool enable)
 {
 	u32 reg;
@@ -1322,3 +1325,49 @@ void analogix_dp_disable_scrambling(struct analogix_dp_device *dp)
 	reg |= SCRAMBLING_DISABLE;
 	writel(reg, dp->reg_base + ANALOGIX_DP_TRAINING_PTN_SET);
 }
+
+void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
+{
+	writel(PSR_VID_CRC_FLUSH | PSR_VID_CRC_ENABLE,
+	       dp->reg_base + ANALOGIX_DP_CRC_CON);
+}
+
+void analogix_dp_send_psr_spd(struct analogix_dp_device *dp, int db1)
+{
+	unsigned int val;
+	unsigned int i;
+
+	/* don't send info frame */
+	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+	val &= ~IF_EN;
+	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+
+	/* configure single frame update mode */
+	writel(PSR_FRAME_UP_TYPE_BURST | PSR_CRC_SEL_HARDWARE,
+	       dp->reg_base + ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL);
+
+	/* configure VSC HB0~HB3 and PB0~PB3 */
+	for (i = 0; i < 4; i++)
+		writel(psr_spd_hbx[i], dp->reg_base + ANALOGIX_DP_SPD_HB + i*4);
+	for (i = 0; i < 4; i++)
+		writel(psr_spd_pbx[i], dp->reg_base + ANALOGIX_DP_SPD_PB + i*4);
+
+	/* configure DB0 / DB1 values */
+	writel(0x0, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB0);
+	writel(db1, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB1);
+
+	/* set reuse spd inforframe */
+	val = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_3);
+	val |= REUSE_SPD_EN;
+	writel(val, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_3);
+
+	/* mark info frame update */
+	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+	val = (val | IF_UP) & ~IF_EN;
+	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+
+	/* send info frame */
+	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+	val |= IF_EN;
+	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+}
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
index cdcc6c5..fd232b2 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
@@ -22,6 +22,8 @@
 #define ANALOGIX_DP_VIDEO_CTL_8			0x3C
 #define ANALOGIX_DP_VIDEO_CTL_10		0x44
 
+#define ANALOGIX_DP_SPDIF_AUDIO_CTL_0		0xD8
+
 #define ANALOGIX_DP_PLL_REG_1			0xfc
 #define ANALOGIX_DP_PLL_REG_2			0x9e4
 #define ANALOGIX_DP_PLL_REG_3			0x9e8
@@ -30,6 +32,15 @@
 
 #define ANALOGIX_DP_PD				0x12c
 
+#define ANALOGIX_DP_IF_TYPE			0x244
+#define ANALOGIX_DP_IF_PKT_DB1			0x254
+#define ANALOGIX_DP_IF_PKT_DB2			0x258
+#define ANALOGIX_DP_SPD_HB			0x2F8
+#define ANALOGIX_DP_SPD_PB			0x308
+#define ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL	0x318
+#define ANALOGIX_DP_VSC_SHADOW_DB0		0x31C
+#define ANALOGIX_DP_VSC_SHADOW_DB1		0x320
+
 #define ANALOGIX_DP_LANE_MAP			0x35C
 
 #define ANALOGIX_DP_ANALOG_CTL_1		0x370
@@ -103,6 +114,8 @@
 
 #define ANALOGIX_DP_SOC_GENERAL_CTL		0x800
 
+#define ANALOGIX_DP_CRC_CON			0x890
+
 /* ANALOGIX_DP_TX_SW_RESET */
 #define RESET_DP_TX				(0x1 << 0)
 
@@ -151,6 +164,7 @@
 #define VID_CHK_UPDATE_TYPE_SHIFT		(4)
 #define VID_CHK_UPDATE_TYPE_1			(0x1 << 4)
 #define VID_CHK_UPDATE_TYPE_0			(0x0 << 4)
+#define REUSE_SPD_EN				(0x1 << 3)
 
 /* ANALOGIX_DP_VIDEO_CTL_8 */
 #define VID_HRES_TH(x)				(((x) & 0xf) << 4)
@@ -167,6 +181,12 @@
 #define REF_CLK_27M				(0x0 << 0)
 #define REF_CLK_MASK				(0x1 << 0)
 
+/* ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL */
+#define PSR_FRAME_UP_TYPE_BURST			(0x1 << 0)
+#define PSR_FRAME_UP_TYPE_SINGLE		(0x0 << 0)
+#define PSR_CRC_SEL_HARDWARE			(0x1 << 1)
+#define PSR_CRC_SEL_MANUALLY			(0x0 << 1)
+
 /* ANALOGIX_DP_LANE_MAP */
 #define LANE3_MAP_LOGIC_LANE_0			(0x0 << 6)
 #define LANE3_MAP_LOGIC_LANE_1			(0x1 << 6)
@@ -376,4 +396,12 @@
 #define VIDEO_MODE_SLAVE_MODE			(0x1 << 0)
 #define VIDEO_MODE_MASTER_MODE			(0x0 << 0)
 
+/* ANALOGIX_DP_PKT_SEND_CTL */
+#define IF_UP					(0x1 << 4)
+#define IF_EN					(0x1 << 0)
+
+/* ANALOGIX_DP_CRC_CON */
+#define PSR_VID_CRC_FLUSH			(0x1 << 2)
+#define PSR_VID_CRC_ENABLE			(0x1 << 0)
+
 #endif /* _ANALOGIX_DP_REG_H */
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 261b86d..9cd8838 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -38,6 +38,9 @@ struct analogix_dp_plat_data {
 			 struct drm_connector *);
 };
 
+int analogix_dp_enable_psr(struct device *dev);
+int analogix_dp_disable_psr(struct device *dev);
+
 int analogix_dp_resume(struct device *dev);
 int analogix_dp_suspend(struct device *dev);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4 4/4] drm/rockchip: analogix_dp: implement PSR function
  2016-07-14  4:15 [PATCH v4 0/4] Add PSR function support for Analogix/Rockchip DP Yakir Yang
                   ` (2 preceding siblings ...)
  2016-07-14  4:15 ` [PATCH v4 3/4] drm/bridge: analogix_dp: add the PSR function support Yakir Yang
@ 2016-07-14  4:15 ` Yakir Yang
  2016-07-14 15:26   ` Sean Paul
  2016-07-15 10:55 ` [PATCH v4.1 1/4] drm/rockchip: vop: export line flag function Yakir Yang
  2016-07-15 10:55 ` [PATCH v4.1 3/4] drm/bridge: analogix_dp: add the PSR function support Yakir Yang
  5 siblings, 1 reply; 21+ messages in thread
From: Yakir Yang @ 2016-07-14  4:15 UTC (permalink / raw)
  To: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner
  Cc: Javier Martinez Canillas, Stéphane Marchesin, Sean Paul,
	Tomasz Figa, David Airlie, daniel.vetter, Thierry Reding,
	dianders, Krzysztof Kozlowski, emil.l.velikov, Dan Carpenter,
	Yakir Yang, linux-kernel, dri-devel, linux-samsung-soc,
	linux-rockchip

Alway enable the PSR function for Rockchip analogix_dp driver. If panel
don't support PSR, then the core analogix_dp would ignore this setting.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v4:
- Return 'void' instead of 'int' in analogix_dp_psr_set(). (Sean)
- Pull the 10ms delay time out into a #define. (Sean)
- Improved the code of analogix_dp_psr_work(). (Sean)
- Indented with spaces for new numbers in rockchip_dp_device struct. (Stéphane, reviewed at Google gerrit)
    [https://chromium-review.googlesource.com/#/c/349085/33/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c@83]

Changes in v3:
- split the common psr logic into a seperate driver, make this to a
  simple sub-psr device driver.

Changes in v2:
- remove vblank notify out (Daniel)
- create a psr_active() callback in vop data struct.

 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 57 +++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index e81e19a..aa916f4 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -32,6 +32,7 @@
 #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
@@ -41,6 +42,9 @@
 
 #define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
 
+#define PSR_SET_DELAY_TIME		msecs_to_jiffies(10)
+#define PSR_WAIT_LINE_FLAG_TIMEOUT_MS	100
+
 #define to_dp(nm)	container_of(nm, struct rockchip_dp_device, nm)
 
 /**
@@ -68,11 +72,55 @@ struct rockchip_dp_device {
 	struct regmap            *grf;
 	struct reset_control     *rst;
 
+	struct delayed_work      psr_work;
+	unsigned int             psr_state;
+
 	const struct rockchip_dp_chip_data *data;
 
 	struct analogix_dp_plat_data plat_data;
 };
 
+static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled)
+{
+	struct rockchip_dp_device *dp = to_dp(encoder);
+
+	dev_dbg(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit");
+
+	if (enabled)
+		dp->psr_state = EDP_VSC_PSR_STATE_ACTIVE;
+	else
+		dp->psr_state = ~EDP_VSC_PSR_STATE_ACTIVE;
+
+	schedule_delayed_work(&dp->psr_work, PSR_SET_DELAY_TIME);
+}
+
+static void analogix_dp_psr_work(struct work_struct *work)
+{
+	struct rockchip_dp_device *dp =
+				container_of(work, typeof(*dp), psr_work.work);
+	struct drm_crtc *crtc = dp->encoder.crtc;
+	int psr_state = dp->psr_state;
+	int vact_end;
+	int ret;
+
+	if (!crtc)
+		return;
+
+	vact_end = crtc->mode.vtotal - crtc->mode.vsync_start + crtc->mode.vdisplay;
+
+	ret = rockchip_drm_wait_line_flag(dp->encoder.crtc, vact_end,
+					  PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
+	if (ret) {
+		dev_err(dp->dev, "line flag interrupt did not arrive\n");
+		return;
+	}
+
+	if (psr_state == EDP_VSC_PSR_STATE_ACTIVE)
+		analogix_dp_enable_psr(dp->dev);
+	else
+		analogix_dp_disable_psr(dp->dev);
+}
+
 static int rockchip_dp_pre_init(struct rockchip_dp_device *dp)
 {
 	reset_control_assert(dp->rst);
@@ -340,12 +388,21 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
 	dp->plat_data.power_off = rockchip_dp_powerdown;
 	dp->plat_data.get_modes = rockchip_dp_get_modes;
 
+	dp->psr_state = ~EDP_VSC_PSR_STATE_ACTIVE;
+	INIT_DELAYED_WORK(&dp->psr_work, analogix_dp_psr_work);
+
+	rockchip_drm_psr_register(&dp->encoder, analogix_dp_psr_set);
+
 	return analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
 }
 
 static void rockchip_dp_unbind(struct device *dev, struct device *master,
 			       void *data)
 {
+	struct rockchip_dp_device *dp = dev_get_drvdata(dev);
+
+	rockchip_drm_psr_unregister(&dp->encoder);
+
 	return analogix_dp_unbind(dev, master, data);
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/4] drm/rockchip: vop: export line flag function
  2016-07-14  4:15 ` [PATCH v4 1/4] drm/rockchip: vop: export line flag function Yakir Yang
@ 2016-07-14 14:46   ` Sean Paul
  2016-07-15  1:43     ` Yakir Yang
  0 siblings, 1 reply; 21+ messages in thread
From: Sean Paul @ 2016-07-14 14:46 UTC (permalink / raw)
  To: Yakir Yang
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Krzysztof Kozlowski, linux-samsung-soc, linux-kernel,
	linux-rockchip, daniel.vetter, emil.l.velikov, dianders,
	dri-devel, Tomasz Figa, Javier Martinez Canillas,
	Stéphane Marchesin, Thierry Reding, Dan Carpenter

On Thu, Jul 14, 2016 at 12:15:44PM +0800, Yakir Yang wrote:
> VOP have integrated a hardware counter which indicate the exact display
> line that vop is scanning. And if we're interested in a specific line,
> we can set the line number to vop line_flag register, and then vop would
> generate a line_flag interrupt for it.
> 
> For example eDP PSR function is interested in the vertical blanking
> period, then driver could set the line number to zero.
> 
> This patch have exported a symbol that allow other driver to listen the
> line flag event with given timeout limit:
> -  rockchip_drm_wait_line_flag()
> 
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
> Changes in v4:
> - Avoid the weird behavior in rockchip_drm_wait_line_flag(). (Sean)
> - Make line_flag_num_x to an array. (Sean)
> - Remove the unused vop_cfg_done() in vop_line_flag_irq_enable(). (Stephane, reviewed in Google gerrit)
>     [https://chromium-review.googlesource.com/#/c/349084/33/drivers/gpu/drm/rockchip/rockchip_drm_vop.c@466]
> 
> Changes in v3:
> - Export the 'rockchip_drm_wait_line_flag' symbol, and document it.
> - Add 'line_flag_num_0' for RK3288/RK3036
> - Remove the notify for waiting line_flag event (Daniel)
> 
> Changes in v2:
> - Introduce in v2, split VOP line flag changes out
> 
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   3 +
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 118 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |   2 +
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   4 +
>  4 files changed, 127 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index ea39329..239b830 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -70,4 +70,7 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
>  				   struct device *dev);
>  void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>  				    struct device *dev);
> +int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
> +				unsigned int mstimeout);
> +
>  #endif /* _ROCKCHIP_DRM_DRV_H_ */
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index c8a62a8..69d32f1 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -121,6 +121,8 @@ struct vop {
>  	/* protected by dev->event_lock */
>  	struct drm_pending_vblank_event *event;
>  
> +	struct completion line_flag_completion;
> +
>  	const struct vop_data *data;
>  
>  	uint32_t *regsbak;
> @@ -431,6 +433,71 @@ static void vop_dsp_hold_valid_irq_disable(struct vop *vop)
>  	spin_unlock_irqrestore(&vop->irq_lock, flags);
>  }
>  
> +/*
> + * (1) each frame starts at the start of the Vsync pulse which is signaled by
> + *     the "FRAME_SYNC" interrupt.
> + * (2) the active data region of each frame ends at dsp_vact_end
> + * (3) we should program this same number (dsp_vact_end) into dsp_line_frag_num,
> + *      to get "LINE_FLAG" interrupt at the end of the active on screen data.
> + *
> + * VOP_INTR_CTRL0.dsp_line_frag_num = VOP_DSP_VACT_ST_END.dsp_vact_end
> + * Interrupts
> + * LINE_FLAG -------------------------------+
> + * FRAME_SYNC ----+                         |
> + *                |                         |
> + *                v                         v
> + *                | Vsync | Vbp |  Vactive  | Vfp |
> + *                        ^     ^           ^     ^
> + *                        |     |           |     |
> + *                        |     |           |     |
> + * dsp_vs_end ------------+     |           |     |   VOP_DSP_VTOTAL_VS_END
> + * dsp_vact_start --------------+           |     |   VOP_DSP_VACT_ST_END
> + * dsp_vact_end ----------------------------+     |   VOP_DSP_VACT_ST_END
> + * dsp_total -------------------------------------+   VOP_DSP_VTOTAL_VS_END
> + */
> +static bool vop_line_flag_irq_is_enabled(struct vop *vop)
> +{
> +	uint32_t line_flag_irq;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&vop->irq_lock, flags);
> +
> +	line_flag_irq = VOP_INTR_GET_TYPE(vop, enable, LINE_FLAG_INTR);
> +
> +	spin_unlock_irqrestore(&vop->irq_lock, flags);
> +
> +	return !!line_flag_irq;
> +}
> +
> +static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
> +{
> +	unsigned long flags;
> +
> +	if (WARN_ON(!vop->is_enabled))
> +		return;
> +
> +	spin_lock_irqsave(&vop->irq_lock, flags);
> +
> +	VOP_CTRL_SET(vop, line_flag_num[0], line_num);
> +	VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
> +
> +	spin_unlock_irqrestore(&vop->irq_lock, flags);
> +}
> +
> +static void vop_line_flag_irq_disable(struct vop *vop)
> +{
> +	unsigned long flags;
> +
> +	if (WARN_ON(!vop->is_enabled))
> +		return;
> +
> +	spin_lock_irqsave(&vop->irq_lock, flags);
> +
> +	VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 0);
> +
> +	spin_unlock_irqrestore(&vop->irq_lock, flags);
> +}
> +
>  static void vop_enable(struct drm_crtc *crtc)
>  {
>  	struct vop *vop = to_vop(crtc);
> @@ -1157,6 +1224,13 @@ static irqreturn_t vop_isr(int irq, void *data)
>  		ret = IRQ_HANDLED;
>  	}
>  
> +	if (active_irqs & LINE_FLAG_INTR) {
> +		if (!completion_done(&vop->line_flag_completion))
> +			complete(&vop->line_flag_completion);

You still have the completion_done/complete here.

If you just call complete() without checking if it's done, you'll remove the
race I mentioned in my first review.

Sean

> +		active_irqs &= ~LINE_FLAG_INTR;
> +		ret = IRQ_HANDLED;
> +	}
> +
>  	if (active_irqs & FS_INTR) {
>  		drm_crtc_handle_vblank(crtc);
>  		vop_handle_vblank(vop);
> @@ -1255,6 +1329,7 @@ static int vop_create_crtc(struct vop *vop)
>  
>  	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);
>  
> @@ -1411,6 +1486,49 @@ static void vop_win_init(struct vop *vop)
>  	}
>  }
>  
> +/**
> + * rockchip_drm_wait_line_flag - acqiure the give line flag event
> + * @crtc: CRTC to enable line flag
> + * @line_num: interested line number
> + * @mstimeout: millisecond for timeout
> + *
> + * Driver would hold here until the interested line flag interrupt have
> + * happened or timeout to wait.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
> +				unsigned int mstimeout)
> +{
> +	struct vop *vop = to_vop(crtc);
> +	unsigned long jiffies_left;
> +
> +	if (!crtc || !vop->is_enabled)
> +		return -ENODEV;
> +
> +	if (line_num > crtc->mode.vtotal || mstimeout <= 0)
> +		return -EINVAL;
> +
> +	if (vop_line_flag_irq_is_enabled(vop))
> +		return -EBUSY;
> +
> +	reinit_completion(&vop->line_flag_completion);
> +	vop_line_flag_irq_enable(vop, line_num);
> +
> +	jiffies_left = wait_for_completion_timeout(&vop->line_flag_completion,
> +						   msecs_to_jiffies(mstimeout));
> +	vop_line_flag_irq_disable(vop);
> +
> +	if (jiffies_left == 0) {
> +		dev_err(vop->dev, "Timeout waiting for IRQ\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(rockchip_drm_wait_line_flag);
> +
>  static int vop_bind(struct device *dev, struct device *master, void *data)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index ff4f52e..1dbc526 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -61,6 +61,8 @@ struct vop_ctrl {
>  	struct vop_reg hpost_st_end;
>  	struct vop_reg vpost_st_end;
>  
> +	struct vop_reg line_flag_num[2];
> +
>  	struct vop_reg cfg_done;
>  };
>  
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 6f42e56..eea8427 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -122,6 +122,7 @@ static const struct vop_ctrl rk3036_ctrl_data = {
>  	.hact_st_end = VOP_REG(RK3036_DSP_HACT_ST_END, 0x1fff1fff, 0),
>  	.vtotal_pw = VOP_REG(RK3036_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
>  	.vact_st_end = VOP_REG(RK3036_DSP_VACT_ST_END, 0x1fff1fff, 0),
> +	.line_flag_num[0] = VOP_REG(RK3036_INT_STATUS, 0xfff, 12),
>  	.cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
>  };
>  
> @@ -221,6 +222,7 @@ static const struct vop_ctrl rk3288_ctrl_data = {
>  	.vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
>  	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>  	.vpost_st_end = VOP_REG(RK3288_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
> +	.line_flag_num[0] = VOP_REG(RK3288_INTR_CTRL0, 0x1fff, 12),
>  	.cfg_done = VOP_REG(RK3288_REG_CFG_DONE, 0x1, 0),
>  };
>  
> @@ -299,6 +301,8 @@ static const struct vop_ctrl rk3399_ctrl_data = {
>  	.vact_st_end = VOP_REG(RK3399_DSP_VACT_ST_END, 0x1fff1fff, 0),
>  	.hpost_st_end = VOP_REG(RK3399_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>  	.vpost_st_end = VOP_REG(RK3399_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
> +	.line_flag_num[0] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 0),
> +	.line_flag_num[1] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 16),
>  	.cfg_done = VOP_REG_MASK(RK3399_REG_CFG_DONE, 0x1, 0),
>  };
>  
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/4] drm/rockchip: add an common abstracted PSR driver
  2016-07-14  4:15 ` [PATCH v4 2/4] drm/rockchip: add an common abstracted PSR driver Yakir Yang
@ 2016-07-14 15:14   ` Sean Paul
  2016-07-15  1:43     ` Yakir Yang
  2016-07-24  7:14     ` Yakir Yang
  2016-07-23  4:04   ` Doug Anderson
  1 sibling, 2 replies; 21+ messages in thread
From: Sean Paul @ 2016-07-14 15:14 UTC (permalink / raw)
  To: Yakir Yang
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Krzysztof Kozlowski, linux-samsung-soc, linux-kernel,
	linux-rockchip, daniel.vetter, emil.l.velikov, dianders,
	dri-devel, Tomasz Figa, Javier Martinez Canillas,
	Stéphane Marchesin, Thierry Reding, Dan Carpenter

On Thu, Jul 14, 2016 at 12:15:49PM +0800, Yakir Yang wrote:
> The PSR driver have exported four symbols for specific device driver:
> - rockchip_drm_psr_register()
> - rockchip_drm_psr_unregister()
> - rockchip_drm_psr_enable()
> - rockchip_drm_psr_disable()
> - rockchip_drm_psr_flush()
> 
> Encoder driver should call the register/unregister interfaces to hook
> itself into common PSR driver, encoder have implement the 'psr_set'
> callback which use the set PSR state in hardware side.
> 
> Crtc driver would call the enable/disable interfaces when vblank is
> enable/disable, after that the common PSR driver would call the encoder
> registered callback to set the PSR state.
> 
> Fb driver would call the flush interface in 'fb->dirty' callback, this
> helper function would force all PSR enabled encoders to exit from PSR
> for 3 seconds.
> 
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>


I still don't think it's a good idea to pull this out into a separate PSR
driver, but I suppose we can integrate it at a later time if it becomes
cumbersome.

Reviewed-by: Sean Paul <seanpaul@chromium.org>


> ---
> Changes in v4:
> - Tuck the global "psr_list" & "psr_list_mutex" in struct rockchip_drm_private. (Sean)
> - Move the access of "psr->state" under "psr->state_mutex"'s protect. (Sean)
> - Let "psr->state = PSR_FLUSH" under "psr->state_mutex"'s protect. (Sean)
> - Collect psr_enable() and psr_disable() into psr_set_state()
> - s/5\ second/PSR_FLUSH_TIMEOUT/ (Sean)
> - Flush the psr callback in vop_crtc_disable(). (Stéphane, reviewed in Google gerrit)
>     [https://chromium-review.googlesource.com/#/c/349084/6/drivers/gpu/drm/rockchip/rockchip_drm_vop.c@475]
> - Add the missing file head with license. (Stéphane, reviewed in Google gerrit)
>     [https://chromium-review.googlesource.com/#/c/357563/1/drivers/gpu/drm/rockchip/rockchip_drm_psr.h@3]
> 
> Changes in v3:
> - split the psr flow into an common abstracted PSR driver
> - implement the 'fb->dirty' callback function (Daniel)
> - avoid to use notify to acqiure for vact event (Daniel)
> - remove psr_active() callback which introduce in v2
> 
> Changes in v2: None
> 
>  drivers/gpu/drm/rockchip/Makefile           |   2 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c |   4 +
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   3 +
>  drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  12 ++
>  drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 223 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/rockchip/rockchip_drm_psr.h |  26 ++++
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  29 ++++
>  7 files changed, 298 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.c
>  create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.h
> 
> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
> index 05d0713..9746365 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -3,7 +3,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_vop.o
> +		rockchip_drm_gem.o rockchip_drm_psr.o rockchip_drm_vop.o
>  rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>  
>  obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index d665fb0..26c12b3 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -156,6 +156,9 @@ static int rockchip_drm_bind(struct device *dev)
>  
>  	drm_dev->dev_private = private;
>  
> +	INIT_LIST_HEAD(&private->psr_list);
> +	mutex_init(&private->psr_list_mutex);
> +
>  	drm_mode_config_init(drm_dev);
>  
>  	rockchip_drm_mode_config_init(drm_dev);
> @@ -218,6 +221,7 @@ static int rockchip_drm_bind(struct device *dev)
>  
>  	if (is_support_iommu)
>  		arm_iommu_release_mapping(mapping);
> +
>  	return 0;
>  err_fbdev_fini:
>  	rockchip_drm_fbdev_fini(drm_dev);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index 239b830..9c34c9e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -61,6 +61,9 @@ struct rockchip_drm_private {
>  	struct drm_gem_object *fbdev_bo;
>  	const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
>  	struct drm_atomic_state *state;
> +
> +	struct list_head psr_list;
> +	struct mutex psr_list_mutex;
>  };
>  
>  int rockchip_register_crtc_funcs(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index 20f12bc..36afd9c 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -21,6 +21,7 @@
>  
>  #include "rockchip_drm_drv.h"
>  #include "rockchip_drm_gem.h"
> +#include "rockchip_drm_psr.h"
>  
>  #define to_rockchip_fb(x) container_of(x, struct rockchip_drm_fb, fb)
>  
> @@ -66,9 +67,20 @@ static int rockchip_drm_fb_create_handle(struct drm_framebuffer *fb,
>  				     rockchip_fb->obj[0], handle);
>  }
>  
> +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(fb->dev);
> +	return 0;
> +}
> +
>  static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
>  	.destroy	= rockchip_drm_fb_destroy,
>  	.create_handle	= rockchip_drm_fb_create_handle,
> +	.dirty		= rockchip_drm_fb_dirty,
>  };
>  
>  static struct rockchip_drm_fb *
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> new file mode 100644
> index 0000000..e79ea18
> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> @@ -0,0 +1,223 @@
> +/*
> + * 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_crtc_helper.h>
> +
> +#include "rockchip_drm_drv.h"
> +#include "rockchip_drm_psr.h"
> +
> +#define PSR_FLUSH_TIMEOUT	msecs_to_jiffies(3000) /* 3 seconds */
> +
> +static LIST_HEAD(psr_list);
> +static DEFINE_MUTEX(psr_list_mutex);
> +
> +enum psr_state {
> +	PSR_FLUSH,
> +	PSR_ENABLE,
> +	PSR_DISABLE,
> +};
> +
> +struct psr_drv {
> +	struct list_head list;
> +	enum psr_state state;
> +	struct mutex state_mutex;
> +
> +	struct timer_list flush_timer;
> +
> +	struct drm_encoder *encoder;
> +	void (*set)(struct drm_encoder *encoder, bool enable);
> +};
> +
> +static struct psr_drv *find_psr_by_crtc(struct drm_crtc *crtc)
> +{
> +	struct rockchip_drm_private *drm_drv = crtc->dev->dev_private;
> +	struct psr_drv *psr;
> +
> +	mutex_lock(&drm_drv->psr_list_mutex);
> +	list_for_each_entry(psr, &drm_drv->psr_list, list) {
> +		if (psr->encoder->crtc == crtc) {
> +			mutex_unlock(&drm_drv->psr_list_mutex);
> +			return psr;
> +		}
> +	}
> +	mutex_unlock(&drm_drv->psr_list_mutex);
> +
> +	return ERR_PTR(-ENODEV);
> +}
> +
> +static void psr_set_state(struct psr_drv *psr, enum psr_state state)
> +{
> +	mutex_lock(&psr->state_mutex);
> +
> +	if (psr->state == state) {
> +		mutex_unlock(&psr->state_mutex);
> +		return;
> +	}
> +
> +	psr->state = state;
> +	switch (state) {
> +	case PSR_ENABLE:
> +		psr->set(psr->encoder, true);
> +		break;
> +
> +	case PSR_DISABLE:
> +	case PSR_FLUSH:
> +		psr->set(psr->encoder, false);
> +		break;
> +	};
> +
> +	mutex_unlock(&psr->state_mutex);
> +}
> +
> +static void psr_flush_handler(unsigned long data)
> +{
> +	struct psr_drv *psr = (struct psr_drv *)data;
> +
> +	if (!psr || psr->state != PSR_FLUSH)
> +		return;
> +
> +	psr_set_state(psr, PSR_ENABLE);
> +}
> +
> +/**
> + * rockchip_drm_psr_enable - enable the encoder PSR which bind to given CRTC
> + * @crtc: CRTC to obtain the PSR encoder
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int rockchip_drm_psr_enable(struct drm_crtc *crtc)
> +{
> +	struct psr_drv *psr = find_psr_by_crtc(crtc);
> +
> +	if (IS_ERR(psr))
> +		return PTR_ERR(psr);
> +
> +	psr_set_state(psr, PSR_ENABLE);
> +	return 0;
> +}
> +EXPORT_SYMBOL(rockchip_drm_psr_enable);
> +
> +/**
> + * rockchip_drm_psr_disable - disable the encoder PSR which bind to given CRTC
> + * @crtc: CRTC to obtain the PSR encoder
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int rockchip_drm_psr_disable(struct drm_crtc *crtc)
> +{
> +	struct psr_drv *psr = find_psr_by_crtc(crtc);
> +
> +	if (IS_ERR(psr))
> +		return PTR_ERR(psr);
> +
> +	psr_set_state(psr, PSR_DISABLE);
> +	return 0;
> +}
> +EXPORT_SYMBOL(rockchip_drm_psr_disable);
> +
> +/**
> + * rockchip_drm_psr_flush - 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(struct drm_device *dev)
> +{
> +	struct rockchip_drm_private *drm_drv = dev->dev_private;
> +	struct psr_drv *psr;
> +
> +	mutex_lock(&drm_drv->psr_list_mutex);
> +	list_for_each_entry(psr, &drm_drv->psr_list, list) {
> +		if (psr->state == PSR_DISABLE)
> +			continue;
> +
> +		mod_timer(&psr->flush_timer,
> +			  round_jiffies_up(jiffies + PSR_FLUSH_TIMEOUT));
> +
> +		psr_set_state(psr, PSR_FLUSH);
> +	}
> +	mutex_unlock(&drm_drv->psr_list_mutex);
> +}
> +EXPORT_SYMBOL(rockchip_drm_psr_flush);
> +
> +/**
> + * rockchip_drm_psr_register - register encoder to psr driver
> + * @encoder: encoder that obtain the PSR function
> + * @psr_set: call back to set PSR state
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int rockchip_drm_psr_register(struct drm_encoder *encoder,
> +			void (*psr_set)(struct drm_encoder *, bool enable))
> +{
> +	struct rockchip_drm_private *drm_drv = encoder->dev->dev_private;
> +	struct psr_drv *psr;
> +
> +	if (!encoder || !psr_set)
> +		return -EINVAL;
> +
> +	psr = kzalloc(sizeof(struct psr_drv), GFP_KERNEL);
> +	if (!psr)
> +		return -ENOMEM;
> +
> +	setup_timer(&psr->flush_timer, psr_flush_handler, (unsigned long)psr);
> +
> +	mutex_init(&psr->state_mutex);
> +
> +	psr->state = PSR_DISABLE;
> +	psr->encoder = encoder;
> +	psr->set = psr_set;
> +
> +	mutex_lock(&drm_drv->psr_list_mutex);
> +	list_add_tail(&psr->list, &drm_drv->psr_list);
> +	mutex_unlock(&drm_drv->psr_list_mutex);
> +
> +	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
> + *
> + * 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;
> +
> +	mutex_lock(&drm_drv->psr_list_mutex);
> +	list_for_each_entry(psr, &drm_drv->psr_list, list) {
> +		if (psr->encoder == encoder) {
> +			del_timer(&psr->flush_timer);
> +			list_del(&psr->list);
> +			kfree(psr);
> +		}
> +	}
> +	mutex_unlock(&drm_drv->psr_list_mutex);
> +}
> +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
> new file mode 100644
> index 0000000..c35b688
> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h
> @@ -0,0 +1,26 @@
> +/*
> + * 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(struct drm_device *dev);
> +int rockchip_drm_psr_enable(struct drm_crtc *crtc);
> +int rockchip_drm_psr_disable(struct drm_crtc *crtc);
> +
> +int rockchip_drm_psr_register(struct drm_encoder *encoder,
> +			void (*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 69d32f1..e702fb3 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -34,6 +34,7 @@
>  #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"
>  
>  #define __REG_SET_RELAXED(x, off, mask, shift, v, write_mask) \
> @@ -87,6 +88,8 @@
>  #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)
>  
> +#define VOP_PSR_SET_DELAY_TIME		msecs_to_jiffies(10)
> +
>  struct vop_plane_state {
>  	struct drm_plane_state base;
>  	int format;
> @@ -121,6 +124,9 @@ struct vop {
>  	/* protected by dev->event_lock */
>  	struct drm_pending_vblank_event *event;
>  
> +	bool psr_enabled;
> +	struct delayed_work psr_work;
> +
>  	struct completion line_flag_completion;
>  
>  	const struct vop_data *data;
> @@ -629,6 +635,9 @@ static void vop_crtc_disable(struct drm_crtc *crtc)
>  
>  		crtc->state->event = NULL;
>  	}
> +
> +	vop->psr_enabled = false;
> +	INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
>  }
>  
>  static void vop_plane_destroy(struct drm_plane *plane)
> @@ -923,6 +932,16 @@ static const struct drm_plane_funcs vop_plane_funcs = {
>  	.atomic_destroy_state = vop_atomic_plane_destroy_state,
>  };
>  
> +static void vop_psr_work(struct work_struct *work)
> +{
> +	struct vop *vop = container_of(work, typeof(*vop), psr_work.work);
> +
> +	if (vop->psr_enabled)
> +		rockchip_drm_psr_enable(&vop->crtc);
> +	else
> +		rockchip_drm_psr_disable(&vop->crtc);
> +}
> +
>  static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
>  {
>  	struct vop *vop = to_vop(crtc);
> @@ -937,6 +956,9 @@ static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
>  
>  	spin_unlock_irqrestore(&vop->irq_lock, flags);
>  
> +	vop->psr_enabled = false;
> +	schedule_delayed_work(&vop->psr_work, VOP_PSR_SET_DELAY_TIME);
> +
>  	return 0;
>  }
>  
> @@ -953,6 +975,9 @@ 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);
> +
> +	vop->psr_enabled = true;
> +	schedule_delayed_work(&vop->psr_work, VOP_PSR_SET_DELAY_TIME);
>  }
>  
>  static void vop_crtc_wait_for_update(struct drm_crtc *crtc)
> @@ -1597,6 +1622,10 @@ static int vop_bind(struct device *dev, struct device *master, void *data)
>  		return ret;
>  
>  	pm_runtime_enable(&pdev->dev);
> +
> +	vop->psr_enabled = false;
> +	INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
> +
>  	return 0;
>  }
>  
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 3/4] drm/bridge: analogix_dp: add the PSR function support
  2016-07-14  4:15 ` [PATCH v4 3/4] drm/bridge: analogix_dp: add the PSR function support Yakir Yang
@ 2016-07-14 15:23   ` Sean Paul
  0 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2016-07-14 15:23 UTC (permalink / raw)
  To: Yakir Yang
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Krzysztof Kozlowski, linux-samsung-soc, linux-kernel,
	linux-rockchip, daniel.vetter, emil.l.velikov, dianders,
	dri-devel, Tomasz Figa, Javier Martinez Canillas,
	Stéphane Marchesin, Thierry Reding, Dan Carpenter

On Thu, Jul 14, 2016 at 12:15:53PM +0800, Yakir Yang wrote:
> The full name of PSR is Panel Self Refresh, panel device could refresh
> itself with the hardware framebuffer in panel, this would make lots of
> sense to save the power consumption.
> 
> This patch have exported two symbols for platform driver to implement
> the PSR function in hardware side:
> - analogix_dp_active_psr()
> - analogix_dp_inactive_psr()
> 
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> ---
> Changes in v4:
> - Downgrade the PSR version print message to debug level. (Sean)
> - Return 'void' instead of 'int' in analogix_dp_enable_sink_psr(). (Sean)
> - Delete the unused read dpcd operations in analogix_dp_enable_sink_psr(). (Sean)
> - Delete the arbitrary usleep_range in analogix_dp_enable_psr_crc. (Sean).
> - Clean up the hardcoded values in analogix_dp_send_psr_spd(). (Sean)
> - Rename "active/inactive" to "enable/disable". (Sean, Dominik)
> - Keep set the PSR_VID_CRC_FLUSH gate in analogix_dp_enable_psr_crc().
> 
> Changes in v3:
> - split analogix_dp_enable_psr(), make it more clearly
>     analogix_dp_detect_sink_psr()
>     analogix_dp_enable_sink_psr()
> - remove some nosie register setting comments
> 
> Changes in v2:
> - introduce in v2, splite the common Analogix DP changes out
> 
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 60 ++++++++++++++++++++++
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  4 ++
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 49 ++++++++++++++++++
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h  | 28 ++++++++++
>  include/drm/bridge/analogix_dp.h                   |  3 ++
>  5 files changed, 144 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 32715da..1fec91a 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -97,6 +97,62 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
>  	return 0;
>  }
>  
> +int analogix_dp_enable_psr(struct device *dev)
> +{
> +	struct analogix_dp_device *dp = dev_get_drvdata(dev);
> +
> +	if (!dp->psr_support)
> +		return -EINVAL;
> +
> +	analogix_dp_send_psr_spd(dp, EDP_VSC_PSR_STATE_ACTIVE |
> +				 EDP_VSC_PSR_CRC_VALUES_VALID);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
> +
> +int analogix_dp_disable_psr(struct device *dev)
> +{
> +	struct analogix_dp_device *dp = dev_get_drvdata(dev);
> +
> +	if (!dp->psr_support)
> +		return -EINVAL;
> +
> +	analogix_dp_send_psr_spd(dp, 0);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
> +
> +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);
> +	dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version);
> +
> +	return (psr_version & DP_PSR_IS_SUPPORTED) ? true : false;
> +}
> +
> +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);
> +	psr_en &= ~DP_PSR_ENABLE;
> +	analogix_dp_write_byte_to_dpcd(dp, 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);
> +
> +	/* 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);
> +
> +	analogix_dp_enable_psr_crc(dp);
> +}
> +
>  static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
>  {
>  	int i;
> @@ -921,6 +977,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
>  
>  	/* Enable video */
>  	analogix_dp_start_video(dp);
> +
> +	dp->psr_support = analogix_dp_detect_sink_psr(dp);
> +	if (dp->psr_support)
> +		analogix_dp_enable_sink_psr(dp);
>  }
>  
>  int analogix_dp_get_modes(struct drm_connector *connector)
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index b456380..6ca5dde 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -177,6 +177,7 @@ struct analogix_dp_device {
>  	int			hpd_gpio;
>  	bool                    force_hpd;
>  	unsigned char           edid[EDID_BLOCK_LENGTH * 2];
> +	bool			psr_support;
>  
>  	struct analogix_dp_plat_data *plat_data;
>  };
> @@ -278,4 +279,7 @@ int analogix_dp_is_video_stream_on(struct analogix_dp_device *dp);
>  void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>  void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>  void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
> +void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
> +void analogix_dp_send_psr_spd(struct analogix_dp_device *dp, int db1);
> +
>  #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 48030f0..26446ae 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -26,6 +26,9 @@
>  #define COMMON_INT_MASK_4	(HOTPLUG_CHG | HPD_LOST | PLUG)
>  #define INT_STA_MASK		INT_HPD
>  
> +static u32 psr_spd_hbx[] = { 0x00, 0x07, 0x02, 0x08 };
> +static u32 psr_spd_pbx[] = { 0x00, 0x16, 0xCE, 0x5D };

These need to be const. Can you also please add a comment explaining where you
got the values from?

> +
>  void analogix_dp_enable_video_mute(struct analogix_dp_device *dp, bool enable)
>  {
>  	u32 reg;
> @@ -1322,3 +1325,49 @@ void analogix_dp_disable_scrambling(struct analogix_dp_device *dp)
>  	reg |= SCRAMBLING_DISABLE;
>  	writel(reg, dp->reg_base + ANALOGIX_DP_TRAINING_PTN_SET);
>  }
> +
> +void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
> +{
> +	writel(PSR_VID_CRC_FLUSH | PSR_VID_CRC_ENABLE,
> +	       dp->reg_base + ANALOGIX_DP_CRC_CON);
> +}
> +
> +void analogix_dp_send_psr_spd(struct analogix_dp_device *dp, int db1)
> +{
> +	unsigned int val;
> +	unsigned int i;
> +
> +	/* don't send info frame */
> +	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +	val &= ~IF_EN;
> +	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +
> +	/* configure single frame update mode */
> +	writel(PSR_FRAME_UP_TYPE_BURST | PSR_CRC_SEL_HARDWARE,
> +	       dp->reg_base + ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL);
> +
> +	/* configure VSC HB0~HB3 and PB0~PB3 */
> +	for (i = 0; i < 4; i++)
> +		writel(psr_spd_hbx[i], dp->reg_base + ANALOGIX_DP_SPD_HB + i*4);
> +	for (i = 0; i < 4; i++)
> +		writel(psr_spd_pbx[i], dp->reg_base + ANALOGIX_DP_SPD_PB + i*4);
> +
> +	/* configure DB0 / DB1 values */
> +	writel(0x0, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB0);
> +	writel(db1, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB1);
> +
> +	/* set reuse spd inforframe */
> +	val = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_3);
> +	val |= REUSE_SPD_EN;
> +	writel(val, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_3);
> +
> +	/* mark info frame update */
> +	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +	val = (val | IF_UP) & ~IF_EN;
> +	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +
> +	/* send info frame */
> +	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +	val |= IF_EN;
> +	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +}
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
> index cdcc6c5..fd232b2 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
> @@ -22,6 +22,8 @@
>  #define ANALOGIX_DP_VIDEO_CTL_8			0x3C
>  #define ANALOGIX_DP_VIDEO_CTL_10		0x44
>  
> +#define ANALOGIX_DP_SPDIF_AUDIO_CTL_0		0xD8
> +
>  #define ANALOGIX_DP_PLL_REG_1			0xfc
>  #define ANALOGIX_DP_PLL_REG_2			0x9e4
>  #define ANALOGIX_DP_PLL_REG_3			0x9e8
> @@ -30,6 +32,15 @@
>  
>  #define ANALOGIX_DP_PD				0x12c
>  
> +#define ANALOGIX_DP_IF_TYPE			0x244
> +#define ANALOGIX_DP_IF_PKT_DB1			0x254
> +#define ANALOGIX_DP_IF_PKT_DB2			0x258
> +#define ANALOGIX_DP_SPD_HB			0x2F8
> +#define ANALOGIX_DP_SPD_PB			0x308
> +#define ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL	0x318
> +#define ANALOGIX_DP_VSC_SHADOW_DB0		0x31C
> +#define ANALOGIX_DP_VSC_SHADOW_DB1		0x320
> +
>  #define ANALOGIX_DP_LANE_MAP			0x35C
>  
>  #define ANALOGIX_DP_ANALOG_CTL_1		0x370
> @@ -103,6 +114,8 @@
>  
>  #define ANALOGIX_DP_SOC_GENERAL_CTL		0x800
>  
> +#define ANALOGIX_DP_CRC_CON			0x890
> +
>  /* ANALOGIX_DP_TX_SW_RESET */
>  #define RESET_DP_TX				(0x1 << 0)
>  
> @@ -151,6 +164,7 @@
>  #define VID_CHK_UPDATE_TYPE_SHIFT		(4)
>  #define VID_CHK_UPDATE_TYPE_1			(0x1 << 4)
>  #define VID_CHK_UPDATE_TYPE_0			(0x0 << 4)
> +#define REUSE_SPD_EN				(0x1 << 3)
>  
>  /* ANALOGIX_DP_VIDEO_CTL_8 */
>  #define VID_HRES_TH(x)				(((x) & 0xf) << 4)
> @@ -167,6 +181,12 @@
>  #define REF_CLK_27M				(0x0 << 0)
>  #define REF_CLK_MASK				(0x1 << 0)
>  
> +/* ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL */
> +#define PSR_FRAME_UP_TYPE_BURST			(0x1 << 0)
> +#define PSR_FRAME_UP_TYPE_SINGLE		(0x0 << 0)
> +#define PSR_CRC_SEL_HARDWARE			(0x1 << 1)
> +#define PSR_CRC_SEL_MANUALLY			(0x0 << 1)
> +
>  /* ANALOGIX_DP_LANE_MAP */
>  #define LANE3_MAP_LOGIC_LANE_0			(0x0 << 6)
>  #define LANE3_MAP_LOGIC_LANE_1			(0x1 << 6)
> @@ -376,4 +396,12 @@
>  #define VIDEO_MODE_SLAVE_MODE			(0x1 << 0)
>  #define VIDEO_MODE_MASTER_MODE			(0x0 << 0)
>  
> +/* ANALOGIX_DP_PKT_SEND_CTL */
> +#define IF_UP					(0x1 << 4)
> +#define IF_EN					(0x1 << 0)
> +
> +/* ANALOGIX_DP_CRC_CON */
> +#define PSR_VID_CRC_FLUSH			(0x1 << 2)
> +#define PSR_VID_CRC_ENABLE			(0x1 << 0)
> +
>  #endif /* _ANALOGIX_DP_REG_H */
> diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
> index 261b86d..9cd8838 100644
> --- a/include/drm/bridge/analogix_dp.h
> +++ b/include/drm/bridge/analogix_dp.h
> @@ -38,6 +38,9 @@ struct analogix_dp_plat_data {
>  			 struct drm_connector *);
>  };
>  
> +int analogix_dp_enable_psr(struct device *dev);
> +int analogix_dp_disable_psr(struct device *dev);
> +
>  int analogix_dp_resume(struct device *dev);
>  int analogix_dp_suspend(struct device *dev);
>  
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 4/4] drm/rockchip: analogix_dp: implement PSR function
  2016-07-14  4:15 ` [PATCH v4 4/4] drm/rockchip: analogix_dp: implement PSR function Yakir Yang
@ 2016-07-14 15:26   ` Sean Paul
  2016-07-15  5:45     ` Yakir Yang
  0 siblings, 1 reply; 21+ messages in thread
From: Sean Paul @ 2016-07-14 15:26 UTC (permalink / raw)
  To: Yakir Yang
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Krzysztof Kozlowski, linux-samsung-soc, linux-kernel,
	linux-rockchip, daniel.vetter, emil.l.velikov, dianders,
	dri-devel, Tomasz Figa, Javier Martinez Canillas,
	Stéphane Marchesin, Thierry Reding, Dan Carpenter

On Thu, Jul 14, 2016 at 12:15:58PM +0800, Yakir Yang wrote:
> Alway enable the PSR function for Rockchip analogix_dp driver. If panel
> don't support PSR, then the core analogix_dp would ignore this setting.
> 
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
> Changes in v4:
> - Return 'void' instead of 'int' in analogix_dp_psr_set(). (Sean)
> - Pull the 10ms delay time out into a #define. (Sean)
> - Improved the code of analogix_dp_psr_work(). (Sean)
> - Indented with spaces for new numbers in rockchip_dp_device struct. (Stéphane, reviewed at Google gerrit)
>     [https://chromium-review.googlesource.com/#/c/349085/33/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c@83]
> 
> Changes in v3:
> - split the common psr logic into a seperate driver, make this to a
>   simple sub-psr device driver.
> 
> Changes in v2:
> - remove vblank notify out (Daniel)
> - create a psr_active() callback in vop data struct.
> 
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 57 +++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index e81e19a..aa916f4 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -32,6 +32,7 @@
>  #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
> @@ -41,6 +42,9 @@
>  
>  #define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
>  
> +#define PSR_SET_DELAY_TIME		msecs_to_jiffies(10)
> +#define PSR_WAIT_LINE_FLAG_TIMEOUT_MS	100
> +
>  #define to_dp(nm)	container_of(nm, struct rockchip_dp_device, nm)
>  
>  /**
> @@ -68,11 +72,55 @@ struct rockchip_dp_device {
>  	struct regmap            *grf;
>  	struct reset_control     *rst;
>  
> +	struct delayed_work      psr_work;
> +	unsigned int             psr_state;
> +
>  	const struct rockchip_dp_chip_data *data;
>  
>  	struct analogix_dp_plat_data plat_data;
>  };
>  
> +static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled)
> +{
> +	struct rockchip_dp_device *dp = to_dp(encoder);
> +
> +	dev_dbg(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit");
> +
> +	if (enabled)
> +		dp->psr_state = EDP_VSC_PSR_STATE_ACTIVE;
> +	else
> +		dp->psr_state = ~EDP_VSC_PSR_STATE_ACTIVE;
> +
> +	schedule_delayed_work(&dp->psr_work, PSR_SET_DELAY_TIME);
> +}
> +
> +static void analogix_dp_psr_work(struct work_struct *work)
> +{
> +	struct rockchip_dp_device *dp =
> +				container_of(work, typeof(*dp), psr_work.work);
> +	struct drm_crtc *crtc = dp->encoder.crtc;
> +	int psr_state = dp->psr_state;
> +	int vact_end;
> +	int ret;
> +
> +	if (!crtc)
> +		return;
> +
> +	vact_end = crtc->mode.vtotal - crtc->mode.vsync_start + crtc->mode.vdisplay;
> +
> +	ret = rockchip_drm_wait_line_flag(dp->encoder.crtc, vact_end,
> +					  PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
> +	if (ret) {
> +		dev_err(dp->dev, "line flag interrupt did not arrive\n");
> +		return;
> +	}
> +
> +	if (psr_state == EDP_VSC_PSR_STATE_ACTIVE)
> +		analogix_dp_enable_psr(dp->dev);
> +	else
> +		analogix_dp_disable_psr(dp->dev);
> +}
> +
>  static int rockchip_dp_pre_init(struct rockchip_dp_device *dp)
>  {
>  	reset_control_assert(dp->rst);
> @@ -340,12 +388,21 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
>  	dp->plat_data.power_off = rockchip_dp_powerdown;
>  	dp->plat_data.get_modes = rockchip_dp_get_modes;
>  
> +	dp->psr_state = ~EDP_VSC_PSR_STATE_ACTIVE;
> +	INIT_DELAYED_WORK(&dp->psr_work, analogix_dp_psr_work);
> +
> +	rockchip_drm_psr_register(&dp->encoder, analogix_dp_psr_set);
> +
>  	return analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
>  }
>  
>  static void rockchip_dp_unbind(struct device *dev, struct device *master,
>  			       void *data)
>  {
> +	struct rockchip_dp_device *dp = dev_get_drvdata(dev);
> +
> +	rockchip_drm_psr_unregister(&dp->encoder);
> +
>  	return analogix_dp_unbind(dev, master, data);
>  }
>  
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/4] drm/rockchip: vop: export line flag function
  2016-07-14 14:46   ` Sean Paul
@ 2016-07-15  1:43     ` Yakir Yang
  0 siblings, 0 replies; 21+ messages in thread
From: Yakir Yang @ 2016-07-15  1:43 UTC (permalink / raw)
  To: Sean Paul
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Krzysztof Kozlowski, linux-samsung-soc, linux-kernel,
	linux-rockchip, daniel.vetter, emil.l.velikov, dianders,
	dri-devel, Tomasz Figa, Javier Martinez Canillas,
	Stéphane Marchesin, Thierry Reding, Dan Carpenter

Sean,

On 07/14/2016 10:46 PM, Sean Paul wrote:
> On Thu, Jul 14, 2016 at 12:15:44PM +0800, Yakir Yang wrote:
>> VOP have integrated a hardware counter which indicate the exact display
>> line that vop is scanning. And if we're interested in a specific line,
>> we can set the line number to vop line_flag register, and then vop would
>> generate a line_flag interrupt for it.
>>
>> For example eDP PSR function is interested in the vertical blanking
>> period, then driver could set the line number to zero.
>>
>> This patch have exported a symbol that allow other driver to listen the
>> line flag event with given timeout limit:
>> -  rockchip_drm_wait_line_flag()
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> ---
>> Changes in v4:
>> - Avoid the weird behavior in rockchip_drm_wait_line_flag(). (Sean)
>> - Make line_flag_num_x to an array. (Sean)
>> - Remove the unused vop_cfg_done() in vop_line_flag_irq_enable(). (Stephane, reviewed in Google gerrit)
>>      [https://chromium-review.googlesource.com/#/c/349084/33/drivers/gpu/drm/rockchip/rockchip_drm_vop.c@466]
>>
>> Changes in v3:
>> - Export the 'rockchip_drm_wait_line_flag' symbol, and document it.
>> - Add 'line_flag_num_0' for RK3288/RK3036
>> - Remove the notify for waiting line_flag event (Daniel)
>>
>> Changes in v2:
>> - Introduce in v2, split VOP line flag changes out
>>
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   3 +
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 118 ++++++++++++++++++++++++++++
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.h |   2 +
>>   drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   4 +
>>   4 files changed, 127 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> index ea39329..239b830 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> @@ -70,4 +70,7 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
>>   				   struct device *dev);
>>   void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>>   				    struct device *dev);
>> +int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
>> +				unsigned int mstimeout);
>> +
>>   #endif /* _ROCKCHIP_DRM_DRV_H_ */
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> index c8a62a8..69d32f1 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> @@ -121,6 +121,8 @@ struct vop {
>>   	/* protected by dev->event_lock */
>>   	struct drm_pending_vblank_event *event;
>>   
>> +	struct completion line_flag_completion;
>> +
>>   	const struct vop_data *data;
>>   
>>   	uint32_t *regsbak;
>> @@ -431,6 +433,71 @@ static void vop_dsp_hold_valid_irq_disable(struct vop *vop)
>>   	spin_unlock_irqrestore(&vop->irq_lock, flags);
>>   }
>>   
>> +/*
>> + * (1) each frame starts at the start of the Vsync pulse which is signaled by
>> + *     the "FRAME_SYNC" interrupt.
>> + * (2) the active data region of each frame ends at dsp_vact_end
>> + * (3) we should program this same number (dsp_vact_end) into dsp_line_frag_num,
>> + *      to get "LINE_FLAG" interrupt at the end of the active on screen data.
>> + *
>> + * VOP_INTR_CTRL0.dsp_line_frag_num = VOP_DSP_VACT_ST_END.dsp_vact_end
>> + * Interrupts
>> + * LINE_FLAG -------------------------------+
>> + * FRAME_SYNC ----+                         |
>> + *                |                         |
>> + *                v                         v
>> + *                | Vsync | Vbp |  Vactive  | Vfp |
>> + *                        ^     ^           ^     ^
>> + *                        |     |           |     |
>> + *                        |     |           |     |
>> + * dsp_vs_end ------------+     |           |     |   VOP_DSP_VTOTAL_VS_END
>> + * dsp_vact_start --------------+           |     |   VOP_DSP_VACT_ST_END
>> + * dsp_vact_end ----------------------------+     |   VOP_DSP_VACT_ST_END
>> + * dsp_total -------------------------------------+   VOP_DSP_VTOTAL_VS_END
>> + */
>> +static bool vop_line_flag_irq_is_enabled(struct vop *vop)
>> +{
>> +	uint32_t line_flag_irq;
>> +	unsigned long flags;
>> +
>> +	spin_lock_irqsave(&vop->irq_lock, flags);
>> +
>> +	line_flag_irq = VOP_INTR_GET_TYPE(vop, enable, LINE_FLAG_INTR);
>> +
>> +	spin_unlock_irqrestore(&vop->irq_lock, flags);
>> +
>> +	return !!line_flag_irq;
>> +}
>> +
>> +static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
>> +{
>> +	unsigned long flags;
>> +
>> +	if (WARN_ON(!vop->is_enabled))
>> +		return;
>> +
>> +	spin_lock_irqsave(&vop->irq_lock, flags);
>> +
>> +	VOP_CTRL_SET(vop, line_flag_num[0], line_num);
>> +	VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
>> +
>> +	spin_unlock_irqrestore(&vop->irq_lock, flags);
>> +}
>> +
>> +static void vop_line_flag_irq_disable(struct vop *vop)
>> +{
>> +	unsigned long flags;
>> +
>> +	if (WARN_ON(!vop->is_enabled))
>> +		return;
>> +
>> +	spin_lock_irqsave(&vop->irq_lock, flags);
>> +
>> +	VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 0);
>> +
>> +	spin_unlock_irqrestore(&vop->irq_lock, flags);
>> +}
>> +
>>   static void vop_enable(struct drm_crtc *crtc)
>>   {
>>   	struct vop *vop = to_vop(crtc);
>> @@ -1157,6 +1224,13 @@ static irqreturn_t vop_isr(int irq, void *data)
>>   		ret = IRQ_HANDLED;
>>   	}
>>   
>> +	if (active_irqs & LINE_FLAG_INTR) {
>> +		if (!completion_done(&vop->line_flag_completion))
>> +			complete(&vop->line_flag_completion);
> You still have the completion_done/complete here.
>
> If you just call complete() without checking if it's done, you'll remove the
> race I mentioned in my first review.

Ah, forget to update this one, will send a new v4.1 patch to fix it.

Thanks a lot,
- Yakir

> Sean
>
>> +		active_irqs &= ~LINE_FLAG_INTR;
>> +		ret = IRQ_HANDLED;
>> +	}
>> +
>>   	if (active_irqs & FS_INTR) {
>>   		drm_crtc_handle_vblank(crtc);
>>   		vop_handle_vblank(vop);
>> @@ -1255,6 +1329,7 @@ static int vop_create_crtc(struct vop *vop)
>>   
>>   	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);
>>   
>> @@ -1411,6 +1486,49 @@ static void vop_win_init(struct vop *vop)
>>   	}
>>   }
>>   
>> +/**
>> + * rockchip_drm_wait_line_flag - acqiure the give line flag event
>> + * @crtc: CRTC to enable line flag
>> + * @line_num: interested line number
>> + * @mstimeout: millisecond for timeout
>> + *
>> + * Driver would hold here until the interested line flag interrupt have
>> + * happened or timeout to wait.
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
>> +				unsigned int mstimeout)
>> +{
>> +	struct vop *vop = to_vop(crtc);
>> +	unsigned long jiffies_left;
>> +
>> +	if (!crtc || !vop->is_enabled)
>> +		return -ENODEV;
>> +
>> +	if (line_num > crtc->mode.vtotal || mstimeout <= 0)
>> +		return -EINVAL;
>> +
>> +	if (vop_line_flag_irq_is_enabled(vop))
>> +		return -EBUSY;
>> +
>> +	reinit_completion(&vop->line_flag_completion);
>> +	vop_line_flag_irq_enable(vop, line_num);
>> +
>> +	jiffies_left = wait_for_completion_timeout(&vop->line_flag_completion,
>> +						   msecs_to_jiffies(mstimeout));
>> +	vop_line_flag_irq_disable(vop);
>> +
>> +	if (jiffies_left == 0) {
>> +		dev_err(vop->dev, "Timeout waiting for IRQ\n");
>> +		return -ETIMEDOUT;
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(rockchip_drm_wait_line_flag);
>> +
>>   static int vop_bind(struct device *dev, struct device *master, void *data)
>>   {
>>   	struct platform_device *pdev = to_platform_device(dev);
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> index ff4f52e..1dbc526 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> @@ -61,6 +61,8 @@ struct vop_ctrl {
>>   	struct vop_reg hpost_st_end;
>>   	struct vop_reg vpost_st_end;
>>   
>> +	struct vop_reg line_flag_num[2];
>> +
>>   	struct vop_reg cfg_done;
>>   };
>>   
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> index 6f42e56..eea8427 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> @@ -122,6 +122,7 @@ static const struct vop_ctrl rk3036_ctrl_data = {
>>   	.hact_st_end = VOP_REG(RK3036_DSP_HACT_ST_END, 0x1fff1fff, 0),
>>   	.vtotal_pw = VOP_REG(RK3036_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
>>   	.vact_st_end = VOP_REG(RK3036_DSP_VACT_ST_END, 0x1fff1fff, 0),
>> +	.line_flag_num[0] = VOP_REG(RK3036_INT_STATUS, 0xfff, 12),
>>   	.cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
>>   };
>>   
>> @@ -221,6 +222,7 @@ static const struct vop_ctrl rk3288_ctrl_data = {
>>   	.vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
>>   	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>>   	.vpost_st_end = VOP_REG(RK3288_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
>> +	.line_flag_num[0] = VOP_REG(RK3288_INTR_CTRL0, 0x1fff, 12),
>>   	.cfg_done = VOP_REG(RK3288_REG_CFG_DONE, 0x1, 0),
>>   };
>>   
>> @@ -299,6 +301,8 @@ static const struct vop_ctrl rk3399_ctrl_data = {
>>   	.vact_st_end = VOP_REG(RK3399_DSP_VACT_ST_END, 0x1fff1fff, 0),
>>   	.hpost_st_end = VOP_REG(RK3399_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>>   	.vpost_st_end = VOP_REG(RK3399_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
>> +	.line_flag_num[0] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 0),
>> +	.line_flag_num[1] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 16),
>>   	.cfg_done = VOP_REG_MASK(RK3399_REG_CFG_DONE, 0x1, 0),
>>   };
>>   
>> -- 
>> 1.9.1
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/4] drm/rockchip: add an common abstracted PSR driver
  2016-07-14 15:14   ` Sean Paul
@ 2016-07-15  1:43     ` Yakir Yang
  2016-07-24  7:14     ` Yakir Yang
  1 sibling, 0 replies; 21+ messages in thread
From: Yakir Yang @ 2016-07-15  1:43 UTC (permalink / raw)
  To: Sean Paul
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Krzysztof Kozlowski, linux-samsung-soc, linux-kernel,
	linux-rockchip, daniel.vetter, emil.l.velikov, dianders,
	dri-devel, Tomasz Figa, Javier Martinez Canillas,
	Stéphane Marchesin, Thierry Reding, Dan Carpenter

Sean,

On 07/14/2016 11:14 PM, Sean Paul wrote:
> On Thu, Jul 14, 2016 at 12:15:49PM +0800, Yakir Yang wrote:
>> The PSR driver have exported four symbols for specific device driver:
>> - rockchip_drm_psr_register()
>> - rockchip_drm_psr_unregister()
>> - rockchip_drm_psr_enable()
>> - rockchip_drm_psr_disable()
>> - rockchip_drm_psr_flush()
>>
>> Encoder driver should call the register/unregister interfaces to hook
>> itself into common PSR driver, encoder have implement the 'psr_set'
>> callback which use the set PSR state in hardware side.
>>
>> Crtc driver would call the enable/disable interfaces when vblank is
>> enable/disable, after that the common PSR driver would call the encoder
>> registered callback to set the PSR state.
>>
>> Fb driver would call the flush interface in 'fb->dirty' callback, this
>> helper function would force all PSR enabled encoders to exit from PSR
>> for 3 seconds.
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>
> I still don't think it's a good idea to pull this out into a separate PSR
> driver, but I suppose we can integrate it at a later time if it becomes
> cumbersome.
>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>

Thanks :-D

- Yakir

>
>> ---
>> Changes in v4:
>> - Tuck the global "psr_list" & "psr_list_mutex" in struct rockchip_drm_private. (Sean)
>> - Move the access of "psr->state" under "psr->state_mutex"'s protect. (Sean)
>> - Let "psr->state = PSR_FLUSH" under "psr->state_mutex"'s protect. (Sean)
>> - Collect psr_enable() and psr_disable() into psr_set_state()
>> - s/5\ second/PSR_FLUSH_TIMEOUT/ (Sean)
>> - Flush the psr callback in vop_crtc_disable(). (Stéphane, reviewed in Google gerrit)
>>      [https://chromium-review.googlesource.com/#/c/349084/6/drivers/gpu/drm/rockchip/rockchip_drm_vop.c@475]
>> - Add the missing file head with license. (Stéphane, reviewed in Google gerrit)
>>      [https://chromium-review.googlesource.com/#/c/357563/1/drivers/gpu/drm/rockchip/rockchip_drm_psr.h@3]
>>
>> Changes in v3:
>> - split the psr flow into an common abstracted PSR driver
>> - implement the 'fb->dirty' callback function (Daniel)
>> - avoid to use notify to acqiure for vact event (Daniel)
>> - remove psr_active() callback which introduce in v2
>>
>> Changes in v2: None
>>
>>   drivers/gpu/drm/rockchip/Makefile           |   2 +-
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.c |   4 +
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   3 +
>>   drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  12 ++
>>   drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 223 ++++++++++++++++++++++++++++
>>   drivers/gpu/drm/rockchip/rockchip_drm_psr.h |  26 ++++
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  29 ++++
>>   7 files changed, 298 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.c
>>   create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.h
>>
>> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
>> index 05d0713..9746365 100644
>> --- a/drivers/gpu/drm/rockchip/Makefile
>> +++ b/drivers/gpu/drm/rockchip/Makefile
>> @@ -3,7 +3,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_vop.o
>> +		rockchip_drm_gem.o rockchip_drm_psr.o rockchip_drm_vop.o
>>   rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>>   
>>   obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> index d665fb0..26c12b3 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> @@ -156,6 +156,9 @@ static int rockchip_drm_bind(struct device *dev)
>>   
>>   	drm_dev->dev_private = private;
>>   
>> +	INIT_LIST_HEAD(&private->psr_list);
>> +	mutex_init(&private->psr_list_mutex);
>> +
>>   	drm_mode_config_init(drm_dev);
>>   
>>   	rockchip_drm_mode_config_init(drm_dev);
>> @@ -218,6 +221,7 @@ static int rockchip_drm_bind(struct device *dev)
>>   
>>   	if (is_support_iommu)
>>   		arm_iommu_release_mapping(mapping);
>> +
>>   	return 0;
>>   err_fbdev_fini:
>>   	rockchip_drm_fbdev_fini(drm_dev);
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> index 239b830..9c34c9e 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> @@ -61,6 +61,9 @@ struct rockchip_drm_private {
>>   	struct drm_gem_object *fbdev_bo;
>>   	const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
>>   	struct drm_atomic_state *state;
>> +
>> +	struct list_head psr_list;
>> +	struct mutex psr_list_mutex;
>>   };
>>   
>>   int rockchip_register_crtc_funcs(struct drm_crtc *crtc,
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> index 20f12bc..36afd9c 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> @@ -21,6 +21,7 @@
>>   
>>   #include "rockchip_drm_drv.h"
>>   #include "rockchip_drm_gem.h"
>> +#include "rockchip_drm_psr.h"
>>   
>>   #define to_rockchip_fb(x) container_of(x, struct rockchip_drm_fb, fb)
>>   
>> @@ -66,9 +67,20 @@ static int rockchip_drm_fb_create_handle(struct drm_framebuffer *fb,
>>   				     rockchip_fb->obj[0], handle);
>>   }
>>   
>> +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(fb->dev);
>> +	return 0;
>> +}
>> +
>>   static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
>>   	.destroy	= rockchip_drm_fb_destroy,
>>   	.create_handle	= rockchip_drm_fb_create_handle,
>> +	.dirty		= rockchip_drm_fb_dirty,
>>   };
>>   
>>   static struct rockchip_drm_fb *
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
>> new file mode 100644
>> index 0000000..e79ea18
>> --- /dev/null
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
>> @@ -0,0 +1,223 @@
>> +/*
>> + * 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_crtc_helper.h>
>> +
>> +#include "rockchip_drm_drv.h"
>> +#include "rockchip_drm_psr.h"
>> +
>> +#define PSR_FLUSH_TIMEOUT	msecs_to_jiffies(3000) /* 3 seconds */
>> +
>> +static LIST_HEAD(psr_list);
>> +static DEFINE_MUTEX(psr_list_mutex);
>> +
>> +enum psr_state {
>> +	PSR_FLUSH,
>> +	PSR_ENABLE,
>> +	PSR_DISABLE,
>> +};
>> +
>> +struct psr_drv {
>> +	struct list_head list;
>> +	enum psr_state state;
>> +	struct mutex state_mutex;
>> +
>> +	struct timer_list flush_timer;
>> +
>> +	struct drm_encoder *encoder;
>> +	void (*set)(struct drm_encoder *encoder, bool enable);
>> +};
>> +
>> +static struct psr_drv *find_psr_by_crtc(struct drm_crtc *crtc)
>> +{
>> +	struct rockchip_drm_private *drm_drv = crtc->dev->dev_private;
>> +	struct psr_drv *psr;
>> +
>> +	mutex_lock(&drm_drv->psr_list_mutex);
>> +	list_for_each_entry(psr, &drm_drv->psr_list, list) {
>> +		if (psr->encoder->crtc == crtc) {
>> +			mutex_unlock(&drm_drv->psr_list_mutex);
>> +			return psr;
>> +		}
>> +	}
>> +	mutex_unlock(&drm_drv->psr_list_mutex);
>> +
>> +	return ERR_PTR(-ENODEV);
>> +}
>> +
>> +static void psr_set_state(struct psr_drv *psr, enum psr_state state)
>> +{
>> +	mutex_lock(&psr->state_mutex);
>> +
>> +	if (psr->state == state) {
>> +		mutex_unlock(&psr->state_mutex);
>> +		return;
>> +	}
>> +
>> +	psr->state = state;
>> +	switch (state) {
>> +	case PSR_ENABLE:
>> +		psr->set(psr->encoder, true);
>> +		break;
>> +
>> +	case PSR_DISABLE:
>> +	case PSR_FLUSH:
>> +		psr->set(psr->encoder, false);
>> +		break;
>> +	};
>> +
>> +	mutex_unlock(&psr->state_mutex);
>> +}
>> +
>> +static void psr_flush_handler(unsigned long data)
>> +{
>> +	struct psr_drv *psr = (struct psr_drv *)data;
>> +
>> +	if (!psr || psr->state != PSR_FLUSH)
>> +		return;
>> +
>> +	psr_set_state(psr, PSR_ENABLE);
>> +}
>> +
>> +/**
>> + * rockchip_drm_psr_enable - enable the encoder PSR which bind to given CRTC
>> + * @crtc: CRTC to obtain the PSR encoder
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int rockchip_drm_psr_enable(struct drm_crtc *crtc)
>> +{
>> +	struct psr_drv *psr = find_psr_by_crtc(crtc);
>> +
>> +	if (IS_ERR(psr))
>> +		return PTR_ERR(psr);
>> +
>> +	psr_set_state(psr, PSR_ENABLE);
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(rockchip_drm_psr_enable);
>> +
>> +/**
>> + * rockchip_drm_psr_disable - disable the encoder PSR which bind to given CRTC
>> + * @crtc: CRTC to obtain the PSR encoder
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int rockchip_drm_psr_disable(struct drm_crtc *crtc)
>> +{
>> +	struct psr_drv *psr = find_psr_by_crtc(crtc);
>> +
>> +	if (IS_ERR(psr))
>> +		return PTR_ERR(psr);
>> +
>> +	psr_set_state(psr, PSR_DISABLE);
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(rockchip_drm_psr_disable);
>> +
>> +/**
>> + * rockchip_drm_psr_flush - 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(struct drm_device *dev)
>> +{
>> +	struct rockchip_drm_private *drm_drv = dev->dev_private;
>> +	struct psr_drv *psr;
>> +
>> +	mutex_lock(&drm_drv->psr_list_mutex);
>> +	list_for_each_entry(psr, &drm_drv->psr_list, list) {
>> +		if (psr->state == PSR_DISABLE)
>> +			continue;
>> +
>> +		mod_timer(&psr->flush_timer,
>> +			  round_jiffies_up(jiffies + PSR_FLUSH_TIMEOUT));
>> +
>> +		psr_set_state(psr, PSR_FLUSH);
>> +	}
>> +	mutex_unlock(&drm_drv->psr_list_mutex);
>> +}
>> +EXPORT_SYMBOL(rockchip_drm_psr_flush);
>> +
>> +/**
>> + * rockchip_drm_psr_register - register encoder to psr driver
>> + * @encoder: encoder that obtain the PSR function
>> + * @psr_set: call back to set PSR state
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int rockchip_drm_psr_register(struct drm_encoder *encoder,
>> +			void (*psr_set)(struct drm_encoder *, bool enable))
>> +{
>> +	struct rockchip_drm_private *drm_drv = encoder->dev->dev_private;
>> +	struct psr_drv *psr;
>> +
>> +	if (!encoder || !psr_set)
>> +		return -EINVAL;
>> +
>> +	psr = kzalloc(sizeof(struct psr_drv), GFP_KERNEL);
>> +	if (!psr)
>> +		return -ENOMEM;
>> +
>> +	setup_timer(&psr->flush_timer, psr_flush_handler, (unsigned long)psr);
>> +
>> +	mutex_init(&psr->state_mutex);
>> +
>> +	psr->state = PSR_DISABLE;
>> +	psr->encoder = encoder;
>> +	psr->set = psr_set;
>> +
>> +	mutex_lock(&drm_drv->psr_list_mutex);
>> +	list_add_tail(&psr->list, &drm_drv->psr_list);
>> +	mutex_unlock(&drm_drv->psr_list_mutex);
>> +
>> +	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
>> + *
>> + * 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;
>> +
>> +	mutex_lock(&drm_drv->psr_list_mutex);
>> +	list_for_each_entry(psr, &drm_drv->psr_list, list) {
>> +		if (psr->encoder == encoder) {
>> +			del_timer(&psr->flush_timer);
>> +			list_del(&psr->list);
>> +			kfree(psr);
>> +		}
>> +	}
>> +	mutex_unlock(&drm_drv->psr_list_mutex);
>> +}
>> +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
>> new file mode 100644
>> index 0000000..c35b688
>> --- /dev/null
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h
>> @@ -0,0 +1,26 @@
>> +/*
>> + * 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(struct drm_device *dev);
>> +int rockchip_drm_psr_enable(struct drm_crtc *crtc);
>> +int rockchip_drm_psr_disable(struct drm_crtc *crtc);
>> +
>> +int rockchip_drm_psr_register(struct drm_encoder *encoder,
>> +			void (*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 69d32f1..e702fb3 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> @@ -34,6 +34,7 @@
>>   #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"
>>   
>>   #define __REG_SET_RELAXED(x, off, mask, shift, v, write_mask) \
>> @@ -87,6 +88,8 @@
>>   #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)
>>   
>> +#define VOP_PSR_SET_DELAY_TIME		msecs_to_jiffies(10)
>> +
>>   struct vop_plane_state {
>>   	struct drm_plane_state base;
>>   	int format;
>> @@ -121,6 +124,9 @@ struct vop {
>>   	/* protected by dev->event_lock */
>>   	struct drm_pending_vblank_event *event;
>>   
>> +	bool psr_enabled;
>> +	struct delayed_work psr_work;
>> +
>>   	struct completion line_flag_completion;
>>   
>>   	const struct vop_data *data;
>> @@ -629,6 +635,9 @@ static void vop_crtc_disable(struct drm_crtc *crtc)
>>   
>>   		crtc->state->event = NULL;
>>   	}
>> +
>> +	vop->psr_enabled = false;
>> +	INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
>>   }
>>   
>>   static void vop_plane_destroy(struct drm_plane *plane)
>> @@ -923,6 +932,16 @@ static const struct drm_plane_funcs vop_plane_funcs = {
>>   	.atomic_destroy_state = vop_atomic_plane_destroy_state,
>>   };
>>   
>> +static void vop_psr_work(struct work_struct *work)
>> +{
>> +	struct vop *vop = container_of(work, typeof(*vop), psr_work.work);
>> +
>> +	if (vop->psr_enabled)
>> +		rockchip_drm_psr_enable(&vop->crtc);
>> +	else
>> +		rockchip_drm_psr_disable(&vop->crtc);
>> +}
>> +
>>   static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
>>   {
>>   	struct vop *vop = to_vop(crtc);
>> @@ -937,6 +956,9 @@ static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
>>   
>>   	spin_unlock_irqrestore(&vop->irq_lock, flags);
>>   
>> +	vop->psr_enabled = false;
>> +	schedule_delayed_work(&vop->psr_work, VOP_PSR_SET_DELAY_TIME);
>> +
>>   	return 0;
>>   }
>>   
>> @@ -953,6 +975,9 @@ 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);
>> +
>> +	vop->psr_enabled = true;
>> +	schedule_delayed_work(&vop->psr_work, VOP_PSR_SET_DELAY_TIME);
>>   }
>>   
>>   static void vop_crtc_wait_for_update(struct drm_crtc *crtc)
>> @@ -1597,6 +1622,10 @@ static int vop_bind(struct device *dev, struct device *master, void *data)
>>   		return ret;
>>   
>>   	pm_runtime_enable(&pdev->dev);
>> +
>> +	vop->psr_enabled = false;
>> +	INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
>> +
>>   	return 0;
>>   }
>>   
>> -- 
>> 1.9.1
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 4/4] drm/rockchip: analogix_dp: implement PSR function
  2016-07-14 15:26   ` Sean Paul
@ 2016-07-15  5:45     ` Yakir Yang
  0 siblings, 0 replies; 21+ messages in thread
From: Yakir Yang @ 2016-07-15  5:45 UTC (permalink / raw)
  To: Sean Paul
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Krzysztof Kozlowski, linux-samsung-soc, linux-kernel,
	linux-rockchip, daniel.vetter, emil.l.velikov, dianders,
	dri-devel, Tomasz Figa, Javier Martinez Canillas,
	Stéphane Marchesin, Thierry Reding, Dan Carpenter

Sean,

On 07/14/2016 11:26 PM, Sean Paul wrote:
> On Thu, Jul 14, 2016 at 12:15:58PM +0800, Yakir Yang wrote:
>> Alway enable the PSR function for Rockchip analogix_dp driver. If panel
>> don't support PSR, then the core analogix_dp would ignore this setting.
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>

Thanks :-D

- Yakir

>> ---
>> Changes in v4:
>> - Return 'void' instead of 'int' in analogix_dp_psr_set(). (Sean)
>> - Pull the 10ms delay time out into a #define. (Sean)
>> - Improved the code of analogix_dp_psr_work(). (Sean)
>> - Indented with spaces for new numbers in rockchip_dp_device struct. (Stéphane, reviewed at Google gerrit)
>>      [https://chromium-review.googlesource.com/#/c/349085/33/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c@83]
>>
>> Changes in v3:
>> - split the common psr logic into a seperate driver, make this to a
>>    simple sub-psr device driver.
>>
>> Changes in v2:
>> - remove vblank notify out (Daniel)
>> - create a psr_active() callback in vop data struct.
>>
>>   drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 57 +++++++++++++++++++++++++
>>   1 file changed, 57 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> index e81e19a..aa916f4 100644
>> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> @@ -32,6 +32,7 @@
>>   #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
>> @@ -41,6 +42,9 @@
>>   
>>   #define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
>>   
>> +#define PSR_SET_DELAY_TIME		msecs_to_jiffies(10)
>> +#define PSR_WAIT_LINE_FLAG_TIMEOUT_MS	100
>> +
>>   #define to_dp(nm)	container_of(nm, struct rockchip_dp_device, nm)
>>   
>>   /**
>> @@ -68,11 +72,55 @@ struct rockchip_dp_device {
>>   	struct regmap            *grf;
>>   	struct reset_control     *rst;
>>   
>> +	struct delayed_work      psr_work;
>> +	unsigned int             psr_state;
>> +
>>   	const struct rockchip_dp_chip_data *data;
>>   
>>   	struct analogix_dp_plat_data plat_data;
>>   };
>>   
>> +static void analogix_dp_psr_set(struct drm_encoder *encoder, bool enabled)
>> +{
>> +	struct rockchip_dp_device *dp = to_dp(encoder);
>> +
>> +	dev_dbg(dp->dev, "%s PSR...\n", enabled ? "Entry" : "Exit");
>> +
>> +	if (enabled)
>> +		dp->psr_state = EDP_VSC_PSR_STATE_ACTIVE;
>> +	else
>> +		dp->psr_state = ~EDP_VSC_PSR_STATE_ACTIVE;
>> +
>> +	schedule_delayed_work(&dp->psr_work, PSR_SET_DELAY_TIME);
>> +}
>> +
>> +static void analogix_dp_psr_work(struct work_struct *work)
>> +{
>> +	struct rockchip_dp_device *dp =
>> +				container_of(work, typeof(*dp), psr_work.work);
>> +	struct drm_crtc *crtc = dp->encoder.crtc;
>> +	int psr_state = dp->psr_state;
>> +	int vact_end;
>> +	int ret;
>> +
>> +	if (!crtc)
>> +		return;
>> +
>> +	vact_end = crtc->mode.vtotal - crtc->mode.vsync_start + crtc->mode.vdisplay;
>> +
>> +	ret = rockchip_drm_wait_line_flag(dp->encoder.crtc, vact_end,
>> +					  PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
>> +	if (ret) {
>> +		dev_err(dp->dev, "line flag interrupt did not arrive\n");
>> +		return;
>> +	}
>> +
>> +	if (psr_state == EDP_VSC_PSR_STATE_ACTIVE)
>> +		analogix_dp_enable_psr(dp->dev);
>> +	else
>> +		analogix_dp_disable_psr(dp->dev);
>> +}
>> +
>>   static int rockchip_dp_pre_init(struct rockchip_dp_device *dp)
>>   {
>>   	reset_control_assert(dp->rst);
>> @@ -340,12 +388,21 @@ static int rockchip_dp_bind(struct device *dev, struct device *master,
>>   	dp->plat_data.power_off = rockchip_dp_powerdown;
>>   	dp->plat_data.get_modes = rockchip_dp_get_modes;
>>   
>> +	dp->psr_state = ~EDP_VSC_PSR_STATE_ACTIVE;
>> +	INIT_DELAYED_WORK(&dp->psr_work, analogix_dp_psr_work);
>> +
>> +	rockchip_drm_psr_register(&dp->encoder, analogix_dp_psr_set);
>> +
>>   	return analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
>>   }
>>   
>>   static void rockchip_dp_unbind(struct device *dev, struct device *master,
>>   			       void *data)
>>   {
>> +	struct rockchip_dp_device *dp = dev_get_drvdata(dev);
>> +
>> +	rockchip_drm_psr_unregister(&dp->encoder);
>> +
>>   	return analogix_dp_unbind(dev, master, data);
>>   }
>>   
>> -- 
>> 1.9.1
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4.1 1/4] drm/rockchip: vop: export line flag function
  2016-07-14  4:15 [PATCH v4 0/4] Add PSR function support for Analogix/Rockchip DP Yakir Yang
                   ` (3 preceding siblings ...)
  2016-07-14  4:15 ` [PATCH v4 4/4] drm/rockchip: analogix_dp: implement PSR function Yakir Yang
@ 2016-07-15 10:55 ` Yakir Yang
  2016-07-15 13:04   ` Sean Paul
  2016-07-15 10:55 ` [PATCH v4.1 3/4] drm/bridge: analogix_dp: add the PSR function support Yakir Yang
  5 siblings, 1 reply; 21+ messages in thread
From: Yakir Yang @ 2016-07-15 10:55 UTC (permalink / raw)
  To: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner
  Cc: Javier Martinez Canillas, Stéphane Marchesin, Sean Paul,
	Tomasz Figa, David Airlie, daniel.vetter, Thierry Reding,
	dianders, Krzysztof Kozlowski, emil.l.velikov, Dan Carpenter,
	Yakir Yang, linux-kernel, dri-devel, linux-samsung-soc,
	linux-rockchip

VOP have integrated a hardware counter which indicate the exact display
line that vop is scanning. And if we're interested in a specific line,
we can set the line number to vop line_flag register, and then vop would
generate a line_flag interrupt for it.

For example eDP PSR function is interested in the vertical blanking
period, then driver could set the line number to zero.

This patch have exported a symbol that allow other driver to listen the
line flag event with given timeout limit:
-  rockchip_drm_wait_line_flag()

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v4.1:
- Remove the completion_done() check in irq handler (Sean)

Changes in v4:
- Avoid the weird behavior in rockchip_drm_wait_line_flag(). (Sean)
- Make line_flag_num_x to an array. (Sean)
- Remove the unused vop_cfg_done() in vop_line_flag_irq_enable(). (Stephane, reviewed in Google gerrit)
    [https://chromium-review.googlesource.com/#/c/349084/33/drivers/gpu/drm/rockchip/rockchip_drm_vop.c@466]

Changes in v3:
- Export the 'rockchip_drm_wait_line_flag' symbol, and document it.
- Add 'line_flag_num_0' for RK3288/RK3036
- Remove the notify for waiting line_flag event (Daniel)

Changes in v2:
- Introduce in v2, split VOP line flag changes out

 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   3 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 117 ++++++++++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h |   2 +
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   4 +
 4 files changed, 126 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index ea39329..239b830 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -70,4 +70,7 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
 				   struct device *dev);
 void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 				    struct device *dev);
+int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
+				unsigned int mstimeout);
+
 #endif /* _ROCKCHIP_DRM_DRV_H_ */
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index c8a62a8..8a4f36e 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -121,6 +121,8 @@ struct vop {
 	/* protected by dev->event_lock */
 	struct drm_pending_vblank_event *event;
 
+	struct completion line_flag_completion;
+
 	const struct vop_data *data;
 
 	uint32_t *regsbak;
@@ -431,6 +433,71 @@ static void vop_dsp_hold_valid_irq_disable(struct vop *vop)
 	spin_unlock_irqrestore(&vop->irq_lock, flags);
 }
 
+/*
+ * (1) each frame starts at the start of the Vsync pulse which is signaled by
+ *     the "FRAME_SYNC" interrupt.
+ * (2) the active data region of each frame ends at dsp_vact_end
+ * (3) we should program this same number (dsp_vact_end) into dsp_line_frag_num,
+ *      to get "LINE_FLAG" interrupt at the end of the active on screen data.
+ *
+ * VOP_INTR_CTRL0.dsp_line_frag_num = VOP_DSP_VACT_ST_END.dsp_vact_end
+ * Interrupts
+ * LINE_FLAG -------------------------------+
+ * FRAME_SYNC ----+                         |
+ *                |                         |
+ *                v                         v
+ *                | Vsync | Vbp |  Vactive  | Vfp |
+ *                        ^     ^           ^     ^
+ *                        |     |           |     |
+ *                        |     |           |     |
+ * dsp_vs_end ------------+     |           |     |   VOP_DSP_VTOTAL_VS_END
+ * dsp_vact_start --------------+           |     |   VOP_DSP_VACT_ST_END
+ * dsp_vact_end ----------------------------+     |   VOP_DSP_VACT_ST_END
+ * dsp_total -------------------------------------+   VOP_DSP_VTOTAL_VS_END
+ */
+static bool vop_line_flag_irq_is_enabled(struct vop *vop)
+{
+	uint32_t line_flag_irq;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vop->irq_lock, flags);
+
+	line_flag_irq = VOP_INTR_GET_TYPE(vop, enable, LINE_FLAG_INTR);
+
+	spin_unlock_irqrestore(&vop->irq_lock, flags);
+
+	return !!line_flag_irq;
+}
+
+static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
+{
+	unsigned long flags;
+
+	if (WARN_ON(!vop->is_enabled))
+		return;
+
+	spin_lock_irqsave(&vop->irq_lock, flags);
+
+	VOP_CTRL_SET(vop, line_flag_num[0], line_num);
+	VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
+
+	spin_unlock_irqrestore(&vop->irq_lock, flags);
+}
+
+static void vop_line_flag_irq_disable(struct vop *vop)
+{
+	unsigned long flags;
+
+	if (WARN_ON(!vop->is_enabled))
+		return;
+
+	spin_lock_irqsave(&vop->irq_lock, flags);
+
+	VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 0);
+
+	spin_unlock_irqrestore(&vop->irq_lock, flags);
+}
+
 static void vop_enable(struct drm_crtc *crtc)
 {
 	struct vop *vop = to_vop(crtc);
@@ -1157,6 +1224,12 @@ static irqreturn_t vop_isr(int irq, void *data)
 		ret = IRQ_HANDLED;
 	}
 
+	if (active_irqs & LINE_FLAG_INTR) {
+		complete(&vop->line_flag_completion);
+		active_irqs &= ~LINE_FLAG_INTR;
+		ret = IRQ_HANDLED;
+	}
+
 	if (active_irqs & FS_INTR) {
 		drm_crtc_handle_vblank(crtc);
 		vop_handle_vblank(vop);
@@ -1255,6 +1328,7 @@ static int vop_create_crtc(struct vop *vop)
 
 	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);
 
@@ -1411,6 +1485,49 @@ static void vop_win_init(struct vop *vop)
 	}
 }
 
+/**
+ * rockchip_drm_wait_line_flag - acqiure the give line flag event
+ * @crtc: CRTC to enable line flag
+ * @line_num: interested line number
+ * @mstimeout: millisecond for timeout
+ *
+ * Driver would hold here until the interested line flag interrupt have
+ * happened or timeout to wait.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
+				unsigned int mstimeout)
+{
+	struct vop *vop = to_vop(crtc);
+	unsigned long jiffies_left;
+
+	if (!crtc || !vop->is_enabled)
+		return -ENODEV;
+
+	if (line_num > crtc->mode.vtotal || mstimeout <= 0)
+		return -EINVAL;
+
+	if (vop_line_flag_irq_is_enabled(vop))
+		return -EBUSY;
+
+	reinit_completion(&vop->line_flag_completion);
+	vop_line_flag_irq_enable(vop, line_num);
+
+	jiffies_left = wait_for_completion_timeout(&vop->line_flag_completion,
+						   msecs_to_jiffies(mstimeout));
+	vop_line_flag_irq_disable(vop);
+
+	if (jiffies_left == 0) {
+		dev_err(vop->dev, "Timeout waiting for IRQ\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(rockchip_drm_wait_line_flag);
+
 static int vop_bind(struct device *dev, struct device *master, void *data)
 {
 	struct platform_device *pdev = to_platform_device(dev);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index ff4f52e..1dbc526 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -61,6 +61,8 @@ struct vop_ctrl {
 	struct vop_reg hpost_st_end;
 	struct vop_reg vpost_st_end;
 
+	struct vop_reg line_flag_num[2];
+
 	struct vop_reg cfg_done;
 };
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 6f42e56..eea8427 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -122,6 +122,7 @@ static const struct vop_ctrl rk3036_ctrl_data = {
 	.hact_st_end = VOP_REG(RK3036_DSP_HACT_ST_END, 0x1fff1fff, 0),
 	.vtotal_pw = VOP_REG(RK3036_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
 	.vact_st_end = VOP_REG(RK3036_DSP_VACT_ST_END, 0x1fff1fff, 0),
+	.line_flag_num[0] = VOP_REG(RK3036_INT_STATUS, 0xfff, 12),
 	.cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
 };
 
@@ -221,6 +222,7 @@ static const struct vop_ctrl rk3288_ctrl_data = {
 	.vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
 	.hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
 	.vpost_st_end = VOP_REG(RK3288_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
+	.line_flag_num[0] = VOP_REG(RK3288_INTR_CTRL0, 0x1fff, 12),
 	.cfg_done = VOP_REG(RK3288_REG_CFG_DONE, 0x1, 0),
 };
 
@@ -299,6 +301,8 @@ static const struct vop_ctrl rk3399_ctrl_data = {
 	.vact_st_end = VOP_REG(RK3399_DSP_VACT_ST_END, 0x1fff1fff, 0),
 	.hpost_st_end = VOP_REG(RK3399_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
 	.vpost_st_end = VOP_REG(RK3399_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
+	.line_flag_num[0] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 0),
+	.line_flag_num[1] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 16),
 	.cfg_done = VOP_REG_MASK(RK3399_REG_CFG_DONE, 0x1, 0),
 };
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v4.1 3/4] drm/bridge: analogix_dp: add the PSR function support
  2016-07-14  4:15 [PATCH v4 0/4] Add PSR function support for Analogix/Rockchip DP Yakir Yang
                   ` (4 preceding siblings ...)
  2016-07-15 10:55 ` [PATCH v4.1 1/4] drm/rockchip: vop: export line flag function Yakir Yang
@ 2016-07-15 10:55 ` Yakir Yang
  2016-07-15 13:13   ` Sean Paul
  5 siblings, 1 reply; 21+ messages in thread
From: Yakir Yang @ 2016-07-15 10:55 UTC (permalink / raw)
  To: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner
  Cc: Javier Martinez Canillas, Stéphane Marchesin, Sean Paul,
	Tomasz Figa, David Airlie, daniel.vetter, Thierry Reding,
	dianders, Krzysztof Kozlowski, emil.l.velikov, Dan Carpenter,
	Yakir Yang, linux-kernel, dri-devel, linux-samsung-soc,
	linux-rockchip

The full name of PSR is Panel Self Refresh, panel device could refresh
itself with the hardware framebuffer in panel, this would make lots of
sense to save the power consumption.

This patch have exported two symbols for platform driver to implement
the PSR function in hardware side:
- analogix_dp_active_psr()
- analogix_dp_inactive_psr()

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
---
Changes in v4.1:
- Take use of existing edp_psr_vsc struct to swap HBx and DBx setting. (Sean)
- Remove PSR_VID_CRC_FLUSH setting analogix_dp_enable_psr_crc().
- Add comment about PBx magic numbers. (Sean)

Changes in v4:
- Downgrade the PSR version print message to debug level. (Sean)
- Return 'void' instead of 'int' in analogix_dp_enable_sink_psr(). (Sean)
- Delete the unused read dpcd operations in analogix_dp_enable_sink_psr(). (Sean)
- Delete the arbitrary usleep_range in analogix_dp_enable_psr_crc. (Sean).
- Clean up the hardcoded values in analogix_dp_send_psr_spd(). (Sean)
- Rename "active/inactive" to "enable/disable". (Sean, Dominik)
- Keep set the PSR_VID_CRC_FLUSH gate in analogix_dp_enable_psr_crc().

Changes in v3:
- split analogix_dp_enable_psr(), make it more clearly
    analogix_dp_detect_sink_psr()
    analogix_dp_enable_sink_psr()
- remove some nosie register setting comments

Changes in v2:
- introduce in v2, splite the common Analogix DP changes out

 drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 81 ++++++++++++++++++++++
 drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  5 ++
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 51 ++++++++++++++
 drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h  | 34 +++++++++
 include/drm/bridge/analogix_dp.h                   |  3 +
 5 files changed, 174 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 32715da..381b25e 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -97,6 +97,83 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
 	return 0;
 }
 
+int analogix_dp_enable_psr(struct device *dev)
+{
+	struct analogix_dp_device *dp = dev_get_drvdata(dev);
+	struct edp_vsc_psr psr_vsc;
+
+	if (!dp->psr_support)
+		return -EINVAL;
+
+	/* 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;
+
+	analogix_dp_send_psr_spd(dp, &psr_vsc);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
+
+int analogix_dp_disable_psr(struct device *dev)
+{
+	struct analogix_dp_device *dp = dev_get_drvdata(dev);
+	struct edp_vsc_psr psr_vsc;
+
+	if (!dp->psr_support)
+		return -EINVAL;
+
+	/* 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;
+
+	analogix_dp_send_psr_spd(dp, &psr_vsc);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
+
+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);
+	dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version);
+
+	return (psr_version & DP_PSR_IS_SUPPORTED) ? true : false;
+}
+
+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);
+	psr_en &= ~DP_PSR_ENABLE;
+	analogix_dp_write_byte_to_dpcd(dp, 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);
+
+	/* 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);
+
+	analogix_dp_enable_psr_crc(dp);
+}
+
 static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
 {
 	int i;
@@ -921,6 +998,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
 
 	/* Enable video */
 	analogix_dp_start_video(dp);
+
+	dp->psr_support = analogix_dp_detect_sink_psr(dp);
+	if (dp->psr_support)
+		analogix_dp_enable_sink_psr(dp);
 }
 
 int analogix_dp_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
index b456380..17fbcee 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
@@ -177,6 +177,7 @@ struct analogix_dp_device {
 	int			hpd_gpio;
 	bool                    force_hpd;
 	unsigned char           edid[EDID_BLOCK_LENGTH * 2];
+	bool			psr_support;
 
 	struct analogix_dp_plat_data *plat_data;
 };
@@ -278,4 +279,8 @@ int analogix_dp_is_video_stream_on(struct analogix_dp_device *dp);
 void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
 void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
 void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
+void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
+void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+			      struct edp_vsc_psr *vsc);
+
 #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 48030f0..52c1b6b 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
@@ -1322,3 +1322,54 @@ void analogix_dp_disable_scrambling(struct analogix_dp_device *dp)
 	reg |= SCRAMBLING_DISABLE;
 	writel(reg, dp->reg_base + ANALOGIX_DP_TRAINING_PTN_SET);
 }
+
+void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
+{
+	writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
+}
+
+void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
+			      struct edp_vsc_psr *vsc)
+{
+	unsigned int val;
+
+	/* don't send info frame */
+	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+	val &= ~IF_EN;
+	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+
+	/* configure single frame update mode */
+	writel(PSR_FRAME_UP_TYPE_BURST | PSR_CRC_SEL_HARDWARE,
+	       dp->reg_base + ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL);
+
+	/* configure VSC HB0~HB3 */
+	writel(vsc->sdp_header.HB0, dp->reg_base + ANALOGIX_DP_SPD_HB0);
+	writel(vsc->sdp_header.HB1, dp->reg_base + ANALOGIX_DP_SPD_HB1);
+	writel(vsc->sdp_header.HB2, dp->reg_base + ANALOGIX_DP_SPD_HB2);
+	writel(vsc->sdp_header.HB3, dp->reg_base + ANALOGIX_DP_SPD_HB3);
+
+	/* configure reused VSC PB0~PB3, magic number from vendor */
+	writel(0x00, dp->reg_base + ANALOGIX_DP_SPD_PB0);
+	writel(0x16, dp->reg_base + ANALOGIX_DP_SPD_PB1);
+	writel(0xCE, dp->reg_base + ANALOGIX_DP_SPD_PB2);
+	writel(0x5D, dp->reg_base + ANALOGIX_DP_SPD_PB3);
+
+	/* configure DB0 / DB1 values */
+	writel(vsc->DB0, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB0);
+	writel(vsc->DB1, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB1);
+
+	/* set reuse spd inforframe */
+	val = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_3);
+	val |= REUSE_SPD_EN;
+	writel(val, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_3);
+
+	/* mark info frame update */
+	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+	val = (val | IF_UP) & ~IF_EN;
+	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+
+	/* send info frame */
+	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+	val |= IF_EN;
+	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
+}
diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
index cdcc6c5..40200c6 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
@@ -22,6 +22,8 @@
 #define ANALOGIX_DP_VIDEO_CTL_8			0x3C
 #define ANALOGIX_DP_VIDEO_CTL_10		0x44
 
+#define ANALOGIX_DP_SPDIF_AUDIO_CTL_0		0xD8
+
 #define ANALOGIX_DP_PLL_REG_1			0xfc
 #define ANALOGIX_DP_PLL_REG_2			0x9e4
 #define ANALOGIX_DP_PLL_REG_3			0x9e8
@@ -30,6 +32,21 @@
 
 #define ANALOGIX_DP_PD				0x12c
 
+#define ANALOGIX_DP_IF_TYPE			0x244
+#define ANALOGIX_DP_IF_PKT_DB1			0x254
+#define ANALOGIX_DP_IF_PKT_DB2			0x258
+#define ANALOGIX_DP_SPD_HB0			0x2F8
+#define ANALOGIX_DP_SPD_HB1			0x2FC
+#define ANALOGIX_DP_SPD_HB2			0x300
+#define ANALOGIX_DP_SPD_HB3			0x304
+#define ANALOGIX_DP_SPD_PB0			0x308
+#define ANALOGIX_DP_SPD_PB1			0x30C
+#define ANALOGIX_DP_SPD_PB2			0x310
+#define ANALOGIX_DP_SPD_PB3			0x314
+#define ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL	0x318
+#define ANALOGIX_DP_VSC_SHADOW_DB0		0x31C
+#define ANALOGIX_DP_VSC_SHADOW_DB1		0x320
+
 #define ANALOGIX_DP_LANE_MAP			0x35C
 
 #define ANALOGIX_DP_ANALOG_CTL_1		0x370
@@ -103,6 +120,8 @@
 
 #define ANALOGIX_DP_SOC_GENERAL_CTL		0x800
 
+#define ANALOGIX_DP_CRC_CON			0x890
+
 /* ANALOGIX_DP_TX_SW_RESET */
 #define RESET_DP_TX				(0x1 << 0)
 
@@ -151,6 +170,7 @@
 #define VID_CHK_UPDATE_TYPE_SHIFT		(4)
 #define VID_CHK_UPDATE_TYPE_1			(0x1 << 4)
 #define VID_CHK_UPDATE_TYPE_0			(0x0 << 4)
+#define REUSE_SPD_EN				(0x1 << 3)
 
 /* ANALOGIX_DP_VIDEO_CTL_8 */
 #define VID_HRES_TH(x)				(((x) & 0xf) << 4)
@@ -167,6 +187,12 @@
 #define REF_CLK_27M				(0x0 << 0)
 #define REF_CLK_MASK				(0x1 << 0)
 
+/* ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL */
+#define PSR_FRAME_UP_TYPE_BURST			(0x1 << 0)
+#define PSR_FRAME_UP_TYPE_SINGLE		(0x0 << 0)
+#define PSR_CRC_SEL_HARDWARE			(0x1 << 1)
+#define PSR_CRC_SEL_MANUALLY			(0x0 << 1)
+
 /* ANALOGIX_DP_LANE_MAP */
 #define LANE3_MAP_LOGIC_LANE_0			(0x0 << 6)
 #define LANE3_MAP_LOGIC_LANE_1			(0x1 << 6)
@@ -376,4 +402,12 @@
 #define VIDEO_MODE_SLAVE_MODE			(0x1 << 0)
 #define VIDEO_MODE_MASTER_MODE			(0x0 << 0)
 
+/* ANALOGIX_DP_PKT_SEND_CTL */
+#define IF_UP					(0x1 << 4)
+#define IF_EN					(0x1 << 0)
+
+/* ANALOGIX_DP_CRC_CON */
+#define PSR_VID_CRC_FLUSH			(0x1 << 2)
+#define PSR_VID_CRC_ENABLE			(0x1 << 0)
+
 #endif /* _ANALOGIX_DP_REG_H */
diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
index 261b86d..9cd8838 100644
--- a/include/drm/bridge/analogix_dp.h
+++ b/include/drm/bridge/analogix_dp.h
@@ -38,6 +38,9 @@ struct analogix_dp_plat_data {
 			 struct drm_connector *);
 };
 
+int analogix_dp_enable_psr(struct device *dev);
+int analogix_dp_disable_psr(struct device *dev);
+
 int analogix_dp_resume(struct device *dev);
 int analogix_dp_suspend(struct device *dev);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v4.1 1/4] drm/rockchip: vop: export line flag function
  2016-07-15 10:55 ` [PATCH v4.1 1/4] drm/rockchip: vop: export line flag function Yakir Yang
@ 2016-07-15 13:04   ` Sean Paul
  2016-07-16  2:30     ` Yakir Yang
  0 siblings, 1 reply; 21+ messages in thread
From: Sean Paul @ 2016-07-15 13:04 UTC (permalink / raw)
  To: Yakir Yang
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Javier Martinez Canillas, Stéphane Marchesin, Tomasz Figa,
	David Airlie, Daniel Vetter, Thierry Reding, Douglas Anderson,
	Krzysztof Kozlowski, Emil Velikov, Dan Carpenter,
	Linux Kernel Mailing List, dri-devel, linux-samsung-soc,
	linux-rockchip

On Fri, Jul 15, 2016 at 6:55 AM, Yakir Yang <ykk@rock-chips.com> wrote:
> VOP have integrated a hardware counter which indicate the exact display
> line that vop is scanning. And if we're interested in a specific line,
> we can set the line number to vop line_flag register, and then vop would
> generate a line_flag interrupt for it.
>
> For example eDP PSR function is interested in the vertical blanking
> period, then driver could set the line number to zero.
>
> This patch have exported a symbol that allow other driver to listen the
> line flag event with given timeout limit:
> -  rockchip_drm_wait_line_flag()
>
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>


Thanks for the update.

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
> Changes in v4.1:
> - Remove the completion_done() check in irq handler (Sean)
>
> Changes in v4:
> - Avoid the weird behavior in rockchip_drm_wait_line_flag(). (Sean)
> - Make line_flag_num_x to an array. (Sean)
> - Remove the unused vop_cfg_done() in vop_line_flag_irq_enable(). (Stephane, reviewed in Google gerrit)
>     [https://chromium-review.googlesource.com/#/c/349084/33/drivers/gpu/drm/rockchip/rockchip_drm_vop.c@466]
>
> Changes in v3:
> - Export the 'rockchip_drm_wait_line_flag' symbol, and document it.
> - Add 'line_flag_num_0' for RK3288/RK3036
> - Remove the notify for waiting line_flag event (Daniel)
>
> Changes in v2:
> - Introduce in v2, split VOP line flag changes out
>
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   3 +
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 117 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.h |   2 +
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   4 +
>  4 files changed, 126 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index ea39329..239b830 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -70,4 +70,7 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
>                                    struct device *dev);
>  void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>                                     struct device *dev);
> +int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
> +                               unsigned int mstimeout);
> +
>  #endif /* _ROCKCHIP_DRM_DRV_H_ */
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index c8a62a8..8a4f36e 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -121,6 +121,8 @@ struct vop {
>         /* protected by dev->event_lock */
>         struct drm_pending_vblank_event *event;
>
> +       struct completion line_flag_completion;
> +
>         const struct vop_data *data;
>
>         uint32_t *regsbak;
> @@ -431,6 +433,71 @@ static void vop_dsp_hold_valid_irq_disable(struct vop *vop)
>         spin_unlock_irqrestore(&vop->irq_lock, flags);
>  }
>
> +/*
> + * (1) each frame starts at the start of the Vsync pulse which is signaled by
> + *     the "FRAME_SYNC" interrupt.
> + * (2) the active data region of each frame ends at dsp_vact_end
> + * (3) we should program this same number (dsp_vact_end) into dsp_line_frag_num,
> + *      to get "LINE_FLAG" interrupt at the end of the active on screen data.
> + *
> + * VOP_INTR_CTRL0.dsp_line_frag_num = VOP_DSP_VACT_ST_END.dsp_vact_end
> + * Interrupts
> + * LINE_FLAG -------------------------------+
> + * FRAME_SYNC ----+                         |
> + *                |                         |
> + *                v                         v
> + *                | Vsync | Vbp |  Vactive  | Vfp |
> + *                        ^     ^           ^     ^
> + *                        |     |           |     |
> + *                        |     |           |     |
> + * dsp_vs_end ------------+     |           |     |   VOP_DSP_VTOTAL_VS_END
> + * dsp_vact_start --------------+           |     |   VOP_DSP_VACT_ST_END
> + * dsp_vact_end ----------------------------+     |   VOP_DSP_VACT_ST_END
> + * dsp_total -------------------------------------+   VOP_DSP_VTOTAL_VS_END
> + */
> +static bool vop_line_flag_irq_is_enabled(struct vop *vop)
> +{
> +       uint32_t line_flag_irq;
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&vop->irq_lock, flags);
> +
> +       line_flag_irq = VOP_INTR_GET_TYPE(vop, enable, LINE_FLAG_INTR);
> +
> +       spin_unlock_irqrestore(&vop->irq_lock, flags);
> +
> +       return !!line_flag_irq;
> +}
> +
> +static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
> +{
> +       unsigned long flags;
> +
> +       if (WARN_ON(!vop->is_enabled))
> +               return;
> +
> +       spin_lock_irqsave(&vop->irq_lock, flags);
> +
> +       VOP_CTRL_SET(vop, line_flag_num[0], line_num);
> +       VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
> +
> +       spin_unlock_irqrestore(&vop->irq_lock, flags);
> +}
> +
> +static void vop_line_flag_irq_disable(struct vop *vop)
> +{
> +       unsigned long flags;
> +
> +       if (WARN_ON(!vop->is_enabled))
> +               return;
> +
> +       spin_lock_irqsave(&vop->irq_lock, flags);
> +
> +       VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 0);
> +
> +       spin_unlock_irqrestore(&vop->irq_lock, flags);
> +}
> +
>  static void vop_enable(struct drm_crtc *crtc)
>  {
>         struct vop *vop = to_vop(crtc);
> @@ -1157,6 +1224,12 @@ static irqreturn_t vop_isr(int irq, void *data)
>                 ret = IRQ_HANDLED;
>         }
>
> +       if (active_irqs & LINE_FLAG_INTR) {
> +               complete(&vop->line_flag_completion);
> +               active_irqs &= ~LINE_FLAG_INTR;
> +               ret = IRQ_HANDLED;
> +       }
> +
>         if (active_irqs & FS_INTR) {
>                 drm_crtc_handle_vblank(crtc);
>                 vop_handle_vblank(vop);
> @@ -1255,6 +1328,7 @@ static int vop_create_crtc(struct vop *vop)
>
>         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);
>
> @@ -1411,6 +1485,49 @@ static void vop_win_init(struct vop *vop)
>         }
>  }
>
> +/**
> + * rockchip_drm_wait_line_flag - acqiure the give line flag event
> + * @crtc: CRTC to enable line flag
> + * @line_num: interested line number
> + * @mstimeout: millisecond for timeout
> + *
> + * Driver would hold here until the interested line flag interrupt have
> + * happened or timeout to wait.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
> +                               unsigned int mstimeout)
> +{
> +       struct vop *vop = to_vop(crtc);
> +       unsigned long jiffies_left;
> +
> +       if (!crtc || !vop->is_enabled)
> +               return -ENODEV;
> +
> +       if (line_num > crtc->mode.vtotal || mstimeout <= 0)
> +               return -EINVAL;
> +
> +       if (vop_line_flag_irq_is_enabled(vop))
> +               return -EBUSY;
> +
> +       reinit_completion(&vop->line_flag_completion);
> +       vop_line_flag_irq_enable(vop, line_num);
> +
> +       jiffies_left = wait_for_completion_timeout(&vop->line_flag_completion,
> +                                                  msecs_to_jiffies(mstimeout));
> +       vop_line_flag_irq_disable(vop);
> +
> +       if (jiffies_left == 0) {
> +               dev_err(vop->dev, "Timeout waiting for IRQ\n");
> +               return -ETIMEDOUT;
> +       }
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL(rockchip_drm_wait_line_flag);
> +
>  static int vop_bind(struct device *dev, struct device *master, void *data)
>  {
>         struct platform_device *pdev = to_platform_device(dev);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> index ff4f52e..1dbc526 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
> @@ -61,6 +61,8 @@ struct vop_ctrl {
>         struct vop_reg hpost_st_end;
>         struct vop_reg vpost_st_end;
>
> +       struct vop_reg line_flag_num[2];
> +
>         struct vop_reg cfg_done;
>  };
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 6f42e56..eea8427 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -122,6 +122,7 @@ static const struct vop_ctrl rk3036_ctrl_data = {
>         .hact_st_end = VOP_REG(RK3036_DSP_HACT_ST_END, 0x1fff1fff, 0),
>         .vtotal_pw = VOP_REG(RK3036_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
>         .vact_st_end = VOP_REG(RK3036_DSP_VACT_ST_END, 0x1fff1fff, 0),
> +       .line_flag_num[0] = VOP_REG(RK3036_INT_STATUS, 0xfff, 12),
>         .cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
>  };
>
> @@ -221,6 +222,7 @@ static const struct vop_ctrl rk3288_ctrl_data = {
>         .vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
>         .hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>         .vpost_st_end = VOP_REG(RK3288_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
> +       .line_flag_num[0] = VOP_REG(RK3288_INTR_CTRL0, 0x1fff, 12),
>         .cfg_done = VOP_REG(RK3288_REG_CFG_DONE, 0x1, 0),
>  };
>
> @@ -299,6 +301,8 @@ static const struct vop_ctrl rk3399_ctrl_data = {
>         .vact_st_end = VOP_REG(RK3399_DSP_VACT_ST_END, 0x1fff1fff, 0),
>         .hpost_st_end = VOP_REG(RK3399_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>         .vpost_st_end = VOP_REG(RK3399_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
> +       .line_flag_num[0] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 0),
> +       .line_flag_num[1] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 16),
>         .cfg_done = VOP_REG_MASK(RK3399_REG_CFG_DONE, 0x1, 0),
>  };
>
> --
> 1.9.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4.1 3/4] drm/bridge: analogix_dp: add the PSR function support
  2016-07-15 10:55 ` [PATCH v4.1 3/4] drm/bridge: analogix_dp: add the PSR function support Yakir Yang
@ 2016-07-15 13:13   ` Sean Paul
  2016-07-16  2:31     ` Yakir Yang
  0 siblings, 1 reply; 21+ messages in thread
From: Sean Paul @ 2016-07-15 13:13 UTC (permalink / raw)
  To: Yakir Yang
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Krzysztof Kozlowski, linux-samsung-soc, linux-kernel,
	linux-rockchip, daniel.vetter, emil.l.velikov, dianders,
	dri-devel, Tomasz Figa, Javier Martinez Canillas,
	Stéphane Marchesin, Thierry Reding, Dan Carpenter

On Fri, Jul 15, 2016 at 06:55:17PM +0800, Yakir Yang wrote:
> The full name of PSR is Panel Self Refresh, panel device could refresh
> itself with the hardware framebuffer in panel, this would make lots of
> sense to save the power consumption.
> 
> This patch have exported two symbols for platform driver to implement
> the PSR function in hardware side:
> - analogix_dp_active_psr()
> - analogix_dp_inactive_psr()
> 
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> ---
> Changes in v4.1:
> - Take use of existing edp_psr_vsc struct to swap HBx and DBx setting. (Sean)
> - Remove PSR_VID_CRC_FLUSH setting analogix_dp_enable_psr_crc().
> - Add comment about PBx magic numbers. (Sean)
> 
> Changes in v4:
> - Downgrade the PSR version print message to debug level. (Sean)
> - Return 'void' instead of 'int' in analogix_dp_enable_sink_psr(). (Sean)
> - Delete the unused read dpcd operations in analogix_dp_enable_sink_psr(). (Sean)
> - Delete the arbitrary usleep_range in analogix_dp_enable_psr_crc. (Sean).
> - Clean up the hardcoded values in analogix_dp_send_psr_spd(). (Sean)
> - Rename "active/inactive" to "enable/disable". (Sean, Dominik)
> - Keep set the PSR_VID_CRC_FLUSH gate in analogix_dp_enable_psr_crc().
> 
> Changes in v3:
> - split analogix_dp_enable_psr(), make it more clearly
>     analogix_dp_detect_sink_psr()
>     analogix_dp_enable_sink_psr()
> - remove some nosie register setting comments
> 
> Changes in v2:
> - introduce in v2, splite the common Analogix DP changes out
> 
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 81 ++++++++++++++++++++++
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  5 ++
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 51 ++++++++++++++
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h  | 34 +++++++++
>  include/drm/bridge/analogix_dp.h                   |  3 +
>  5 files changed, 174 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 32715da..381b25e 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -97,6 +97,83 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
>  	return 0;
>  }
>  
> +int analogix_dp_enable_psr(struct device *dev)
> +{
> +	struct analogix_dp_device *dp = dev_get_drvdata(dev);
> +	struct edp_vsc_psr psr_vsc;
> +
> +	if (!dp->psr_support)
> +		return -EINVAL;
> +
> +	/* 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;
> +
> +	analogix_dp_send_psr_spd(dp, &psr_vsc);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
> +
> +int analogix_dp_disable_psr(struct device *dev)
> +{
> +	struct analogix_dp_device *dp = dev_get_drvdata(dev);
> +	struct edp_vsc_psr psr_vsc;
> +
> +	if (!dp->psr_support)
> +		return -EINVAL;
> +
> +	/* 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;
> +
> +	analogix_dp_send_psr_spd(dp, &psr_vsc);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
> +
> +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);
> +	dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version);
> +
> +	return (psr_version & DP_PSR_IS_SUPPORTED) ? true : false;
> +}
> +
> +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);
> +	psr_en &= ~DP_PSR_ENABLE;
> +	analogix_dp_write_byte_to_dpcd(dp, 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);
> +
> +	/* 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);
> +
> +	analogix_dp_enable_psr_crc(dp);
> +}
> +
>  static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
>  {
>  	int i;
> @@ -921,6 +998,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
>  
>  	/* Enable video */
>  	analogix_dp_start_video(dp);
> +
> +	dp->psr_support = analogix_dp_detect_sink_psr(dp);
> +	if (dp->psr_support)
> +		analogix_dp_enable_sink_psr(dp);
>  }
>  
>  int analogix_dp_get_modes(struct drm_connector *connector)
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index b456380..17fbcee 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -177,6 +177,7 @@ struct analogix_dp_device {
>  	int			hpd_gpio;
>  	bool                    force_hpd;
>  	unsigned char           edid[EDID_BLOCK_LENGTH * 2];
> +	bool			psr_support;
>  
>  	struct analogix_dp_plat_data *plat_data;
>  };
> @@ -278,4 +279,8 @@ int analogix_dp_is_video_stream_on(struct analogix_dp_device *dp);
>  void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>  void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>  void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
> +void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
> +void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> +			      struct edp_vsc_psr *vsc);
> +
>  #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 48030f0..52c1b6b 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -1322,3 +1322,54 @@ void analogix_dp_disable_scrambling(struct analogix_dp_device *dp)
>  	reg |= SCRAMBLING_DISABLE;
>  	writel(reg, dp->reg_base + ANALOGIX_DP_TRAINING_PTN_SET);
>  }
> +
> +void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
> +{
> +	writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
> +}
> +
> +void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> +			      struct edp_vsc_psr *vsc)
> +{
> +	unsigned int val;
> +
> +	/* don't send info frame */
> +	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +	val &= ~IF_EN;
> +	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +
> +	/* configure single frame update mode */
> +	writel(PSR_FRAME_UP_TYPE_BURST | PSR_CRC_SEL_HARDWARE,
> +	       dp->reg_base + ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL);
> +
> +	/* configure VSC HB0~HB3 */
> +	writel(vsc->sdp_header.HB0, dp->reg_base + ANALOGIX_DP_SPD_HB0);
> +	writel(vsc->sdp_header.HB1, dp->reg_base + ANALOGIX_DP_SPD_HB1);
> +	writel(vsc->sdp_header.HB2, dp->reg_base + ANALOGIX_DP_SPD_HB2);
> +	writel(vsc->sdp_header.HB3, dp->reg_base + ANALOGIX_DP_SPD_HB3);
> +
> +	/* configure reused VSC PB0~PB3, magic number from vendor */
> +	writel(0x00, dp->reg_base + ANALOGIX_DP_SPD_PB0);
> +	writel(0x16, dp->reg_base + ANALOGIX_DP_SPD_PB1);
> +	writel(0xCE, dp->reg_base + ANALOGIX_DP_SPD_PB2);
> +	writel(0x5D, dp->reg_base + ANALOGIX_DP_SPD_PB3);
> +
> +	/* configure DB0 / DB1 values */
> +	writel(vsc->DB0, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB0);
> +	writel(vsc->DB1, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB1);
> +
> +	/* set reuse spd inforframe */
> +	val = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_3);
> +	val |= REUSE_SPD_EN;
> +	writel(val, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_3);
> +
> +	/* mark info frame update */
> +	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +	val = (val | IF_UP) & ~IF_EN;
> +	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +
> +	/* send info frame */
> +	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +	val |= IF_EN;
> +	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +}
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
> index cdcc6c5..40200c6 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
> @@ -22,6 +22,8 @@
>  #define ANALOGIX_DP_VIDEO_CTL_8			0x3C
>  #define ANALOGIX_DP_VIDEO_CTL_10		0x44
>  
> +#define ANALOGIX_DP_SPDIF_AUDIO_CTL_0		0xD8
> +
>  #define ANALOGIX_DP_PLL_REG_1			0xfc
>  #define ANALOGIX_DP_PLL_REG_2			0x9e4
>  #define ANALOGIX_DP_PLL_REG_3			0x9e8
> @@ -30,6 +32,21 @@
>  
>  #define ANALOGIX_DP_PD				0x12c
>  
> +#define ANALOGIX_DP_IF_TYPE			0x244
> +#define ANALOGIX_DP_IF_PKT_DB1			0x254
> +#define ANALOGIX_DP_IF_PKT_DB2			0x258
> +#define ANALOGIX_DP_SPD_HB0			0x2F8
> +#define ANALOGIX_DP_SPD_HB1			0x2FC
> +#define ANALOGIX_DP_SPD_HB2			0x300
> +#define ANALOGIX_DP_SPD_HB3			0x304
> +#define ANALOGIX_DP_SPD_PB0			0x308
> +#define ANALOGIX_DP_SPD_PB1			0x30C
> +#define ANALOGIX_DP_SPD_PB2			0x310
> +#define ANALOGIX_DP_SPD_PB3			0x314
> +#define ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL	0x318
> +#define ANALOGIX_DP_VSC_SHADOW_DB0		0x31C
> +#define ANALOGIX_DP_VSC_SHADOW_DB1		0x320
> +
>  #define ANALOGIX_DP_LANE_MAP			0x35C
>  
>  #define ANALOGIX_DP_ANALOG_CTL_1		0x370
> @@ -103,6 +120,8 @@
>  
>  #define ANALOGIX_DP_SOC_GENERAL_CTL		0x800
>  
> +#define ANALOGIX_DP_CRC_CON			0x890
> +
>  /* ANALOGIX_DP_TX_SW_RESET */
>  #define RESET_DP_TX				(0x1 << 0)
>  
> @@ -151,6 +170,7 @@
>  #define VID_CHK_UPDATE_TYPE_SHIFT		(4)
>  #define VID_CHK_UPDATE_TYPE_1			(0x1 << 4)
>  #define VID_CHK_UPDATE_TYPE_0			(0x0 << 4)
> +#define REUSE_SPD_EN				(0x1 << 3)
>  
>  /* ANALOGIX_DP_VIDEO_CTL_8 */
>  #define VID_HRES_TH(x)				(((x) & 0xf) << 4)
> @@ -167,6 +187,12 @@
>  #define REF_CLK_27M				(0x0 << 0)
>  #define REF_CLK_MASK				(0x1 << 0)
>  
> +/* ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL */
> +#define PSR_FRAME_UP_TYPE_BURST			(0x1 << 0)
> +#define PSR_FRAME_UP_TYPE_SINGLE		(0x0 << 0)
> +#define PSR_CRC_SEL_HARDWARE			(0x1 << 1)
> +#define PSR_CRC_SEL_MANUALLY			(0x0 << 1)
> +
>  /* ANALOGIX_DP_LANE_MAP */
>  #define LANE3_MAP_LOGIC_LANE_0			(0x0 << 6)
>  #define LANE3_MAP_LOGIC_LANE_1			(0x1 << 6)
> @@ -376,4 +402,12 @@
>  #define VIDEO_MODE_SLAVE_MODE			(0x1 << 0)
>  #define VIDEO_MODE_MASTER_MODE			(0x0 << 0)
>  
> +/* ANALOGIX_DP_PKT_SEND_CTL */
> +#define IF_UP					(0x1 << 4)
> +#define IF_EN					(0x1 << 0)
> +
> +/* ANALOGIX_DP_CRC_CON */
> +#define PSR_VID_CRC_FLUSH			(0x1 << 2)
> +#define PSR_VID_CRC_ENABLE			(0x1 << 0)
> +
>  #endif /* _ANALOGIX_DP_REG_H */
> diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
> index 261b86d..9cd8838 100644
> --- a/include/drm/bridge/analogix_dp.h
> +++ b/include/drm/bridge/analogix_dp.h
> @@ -38,6 +38,9 @@ struct analogix_dp_plat_data {
>  			 struct drm_connector *);
>  };
>  
> +int analogix_dp_enable_psr(struct device *dev);
> +int analogix_dp_disable_psr(struct device *dev);
> +
>  int analogix_dp_resume(struct device *dev);
>  int analogix_dp_suspend(struct device *dev);
>  
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4.1 1/4] drm/rockchip: vop: export line flag function
  2016-07-15 13:04   ` Sean Paul
@ 2016-07-16  2:30     ` Yakir Yang
  0 siblings, 0 replies; 21+ messages in thread
From: Yakir Yang @ 2016-07-16  2:30 UTC (permalink / raw)
  To: Sean Paul
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Javier Martinez Canillas, Stéphane Marchesin, Tomasz Figa,
	David Airlie, Daniel Vetter, Thierry Reding, Douglas Anderson,
	Krzysztof Kozlowski, Emil Velikov, Dan Carpenter,
	Linux Kernel Mailing List, dri-devel, linux-samsung-soc,
	linux-rockchip

Sean,

On 07/15/2016 09:04 PM, Sean Paul wrote:
> On Fri, Jul 15, 2016 at 6:55 AM, Yakir Yang <ykk@rock-chips.com> wrote:
>> VOP have integrated a hardware counter which indicate the exact display
>> line that vop is scanning. And if we're interested in a specific line,
>> we can set the line number to vop line_flag register, and then vop would
>> generate a line_flag interrupt for it.
>>
>> For example eDP PSR function is interested in the vertical blanking
>> period, then driver could set the line number to zero.
>>
>> This patch have exported a symbol that allow other driver to listen the
>> line flag event with given timeout limit:
>> -  rockchip_drm_wait_line_flag()
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>
> Thanks for the update.
>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>

Thanks for your reviewed  :-D

- Yakir

>> ---
>> Changes in v4.1:
>> - Remove the completion_done() check in irq handler (Sean)
>>
>> Changes in v4:
>> - Avoid the weird behavior in rockchip_drm_wait_line_flag(). (Sean)
>> - Make line_flag_num_x to an array. (Sean)
>> - Remove the unused vop_cfg_done() in vop_line_flag_irq_enable(). (Stephane, reviewed in Google gerrit)
>>      [https://chromium-review.googlesource.com/#/c/349084/33/drivers/gpu/drm/rockchip/rockchip_drm_vop.c@466]
>>
>> Changes in v3:
>> - Export the 'rockchip_drm_wait_line_flag' symbol, and document it.
>> - Add 'line_flag_num_0' for RK3288/RK3036
>> - Remove the notify for waiting line_flag event (Daniel)
>>
>> Changes in v2:
>> - Introduce in v2, split VOP line flag changes out
>>
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   3 +
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 117 ++++++++++++++++++++++++++++
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.h |   2 +
>>   drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   4 +
>>   4 files changed, 126 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> index ea39329..239b830 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> @@ -70,4 +70,7 @@ int rockchip_drm_dma_attach_device(struct drm_device *drm_dev,
>>                                     struct device *dev);
>>   void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>>                                      struct device *dev);
>> +int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
>> +                               unsigned int mstimeout);
>> +
>>   #endif /* _ROCKCHIP_DRM_DRV_H_ */
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> index c8a62a8..8a4f36e 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> @@ -121,6 +121,8 @@ struct vop {
>>          /* protected by dev->event_lock */
>>          struct drm_pending_vblank_event *event;
>>
>> +       struct completion line_flag_completion;
>> +
>>          const struct vop_data *data;
>>
>>          uint32_t *regsbak;
>> @@ -431,6 +433,71 @@ static void vop_dsp_hold_valid_irq_disable(struct vop *vop)
>>          spin_unlock_irqrestore(&vop->irq_lock, flags);
>>   }
>>
>> +/*
>> + * (1) each frame starts at the start of the Vsync pulse which is signaled by
>> + *     the "FRAME_SYNC" interrupt.
>> + * (2) the active data region of each frame ends at dsp_vact_end
>> + * (3) we should program this same number (dsp_vact_end) into dsp_line_frag_num,
>> + *      to get "LINE_FLAG" interrupt at the end of the active on screen data.
>> + *
>> + * VOP_INTR_CTRL0.dsp_line_frag_num = VOP_DSP_VACT_ST_END.dsp_vact_end
>> + * Interrupts
>> + * LINE_FLAG -------------------------------+
>> + * FRAME_SYNC ----+                         |
>> + *                |                         |
>> + *                v                         v
>> + *                | Vsync | Vbp |  Vactive  | Vfp |
>> + *                        ^     ^           ^     ^
>> + *                        |     |           |     |
>> + *                        |     |           |     |
>> + * dsp_vs_end ------------+     |           |     |   VOP_DSP_VTOTAL_VS_END
>> + * dsp_vact_start --------------+           |     |   VOP_DSP_VACT_ST_END
>> + * dsp_vact_end ----------------------------+     |   VOP_DSP_VACT_ST_END
>> + * dsp_total -------------------------------------+   VOP_DSP_VTOTAL_VS_END
>> + */
>> +static bool vop_line_flag_irq_is_enabled(struct vop *vop)
>> +{
>> +       uint32_t line_flag_irq;
>> +       unsigned long flags;
>> +
>> +       spin_lock_irqsave(&vop->irq_lock, flags);
>> +
>> +       line_flag_irq = VOP_INTR_GET_TYPE(vop, enable, LINE_FLAG_INTR);
>> +
>> +       spin_unlock_irqrestore(&vop->irq_lock, flags);
>> +
>> +       return !!line_flag_irq;
>> +}
>> +
>> +static void vop_line_flag_irq_enable(struct vop *vop, int line_num)
>> +{
>> +       unsigned long flags;
>> +
>> +       if (WARN_ON(!vop->is_enabled))
>> +               return;
>> +
>> +       spin_lock_irqsave(&vop->irq_lock, flags);
>> +
>> +       VOP_CTRL_SET(vop, line_flag_num[0], line_num);
>> +       VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 1);
>> +
>> +       spin_unlock_irqrestore(&vop->irq_lock, flags);
>> +}
>> +
>> +static void vop_line_flag_irq_disable(struct vop *vop)
>> +{
>> +       unsigned long flags;
>> +
>> +       if (WARN_ON(!vop->is_enabled))
>> +               return;
>> +
>> +       spin_lock_irqsave(&vop->irq_lock, flags);
>> +
>> +       VOP_INTR_SET_TYPE(vop, enable, LINE_FLAG_INTR, 0);
>> +
>> +       spin_unlock_irqrestore(&vop->irq_lock, flags);
>> +}
>> +
>>   static void vop_enable(struct drm_crtc *crtc)
>>   {
>>          struct vop *vop = to_vop(crtc);
>> @@ -1157,6 +1224,12 @@ static irqreturn_t vop_isr(int irq, void *data)
>>                  ret = IRQ_HANDLED;
>>          }
>>
>> +       if (active_irqs & LINE_FLAG_INTR) {
>> +               complete(&vop->line_flag_completion);
>> +               active_irqs &= ~LINE_FLAG_INTR;
>> +               ret = IRQ_HANDLED;
>> +       }
>> +
>>          if (active_irqs & FS_INTR) {
>>                  drm_crtc_handle_vblank(crtc);
>>                  vop_handle_vblank(vop);
>> @@ -1255,6 +1328,7 @@ static int vop_create_crtc(struct vop *vop)
>>
>>          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);
>>
>> @@ -1411,6 +1485,49 @@ static void vop_win_init(struct vop *vop)
>>          }
>>   }
>>
>> +/**
>> + * rockchip_drm_wait_line_flag - acqiure the give line flag event
>> + * @crtc: CRTC to enable line flag
>> + * @line_num: interested line number
>> + * @mstimeout: millisecond for timeout
>> + *
>> + * Driver would hold here until the interested line flag interrupt have
>> + * happened or timeout to wait.
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
>> +                               unsigned int mstimeout)
>> +{
>> +       struct vop *vop = to_vop(crtc);
>> +       unsigned long jiffies_left;
>> +
>> +       if (!crtc || !vop->is_enabled)
>> +               return -ENODEV;
>> +
>> +       if (line_num > crtc->mode.vtotal || mstimeout <= 0)
>> +               return -EINVAL;
>> +
>> +       if (vop_line_flag_irq_is_enabled(vop))
>> +               return -EBUSY;
>> +
>> +       reinit_completion(&vop->line_flag_completion);
>> +       vop_line_flag_irq_enable(vop, line_num);
>> +
>> +       jiffies_left = wait_for_completion_timeout(&vop->line_flag_completion,
>> +                                                  msecs_to_jiffies(mstimeout));
>> +       vop_line_flag_irq_disable(vop);
>> +
>> +       if (jiffies_left == 0) {
>> +               dev_err(vop->dev, "Timeout waiting for IRQ\n");
>> +               return -ETIMEDOUT;
>> +       }
>> +
>> +       return 0;
>> +}
>> +EXPORT_SYMBOL(rockchip_drm_wait_line_flag);
>> +
>>   static int vop_bind(struct device *dev, struct device *master, void *data)
>>   {
>>          struct platform_device *pdev = to_platform_device(dev);
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> index ff4f52e..1dbc526 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
>> @@ -61,6 +61,8 @@ struct vop_ctrl {
>>          struct vop_reg hpost_st_end;
>>          struct vop_reg vpost_st_end;
>>
>> +       struct vop_reg line_flag_num[2];
>> +
>>          struct vop_reg cfg_done;
>>   };
>>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> index 6f42e56..eea8427 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> @@ -122,6 +122,7 @@ static const struct vop_ctrl rk3036_ctrl_data = {
>>          .hact_st_end = VOP_REG(RK3036_DSP_HACT_ST_END, 0x1fff1fff, 0),
>>          .vtotal_pw = VOP_REG(RK3036_DSP_VTOTAL_VS_END, 0x1fff1fff, 0),
>>          .vact_st_end = VOP_REG(RK3036_DSP_VACT_ST_END, 0x1fff1fff, 0),
>> +       .line_flag_num[0] = VOP_REG(RK3036_INT_STATUS, 0xfff, 12),
>>          .cfg_done = VOP_REG(RK3036_REG_CFG_DONE, 0x1, 0),
>>   };
>>
>> @@ -221,6 +222,7 @@ static const struct vop_ctrl rk3288_ctrl_data = {
>>          .vact_st_end = VOP_REG(RK3288_DSP_VACT_ST_END, 0x1fff1fff, 0),
>>          .hpost_st_end = VOP_REG(RK3288_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>>          .vpost_st_end = VOP_REG(RK3288_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
>> +       .line_flag_num[0] = VOP_REG(RK3288_INTR_CTRL0, 0x1fff, 12),
>>          .cfg_done = VOP_REG(RK3288_REG_CFG_DONE, 0x1, 0),
>>   };
>>
>> @@ -299,6 +301,8 @@ static const struct vop_ctrl rk3399_ctrl_data = {
>>          .vact_st_end = VOP_REG(RK3399_DSP_VACT_ST_END, 0x1fff1fff, 0),
>>          .hpost_st_end = VOP_REG(RK3399_POST_DSP_HACT_INFO, 0x1fff1fff, 0),
>>          .vpost_st_end = VOP_REG(RK3399_POST_DSP_VACT_INFO, 0x1fff1fff, 0),
>> +       .line_flag_num[0] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 0),
>> +       .line_flag_num[1] = VOP_REG(RK3399_LINE_FLAG, 0xffff, 16),
>>          .cfg_done = VOP_REG_MASK(RK3399_REG_CFG_DONE, 0x1, 0),
>>   };
>>
>> --
>> 1.9.1
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4.1 3/4] drm/bridge: analogix_dp: add the PSR function support
  2016-07-15 13:13   ` Sean Paul
@ 2016-07-16  2:31     ` Yakir Yang
  0 siblings, 0 replies; 21+ messages in thread
From: Yakir Yang @ 2016-07-16  2:31 UTC (permalink / raw)
  To: Sean Paul
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Krzysztof Kozlowski, linux-samsung-soc, linux-kernel,
	linux-rockchip, daniel.vetter, emil.l.velikov, dianders,
	dri-devel, Tomasz Figa, Javier Martinez Canillas,
	Stéphane Marchesin, Thierry Reding, Dan Carpenter

Sean,

On 07/15/2016 09:13 PM, Sean Paul wrote:
> On Fri, Jul 15, 2016 at 06:55:17PM +0800, Yakir Yang wrote:
>> The full name of PSR is Panel Self Refresh, panel device could refresh
>> itself with the hardware framebuffer in panel, this would make lots of
>> sense to save the power consumption.
>>
>> This patch have exported two symbols for platform driver to implement
>> the PSR function in hardware side:
>> - analogix_dp_active_psr()
>> - analogix_dp_inactive_psr()
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>

Thanks for your reviewed  :-D

- Yakir

>
>> ---
>> Changes in v4.1:
>> - Take use of existing edp_psr_vsc struct to swap HBx and DBx setting. (Sean)
>> - Remove PSR_VID_CRC_FLUSH setting analogix_dp_enable_psr_crc().
>> - Add comment about PBx magic numbers. (Sean)
>>
>> Changes in v4:
>> - Downgrade the PSR version print message to debug level. (Sean)
>> - Return 'void' instead of 'int' in analogix_dp_enable_sink_psr(). (Sean)
>> - Delete the unused read dpcd operations in analogix_dp_enable_sink_psr(). (Sean)
>> - Delete the arbitrary usleep_range in analogix_dp_enable_psr_crc. (Sean).
>> - Clean up the hardcoded values in analogix_dp_send_psr_spd(). (Sean)
>> - Rename "active/inactive" to "enable/disable". (Sean, Dominik)
>> - Keep set the PSR_VID_CRC_FLUSH gate in analogix_dp_enable_psr_crc().
>>
>> Changes in v3:
>> - split analogix_dp_enable_psr(), make it more clearly
>>      analogix_dp_detect_sink_psr()
>>      analogix_dp_enable_sink_psr()
>> - remove some nosie register setting comments
>>
>> Changes in v2:
>> - introduce in v2, splite the common Analogix DP changes out
>>
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 81 ++++++++++++++++++++++
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  5 ++
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 51 ++++++++++++++
>>   drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h  | 34 +++++++++
>>   include/drm/bridge/analogix_dp.h                   |  3 +
>>   5 files changed, 174 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 32715da..381b25e 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -97,6 +97,83 @@ static int analogix_dp_detect_hpd(struct analogix_dp_device *dp)
>>   	return 0;
>>   }
>>   
>> +int analogix_dp_enable_psr(struct device *dev)
>> +{
>> +	struct analogix_dp_device *dp = dev_get_drvdata(dev);
>> +	struct edp_vsc_psr psr_vsc;
>> +
>> +	if (!dp->psr_support)
>> +		return -EINVAL;
>> +
>> +	/* 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;
>> +
>> +	analogix_dp_send_psr_spd(dp, &psr_vsc);
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>> +
>> +int analogix_dp_disable_psr(struct device *dev)
>> +{
>> +	struct analogix_dp_device *dp = dev_get_drvdata(dev);
>> +	struct edp_vsc_psr psr_vsc;
>> +
>> +	if (!dp->psr_support)
>> +		return -EINVAL;
>> +
>> +	/* 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;
>> +
>> +	analogix_dp_send_psr_spd(dp, &psr_vsc);
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>> +
>> +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);
>> +	dev_dbg(dp->dev, "Panel PSR version : %x\n", psr_version);
>> +
>> +	return (psr_version & DP_PSR_IS_SUPPORTED) ? true : false;
>> +}
>> +
>> +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);
>> +	psr_en &= ~DP_PSR_ENABLE;
>> +	analogix_dp_write_byte_to_dpcd(dp, 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);
>> +
>> +	/* 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);
>> +
>> +	analogix_dp_enable_psr_crc(dp);
>> +}
>> +
>>   static unsigned char analogix_dp_calc_edid_check_sum(unsigned char *edid_data)
>>   {
>>   	int i;
>> @@ -921,6 +998,10 @@ static void analogix_dp_commit(struct analogix_dp_device *dp)
>>   
>>   	/* Enable video */
>>   	analogix_dp_start_video(dp);
>> +
>> +	dp->psr_support = analogix_dp_detect_sink_psr(dp);
>> +	if (dp->psr_support)
>> +		analogix_dp_enable_sink_psr(dp);
>>   }
>>   
>>   int analogix_dp_get_modes(struct drm_connector *connector)
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> index b456380..17fbcee 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
>> @@ -177,6 +177,7 @@ struct analogix_dp_device {
>>   	int			hpd_gpio;
>>   	bool                    force_hpd;
>>   	unsigned char           edid[EDID_BLOCK_LENGTH * 2];
>> +	bool			psr_support;
>>   
>>   	struct analogix_dp_plat_data *plat_data;
>>   };
>> @@ -278,4 +279,8 @@ int analogix_dp_is_video_stream_on(struct analogix_dp_device *dp);
>>   void analogix_dp_config_video_slave_mode(struct analogix_dp_device *dp);
>>   void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>>   void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>> +void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
>> +void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> +			      struct edp_vsc_psr *vsc);
>> +
>>   #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 48030f0..52c1b6b 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
>> @@ -1322,3 +1322,54 @@ void analogix_dp_disable_scrambling(struct analogix_dp_device *dp)
>>   	reg |= SCRAMBLING_DISABLE;
>>   	writel(reg, dp->reg_base + ANALOGIX_DP_TRAINING_PTN_SET);
>>   }
>> +
>> +void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp)
>> +{
>> +	writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
>> +}
>> +
>> +void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>> +			      struct edp_vsc_psr *vsc)
>> +{
>> +	unsigned int val;
>> +
>> +	/* don't send info frame */
>> +	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> +	val &= ~IF_EN;
>> +	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> +
>> +	/* configure single frame update mode */
>> +	writel(PSR_FRAME_UP_TYPE_BURST | PSR_CRC_SEL_HARDWARE,
>> +	       dp->reg_base + ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL);
>> +
>> +	/* configure VSC HB0~HB3 */
>> +	writel(vsc->sdp_header.HB0, dp->reg_base + ANALOGIX_DP_SPD_HB0);
>> +	writel(vsc->sdp_header.HB1, dp->reg_base + ANALOGIX_DP_SPD_HB1);
>> +	writel(vsc->sdp_header.HB2, dp->reg_base + ANALOGIX_DP_SPD_HB2);
>> +	writel(vsc->sdp_header.HB3, dp->reg_base + ANALOGIX_DP_SPD_HB3);
>> +
>> +	/* configure reused VSC PB0~PB3, magic number from vendor */
>> +	writel(0x00, dp->reg_base + ANALOGIX_DP_SPD_PB0);
>> +	writel(0x16, dp->reg_base + ANALOGIX_DP_SPD_PB1);
>> +	writel(0xCE, dp->reg_base + ANALOGIX_DP_SPD_PB2);
>> +	writel(0x5D, dp->reg_base + ANALOGIX_DP_SPD_PB3);
>> +
>> +	/* configure DB0 / DB1 values */
>> +	writel(vsc->DB0, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB0);
>> +	writel(vsc->DB1, dp->reg_base + ANALOGIX_DP_VSC_SHADOW_DB1);
>> +
>> +	/* set reuse spd inforframe */
>> +	val = readl(dp->reg_base + ANALOGIX_DP_VIDEO_CTL_3);
>> +	val |= REUSE_SPD_EN;
>> +	writel(val, dp->reg_base + ANALOGIX_DP_VIDEO_CTL_3);
>> +
>> +	/* mark info frame update */
>> +	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> +	val = (val | IF_UP) & ~IF_EN;
>> +	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> +
>> +	/* send info frame */
>> +	val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> +	val |= IF_EN;
>> +	writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
>> +}
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
>> index cdcc6c5..40200c6 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.h
>> @@ -22,6 +22,8 @@
>>   #define ANALOGIX_DP_VIDEO_CTL_8			0x3C
>>   #define ANALOGIX_DP_VIDEO_CTL_10		0x44
>>   
>> +#define ANALOGIX_DP_SPDIF_AUDIO_CTL_0		0xD8
>> +
>>   #define ANALOGIX_DP_PLL_REG_1			0xfc
>>   #define ANALOGIX_DP_PLL_REG_2			0x9e4
>>   #define ANALOGIX_DP_PLL_REG_3			0x9e8
>> @@ -30,6 +32,21 @@
>>   
>>   #define ANALOGIX_DP_PD				0x12c
>>   
>> +#define ANALOGIX_DP_IF_TYPE			0x244
>> +#define ANALOGIX_DP_IF_PKT_DB1			0x254
>> +#define ANALOGIX_DP_IF_PKT_DB2			0x258
>> +#define ANALOGIX_DP_SPD_HB0			0x2F8
>> +#define ANALOGIX_DP_SPD_HB1			0x2FC
>> +#define ANALOGIX_DP_SPD_HB2			0x300
>> +#define ANALOGIX_DP_SPD_HB3			0x304
>> +#define ANALOGIX_DP_SPD_PB0			0x308
>> +#define ANALOGIX_DP_SPD_PB1			0x30C
>> +#define ANALOGIX_DP_SPD_PB2			0x310
>> +#define ANALOGIX_DP_SPD_PB3			0x314
>> +#define ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL	0x318
>> +#define ANALOGIX_DP_VSC_SHADOW_DB0		0x31C
>> +#define ANALOGIX_DP_VSC_SHADOW_DB1		0x320
>> +
>>   #define ANALOGIX_DP_LANE_MAP			0x35C
>>   
>>   #define ANALOGIX_DP_ANALOG_CTL_1		0x370
>> @@ -103,6 +120,8 @@
>>   
>>   #define ANALOGIX_DP_SOC_GENERAL_CTL		0x800
>>   
>> +#define ANALOGIX_DP_CRC_CON			0x890
>> +
>>   /* ANALOGIX_DP_TX_SW_RESET */
>>   #define RESET_DP_TX				(0x1 << 0)
>>   
>> @@ -151,6 +170,7 @@
>>   #define VID_CHK_UPDATE_TYPE_SHIFT		(4)
>>   #define VID_CHK_UPDATE_TYPE_1			(0x1 << 4)
>>   #define VID_CHK_UPDATE_TYPE_0			(0x0 << 4)
>> +#define REUSE_SPD_EN				(0x1 << 3)
>>   
>>   /* ANALOGIX_DP_VIDEO_CTL_8 */
>>   #define VID_HRES_TH(x)				(((x) & 0xf) << 4)
>> @@ -167,6 +187,12 @@
>>   #define REF_CLK_27M				(0x0 << 0)
>>   #define REF_CLK_MASK				(0x1 << 0)
>>   
>> +/* ANALOGIX_DP_PSR_FRAME_UPDATE_CTRL */
>> +#define PSR_FRAME_UP_TYPE_BURST			(0x1 << 0)
>> +#define PSR_FRAME_UP_TYPE_SINGLE		(0x0 << 0)
>> +#define PSR_CRC_SEL_HARDWARE			(0x1 << 1)
>> +#define PSR_CRC_SEL_MANUALLY			(0x0 << 1)
>> +
>>   /* ANALOGIX_DP_LANE_MAP */
>>   #define LANE3_MAP_LOGIC_LANE_0			(0x0 << 6)
>>   #define LANE3_MAP_LOGIC_LANE_1			(0x1 << 6)
>> @@ -376,4 +402,12 @@
>>   #define VIDEO_MODE_SLAVE_MODE			(0x1 << 0)
>>   #define VIDEO_MODE_MASTER_MODE			(0x0 << 0)
>>   
>> +/* ANALOGIX_DP_PKT_SEND_CTL */
>> +#define IF_UP					(0x1 << 4)
>> +#define IF_EN					(0x1 << 0)
>> +
>> +/* ANALOGIX_DP_CRC_CON */
>> +#define PSR_VID_CRC_FLUSH			(0x1 << 2)
>> +#define PSR_VID_CRC_ENABLE			(0x1 << 0)
>> +
>>   #endif /* _ANALOGIX_DP_REG_H */
>> diff --git a/include/drm/bridge/analogix_dp.h b/include/drm/bridge/analogix_dp.h
>> index 261b86d..9cd8838 100644
>> --- a/include/drm/bridge/analogix_dp.h
>> +++ b/include/drm/bridge/analogix_dp.h
>> @@ -38,6 +38,9 @@ struct analogix_dp_plat_data {
>>   			 struct drm_connector *);
>>   };
>>   
>> +int analogix_dp_enable_psr(struct device *dev);
>> +int analogix_dp_disable_psr(struct device *dev);
>> +
>>   int analogix_dp_resume(struct device *dev);
>>   int analogix_dp_suspend(struct device *dev);
>>   
>> -- 
>> 1.9.1
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/4] drm/rockchip: add an common abstracted PSR driver
  2016-07-14  4:15 ` [PATCH v4 2/4] drm/rockchip: add an common abstracted PSR driver Yakir Yang
  2016-07-14 15:14   ` Sean Paul
@ 2016-07-23  4:04   ` Doug Anderson
  2016-07-24  7:08     ` Yakir Yang
  1 sibling, 1 reply; 21+ messages in thread
From: Doug Anderson @ 2016-07-23  4:04 UTC (permalink / raw)
  To: Yakir Yang
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Javier Martinez Canillas, Stéphane Marchesin, Sean Paul,
	Tomasz Figa, David Airlie, Daniel Vetter, Thierry Reding,
	Krzysztof Kozlowski, Emil Velikov, Dan Carpenter, linux-kernel,
	dri-devel, linux-samsung-soc, open list:ARM/Rockchip SoC...

Yakir,

On Wed, Jul 13, 2016 at 9:15 PM, Yakir Yang <ykk@rock-chips.com> wrote:
> +static void psr_set_state(struct psr_drv *psr, enum psr_state state)
> +{
> +       mutex_lock(&psr->state_mutex);
> +
> +       if (psr->state == state) {
> +               mutex_unlock(&psr->state_mutex);
> +               return;
> +       }
> +
> +       psr->state = state;
> +       switch (state) {
> +       case PSR_ENABLE:
> +               psr->set(psr->encoder, true);
> +               break;
> +
> +       case PSR_DISABLE:
> +       case PSR_FLUSH:
> +               psr->set(psr->encoder, false);
> +               break;
> +       };
> +
> +       mutex_unlock(&psr->state_mutex);
> +}
> +
> +static void psr_flush_handler(unsigned long data)
> +{
> +       struct psr_drv *psr = (struct psr_drv *)data;
> +
> +       if (!psr || psr->state != PSR_FLUSH)
> +               return;
> +
> +       psr_set_state(psr, PSR_ENABLE);

As mentioned in a separate thread, this is probably not OK.
psr_set_state() grabs a mutex and that might sleep.  ...but
psr_flush_handler() is a timer.  I'm nearly certain that timers can't
sleep.

I believe this is the source of "sleeping function called from invalid
context" that I've seen at times.

-Doug

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/4] drm/rockchip: add an common abstracted PSR driver
  2016-07-23  4:04   ` Doug Anderson
@ 2016-07-24  7:08     ` Yakir Yang
  0 siblings, 0 replies; 21+ messages in thread
From: Yakir Yang @ 2016-07-24  7:08 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Javier Martinez Canillas, Stéphane Marchesin, Sean Paul,
	Tomasz Figa, David Airlie, Daniel Vetter, Thierry Reding,
	Krzysztof Kozlowski, Emil Velikov, Dan Carpenter, linux-kernel,
	dri-devel, linux-samsung-soc, open list:ARM/Rockchip SoC...

Doug,

On 07/23/2016 12:04 PM, Doug Anderson wrote:
> Yakir,
>
> On Wed, Jul 13, 2016 at 9:15 PM, Yakir Yang <ykk@rock-chips.com> wrote:
>> +static void psr_set_state(struct psr_drv *psr, enum psr_state state)
>> +{
>> +       mutex_lock(&psr->state_mutex);
>> +
>> +       if (psr->state == state) {
>> +               mutex_unlock(&psr->state_mutex);
>> +               return;
>> +       }
>> +
>> +       psr->state = state;
>> +       switch (state) {
>> +       case PSR_ENABLE:
>> +               psr->set(psr->encoder, true);
>> +               break;
>> +
>> +       case PSR_DISABLE:
>> +       case PSR_FLUSH:
>> +               psr->set(psr->encoder, false);
>> +               break;
>> +       };
>> +
>> +       mutex_unlock(&psr->state_mutex);
>> +}
>> +
>> +static void psr_flush_handler(unsigned long data)
>> +{
>> +       struct psr_drv *psr = (struct psr_drv *)data;
>> +
>> +       if (!psr || psr->state != PSR_FLUSH)
>> +               return;
>> +
>> +       psr_set_state(psr, PSR_ENABLE);
> As mentioned in a separate thread, this is probably not OK.
> psr_set_state() grabs a mutex and that might sleep.  ...but
> psr_flush_handler() is a timer.  I'm nearly certain that timers can't
> sleep.
>
> I believe this is the source of "sleeping function called from invalid
> context" that I've seen at times.

Thanks for your reported, i have wrote a patch[0] to fix this problem in 
my v5. If you're happy to review, that would be great ;)

[0]: https://patchwork.kernel.org/patch/9244805/

- Yakir

>
> -Doug
>
>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 2/4] drm/rockchip: add an common abstracted PSR driver
  2016-07-14 15:14   ` Sean Paul
  2016-07-15  1:43     ` Yakir Yang
@ 2016-07-24  7:14     ` Yakir Yang
  1 sibling, 0 replies; 21+ messages in thread
From: Yakir Yang @ 2016-07-24  7:14 UTC (permalink / raw)
  To: Sean Paul
  Cc: Mark Yao, Inki Dae, Jingoo Han, Heiko Stuebner,
	Krzysztof Kozlowski, linux-samsung-soc, linux-kernel,
	linux-rockchip, daniel.vetter, emil.l.velikov, dianders,
	dri-devel, Tomasz Figa, Javier Martinez Canillas,
	Stéphane Marchesin, Thierry Reding, Dan Carpenter

Sean,

On 07/14/2016 11:14 PM, Sean Paul wrote:
> On Thu, Jul 14, 2016 at 12:15:49PM +0800, Yakir Yang wrote:
>> The PSR driver have exported four symbols for specific device driver:
>> - rockchip_drm_psr_register()
>> - rockchip_drm_psr_unregister()
>> - rockchip_drm_psr_enable()
>> - rockchip_drm_psr_disable()
>> - rockchip_drm_psr_flush()
>>
>> Encoder driver should call the register/unregister interfaces to hook
>> itself into common PSR driver, encoder have implement the 'psr_set'
>> callback which use the set PSR state in hardware side.
>>
>> Crtc driver would call the enable/disable interfaces when vblank is
>> enable/disable, after that the common PSR driver would call the encoder
>> registered callback to set the PSR state.
>>
>> Fb driver would call the flush interface in 'fb->dirty' callback, this
>> helper function would force all PSR enabled encoders to exit from PSR
>> for 3 seconds.
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>
> I still don't think it's a good idea to pull this out into a separate PSR
> driver, but I suppose we can integrate it at a later time if it becomes
> cumbersome.
>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>

In order to make it safely to call those symbols at interrupt context, i 
have made lots of changes about this patch. It's not suitable to take 
your reviewed flag at the v5, if you're happy to review the new one [0], 
that would be very nice  :-D

[0]: https://patchwork.kernel.org/patch/9244805/

Thanks,
- Yakir


>
>> ---
>> Changes in v4:
>> - Tuck the global "psr_list" & "psr_list_mutex" in struct rockchip_drm_private. (Sean)
>> - Move the access of "psr->state" under "psr->state_mutex"'s protect. (Sean)
>> - Let "psr->state = PSR_FLUSH" under "psr->state_mutex"'s protect. (Sean)
>> - Collect psr_enable() and psr_disable() into psr_set_state()
>> - s/5\ second/PSR_FLUSH_TIMEOUT/ (Sean)
>> - Flush the psr callback in vop_crtc_disable(). (Stéphane, reviewed in Google gerrit)
>>      [https://chromium-review.googlesource.com/#/c/349084/6/drivers/gpu/drm/rockchip/rockchip_drm_vop.c@475]
>> - Add the missing file head with license. (Stéphane, reviewed in Google gerrit)
>>      [https://chromium-review.googlesource.com/#/c/357563/1/drivers/gpu/drm/rockchip/rockchip_drm_psr.h@3]
>>
>> Changes in v3:
>> - split the psr flow into an common abstracted PSR driver
>> - implement the 'fb->dirty' callback function (Daniel)
>> - avoid to use notify to acqiure for vact event (Daniel)
>> - remove psr_active() callback which introduce in v2
>>
>> Changes in v2: None
>>
>>   drivers/gpu/drm/rockchip/Makefile           |   2 +-
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.c |   4 +
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   3 +
>>   drivers/gpu/drm/rockchip/rockchip_drm_fb.c  |  12 ++
>>   drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 223 ++++++++++++++++++++++++++++
>>   drivers/gpu/drm/rockchip/rockchip_drm_psr.h |  26 ++++
>>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  29 ++++
>>   7 files changed, 298 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.c
>>   create mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_psr.h
>>
>> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
>> index 05d0713..9746365 100644
>> --- a/drivers/gpu/drm/rockchip/Makefile
>> +++ b/drivers/gpu/drm/rockchip/Makefile
>> @@ -3,7 +3,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_vop.o
>> +		rockchip_drm_gem.o rockchip_drm_psr.o rockchip_drm_vop.o
>>   rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>>   
>>   obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> index d665fb0..26c12b3 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> @@ -156,6 +156,9 @@ static int rockchip_drm_bind(struct device *dev)
>>   
>>   	drm_dev->dev_private = private;
>>   
>> +	INIT_LIST_HEAD(&private->psr_list);
>> +	mutex_init(&private->psr_list_mutex);
>> +
>>   	drm_mode_config_init(drm_dev);
>>   
>>   	rockchip_drm_mode_config_init(drm_dev);
>> @@ -218,6 +221,7 @@ static int rockchip_drm_bind(struct device *dev)
>>   
>>   	if (is_support_iommu)
>>   		arm_iommu_release_mapping(mapping);
>> +
>>   	return 0;
>>   err_fbdev_fini:
>>   	rockchip_drm_fbdev_fini(drm_dev);
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> index 239b830..9c34c9e 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> @@ -61,6 +61,9 @@ struct rockchip_drm_private {
>>   	struct drm_gem_object *fbdev_bo;
>>   	const struct rockchip_crtc_funcs *crtc_funcs[ROCKCHIP_MAX_CRTC];
>>   	struct drm_atomic_state *state;
>> +
>> +	struct list_head psr_list;
>> +	struct mutex psr_list_mutex;
>>   };
>>   
>>   int rockchip_register_crtc_funcs(struct drm_crtc *crtc,
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> index 20f12bc..36afd9c 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
>> @@ -21,6 +21,7 @@
>>   
>>   #include "rockchip_drm_drv.h"
>>   #include "rockchip_drm_gem.h"
>> +#include "rockchip_drm_psr.h"
>>   
>>   #define to_rockchip_fb(x) container_of(x, struct rockchip_drm_fb, fb)
>>   
>> @@ -66,9 +67,20 @@ static int rockchip_drm_fb_create_handle(struct drm_framebuffer *fb,
>>   				     rockchip_fb->obj[0], handle);
>>   }
>>   
>> +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(fb->dev);
>> +	return 0;
>> +}
>> +
>>   static const struct drm_framebuffer_funcs rockchip_drm_fb_funcs = {
>>   	.destroy	= rockchip_drm_fb_destroy,
>>   	.create_handle	= rockchip_drm_fb_create_handle,
>> +	.dirty		= rockchip_drm_fb_dirty,
>>   };
>>   
>>   static struct rockchip_drm_fb *
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
>> new file mode 100644
>> index 0000000..e79ea18
>> --- /dev/null
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
>> @@ -0,0 +1,223 @@
>> +/*
>> + * 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_crtc_helper.h>
>> +
>> +#include "rockchip_drm_drv.h"
>> +#include "rockchip_drm_psr.h"
>> +
>> +#define PSR_FLUSH_TIMEOUT	msecs_to_jiffies(3000) /* 3 seconds */
>> +
>> +static LIST_HEAD(psr_list);
>> +static DEFINE_MUTEX(psr_list_mutex);
>> +
>> +enum psr_state {
>> +	PSR_FLUSH,
>> +	PSR_ENABLE,
>> +	PSR_DISABLE,
>> +};
>> +
>> +struct psr_drv {
>> +	struct list_head list;
>> +	enum psr_state state;
>> +	struct mutex state_mutex;
>> +
>> +	struct timer_list flush_timer;
>> +
>> +	struct drm_encoder *encoder;
>> +	void (*set)(struct drm_encoder *encoder, bool enable);
>> +};
>> +
>> +static struct psr_drv *find_psr_by_crtc(struct drm_crtc *crtc)
>> +{
>> +	struct rockchip_drm_private *drm_drv = crtc->dev->dev_private;
>> +	struct psr_drv *psr;
>> +
>> +	mutex_lock(&drm_drv->psr_list_mutex);
>> +	list_for_each_entry(psr, &drm_drv->psr_list, list) {
>> +		if (psr->encoder->crtc == crtc) {
>> +			mutex_unlock(&drm_drv->psr_list_mutex);
>> +			return psr;
>> +		}
>> +	}
>> +	mutex_unlock(&drm_drv->psr_list_mutex);
>> +
>> +	return ERR_PTR(-ENODEV);
>> +}
>> +
>> +static void psr_set_state(struct psr_drv *psr, enum psr_state state)
>> +{
>> +	mutex_lock(&psr->state_mutex);
>> +
>> +	if (psr->state == state) {
>> +		mutex_unlock(&psr->state_mutex);
>> +		return;
>> +	}
>> +
>> +	psr->state = state;
>> +	switch (state) {
>> +	case PSR_ENABLE:
>> +		psr->set(psr->encoder, true);
>> +		break;
>> +
>> +	case PSR_DISABLE:
>> +	case PSR_FLUSH:
>> +		psr->set(psr->encoder, false);
>> +		break;
>> +	};
>> +
>> +	mutex_unlock(&psr->state_mutex);
>> +}
>> +
>> +static void psr_flush_handler(unsigned long data)
>> +{
>> +	struct psr_drv *psr = (struct psr_drv *)data;
>> +
>> +	if (!psr || psr->state != PSR_FLUSH)
>> +		return;
>> +
>> +	psr_set_state(psr, PSR_ENABLE);
>> +}
>> +
>> +/**
>> + * rockchip_drm_psr_enable - enable the encoder PSR which bind to given CRTC
>> + * @crtc: CRTC to obtain the PSR encoder
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int rockchip_drm_psr_enable(struct drm_crtc *crtc)
>> +{
>> +	struct psr_drv *psr = find_psr_by_crtc(crtc);
>> +
>> +	if (IS_ERR(psr))
>> +		return PTR_ERR(psr);
>> +
>> +	psr_set_state(psr, PSR_ENABLE);
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(rockchip_drm_psr_enable);
>> +
>> +/**
>> + * rockchip_drm_psr_disable - disable the encoder PSR which bind to given CRTC
>> + * @crtc: CRTC to obtain the PSR encoder
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int rockchip_drm_psr_disable(struct drm_crtc *crtc)
>> +{
>> +	struct psr_drv *psr = find_psr_by_crtc(crtc);
>> +
>> +	if (IS_ERR(psr))
>> +		return PTR_ERR(psr);
>> +
>> +	psr_set_state(psr, PSR_DISABLE);
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(rockchip_drm_psr_disable);
>> +
>> +/**
>> + * rockchip_drm_psr_flush - 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(struct drm_device *dev)
>> +{
>> +	struct rockchip_drm_private *drm_drv = dev->dev_private;
>> +	struct psr_drv *psr;
>> +
>> +	mutex_lock(&drm_drv->psr_list_mutex);
>> +	list_for_each_entry(psr, &drm_drv->psr_list, list) {
>> +		if (psr->state == PSR_DISABLE)
>> +			continue;
>> +
>> +		mod_timer(&psr->flush_timer,
>> +			  round_jiffies_up(jiffies + PSR_FLUSH_TIMEOUT));
>> +
>> +		psr_set_state(psr, PSR_FLUSH);
>> +	}
>> +	mutex_unlock(&drm_drv->psr_list_mutex);
>> +}
>> +EXPORT_SYMBOL(rockchip_drm_psr_flush);
>> +
>> +/**
>> + * rockchip_drm_psr_register - register encoder to psr driver
>> + * @encoder: encoder that obtain the PSR function
>> + * @psr_set: call back to set PSR state
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int rockchip_drm_psr_register(struct drm_encoder *encoder,
>> +			void (*psr_set)(struct drm_encoder *, bool enable))
>> +{
>> +	struct rockchip_drm_private *drm_drv = encoder->dev->dev_private;
>> +	struct psr_drv *psr;
>> +
>> +	if (!encoder || !psr_set)
>> +		return -EINVAL;
>> +
>> +	psr = kzalloc(sizeof(struct psr_drv), GFP_KERNEL);
>> +	if (!psr)
>> +		return -ENOMEM;
>> +
>> +	setup_timer(&psr->flush_timer, psr_flush_handler, (unsigned long)psr);
>> +
>> +	mutex_init(&psr->state_mutex);
>> +
>> +	psr->state = PSR_DISABLE;
>> +	psr->encoder = encoder;
>> +	psr->set = psr_set;
>> +
>> +	mutex_lock(&drm_drv->psr_list_mutex);
>> +	list_add_tail(&psr->list, &drm_drv->psr_list);
>> +	mutex_unlock(&drm_drv->psr_list_mutex);
>> +
>> +	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
>> + *
>> + * 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;
>> +
>> +	mutex_lock(&drm_drv->psr_list_mutex);
>> +	list_for_each_entry(psr, &drm_drv->psr_list, list) {
>> +		if (psr->encoder == encoder) {
>> +			del_timer(&psr->flush_timer);
>> +			list_del(&psr->list);
>> +			kfree(psr);
>> +		}
>> +	}
>> +	mutex_unlock(&drm_drv->psr_list_mutex);
>> +}
>> +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
>> new file mode 100644
>> index 0000000..c35b688
>> --- /dev/null
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.h
>> @@ -0,0 +1,26 @@
>> +/*
>> + * 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(struct drm_device *dev);
>> +int rockchip_drm_psr_enable(struct drm_crtc *crtc);
>> +int rockchip_drm_psr_disable(struct drm_crtc *crtc);
>> +
>> +int rockchip_drm_psr_register(struct drm_encoder *encoder,
>> +			void (*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 69d32f1..e702fb3 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>> @@ -34,6 +34,7 @@
>>   #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"
>>   
>>   #define __REG_SET_RELAXED(x, off, mask, shift, v, write_mask) \
>> @@ -87,6 +88,8 @@
>>   #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)
>>   
>> +#define VOP_PSR_SET_DELAY_TIME		msecs_to_jiffies(10)
>> +
>>   struct vop_plane_state {
>>   	struct drm_plane_state base;
>>   	int format;
>> @@ -121,6 +124,9 @@ struct vop {
>>   	/* protected by dev->event_lock */
>>   	struct drm_pending_vblank_event *event;
>>   
>> +	bool psr_enabled;
>> +	struct delayed_work psr_work;
>> +
>>   	struct completion line_flag_completion;
>>   
>>   	const struct vop_data *data;
>> @@ -629,6 +635,9 @@ static void vop_crtc_disable(struct drm_crtc *crtc)
>>   
>>   		crtc->state->event = NULL;
>>   	}
>> +
>> +	vop->psr_enabled = false;
>> +	INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
>>   }
>>   
>>   static void vop_plane_destroy(struct drm_plane *plane)
>> @@ -923,6 +932,16 @@ static const struct drm_plane_funcs vop_plane_funcs = {
>>   	.atomic_destroy_state = vop_atomic_plane_destroy_state,
>>   };
>>   
>> +static void vop_psr_work(struct work_struct *work)
>> +{
>> +	struct vop *vop = container_of(work, typeof(*vop), psr_work.work);
>> +
>> +	if (vop->psr_enabled)
>> +		rockchip_drm_psr_enable(&vop->crtc);
>> +	else
>> +		rockchip_drm_psr_disable(&vop->crtc);
>> +}
>> +
>>   static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
>>   {
>>   	struct vop *vop = to_vop(crtc);
>> @@ -937,6 +956,9 @@ static int vop_crtc_enable_vblank(struct drm_crtc *crtc)
>>   
>>   	spin_unlock_irqrestore(&vop->irq_lock, flags);
>>   
>> +	vop->psr_enabled = false;
>> +	schedule_delayed_work(&vop->psr_work, VOP_PSR_SET_DELAY_TIME);
>> +
>>   	return 0;
>>   }
>>   
>> @@ -953,6 +975,9 @@ 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);
>> +
>> +	vop->psr_enabled = true;
>> +	schedule_delayed_work(&vop->psr_work, VOP_PSR_SET_DELAY_TIME);
>>   }
>>   
>>   static void vop_crtc_wait_for_update(struct drm_crtc *crtc)
>> @@ -1597,6 +1622,10 @@ static int vop_bind(struct device *dev, struct device *master, void *data)
>>   		return ret;
>>   
>>   	pm_runtime_enable(&pdev->dev);
>> +
>> +	vop->psr_enabled = false;
>> +	INIT_DELAYED_WORK(&vop->psr_work, vop_psr_work);
>> +
>>   	return 0;
>>   }
>>   
>> -- 
>> 1.9.1
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2016-07-24  7:14 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-14  4:15 [PATCH v4 0/4] Add PSR function support for Analogix/Rockchip DP Yakir Yang
2016-07-14  4:15 ` [PATCH v4 1/4] drm/rockchip: vop: export line flag function Yakir Yang
2016-07-14 14:46   ` Sean Paul
2016-07-15  1:43     ` Yakir Yang
2016-07-14  4:15 ` [PATCH v4 2/4] drm/rockchip: add an common abstracted PSR driver Yakir Yang
2016-07-14 15:14   ` Sean Paul
2016-07-15  1:43     ` Yakir Yang
2016-07-24  7:14     ` Yakir Yang
2016-07-23  4:04   ` Doug Anderson
2016-07-24  7:08     ` Yakir Yang
2016-07-14  4:15 ` [PATCH v4 3/4] drm/bridge: analogix_dp: add the PSR function support Yakir Yang
2016-07-14 15:23   ` Sean Paul
2016-07-14  4:15 ` [PATCH v4 4/4] drm/rockchip: analogix_dp: implement PSR function Yakir Yang
2016-07-14 15:26   ` Sean Paul
2016-07-15  5:45     ` Yakir Yang
2016-07-15 10:55 ` [PATCH v4.1 1/4] drm/rockchip: vop: export line flag function Yakir Yang
2016-07-15 13:04   ` Sean Paul
2016-07-16  2:30     ` Yakir Yang
2016-07-15 10:55 ` [PATCH v4.1 3/4] drm/bridge: analogix_dp: add the PSR function support Yakir Yang
2016-07-15 13:13   ` Sean Paul
2016-07-16  2:31     ` Yakir Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).