[Intel-gfx] [PATCH] drm/i915: Add Baytrail PSR Support.

[Daniel Vetter]

On Thu, Jan 30, 2014 at 03:01:08PM -0200, Rodrigo Vivi wrote:
> On Thu, Jan 30, 2014 at 11:02 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > On Wed, Jan 29, 2014 at 01:50:06PM -0200, Rodrigo Vivi wrote:
> >> @@ -7501,6 +7501,9 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
> >>       u32 base = 0, pos = 0;
> >>       bool visible;
> >>
> >> +     if (IS_VALLEYVIEW(dev))
> >> +             intel_edp_psr_inactivate(dev);
> >> +
> >
> > Inactivate means that we turn off PSR for some period of time until idle
> > again, right?
> 
> Yes, right.
> 
> > In the case of a moving cursor that means indefinitely.
> That's true... So I think we really need a work queue delaying the enable.
> Or do you have any better idea?

Yeah, sounds like we need a delayed work-queue to re-enable psr, also for
gtt mmap writes. See Chris' latest crazy example of what the X server is
currently allowed to do.

> > Also it sounds overkill to have to disable PSR just for the cursor
> > sprite. Is there not some preferrable alternatives or hw that dtrt?
> 
> Yeap, I totally agree. The other option implemented by other drivers
> is to use PSR SW control mode where driver controls all entry and exit
> flow, since Baytrail cannot do it right on HW mode. But even the force
> exit on this case was using the PSR_RESET that I'm using on inactivate
> function. and they exported that over ioctl.
> So I preferred to let all this on kernel side using hardware as much
> as possible and workarounding the cases where hardware buggy on screen
> update detection, although it looks like an overkill for a single
> cursor update. :/

Yeah, exporting psr to userspace is imo a complete no-go. We can give
userspace hints and eventually go to an explicit damage tracking model,
but it's ultimately the kernel's responsibility to shovel pixels to the
screen.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch