2013-07-23 02:02:22

by Keith Packard

[permalink] [raw]
Subject: drm: Asynchronouse page flipping interface and Intel implementation

Here's a sequence of five patches that exposes an interface to request
of the driver that the page flipping request be executed without
waiting for vblank. It's optional, and drivers can expose whether it
is supported through the existing GETCAP ioctl.

This supports only Ivybridge and Sandybridge Intel graphics chips as
that's what I've got to test on; of course all of the other drivers
have been modified so that they still compile cleanly.


--
[email protected]


2013-07-23 02:02:23

by Keith Packard

[permalink] [raw]
Subject: [PATCH 5/5] drm/i915: Add async page flip support for SNB

Just copies the IVB code

Signed-off-by: Keith Packard <[email protected]>
---
drivers/gpu/drm/i915/intel_display.c | 39 ++++++++++++++++++++++++++++++++----
1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1bcc6b4..9d7919b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7463,20 +7463,51 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_ring_buffer *ring = &dev_priv->ring[RCS];
uint32_t pf, pipesrc;
+ uint32_t cmd;
+ uint32_t base;
int ret;

ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
if (ret)
goto err;

+ cmd = MI_DISPLAY_FLIP | MI_DISPLAY_FLIP_PLANE(intel_crtc->plane);
+ base = i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset;
+
+ if (flags & DRM_MODE_PAGE_FLIP_ASYNC) {
+
+ /* XXX check limitations for async flip here */
+
+ if (fb->pitches[0] != I915_READ(DSPSTRIDE(intel_crtc->plane))) {
+ WARN_ONCE(1, "mismatching stride in async plane flip (%d != %d)\n",
+ fb->pitches[0], I915_READ(DSPSTRIDE(intel_crtc->plane)));
+ ret = -EINVAL;
+ goto err_unpin;
+ }
+
+ if (obj->tiling_mode != I915_TILING_X) {
+ WARN_ONCE(1, "async plane flip requires X tiling\n");
+ ret = -EINVAL;
+ goto err_unpin;
+ }
+
+ if ((I915_READ(DSPCNTR(intel_crtc->plane)) & DISPPLANE_TILED) == 0) {
+ WARN_ONCE(1, "display not currently tiled in async plane flip\n");
+ ret = -EINVAL;
+ goto err_unpin;
+ }
+
+ cmd |= MI_DISPLAY_FLIP_ASYNC_INDICATOR;
+ base |= MI_DISPLAY_FLIP_TYPE_ASYNC;
+ }
+
ret = intel_ring_begin(ring, 4);
if (ret)
goto err_unpin;

- intel_ring_emit(ring, MI_DISPLAY_FLIP |
- MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
+ intel_ring_emit(ring, cmd);
intel_ring_emit(ring, fb->pitches[0] | obj->tiling_mode);
- intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
+ intel_ring_emit(ring, base);

/* Contrary to the suggestions in the documentation,
* "Enable Panel Fitter" does not seem to be required when page
@@ -9738,7 +9769,7 @@ void intel_modeset_init(struct drm_device *dev)
dev->mode_config.max_height = 8192;
}

- if (IS_GEN7(dev))
+ if (IS_GEN6(dev) || IS_GEN7(dev))
dev->mode_config.async_page_flip = true;

dev->mode_config.fb_base = dev_priv->gtt.mappable_base;
--
1.8.3.2

2013-07-23 02:02:25

by Keith Packard

[permalink] [raw]
Subject: [PATCH 3/5] drm: Advertise async page flip ability through GETCAP ioctl

Let applications know whether the kernel supports asynchronous page
flipping.

Signed-off-by: Keith Packard <[email protected]>
---
drivers/gpu/drm/drm_crtc.c | 3 +++
drivers/gpu/drm/drm_ioctl.c | 3 +++
include/drm/drm_crtc.h | 3 +++
include/uapi/drm/drm.h | 1 +
4 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 989072c..0909af6 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3514,6 +3514,9 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
page_flip->reserved != 0)
return -EINVAL;

+ if ((page_flip->flags & DRM_MODE_PAGE_FLIP_ASYNC) && !dev->mode_config.async_page_flip)
+ return -EINVAL;
+
obj = drm_mode_object_find(dev, page_flip->crtc_id, DRM_MODE_OBJECT_CRTC);
if (!obj)
return -EINVAL;
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index ffd7a7b..7602177 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -303,6 +303,9 @@ int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
case DRM_CAP_TIMESTAMP_MONOTONIC:
req->value = drm_timestamp_monotonic;
break;
+ case DRM_CAP_ASYNC_PAGE_FLIP:
+ req->value = dev->mode_config.async_page_flip;
+ break;
default:
return -EINVAL;
}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 0820ab6..13d215f 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -845,6 +845,9 @@ struct drm_mode_config {

/* dumb ioctl parameters */
uint32_t preferred_depth, prefer_shadow;
+
+ /* whether async page flip is supported or not */
+ bool async_page_flip;
};

#define obj_to_crtc(x) container_of(x, struct drm_crtc, base)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 238a166..2adfaa5 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -780,6 +780,7 @@ struct drm_event_vblank {
#define DRM_CAP_DUMB_PREFER_SHADOW 0x4
#define DRM_CAP_PRIME 0x5
#define DRM_CAP_TIMESTAMP_MONOTONIC 0x6
+#define DRM_CAP_ASYNC_PAGE_FLIP 0x7

#define DRM_PRIME_CAP_IMPORT 0x1
#define DRM_PRIME_CAP_EXPORT 0x2
--
1.8.3.2

2013-07-23 02:02:21

by Keith Packard

[permalink] [raw]
Subject: [PATCH 2/5] drm: Add DRM_MODE_PAGE_FLIP_ASYNC flag definition

This requests that the driver perform the page flip as soon as
possible, not necessarily waiting for vblank.

Signed-off-by: Keith Packard <[email protected]>
---
include/uapi/drm/drm_mode.h | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 53db7ce..5508117 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -412,7 +412,8 @@ struct drm_mode_crtc_lut {
};

#define DRM_MODE_PAGE_FLIP_EVENT 0x01
-#define DRM_MODE_PAGE_FLIP_FLAGS DRM_MODE_PAGE_FLIP_EVENT
+#define DRM_MODE_PAGE_FLIP_ASYNC 0x02
+#define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)

/*
* Request a page flip on the specified crtc.
@@ -426,11 +427,14 @@ struct drm_mode_crtc_lut {
* flip is already pending as the ioctl is called, EBUSY will be
* returned.
*
- * The ioctl supports one flag, DRM_MODE_PAGE_FLIP_EVENT, which will
- * request that drm sends back a vblank event (see drm.h: struct
- * drm_event_vblank) when the page flip is done. The user_data field
- * passed in with this ioctl will be returned as the user_data field
- * in the vblank event struct.
+ * Flag DRM_MODE_PAGE_FLIP_EVENT requests that drm sends back a vblank
+ * event (see drm.h: struct drm_event_vblank) when the page flip is
+ * done. The user_data field passed in with this ioctl will be
+ * returned as the user_data field in the vblank event struct.
+ *
+ * Flag DRM_MODE_PAGE_FLIP_ASYNC requests that the flip happen
+ * 'as soon as possible', meaning that it not delay waiting for vblank.
+ * This may cause tearing on the screen.
*
* The reserved field must be zero until we figure out something
* clever to use it for.
--
1.8.3.2

2013-07-23 02:03:13

by Keith Packard

[permalink] [raw]
Subject: [PATCH 1/5] drm: Pass page flip ioctl flags to driver

This lets drivers see the flags requested by the application

Signed-off-by: Keith Packard <[email protected]>
---
drivers/gpu/drm/drm_crtc.c | 2 +-
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 5 +++--
drivers/gpu/drm/i915/i915_drv.h | 3 ++-
drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++++--------
drivers/gpu/drm/nouveau/nouveau_display.c | 3 ++-
drivers/gpu/drm/nouveau/nouveau_display.h | 3 ++-
drivers/gpu/drm/omapdrm/omap_crtc.c | 3 ++-
drivers/gpu/drm/radeon/radeon_display.c | 3 ++-
drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 3 ++-
drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 3 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 3 ++-
drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 3 ++-
include/drm/drm_crtc.h | 3 ++-
13 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index fc83bb9..989072c 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3587,7 +3587,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
}

old_fb = crtc->fb;
- ret = crtc->funcs->page_flip(crtc, fb, e);
+ ret = crtc->funcs->page_flip(crtc, fb, e, page_flip->flags);
if (ret) {
if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
spin_lock_irqsave(&dev->event_lock, flags);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 9a35d17..14f5c1d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -184,8 +184,9 @@ static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
};

static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
- struct drm_framebuffer *fb,
- struct drm_pending_vblank_event *event)
+ struct drm_framebuffer *fb,
+ struct drm_pending_vblank_event *event,
+ uint32_t page_flip_flags)
{
struct drm_device *dev = crtc->dev;
struct exynos_drm_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cef35d3..c7cb2de 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -380,7 +380,8 @@ struct drm_i915_display_funcs {
void (*init_clock_gating)(struct drm_device *dev);
int (*queue_flip)(struct drm_device *dev, struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_i915_gem_object *obj);
+ struct drm_i915_gem_object *obj,
+ uint32_t flags);
int (*update_plane)(struct drm_crtc *crtc, struct drm_framebuffer *fb,
int x, int y);
void (*hpd_irq_setup)(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ae3dc5d..bdb8854 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7320,7 +7320,8 @@ inline static void intel_mark_page_flip_active(struct intel_crtc *intel_crtc)
static int intel_gen2_queue_flip(struct drm_device *dev,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_i915_gem_object *obj)
+ struct drm_i915_gem_object *obj,
+ uint32_t flags)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -7364,7 +7365,8 @@ err:
static int intel_gen3_queue_flip(struct drm_device *dev,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_i915_gem_object *obj)
+ struct drm_i915_gem_object *obj,
+ uint32_t flags)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -7405,7 +7407,8 @@ err:
static int intel_gen4_queue_flip(struct drm_device *dev,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_i915_gem_object *obj)
+ struct drm_i915_gem_object *obj,
+ uint32_t flags)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -7453,7 +7456,8 @@ err:
static int intel_gen6_queue_flip(struct drm_device *dev,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_i915_gem_object *obj)
+ struct drm_i915_gem_object *obj,
+ uint32_t flags)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -7503,7 +7507,8 @@ err:
static int intel_gen7_queue_flip(struct drm_device *dev,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_i915_gem_object *obj)
+ struct drm_i915_gem_object *obj,
+ uint32_t flags)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -7553,14 +7558,16 @@ err:
static int intel_default_queue_flip(struct drm_device *dev,
struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_i915_gem_object *obj)
+ struct drm_i915_gem_object *obj,
+ uint32_t flags)
{
return -ENODEV;
}

static int intel_crtc_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_pending_vblank_event *event)
+ struct drm_pending_vblank_event *event,
+ uint32_t page_flip_flags)
{
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7630,7 +7637,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
atomic_inc(&intel_crtc->unpin_work_count);
intel_crtc->reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);

- ret = dev_priv->display.queue_flip(dev, crtc, fb, obj);
+ ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, page_flip_flags);
if (ret)
goto cleanup_pending;

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 708b2d1..fdb43c5 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -515,7 +515,8 @@ fail:

int
nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
- struct drm_pending_vblank_event *event)
+ struct drm_pending_vblank_event *event,
+ uint32_t page_flip_flags)
{
struct drm_device *dev = crtc->dev;
struct nouveau_drm *drm = nouveau_drm(dev);
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
index 1ea3e47..12bcffe 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -60,7 +60,8 @@ int nouveau_display_suspend(struct drm_device *dev);
void nouveau_display_resume(struct drm_device *dev);

int nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
- struct drm_pending_vblank_event *event);
+ struct drm_pending_vblank_event *event,
+ uint32_t page_flip_flags);
int nouveau_finish_page_flip(struct nouveau_channel *,
struct nouveau_page_flip_state *);

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 11a5263..0fd2eb1 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -331,7 +331,8 @@ static void page_flip_cb(void *arg)

static int omap_crtc_page_flip_locked(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_pending_vblank_event *event)
+ struct drm_pending_vblank_event *event,
+ uint32_t page_flip_flags)
{
struct drm_device *dev = crtc->dev;
struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index c2b67b4..358bd96 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -345,7 +345,8 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id)

static int radeon_crtc_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_pending_vblank_event *event)
+ struct drm_pending_vblank_event *event,
+ uint32_t page_flip_flags)
{
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
index 99e2034..54bad98 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c
@@ -465,7 +465,8 @@ void shmob_drm_crtc_finish_page_flip(struct shmob_drm_crtc *scrtc)

static int shmob_drm_crtc_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_pending_vblank_event *event)
+ struct drm_pending_vblank_event *event,
+ uint32_t page_flip_flags)
{
struct shmob_drm_crtc *scrtc = to_shmob_crtc(crtc);
struct drm_device *dev = scrtc->crtc.dev;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 7418dcd..8615773 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -156,7 +156,8 @@ static void tilcdc_crtc_destroy(struct drm_crtc *crtc)

static int tilcdc_crtc_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_pending_vblank_event *event)
+ struct drm_pending_vblank_event *event,
+ uint32_t page_flip_flags)
{
struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
struct drm_device *dev = crtc->dev;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index d4607b2..fc43c06 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1706,7 +1706,8 @@ int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,

int vmw_du_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_pending_vblank_event *event)
+ struct drm_pending_vblank_event *event,
+ uint32_t page_flip_flags)
{
struct vmw_private *dev_priv = vmw_priv(crtc->dev);
struct drm_framebuffer *old_fb = crtc->fb;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 6fa89c9..8d038c3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -123,7 +123,8 @@ struct vmw_display_unit {
void vmw_display_unit_cleanup(struct vmw_display_unit *du);
int vmw_du_page_flip(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_pending_vblank_event *event);
+ struct drm_pending_vblank_event *event,
+ uint32_t page_flip_flags);
void vmw_du_crtc_save(struct drm_crtc *crtc);
void vmw_du_crtc_restore(struct drm_crtc *crtc);
void vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index fa12a2f..0820ab6 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -363,7 +363,8 @@ struct drm_crtc_funcs {
*/
int (*page_flip)(struct drm_crtc *crtc,
struct drm_framebuffer *fb,
- struct drm_pending_vblank_event *event);
+ struct drm_pending_vblank_event *event,
+ uint32_t flags);

int (*set_property)(struct drm_crtc *crtc,
struct drm_property *property, uint64_t val);
--
1.8.3.2

2013-07-23 02:03:31

by Keith Packard

[permalink] [raw]
Subject: [PATCH 4/5] drm/i915: Add async page flip support for IVB

This adds the necesary register defines for async page flipping
through the command ring, and then hooks those up for Ivybridge (gen7)
page flipping.

Signed-off-by: Keith Packard <[email protected]>
---
drivers/gpu/drm/i915/i915_reg.h | 6 ++++++
drivers/gpu/drm/i915/intel_display.c | 40 ++++++++++++++++++++++++++++++++++--
2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc3d6a7..029cfb0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -209,6 +209,7 @@
#define MI_LOAD_SCAN_LINES_INCL MI_INSTR(0x12, 0)
#define MI_DISPLAY_FLIP MI_INSTR(0x14, 2)
#define MI_DISPLAY_FLIP_I915 MI_INSTR(0x14, 1)
+#define MI_DISPLAY_FLIP_ASYNC_INDICATOR (1 << 22)
#define MI_DISPLAY_FLIP_PLANE(n) ((n) << 20)
/* IVB has funny definitions for which plane to flip. */
#define MI_DISPLAY_FLIP_IVB_PLANE_A (0 << 19)
@@ -217,6 +218,11 @@
#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)
+/* These go in the bottom of the base address value */
+#define MI_DISPLAY_FLIP_TYPE_SYNC (0 << 0)
+#define MI_DISPLAY_FLIP_TYPE_ASYNC (1 << 0)
+#define MI_DISPLAY_FLIP_TYPE_STEREO (2 << 0)
+#define MI_DISPLAY_FLIP_TYPE_SYNCHRONOUS (0 << 0)
#define MI_ARB_ON_OFF MI_INSTR(0x08, 0)
#define MI_ARB_ENABLE (1<<0)
#define MI_ARB_DISABLE (0<<0)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bdb8854..1bcc6b4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7514,6 +7514,8 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_ring_buffer *ring = &dev_priv->ring[BCS];
uint32_t plane_bit = 0;
+ uint32_t cmd;
+ uint32_t base;
int ret;

ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
@@ -7536,13 +7538,43 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
goto err_unpin;
}

+ cmd = MI_DISPLAY_FLIP_I915 | plane_bit;
+ base = i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset;
+
+ if (flags & DRM_MODE_PAGE_FLIP_ASYNC) {
+
+ /* XXX check limitations for async flip here */
+
+ if (fb->pitches[0] != I915_READ(DSPSTRIDE(intel_crtc->plane))) {
+ WARN_ONCE(1, "mismatching stride in async plane flip (%d != %d)\n",
+ fb->pitches[0], I915_READ(DSPSTRIDE(intel_crtc->plane)));
+ ret = -EINVAL;
+ goto err_unpin;
+ }
+
+ if (obj->tiling_mode != I915_TILING_X) {
+ WARN_ONCE(1, "async plane flip requires X tiling\n");
+ ret = -EINVAL;
+ goto err_unpin;
+ }
+
+ if ((I915_READ(DSPCNTR(intel_crtc->plane)) & DISPPLANE_TILED) == 0) {
+ WARN_ONCE(1, "display not currently tiled in async plane flip\n");
+ ret = -EINVAL;
+ goto err_unpin;
+ }
+
+ cmd |= MI_DISPLAY_FLIP_ASYNC_INDICATOR;
+ base |= MI_DISPLAY_FLIP_TYPE_ASYNC;
+ }
+
ret = intel_ring_begin(ring, 4);
if (ret)
goto err_unpin;

- intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit);
+ intel_ring_emit(ring, cmd);
intel_ring_emit(ring, (fb->pitches[0] | obj->tiling_mode));
- intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
+ intel_ring_emit(ring, base);
intel_ring_emit(ring, (MI_NOOP));

intel_mark_page_flip_active(intel_crtc);
@@ -9705,6 +9737,10 @@ void intel_modeset_init(struct drm_device *dev)
dev->mode_config.max_width = 8192;
dev->mode_config.max_height = 8192;
}
+
+ if (IS_GEN7(dev))
+ dev->mode_config.async_page_flip = true;
+
dev->mode_config.fb_base = dev_priv->gtt.mappable_base;

DRM_DEBUG_KMS("%d display pipe%s available.\n",
--
1.8.3.2

2013-07-23 05:29:04

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH 4/5] drm/i915: Add async page flip support for IVB

On Mon, Jul 22, 2013 at 06:50:01PM -0700, Keith Packard wrote:
> This adds the necesary register defines for async page flipping
> through the command ring, and then hooks those up for Ivybridge (gen7)
> page flipping.
>
> Signed-off-by: Keith Packard <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 6 ++++++
> drivers/gpu/drm/i915/intel_display.c | 40 ++++++++++++++++++++++++++++++++++--
> 2 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index dc3d6a7..029cfb0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -209,6 +209,7 @@
> #define MI_LOAD_SCAN_LINES_INCL MI_INSTR(0x12, 0)
> #define MI_DISPLAY_FLIP MI_INSTR(0x14, 2)
> #define MI_DISPLAY_FLIP_I915 MI_INSTR(0x14, 1)
> +#define MI_DISPLAY_FLIP_ASYNC_INDICATOR (1 << 22)
> #define MI_DISPLAY_FLIP_PLANE(n) ((n) << 20)
> /* IVB has funny definitions for which plane to flip. */
> #define MI_DISPLAY_FLIP_IVB_PLANE_A (0 << 19)
> @@ -217,6 +218,11 @@
> #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)
> +/* These go in the bottom of the base address value */
> +#define MI_DISPLAY_FLIP_TYPE_SYNC (0 << 0)
> +#define MI_DISPLAY_FLIP_TYPE_ASYNC (1 << 0)
> +#define MI_DISPLAY_FLIP_TYPE_STEREO (2 << 0)
> +#define MI_DISPLAY_FLIP_TYPE_SYNCHRONOUS (0 << 0)
> #define MI_ARB_ON_OFF MI_INSTR(0x08, 0)
> #define MI_ARB_ENABLE (1<<0)
> #define MI_ARB_DISABLE (0<<0)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bdb8854..1bcc6b4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7514,6 +7514,8 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_ring_buffer *ring = &dev_priv->ring[BCS];
> uint32_t plane_bit = 0;
> + uint32_t cmd;
> + uint32_t base;
> int ret;
>
> ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
> @@ -7536,13 +7538,43 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
> goto err_unpin;
> }
>
> + cmd = MI_DISPLAY_FLIP_I915 | plane_bit;
> + base = i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset;
> +
> + if (flags & DRM_MODE_PAGE_FLIP_ASYNC) {
> +
> + /* XXX check limitations for async flip here */
> +
> + if (fb->pitches[0] != I915_READ(DSPSTRIDE(intel_crtc->plane))) {
> + WARN_ONCE(1, "mismatching stride in async plane flip (%d != %d)\n",
> + fb->pitches[0], I915_READ(DSPSTRIDE(intel_crtc->plane)));
> + ret = -EINVAL;
> + goto err_unpin;
> + }
> +
> + if (obj->tiling_mode != I915_TILING_X) {
> + WARN_ONCE(1, "async plane flip requires X tiling\n");
> + ret = -EINVAL;
> + goto err_unpin;
> + }

Matching tiling modes is actually already a requirement on gen4+ (since
the tiling bit and the linear/tiled offset registers can't be changed with
a MI_DISPLAY_FLIP command). But atm we fail to check that in the common
code, so imo better to move that to there.

We already check for matching strides in common code, so with the tile
check added we could drop this all here.
-Daniel

> +
> + if ((I915_READ(DSPCNTR(intel_crtc->plane)) & DISPPLANE_TILED) == 0) {
> + WARN_ONCE(1, "display not currently tiled in async plane flip\n");
> + ret = -EINVAL;
> + goto err_unpin;
> + }
> +
> + cmd |= MI_DISPLAY_FLIP_ASYNC_INDICATOR;
> + base |= MI_DISPLAY_FLIP_TYPE_ASYNC;
> + }
> +
> ret = intel_ring_begin(ring, 4);
> if (ret)
> goto err_unpin;
>
> - intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit);
> + intel_ring_emit(ring, cmd);
> intel_ring_emit(ring, (fb->pitches[0] | obj->tiling_mode));
> - intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
> + intel_ring_emit(ring, base);
> intel_ring_emit(ring, (MI_NOOP));
>
> intel_mark_page_flip_active(intel_crtc);
> @@ -9705,6 +9737,10 @@ void intel_modeset_init(struct drm_device *dev)
> dev->mode_config.max_width = 8192;
> dev->mode_config.max_height = 8192;
> }
> +
> + if (IS_GEN7(dev))
> + dev->mode_config.async_page_flip = true;
> +
> dev->mode_config.fb_base = dev_priv->gtt.mappable_base;
>
> DRM_DEBUG_KMS("%d display pipe%s available.\n",
> --
> 1.8.3.2
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

2013-07-23 20:16:05

by Daniel Vetter

[permalink] [raw]
Subject: Re: drm: Asynchronouse page flipping interface and Intel implementation

On Mon, Jul 22, 2013 at 06:49:57PM -0700, Keith Packard wrote:
> Here's a sequence of five patches that exposes an interface to request
> of the driver that the page flipping request be executed without
> waiting for vblank. It's optional, and drivers can expose whether it
> is supported through the existing GETCAP ioctl.
>
> This supports only Ivybridge and Sandybridge Intel graphics chips as
> that's what I've got to test on; of course all of the other drivers
> have been modified so that they still compile cleanly.

Quick comments on the i915 kernel part:
- Iirc the w/a database has a bunch of entries about async flips. Those
need to be addressed and annoted with the new w/a tag comment format
Damien recently created.

- We should exercise userspace abi cornercases and check that we get an
-EINVAL everywhere we expect one. Since the current vblank-synced
pageflips seem to also get this wrong it'd be good to exercise both
modes.

- kms_flip needs to be extended to beat on async flips. Obviously the
timing checks don't apply but otherwise I think it won't hurt to enable
all the existing interaction tests with async flips, too. On top of that
I think we should test interactions between async flips and vblank
synced flips. Without the timing checks the other tests will run as fast
as possible, so that should gives us good coverage of the
non-ratelimited nature of async flips - even with vblank flips we've hit
ugly issues by e.g. starving the unpin workers.

Oh and my little comment about moving all checks into core code is a bit
wrong, we'd still need to check for X-tiling with async flips ofc.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

2013-07-24 21:23:12

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH 4/5] drm/i915: Add async page flip support for IVB

On Wed, Jul 24, 2013 at 01:26:32PM -0700, Keith Packard wrote:
> Daniel Vetter <[email protected]> writes:
>
> > Matching tiling modes is actually already a requirement on gen4+ (since
> > the tiling bit and the linear/tiled offset registers can't be changed with
> > a MI_DISPLAY_FLIP command).
>
> Async flip has a harder requirement -- you must use X tiling, both
> before and after the flip. Oh, and async flips require a 32KB aligned
> buffer, which I'm not actually checking for. Not sure how to

We could just unconditionally increase the alignement in
intel_pin_and_fence_fb_obj - we already have more strict requirements due
to a bunch of w/a in other places. So shouldn't hurt at all really.

> > But atm we fail to check that in the common
> > code, so imo better to move that to there.
> >
> > We already check for matching strides in common code, so with the tile
> > check added we could drop this all here.
>
> I don't see a requirement for matching stride and tile parameter in the
> MI_DISPLAY_FLIP docs for synchronous operations, at least on DevGT+. Is
> the common code too restrictive?

Hm right, I've mixed that. I guess we could be a bit more sloppy with
this, otoh no one yet seems to want it and Ville's pageflip stuff is
rather madly flexible.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

2013-07-25 07:48:05

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH 4/5] drm/i915: Add async page flip support for IVB

On Wed, Jul 24, 2013 at 06:40:16PM -0700, Keith Packard wrote:
> Daniel Vetter <[email protected]> writes:
>
> > We could just unconditionally increase the alignement in
> > intel_pin_and_fence_fb_obj - we already have more strict requirements due
> > to a bunch of w/a in other places. So shouldn't hurt at all really.
>
> That seems like a fine plan; 32kB isn't that onerous. Do you want the
> trivial patch to do this from me then?

Yes please, merging patches from other people is much easier than begging for
review for my own ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

2013-07-25 19:18:45

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH 4/5] drm/i915: Add async page flip support for IVB

On Thu, Jul 25, 2013 at 8:13 PM, Keith Packard <[email protected]> wrote:
> Daniel Vetter <[email protected]> writes:
>
>> On Wed, Jul 24, 2013 at 06:40:16PM -0700, Keith Packard wrote:
>>> Daniel Vetter <[email protected]> writes:
>>>
>>> > We could just unconditionally increase the alignement in
>>> > intel_pin_and_fence_fb_obj - we already have more strict requirements due
>>> > to a bunch of w/a in other places. So shouldn't hurt at all really.
>>>
>>> That seems like a fine plan; 32kB isn't that onerous. Do you want the
>>> trivial patch to do this from me then?
>>
>> Yes please, merging patches from other people is much easier than begging for
>> review for my own ;-)
>> -Daniel
>
> Here's a replacement for patch #4 that just adds the alignment
> requirement there. Do you want any other changes in this series?

Generally I think checking our current sw state instead of reading hw
registers would be safer, e.g. in case we start to queue up more than
one pageflip. For async pageflips in benchmark mode "flip as fast as
you can queue" would be a sensible mode.
-Daniel

>
> From 9a51e7118fce58c835cabb192f6b6e0a4a5f6660 Mon Sep 17 00:00:00 2001
> From: Keith Packard <[email protected]>
> Date: Mon, 22 Jul 2013 18:12:28 -0700
> Subject: [PATCH 4/5] drm/i915: Add async page flip support for IVB
>
> This adds the necesary register defines for async page flipping
> through the command ring, and then hooks those up for Ivybridge (gen7)
> page flipping.
>
> Signed-off-by: Keith Packard <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 6 +++++
> drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++++++++++++----
> 2 files changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index dc3d6a7..029cfb0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -209,6 +209,7 @@
> #define MI_LOAD_SCAN_LINES_INCL MI_INSTR(0x12, 0)
> #define MI_DISPLAY_FLIP MI_INSTR(0x14, 2)
> #define MI_DISPLAY_FLIP_I915 MI_INSTR(0x14, 1)
> +#define MI_DISPLAY_FLIP_ASYNC_INDICATOR (1 << 22)
> #define MI_DISPLAY_FLIP_PLANE(n) ((n) << 20)
> /* IVB has funny definitions for which plane to flip. */
> #define MI_DISPLAY_FLIP_IVB_PLANE_A (0 << 19)
> @@ -217,6 +218,11 @@
> #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)
> +/* These go in the bottom of the base address value */
> +#define MI_DISPLAY_FLIP_TYPE_SYNC (0 << 0)
> +#define MI_DISPLAY_FLIP_TYPE_ASYNC (1 << 0)
> +#define MI_DISPLAY_FLIP_TYPE_STEREO (2 << 0)
> +#define MI_DISPLAY_FLIP_TYPE_SYNCHRONOUS (0 << 0)
> #define MI_ARB_ON_OFF MI_INSTR(0x08, 0)
> #define MI_ARB_ENABLE (1<<0)
> #define MI_ARB_DISABLE (0<<0)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bdb8854..f2624a4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1833,8 +1833,10 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
> alignment = 64 * 1024;
> break;
> case I915_TILING_X:
> - /* pin() will align the object as required by fence */
> - alignment = 0;
> + /* Async page flipping requires X tiling and 32kB alignment, so just
> + * make all X tiled frame buffers aligned for that
> + */
> + alignment = 32 * 1024;
> break;
> case I915_TILING_Y:
> /* Despite that we check this in framebuffer_init userspace can
> @@ -7514,6 +7516,8 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_ring_buffer *ring = &dev_priv->ring[BCS];
> uint32_t plane_bit = 0;
> + uint32_t cmd;
> + uint32_t base;
> int ret;
>
> ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
> @@ -7536,13 +7540,43 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
> goto err_unpin;
> }
>
> + cmd = MI_DISPLAY_FLIP_I915 | plane_bit;
> + base = i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset;
> +
> + if (flags & DRM_MODE_PAGE_FLIP_ASYNC) {
> +
> + /* XXX check limitations for async flip here */
> +
> + if (fb->pitches[0] != I915_READ(DSPSTRIDE(intel_crtc->plane))) {
> + WARN_ONCE(1, "mismatching stride in async plane flip (%d != %d)\n",
> + fb->pitches[0], I915_READ(DSPSTRIDE(intel_crtc->plane)));
> + ret = -EINVAL;
> + goto err_unpin;
> + }
> +
> + if (obj->tiling_mode != I915_TILING_X) {
> + WARN_ONCE(1, "async plane flip requires X tiling\n");
> + ret = -EINVAL;
> + goto err_unpin;
> + }
> +
> + if ((I915_READ(DSPCNTR(intel_crtc->plane)) & DISPPLANE_TILED) == 0) {
> + WARN_ONCE(1, "display not currently tiled in async plane flip\n");
> + ret = -EINVAL;
> + goto err_unpin;
> + }
> +
> + cmd |= MI_DISPLAY_FLIP_ASYNC_INDICATOR;
> + base |= MI_DISPLAY_FLIP_TYPE_ASYNC;
> + }
> +
> ret = intel_ring_begin(ring, 4);
> if (ret)
> goto err_unpin;
>
> - intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit);
> + intel_ring_emit(ring, cmd);
> intel_ring_emit(ring, (fb->pitches[0] | obj->tiling_mode));
> - intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
> + intel_ring_emit(ring, base);
> intel_ring_emit(ring, (MI_NOOP));
>
> intel_mark_page_flip_active(intel_crtc);
> @@ -9705,6 +9739,10 @@ void intel_modeset_init(struct drm_device *dev)
> dev->mode_config.max_width = 8192;
> dev->mode_config.max_height = 8192;
> }
> +
> + if (IS_GEN7(dev))
> + dev->mode_config.async_page_flip = true;
> +
> dev->mode_config.fb_base = dev_priv->gtt.mappable_base;
>
> DRM_DEBUG_KMS("%d display pipe%s available.\n",
> --
> 1.8.3.2
>
>
>
> --
> [email protected]



--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

2013-07-25 22:15:31

by Keith Packard

[permalink] [raw]
Subject: Re: [PATCH 4/5] drm/i915: Add async page flip support for IVB

> Generally I think checking our current sw state instead of reading hw
> registers would be safer, e.g. in case we start to queue up more than
> one pageflip. For async pageflips in benchmark mode "flip as fast as
> you can queue" would be a sensible mode.

Ok, I've moved the tiling checks to the general code and removed the
stride checks as those are already present there. These were moved to
the general code because the pointer to the previous FB has already
been overwritten by the time the queue_flip functions are called.

This should be followed by replacements for the last to patches in the
series.

-keith

2013-07-25 22:15:35

by Keith Packard

[permalink] [raw]
Subject: [PATCH 2/2] drm/i915: Add async page flip support for SNB

Just copies the IVB code

Signed-off-by: Keith Packard <[email protected]>
---
drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 166aa2c..4a118c3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7465,20 +7465,29 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_ring_buffer *ring = &dev_priv->ring[RCS];
uint32_t pf, pipesrc;
+ uint32_t cmd;
+ uint32_t base;
int ret;

ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
if (ret)
goto err;

+ cmd = MI_DISPLAY_FLIP | MI_DISPLAY_FLIP_PLANE(intel_crtc->plane);
+ base = i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset;
+
+ if (flags & DRM_MODE_PAGE_FLIP_ASYNC) {
+ cmd |= MI_DISPLAY_FLIP_ASYNC_INDICATOR;
+ base |= MI_DISPLAY_FLIP_TYPE_ASYNC;
+ }
+
ret = intel_ring_begin(ring, 4);
if (ret)
goto err_unpin;

- intel_ring_emit(ring, MI_DISPLAY_FLIP |
- MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
+ intel_ring_emit(ring, cmd);
intel_ring_emit(ring, fb->pitches[0] | obj->tiling_mode);
- intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
+ intel_ring_emit(ring, base);

/* Contrary to the suggestions in the documentation,
* "Enable Panel Fitter" does not seem to be required when page
@@ -9731,7 +9740,7 @@ void intel_modeset_init(struct drm_device *dev)
dev->mode_config.max_height = 8192;
}

- if (IS_GEN7(dev))
+ if (IS_GEN6(dev) || IS_GEN7(dev))
dev->mode_config.async_page_flip = true;

dev->mode_config.fb_base = dev_priv->gtt.mappable_base;
--
1.8.3.2

2013-07-25 22:15:37

by Keith Packard

[permalink] [raw]
Subject: [PATCH 1/2] drm/i915: Add async page flip support for IVB

This adds the necesary register defines for async page flipping
through the command ring, and then hooks those up for Ivybridge (gen7)
page flipping.

Signed-off-by: Keith Packard <[email protected]>
---
drivers/gpu/drm/i915/i915_reg.h | 6 ++++++
drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++----
2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc3d6a7..029cfb0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -209,6 +209,7 @@
#define MI_LOAD_SCAN_LINES_INCL MI_INSTR(0x12, 0)
#define MI_DISPLAY_FLIP MI_INSTR(0x14, 2)
#define MI_DISPLAY_FLIP_I915 MI_INSTR(0x14, 1)
+#define MI_DISPLAY_FLIP_ASYNC_INDICATOR (1 << 22)
#define MI_DISPLAY_FLIP_PLANE(n) ((n) << 20)
/* IVB has funny definitions for which plane to flip. */
#define MI_DISPLAY_FLIP_IVB_PLANE_A (0 << 19)
@@ -217,6 +218,11 @@
#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)
+/* These go in the bottom of the base address value */
+#define MI_DISPLAY_FLIP_TYPE_SYNC (0 << 0)
+#define MI_DISPLAY_FLIP_TYPE_ASYNC (1 << 0)
+#define MI_DISPLAY_FLIP_TYPE_STEREO (2 << 0)
+#define MI_DISPLAY_FLIP_TYPE_SYNCHRONOUS (0 << 0)
#define MI_ARB_ON_OFF MI_INSTR(0x08, 0)
#define MI_ARB_ENABLE (1<<0)
#define MI_ARB_DISABLE (0<<0)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bdb8854..166aa2c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1833,8 +1833,10 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
alignment = 64 * 1024;
break;
case I915_TILING_X:
- /* pin() will align the object as required by fence */
- alignment = 0;
+ /* Async page flipping requires X tiling and 32kB alignment, so just
+ * make all X tiled frame buffers aligned for that
+ */
+ alignment = 32 * 1024;
break;
case I915_TILING_Y:
/* Despite that we check this in framebuffer_init userspace can
@@ -7514,6 +7516,8 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_ring_buffer *ring = &dev_priv->ring[BCS];
uint32_t plane_bit = 0;
+ uint32_t cmd;
+ uint32_t base;
int ret;

ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
@@ -7536,13 +7540,21 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
goto err_unpin;
}

+ cmd = MI_DISPLAY_FLIP_I915 | plane_bit;
+ base = i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset;
+
+ if (flags & DRM_MODE_PAGE_FLIP_ASYNC) {
+ cmd |= MI_DISPLAY_FLIP_ASYNC_INDICATOR;
+ base |= MI_DISPLAY_FLIP_TYPE_ASYNC;
+ }
+
ret = intel_ring_begin(ring, 4);
if (ret)
goto err_unpin;

- intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit);
+ intel_ring_emit(ring, cmd);
intel_ring_emit(ring, (fb->pitches[0] | obj->tiling_mode));
- intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
+ intel_ring_emit(ring, base);
intel_ring_emit(ring, (MI_NOOP));

intel_mark_page_flip_active(intel_crtc);
@@ -7591,6 +7603,19 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
fb->pitches[0] != crtc->fb->pitches[0]))
return -EINVAL;

+ /* Check tiling restrictions specific to asynchronous flips
+ */
+ if (page_flip_flags & DRM_MODE_PAGE_FLIP_ASYNC) {
+
+ /* New FB must be X tiled */
+ if (obj->tiling_mode != I915_TILING_X)
+ return -EINVAL;
+
+ /* Current FB must be X tiled */
+ if (to_intel_framebuffer(old_fb)->obj->tiling_mode != I915_TILING_X)
+ return -EINVAL;
+ }
+
work = kzalloc(sizeof *work, GFP_KERNEL);
if (work == NULL)
return -ENOMEM;
@@ -9705,6 +9730,10 @@ void intel_modeset_init(struct drm_device *dev)
dev->mode_config.max_width = 8192;
dev->mode_config.max_height = 8192;
}
+
+ if (IS_GEN7(dev))
+ dev->mode_config.async_page_flip = true;
+
dev->mode_config.fb_base = dev_priv->gtt.mappable_base;

DRM_DEBUG_KMS("%d display pipe%s available.\n",
--
1.8.3.2

2013-07-30 16:28:50

by Ville Syrjälä

[permalink] [raw]
Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add async page flip support for SNB

On Thu, Jul 25, 2013 at 03:15:15PM -0700, Keith Packard wrote:
> Just copies the IVB code
>
> Signed-off-by: Keith Packard <[email protected]>
> ---
> drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 166aa2c..4a118c3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7465,20 +7465,29 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_ring_buffer *ring = &dev_priv->ring[RCS];
> uint32_t pf, pipesrc;
> + uint32_t cmd;
> + uint32_t base;
> int ret;
>
> ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
> if (ret)
> goto err;
>
> + cmd = MI_DISPLAY_FLIP | MI_DISPLAY_FLIP_PLANE(intel_crtc->plane);
> + base = i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset;
> +
> + if (flags & DRM_MODE_PAGE_FLIP_ASYNC) {
> + cmd |= MI_DISPLAY_FLIP_ASYNC_INDICATOR;
> + base |= MI_DISPLAY_FLIP_TYPE_ASYNC;
> + }
> +
> ret = intel_ring_begin(ring, 4);
> if (ret)
> goto err_unpin;
>
> - intel_ring_emit(ring, MI_DISPLAY_FLIP |
> - MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
> + intel_ring_emit(ring, cmd);
> intel_ring_emit(ring, fb->pitches[0] | obj->tiling_mode);
> - intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
> + intel_ring_emit(ring, base);
>
> /* Contrary to the suggestions in the documentation,
> * "Enable Panel Fitter" does not seem to be required when page

This PF flip stuff is a bit of a mystery. I'm not sure it exists on SNB
anymore. Some of the docs say that it's MBZ for SNB/IVB. Gen4/5 docs
say that DW3 must not be sent w/ async flips, and some SNB+ docs say
that it must not be sent with either sync or async flips.

Did you test this patch on actual hardware, and if so did it work as
expected? :)

I guess one would need to perform some empirical testing to figure out
what DW3 actually does.

> @@ -9731,7 +9740,7 @@ void intel_modeset_init(struct drm_device *dev)
> dev->mode_config.max_height = 8192;
> }
>
> - if (IS_GEN7(dev))
> + if (IS_GEN6(dev) || IS_GEN7(dev))
> dev->mode_config.async_page_flip = true;
>
> dev->mode_config.fb_base = dev_priv->gtt.mappable_base;
> --
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrj?l?
Intel OTC

2013-07-30 16:42:39

by Ville Syrjälä

[permalink] [raw]
Subject: Re: [PATCH 1/2] drm/i915: Add async page flip support for IVB

On Thu, Jul 25, 2013 at 03:15:14PM -0700, Keith Packard wrote:
> This adds the necesary register defines for async page flipping
> through the command ring, and then hooks those up for Ivybridge (gen7)
> page flipping.

Maybe mention hsw in the patch subject/description too.

>
> Signed-off-by: Keith Packard <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 6 ++++++
> drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++++++++++++++++++++----
> 2 files changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index dc3d6a7..029cfb0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -209,6 +209,7 @@
> #define MI_LOAD_SCAN_LINES_INCL MI_INSTR(0x12, 0)
> #define MI_DISPLAY_FLIP MI_INSTR(0x14, 2)
> #define MI_DISPLAY_FLIP_I915 MI_INSTR(0x14, 1)
> +#define MI_DISPLAY_FLIP_ASYNC_INDICATOR (1 << 22)
> #define MI_DISPLAY_FLIP_PLANE(n) ((n) << 20)
> /* IVB has funny definitions for which plane to flip. */
> #define MI_DISPLAY_FLIP_IVB_PLANE_A (0 << 19)
> @@ -217,6 +218,11 @@
> #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)
> +/* These go in the bottom of the base address value */
> +#define MI_DISPLAY_FLIP_TYPE_SYNC (0 << 0)
> +#define MI_DISPLAY_FLIP_TYPE_ASYNC (1 << 0)
> +#define MI_DISPLAY_FLIP_TYPE_STEREO (2 << 0)
> +#define MI_DISPLAY_FLIP_TYPE_SYNCHRONOUS (0 << 0)

Duplicated bit.

> #define MI_ARB_ON_OFF MI_INSTR(0x08, 0)
> #define MI_ARB_ENABLE (1<<0)
> #define MI_ARB_DISABLE (0<<0)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bdb8854..166aa2c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1833,8 +1833,10 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
> alignment = 64 * 1024;
> break;
> case I915_TILING_X:
> - /* pin() will align the object as required by fence */
> - alignment = 0;
> + /* Async page flipping requires X tiling and 32kB alignment, so just
> + * make all X tiled frame buffers aligned for that
> + */
> + alignment = 32 * 1024;

You could limit this to gens for which you implemented async flips.

gen2/3 seem to require a 256KB alignment for async flips.

> break;
> case I915_TILING_Y:
> /* Despite that we check this in framebuffer_init userspace can
> @@ -7514,6 +7516,8 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_ring_buffer *ring = &dev_priv->ring[BCS];
> uint32_t plane_bit = 0;
> + uint32_t cmd;
> + uint32_t base;
> int ret;
>
> ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
> @@ -7536,13 +7540,21 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
> goto err_unpin;
> }
>
> + cmd = MI_DISPLAY_FLIP_I915 | plane_bit;
> + base = i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset;
> +
> + if (flags & DRM_MODE_PAGE_FLIP_ASYNC) {
> + cmd |= MI_DISPLAY_FLIP_ASYNC_INDICATOR;
> + base |= MI_DISPLAY_FLIP_TYPE_ASYNC;
> + }
> +
> ret = intel_ring_begin(ring, 4);
> if (ret)
> goto err_unpin;
>
> - intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit);
> + intel_ring_emit(ring, cmd);
> intel_ring_emit(ring, (fb->pitches[0] | obj->tiling_mode));
> - intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset);
> + intel_ring_emit(ring, base);
> intel_ring_emit(ring, (MI_NOOP));
>
> intel_mark_page_flip_active(intel_crtc);
> @@ -7591,6 +7603,19 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
> fb->pitches[0] != crtc->fb->pitches[0]))
> return -EINVAL;
>
> + /* Check tiling restrictions specific to asynchronous flips
> + */
> + if (page_flip_flags & DRM_MODE_PAGE_FLIP_ASYNC) {
> +
> + /* New FB must be X tiled */
> + if (obj->tiling_mode != I915_TILING_X)
> + return -EINVAL;
> +
> + /* Current FB must be X tiled */
> + if (to_intel_framebuffer(old_fb)->obj->tiling_mode != I915_TILING_X)
> + return -EINVAL;
> + }
> +
> work = kzalloc(sizeof *work, GFP_KERNEL);
> if (work == NULL)
> return -ENOMEM;
> @@ -9705,6 +9730,10 @@ void intel_modeset_init(struct drm_device *dev)
> dev->mode_config.max_width = 8192;
> dev->mode_config.max_height = 8192;
> }
> +
> + if (IS_GEN7(dev))
> + dev->mode_config.async_page_flip = true;
> +
> dev->mode_config.fb_base = dev_priv->gtt.mappable_base;
>
> DRM_DEBUG_KMS("%d display pipe%s available.\n",
> --
> 1.8.3.2
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

--
Ville Syrj?l?
Intel OTC