diff mbox

[v4,2/4] drm/rockchip: add an common abstracted PSR driver

Message ID 1468469749-12636-1-git-send-email-ykk@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yakir Yang July 14, 2016, 4:15 a.m. UTC
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

Comments

Sean Paul July 14, 2016, 3:14 p.m. UTC | #1
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
Yakir Yang July 15, 2016, 1:43 a.m. UTC | #2
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
>
>
Doug Anderson July 23, 2016, 4:04 a.m. UTC | #3
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
Yakir Yang July 24, 2016, 7:08 a.m. UTC | #4
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
>
>
>
Yakir Yang July 24, 2016, 7:14 a.m. UTC | #5
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
>
>
diff mbox

Patch

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;
 }