From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: Track frontbuffer invalidation/flushing Date: Thu, 19 Jun 2014 15:54:49 +0200 Message-ID: <20140619135449.GU5821@phenom.ffwll.local> References: <1403092757-31421-15-git-send-email-daniel.vetter@ffwll.ch> <1403181687-26717-1-git-send-email-daniel.vetter@ffwll.ch> <20140619130226.GE25975@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f174.google.com (mail-we0-f174.google.com [74.125.82.174]) by gabe.freedesktop.org (Postfix) with ESMTP id 9764C6E8BD for ; Thu, 19 Jun 2014 06:54:57 -0700 (PDT) Received: by mail-we0-f174.google.com with SMTP id u57so2421293wes.33 for ; Thu, 19 Jun 2014 06:54:56 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140619130226.GE25975@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Daniel Vetter , Intel Graphics Development , Rodrigo Vivi List-Id: intel-gfx@lists.freedesktop.org On Thu, Jun 19, 2014 at 02:02:26PM +0100, Chris Wilson wrote: > On Thu, Jun 19, 2014 at 02:41:27PM +0200, Daniel Vetter wrote: > > @@ -3626,6 +3628,8 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write) > > obj->dirty = 1; > > } > > > > + intel_fb_obj_invalidate(obj, NULL); > We should only invalidate when we start writing. > > > + > > trace_i915_gem_object_change_domain(obj, > > old_read_domains, > > old_write_domain); > > @@ -3962,6 +3966,8 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write) > > obj->base.write_domain = I915_GEM_DOMAIN_CPU; > > } > > > > + intel_fb_obj_invalidate(obj, NULL); > Again only for writes. Now I think that this is skippable as the CPU > domain is explicitly incoherent with display, but for symmetry of > invalidate/flush it is required. Good call, will fix up both places. > > > + > > trace_i915_gem_object_change_domain(obj, > > old_read_domains, > > old_write_domain); > > > +void intel_frontbuffer_flush(struct drm_device *dev, > > + unsigned frontbuffer_bits) > > +{ > > + struct drm_i915_private *dev_priv = dev->dev_private; > > + > > + /* Delay flushing when rings are still busy.*/ > > + mutex_lock(&dev_priv->fb_tracking.lock); > > + frontbuffer_bits &= ~dev_priv->fb_tracking.busy_bits; > > + mutex_unlock(&dev_priv->fb_tracking.lock); > > + > > + intel_mark_fb_busy(dev, frontbuffer_bits, NULL); > > + > > + intel_edp_psr_exit(dev); > Please tell me to plan to change this functions name. As we certainly > don't want to disable PSR here, but hopefully reenable it. Yeah, the psr patch on top of this splits psr_exit into psr_invalidate and psr_flush and generally dtrt. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch