Em Qui, 2016-01-21 às 15:17 +0100, Maarten Lankhorst escreveu: > Op 19-01-16 om 14:35 schreef Paulo Zanoni: > > Instead of waiting for 50ms, just wait until the next vblank, since > > it's the minimum requirement. The whole infrastructure of FBC is > > based > > on vblanks, so waiting for X vblanks instead of X milliseconds > > sounds > > like the correct way to go. Besides, 50ms may be less than a vblank > > on > > super slow modes that may or may not exist. > > > > There are some small improvements in PC state residency (due to the > > fact that we're now using 16ms for the common modes instead of > > 50ms), > > but the biggest advantage is still the correctness of being > > vblank-based instead of time-based. > > > > v2: > > - Rebase after changing the patch order. > > - Update the commit message. > > v3: > > - Fix bogus vblank_get() instead of vblank_count() (Ville). > > - Don't forget to call drm_crtc_vblank_{get,put} (Chris, Ville) > > - Adjust the performance details on the commit message. > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > drivers/gpu/drm/i915/intel_fbc.c | 43 > > ++++++++++++++++++++++++++++------------ > > 2 files changed, 31 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index af30148..33217a4 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -925,9 +925,9 @@ struct i915_fbc { > > > > struct intel_fbc_work { > > bool scheduled; > > + u32 scheduled_vblank; > > struct work_struct work; > > struct drm_framebuffer *fb; > > - unsigned long enable_jiffies; > > } work; > > > > const char *no_fbc_reason; > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c > > b/drivers/gpu/drm/i915/intel_fbc.c > > index a1988a4..6b43ec3 100644 > > --- a/drivers/gpu/drm/i915/intel_fbc.c > > +++ b/drivers/gpu/drm/i915/intel_fbc.c > > @@ -381,7 +381,15 @@ static void intel_fbc_work_fn(struct > > work_struct *__work) > > container_of(__work, struct drm_i915_private, > > fbc.work.work); > > struct intel_fbc_work *work = &dev_priv->fbc.work; > > struct intel_crtc *crtc = dev_priv->fbc.crtc; > > - int delay_ms = 50; > > + struct drm_vblank_crtc *vblank = &dev_priv->dev- > > >vblank[crtc->pipe]; > > + > > + mutex_lock(&dev_priv->fbc.lock); > > + if (drm_crtc_vblank_get(&crtc->base)) { > > + DRM_ERROR("vblank not available for FBC on pipe > > %c\n", > > + pipe_name(crtc->pipe)); > > + goto out; > > + } > > + mutex_unlock(&dev_priv->fbc.lock); > What does the lock protect here? Hmmm, yeah, it protects nothing... Well observed. > > ~Maarten > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx