public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel@ffwll.ch>,
	akash.goel@intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915 : Avoid superfluous invalidation of CPU cache lines
Date: Wed, 25 Nov 2015 13:02:20 +0200	[thread overview]
Message-ID: <20151125110220.GM4437@intel.com> (raw)
In-Reply-To: <20151124223938.GC16277@nuc-i3427.alporthouse.com>

On Tue, Nov 24, 2015 at 10:39:38PM +0000, Chris Wilson wrote:
> On Tue, Nov 24, 2015 at 07:14:31PM +0100, Daniel Vetter wrote:
> > On Tue, Nov 24, 2015 at 12:04:06PM +0200, Ville Syrjälä wrote:
> > > On Tue, Nov 24, 2015 at 03:35:24PM +0530, akash.goel@intel.com wrote:
> > > > From: Akash Goel <akash.goel@intel.com>
> > > > 
> > > > When the object is moved out of CPU read domain, the cachelines
> > > > are not invalidated immediately. The invalidation is deferred till
> > > > next time the object is brought back into CPU read domain.
> > > > But the invalidation is done unconditionally, i.e. even for the case
> > > > where the cachelines were flushed previously, when the object moved out
> > > > of CPU write domain. This is avoidable and would lead to some optimization.
> > > > Though this is not a hypothetical case, but is unlikely to occur often.
> > > > The aim is to detect changes to the backing storage whilst the
> > > > data is potentially in the CPU cache, and only clflush in those case.
> > > > 
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Signed-off-by: Akash Goel <akash.goel@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.h | 1 +
> > > >  drivers/gpu/drm/i915/i915_gem.c | 9 ++++++++-
> > > >  2 files changed, 9 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > index df9316f..fedb71d 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -2098,6 +2098,7 @@ struct drm_i915_gem_object {
> > > >  	unsigned long gt_ro:1;
> > > >  	unsigned int cache_level:3;
> > > >  	unsigned int cache_dirty:1;
> > > > +	unsigned int cache_clean:1;
> > > 
> > > So now we have cache_dirty and cache_clean which seems redundant,
> > > except somehow cache_dirty != !cache_clean?
> 
> Exactly, not entirely redundant. I did think something along MESI lines
> would be useful, but that didn't capture the different meanings we
> employ.
> 
> cache_dirty tracks whether we have been eliding the clflush.
> 
> cache_clean tracks whether we know the cache has been completely
> clflushed.

Can we know that with speculative prefetching and whatnot?

> 
> (cache_clean implies !cache_dirty, but
> !cache_clean does not imply cache_dirty)
> 
> > We also have read_domains & DOMAIN_CPU. Which is which?
> 
> DOMAIN_CPU implies that the object may be in the cpu cache (modulo the
> clflush elision above).
> 
> DOMAIN_CPU implies !cache_clean
> 
> and even
> 
> cache_clean implies !DOMAIN_CPU
> 
> but
> 
> !DOMAIN_CPU does not imply cache_clean
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2015-11-25 11:02 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-24 10:05 [PATCH] drm/i915 : Avoid superfluous invalidation of CPU cache lines akash.goel
2015-11-24 10:04 ` Ville Syrjälä
2015-11-24 18:14   ` Daniel Vetter
2015-11-24 22:39     ` Chris Wilson
2015-11-25  5:29       ` [PATCH v2] " akash.goel
2015-11-25  9:21       ` [PATCH] " Daniel Vetter
2015-11-25  9:27         ` Goel, Akash
2015-11-25 10:00           ` Daniel Vetter
2015-11-30  6:24             ` Goel, Akash
2015-11-30  8:15               ` Daniel Vetter
2015-12-01 12:07                 ` Goel, Akash
2015-11-25 11:02       ` Ville Syrjälä [this message]
2015-11-25 17:28         ` Chris Wilson
2015-11-26  3:39           ` Goel, Akash
2015-11-26 10:57             ` Chris Wilson
2015-11-30  7:11               ` [PATCH v3] " akash.goel
2015-12-01 12:34                 ` Ville Syrjälä
2015-12-01 13:09                   ` Chris Wilson
2015-12-01 13:28                     ` Ville Syrjälä
2015-12-01 13:49                       ` Chris Wilson
2015-12-01 14:00                         ` Ville Syrjälä
2015-12-01 15:00                           ` Goel, Akash
2015-12-02  8:07                             ` [PATCH v4] " akash.goel
2015-12-06 17:03                               ` Chris Wilson
2015-11-24 10:10 ` [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=20151125110220.GM4437@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=akash.goel@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel@ffwll.ch \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox