[Intel-gfx] [PATCH 8/9] drm/i915/psr: Restrict single-shot updates to the PSR pipe

[Paulo Zanoni]

2015-06-18 5:30 GMT-03:00 Daniel Vetter <daniel.vetter at 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 at 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 at intel.com>
> Cc: Durgadoss R <durgadoss.r at intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni