[Intel-gfx] [PATCH 74/89] drm/i915/skl: Implement queue_flip
Paulo Zanoni
przanoni at gmail.com
Tue Sep 23 22:06:35 CEST 2014
2014-09-04 8:27 GMT-03:00 Damien Lespiau <damien.lespiau at intel.com>:
> A few bits have changed in MI_DISPLAY_FLIP to accomodate the new planes.
> DE_RRMR seems to have kept its plane flip bits backward compatible.
>
> v2: Rebase on top of nightly
> v2: Rebase on top of nightly (minor conflict in i915_reg.h)
>
> Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 10 +++++
> drivers/gpu/drm/i915/intel_display.c | 78 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 88 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 84a0de6..176e84e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -240,6 +240,16 @@
> #define MI_DISPLAY_FLIP_IVB_SPRITE_B (3 << 19)
> #define MI_DISPLAY_FLIP_IVB_PLANE_C (4 << 19)
> #define MI_DISPLAY_FLIP_IVB_SPRITE_C (5 << 19)
> +/* SKL ones */
> +#define MI_DISPLAY_FLIP_SKL_PLANE_1_A (0 << 8)
> +#define MI_DISPLAY_FLIP_SKL_PLANE_1_B (1 << 8)
> +#define MI_DISPLAY_FLIP_SKL_PLANE_1_C (2 << 8)
> +#define MI_DISPLAY_FLIP_SKL_PLANE_2_A (4 << 8)
> +#define MI_DISPLAY_FLIP_SKL_PLANE_2_B (5 << 8)
> +#define MI_DISPLAY_FLIP_SKL_PLANE_2_C (6 << 8)
> +#define MI_DISPLAY_FLIP_SKL_PLANE_3_A (7 << 8)
> +#define MI_DISPLAY_FLIP_SKL_PLANE_3_B (8 << 8)
> +#define MI_DISPLAY_FLIP_SKL_PLANE_3_C (9 << 8)
BSpec seems to mention these bits on CHV too... Maybe we want the new
function to run on CHV + GEN9? Ping Ville.
> #define MI_SEMAPHORE_MBOX MI_INSTR(0x16, 1) /* gen6, gen7 */
> #define MI_SEMAPHORE_GLOBAL_GTT (1<<22)
> #define MI_SEMAPHORE_UPDATE (1<<21)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index abd4201..393bd19 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9913,6 +9913,81 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
> return 0;
> }
>
> +static int intel_gen9_queue_flip(struct drm_device *dev,
> + struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + struct drm_i915_gem_object *obj,
> + struct intel_engine_cs *ring,
> + uint32_t flags)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + uint32_t plane = 0, stride;
> + int ret;
> +
> + ring = obj->ring;
> + if (ring == NULL || ring->id != BCS)
> + ring = &dev_priv->ring[RCS];
Why do we need these lines above? The other Gens don't have it. And it
looks like ring shouldn't really be NULL, otherwise the other gens are
going to crash.
> +
> + ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
> + if (ret)
> + goto err;
Our only caller (intel_crtc_page_flip) seems to do this for us
already. Also, the other gens don't do this at their queue_flip
implementations.
> +
> + switch(intel_crtc->pipe) {
> + case PIPE_A:
> + plane = MI_DISPLAY_FLIP_SKL_PLANE_1_A;
> + break;
> + case PIPE_B:
> + plane = MI_DISPLAY_FLIP_SKL_PLANE_1_B;
> + break;
> + case PIPE_C:
> + plane = MI_DISPLAY_FLIP_SKL_PLANE_1_C;
> + break;
> + default:
> + BUG();
The gen7 version does WARN_ONCE() and returns -ENODEV instead of
BUG(). Seems more reasonable to just not update the screen instead of
killing the whole machine.
> + }
> +
> + switch (obj->tiling_mode) {
> + case I915_TILING_NONE:
> + stride = fb->pitches[0] >> 6;
> + break;
> + case I915_TILING_X:
> + stride = fb->pitches[0] >> 9;
> + break;
> + default:
> + BUG();
Also replace this for a WARN and return an error code?
> + }
> +
> + ret = intel_ring_begin(ring, 10);
> + if (ret)
> + goto err_unpin;
> +
> + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> + intel_ring_emit(ring, DERRMR);
> + intel_ring_emit(ring, ~(DERRMR_PIPEA_PRI_FLIP_DONE |
> + DERRMR_PIPEB_PRI_FLIP_DONE |
> + DERRMR_PIPEC_PRI_FLIP_DONE));
> + intel_ring_emit(ring, MI_STORE_REGISTER_MEM_GEN8(1) |
> + MI_SRM_LRM_GLOBAL_GTT);
> + intel_ring_emit(ring, DERRMR);
> + intel_ring_emit(ring, ring->scratch.gtt_offset + 256);
> + intel_ring_emit(ring, 0);
Do we still need the DERRMR thing? BSpec doesn't seem to mention it
anymore on the SKL page. And we only do it for RCS on Gens 7/8.
> +
> + intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane);
> + intel_ring_emit(ring, stride << 6 | obj->tiling_mode);
> + intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj));
Gen 7 uses intel_crtc->unpin_work->gtt_offset, which seems to be a
little faster than calling i915_gem_obj_ggtt_offset() again. The
work->gtt_offset was just set by the function that calls queue_flip(),
so it should be correct.
> +
> + intel_mark_page_flip_active(intel_crtc);
> + __intel_ring_advance(ring);
> +
> + return 0;
> +
> +err_unpin:
> + intel_unpin_fb_obj(obj);
> +err:
> + return ret;
> +}
> +
> static int intel_default_queue_flip(struct drm_device *dev,
> struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> @@ -12740,6 +12815,9 @@ static void intel_init_display(struct drm_device *dev)
> case 8: /* FIXME(BDW): Check that the gen8 RCS flip works. */
> dev_priv->display.queue_flip = intel_gen7_queue_flip;
> break;
> + case 9:
> + dev_priv->display.queue_flip = intel_gen9_queue_flip;
> + break;
> }
>
> intel_panel_init_backlight_funcs(dev);
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
More information about the Intel-gfx
mailing list