[PATCH 06/91] drm/i915: Consolidate checks for memcpy-from-wc support

[Chris Wilson]

On Thu, Jan 05, 2017 at 01:34:20PM +0000, Tvrtko Ursulin wrote:
> 
> On 05/01/2017 10:34, 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            | 3 +++
> > drivers/gpu/drm/i915/i915_gpu_error.c      | 2 +-
> > drivers/gpu/drm/i915/i915_guc_submission.c | 2 +-
> > 4 files changed, 6 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..73aefb0300ba 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_has_memcpy_from_wc((uintptr_t)batch_start_offset)) {
> 
> I am not convinced having i915_has_memcpy_from_wc take a parameter
> is useful with a) this being the only call site, and b) it is not
> checking the alignment on two other parameters anyway.

It is... They are implicitly or'ed in. ;)
 
> > 		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..946d6ecd52d5 100644
> >--- a/drivers/gpu/drm/i915/i915_drv.h
> >+++ b/drivers/gpu/drm/i915/i915_drv.h
> >@@ -4032,6 +4032,9 @@ __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);
> >
> >+#define i915_has_memcpy_from_wc(align) \
> >+	i915_memcpy_from_wc((void *)(align), NULL, 0)
> 
> I wouldn't call this parameter align in either case.

It is the alignment of the pointers (+stride) that dictate whether or
not we can use the aligned movnt instructions.

#define i915_can_memcpy_from_wc(dst, src, stride)
	i915_memcpy_from_wc((void *)((uintptr_t)dst | (uintptr)src | stride), NULL, 0)

Better?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre