[Intel-gfx] [PATCH] drm/i915: Reject set-tiling-ioctl with stride==0 and a tiling mode

[Tvrtko Ursulin]

On 03/02/2017 10:56, Chris Wilson wrote:
> In commit 957870f93412 ("drm/i915: Split out i915_gem_object_set_tiling()"),
> I swapped an alignment check for IS_ALIGNED and in the process removed
> the less-than check. That check turns out to be important as it was the
> only rejection for stride == 0. Tvrtko did spot it, but I was
> overconfident in the IS_ALIGNED() conversion.
>
> Fixes: 957870f93412 ("drm/i915: Split out i915_gem_object_set_tiling()")
> Testcase: igt/gem_tiling_max_stride
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
> index c96140cf47e7..a80f362d2ac1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_tiling.c
> +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
> @@ -173,7 +173,7 @@ i915_tiling_ok(struct drm_i915_gem_object *obj,
>  	else
>  		tile_width = 512;
>
> -	if (!IS_ALIGNED(stride, tile_width))
> +	if (!stride || !IS_ALIGNED(stride, tile_width))
>  		return false;
>
>  	/* 965+ just needs multiples of tile width */
>

Not sure that I spotted this in the original review, maybe. I got thrown 
off just now by the strange ending of this function in thinking 
is_power_of_2(stride) is last. Perhaps the special case of gen < 4 
should be an exception and return true the last statement.

	if (INTEL_GEN(i915) < 4)
		return is_power_of_2(stride);

	return true;

Anyway,

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko