Re: [PATCH 15/16] drm/i915: Repack fence tiling mode and stride into a single integer

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ma, 2016-08-01 at 19:22 +0100, Chris Wilson wrote:
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2215,13 +2215,10 @@ struct drm_i915_gem_object {
>  
>  	atomic_t frontbuffer_bits;
>  
> -	/**
> -	 * Current tiling mode for the object.
> -	 */
> -	unsigned int tiling_mode;
> -
>  	/** Current tiling stride for the object, if it's tiled. */
> -	uint32_t stride;
> +	unsigned int tiling_and_stride;
> +#define TILING_MASK 0x3

No magics and add appropriate BUILD_BUG_ON too when stuffing bits
inside same variable.

#define TILING_MASK (BIT(31 - __builtin_clz(I915_TILING_LAST)) -1)

or something :P

> +#define STRIDE_MASK (~TILING_MASK)
>  
>  	unsigned int has_wc_mmap;
>  	/** Count of VMA actually bound by this object */
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2465,9 +2465,8 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
>  		return false;
>  	}
>  
> -	obj->tiling_mode = plane_config->tiling;
> -	if (obj->tiling_mode == I915_TILING_X)
> -		obj->stride = fb->pitches[0];
> +	if (plane_config->tiling == I915_TILING_X)
> +		obj->tiling_and_stride = fb->pitches[0] | I915_TILING_X;

This is not equivalent code.

>  	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> @@ -14919,15 +14918,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  	if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) {
>  		/* Enforce that fb modifier and tiling mode match, but only for
>  		 * X-tiled. This is needed for FBC. */
> -		if (!!(obj->tiling_mode == I915_TILING_X) !=
> +		if (!!(i915_gem_object_get_tiling(obj) == I915_TILING_X) !=

A note, !! are redundant as == returns bool already. Could remove while
touching.

Regards, Joonas

>  		    !!(mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)) {
>  			DRM_DEBUG("tiling_mode doesn't match fb modifier\n");
>  			return -EINVAL;
>  		}
>  	} else {

-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx