Re: [PATCH 1/2] drm/i915: Combine write_domain flushes to a single function

From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/i915: Combine write_domain flushes to a single function
Date: Wed, 12 Apr 2017 11:42:07 +0300	[thread overview]
Message-ID: <1491986527.3274.12.camel@linux.intel.com> (raw)
In-Reply-To: <20170411134729.9940-1-chris@chris-wilson.co.uk>

On ti, 2017-04-11 at 14:47 +0100, Chris Wilson wrote:
> In the next patch, we will introduce a new cache domain for
> differentiating between GTT access and direct WC access. This will
> require us to include WC in our write_domain flushes. Rather than
> duplicate a third function, combine the existing two into one and
> flushing WC writes will then be automatically handled as well.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> +static inline enum fb_op_origin
> +write_origin(struct drm_i915_gem_object *obj, unsigned int domain)

fb_write_origin? Just for added clarity.

> +{
> +	return (domain == I915_GEM_DOMAIN_GTT ?
> +		obj->frontbuffer_ggtt_origin : ORIGIN_CPU);
> +}

<SNIP>

> @@ -794,7 +848,7 @@ int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj,
>  			goto out;
>  	}
>  
> -	i915_gem_object_flush_gtt_write_domain(obj);
> +	flush_write_domain(obj, I915_GEM_DOMAIN_CPU);

Let me tell ya, this reads really confusing. Maybe

	flush_write_domain(obj, ~I915_GEM_DOMAIN_CPU);

and call the parameter "mask" instead of "exclude..."

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx