[PATCH 2/3] drm/rockchip: Don't key off vblank for psr
Sean,
Thanks for this good fix.
On 08/19/2016 07:34 AM, Sean Paul wrote:
> Instead of keying off vblank for psr, just flush every time
> we get an atomic update. This ensures that cursor updates
> will properly disable psr (without turning vblank on/off),
> and unifies the paths between fb_dirty and atomic psr
> enable/disable.
>
> Signed-off-by: Sean Paul
Reviewed-by: Yakir Yang
Also I have verified this patch on RK3399 Kevin, eDP PSR works rightly, so
Tested-by: Yakir Yang
- Yakir
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
> drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 72
> -
> drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 8 ++--
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 10 ++--
> 4 files changed, 62 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> index ba45d9d..10cafbc 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
> @@ -70,7 +70,7 @@ static int rockchip_drm_fb_dirty(struct drm_framebuffer *fb,
>struct drm_clip_rect *clips,
>unsigned int num_clips)
> {
> - rockchip_drm_psr_flush(fb->dev);
> + rockchip_drm_psr_flush_all(fb->dev);
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> index c6ac5d0..de6252f 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
> @@ -31,6 +31,7 @@ struct psr_drv {
> struct drm_encoder *encoder;
>
> spinlock_t lock;
> + boolactive;
> enum psr_state state;
>
> struct timer_list flush_timer;
> @@ -67,11 +68,7 @@ static void psr_set_state_locked(struct psr_drv *psr, enum
> psr_state state)
>* v ||
>* PSR_DISABLE < - - - - - - - - -
>*/
> - if (state == psr->state)
> - return;
> -
> - /* Requesting a flush when disabled is a noop */
> - if (state == PSR_FLUSH && psr->state == PSR_DISABLE)
> + if (state == psr->state || !psr->active)
> return;
>
> psr->state = state;
> @@ -115,45 +112,79 @@ static void psr_flush_handler(unsigned long data)
> }
>
> /**
> - * rockchip_drm_psr_enable - enable the encoder PSR which bind to given CRTC
> + * rockchip_drm_psr_activate - activate PSR on the given pipe
>* @crtc: CRTC to obtain the PSR encoder
>*
>* Returns:
>* Zero on success, negative errno on failure.
>*/
> -int rockchip_drm_psr_enable(struct drm_crtc *crtc)
> +int rockchip_drm_psr_activate(struct drm_crtc *crtc)
> {
> struct psr_drv *psr = find_psr_by_crtc(crtc);
> + unsigned long flags;
>
> if (IS_ERR(psr))
> return PTR_ERR(psr);
>
> - psr_set_state(psr, PSR_ENABLE);
> + spin_lock_irqsave(>lock, flags);
> + psr->active = true;
> + spin_unlock_irqrestore(>lock, flags);
> +
> return 0;
> }
> -EXPORT_SYMBOL(rockchip_drm_psr_enable);
> +EXPORT_SYMBOL(rockchip_drm_psr_activate);
>
> /**
> - * rockchip_drm_psr_disable - disable the encoder PSR which bind to given
> CRTC
> + * rockchip_drm_psr_deactivate - deactivate PSR on the given pipe
>* @crtc: CRTC to obtain the PSR encoder
>*
>* Returns:
>* Zero on success, negative errno on failure.
>*/
> -int rockchip_drm_psr_disable(struct drm_crtc *crtc)
> +int rockchip_drm_psr_deactivate(struct drm_crtc *crtc)
> {
> struct psr_drv *psr = find_psr_by_crtc(crtc);
> + unsigned long flags;
> +
> + if (IS_ERR(psr))
> + return PTR_ERR(psr);
> +
> + spin_lock_irqsave(>lock, flags);
> + psr->active = false;
> + spin_unlock_irqrestore(>lock, flags);
> + del_timer_sync(>flush_timer);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(rockchip_drm_psr_deactivate);
> +
> +static void rockchip_drm_do_flush(struct psr_drv *psr)
> +{
> + mod_timer(>flush_timer,
> + round_jiffies_up(jiffies + PSR_FLUSH_TIMEOUT));
> + psr_set_state(psr, PSR_FLUSH);
> +}
>
> +/**
> + * rockchip_drm_psr_flush - flush a single pipe
> + * @crtc: CRTC of the pipe to flush
> + *
> + * Returns:
> + * 0 on success, -errno on fail
> + */
> +int rockchip_drm_psr_flush(struct drm_crtc *crtc)
> +{
> + struct psr_drv *psr = find_psr_by_crtc(crtc);
> if (IS_ERR(psr))
> return PTR_ERR(psr);
>
> - psr_set_state(psr, PSR_DISABLE);
> + rockchip_drm_do_flush(psr);
> return 0;
> }
> -EXPORT_SYMBOL(rockchip_drm_psr_disable);
> +EXPORT_SYMBOL(rockchip_drm_psr_flush);
>
> /**
> - * rockchip_drm_psr_flush - force to flush all registered PSR encoders
> + * rockchip_drm_psr_flush_all - force to flush all registered PSR encoders
>* @dev: drm device
>
[PATCH 2/3] drm/rockchip: Don't key off vblank for psr
Instead of keying off vblank for psr, just flush every time
we get an atomic update. This ensures that cursor updates
will properly disable psr (without turning vblank on/off),
and unifies the paths between fb_dirty and atomic psr
enable/disable.
Signed-off-by: Sean Paul
---
drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 72 -
drivers/gpu/drm/rockchip/rockchip_drm_psr.h | 8 ++--
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 10 ++--
4 files changed, 62 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
index ba45d9d..10cafbc 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c
@@ -70,7 +70,7 @@ static int rockchip_drm_fb_dirty(struct drm_framebuffer *fb,
struct drm_clip_rect *clips,
unsigned int num_clips)
{
- rockchip_drm_psr_flush(fb->dev);
+ rockchip_drm_psr_flush_all(fb->dev);
return 0;
}
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
index c6ac5d0..de6252f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c
@@ -31,6 +31,7 @@ struct psr_drv {
struct drm_encoder *encoder;
spinlock_t lock;
+ boolactive;
enum psr_state state;
struct timer_list flush_timer;
@@ -67,11 +68,7 @@ static void psr_set_state_locked(struct psr_drv *psr, enum
psr_state state)
* v ||
* PSR_DISABLE < - - - - - - - - -
*/
- if (state == psr->state)
- return;
-
- /* Requesting a flush when disabled is a noop */
- if (state == PSR_FLUSH && psr->state == PSR_DISABLE)
+ if (state == psr->state || !psr->active)
return;
psr->state = state;
@@ -115,45 +112,79 @@ static void psr_flush_handler(unsigned long data)
}
/**
- * rockchip_drm_psr_enable - enable the encoder PSR which bind to given CRTC
+ * rockchip_drm_psr_activate - activate PSR on the given pipe
* @crtc: CRTC to obtain the PSR encoder
*
* Returns:
* Zero on success, negative errno on failure.
*/
-int rockchip_drm_psr_enable(struct drm_crtc *crtc)
+int rockchip_drm_psr_activate(struct drm_crtc *crtc)
{
struct psr_drv *psr = find_psr_by_crtc(crtc);
+ unsigned long flags;
if (IS_ERR(psr))
return PTR_ERR(psr);
- psr_set_state(psr, PSR_ENABLE);
+ spin_lock_irqsave(>lock, flags);
+ psr->active = true;
+ spin_unlock_irqrestore(>lock, flags);
+
return 0;
}
-EXPORT_SYMBOL(rockchip_drm_psr_enable);
+EXPORT_SYMBOL(rockchip_drm_psr_activate);
/**
- * rockchip_drm_psr_disable - disable the encoder PSR which bind to given CRTC
+ * rockchip_drm_psr_deactivate - deactivate PSR on the given pipe
* @crtc: CRTC to obtain the PSR encoder
*
* Returns:
* Zero on success, negative errno on failure.
*/
-int rockchip_drm_psr_disable(struct drm_crtc *crtc)
+int rockchip_drm_psr_deactivate(struct drm_crtc *crtc)
{
struct psr_drv *psr = find_psr_by_crtc(crtc);
+ unsigned long flags;
+
+ if (IS_ERR(psr))
+ return PTR_ERR(psr);
+
+ spin_lock_irqsave(>lock, flags);
+ psr->active = false;
+ spin_unlock_irqrestore(>lock, flags);
+ del_timer_sync(>flush_timer);
+
+ return 0;
+}
+EXPORT_SYMBOL(rockchip_drm_psr_deactivate);
+
+static void rockchip_drm_do_flush(struct psr_drv *psr)
+{
+ mod_timer(>flush_timer,
+ round_jiffies_up(jiffies + PSR_FLUSH_TIMEOUT));
+ psr_set_state(psr, PSR_FLUSH);
+}
+/**
+ * rockchip_drm_psr_flush - flush a single pipe
+ * @crtc: CRTC of the pipe to flush
+ *
+ * Returns:
+ * 0 on success, -errno on fail
+ */
+int rockchip_drm_psr_flush(struct drm_crtc *crtc)
+{
+ struct psr_drv *psr = find_psr_by_crtc(crtc);
if (IS_ERR(psr))
return PTR_ERR(psr);
- psr_set_state(psr, PSR_DISABLE);
+ rockchip_drm_do_flush(psr);
return 0;
}
-EXPORT_SYMBOL(rockchip_drm_psr_disable);
+EXPORT_SYMBOL(rockchip_drm_psr_flush);
/**
- * rockchip_drm_psr_flush - force to flush all registered PSR encoders
+ * rockchip_drm_psr_flush_all - force to flush all registered PSR encoders
* @dev: drm device
*
* Disable the PSR function for all registered encoders, and then enable the
@@ -164,22 +195,18 @@ EXPORT_SYMBOL(rockchip_drm_psr_disable);
* Returns:
* Zero on success, negative errno on failure.
*/
-void rockchip_drm_psr_flush(struct drm_device *dev)
+void rockchip_drm_psr_flush_all(struct drm_device *dev)
{
struct rockchip_drm_private *drm_drv = dev->dev_private;
struct psr_drv *psr;
unsigned long flags;
