[Intel-gfx] [PATCH 13/22] drm/i915: Combine all i915_vma bitfields into a single set of flags

Joonas Lahtinen joonas.lahtinen at linux.intel.com
Fri Jul 29 07:30:26 UTC 2016


On ke, 2016-07-27 at 12:14 +0100, Chris Wilson wrote:
> @@ -2979,7 +2980,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
>  	u64 min_alignment;
>  	int ret;
>  
> -	GEM_BUG_ON(vma->bound);
> +	GEM_BUG_ON(vma->flags & (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND));

How bout i915_vma_is_bound?

>  	/* Pin early to prevent the shrinker/eviction logic from destroying
> @@ -3712,7 +3714,7 @@ i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
>  	 */
>  	__i915_vma_pin(vma);
>  
> -	if (!bound) {
> +	if ((bound & (I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND)) == 0) {

In this case especially !(bound & ...) looks far more readable. Again,
I'm against flip-flopping between styles, but I understand these are
old patches, so we can unify stuff at the end of churn.

> @@ -3682,8 +3682,8 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)
>  	if (WARN_ON(!vma->obj->map_and_fenceable))
>  		return IO_ERR_PTR(-ENODEV);
>  
> -	GEM_BUG_ON(!vma->is_ggtt);
> -	GEM_BUG_ON((vma->bound & GLOBAL_BIND) == 0);
> +	GEM_BUG_ON(!i915_vma_is_ggtt(vma));
> +	GEM_BUG_ON((vma->flags & I915_VMA_GLOBAL_BIND) == 0);

Again !(vma->flags & ) is more readable.

But GEM_BUG_ON(!i915_vma_is_bound(vma)) would again be possible.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation


More information about the Intel-gfx mailing list