[Intel-gfx] [PATCH] drm/i915: set scan-buffer as uncached on Sandybridge

[Zou, Nanhai]

Hi Chris,
	We only need front buffer to be uncached.
	I think that will not affect performance since GPU will seldom read back from front buffer. 
	We will measure performance on that.
Thanks
Zou Nan hai

>>-----Original Message-----
>>From: intel-gfx-bounces+nanhai.zou=intel.com at lists.freedesktop.org
>>[mailto:intel-gfx-bounces+nanhai.zou=intel.com at lists.freedesktop.org] On
>>Behalf Of Chris Wilson
>>Sent: 2010年10月25日 16:23
>>To: Zhenyu Wang; intel-gfx at lists.freedesktop.org
>>Cc: stable team
>>Subject: Re: [Intel-gfx] [PATCH] drm/i915: set scan-buffer as uncached on
>>Sandybridge
>>
>>[I haven't seen my first email arrive yet...]
>>
>>On Mon, 25 Oct 2010 09:42:31 +0800, Zhenyu Wang <zhenyuw at linux.intel.com>
>>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
>>_______________________________________________
>>Intel-gfx mailing list
>>Intel-gfx at lists.freedesktop.org
>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx