From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH] drm/i915: set scan-buffer as uncached on Sandybridge Date: Mon, 25 Oct 2010 09:22:41 +0100 Message-ID: <849307$a48ipo@azsmga001.ch.intel.com> References: <1287970951-3677-1-git-send-email-zhenyuw@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id C11879E769 for ; Mon, 25 Oct 2010 01:22:46 -0700 (PDT) In-Reply-To: <1287970951-3677-1-git-send-email-zhenyuw@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Zhenyu Wang , intel-gfx@lists.freedesktop.org Cc: stable team List-Id: intel-gfx@lists.freedesktop.org [I haven't seen my first email arrive yet...] On Mon, 25 Oct 2010 09:42:31 +0800, Zhenyu Wang wrote: > Display engine on Sandybridge is not coherent with LLC, so > try to always bind display buffer as uncached on Sandybridge. > This fixed screen artifacts seen by using blit engine on Sandybridge. [snip] > @@ -1579,6 +1579,14 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > obj = intel_fb->obj; > obj_priv = to_intel_bo(obj); > > + /* > + * Set uncacheable for scan-out buffer on Sandybridge. > + * Display engine is non-coherent with LLC, and read from > + * main memory only. This ensures data is coherent with display. > + */ > + if (IS_GEN6(dev)) > + obj_priv->agp_type = AGP_USER_UNCACHED_MEMORY; > + [Quick recap of first email] You can't do this here, changing the AGP type of a potentially bound object without any locking. Move this into intel_pin_and_fence_fb_obj(), give it a decent name and make it robust. int i915_gem_object_enable_scanout(struct drm_i915_gem_object *obj) { int type = IS_GEN6(dev) ? AGP_USER_UNCACHED_MEMORY : AGP_USER_MEMORY; /* These names are absolutely terrible. we should have just made * AGR_USER_CACHED_MEMORY the default on gen6 and kept the AGP_USER_MEMORY * as uncached on all generations, rather than mixing up the cacheability * status of common enums. */ if (obj->agp_type == type) return 0; if (obj->gtt_space) { int ret = i915_gem_object_unbind(&obj->base); if (ret) return ret; } obj->agp_type = type; return 0; } [And the additional bit of insight...] Also add a call from intel_user_framebuffer_create, as being the earliest opportunity to avoid the unbind penalty. What is the performance impact of rendering to an uncached buffer on Sandybridge? Should we only be changing the caching status whilst pinned as the framebuffer? Rather than unbinding/rebinding, should we introduce a function [once we have a native GTT interface, Daniel!] to simply update the flags on the PTEs for an object (obviously with suitable flushing). -Chris -- Chris Wilson, Intel Open Source Technology Centre