On 15/07/16 17:30, Goel, Akash wrote:
On 7/15/2016 5:01 PM, Tvrtko Ursulin wrote:
On 10/07/16 14:41, akash.g...@intel.com wrote:
From: Chris Wilson <ch...@chris-wilson.co.uk>
vmaps has a provision for controlling the page protection bits, with
which
we can use to control the mapping type, e.g. WB, WC, UC or even WT.
To allow the caller to choose their mapping type, we add a parameter to
i915_gem_object_pin_map - but we still only allow one vmap to be cached
per object. If the object is currently not pinned, then we recreate the
previous vmap with the new access type, but if it was pinned we
report an
error. This effectively limits the access via i915_gem_object_pin_map
to a
single mapping type for the lifetime of the object. Not usually a
problem,
but something to be aware of when setting up the object's vmap.
We will want to vary the access type to enable WC mappings of ringbuffer
and context objects on !llc platforms, as well as other objects where we
need coherent access to the GPU's pages without going through the GTT
v2: Remove the redundant braces around pin count check and fix the
marker
in documentation (Chris)
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Signed-off-by: Akash Goel <akash.g...@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 4 ++-
drivers/gpu/drm/i915/i915_gem.c | 57
+++++++++++++++++++++++-------
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 2 +-
drivers/gpu/drm/i915/i915_guc_submission.c | 2 +-
drivers/gpu/drm/i915/intel_lrc.c | 8 ++---
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
6 files changed, 54 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 6e2ddfa..84afa17 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3248,6 +3248,7 @@ static inline void
i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj)
/**
* i915_gem_object_pin_map - return a contiguous mapping of the
entire object
* @obj - the object to map into kernel address space
+ * @use_wc - whether the mapping should be using WC or WB pgprot_t
*
* Calls i915_gem_object_pin_pages() to prevent reaping of the
object's
* pages and then returns a contiguous mapping of the backing
storage into
@@ -3259,7 +3260,8 @@ static inline void
i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj)
* Returns the pointer through which to access the mapped object,
or an
* ERR_PTR() on error.
*/
-void *__must_check i915_gem_object_pin_map(struct drm_i915_gem_object
*obj);
+void *__must_check i915_gem_object_pin_map(struct drm_i915_gem_object
*obj,
+ bool use_wc);
Could you make it an enum instead of a bool? Commit message suggests
more modes will potentially be added and if so, and we start with an
enum straight away, it will make for less churn in the future.
func(something, true) is always also quite unreadabe in the code because
one has to remember or remind himself what it really means.
Something like func(something, MAP_WC) would be simply self-documenting.
Thanks nice suggestion, will do that.
enum only or macros also will do ?
#define MAP_CACHED 0x1
#define MAP_WC 0x2
I think enum is better since you then have type checking on the arguments.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
