[Intel-gfx] [PATCH] drm/i915: Update write_domains on active list after flush.

[Daniel Vetter]

Hi Chris,

You've beaten me by a few hours - I've just noticed your email while I was
prepping my own forward port. One issue I've noticed while forward-proting
your patch, see the inlined comment.

-Daniel

On Sun, Jan 31, 2010 at 03:55:10PM +0000, Chris Wilson wrote:
... snipped ..

> @@ -1631,21 +1633,21 @@ i915_add_request(struct drm_device *dev, struct drm_file *file_priv,
>  		INIT_LIST_HEAD(&request->client_list);
>  	}
>  
> -	/* Associate any objects on the flushing list matching the write
> +	/* Associate any objects on the gpu_write_list matching the write
>  	 * domain we're flushing with our flush.
>  	 */
> -	if (flush_domains != 0) {
> +	if (flush_domains & I915_GEM_GPU_DOMAINS) {
>  		struct drm_i915_gem_object *obj_priv, *next;
>  
>  		list_for_each_entry_safe(obj_priv, next,
> -					 &dev_priv->mm.flushing_list, list) {
> +					 &dev_priv->mm.gpu_write_list,
> +					 gpu_write_list) {
>  			struct drm_gem_object *obj = obj_priv->obj;
>  
> -			if ((obj->write_domain & flush_domains) ==
> -			    obj->write_domain) {
> +			if (obj->write_domain & flush_domains) {

IMHO this change will screw up write_domain accounting iff you allow
userspace to set multiple write domains. I still think allowing that is a
gross hack, but that's a rant for a separate email ;)

>  				uint32_t old_write_domain = obj->write_domain;
> -
>  				obj->write_domain = 0;
> +				list_del_init(&obj_priv->gpu_write_list);
>  				i915_gem_object_move_to_active(obj, seqno);
>  
>  				trace_i915_gem_object_change_domain(obj,
-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48