[PATCH 07/40] drm/i915: Consolidate checks for memcpy-from-wc support

[Chris Wilson]

On Thu, Jan 05, 2017 at 04:33:49PM +0000, Tvrtko Ursulin wrote:
> 
> On 05/01/2017 14:54, Chris Wilson wrote:
> >In order to silence sparse:
> >
> >../drivers/gpu/drm/i915/i915_gpu_error.c:200:39: warning: Using plain integer as NULL pointer
> >
> >add a helper to check whether we have sse4.1 and that the desired
> >alignment is valid for acceleration.
> >
> >Reported-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >---
> > drivers/gpu/drm/i915/i915_cmd_parser.c     |  2 +-
> > drivers/gpu/drm/i915/i915_drv.h            | 16 ++++++++++++++++
> > drivers/gpu/drm/i915/i915_gpu_error.c      |  2 +-
> > drivers/gpu/drm/i915/i915_guc_submission.c |  2 +-
> > 4 files changed, 19 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> >index 6c310a8a97a7..21b1cd917d81 100644
> >--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> >+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> >@@ -1079,7 +1079,7 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
> >
> > 	src = ERR_PTR(-ENODEV);
> > 	if (src_needs_clflush &&
> >-	    i915_memcpy_from_wc((void *)(uintptr_t)batch_start_offset, NULL, 0)) {
> >+	    i915_can_memcpy_from_wc(NULL, batch_start_offset, 0)) {
> > 		src = i915_gem_object_pin_map(src_obj, I915_MAP_WC);
> > 		if (!IS_ERR(src)) {
> > 			i915_memcpy_from_wc(dst,
> >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >index 7b436621d038..618c81b2726c 100644
> >--- a/drivers/gpu/drm/i915/i915_drv.h
> >+++ b/drivers/gpu/drm/i915/i915_drv.h
> >@@ -4032,6 +4032,22 @@ __i915_request_irq_complete(struct drm_i915_gem_request *req)
> > void i915_memcpy_init_early(struct drm_i915_private *dev_priv);
> > bool i915_memcpy_from_wc(void *dst, const void *src, unsigned long len);
> >
> >+/* The movntdqa instructions used for memcpy-from-wc require 16-byte alignment,
> >+ * as well as SSE4.1 support. i915_memcpy_from_wc() will report if it cannot
> >+ * perform the operation. To check beforehand, pass in the parameters to
> >+ * to i915_can_memcpy_from_wc() - since we only care about the low 4 bits,
> >+ * you only need to pass in the minor offsets, page-aligned pointers are
> >+ * always valid.
> >+ *
> >+ * For just checking for SSE4.1, in the foreknowlege that the future use
> 
> Just a typo in foreknowledge.
> 
> >+ * will be correctly aligned, just use i915_has_memcpy_from_wc().
> >+ */
> >+#define i915_can_memcpy_from_wc(dst, src, len) \
> >+	i915_memcpy_from_wc((void *)((uintptr_t)(dst) | (uintptr_t)(src) | (len)), NULL, 0)
> 
> Minor, but why do you need uintptr_t casts here? I never like to see
> these types in the kernel. Could you cast len to void * just as
> well?

The '|' operator is not defined for pointers, so we have to go through a
large integer and back. Prefer unsigned long?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre