| Message ID | 1422984152-5022-4-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 03, 2015 at 05:22:31PM +0000, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Start using frame buffer modifiers instead of object tiling mode > for display purposes. > > To ensure compatibility with old userspace which is using set_tiling > and does not know about frame buffer modifiers, the latter are faked > internally when tile object is set for display. This way all interested > call sites can use fb modifiers exclusively. > > Also ensure tiling specified via fb modifiers must match object tiling > used for fencing if both are specified. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 95 +++++++++++++++++++++++++----------- > 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, 85 insertions(+), 45 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 7a3ed61..6825016 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2198,6 +2198,19 @@ intel_fb_align_height(struct drm_device *dev, int height, unsigned int tiling) > return ALIGN(height, tile_height); > } > > +static unsigned int intel_fb_modifier_to_tiling(u64 mod) > +{ > + BUILD_BUG_ON((I915_FORMAT_MOD_X_TILED & 0x00ffffffffffffffL) != > + I915_TILING_X); > + > + return mod & 1; > +} > + > +unsigned int intel_fb_tiling_mode(struct drm_framebuffer *fb) > +{ > + return intel_fb_modifier_to_tiling(fb->modifier[0]); > +} I expect that these here will create a bit of churn with the skl patches you have based, since I really don't want a new I915_TILING_FANCY define in the enum space used by obj->tiling mode. But makes sense for backwards compat with older platforms and less churn in code. With igt for the new cases in addfb and review this is imo good to get in. -Daniel > + > int > intel_pin_and_fence_fb_obj(struct drm_plane *plane, > struct drm_framebuffer *fb, > @@ -2211,7 +2224,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane, > > WARN_ON(!mutex_is_locked(&dev->struct_mutex)); > > - switch (obj->tiling_mode) { > + switch (intel_fb_tiling_mode(fb)) { > case I915_TILING_NONE: > if (INTEL_INFO(dev)->gen >= 9) > alignment = 256 * 1024; > @@ -2447,7 +2460,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc, > continue; > > if (i915_gem_obj_ggtt_offset(obj) == plane_config->base) { > - if (obj->tiling_mode != I915_TILING_NONE) > + if (intel_fb_tiling_mode(c->primary->fb) != > + I915_TILING_NONE) > dev_priv->preserve_bios_swizzle = true; > > drm_framebuffer_reference(c->primary->fb); > @@ -2471,6 +2485,7 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, > u32 dspcntr; > u32 reg = DSPCNTR(plane); > int pixel_size; > + unsigned int tiling_mode = intel_fb_tiling_mode(fb); > > if (!intel_crtc->primary_enabled) { > I915_WRITE(reg, 0); > @@ -2542,8 +2557,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)) > @@ -2553,7 +2567,7 @@ 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; > @@ -2603,6 +2617,7 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc, > u32 dspcntr; > u32 reg = DSPCNTR(plane); > int pixel_size; > + unsigned int tiling_mode = intel_fb_tiling_mode(fb); > > if (!intel_crtc->primary_enabled) { > I915_WRITE(reg, 0); > @@ -2651,7 +2666,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)) > @@ -2659,7 +2674,7 @@ 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; > @@ -2747,7 +2762,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 (intel_fb_tiling_mode(fb)) { > case I915_TILING_NONE: > stride = fb->pitches[0] >> 6; > break; > @@ -9288,7 +9303,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); > + intel_fb_tiling_mode(fb)); > > /* XXX Enabling the panel-fitter across page-flip is so far > * untested on non-native modes, so ignore it for now. > @@ -9321,7 +9336,7 @@ 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] | intel_fb_tiling_mode(fb)); > intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset); > > /* Contrary to the suggestions in the documentation, > @@ -9425,7 +9440,7 @@ 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] | intel_fb_tiling_mode(fb))); > intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset); > intel_ring_emit(ring, (MI_NOOP)); > > @@ -9466,14 +9481,13 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc) > struct drm_device *dev = intel_crtc->base.dev; > 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; > + unsigned int tiling_mode = intel_fb_tiling_mode(fb); > > ctl = I915_READ(PLANE_CTL(pipe, 0)); > ctl &= ~PLANE_CTL_TILED_MASK; > - if (obj->tiling_mode == I915_TILING_X) > + if (tiling_mode == I915_TILING_X) > ctl |= PLANE_CTL_TILED_X; > > /* > @@ -9481,7 +9495,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 (tiling_mode == I915_TILING_X) > stride = fb->pitches[0] >> 9; /* X tiles are 512 bytes wide */ > > /* > @@ -9499,16 +9513,14 @@ static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc) > { > struct drm_device *dev = intel_crtc->base.dev; > 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; > + struct drm_framebuffer *fb = intel_crtc->base.primary->fb; > 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(fb) != I915_TILING_NONE) > dspcntr |= DISPPLANE_TILED; > else > dspcntr &= ~DISPPLANE_TILED; > @@ -9591,6 +9603,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; > + unsigned int tiling_mode = intel_fb_tiling_mode(fb); > int ret; > > switch(intel_crtc->pipe) { > @@ -9608,7 +9621,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; > @@ -9636,7 +9649,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); > @@ -9811,7 +9824,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > > if (IS_VALLEYVIEW(dev)) { > ring = &dev_priv->ring[BCS]; > - if (obj->tiling_mode != intel_fb_obj(work->old_fb)->tiling_mode) > + if (intel_fb_tiling_mode(fb) != > + intel_fb_tiling_mode(work->old_fb)) > /* vlv: DISPLAY_FLIP fails to change tiling */ > ring = NULL; > } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) { > @@ -12187,7 +12201,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 && > + intel_fb_tiling_mode(fb)) { > DRM_DEBUG_KMS("cursor cannot be tiled\n"); > ret = -EINVAL; > } > @@ -12668,6 +12683,7 @@ static int intel_framebuffer_init(struct drm_device *dev, > { > int aligned_height; > int pitch_limit; > + unsigned int tiling_mode = I915_TILING_NONE; > int ret; > > WARN_ON(!mutex_is_locked(&dev->struct_mutex)); > @@ -12677,6 +12693,29 @@ static int intel_framebuffer_init(struct drm_device *dev, > return -EINVAL; > } > > + /* If obj is tiled and fb modifier not set propagate it in > + * for backward compatibility with old userspace. > + */ > + if (obj->tiling_mode && !(mode_cmd->flags & DRM_MODE_FB_MODIFIERS)) { > + tiling_mode = obj->tiling_mode; > + mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED; > + } > + > + /* Get tiling mode from fb modifier if set. */ > + if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) > + tiling_mode = intel_fb_modifier_to_tiling(mode_cmd->modifier[0]); > + > + /* Ensure new userspace is using the interface correctly by only > + * allowing old usage of set_tiling if it matches with the > + * fb modifier tiling. > + */ > + if (obj->tiling_mode != I915_TILING_NONE && > + obj->tiling_mode != tiling_mode) { > + DRM_ERROR("Tiling modifier mismatch %u vs obj %u!\n", > + tiling_mode, obj->tiling_mode); > + return -EINVAL; > + } > + > if (mode_cmd->pitches[0] & 63) { > DRM_DEBUG("pitch (%d) must be at least 64 byte aligned\n", > mode_cmd->pitches[0]); > @@ -12686,12 +12725,12 @@ static int intel_framebuffer_init(struct drm_device *dev, > if (INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev)) { > pitch_limit = 32*1024; > } else if (INTEL_INFO(dev)->gen >= 4) { > - if (obj->tiling_mode) > + if (tiling_mode) > pitch_limit = 16*1024; > else > pitch_limit = 32*1024; > } else if (INTEL_INFO(dev)->gen >= 3) { > - if (obj->tiling_mode) > + if (tiling_mode) > pitch_limit = 8*1024; > else > pitch_limit = 16*1024; > @@ -12701,12 +12740,12 @@ static int intel_framebuffer_init(struct drm_device *dev, > > if (mode_cmd->pitches[0] > pitch_limit) { > DRM_DEBUG("%s pitch (%d) must be at less than %d\n", > - obj->tiling_mode ? "tiled" : "linear", > + tiling_mode ? "tiled" : "linear", > mode_cmd->pitches[0], pitch_limit); > return -EINVAL; > } > > - if (obj->tiling_mode != I915_TILING_NONE && > + if (tiling_mode != I915_TILING_NONE && obj->stride && > mode_cmd->pitches[0] != obj->stride) { > DRM_DEBUG("pitch (%d) must match tiling stride (%d)\n", > mode_cmd->pitches[0], obj->stride); > @@ -12761,7 +12800,7 @@ static int intel_framebuffer_init(struct drm_device *dev, > return -EINVAL; > > aligned_height = intel_fb_align_height(dev, mode_cmd->height, > - obj->tiling_mode); > + tiling_mode); > /* FIXME drm helper for size checks (especially planar formats)? */ > if (obj->base.size < aligned_height * mode_cmd->pitches[0]) > return -EINVAL; > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index f048f8b..b40dad8 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1008,6 +1008,8 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode, > void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc); > void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file); > > +unsigned int intel_fb_tiling_mode(struct drm_framebuffer *fb); > + > /* intel_dp.c */ > void intel_dp_init(struct drm_device *dev, int output_reg, enum port port); > bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 6ece663..de9e433 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -1182,12 +1182,9 @@ 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); > - > /* self-refresh seems busted with untiled */ > - if (obj->tiling_mode == I915_TILING_NONE) > + if (intel_fb_tiling_mode(enabled->primary->fb) == > + 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..8f89766 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 (intel_fb_tiling_mode(fb)) { > 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 = intel_fb_tiling_mode(fb); > > 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 = intel_fb_tiling_mode(fb); > > 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 = intel_fb_tiling_mode(fb); > > 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 (intel_fb_tiling_mode(fb)) { > 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
On 02/03/2015 07:47 PM, Daniel Vetter wrote: > On Tue, Feb 03, 2015 at 05:22:31PM +0000, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Start using frame buffer modifiers instead of object tiling mode >> for display purposes. >> >> To ensure compatibility with old userspace which is using set_tiling >> and does not know about frame buffer modifiers, the latter are faked >> internally when tile object is set for display. This way all interested >> call sites can use fb modifiers exclusively. >> >> Also ensure tiling specified via fb modifiers must match object tiling >> used for fencing if both are specified. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 95 +++++++++++++++++++++++++----------- >> 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, 85 insertions(+), 45 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 7a3ed61..6825016 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -2198,6 +2198,19 @@ intel_fb_align_height(struct drm_device *dev, int height, unsigned int tiling) >> return ALIGN(height, tile_height); >> } >> >> +static unsigned int intel_fb_modifier_to_tiling(u64 mod) >> +{ >> + BUILD_BUG_ON((I915_FORMAT_MOD_X_TILED & 0x00ffffffffffffffL) != >> + I915_TILING_X); >> + >> + return mod & 1; >> +} >> + >> +unsigned int intel_fb_tiling_mode(struct drm_framebuffer *fb) >> +{ >> + return intel_fb_modifier_to_tiling(fb->modifier[0]); >> +} > > I expect that these here will create a bit of churn with the skl patches > you have based, since I really don't want a new I915_TILING_FANCY define > in the enum space used by obj->tiling mode. But makes sense for backwards > compat with older platforms and less churn in code. I thought we talked about effectively creating a new enum space for fb tiling? By masking out bits from the fb modifier, no? Only thing for backward compatibility is that object X tiling and fb X tiling == 1. > With igt for the new cases in addfb and review this is imo good to get in. I can do the IGT, but who is doing the libdrm part? :) Regards, Tvrtko
On Wed, Feb 04, 2015 at 10:01:45AM +0000, Tvrtko Ursulin wrote: > > On 02/03/2015 07:47 PM, Daniel Vetter wrote: > >On Tue, Feb 03, 2015 at 05:22:31PM +0000, Tvrtko Ursulin wrote: > >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >> > >>Start using frame buffer modifiers instead of object tiling mode > >>for display purposes. > >> > >>To ensure compatibility with old userspace which is using set_tiling > >>and does not know about frame buffer modifiers, the latter are faked > >>internally when tile object is set for display. This way all interested > >>call sites can use fb modifiers exclusively. > >> > >>Also ensure tiling specified via fb modifiers must match object tiling > >>used for fencing if both are specified. > >> > >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>--- > >> drivers/gpu/drm/i915/intel_display.c | 95 +++++++++++++++++++++++++----------- > >> 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, 85 insertions(+), 45 deletions(-) > >> > >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >>index 7a3ed61..6825016 100644 > >>--- a/drivers/gpu/drm/i915/intel_display.c > >>+++ b/drivers/gpu/drm/i915/intel_display.c > >>@@ -2198,6 +2198,19 @@ intel_fb_align_height(struct drm_device *dev, int height, unsigned int tiling) > >> return ALIGN(height, tile_height); > >> } > >> > >>+static unsigned int intel_fb_modifier_to_tiling(u64 mod) > >>+{ > >>+ BUILD_BUG_ON((I915_FORMAT_MOD_X_TILED & 0x00ffffffffffffffL) != > >>+ I915_TILING_X); > >>+ > >>+ return mod & 1; > >>+} > >>+ > >>+unsigned int intel_fb_tiling_mode(struct drm_framebuffer *fb) > >>+{ > >>+ return intel_fb_modifier_to_tiling(fb->modifier[0]); > >>+} > > > >I expect that these here will create a bit of churn with the skl patches > >you have based, since I really don't want a new I915_TILING_FANCY define > >in the enum space used by obj->tiling mode. But makes sense for backwards > >compat with older platforms and less churn in code. > > I thought we talked about effectively creating a new enum space for fb > tiling? By masking out bits from the fb modifier, no? Only thing for > backward compatibility is that object X tiling and fb X tiling == 1. intel_fb_tiling_mode maps modifier (the new enum space) to obj->tiling_mode (the old enum space). Means a notch less churn in legacy code (but if that's the metric I'd just have kept using obj->tiling_mode there). But means that you get to chance skl code twice, because I very much don't want a new I915_TILING_DEFINE but instead the skl code should check the new modifiers directly. Otherwise we can mash up tiling modes valid just for ggtt fencing and fb modifiers in general. Maybe I wasn't really clear with what I've meant ... > >With igt for the new cases in addfb and review this is imo good to get in. > > I can do the IGT, but who is doing the libdrm part? :) Generally when we do igts for new interfaces we just copypaste the new struct definitions with local_ prefixed to avoid blocking the test on a new libdrm release. So no one needs to do a libdrm patch ;-) -Daniel
On 02/04/2015 02:25 PM, Daniel Vetter wrote: > On Wed, Feb 04, 2015 at 10:01:45AM +0000, Tvrtko Ursulin wrote: >> >> On 02/03/2015 07:47 PM, Daniel Vetter wrote: >>> On Tue, Feb 03, 2015 at 05:22:31PM +0000, Tvrtko Ursulin wrote: >>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>> >>>> Start using frame buffer modifiers instead of object tiling mode >>>> for display purposes. >>>> >>>> To ensure compatibility with old userspace which is using set_tiling >>>> and does not know about frame buffer modifiers, the latter are faked >>>> internally when tile object is set for display. This way all interested >>>> call sites can use fb modifiers exclusively. >>>> >>>> Also ensure tiling specified via fb modifiers must match object tiling >>>> used for fencing if both are specified. >>>> >>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>> --- >>>> drivers/gpu/drm/i915/intel_display.c | 95 +++++++++++++++++++++++++----------- >>>> 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, 85 insertions(+), 45 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >>>> index 7a3ed61..6825016 100644 >>>> --- a/drivers/gpu/drm/i915/intel_display.c >>>> +++ b/drivers/gpu/drm/i915/intel_display.c >>>> @@ -2198,6 +2198,19 @@ intel_fb_align_height(struct drm_device *dev, int height, unsigned int tiling) >>>> return ALIGN(height, tile_height); >>>> } >>>> >>>> +static unsigned int intel_fb_modifier_to_tiling(u64 mod) >>>> +{ >>>> + BUILD_BUG_ON((I915_FORMAT_MOD_X_TILED & 0x00ffffffffffffffL) != >>>> + I915_TILING_X); >>>> + >>>> + return mod & 1; >>>> +} >>>> + >>>> +unsigned int intel_fb_tiling_mode(struct drm_framebuffer *fb) >>>> +{ >>>> + return intel_fb_modifier_to_tiling(fb->modifier[0]); >>>> +} >>> >>> I expect that these here will create a bit of churn with the skl patches >>> you have based, since I really don't want a new I915_TILING_FANCY define >>> in the enum space used by obj->tiling mode. But makes sense for backwards >>> compat with older platforms and less churn in code. >> >> I thought we talked about effectively creating a new enum space for fb >> tiling? By masking out bits from the fb modifier, no? Only thing for >> backward compatibility is that object X tiling and fb X tiling == 1. > > intel_fb_tiling_mode maps modifier (the new enum space) to > obj->tiling_mode (the old enum space). Means a notch less churn in legacy > code (but if that's the metric I'd just have kept using obj->tiling_mode > there). But means that you get to chance skl code twice, because I very > much don't want a new I915_TILING_DEFINE but instead the skl code should > check the new modifiers directly. Otherwise we can mash up tiling modes > valid just for ggtt fencing and fb modifiers in general. > > Maybe I wasn't really clear with what I've meant ... It does seem it is taking very long to get on the same page here. :/ I did not plan to add new I915_TILING_xxx. I was exploiting the fact both map to the same value, with masking. So legacy continues to work since this will be true forever. (ABI) Then the plan was to add a new namespace for display tiling enums. This was since fb modifier could contain more than tiling and this way it is possible to mask out and case-switch just as the current code does. There are three namespaces here: 1. I915_TILING_xxx 2. I915_FORMAT_MOD_ (fb modifiers) 3. Tiling as programmed to display hardware And then add a fourth one: 4. I915_DISPLAY_TILING_xxx At this step also add something like I915_FORMAT_MOD_TILING_MASK and redefine I915_FORMAT_MOD_X_TILE to be fourcc_mod(INTEL, I915_DISPLAY_TILING_X). (Instead of hardcoded 1) At call sites (opencoded): switch (fb->modifier[0] & I915_FORMAT_MOD_TILING) { case I915_DISPLAY_TILING_X: ... I mean we could do: switch (fb->modifier[0]) { case I915_FORMAT_MOD_X_TILE: ... If fb modifiers won't have any overlap, like for example: #define I915_FORMAT_MOD_X_TILE fourcc_mod(INTEL, 1) #define I915_FORMAT_MOD_X_TILE_AND_UNRELATED fourcc_mod(INTEL, 1<<8 && 1) Then the direct usage stops working.. Up to you, I have to unblock other stuff so we can't strangle this for too long. >>> With igt for the new cases in addfb and review this is imo good to get in. >> >> I can do the IGT, but who is doing the libdrm part? :) > > Generally when we do igts for new interfaces we just copypaste the new > struct definitions with local_ prefixed to avoid blocking the test on a > new libdrm release. So no one needs to do a libdrm patch ;-) Okay I can do that. Even better that's what I already did. :) Regards, Tvrtko
On Wed, Feb 04, 2015 at 03:09:38PM +0000, Tvrtko Ursulin wrote: > On 02/04/2015 02:25 PM, Daniel Vetter wrote: > >On Wed, Feb 04, 2015 at 10:01:45AM +0000, Tvrtko Ursulin wrote: > >> > >>On 02/03/2015 07:47 PM, Daniel Vetter wrote: > >>>On Tue, Feb 03, 2015 at 05:22:31PM +0000, Tvrtko Ursulin wrote: > >>>>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>> > >>>>Start using frame buffer modifiers instead of object tiling mode > >>>>for display purposes. > >>>> > >>>>To ensure compatibility with old userspace which is using set_tiling > >>>>and does not know about frame buffer modifiers, the latter are faked > >>>>internally when tile object is set for display. This way all interested > >>>>call sites can use fb modifiers exclusively. > >>>> > >>>>Also ensure tiling specified via fb modifiers must match object tiling > >>>>used for fencing if both are specified. > >>>> > >>>>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>>--- > >>>> drivers/gpu/drm/i915/intel_display.c | 95 +++++++++++++++++++++++++----------- > >>>> 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, 85 insertions(+), 45 deletions(-) > >>>> > >>>>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >>>>index 7a3ed61..6825016 100644 > >>>>--- a/drivers/gpu/drm/i915/intel_display.c > >>>>+++ b/drivers/gpu/drm/i915/intel_display.c > >>>>@@ -2198,6 +2198,19 @@ intel_fb_align_height(struct drm_device *dev, int height, unsigned int tiling) > >>>> return ALIGN(height, tile_height); > >>>> } > >>>> > >>>>+static unsigned int intel_fb_modifier_to_tiling(u64 mod) > >>>>+{ > >>>>+ BUILD_BUG_ON((I915_FORMAT_MOD_X_TILED & 0x00ffffffffffffffL) != > >>>>+ I915_TILING_X); > >>>>+ > >>>>+ return mod & 1; > >>>>+} > >>>>+ > >>>>+unsigned int intel_fb_tiling_mode(struct drm_framebuffer *fb) > >>>>+{ > >>>>+ return intel_fb_modifier_to_tiling(fb->modifier[0]); > >>>>+} > >>> > >>>I expect that these here will create a bit of churn with the skl patches > >>>you have based, since I really don't want a new I915_TILING_FANCY define > >>>in the enum space used by obj->tiling mode. But makes sense for backwards > >>>compat with older platforms and less churn in code. > >> > >>I thought we talked about effectively creating a new enum space for fb > >>tiling? By masking out bits from the fb modifier, no? Only thing for > >>backward compatibility is that object X tiling and fb X tiling == 1. > > > >intel_fb_tiling_mode maps modifier (the new enum space) to > >obj->tiling_mode (the old enum space). Means a notch less churn in legacy > >code (but if that's the metric I'd just have kept using obj->tiling_mode > >there). But means that you get to chance skl code twice, because I very > >much don't want a new I915_TILING_DEFINE but instead the skl code should > >check the new modifiers directly. Otherwise we can mash up tiling modes > >valid just for ggtt fencing and fb modifiers in general. > > > >Maybe I wasn't really clear with what I've meant ... > > It does seem it is taking very long to get on the same page here. :/ > > I did not plan to add new I915_TILING_xxx. I was exploiting the fact both > map to the same value, with masking. So legacy continues to work since this > will be true forever. (ABI) > > Then the plan was to add a new namespace for display tiling enums. > > This was since fb modifier could contain more than tiling and this way it is > possible to mask out and case-switch just as the current code does. > > There are three namespaces here: > > 1. I915_TILING_xxx > 2. I915_FORMAT_MOD_ (fb modifiers) > 3. Tiling as programmed to display hardware > > And then add a fourth one: > > 4. I915_DISPLAY_TILING_xxx > > At this step also add something like I915_FORMAT_MOD_TILING_MASK and > redefine I915_FORMAT_MOD_X_TILE to be fourcc_mod(INTEL, > I915_DISPLAY_TILING_X). (Instead of hardcoded 1) > > At call sites (opencoded): > > switch (fb->modifier[0] & I915_FORMAT_MOD_TILING) { > case I915_DISPLAY_TILING_X: This is kinda what I'd have done, expect that you can cleverly define the mask to include the vendor prefix, i.e. #define I915_FORMAT_MOD_TILING_MASK ((0xff << 56) | 0xff) and then you don't need yet another set of defines. And still have the clear separation between I915_TILING_FOO and the new fb modifier stuff. > ... > > I mean we could do: > > switch (fb->modifier[0]) { > case I915_FORMAT_MOD_X_TILE: Or this. Since we don't yet have anything else than tiling modes you'll get away with it and can postpone the mask stuff to whomever ends up implementing the non-tiling fb modifiers. > ... > > If fb modifiers won't have any overlap, like for example: > > #define I915_FORMAT_MOD_X_TILE fourcc_mod(INTEL, 1) > #define I915_FORMAT_MOD_X_TILE_AND_UNRELATED fourcc_mod(INTEL, 1<<8 && 1) > > Then the direct usage stops working.. > > Up to you, I have to unblock other stuff so we can't strangle this for too > long. The super-minimal approach would be to shrink this patch down to the fixup/check code in framebuffer_init and then move the conversion for skl display code (and just that) into the next series which adds the fancy skl patches. And use one of the switch statements above to decode the fb modifier. Goes well with my default stance of "in case of doubt, pick less churn". Cheers, Daniel
On 02/04/2015 03:33 PM, Daniel Vetter wrote: > On Wed, Feb 04, 2015 at 03:09:38PM +0000, Tvrtko Ursulin wrote: >> On 02/04/2015 02:25 PM, Daniel Vetter wrote: >>> On Wed, Feb 04, 2015 at 10:01:45AM +0000, Tvrtko Ursulin wrote: >>>> >>>> On 02/03/2015 07:47 PM, Daniel Vetter wrote: >>>>> On Tue, Feb 03, 2015 at 05:22:31PM +0000, Tvrtko Ursulin wrote: >>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>>>> >>>>>> Start using frame buffer modifiers instead of object tiling mode >>>>>> for display purposes. >>>>>> >>>>>> To ensure compatibility with old userspace which is using set_tiling >>>>>> and does not know about frame buffer modifiers, the latter are faked >>>>>> internally when tile object is set for display. This way all interested >>>>>> call sites can use fb modifiers exclusively. >>>>>> >>>>>> Also ensure tiling specified via fb modifiers must match object tiling >>>>>> used for fencing if both are specified. >>>>>> >>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>>>>> --- >>>>>> drivers/gpu/drm/i915/intel_display.c | 95 +++++++++++++++++++++++++----------- >>>>>> 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, 85 insertions(+), 45 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >>>>>> index 7a3ed61..6825016 100644 >>>>>> --- a/drivers/gpu/drm/i915/intel_display.c >>>>>> +++ b/drivers/gpu/drm/i915/intel_display.c >>>>>> @@ -2198,6 +2198,19 @@ intel_fb_align_height(struct drm_device *dev, int height, unsigned int tiling) >>>>>> return ALIGN(height, tile_height); >>>>>> } >>>>>> >>>>>> +static unsigned int intel_fb_modifier_to_tiling(u64 mod) >>>>>> +{ >>>>>> + BUILD_BUG_ON((I915_FORMAT_MOD_X_TILED & 0x00ffffffffffffffL) != >>>>>> + I915_TILING_X); >>>>>> + >>>>>> + return mod & 1; >>>>>> +} >>>>>> + >>>>>> +unsigned int intel_fb_tiling_mode(struct drm_framebuffer *fb) >>>>>> +{ >>>>>> + return intel_fb_modifier_to_tiling(fb->modifier[0]); >>>>>> +} >>>>> >>>>> I expect that these here will create a bit of churn with the skl patches >>>>> you have based, since I really don't want a new I915_TILING_FANCY define >>>>> in the enum space used by obj->tiling mode. But makes sense for backwards >>>>> compat with older platforms and less churn in code. >>>> >>>> I thought we talked about effectively creating a new enum space for fb >>>> tiling? By masking out bits from the fb modifier, no? Only thing for >>>> backward compatibility is that object X tiling and fb X tiling == 1. >>> >>> intel_fb_tiling_mode maps modifier (the new enum space) to >>> obj->tiling_mode (the old enum space). Means a notch less churn in legacy >>> code (but if that's the metric I'd just have kept using obj->tiling_mode >>> there). But means that you get to chance skl code twice, because I very >>> much don't want a new I915_TILING_DEFINE but instead the skl code should >>> check the new modifiers directly. Otherwise we can mash up tiling modes >>> valid just for ggtt fencing and fb modifiers in general. >>> >>> Maybe I wasn't really clear with what I've meant ... >> >> It does seem it is taking very long to get on the same page here. :/ >> >> I did not plan to add new I915_TILING_xxx. I was exploiting the fact both >> map to the same value, with masking. So legacy continues to work since this >> will be true forever. (ABI) >> >> Then the plan was to add a new namespace for display tiling enums. >> >> This was since fb modifier could contain more than tiling and this way it is >> possible to mask out and case-switch just as the current code does. >> >> There are three namespaces here: >> >> 1. I915_TILING_xxx >> 2. I915_FORMAT_MOD_ (fb modifiers) >> 3. Tiling as programmed to display hardware >> >> And then add a fourth one: >> >> 4. I915_DISPLAY_TILING_xxx >> >> At this step also add something like I915_FORMAT_MOD_TILING_MASK and >> redefine I915_FORMAT_MOD_X_TILE to be fourcc_mod(INTEL, >> I915_DISPLAY_TILING_X). (Instead of hardcoded 1) >> >> At call sites (opencoded): >> >> switch (fb->modifier[0] & I915_FORMAT_MOD_TILING) { >> case I915_DISPLAY_TILING_X: > > This is kinda what I'd have done, expect that you can cleverly define the > mask to include the vendor prefix, i.e. > > #define I915_FORMAT_MOD_TILING_MASK ((0xff << 56) | 0xff) > > and then you don't need yet another set of defines. And still have the > clear separation between I915_TILING_FOO and the new fb modifier stuff. Hm side question - maybe DRM patch could instead of allow_fb_modifiers boolean take allow_fb_modifier = VENDORA | VENDORB, and then stem at the source any attempts to pass unsupported ones to the driver. :) >> ... >> >> I mean we could do: >> >> switch (fb->modifier[0]) { >> case I915_FORMAT_MOD_X_TILE: > > Or this. Since we don't yet have anything else than tiling modes you'll > get away with it and can postpone the mask stuff to whomever ends up > implementing the non-tiling fb modifiers. Not nice but you told me to do it. :D >> ... >> >> If fb modifiers won't have any overlap, like for example: >> >> #define I915_FORMAT_MOD_X_TILE fourcc_mod(INTEL, 1) >> #define I915_FORMAT_MOD_X_TILE_AND_UNRELATED fourcc_mod(INTEL, 1<<8 && 1) >> >> Then the direct usage stops working.. >> >> Up to you, I have to unblock other stuff so we can't strangle this for too >> long. > > The super-minimal approach would be to shrink this patch down to the > fixup/check code in framebuffer_init and then move the conversion for skl > display code (and just that) into the next series which adds the fancy skl > patches. And use one of the switch statements above to decode the fb > modifier. Goes well with my default stance of "in case of doubt, pick less > churn". Disallow fb modifiers on gen < 9 regardless of DRM_CAP? Sounds nasty.. Regards, Tvrtko
On Wed, Feb 04, 2015 at 03:44:58PM +0000, Tvrtko Ursulin wrote: > > On 02/04/2015 03:33 PM, Daniel Vetter wrote: > >On Wed, Feb 04, 2015 at 03:09:38PM +0000, Tvrtko Ursulin wrote: > >>On 02/04/2015 02:25 PM, Daniel Vetter wrote: > >>>On Wed, Feb 04, 2015 at 10:01:45AM +0000, Tvrtko Ursulin wrote: > >>>> > >>>>On 02/03/2015 07:47 PM, Daniel Vetter wrote: > >>>>>On Tue, Feb 03, 2015 at 05:22:31PM +0000, Tvrtko Ursulin wrote: > >>>>>>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>>>> > >>>>>>Start using frame buffer modifiers instead of object tiling mode > >>>>>>for display purposes. > >>>>>> > >>>>>>To ensure compatibility with old userspace which is using set_tiling > >>>>>>and does not know about frame buffer modifiers, the latter are faked > >>>>>>internally when tile object is set for display. This way all interested > >>>>>>call sites can use fb modifiers exclusively. > >>>>>> > >>>>>>Also ensure tiling specified via fb modifiers must match object tiling > >>>>>>used for fencing if both are specified. > >>>>>> > >>>>>>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>>>>>--- > >>>>>> drivers/gpu/drm/i915/intel_display.c | 95 +++++++++++++++++++++++++----------- > >>>>>> 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, 85 insertions(+), 45 deletions(-) > >>>>>> > >>>>>>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >>>>>>index 7a3ed61..6825016 100644 > >>>>>>--- a/drivers/gpu/drm/i915/intel_display.c > >>>>>>+++ b/drivers/gpu/drm/i915/intel_display.c > >>>>>>@@ -2198,6 +2198,19 @@ intel_fb_align_height(struct drm_device *dev, int height, unsigned int tiling) > >>>>>> return ALIGN(height, tile_height); > >>>>>> } > >>>>>> > >>>>>>+static unsigned int intel_fb_modifier_to_tiling(u64 mod) > >>>>>>+{ > >>>>>>+ BUILD_BUG_ON((I915_FORMAT_MOD_X_TILED & 0x00ffffffffffffffL) != > >>>>>>+ I915_TILING_X); > >>>>>>+ > >>>>>>+ return mod & 1; > >>>>>>+} > >>>>>>+ > >>>>>>+unsigned int intel_fb_tiling_mode(struct drm_framebuffer *fb) > >>>>>>+{ > >>>>>>+ return intel_fb_modifier_to_tiling(fb->modifier[0]); > >>>>>>+} > >>>>> > >>>>>I expect that these here will create a bit of churn with the skl patches > >>>>>you have based, since I really don't want a new I915_TILING_FANCY define > >>>>>in the enum space used by obj->tiling mode. But makes sense for backwards > >>>>>compat with older platforms and less churn in code. > >>>> > >>>>I thought we talked about effectively creating a new enum space for fb > >>>>tiling? By masking out bits from the fb modifier, no? Only thing for > >>>>backward compatibility is that object X tiling and fb X tiling == 1. > >>> > >>>intel_fb_tiling_mode maps modifier (the new enum space) to > >>>obj->tiling_mode (the old enum space). Means a notch less churn in legacy > >>>code (but if that's the metric I'd just have kept using obj->tiling_mode > >>>there). But means that you get to chance skl code twice, because I very > >>>much don't want a new I915_TILING_DEFINE but instead the skl code should > >>>check the new modifiers directly. Otherwise we can mash up tiling modes > >>>valid just for ggtt fencing and fb modifiers in general. > >>> > >>>Maybe I wasn't really clear with what I've meant ... > >> > >>It does seem it is taking very long to get on the same page here. :/ > >> > >>I did not plan to add new I915_TILING_xxx. I was exploiting the fact both > >>map to the same value, with masking. So legacy continues to work since this > >>will be true forever. (ABI) > >> > >>Then the plan was to add a new namespace for display tiling enums. > >> > >>This was since fb modifier could contain more than tiling and this way it is > >>possible to mask out and case-switch just as the current code does. > >> > >>There are three namespaces here: > >> > >>1. I915_TILING_xxx > >>2. I915_FORMAT_MOD_ (fb modifiers) > >>3. Tiling as programmed to display hardware > >> > >>And then add a fourth one: > >> > >>4. I915_DISPLAY_TILING_xxx > >> > >>At this step also add something like I915_FORMAT_MOD_TILING_MASK and > >>redefine I915_FORMAT_MOD_X_TILE to be fourcc_mod(INTEL, > >>I915_DISPLAY_TILING_X). (Instead of hardcoded 1) > >> > >>At call sites (opencoded): > >> > >>switch (fb->modifier[0] & I915_FORMAT_MOD_TILING) { > >>case I915_DISPLAY_TILING_X: > > > >This is kinda what I'd have done, expect that you can cleverly define the > >mask to include the vendor prefix, i.e. > > > >#define I915_FORMAT_MOD_TILING_MASK ((0xff << 56) | 0xff) > > > >and then you don't need yet another set of defines. And still have the > >clear separation between I915_TILING_FOO and the new fb modifier stuff. > > Hm side question - maybe DRM patch could instead of allow_fb_modifiers > boolean take allow_fb_modifier = VENDORA | VENDORB, and then stem at the > source any attempts to pass unsupported ones to the driver. :) > > >>... > >> > >>I mean we could do: > >> > >>switch (fb->modifier[0]) { > >>case I915_FORMAT_MOD_X_TILE: > > > >Or this. Since we don't yet have anything else than tiling modes you'll > >get away with it and can postpone the mask stuff to whomever ends up > >implementing the non-tiling fb modifiers. > > Not nice but you told me to do it. :D > > >>... > >> > >>If fb modifiers won't have any overlap, like for example: > >> > >>#define I915_FORMAT_MOD_X_TILE fourcc_mod(INTEL, 1) > >>#define I915_FORMAT_MOD_X_TILE_AND_UNRELATED fourcc_mod(INTEL, 1<<8 && 1) > >> > >>Then the direct usage stops working.. > >> > >>Up to you, I have to unblock other stuff so we can't strangle this for too > >>long. > > > >The super-minimal approach would be to shrink this patch down to the > >fixup/check code in framebuffer_init and then move the conversion for skl > >display code (and just that) into the next series which adds the fancy skl > >patches. And use one of the switch statements above to decode the fb > >modifier. Goes well with my default stance of "in case of doubt, pick less > >churn". > > Disallow fb modifiers on gen < 9 regardless of DRM_CAP? Sounds nasty.. Nope, we'd allow the in framebuffer_init, but the only thing you can do is ask for TILE_X, and it must match with obj->tiling. Hm, that gives a slightly different check than what you have now, but really shouldn't be a restriction since scanout stuff is always allocated as separate buffers and userspace does an unconditional set_tiling when using X-tiled. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7a3ed61..6825016 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2198,6 +2198,19 @@ intel_fb_align_height(struct drm_device *dev, int height, unsigned int tiling) return ALIGN(height, tile_height); } +static unsigned int intel_fb_modifier_to_tiling(u64 mod) +{ + BUILD_BUG_ON((I915_FORMAT_MOD_X_TILED & 0x00ffffffffffffffL) != + I915_TILING_X); + + return mod & 1; +} + +unsigned int intel_fb_tiling_mode(struct drm_framebuffer *fb) +{ + return intel_fb_modifier_to_tiling(fb->modifier[0]); +} + int intel_pin_and_fence_fb_obj(struct drm_plane *plane, struct drm_framebuffer *fb, @@ -2211,7 +2224,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane, WARN_ON(!mutex_is_locked(&dev->struct_mutex)); - switch (obj->tiling_mode) { + switch (intel_fb_tiling_mode(fb)) { case I915_TILING_NONE: if (INTEL_INFO(dev)->gen >= 9) alignment = 256 * 1024; @@ -2447,7 +2460,8 @@ intel_find_plane_obj(struct intel_crtc *intel_crtc, continue; if (i915_gem_obj_ggtt_offset(obj) == plane_config->base) { - if (obj->tiling_mode != I915_TILING_NONE) + if (intel_fb_tiling_mode(c->primary->fb) != + I915_TILING_NONE) dev_priv->preserve_bios_swizzle = true; drm_framebuffer_reference(c->primary->fb); @@ -2471,6 +2485,7 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, u32 dspcntr; u32 reg = DSPCNTR(plane); int pixel_size; + unsigned int tiling_mode = intel_fb_tiling_mode(fb); if (!intel_crtc->primary_enabled) { I915_WRITE(reg, 0); @@ -2542,8 +2557,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)) @@ -2553,7 +2567,7 @@ 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; @@ -2603,6 +2617,7 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc, u32 dspcntr; u32 reg = DSPCNTR(plane); int pixel_size; + unsigned int tiling_mode = intel_fb_tiling_mode(fb); if (!intel_crtc->primary_enabled) { I915_WRITE(reg, 0); @@ -2651,7 +2666,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)) @@ -2659,7 +2674,7 @@ 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; @@ -2747,7 +2762,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 (intel_fb_tiling_mode(fb)) { case I915_TILING_NONE: stride = fb->pitches[0] >> 6; break; @@ -9288,7 +9303,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); + intel_fb_tiling_mode(fb)); /* XXX Enabling the panel-fitter across page-flip is so far * untested on non-native modes, so ignore it for now. @@ -9321,7 +9336,7 @@ 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] | intel_fb_tiling_mode(fb)); intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset); /* Contrary to the suggestions in the documentation, @@ -9425,7 +9440,7 @@ 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] | intel_fb_tiling_mode(fb))); intel_ring_emit(ring, intel_crtc->unpin_work->gtt_offset); intel_ring_emit(ring, (MI_NOOP)); @@ -9466,14 +9481,13 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc) struct drm_device *dev = intel_crtc->base.dev; 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; + unsigned int tiling_mode = intel_fb_tiling_mode(fb); ctl = I915_READ(PLANE_CTL(pipe, 0)); ctl &= ~PLANE_CTL_TILED_MASK; - if (obj->tiling_mode == I915_TILING_X) + if (tiling_mode == I915_TILING_X) ctl |= PLANE_CTL_TILED_X; /* @@ -9481,7 +9495,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 (tiling_mode == I915_TILING_X) stride = fb->pitches[0] >> 9; /* X tiles are 512 bytes wide */ /* @@ -9499,16 +9513,14 @@ static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc) { struct drm_device *dev = intel_crtc->base.dev; 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; + struct drm_framebuffer *fb = intel_crtc->base.primary->fb; 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(fb) != I915_TILING_NONE) dspcntr |= DISPPLANE_TILED; else dspcntr &= ~DISPPLANE_TILED; @@ -9591,6 +9603,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; + unsigned int tiling_mode = intel_fb_tiling_mode(fb); int ret; switch(intel_crtc->pipe) { @@ -9608,7 +9621,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; @@ -9636,7 +9649,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); @@ -9811,7 +9824,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, if (IS_VALLEYVIEW(dev)) { ring = &dev_priv->ring[BCS]; - if (obj->tiling_mode != intel_fb_obj(work->old_fb)->tiling_mode) + if (intel_fb_tiling_mode(fb) != + intel_fb_tiling_mode(work->old_fb)) /* vlv: DISPLAY_FLIP fails to change tiling */ ring = NULL; } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) { @@ -12187,7 +12201,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 && + intel_fb_tiling_mode(fb)) { DRM_DEBUG_KMS("cursor cannot be tiled\n"); ret = -EINVAL; } @@ -12668,6 +12683,7 @@ static int intel_framebuffer_init(struct drm_device *dev, { int aligned_height; int pitch_limit; + unsigned int tiling_mode = I915_TILING_NONE; int ret; WARN_ON(!mutex_is_locked(&dev->struct_mutex)); @@ -12677,6 +12693,29 @@ static int intel_framebuffer_init(struct drm_device *dev, return -EINVAL; } + /* If obj is tiled and fb modifier not set propagate it in + * for backward compatibility with old userspace. + */ + if (obj->tiling_mode && !(mode_cmd->flags & DRM_MODE_FB_MODIFIERS)) { + tiling_mode = obj->tiling_mode; + mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED; + } + + /* Get tiling mode from fb modifier if set. */ + if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) + tiling_mode = intel_fb_modifier_to_tiling(mode_cmd->modifier[0]); + + /* Ensure new userspace is using the interface correctly by only + * allowing old usage of set_tiling if it matches with the + * fb modifier tiling. + */ + if (obj->tiling_mode != I915_TILING_NONE && + obj->tiling_mode != tiling_mode) { + DRM_ERROR("Tiling modifier mismatch %u vs obj %u!\n", + tiling_mode, obj->tiling_mode); + return -EINVAL; + } + if (mode_cmd->pitches[0] & 63) { DRM_DEBUG("pitch (%d) must be at least 64 byte aligned\n", mode_cmd->pitches[0]); @@ -12686,12 +12725,12 @@ static int intel_framebuffer_init(struct drm_device *dev, if (INTEL_INFO(dev)->gen >= 5 && !IS_VALLEYVIEW(dev)) { pitch_limit = 32*1024; } else if (INTEL_INFO(dev)->gen >= 4) { - if (obj->tiling_mode) + if (tiling_mode) pitch_limit = 16*1024; else pitch_limit = 32*1024; } else if (INTEL_INFO(dev)->gen >= 3) { - if (obj->tiling_mode) + if (tiling_mode) pitch_limit = 8*1024; else pitch_limit = 16*1024; @@ -12701,12 +12740,12 @@ static int intel_framebuffer_init(struct drm_device *dev, if (mode_cmd->pitches[0] > pitch_limit) { DRM_DEBUG("%s pitch (%d) must be at less than %d\n", - obj->tiling_mode ? "tiled" : "linear", + tiling_mode ? "tiled" : "linear", mode_cmd->pitches[0], pitch_limit); return -EINVAL; } - if (obj->tiling_mode != I915_TILING_NONE && + if (tiling_mode != I915_TILING_NONE && obj->stride && mode_cmd->pitches[0] != obj->stride) { DRM_DEBUG("pitch (%d) must match tiling stride (%d)\n", mode_cmd->pitches[0], obj->stride); @@ -12761,7 +12800,7 @@ static int intel_framebuffer_init(struct drm_device *dev, return -EINVAL; aligned_height = intel_fb_align_height(dev, mode_cmd->height, - obj->tiling_mode); + tiling_mode); /* FIXME drm helper for size checks (especially planar formats)? */ if (obj->base.size < aligned_height * mode_cmd->pitches[0]) return -EINVAL; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index f048f8b..b40dad8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1008,6 +1008,8 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode, void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc); void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file); +unsigned int intel_fb_tiling_mode(struct drm_framebuffer *fb); + /* intel_dp.c */ void intel_dp_init(struct drm_device *dev, int output_reg, enum port port); bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 6ece663..de9e433 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1182,12 +1182,9 @@ 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); - /* self-refresh seems busted with untiled */ - if (obj->tiling_mode == I915_TILING_NONE) + if (intel_fb_tiling_mode(enabled->primary->fb) == + 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..8f89766 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 (intel_fb_tiling_mode(fb)) { 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 = intel_fb_tiling_mode(fb); 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 = intel_fb_tiling_mode(fb); 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 = intel_fb_tiling_mode(fb); 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 (intel_fb_tiling_mode(fb)) { case I915_TILING_NONE: case I915_TILING_X: break;