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@xxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxx>
Queued for -next, thanks for the review.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
