2015-02-11 6:30 GMT-02:00 Daniel Vetter <daniel@xxxxxxxx>: > On Mon, Feb 09, 2015 at 07:41:17PM +0100, Daniel Vetter wrote: >> On Mon, Feb 09, 2015 at 02:46:31PM -0200, Paulo Zanoni wrote: >> > From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> >> > >> > We need this for FBC, and possibly for PSR too. >> > >> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> >> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> >> > --- >> > drivers/gpu/drm/i915/i915_gem.c | 4 ++++ >> > 1 file changed, 4 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> > index 3d198f8..15910fa 100644 >> > --- a/drivers/gpu/drm/i915/i915_gem.c >> > +++ b/drivers/gpu/drm/i915/i915_gem.c >> > @@ -1111,6 +1111,10 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, >> > ret = i915_gem_phys_pwrite(obj, args, file); >> > else >> > ret = i915_gem_shmem_pwrite(dev, obj, args, file); >> > + >> > + intel_fb_obj_flush(obj, false, ORIGIN_CPU); >> > + } else { >> > + intel_fb_obj_flush(obj, false, ORIGIN_GTT); >> >> A flush alone does nothing. Well should, but you're kinda not quite using >> it correctly in the next patch to convert fbc over to frontbuffer >> tracking. >> >> I guess the docs aren't perfect, so let me try again. There are two kinds >> of events the frontbuffer tracking code supplies to tell its consumers >> that screen changes are happening: >> - invalidate/flush: Invalidate denotes the start of the frontbuffer >> rendering, from that point on psr/fbc/drrs must update the screen with >> the usual refresh rate and not cache anything anywhere. When the flush >> happens (which could easily be after a _very_ long time, e.g. fbcon) >> then only can caching recomence. Caching = enable fbc, allow psr or >> reduce refresh rate. >> - flip events: That's an instantenous event (well at least for consumer, >> internally we need to track it as prepare/complete for async flips), and >> mostly just interesting when the hw doesn't notice flips (some psr modes >> and drrs). >> >> So if you want to add frontbuffer tracking to pwrite then we need both an >> invalidate (before the actual pwrite) and a flush (after the pwrite, like >> you've added here). >> >> The other issue is that there's a bug with the origin assignemnt: >> phys_pwrite also goes through the gtt. I think it would be best if we push >> the fb_obj_invalidate/flush into the relevant pwrite functions. That >> should make it easier to review, since the invalidate/flush will be next >> to the write op. > > btw what's the use-case here? We don't upload stuff to X-tiled buffers > with pwrite, so this isn't really relevant for fbc I think. We can do this, I wrote a testcase for that. Of course, user space needs to be aware of the tiling, but that's documented on the PRM. > > It is a real gap for psr though, since cursor updates are done with > pwrite. A lot of the tests I wrote can be used by PSR, I should plan on doing this later. > But that probably gets papered over by X also updating the > position when the image changes, which means we'll get another (hw) > flip combo. But that's just X, and we indeed don't have a pwrite cursor > case yet in the psr testcase. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx