| Message ID | 1434616228-28358-8-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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;
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(-)