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

[Yakir Yang]

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 


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 


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(>psr_list);

+   mutex_init(>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 

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

[Yakir Yang]

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 


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 


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(>psr_list);

+   mutex_init(>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 

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

[Yakir Yang]

Doug,

On 07/23/2016 12:04 PM, Doug Anderson wrote:

Yakir,

On Wed, Jul 13, 2016 at 9:15 PM, Yakir Yang  wrote:

+static void psr_set_state(struct psr_drv *psr, enum psr_state state)
+{
+   mutex_lock(>state_mutex);
+
+   if (psr->state == state) {
+   mutex_unlock(>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(>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

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

[Yakir Yang]

Doug,

On 07/23/2016 12:04 PM, Doug Anderson wrote:

Yakir,

On Wed, Jul 13, 2016 at 9:15 PM, Yakir Yang  wrote:

+static void psr_set_state(struct psr_drv *psr, enum psr_state state)
+{
+   mutex_lock(>state_mutex);
+
+   if (psr->state == state) {
+   mutex_unlock(>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(>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

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

[Doug Anderson]

Yakir,

On Wed, Jul 13, 2016 at 9:15 PM, Yakir Yang  wrote:
> +static void psr_set_state(struct psr_drv *psr, enum psr_state state)
> +{
> +   mutex_lock(>state_mutex);
> +
> +   if (psr->state == state) {
> +   mutex_unlock(>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(>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

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

[Doug Anderson]

Yakir,

On Wed, Jul 13, 2016 at 9:15 PM, Yakir Yang  wrote:
> +static void psr_set_state(struct psr_drv *psr, enum psr_state state)
> +{
> +   mutex_lock(>state_mutex);
> +
> +   if (psr->state == state) {
> +   mutex_unlock(>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(>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

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

[Yakir Yang]

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 


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 


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(>psr_list);

+   mutex_init(>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 

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

[Yakir Yang]

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 


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 


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(>psr_list);

+   mutex_init(>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) 

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

[Sean Paul]

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 


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 


> ---
> 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(>psr_list);
> + mutex_init(>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

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

[Sean Paul]

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 


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 


> ---
> 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(>psr_list);
> + mutex_init(>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 

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

[Yakir Yang]

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 
---
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(>psr_list);
+   mutex_init(>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,

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

[Yakir Yang]

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 
---
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(>psr_list);
+   mutex_init(>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,
+