All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Treat WC a separate cache domain
Date: Fri, 07 Apr 2017 10:16:52 +0300	[thread overview]
Message-ID: <1491549412.3493.6.camel@linux.intel.com> (raw)
In-Reply-To: <20170405210741.29551-1-chris@chris-wilson.co.uk>

On ke, 2017-04-05 at 22:07 +0100, Chris Wilson wrote:
> When discussing a new WC mmap, we based the interface upon the
> assumption that GTT was fully coherent. How naive! Commits 3b5724d702ef
> ("drm/i915: Wait for writes through the GTT to land before reading
> back") and ed4596ea992d ("drm/i915/guc: WA to address the Ringbuffer
> coherency issue") demonstrate that writes through the GTT are indeed
> delayed and may be overtaken by direct WC access. To be safe, if
> userspace is mixing WC mmaps with other potential GTT access (pwrite,
> GTT mmaps) it should use set_domain(WC).
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96563
> Testcase: igt/gem_pwrite/small-gtt*
> Testcase: igt/drv_selftest/coherency
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> +int
> +i915_gem_object_set_to_wc_domain(struct drm_i915_gem_object *obj, bool write)
> +{
> +	int ret;
> +
> +	lockdep_assert_held(&obj->base.dev->struct_mutex);
> +
> > +	ret = i915_gem_object_wait(obj,
> > +				   I915_WAIT_INTERRUPTIBLE |
> > +				   I915_WAIT_LOCKED |
> +				   (write ? I915_WAIT_ALL : 0),

Could construct into flags variable.

> +	/* Flush and acquire obj->pages so that we are coherent through
> +	 * direct access in memory with previous cached writes through
> +	 * shmemfs and that our cache domain tracking remains valid.
> +	 * For example, if the obj->filp was moved to swap without us
> +	 * being notified and releasing the pages, we would mistakenly
> +	 * continue to assume that the obj remained out of the CPU cached
> +	 * domain.
> +	 */
> +	ret = i915_gem_object_pin_pages(obj);
> +	if (ret)
> +		return ret;
> +
> +	i915_gem_object_flush_cpu_write_domain(obj);
> +	i915_gem_object_flush_gtt_write_domain(obj);

I was thinking if WC should be a "substate" of CPU domain, because we
don't have i915_gem_object_flush_wc_write_domain.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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

  parent reply	other threads:[~2017-04-07  7:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-05 21:07 [PATCH] drm/i915: Treat WC a separate cache domain Chris Wilson
2017-04-05 21:25 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-04-07  7:16 ` Joonas Lahtinen [this message]
2017-04-07  7:44   ` [PATCH] " Chris Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1491549412.3493.6.camel@linux.intel.com \
    --to=joonas.lahtinen@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.