[Intel-gfx] [PATCH v3] drm/i915: Do not use ggtt_view with (aliasing) PPGTT

[Tvrtko Ursulin]

Hi,

On 03/16/2015 12:11 PM, Joonas Lahtinen wrote:
> GGTT views are only applicable when dealing with GGTT. Change the code to
> reject ggtt_view where it should not be used and require it when it should
> be.
>
> v2:
> - Dropped _ppgtt_ infixes, allow both types to be passed
> - Disregard other but normal views when no view is specified
> - More checks that valid parameters are passed
> - More readable error checking
>
> v3:
> - Prefer WARN_ONCE over BUG_ON when there is code path for failure

[snip]

> +i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
> +		    struct i915_address_space *vm);
> +struct i915_vma *
> +i915_gem_obj_to_ggtt_view(struct drm_i915_gem_object *obj,
> +			  const struct i915_ggtt_view *view);

Would i915_gem_obj_to_ggtt_vma be a better name? At least should have 
vma in the name I think.

> +struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
> +				     struct i915_address_space *vm)
>   {
>   	struct i915_vma *vma;
> -	list_for_each_entry(vma, &obj->vma_list, vma_link)
> -		if (vma->vm == vm && vma->ggtt_view.type == view->type)
> +	list_for_each_entry(vma, &obj->vma_list, vma_link) {
> +		if (i915_is_ggtt(vma->vm) &&
> +		    vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL)

Since there are 4-5 instances of this check it may make sense to add a 
helper like i915_is_normal_ggtt_view(vma), but it is not that important 
for me.

The rest looks good to me.

Regards,

Tvrtko