| Message ID | 1436253040-20151-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 07, 2015 at 09:10:40AM +0200, Daniel Vetter wrote: > Burning cpu cycles isn't awesome, so use sleeps instead. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 2 +- > drivers/gpu/drm/i915/intel_pm.c | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 56316c1e945c..87be3f307e40 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1026,7 +1026,7 @@ static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe) > line_mask = DSL_LINEMASK_GEN3; > > line1 = I915_READ(reg) & line_mask; > - mdelay(5); > + msleep(5); msleep() is quite inaccurate, but the accuracy of the delay shouldn't matter too much here. The only functional worry would be if it manages to sleep for exactly one frame and then we'd think DSL had stopped. But since we've already told the pipe to shut down, I don't think it should ever end up scanning out more than one frame here, so I suppose that problem should never happen. So yeah, this seems safe. It might make the modeset a bit slower if it ends up sleeping for a long time, but I suppose it shouldn't be too bad. If we want to optimize for that we could use usleep_range() and change the sleep duration to be just a few scanlines. > line2 = I915_READ(reg) & line_mask; > > return line1 == line2; > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 6eb5d76e6912..1efac89cb738 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4255,7 +4255,7 @@ static void ironlake_enable_drps(struct drm_device *dev) > > if (wait_for_atomic((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 10)) > DRM_ERROR("stuck trying to change perf mode\n"); > - mdelay(1); > + msleep(1); > > ironlake_set_drps(dev, fstart); > > @@ -4286,10 +4286,10 @@ static void ironlake_disable_drps(struct drm_device *dev) > > /* Go back to the starting frequency */ > ironlake_set_drps(dev, dev_priv->ips.fstart); > - mdelay(1); > + msleep(1); > rgvswctl |= MEMCTL_CMD_STS; > I915_WRITE(MEMSWCTL, rgvswctl); > - mdelay(1); > + msleep(1); Init/resume stuff. Doesn't seem too performance critical so msleep() ought to be fine. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > spin_unlock_irq(&mchdev_lock); > } > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Jul 07, 2015 at 11:03:10AM +0300, Ville Syrjälä wrote: > On Tue, Jul 07, 2015 at 09:10:40AM +0200, Daniel Vetter wrote: > > Burning cpu cycles isn't awesome, so use sleeps instead. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 2 +- > > drivers/gpu/drm/i915/intel_pm.c | 6 +++--- > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 56316c1e945c..87be3f307e40 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -1026,7 +1026,7 @@ static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe) > > line_mask = DSL_LINEMASK_GEN3; > > > > line1 = I915_READ(reg) & line_mask; > > - mdelay(5); > > + msleep(5); > > msleep() is quite inaccurate, but the accuracy of the delay shouldn't matter > too much here. The only functional worry would be if it manages to sleep for > exactly one frame and then we'd think DSL had stopped. But since we've already > told the pipe to shut down, I don't think it should ever end up scanning out > more than one frame here, so I suppose that problem should never happen. > So yeah, this seems safe. > > It might make the modeset a bit slower if it ends up sleeping for a long > time, but I suppose it shouldn't be too bad. If we want to optimize for > that we could use usleep_range() and change the sleep duration to be > just a few scanlines. What might be worth a shot is testing whether we get one last vblank when the pipe goes off. Then we could augment the wait_for loop with a wait_event_timeout based loop instead of just msleep. Still means we need to sleep for a bit on gen2/3 to observe that the dsl really stopped, but on gen4+ we might be able to proceed without delays. > > > line2 = I915_READ(reg) & line_mask; > > > > return line1 == line2; > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index 6eb5d76e6912..1efac89cb738 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -4255,7 +4255,7 @@ static void ironlake_enable_drps(struct drm_device *dev) > > > > if (wait_for_atomic((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 10)) > > DRM_ERROR("stuck trying to change perf mode\n"); > > - mdelay(1); > > + msleep(1); > > > > ironlake_set_drps(dev, fstart); > > > > @@ -4286,10 +4286,10 @@ static void ironlake_disable_drps(struct drm_device *dev) > > > > /* Go back to the starting frequency */ > > ironlake_set_drps(dev, dev_priv->ips.fstart); > > - mdelay(1); > > + msleep(1); > > rgvswctl |= MEMCTL_CMD_STS; > > I915_WRITE(MEMSWCTL, rgvswctl); > > - mdelay(1); > > + msleep(1); > > Init/resume stuff. Doesn't seem too performance critical so msleep() > ought to be fine. > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Queued for -next, thanks for the review. -Daniel
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6738
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK 302/302 302/302
SNB 312/316 312/316
IVB 343/343 343/343
BYT -3 287/287 284/287
HSW 380/380 380/380
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*BYT igt@gem_partial_pwrite_pread@reads-display PASS(1) FAIL(1)
*BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1)
*BYT igt@gem_tiled_partial_pwrite_pread@reads PASS(1) FAIL(1)
Note: You need to pay more attention to line start with '*'
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 56316c1e945c..87be3f307e40 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1026,7 +1026,7 @@ static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe) line_mask = DSL_LINEMASK_GEN3; line1 = I915_READ(reg) & line_mask; - mdelay(5); + msleep(5); line2 = I915_READ(reg) & line_mask; return line1 == line2; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 6eb5d76e6912..1efac89cb738 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4255,7 +4255,7 @@ static void ironlake_enable_drps(struct drm_device *dev) if (wait_for_atomic((I915_READ(MEMSWCTL) & MEMCTL_CMD_STS) == 0, 10)) DRM_ERROR("stuck trying to change perf mode\n"); - mdelay(1); + msleep(1); ironlake_set_drps(dev, fstart); @@ -4286,10 +4286,10 @@ static void ironlake_disable_drps(struct drm_device *dev) /* Go back to the starting frequency */ ironlake_set_drps(dev, dev_priv->ips.fstart); - mdelay(1); + msleep(1); rgvswctl |= MEMCTL_CMD_STS; I915_WRITE(MEMSWCTL, rgvswctl); - mdelay(1); + msleep(1); spin_unlock_irq(&mchdev_lock); }
Burning cpu cycles isn't awesome, so use sleeps instead. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 2 +- drivers/gpu/drm/i915/intel_pm.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)