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

[Tvrtko Ursulin]

On 05/01/2017 16:39, Chris Wilson wrote:
> 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?

Blast :), never mind then. Maybe unsigned long would be more idiomatic 
in the kernel, but really don't bother.

Regards,

Tvrtko