| Message ID | 1434616228-28358-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 18, 2015 at 10:30:20AM +0200, Daniel Vetter wrote: > The current/old frontbuffer might still have gpu frontbuffer rendering > pending. But once flipped it won't have the corresponding frontbuffer > bits any more and hence the request retire function won't ever clear > the corresponding busy bits. The async flip tracking (with the > flip_prepare and flip_complete functions) already does this, but > somehow I've forgotten to do this for synchronous flips. > > Note that we don't track outstanding rendering of the new framebuffer > with busy_bits since all our plane update code waits for previous > rendering to complete before displaying a new buffer. Hence a new > buffer will never be busy. > > Reported-by: Paulo Zanoni <przanoni@gmail.com> > Cc: Paulo Zanoni <przanoni@gmail.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > Paulo promised to also extend kms_frontbuffer_tracking with flip vs. > busy buffer tests. > --- > drivers/gpu/drm/i915/intel_drv.h | 18 ++---------------- > drivers/gpu/drm/i915/intel_frontbuffer.c | 26 ++++++++++++++++++++++++++ > 2 files changed, 28 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index bcafefcf048b..b7c69460fb20 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -963,23 +963,9 @@ void intel_frontbuffer_flip_complete(struct drm_device *dev, > unsigned frontbuffer_bits); > void intel_frontbuffer_flush(struct drm_device *dev, > unsigned frontbuffer_bits); > -/** > - * intel_frontbuffer_flip - synchronous frontbuffer flip > - * @dev: DRM device > - * @frontbuffer_bits: frontbuffer plane tracking bits > - * > - * This function gets called after scheduling a flip on @obj. This is for > - * synchronous plane updates which will happen on the next vblank and which will > - * not get delayed by pending gpu rendering. > - * > - * Can be called without any locks held. > - */ > -static inline > +inline inline? > void intel_frontbuffer_flip(struct drm_device *dev, > - unsigned frontbuffer_bits) > -{ > - intel_frontbuffer_flush(dev, frontbuffer_bits); > -} > + unsigned frontbuffer_bits); > > unsigned int intel_fb_align_height(struct drm_device *dev, > unsigned int height, > diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c > index 57095f54c1f2..2a1611a7ce1d 100644 > --- a/drivers/gpu/drm/i915/intel_frontbuffer.c > +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c > @@ -270,3 +270,29 @@ void intel_frontbuffer_flip_complete(struct drm_device *dev, > > intel_frontbuffer_flush(dev, frontbuffer_bits); > } > + > +/** > + * intel_frontbuffer_flip - synchronous frontbuffer flip > + * @dev: DRM device > + * @frontbuffer_bits: frontbuffer plane tracking bits > + * > + * This function gets called after scheduling a flip on @obj. This is for > + * synchronous plane updates which will happen on the next vblank and which will > + * not get delayed by pending gpu rendering. > + * > + * Can be called without any locks held. > + */ > +inline > +void intel_frontbuffer_flip(struct drm_device *dev, > + unsigned frontbuffer_bits) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + mutex_lock(&dev_priv->fb_tracking.lock); > + dev_priv->fb_tracking.flip_bits |= frontbuffer_bits; > + /* Remove stale busy bits due to the old buffer. */ > + dev_priv->fb_tracking.busy_bits &= ~frontbuffer_bits; > + mutex_unlock(&dev_priv->fb_tracking.lock); > + > + intel_frontbuffer_flush(dev, frontbuffer_bits); > +} > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Jun 18, 2015 at 11:32:53AM +0300, Ville Syrjälä wrote: > On Thu, Jun 18, 2015 at 10:30:20AM +0200, Daniel Vetter wrote: > > The current/old frontbuffer might still have gpu frontbuffer rendering > > pending. But once flipped it won't have the corresponding frontbuffer > > bits any more and hence the request retire function won't ever clear > > the corresponding busy bits. The async flip tracking (with the > > flip_prepare and flip_complete functions) already does this, but > > somehow I've forgotten to do this for synchronous flips. > > > > Note that we don't track outstanding rendering of the new framebuffer > > with busy_bits since all our plane update code waits for previous > > rendering to complete before displaying a new buffer. Hence a new > > buffer will never be busy. > > > > Reported-by: Paulo Zanoni <przanoni@gmail.com> > > Cc: Paulo Zanoni <przanoni@gmail.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > Paulo promised to also extend kms_frontbuffer_tracking with flip vs. > > busy buffer tests. > > --- > > drivers/gpu/drm/i915/intel_drv.h | 18 ++---------------- > > drivers/gpu/drm/i915/intel_frontbuffer.c | 26 ++++++++++++++++++++++++++ > > 2 files changed, 28 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index bcafefcf048b..b7c69460fb20 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -963,23 +963,9 @@ void intel_frontbuffer_flip_complete(struct drm_device *dev, > > unsigned frontbuffer_bits); > > void intel_frontbuffer_flush(struct drm_device *dev, > > unsigned frontbuffer_bits); > > -/** > > - * intel_frontbuffer_flip - synchronous frontbuffer flip > > - * @dev: DRM device > > - * @frontbuffer_bits: frontbuffer plane tracking bits > > - * > > - * This function gets called after scheduling a flip on @obj. This is for > > - * synchronous plane updates which will happen on the next vblank and which will > > - * not get delayed by pending gpu rendering. > > - * > > - * Can be called without any locks held. > > - */ > > -static inline > > +inline > > inline? Daniel's probably looked at a perf profile and felt guilty. :) Hijacking the thread purely to promote http://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=nightly&id=fe05042a9dda46f4bdd9e40612bf48befaee1ad3 -Chris
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index bcafefcf048b..b7c69460fb20 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -963,23 +963,9 @@ void intel_frontbuffer_flip_complete(struct drm_device *dev, unsigned frontbuffer_bits); void intel_frontbuffer_flush(struct drm_device *dev, unsigned frontbuffer_bits); -/** - * intel_frontbuffer_flip - synchronous frontbuffer flip - * @dev: DRM device - * @frontbuffer_bits: frontbuffer plane tracking bits - * - * This function gets called after scheduling a flip on @obj. This is for - * synchronous plane updates which will happen on the next vblank and which will - * not get delayed by pending gpu rendering. - * - * Can be called without any locks held. - */ -static inline +inline void intel_frontbuffer_flip(struct drm_device *dev, - unsigned frontbuffer_bits) -{ - intel_frontbuffer_flush(dev, frontbuffer_bits); -} + unsigned frontbuffer_bits); unsigned int intel_fb_align_height(struct drm_device *dev, unsigned int height, diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c index 57095f54c1f2..2a1611a7ce1d 100644 --- a/drivers/gpu/drm/i915/intel_frontbuffer.c +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c @@ -270,3 +270,29 @@ void intel_frontbuffer_flip_complete(struct drm_device *dev, intel_frontbuffer_flush(dev, frontbuffer_bits); } + +/** + * intel_frontbuffer_flip - synchronous frontbuffer flip + * @dev: DRM device + * @frontbuffer_bits: frontbuffer plane tracking bits + * + * This function gets called after scheduling a flip on @obj. This is for + * synchronous plane updates which will happen on the next vblank and which will + * not get delayed by pending gpu rendering. + * + * Can be called without any locks held. + */ +inline +void intel_frontbuffer_flip(struct drm_device *dev, + unsigned frontbuffer_bits) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + + mutex_lock(&dev_priv->fb_tracking.lock); + dev_priv->fb_tracking.flip_bits |= frontbuffer_bits; + /* Remove stale busy bits due to the old buffer. */ + dev_priv->fb_tracking.busy_bits &= ~frontbuffer_bits; + mutex_unlock(&dev_priv->fb_tracking.lock); + + intel_frontbuffer_flush(dev, frontbuffer_bits); +}
The current/old frontbuffer might still have gpu frontbuffer rendering pending. But once flipped it won't have the corresponding frontbuffer bits any more and hence the request retire function won't ever clear the corresponding busy bits. The async flip tracking (with the flip_prepare and flip_complete functions) already does this, but somehow I've forgotten to do this for synchronous flips. Note that we don't track outstanding rendering of the new framebuffer with busy_bits since all our plane update code waits for previous rendering to complete before displaying a new buffer. Hence a new buffer will never be busy. Reported-by: Paulo Zanoni <przanoni@gmail.com> Cc: Paulo Zanoni <przanoni@gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- Paulo promised to also extend kms_frontbuffer_tracking with flip vs. busy buffer tests. --- drivers/gpu/drm/i915/intel_drv.h | 18 ++---------------- drivers/gpu/drm/i915/intel_frontbuffer.c | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 16 deletions(-)