[8/9] drm/i915/psr: Restrict single-shot updates to the PSR pipe
diff mbox

Message ID 1434616228-28358-8-git-send-email-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter June 18, 2015, 8:30 a.m. UTC
The frontbuffer code gives us accurate information about activity,
let's use it. Again this should avoid unecessary updates when multiple
screens are on.

Also realing function paramaters, I couldn't resist that bit of OCD.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Durgadoss R <durgadoss.r@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h         |  7 ++++---
 drivers/gpu/drm/i915/intel_frontbuffer.c |  2 +-
 drivers/gpu/drm/i915/intel_psr.c         | 22 +++++++++++++---------
 3 files changed, 18 insertions(+), 13 deletions(-)

Comments

Chris Wilson June 18, 2015, 8:53 a.m. UTC | #1
On Thu, Jun 18, 2015 at 10:30:27AM +0200, Daniel Vetter wrote:
> The frontbuffer code gives us accurate information about activity,
> let's use it. Again this should avoid unecessary updates when multiple
> screens are on.
> 
> Also realing function paramaters, I couldn't resist that bit of OCD.

realign. I ocd'ed your ocd.
-Chris
Paulo Zanoni June 23, 2015, 8:12 p.m. UTC | #2
2015-06-18 5:30 GMT-03:00 Daniel Vetter <daniel.vetter@ffwll.ch>:
> The frontbuffer code gives us accurate information about activity,
> let's use it. Again this should avoid unecessary updates when multiple
> screens are on.
>
> Also realing function paramaters, I couldn't resist that bit of OCD.

Can't test this since -ENOHW. But it appears correct, so with and only
with the fix mentioned by Chris:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

This bug could also be verified by the idea I mentioned in my comment
to patch 7, if it was implemented.

Or we can also verify it by adding some debugfs messages at the
correct places of intel_psr_single_frame_update(), then run "sudo
./kms_frontbuffer_tracking --run-subtest psr-2p-scndscrn-sfb-flip-blt
--step --step" and watch the PSR single frame updates messages appear
on dmesg as we do flips on the non-PSR screen.

>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h         |  7 ++++---
>  drivers/gpu/drm/i915/intel_frontbuffer.c |  2 +-
>  drivers/gpu/drm/i915/intel_psr.c         | 22 +++++++++++++---------
>  3 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b7c69460fb20..0ba9065dec88 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1310,11 +1310,12 @@ void intel_backlight_unregister(struct drm_device *dev);
>  void intel_psr_enable(struct intel_dp *intel_dp);
>  void intel_psr_disable(struct intel_dp *intel_dp);
>  void intel_psr_invalidate(struct drm_device *dev,
> -                             unsigned frontbuffer_bits);
> +                         unsigned frontbuffer_bits);
>  void intel_psr_flush(struct drm_device *dev,
> -                        unsigned frontbuffer_bits);
> +                    unsigned frontbuffer_bits);
>  void intel_psr_init(struct drm_device *dev);
> -void intel_psr_single_frame_update(struct drm_device *dev);
> +void intel_psr_single_frame_update(struct drm_device *dev,
> +                                  unsigned frontbuffer_bits);
>
>  /* intel_runtime_pm.c */
>  int intel_power_domains_init(struct drm_i915_private *);
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index 3c4862c8629d..ede6ccc45375 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -190,7 +190,7 @@ void intel_frontbuffer_flip_prepare(struct drm_device *dev,
>         dev_priv->fb_tracking.busy_bits &= ~frontbuffer_bits;
>         mutex_unlock(&dev_priv->fb_tracking.lock);
>
> -       intel_psr_single_frame_update(dev);
> +       intel_psr_single_frame_update(dev, frontbuffer_bits);
>  }
>
>  /**
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index e354ceacb628..d79ba58637d7 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -596,13 +596,15 @@ static void intel_psr_exit(struct drm_device *dev)
>  /**
>   * intel_psr_single_frame_update - Single Frame Update
>   * @dev: DRM device
> + * @frontbuffer_bits: frontbuffer plane tracking bits
>   *
>   * Some platforms support a single frame update feature that is used to
>   * send and update only one frame on Remote Frame Buffer.
>   * So far it is only implemented for Valleyview and Cherryview because
>   * hardware requires this to be done before a page flip.
>   */
> -void intel_psr_single_frame_update(struct drm_device *dev)
> +void intel_psr_single_frame_update(struct drm_device *dev,
> +                                  unsigned frontbuffer_bits)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct drm_crtc *crtc;
> @@ -624,14 +626,16 @@ void intel_psr_single_frame_update(struct drm_device *dev)
>
>         crtc = dp_to_dig_port(dev_priv->psr.enabled)->base.base.crtc;
>         pipe = to_intel_crtc(crtc)->pipe;
> -       val = I915_READ(VLV_PSRCTL(pipe));
>
> -       /*
> -        * We need to set this bit before writing registers for a flip.
> -        * This bit will be self-clear when it gets to the PSR active state.
> -        */
> -       I915_WRITE(VLV_PSRCTL(pipe), val | VLV_EDP_PSR_SINGLE_FRAME_UPDATE);
> +       if (frontbuffer_bits & INTEL_FRONTBUFFER_ALL_MASK(pipe)) {
> +               val = I915_READ(VLV_PSRCTL(pipe));
>
> +               /*
> +                * We need to set this bit before writing registers for a flip.
> +                * This bit will be self-clear when it gets to the PSR active state.
> +                */
> +               I915_WRITE(VLV_PSRCTL(pipe), val | VLV_EDP_PSR_SINGLE_FRAME_UPDATE);
> +       }
>         mutex_unlock(&dev_priv->psr.lock);
>  }
>
> @@ -648,7 +652,7 @@ void intel_psr_single_frame_update(struct drm_device *dev)
>   * Dirty frontbuffers relevant to PSR are tracked in busy_frontbuffer_bits."
>   */
>  void intel_psr_invalidate(struct drm_device *dev,
> -                             unsigned frontbuffer_bits)
> +                         unsigned frontbuffer_bits)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct drm_crtc *crtc;
> @@ -685,7 +689,7 @@ void intel_psr_invalidate(struct drm_device *dev,
>   * Dirty frontbuffers relevant to PSR are tracked in busy_frontbuffer_bits.
>   */
>  void intel_psr_flush(struct drm_device *dev,
> -                        unsigned frontbuffer_bits)
> +                    unsigned frontbuffer_bits)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct drm_crtc *crtc;
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b7c69460fb20..0ba9065dec88 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1310,11 +1310,12 @@  void intel_backlight_unregister(struct drm_device *dev);
 void intel_psr_enable(struct intel_dp *intel_dp);
 void intel_psr_disable(struct intel_dp *intel_dp);
 void intel_psr_invalidate(struct drm_device *dev,
-			      unsigned frontbuffer_bits);
+			  unsigned frontbuffer_bits);
 void intel_psr_flush(struct drm_device *dev,
-			 unsigned frontbuffer_bits);
+		     unsigned frontbuffer_bits);
 void intel_psr_init(struct drm_device *dev);
-void intel_psr_single_frame_update(struct drm_device *dev);
+void intel_psr_single_frame_update(struct drm_device *dev,
+				   unsigned frontbuffer_bits);
 
 /* intel_runtime_pm.c */
 int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 3c4862c8629d..ede6ccc45375 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -190,7 +190,7 @@  void intel_frontbuffer_flip_prepare(struct drm_device *dev,
 	dev_priv->fb_tracking.busy_bits &= ~frontbuffer_bits;
 	mutex_unlock(&dev_priv->fb_tracking.lock);
 
-	intel_psr_single_frame_update(dev);
+	intel_psr_single_frame_update(dev, frontbuffer_bits);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index e354ceacb628..d79ba58637d7 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -596,13 +596,15 @@  static void intel_psr_exit(struct drm_device *dev)
 /**
  * intel_psr_single_frame_update - Single Frame Update
  * @dev: DRM device
+ * @frontbuffer_bits: frontbuffer plane tracking bits
  *
  * Some platforms support a single frame update feature that is used to
  * send and update only one frame on Remote Frame Buffer.
  * So far it is only implemented for Valleyview and Cherryview because
  * hardware requires this to be done before a page flip.
  */
-void intel_psr_single_frame_update(struct drm_device *dev)
+void intel_psr_single_frame_update(struct drm_device *dev,
+				   unsigned frontbuffer_bits)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
@@ -624,14 +626,16 @@  void intel_psr_single_frame_update(struct drm_device *dev)
 
 	crtc = dp_to_dig_port(dev_priv->psr.enabled)->base.base.crtc;
 	pipe = to_intel_crtc(crtc)->pipe;
-	val = I915_READ(VLV_PSRCTL(pipe));
 
-	/*
-	 * We need to set this bit before writing registers for a flip.
-	 * This bit will be self-clear when it gets to the PSR active state.
-	 */
-	I915_WRITE(VLV_PSRCTL(pipe), val | VLV_EDP_PSR_SINGLE_FRAME_UPDATE);
+	if (frontbuffer_bits & INTEL_FRONTBUFFER_ALL_MASK(pipe)) {
+		val = I915_READ(VLV_PSRCTL(pipe));
 
+		/*
+		 * We need to set this bit before writing registers for a flip.
+		 * This bit will be self-clear when it gets to the PSR active state.
+		 */
+		I915_WRITE(VLV_PSRCTL(pipe), val | VLV_EDP_PSR_SINGLE_FRAME_UPDATE);
+	}
 	mutex_unlock(&dev_priv->psr.lock);
 }
 
@@ -648,7 +652,7 @@  void intel_psr_single_frame_update(struct drm_device *dev)
  * Dirty frontbuffers relevant to PSR are tracked in busy_frontbuffer_bits."
  */
 void intel_psr_invalidate(struct drm_device *dev,
-			      unsigned frontbuffer_bits)
+			  unsigned frontbuffer_bits)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
@@ -685,7 +689,7 @@  void intel_psr_invalidate(struct drm_device *dev,
  * Dirty frontbuffers relevant to PSR are tracked in busy_frontbuffer_bits.
  */
 void intel_psr_flush(struct drm_device *dev,
-			 unsigned frontbuffer_bits)
+		     unsigned frontbuffer_bits)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;