On 11/25/2015 3:30 PM, Daniel Vetter wrote:
On Wed, Nov 25, 2015 at 02:57:47PM +0530, Goel, Akash wrote:
On 11/25/2015 2:51 PM, Daniel Vetter wrote:
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@xxxxxxxxx wrote:
From: Akash Goel <akash.goel@xxxxxxxxx>
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@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
---
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.
(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
All the above should be in the kerneldoc (per-struct-member comments
please) of drm_i915_gem_object. Akash, can you please amend your patch?
And please make sure we do include that kerneldoc somewhere ... might need
an upfront patch to do that, for just drm_i915_gem_object.
I floated the amended patch, earlier today,
http://lists.freedesktop.org/archives/intel-gfx/2015-November/081194.html.
Please kindly check that.
Already done and replied here because I think this should be lifted to
kerneldoc for the structure itself. That's why I replied here ;-)
-Daniel
Hi Daniel,
I think the patch to provide a kernel-doc for just the
drm_i915_gem_object structure can be submitted independently of this
patch. The kernel-doc part can be done as a follow up patch.
For the current patch, have added the per-struct-member comments for the
'cache_clean' field.
Best regards
Akash
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
