[RFC,4/6] drm/i915: Use framebuffer tiling mode for display purposes
diff mbox

Message ID 1422639418-28845-5-git-send-email-tvrtko.ursulin@linux.intel.com
State New, archived
Headers show

Commit Message

Tvrtko Ursulin Jan. 30, 2015, 5:36 p.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

To prepare for framebuffer modifiers, move tiling definition from the
object into the framebuffer. Move in a way that framebuffer tiling is
now used for display while object tiling remains for fencing.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 46 +++++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_pm.c      |  7 +++---
 drivers/gpu/drm/i915/intel_sprite.c  | 26 ++++++++++----------
 4 files changed, 46 insertions(+), 35 deletions(-)

Comments

Daniel Vetter Feb. 2, 2015, 9:49 a.m. UTC | #1
On Fri, Jan 30, 2015 at 05:36:56PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> To prepare for framebuffer modifiers, move tiling definition from the
> object into the framebuffer. Move in a way that framebuffer tiling is
> now used for display while object tiling remains for fencing.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 46 +++++++++++++++++++++---------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c      |  7 +++---
>  drivers/gpu/drm/i915/intel_sprite.c  | 26 ++++++++++----------
>  4 files changed, 46 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4425e86..e22afbe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2211,7 +2211,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>  
>  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>  
> -	switch (obj->tiling_mode) {
> +	switch (to_intel_framebuffer(fb)->tiling_mode) {
>  	case I915_TILING_NONE:

Imo we should just look at fb->modifier[0] and flip over all the enums. A
bit more invasive, but we also don't need to change all the platform code
at once - set_tiling already guarantees that no one can modify the bo
tiling mode when there's an fb object using it. Which means we can change
the code over at leasure.

>  		if (INTEL_INFO(dev)->gen >= 9)
>  			alignment = 256 * 1024;
> @@ -2474,6 +2474,7 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  	u32 dspcntr;
>  	u32 reg = DSPCNTR(plane);
>  	int pixel_size;
> +	unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
>  
>  	if (!intel_crtc->primary_enabled) {
>  		I915_WRITE(reg, 0);
> @@ -2545,8 +2546,7 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  		BUG();
>  	}
>  
> -	if (INTEL_INFO(dev)->gen >= 4 &&
> -	    obj->tiling_mode != I915_TILING_NONE)
> +	if (INTEL_INFO(dev)->gen >= 4 && tiling_mode != I915_TILING_NONE)
>  		dspcntr |= DISPPLANE_TILED;
>  
>  	if (IS_G4X(dev))
> @@ -2556,7 +2556,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  
>  	if (INTEL_INFO(dev)->gen >= 4) {
>  		intel_crtc->dspaddr_offset =
> -			intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
> +			intel_gen4_compute_page_offset(&x, &y,
> +						       tiling_mode,
>  						       pixel_size,
>  						       fb->pitches[0]);
>  		linear_offset -= intel_crtc->dspaddr_offset;
> @@ -2606,6 +2607,7 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  	u32 dspcntr;
>  	u32 reg = DSPCNTR(plane);
>  	int pixel_size;
> +	unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
>  
>  	if (!intel_crtc->primary_enabled) {
>  		I915_WRITE(reg, 0);
> @@ -2654,7 +2656,7 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  		BUG();
>  	}
>  
> -	if (obj->tiling_mode != I915_TILING_NONE)
> +	if (tiling_mode != I915_TILING_NONE)
>  		dspcntr |= DISPPLANE_TILED;
>  
>  	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
> @@ -2662,7 +2664,8 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  
>  	linear_offset = y * fb->pitches[0] + x * pixel_size;
>  	intel_crtc->dspaddr_offset =
> -		intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
> +		intel_gen4_compute_page_offset(&x, &y,
> +					       tiling_mode,
>  					       pixel_size,
>  					       fb->pitches[0]);
>  	linear_offset -= intel_crtc->dspaddr_offset;
> @@ -2750,7 +2753,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  	 * The stride is either expressed as a multiple of 64 bytes chunks for
>  	 * linear buffers or in number of tiles for tiled buffers.
>  	 */
> -	switch (obj->tiling_mode) {
> +	switch (to_intel_framebuffer(fb)->tiling_mode) {
>  	case I915_TILING_NONE:
>  		stride = fb->pitches[0] >> 6;
>  		break;
> @@ -9291,7 +9294,7 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
>  			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
>  	intel_ring_emit(ring, fb->pitches[0]);
>  	intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset |
> -			obj->tiling_mode);
> +			to_intel_framebuffer(fb)->tiling_mode);
>  
>  	/* XXX Enabling the panel-fitter across page-flip is so far
>  	 * untested on non-native modes, so ignore it for now.
> @@ -9324,7 +9327,8 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
>  
>  	intel_ring_emit(ring, MI_DISPLAY_FLIP |
>  			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
> -	intel_ring_emit(ring, fb->pitches[0] | obj->tiling_mode);
> +	intel_ring_emit(ring,
> +			fb->pitches[0] | to_intel_framebuffer(fb)->tiling_mode);
>  	intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset);
>  
>  	/* Contrary to the suggestions in the documentation,
> @@ -9428,7 +9432,8 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
>  	}
>  
>  	intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit);
> -	intel_ring_emit(ring, (fb->pitches[0] | obj->tiling_mode));
> +	intel_ring_emit(ring,
> +			fb->pitches[0] | to_intel_framebuffer(fb)->tiling_mode);
>  	intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset);
>  	intel_ring_emit(ring, (MI_NOOP));
>  
> @@ -9470,13 +9475,12 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
>  	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> -	struct drm_i915_gem_object *obj = intel_fb->obj;
>  	const enum pipe pipe = intel_crtc->pipe;
>  	u32 ctl, stride;
>  
>  	ctl = I915_READ(PLANE_CTL(pipe, 0));
>  	ctl &= ~PLANE_CTL_TILED_MASK;
> -	if (obj->tiling_mode == I915_TILING_X)
> +	if (intel_fb->tiling_mode == I915_TILING_X)
>  		ctl |= PLANE_CTL_TILED_X;
>  
>  	/*
> @@ -9484,7 +9488,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc)
>  	 * linear buffers or in number of tiles for tiled buffers.
>  	 */
>  	stride = fb->pitches[0] >> 6;
> -	if (obj->tiling_mode == I915_TILING_X)
> +	if (intel_fb->tiling_mode == I915_TILING_X)
>  		stride = fb->pitches[0] >> 9; /* X tiles are 512 bytes wide */
>  
>  	/*
> @@ -9504,14 +9508,13 @@ static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_framebuffer *intel_fb =
>  		to_intel_framebuffer(intel_crtc->base.primary->fb);
> -	struct drm_i915_gem_object *obj = intel_fb->obj;
>  	u32 dspcntr;
>  	u32 reg;
>  
>  	reg = DSPCNTR(intel_crtc->plane);
>  	dspcntr = I915_READ(reg);
>  
> -	if (obj->tiling_mode != I915_TILING_NONE)
> +	if (intel_fb->tiling_mode != I915_TILING_NONE)
>  		dspcntr |= DISPPLANE_TILED;
>  	else
>  		dspcntr &= ~DISPPLANE_TILED;
> @@ -9595,6 +9598,7 @@ static int intel_gen9_queue_flip(struct drm_device *dev,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	uint32_t plane = 0, stride;
>  	int ret;
> +	unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
>  
>  	switch(intel_crtc->pipe) {
>  	case PIPE_A:
> @@ -9611,7 +9615,7 @@ static int intel_gen9_queue_flip(struct drm_device *dev,
>  		return -ENODEV;
>  	}
>  
> -	switch (obj->tiling_mode) {
> +	switch (tiling_mode) {
>  	case I915_TILING_NONE:
>  		stride = fb->pitches[0] >> 6;
>  		break;
> @@ -9639,7 +9643,7 @@ static int intel_gen9_queue_flip(struct drm_device *dev,
>  	intel_ring_emit(ring, 0);
>  
>  	intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane);
> -	intel_ring_emit(ring, stride << 6 | obj->tiling_mode);
> +	intel_ring_emit(ring, stride << 6 | tiling_mode);
>  	intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset);
>  
>  	intel_mark_page_flip_active(intel_crtc);
> @@ -9764,6 +9768,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  	work->event = event;
>  	work->crtc = crtc;
>  	work->old_fb_obj = intel_fb_obj(old_fb);
> +	work->old_tiling_mode = to_intel_framebuffer(old_fb)->tiling_mode;

Hm, that's actually an interesting bugfix - currently userspace could be
sneaky and destroy the old fb immediately after the flip completes and the
change the tiling of the underlying object before the unpin work had a
chance to run (needs some fudgin with rt prios to starve workers to make
this work though).

Imo the right fix is to hold a reference onto the fb and not the
underlying gem object. With that tiling is guaranteed not to change.

>  	INIT_WORK(&work->work, intel_unpin_work_fn);
>  
>  	ret = drm_crtc_vblank_get(crtc);
> @@ -9814,7 +9819,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  
>  	if (IS_VALLEYVIEW(dev)) {
>  		ring = &dev_priv->ring[BCS];
> -		if (obj->tiling_mode != work->old_fb_obj->tiling_mode)
> +		if (to_intel_framebuffer(fb)->tiling_mode !=
> +		    work->old_tiling_mode)
>  			/* vlv: DISPLAY_FLIP fails to change tiling */
>  			ring = NULL;
>  	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> @@ -12190,7 +12196,8 @@ intel_check_cursor_plane(struct drm_plane *plane,
>  
>  	/* we only need to pin inside GTT if cursor is non-phy */
>  	mutex_lock(&dev->struct_mutex);
> -	if (!INTEL_INFO(dev)->cursor_needs_physical && obj->tiling_mode) {
> +	if (!INTEL_INFO(dev)->cursor_needs_physical &&
> +	    to_intel_framebuffer(fb)->tiling_mode) {
>  		DRM_DEBUG_KMS("cursor cannot be tiled\n");
>  		ret = -EINVAL;
>  	}
> @@ -12772,6 +12779,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  	drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
>  	intel_fb->obj = obj;
>  	intel_fb->obj->framebuffer_references++;
> +	intel_fb->tiling_mode = obj->tiling_mode;

One side-effect of using fb->modifier[0] is that if the modifier flag is
_not_ set, we need to reconstruct this field from obj->tiling_mode here.
Otoh if it is set this code here should check that fb->modifier and
obj->tiling_mode are consistent.

Perhaps best to split this change out as a prep patch, like you've done
with the code for the initial framebuffer.

>  
>  	ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
>  	if (ret) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index eef79cc..b8d8a1d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -120,6 +120,7 @@ enum intel_output_type {
>  struct intel_framebuffer {
>  	struct drm_framebuffer base;
>  	struct drm_i915_gem_object *obj;
> +	unsigned int tiling_mode;
>  };
>  
>  struct intel_fbdev {
> @@ -723,6 +724,7 @@ struct intel_unpin_work {
>  	int flip_queued_vblank;
>  	int flip_ready_vblank;
>  	bool enable_stall_check;
> +	unsigned int old_tiling_mode;
>  };
>  
>  struct intel_set_config {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6ece663..0ca4088 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1182,12 +1182,11 @@ static void i9xx_update_wm(struct drm_crtc *unused_crtc)
>  	DRM_DEBUG_KMS("FIFO watermarks - A: %d, B: %d\n", planea_wm, planeb_wm);
>  
>  	if (IS_I915GM(dev) && enabled) {
> -		struct drm_i915_gem_object *obj;
> -
> -		obj = intel_fb_obj(enabled->primary->fb);
> +		struct intel_framebuffer *intel_fb;
>  
>  		/* self-refresh seems busted with untiled */
> -		if (obj->tiling_mode == I915_TILING_NONE)
> +		intel_fb = to_intel_framebuffer(enabled->primary->fb);
> +		if (intel_fb->tiling_mode == I915_TILING_NONE)
>  			enabled = NULL;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 0a52c44..0659802 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -245,7 +245,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
>  		BUG();
>  	}
>  
> -	switch (obj->tiling_mode) {
> +	switch (to_intel_framebuffer(fb)->tiling_mode) {
>  	case I915_TILING_NONE:
>  		stride = fb->pitches[0] >> 6;
>  		break;
> @@ -413,6 +413,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>  	u32 sprctl;
>  	unsigned long sprsurf_offset, linear_offset;
>  	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> +	unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
>  
>  	sprctl = I915_READ(SPCNTR(pipe, plane));
>  
> @@ -471,7 +472,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>  	 */
>  	sprctl |= SP_GAMMA_ENABLE;
>  
> -	if (obj->tiling_mode != I915_TILING_NONE)
> +	if (tiling_mode != I915_TILING_NONE)
>  		sprctl |= SP_TILED;
>  
>  	sprctl |= SP_ENABLE;
> @@ -488,7 +489,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>  
>  	linear_offset = y * fb->pitches[0] + x * pixel_size;
>  	sprsurf_offset = intel_gen4_compute_page_offset(&x, &y,
> -							obj->tiling_mode,
> +							tiling_mode,
>  							pixel_size,
>  							fb->pitches[0]);
>  	linear_offset -= sprsurf_offset;
> @@ -509,7 +510,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>  	I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
>  	I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
>  
> -	if (obj->tiling_mode != I915_TILING_NONE)
> +	if (tiling_mode != I915_TILING_NONE)
>  		I915_WRITE(SPTILEOFF(pipe, plane), (y << 16) | x);
>  	else
>  		I915_WRITE(SPLINOFF(pipe, plane), linear_offset);
> @@ -613,6 +614,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	u32 sprctl, sprscale = 0;
>  	unsigned long sprsurf_offset, linear_offset;
>  	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> +	unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
>  
>  	sprctl = I915_READ(SPRCTL(pipe));
>  
> @@ -652,7 +654,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	 */
>  	sprctl |= SPRITE_GAMMA_ENABLE;
>  
> -	if (obj->tiling_mode != I915_TILING_NONE)
> +	if (tiling_mode != I915_TILING_NONE)
>  		sprctl |= SPRITE_TILED;
>  
>  	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> @@ -680,7 +682,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  
>  	linear_offset = y * fb->pitches[0] + x * pixel_size;
>  	sprsurf_offset =
> -		intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
> +		intel_gen4_compute_page_offset(&x, &y, tiling_mode,
>  					       pixel_size, fb->pitches[0]);
>  	linear_offset -= sprsurf_offset;
>  
> @@ -705,7 +707,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	 * register */
>  	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>  		I915_WRITE(SPROFFSET(pipe), (y << 16) | x);
> -	else if (obj->tiling_mode != I915_TILING_NONE)
> +	else if (tiling_mode != I915_TILING_NONE)
>  		I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
>  	else
>  		I915_WRITE(SPRLINOFF(pipe), linear_offset);
> @@ -818,6 +820,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	unsigned long dvssurf_offset, linear_offset;
>  	u32 dvscntr, dvsscale;
>  	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> +	unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
>  
>  	dvscntr = I915_READ(DVSCNTR(pipe));
>  
> @@ -857,7 +860,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	 */
>  	dvscntr |= DVS_GAMMA_ENABLE;
>  
> -	if (obj->tiling_mode != I915_TILING_NONE)
> +	if (tiling_mode != I915_TILING_NONE)
>  		dvscntr |= DVS_TILED;
>  
>  	if (IS_GEN6(dev))
> @@ -880,7 +883,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  
>  	linear_offset = y * fb->pitches[0] + x * pixel_size;
>  	dvssurf_offset =
> -		intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
> +		intel_gen4_compute_page_offset(&x, &y, tiling_mode,
>  					       pixel_size, fb->pitches[0]);
>  	linear_offset -= dvssurf_offset;
>  
> @@ -897,7 +900,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
>  	I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
>  
> -	if (obj->tiling_mode != I915_TILING_NONE)
> +	if (tiling_mode != I915_TILING_NONE)
>  		I915_WRITE(DVSTILEOFF(pipe), (y << 16) | x);
>  	else
>  		I915_WRITE(DVSLINOFF(pipe), linear_offset);
> @@ -1076,7 +1079,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc);
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
>  	struct drm_framebuffer *fb = state->base.fb;
> -	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>  	int crtc_x, crtc_y;
>  	unsigned int crtc_w, crtc_h;
>  	uint32_t src_x, src_y, src_w, src_h;
> @@ -1107,7 +1109,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
>  	}
>  
>  	/* Sprite planes can be linear or x-tiled surfaces */
> -	switch (obj->tiling_mode) {
> +	switch (to_intel_framebuffer(fb)->tiling_mode) {
>  		case I915_TILING_NONE:
>  		case I915_TILING_X:
>  			break;
> -- 
> 2.2.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Tvrtko Ursulin Feb. 2, 2015, 10:29 a.m. UTC | #2
On 02/02/2015 09:49 AM, Daniel Vetter wrote:
> On Fri, Jan 30, 2015 at 05:36:56PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> To prepare for framebuffer modifiers, move tiling definition from the
>> object into the framebuffer. Move in a way that framebuffer tiling is
>> now used for display while object tiling remains for fencing.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 46 +++++++++++++++++++++---------------
>>   drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>>   drivers/gpu/drm/i915/intel_pm.c      |  7 +++---
>>   drivers/gpu/drm/i915/intel_sprite.c  | 26 ++++++++++----------
>>   4 files changed, 46 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 4425e86..e22afbe 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -2211,7 +2211,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
>>
>>   	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>>
>> -	switch (obj->tiling_mode) {
>> +	switch (to_intel_framebuffer(fb)->tiling_mode) {
>>   	case I915_TILING_NONE:
>
> Imo we should just look at fb->modifier[0] and flip over all the enums. A
> bit more invasive, but we also don't need to change all the platform code
> at once - set_tiling already guarantees that no one can modify the bo
> tiling mode when there's an fb object using it. Which means we can change
> the code over at leasure.

What do you mean by "flip over"?

To make places which need to get fb tiling format use fb->modifier[0] 
directly rather than have them mapped to tiling enums at fb init?

>> @@ -9764,6 +9768,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>>   	work->event = event;
>>   	work->crtc = crtc;
>>   	work->old_fb_obj = intel_fb_obj(old_fb);
>> +	work->old_tiling_mode = to_intel_framebuffer(old_fb)->tiling_mode;
>
> Hm, that's actually an interesting bugfix - currently userspace could be
> sneaky and destroy the old fb immediately after the flip completes and the
> change the tiling of the underlying object before the unpin work had a
> chance to run (needs some fudgin with rt prios to starve workers to make
> this work though).
>
> Imo the right fix is to hold a reference onto the fb and not the
> underlying gem object. With that tiling is guaranteed not to change.

Ok I'll pull it out in a separate patch.

>>   	INIT_WORK(&work->work, intel_unpin_work_fn);
>>
>>   	ret = drm_crtc_vblank_get(crtc);
>> @@ -9814,7 +9819,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>>
>>   	if (IS_VALLEYVIEW(dev)) {
>>   		ring = &dev_priv->ring[BCS];
>> -		if (obj->tiling_mode != work->old_fb_obj->tiling_mode)
>> +		if (to_intel_framebuffer(fb)->tiling_mode !=
>> +		    work->old_tiling_mode)
>>   			/* vlv: DISPLAY_FLIP fails to change tiling */
>>   			ring = NULL;
>>   	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
>> @@ -12190,7 +12196,8 @@ intel_check_cursor_plane(struct drm_plane *plane,
>>
>>   	/* we only need to pin inside GTT if cursor is non-phy */
>>   	mutex_lock(&dev->struct_mutex);
>> -	if (!INTEL_INFO(dev)->cursor_needs_physical && obj->tiling_mode) {
>> +	if (!INTEL_INFO(dev)->cursor_needs_physical &&
>> +	    to_intel_framebuffer(fb)->tiling_mode) {
>>   		DRM_DEBUG_KMS("cursor cannot be tiled\n");
>>   		ret = -EINVAL;
>>   	}
>> @@ -12772,6 +12779,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
>>   	drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
>>   	intel_fb->obj = obj;
>>   	intel_fb->obj->framebuffer_references++;
>> +	intel_fb->tiling_mode = obj->tiling_mode;
>
> One side-effect of using fb->modifier[0] is that if the modifier flag is
> _not_ set, we need to reconstruct this field from obj->tiling_mode here.
> Otoh if it is set this code here should check that fb->modifier and
> obj->tiling_mode are consistent.
>
> Perhaps best to split this change out as a prep patch, like you've done
> with the code for the initial framebuffer.

If I understood correctly what you meant in the first quote then yes.

Regards,

Tvrtko
Daniel Vetter Feb. 2, 2015, 5:09 p.m. UTC | #3
On Mon, Feb 02, 2015 at 10:29:19AM +0000, Tvrtko Ursulin wrote:
> 
> On 02/02/2015 09:49 AM, Daniel Vetter wrote:
> >On Fri, Jan 30, 2015 at 05:36:56PM +0000, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >>To prepare for framebuffer modifiers, move tiling definition from the
> >>object into the framebuffer. Move in a way that framebuffer tiling is
> >>now used for display while object tiling remains for fencing.
> >>
> >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/intel_display.c | 46 +++++++++++++++++++++---------------
> >>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
> >>  drivers/gpu/drm/i915/intel_pm.c      |  7 +++---
> >>  drivers/gpu/drm/i915/intel_sprite.c  | 26 ++++++++++----------
> >>  4 files changed, 46 insertions(+), 35 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>index 4425e86..e22afbe 100644
> >>--- a/drivers/gpu/drm/i915/intel_display.c
> >>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>@@ -2211,7 +2211,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane,
> >>
> >>  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> >>
> >>-	switch (obj->tiling_mode) {
> >>+	switch (to_intel_framebuffer(fb)->tiling_mode) {
> >>  	case I915_TILING_NONE:
> >
> >Imo we should just look at fb->modifier[0] and flip over all the enums. A
> >bit more invasive, but we also don't need to change all the platform code
> >at once - set_tiling already guarantees that no one can modify the bo
> >tiling mode when there's an fb object using it. Which means we can change
> >the code over at leasure.
> 
> What do you mean by "flip over"?
> 
> To make places which need to get fb tiling format use fb->modifier[0]
> directly rather than have them mapped to tiling enums at fb init?

Yes. That way we can drop intel_fb->tiling_mode and have one less somewhat
redundant thing to keep in sync with everything else. Hoping that everyone
just uses the same ime just doesn't work ;-)

> >>@@ -9764,6 +9768,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> >>  	work->event = event;
> >>  	work->crtc = crtc;
> >>  	work->old_fb_obj = intel_fb_obj(old_fb);
> >>+	work->old_tiling_mode = to_intel_framebuffer(old_fb)->tiling_mode;
> >
> >Hm, that's actually an interesting bugfix - currently userspace could be
> >sneaky and destroy the old fb immediately after the flip completes and the
> >change the tiling of the underlying object before the unpin work had a
> >chance to run (needs some fudgin with rt prios to starve workers to make
> >this work though).
> >
> >Imo the right fix is to hold a reference onto the fb and not the
> >underlying gem object. With that tiling is guaranteed not to change.
> 
> Ok I'll pull it out in a separate patch.
> 
> >>  	INIT_WORK(&work->work, intel_unpin_work_fn);
> >>
> >>  	ret = drm_crtc_vblank_get(crtc);
> >>@@ -9814,7 +9819,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> >>
> >>  	if (IS_VALLEYVIEW(dev)) {
> >>  		ring = &dev_priv->ring[BCS];
> >>-		if (obj->tiling_mode != work->old_fb_obj->tiling_mode)
> >>+		if (to_intel_framebuffer(fb)->tiling_mode !=
> >>+		    work->old_tiling_mode)
> >>  			/* vlv: DISPLAY_FLIP fails to change tiling */
> >>  			ring = NULL;
> >>  	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> >>@@ -12190,7 +12196,8 @@ intel_check_cursor_plane(struct drm_plane *plane,
> >>
> >>  	/* we only need to pin inside GTT if cursor is non-phy */
> >>  	mutex_lock(&dev->struct_mutex);
> >>-	if (!INTEL_INFO(dev)->cursor_needs_physical && obj->tiling_mode) {
> >>+	if (!INTEL_INFO(dev)->cursor_needs_physical &&
> >>+	    to_intel_framebuffer(fb)->tiling_mode) {
> >>  		DRM_DEBUG_KMS("cursor cannot be tiled\n");
> >>  		ret = -EINVAL;
> >>  	}
> >>@@ -12772,6 +12779,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
> >>  	drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
> >>  	intel_fb->obj = obj;
> >>  	intel_fb->obj->framebuffer_references++;
> >>+	intel_fb->tiling_mode = obj->tiling_mode;
> >
> >One side-effect of using fb->modifier[0] is that if the modifier flag is
> >_not_ set, we need to reconstruct this field from obj->tiling_mode here.
> >Otoh if it is set this code here should check that fb->modifier and
> >obj->tiling_mode are consistent.
> >
> >Perhaps best to split this change out as a prep patch, like you've done
> >with the code for the initial framebuffer.
> 
> If I understood correctly what you meant in the first quote then yes.

You've already done it, it's the next patch - I just read patch series
sequentially ;-) Patch needs a bit of adjustment ofc due to the removal of
intel_fb->tiling_mode, and needs to be before this one here so that
fb->modifier[0] is guaranteed to be valid also for legacy userspace.
-Daniel

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4425e86..e22afbe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2211,7 +2211,7 @@  intel_pin_and_fence_fb_obj(struct drm_plane *plane,
 
 	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
 
-	switch (obj->tiling_mode) {
+	switch (to_intel_framebuffer(fb)->tiling_mode) {
 	case I915_TILING_NONE:
 		if (INTEL_INFO(dev)->gen >= 9)
 			alignment = 256 * 1024;
@@ -2474,6 +2474,7 @@  static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	u32 dspcntr;
 	u32 reg = DSPCNTR(plane);
 	int pixel_size;
+	unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
 
 	if (!intel_crtc->primary_enabled) {
 		I915_WRITE(reg, 0);
@@ -2545,8 +2546,7 @@  static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 		BUG();
 	}
 
-	if (INTEL_INFO(dev)->gen >= 4 &&
-	    obj->tiling_mode != I915_TILING_NONE)
+	if (INTEL_INFO(dev)->gen >= 4 && tiling_mode != I915_TILING_NONE)
 		dspcntr |= DISPPLANE_TILED;
 
 	if (IS_G4X(dev))
@@ -2556,7 +2556,8 @@  static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 
 	if (INTEL_INFO(dev)->gen >= 4) {
 		intel_crtc->dspaddr_offset =
-			intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
+			intel_gen4_compute_page_offset(&x, &y,
+						       tiling_mode,
 						       pixel_size,
 						       fb->pitches[0]);
 		linear_offset -= intel_crtc->dspaddr_offset;
@@ -2606,6 +2607,7 @@  static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 	u32 dspcntr;
 	u32 reg = DSPCNTR(plane);
 	int pixel_size;
+	unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
 
 	if (!intel_crtc->primary_enabled) {
 		I915_WRITE(reg, 0);
@@ -2654,7 +2656,7 @@  static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 		BUG();
 	}
 
-	if (obj->tiling_mode != I915_TILING_NONE)
+	if (tiling_mode != I915_TILING_NONE)
 		dspcntr |= DISPPLANE_TILED;
 
 	if (!IS_HASWELL(dev) && !IS_BROADWELL(dev))
@@ -2662,7 +2664,8 @@  static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 
 	linear_offset = y * fb->pitches[0] + x * pixel_size;
 	intel_crtc->dspaddr_offset =
-		intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
+		intel_gen4_compute_page_offset(&x, &y,
+					       tiling_mode,
 					       pixel_size,
 					       fb->pitches[0]);
 	linear_offset -= intel_crtc->dspaddr_offset;
@@ -2750,7 +2753,7 @@  static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	 * The stride is either expressed as a multiple of 64 bytes chunks for
 	 * linear buffers or in number of tiles for tiled buffers.
 	 */
-	switch (obj->tiling_mode) {
+	switch (to_intel_framebuffer(fb)->tiling_mode) {
 	case I915_TILING_NONE:
 		stride = fb->pitches[0] >> 6;
 		break;
@@ -9291,7 +9294,7 @@  static int intel_gen4_queue_flip(struct drm_device *dev,
 			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 	intel_ring_emit(ring, fb->pitches[0]);
 	intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset |
-			obj->tiling_mode);
+			to_intel_framebuffer(fb)->tiling_mode);
 
 	/* XXX Enabling the panel-fitter across page-flip is so far
 	 * untested on non-native modes, so ignore it for now.
@@ -9324,7 +9327,8 @@  static int intel_gen6_queue_flip(struct drm_device *dev,
 
 	intel_ring_emit(ring, MI_DISPLAY_FLIP |
 			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
-	intel_ring_emit(ring, fb->pitches[0] | obj->tiling_mode);
+	intel_ring_emit(ring,
+			fb->pitches[0] | to_intel_framebuffer(fb)->tiling_mode);
 	intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset);
 
 	/* Contrary to the suggestions in the documentation,
@@ -9428,7 +9432,8 @@  static int intel_gen7_queue_flip(struct drm_device *dev,
 	}
 
 	intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit);
-	intel_ring_emit(ring, (fb->pitches[0] | obj->tiling_mode));
+	intel_ring_emit(ring,
+			fb->pitches[0] | to_intel_framebuffer(fb)->tiling_mode);
 	intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset);
 	intel_ring_emit(ring, (MI_NOOP));
 
@@ -9470,13 +9475,12 @@  static void skl_do_mmio_flip(struct intel_crtc *intel_crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
 	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
-	struct drm_i915_gem_object *obj = intel_fb->obj;
 	const enum pipe pipe = intel_crtc->pipe;
 	u32 ctl, stride;
 
 	ctl = I915_READ(PLANE_CTL(pipe, 0));
 	ctl &= ~PLANE_CTL_TILED_MASK;
-	if (obj->tiling_mode == I915_TILING_X)
+	if (intel_fb->tiling_mode == I915_TILING_X)
 		ctl |= PLANE_CTL_TILED_X;
 
 	/*
@@ -9484,7 +9488,7 @@  static void skl_do_mmio_flip(struct intel_crtc *intel_crtc)
 	 * linear buffers or in number of tiles for tiled buffers.
 	 */
 	stride = fb->pitches[0] >> 6;
-	if (obj->tiling_mode == I915_TILING_X)
+	if (intel_fb->tiling_mode == I915_TILING_X)
 		stride = fb->pitches[0] >> 9; /* X tiles are 512 bytes wide */
 
 	/*
@@ -9504,14 +9508,13 @@  static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_framebuffer *intel_fb =
 		to_intel_framebuffer(intel_crtc->base.primary->fb);
-	struct drm_i915_gem_object *obj = intel_fb->obj;
 	u32 dspcntr;
 	u32 reg;
 
 	reg = DSPCNTR(intel_crtc->plane);
 	dspcntr = I915_READ(reg);
 
-	if (obj->tiling_mode != I915_TILING_NONE)
+	if (intel_fb->tiling_mode != I915_TILING_NONE)
 		dspcntr |= DISPPLANE_TILED;
 	else
 		dspcntr &= ~DISPPLANE_TILED;
@@ -9595,6 +9598,7 @@  static int intel_gen9_queue_flip(struct drm_device *dev,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	uint32_t plane = 0, stride;
 	int ret;
+	unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
 
 	switch(intel_crtc->pipe) {
 	case PIPE_A:
@@ -9611,7 +9615,7 @@  static int intel_gen9_queue_flip(struct drm_device *dev,
 		return -ENODEV;
 	}
 
-	switch (obj->tiling_mode) {
+	switch (tiling_mode) {
 	case I915_TILING_NONE:
 		stride = fb->pitches[0] >> 6;
 		break;
@@ -9639,7 +9643,7 @@  static int intel_gen9_queue_flip(struct drm_device *dev,
 	intel_ring_emit(ring, 0);
 
 	intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane);
-	intel_ring_emit(ring, stride << 6 | obj->tiling_mode);
+	intel_ring_emit(ring, stride << 6 | tiling_mode);
 	intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset);
 
 	intel_mark_page_flip_active(intel_crtc);
@@ -9764,6 +9768,7 @@  static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	work->event = event;
 	work->crtc = crtc;
 	work->old_fb_obj = intel_fb_obj(old_fb);
+	work->old_tiling_mode = to_intel_framebuffer(old_fb)->tiling_mode;
 	INIT_WORK(&work->work, intel_unpin_work_fn);
 
 	ret = drm_crtc_vblank_get(crtc);
@@ -9814,7 +9819,8 @@  static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
 	if (IS_VALLEYVIEW(dev)) {
 		ring = &dev_priv->ring[BCS];
-		if (obj->tiling_mode != work->old_fb_obj->tiling_mode)
+		if (to_intel_framebuffer(fb)->tiling_mode !=
+		    work->old_tiling_mode)
 			/* vlv: DISPLAY_FLIP fails to change tiling */
 			ring = NULL;
 	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
@@ -12190,7 +12196,8 @@  intel_check_cursor_plane(struct drm_plane *plane,
 
 	/* we only need to pin inside GTT if cursor is non-phy */
 	mutex_lock(&dev->struct_mutex);
-	if (!INTEL_INFO(dev)->cursor_needs_physical && obj->tiling_mode) {
+	if (!INTEL_INFO(dev)->cursor_needs_physical &&
+	    to_intel_framebuffer(fb)->tiling_mode) {
 		DRM_DEBUG_KMS("cursor cannot be tiled\n");
 		ret = -EINVAL;
 	}
@@ -12772,6 +12779,7 @@  static int intel_framebuffer_init(struct drm_device *dev,
 	drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd);
 	intel_fb->obj = obj;
 	intel_fb->obj->framebuffer_references++;
+	intel_fb->tiling_mode = obj->tiling_mode;
 
 	ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
 	if (ret) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index eef79cc..b8d8a1d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -120,6 +120,7 @@  enum intel_output_type {
 struct intel_framebuffer {
 	struct drm_framebuffer base;
 	struct drm_i915_gem_object *obj;
+	unsigned int tiling_mode;
 };
 
 struct intel_fbdev {
@@ -723,6 +724,7 @@  struct intel_unpin_work {
 	int flip_queued_vblank;
 	int flip_ready_vblank;
 	bool enable_stall_check;
+	unsigned int old_tiling_mode;
 };
 
 struct intel_set_config {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6ece663..0ca4088 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1182,12 +1182,11 @@  static void i9xx_update_wm(struct drm_crtc *unused_crtc)
 	DRM_DEBUG_KMS("FIFO watermarks - A: %d, B: %d\n", planea_wm, planeb_wm);
 
 	if (IS_I915GM(dev) && enabled) {
-		struct drm_i915_gem_object *obj;
-
-		obj = intel_fb_obj(enabled->primary->fb);
+		struct intel_framebuffer *intel_fb;
 
 		/* self-refresh seems busted with untiled */
-		if (obj->tiling_mode == I915_TILING_NONE)
+		intel_fb = to_intel_framebuffer(enabled->primary->fb);
+		if (intel_fb->tiling_mode == I915_TILING_NONE)
 			enabled = NULL;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0a52c44..0659802 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -245,7 +245,7 @@  skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 		BUG();
 	}
 
-	switch (obj->tiling_mode) {
+	switch (to_intel_framebuffer(fb)->tiling_mode) {
 	case I915_TILING_NONE:
 		stride = fb->pitches[0] >> 6;
 		break;
@@ -413,6 +413,7 @@  vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 	u32 sprctl;
 	unsigned long sprsurf_offset, linear_offset;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
+	unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
 
 	sprctl = I915_READ(SPCNTR(pipe, plane));
 
@@ -471,7 +472,7 @@  vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 	 */
 	sprctl |= SP_GAMMA_ENABLE;
 
-	if (obj->tiling_mode != I915_TILING_NONE)
+	if (tiling_mode != I915_TILING_NONE)
 		sprctl |= SP_TILED;
 
 	sprctl |= SP_ENABLE;
@@ -488,7 +489,7 @@  vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 
 	linear_offset = y * fb->pitches[0] + x * pixel_size;
 	sprsurf_offset = intel_gen4_compute_page_offset(&x, &y,
-							obj->tiling_mode,
+							tiling_mode,
 							pixel_size,
 							fb->pitches[0]);
 	linear_offset -= sprsurf_offset;
@@ -509,7 +510,7 @@  vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 	I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
 	I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
 
-	if (obj->tiling_mode != I915_TILING_NONE)
+	if (tiling_mode != I915_TILING_NONE)
 		I915_WRITE(SPTILEOFF(pipe, plane), (y << 16) | x);
 	else
 		I915_WRITE(SPLINOFF(pipe, plane), linear_offset);
@@ -613,6 +614,7 @@  ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	u32 sprctl, sprscale = 0;
 	unsigned long sprsurf_offset, linear_offset;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
+	unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
 
 	sprctl = I915_READ(SPRCTL(pipe));
 
@@ -652,7 +654,7 @@  ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	 */
 	sprctl |= SPRITE_GAMMA_ENABLE;
 
-	if (obj->tiling_mode != I915_TILING_NONE)
+	if (tiling_mode != I915_TILING_NONE)
 		sprctl |= SPRITE_TILED;
 
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
@@ -680,7 +682,7 @@  ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 
 	linear_offset = y * fb->pitches[0] + x * pixel_size;
 	sprsurf_offset =
-		intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
+		intel_gen4_compute_page_offset(&x, &y, tiling_mode,
 					       pixel_size, fb->pitches[0]);
 	linear_offset -= sprsurf_offset;
 
@@ -705,7 +707,7 @@  ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	 * register */
 	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
 		I915_WRITE(SPROFFSET(pipe), (y << 16) | x);
-	else if (obj->tiling_mode != I915_TILING_NONE)
+	else if (tiling_mode != I915_TILING_NONE)
 		I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
 	else
 		I915_WRITE(SPRLINOFF(pipe), linear_offset);
@@ -818,6 +820,7 @@  ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	unsigned long dvssurf_offset, linear_offset;
 	u32 dvscntr, dvsscale;
 	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
+	unsigned int tiling_mode = to_intel_framebuffer(fb)->tiling_mode;
 
 	dvscntr = I915_READ(DVSCNTR(pipe));
 
@@ -857,7 +860,7 @@  ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	 */
 	dvscntr |= DVS_GAMMA_ENABLE;
 
-	if (obj->tiling_mode != I915_TILING_NONE)
+	if (tiling_mode != I915_TILING_NONE)
 		dvscntr |= DVS_TILED;
 
 	if (IS_GEN6(dev))
@@ -880,7 +883,7 @@  ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 
 	linear_offset = y * fb->pitches[0] + x * pixel_size;
 	dvssurf_offset =
-		intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode,
+		intel_gen4_compute_page_offset(&x, &y, tiling_mode,
 					       pixel_size, fb->pitches[0]);
 	linear_offset -= dvssurf_offset;
 
@@ -897,7 +900,7 @@  ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
 	I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
 
-	if (obj->tiling_mode != I915_TILING_NONE)
+	if (tiling_mode != I915_TILING_NONE)
 		I915_WRITE(DVSTILEOFF(pipe), (y << 16) | x);
 	else
 		I915_WRITE(DVSLINOFF(pipe), linear_offset);
@@ -1076,7 +1079,6 @@  intel_check_sprite_plane(struct drm_plane *plane,
 	struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc);
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_framebuffer *fb = state->base.fb;
-	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	int crtc_x, crtc_y;
 	unsigned int crtc_w, crtc_h;
 	uint32_t src_x, src_y, src_w, src_h;
@@ -1107,7 +1109,7 @@  intel_check_sprite_plane(struct drm_plane *plane,
 	}
 
 	/* Sprite planes can be linear or x-tiled surfaces */
-	switch (obj->tiling_mode) {
+	switch (to_intel_framebuffer(fb)->tiling_mode) {
 		case I915_TILING_NONE:
 		case I915_TILING_X:
 			break;