* [PATCH 01/11] drm/i915: Move FBC stuff to intel_fbc.c
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
@ 2014-12-08 16:09 ` Paulo Zanoni
2014-12-08 16:09 ` [PATCH 02/11] drm/i915: Introduce FBC DocBook Paulo Zanoni
` (10 subsequent siblings)
11 siblings, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
No functional changes. This is just the begin of a FBC rework.
v2 (Paulo):
- Revert intel_fbc_init() changed parameter.
- Revert set_no_fbc_reason() rename.
- Rebase.
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.h | 3 -
drivers/gpu/drm/i915/i915_suspend.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 22 +-
drivers/gpu/drm/i915/intel_drv.h | 9 +-
drivers/gpu/drm/i915/intel_fbc.c | 675 +++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/intel_pm.c | 651 +--------------------------------
drivers/gpu/drm/i915/intel_sprite.c | 4 +-
8 files changed, 698 insertions(+), 669 deletions(-)
create mode 100644 drivers/gpu/drm/i915/intel_fbc.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index e4083e4..3cf70a6 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -47,6 +47,7 @@ i915-y += intel_renderstate_gen6.o \
i915-y += intel_audio.o \
intel_bios.o \
intel_display.o \
+ intel_fbc.o \
intel_fifo_underrun.o \
intel_frontbuffer.o \
intel_modes.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 11e85cb..b5260bf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2963,9 +2963,6 @@ extern void intel_modeset_setup_hw_state(struct drm_device *dev,
bool force_restore);
extern void i915_redisable_vga(struct drm_device *dev);
extern void i915_redisable_vga_power_on(struct drm_device *dev);
-extern bool intel_fbc_enabled(struct drm_device *dev);
-extern void bdw_fbc_sw_flush(struct drm_device *dev, u32 value);
-extern void intel_disable_fbc(struct drm_device *dev);
extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
extern void intel_init_pch_refclk(struct drm_device *dev);
extern void gen6_set_rps(struct drm_device *dev, u8 val);
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index dfe6617..1e4999d 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -264,7 +264,7 @@ static void i915_restore_display(struct drm_device *dev)
}
/* only restore FBC info on the platform that supports FBC*/
- intel_disable_fbc(dev);
+ intel_fbc_disable(dev);
/* restore FBC interval */
if (HAS_FBC(dev) && INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev))
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a9bdc12..e4789f4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4201,7 +4201,7 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc)
hsw_enable_ips(intel_crtc);
mutex_lock(&dev->struct_mutex);
- intel_update_fbc(dev);
+ intel_fbc_update(dev);
mutex_unlock(&dev->struct_mutex);
/*
@@ -4223,7 +4223,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
intel_crtc_wait_for_pending_flips(crtc);
if (dev_priv->fbc.plane == plane)
- intel_disable_fbc(dev);
+ intel_fbc_disable(dev);
hsw_disable_ips(intel_crtc);
@@ -4526,7 +4526,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
intel_update_watermarks(crtc);
mutex_lock(&dev->struct_mutex);
- intel_update_fbc(dev);
+ intel_fbc_update(dev);
mutex_unlock(&dev->struct_mutex);
}
@@ -4581,7 +4581,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
intel_update_watermarks(crtc);
mutex_lock(&dev->struct_mutex);
- intel_update_fbc(dev);
+ intel_fbc_update(dev);
mutex_unlock(&dev->struct_mutex);
if (intel_crtc_to_shared_dpll(intel_crtc))
@@ -5186,7 +5186,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
intel_update_watermarks(crtc);
mutex_lock(&dev->struct_mutex);
- intel_update_fbc(dev);
+ intel_fbc_update(dev);
mutex_unlock(&dev->struct_mutex);
}
@@ -8947,7 +8947,7 @@ static void intel_unpin_work_fn(struct work_struct *__work)
drm_gem_object_unreference(&work->pending_flip_obj->base);
drm_gem_object_unreference(&work->old_fb_obj->base);
- intel_update_fbc(dev);
+ intel_fbc_update(dev);
if (work->flip_queued_req)
i915_gem_request_assign(&work->flip_queued_req, NULL);
@@ -9744,7 +9744,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
i915_gem_track_fb(work->old_fb_obj, obj,
INTEL_FRONTBUFFER_PRIMARY(pipe));
- intel_disable_fbc(dev);
+ intel_fbc_disable(dev);
intel_frontbuffer_flip_prepare(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));
mutex_unlock(&dev->struct_mutex);
@@ -11815,7 +11815,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
dev_priv->fbc.plane == intel_crtc->plane &&
intel_plane->rotation != BIT(DRM_ROTATE_0)) {
- intel_disable_fbc(dev);
+ intel_fbc_disable(dev);
}
if (state->visible) {
@@ -11850,7 +11850,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));
mutex_lock(&dev->struct_mutex);
- intel_update_fbc(dev);
+ intel_fbc_update(dev);
mutex_unlock(&dev->struct_mutex);
}
}
@@ -13049,7 +13049,7 @@ void intel_modeset_init(struct drm_device *dev)
intel_setup_outputs(dev);
/* Just in case the BIOS is doing something questionable. */
- intel_disable_fbc(dev);
+ intel_fbc_disable(dev);
drm_modeset_lock_all(dev);
intel_modeset_setup_hw_state(dev, false);
@@ -13566,7 +13566,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
intel_unregister_dsm_handler();
- intel_disable_fbc(dev);
+ intel_fbc_disable(dev);
ironlake_teardown_rc6(dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 61a88fa..588b618 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1063,6 +1063,13 @@ static inline void intel_fbdev_restore_mode(struct drm_device *dev)
}
#endif
+/* intel_fbc.c */
+bool intel_fbc_enabled(struct drm_device *dev);
+void intel_fbc_update(struct drm_device *dev);
+void intel_fbc_init(struct drm_i915_private *dev_priv);
+void intel_fbc_disable(struct drm_device *dev);
+void bdw_fbc_sw_flush(struct drm_device *dev, u32 value);
+
/* intel_hdmi.c */
void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
@@ -1169,8 +1176,6 @@ void intel_update_sprite_watermarks(struct drm_plane *plane,
bool enabled, bool scaled);
void intel_init_pm(struct drm_device *dev);
void intel_pm_setup(struct drm_device *dev);
-bool intel_fbc_enabled(struct drm_device *dev);
-void intel_update_fbc(struct drm_device *dev);
void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
void intel_gpu_ips_teardown(void);
void intel_init_gt_powersave(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
new file mode 100644
index 0000000..f1eeb86
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -0,0 +1,675 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#include "intel_drv.h"
+#include "i915_drv.h"
+
+/* FBC, or Frame Buffer Compression, is a technique employed to compress the
+ * framebuffer contents in-memory, aiming at reducing the required bandwidth
+ * during in-memory transfers and, therefore, reduce the power packet.
+ *
+ * The benefits of FBC are mostly visible with solid backgrounds and
+ * variation-less patterns.
+ *
+ * FBC-related functionality can be enabled by the means of the
+ * i915.i915_fbc_enable parameter
+ */
+
+static void i8xx_fbc_disable(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 fbc_ctl;
+
+ dev_priv->fbc.enabled = false;
+
+ /* Disable compression */
+ fbc_ctl = I915_READ(FBC_CONTROL);
+ if ((fbc_ctl & FBC_CTL_EN) == 0)
+ return;
+
+ fbc_ctl &= ~FBC_CTL_EN;
+ I915_WRITE(FBC_CONTROL, fbc_ctl);
+
+ /* Wait for compressing bit to clear */
+ if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10)) {
+ DRM_DEBUG_KMS("FBC idle timed out\n");
+ return;
+ }
+
+ DRM_DEBUG_KMS("disabled FBC\n");
+}
+
+static void i8xx_fbc_enable(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_framebuffer *fb = crtc->primary->fb;
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ int cfb_pitch;
+ int i;
+ u32 fbc_ctl;
+
+ dev_priv->fbc.enabled = true;
+
+ cfb_pitch = dev_priv->fbc.size / FBC_LL_SIZE;
+ if (fb->pitches[0] < cfb_pitch)
+ cfb_pitch = fb->pitches[0];
+
+ /* FBC_CTL wants 32B or 64B units */
+ if (IS_GEN2(dev))
+ cfb_pitch = (cfb_pitch / 32) - 1;
+ else
+ cfb_pitch = (cfb_pitch / 64) - 1;
+
+ /* Clear old tags */
+ for (i = 0; i < (FBC_LL_SIZE / 32) + 1; i++)
+ I915_WRITE(FBC_TAG + (i * 4), 0);
+
+ if (IS_GEN4(dev)) {
+ u32 fbc_ctl2;
+
+ /* Set it up... */
+ fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
+ fbc_ctl2 |= FBC_CTL_PLANE(intel_crtc->plane);
+ I915_WRITE(FBC_CONTROL2, fbc_ctl2);
+ I915_WRITE(FBC_FENCE_OFF, crtc->y);
+ }
+
+ /* enable it... */
+ fbc_ctl = I915_READ(FBC_CONTROL);
+ fbc_ctl &= 0x3fff << FBC_CTL_INTERVAL_SHIFT;
+ fbc_ctl |= FBC_CTL_EN | FBC_CTL_PERIODIC;
+ if (IS_I945GM(dev))
+ fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
+ fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
+ fbc_ctl |= obj->fence_reg;
+ I915_WRITE(FBC_CONTROL, fbc_ctl);
+
+ DRM_DEBUG_KMS("enabled FBC, pitch %d, yoff %d, plane %c\n",
+ cfb_pitch, crtc->y, plane_name(intel_crtc->plane));
+}
+
+static bool i8xx_fbc_enabled(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ return I915_READ(FBC_CONTROL) & FBC_CTL_EN;
+}
+
+static void g4x_fbc_enable(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_framebuffer *fb = crtc->primary->fb;
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ u32 dpfc_ctl;
+
+ dev_priv->fbc.enabled = true;
+
+ dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_SR_EN;
+ if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
+ dpfc_ctl |= DPFC_CTL_LIMIT_2X;
+ else
+ dpfc_ctl |= DPFC_CTL_LIMIT_1X;
+ dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
+
+ I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
+
+ /* enable it... */
+ I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
+
+ DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
+}
+
+static void g4x_fbc_disable(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 dpfc_ctl;
+
+ dev_priv->fbc.enabled = false;
+
+ /* Disable compression */
+ dpfc_ctl = I915_READ(DPFC_CONTROL);
+ if (dpfc_ctl & DPFC_CTL_EN) {
+ dpfc_ctl &= ~DPFC_CTL_EN;
+ I915_WRITE(DPFC_CONTROL, dpfc_ctl);
+
+ DRM_DEBUG_KMS("disabled FBC\n");
+ }
+}
+
+static bool g4x_fbc_enabled(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN;
+}
+
+static void snb_fbc_blit_update(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 blt_ecoskpd;
+
+ /* Make sure blitter notifies FBC of writes */
+
+ /* Blitter is part of Media powerwell on VLV. No impact of
+ * his param in other platforms for now */
+ gen6_gt_force_wake_get(dev_priv, FORCEWAKE_MEDIA);
+
+ blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
+ blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY <<
+ GEN6_BLITTER_LOCK_SHIFT;
+ I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
+ blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
+ I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
+ blt_ecoskpd &= ~(GEN6_BLITTER_FBC_NOTIFY <<
+ GEN6_BLITTER_LOCK_SHIFT);
+ I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
+ POSTING_READ(GEN6_BLITTER_ECOSKPD);
+
+ gen6_gt_force_wake_put(dev_priv, FORCEWAKE_MEDIA);
+}
+
+static void ilk_fbc_enable(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_framebuffer *fb = crtc->primary->fb;
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ u32 dpfc_ctl;
+
+ dev_priv->fbc.enabled = true;
+
+ dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane);
+ if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
+ dev_priv->fbc.threshold++;
+
+ switch (dev_priv->fbc.threshold) {
+ case 4:
+ case 3:
+ dpfc_ctl |= DPFC_CTL_LIMIT_4X;
+ break;
+ case 2:
+ dpfc_ctl |= DPFC_CTL_LIMIT_2X;
+ break;
+ case 1:
+ dpfc_ctl |= DPFC_CTL_LIMIT_1X;
+ break;
+ }
+ dpfc_ctl |= DPFC_CTL_FENCE_EN;
+ if (IS_GEN5(dev))
+ dpfc_ctl |= obj->fence_reg;
+
+ I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->y);
+ I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
+ /* enable it... */
+ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
+
+ if (IS_GEN6(dev)) {
+ I915_WRITE(SNB_DPFC_CTL_SA,
+ SNB_CPU_FENCE_ENABLE | obj->fence_reg);
+ I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
+ snb_fbc_blit_update(dev);
+ }
+
+ DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
+}
+
+static void ilk_fbc_disable(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 dpfc_ctl;
+
+ dev_priv->fbc.enabled = false;
+
+ /* Disable compression */
+ dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
+ if (dpfc_ctl & DPFC_CTL_EN) {
+ dpfc_ctl &= ~DPFC_CTL_EN;
+ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl);
+
+ DRM_DEBUG_KMS("disabled FBC\n");
+ }
+}
+
+static bool ilk_fbc_enabled(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ return I915_READ(ILK_DPFC_CONTROL) & DPFC_CTL_EN;
+}
+
+static void gen7_fbc_enable(struct drm_crtc *crtc)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_framebuffer *fb = crtc->primary->fb;
+ struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ u32 dpfc_ctl;
+
+ dev_priv->fbc.enabled = true;
+
+ dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane);
+ if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
+ dev_priv->fbc.threshold++;
+
+ switch (dev_priv->fbc.threshold) {
+ case 4:
+ case 3:
+ dpfc_ctl |= DPFC_CTL_LIMIT_4X;
+ break;
+ case 2:
+ dpfc_ctl |= DPFC_CTL_LIMIT_2X;
+ break;
+ case 1:
+ dpfc_ctl |= DPFC_CTL_LIMIT_1X;
+ break;
+ }
+
+ dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN;
+
+ if (dev_priv->fbc.false_color)
+ dpfc_ctl |= FBC_CTL_FALSE_COLOR;
+
+ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
+
+ if (IS_IVYBRIDGE(dev)) {
+ /* WaFbcAsynchFlipDisableFbcQueue:ivb */
+ I915_WRITE(ILK_DISPLAY_CHICKEN1,
+ I915_READ(ILK_DISPLAY_CHICKEN1) |
+ ILK_FBCQ_DIS);
+ } else {
+ /* WaFbcAsynchFlipDisableFbcQueue:hsw,bdw */
+ I915_WRITE(CHICKEN_PIPESL_1(intel_crtc->pipe),
+ I915_READ(CHICKEN_PIPESL_1(intel_crtc->pipe)) |
+ HSW_FBCQ_DIS);
+ }
+
+ I915_WRITE(SNB_DPFC_CTL_SA,
+ SNB_CPU_FENCE_ENABLE | obj->fence_reg);
+ I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
+
+ snb_fbc_blit_update(dev);
+
+ DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
+}
+
+bool intel_fbc_enabled(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ return dev_priv->fbc.enabled;
+}
+
+void bdw_fbc_sw_flush(struct drm_device *dev, u32 value)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (!IS_GEN8(dev))
+ return;
+
+ if (!intel_fbc_enabled(dev))
+ return;
+
+ I915_WRITE(MSG_FBC_REND_STATE, value);
+}
+
+static void intel_fbc_work_fn(struct work_struct *__work)
+{
+ struct intel_fbc_work *work =
+ container_of(to_delayed_work(__work),
+ struct intel_fbc_work, work);
+ struct drm_device *dev = work->crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ mutex_lock(&dev->struct_mutex);
+ if (work == dev_priv->fbc.fbc_work) {
+ /* Double check that we haven't switched fb without cancelling
+ * the prior work.
+ */
+ if (work->crtc->primary->fb == work->fb) {
+ dev_priv->display.enable_fbc(work->crtc);
+
+ dev_priv->fbc.plane = to_intel_crtc(work->crtc)->plane;
+ dev_priv->fbc.fb_id = work->crtc->primary->fb->base.id;
+ dev_priv->fbc.y = work->crtc->y;
+ }
+
+ dev_priv->fbc.fbc_work = NULL;
+ }
+ mutex_unlock(&dev->struct_mutex);
+
+ kfree(work);
+}
+
+static void intel_fbc_cancel_work(struct drm_i915_private *dev_priv)
+{
+ if (dev_priv->fbc.fbc_work == NULL)
+ return;
+
+ DRM_DEBUG_KMS("cancelling pending FBC enable\n");
+
+ /* Synchronisation is provided by struct_mutex and checking of
+ * dev_priv->fbc.fbc_work, so we can perform the cancellation
+ * entirely asynchronously.
+ */
+ if (cancel_delayed_work(&dev_priv->fbc.fbc_work->work))
+ /* tasklet was killed before being run, clean up */
+ kfree(dev_priv->fbc.fbc_work);
+
+ /* Mark the work as no longer wanted so that if it does
+ * wake-up (because the work was already running and waiting
+ * for our mutex), it will discover that is no longer
+ * necessary to run.
+ */
+ dev_priv->fbc.fbc_work = NULL;
+}
+
+static void intel_fbc_enable(struct drm_crtc *crtc)
+{
+ struct intel_fbc_work *work;
+ struct drm_device *dev = crtc->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ if (!dev_priv->display.enable_fbc)
+ return;
+
+ intel_fbc_cancel_work(dev_priv);
+
+ work = kzalloc(sizeof(*work), GFP_KERNEL);
+ if (work == NULL) {
+ DRM_ERROR("Failed to allocate FBC work structure\n");
+ dev_priv->display.enable_fbc(crtc);
+ return;
+ }
+
+ work->crtc = crtc;
+ work->fb = crtc->primary->fb;
+ INIT_DELAYED_WORK(&work->work, intel_fbc_work_fn);
+
+ dev_priv->fbc.fbc_work = work;
+
+ /* Delay the actual enabling to let pageflipping cease and the
+ * display to settle before starting the compression. Note that
+ * this delay also serves a second purpose: it allows for a
+ * vblank to pass after disabling the FBC before we attempt
+ * to modify the control registers.
+ *
+ * A more complicated solution would involve tracking vblanks
+ * following the termination of the page-flipping sequence
+ * and indeed performing the enable as a co-routine and not
+ * waiting synchronously upon the vblank.
+ *
+ * WaFbcWaitForVBlankBeforeEnable:ilk,snb
+ */
+ schedule_delayed_work(&work->work, msecs_to_jiffies(50));
+}
+
+void intel_fbc_disable(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ intel_fbc_cancel_work(dev_priv);
+
+ if (!dev_priv->display.disable_fbc)
+ return;
+
+ dev_priv->display.disable_fbc(dev);
+ dev_priv->fbc.plane = -1;
+}
+
+static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
+ enum no_fbc_reason reason)
+{
+ if (dev_priv->fbc.no_fbc_reason == reason)
+ return false;
+
+ dev_priv->fbc.no_fbc_reason = reason;
+ return true;
+}
+
+/**
+ * intel_fbc_update - enable/disable FBC as needed
+ * @dev: the drm_device
+ *
+ * Set up the framebuffer compression hardware at mode set time. We
+ * enable it if possible:
+ * - plane A only (on pre-965)
+ * - no pixel mulitply/line duplication
+ * - no alpha buffer discard
+ * - no dual wide
+ * - framebuffer <= max_hdisplay in width, max_vdisplay in height
+ *
+ * We can't assume that any compression will take place (worst case),
+ * so the compressed buffer has to be the same size as the uncompressed
+ * one. It also must reside (along with the line length buffer) in
+ * stolen memory.
+ *
+ * We need to enable/disable FBC on a global basis.
+ */
+void intel_fbc_update(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_crtc *crtc = NULL, *tmp_crtc;
+ struct intel_crtc *intel_crtc;
+ struct drm_framebuffer *fb;
+ struct drm_i915_gem_object *obj;
+ const struct drm_display_mode *adjusted_mode;
+ unsigned int max_width, max_height;
+
+ if (!HAS_FBC(dev)) {
+ set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED);
+ return;
+ }
+
+ if (!i915.powersave) {
+ if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
+ DRM_DEBUG_KMS("fbc disabled per module param\n");
+ return;
+ }
+
+ /*
+ * If FBC is already on, we just have to verify that we can
+ * keep it that way...
+ * Need to disable if:
+ * - more than one pipe is active
+ * - changing FBC params (stride, fence, mode)
+ * - new fb is too large to fit in compressed buffer
+ * - going to an unsupported config (interlace, pixel multiply, etc.)
+ */
+ for_each_crtc(dev, tmp_crtc) {
+ if (intel_crtc_active(tmp_crtc) &&
+ to_intel_crtc(tmp_crtc)->primary_enabled) {
+ if (crtc) {
+ if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
+ DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
+ goto out_disable;
+ }
+ crtc = tmp_crtc;
+ }
+ }
+
+ if (!crtc || crtc->primary->fb == NULL) {
+ if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT))
+ DRM_DEBUG_KMS("no output, disabling\n");
+ goto out_disable;
+ }
+
+ intel_crtc = to_intel_crtc(crtc);
+ fb = crtc->primary->fb;
+ obj = intel_fb_obj(fb);
+ adjusted_mode = &intel_crtc->config.adjusted_mode;
+
+ if (i915.enable_fbc < 0) {
+ if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
+ DRM_DEBUG_KMS("disabled per chip default\n");
+ goto out_disable;
+ }
+ if (!i915.enable_fbc) {
+ if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
+ DRM_DEBUG_KMS("fbc disabled per module param\n");
+ goto out_disable;
+ }
+ if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ||
+ (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) {
+ if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
+ DRM_DEBUG_KMS("mode incompatible with compression, "
+ "disabling\n");
+ goto out_disable;
+ }
+
+ if (INTEL_INFO(dev)->gen >= 8 || IS_HASWELL(dev)) {
+ max_width = 4096;
+ max_height = 4096;
+ } else if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
+ max_width = 4096;
+ max_height = 2048;
+ } else {
+ max_width = 2048;
+ max_height = 1536;
+ }
+ if (intel_crtc->config.pipe_src_w > max_width ||
+ intel_crtc->config.pipe_src_h > max_height) {
+ if (set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE))
+ DRM_DEBUG_KMS("mode too large for compression, disabling\n");
+ goto out_disable;
+ }
+ if ((INTEL_INFO(dev)->gen < 4 || HAS_DDI(dev)) &&
+ intel_crtc->plane != PLANE_A) {
+ if (set_no_fbc_reason(dev_priv, FBC_BAD_PLANE))
+ DRM_DEBUG_KMS("plane not A, disabling compression\n");
+ goto out_disable;
+ }
+
+ /* The use of a CPU fence is mandatory in order to detect writes
+ * by the CPU to the scanout and trigger updates to the FBC.
+ */
+ if (obj->tiling_mode != I915_TILING_X ||
+ obj->fence_reg == I915_FENCE_REG_NONE) {
+ if (set_no_fbc_reason(dev_priv, FBC_NOT_TILED))
+ DRM_DEBUG_KMS("framebuffer not tiled or fenced, disabling compression\n");
+ goto out_disable;
+ }
+ if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+ to_intel_plane(crtc->primary)->rotation != BIT(DRM_ROTATE_0)) {
+ if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
+ DRM_DEBUG_KMS("Rotation unsupported, disabling\n");
+ goto out_disable;
+ }
+
+ /* If the kernel debugger is active, always disable compression */
+ if (in_dbg_master())
+ goto out_disable;
+
+ if (i915_gem_stolen_setup_compression(dev, obj->base.size,
+ drm_format_plane_cpp(fb->pixel_format, 0))) {
+ if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
+ DRM_DEBUG_KMS("framebuffer too large, disabling compression\n");
+ goto out_disable;
+ }
+
+ /* If the scanout has not changed, don't modify the FBC settings.
+ * Note that we make the fundamental assumption that the fb->obj
+ * cannot be unpinned (and have its GTT offset and fence revoked)
+ * without first being decoupled from the scanout and FBC disabled.
+ */
+ if (dev_priv->fbc.plane == intel_crtc->plane &&
+ dev_priv->fbc.fb_id == fb->base.id &&
+ dev_priv->fbc.y == crtc->y)
+ return;
+
+ if (intel_fbc_enabled(dev)) {
+ /* We update FBC along two paths, after changing fb/crtc
+ * configuration (modeswitching) and after page-flipping
+ * finishes. For the latter, we know that not only did
+ * we disable the FBC at the start of the page-flip
+ * sequence, but also more than one vblank has passed.
+ *
+ * For the former case of modeswitching, it is possible
+ * to switch between two FBC valid configurations
+ * instantaneously so we do need to disable the FBC
+ * before we can modify its control registers. We also
+ * have to wait for the next vblank for that to take
+ * effect. However, since we delay enabling FBC we can
+ * assume that a vblank has passed since disabling and
+ * that we can safely alter the registers in the deferred
+ * callback.
+ *
+ * In the scenario that we go from a valid to invalid
+ * and then back to valid FBC configuration we have
+ * no strict enforcement that a vblank occurred since
+ * disabling the FBC. However, along all current pipe
+ * disabling paths we do need to wait for a vblank at
+ * some point. And we wait before enabling FBC anyway.
+ */
+ DRM_DEBUG_KMS("disabling active FBC for update\n");
+ intel_fbc_disable(dev);
+ }
+
+ intel_fbc_enable(crtc);
+ dev_priv->fbc.no_fbc_reason = FBC_OK;
+ return;
+
+out_disable:
+ /* Multiple disables should be harmless */
+ if (intel_fbc_enabled(dev)) {
+ DRM_DEBUG_KMS("unsupported config, disabling FBC\n");
+ intel_fbc_disable(dev);
+ }
+ i915_gem_stolen_cleanup_compression(dev);
+}
+
+void intel_fbc_init(struct drm_i915_private *dev_priv)
+{
+ if (!HAS_FBC(dev_priv)) {
+ dev_priv->fbc.enabled = false;
+ return;
+ }
+
+ if (INTEL_INFO(dev_priv)->gen >= 7) {
+ dev_priv->display.fbc_enabled = ilk_fbc_enabled;
+ dev_priv->display.enable_fbc = gen7_fbc_enable;
+ dev_priv->display.disable_fbc = ilk_fbc_disable;
+ } else if (INTEL_INFO(dev_priv)->gen >= 5) {
+ dev_priv->display.fbc_enabled = ilk_fbc_enabled;
+ dev_priv->display.enable_fbc = ilk_fbc_enable;
+ dev_priv->display.disable_fbc = ilk_fbc_disable;
+ } else if (IS_GM45(dev_priv)) {
+ dev_priv->display.fbc_enabled = g4x_fbc_enabled;
+ dev_priv->display.enable_fbc = g4x_fbc_enable;
+ dev_priv->display.disable_fbc = g4x_fbc_disable;
+ } else {
+ dev_priv->display.fbc_enabled = i8xx_fbc_enabled;
+ dev_priv->display.enable_fbc = i8xx_fbc_enable;
+ dev_priv->display.disable_fbc = i8xx_fbc_disable;
+
+ /* This value was pulled out of someone's hat */
+ I915_WRITE(FBC_CONTROL, 500 << FBC_CTL_INTERVAL_SHIFT);
+ }
+
+ dev_priv->fbc.enabled = dev_priv->display.fbc_enabled(dev_priv->dev);
+}
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 209751b..8a14fcd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -52,17 +52,6 @@
#define INTEL_RC6p_ENABLE (1<<1)
#define INTEL_RC6pp_ENABLE (1<<2)
-/* FBC, or Frame Buffer Compression, is a technique employed to compress the
- * framebuffer contents in-memory, aiming at reducing the required bandwidth
- * during in-memory transfers and, therefore, reduce the power packet.
- *
- * The benefits of FBC are mostly visible with solid backgrounds and
- * variation-less patterns.
- *
- * FBC-related functionality can be enabled by the means of the
- * i915.i915_enable_fbc parameter
- */
-
static void gen9_init_clock_gating(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -87,613 +76,6 @@ static void gen9_init_clock_gating(struct drm_device *dev)
_MASKED_BIT_ENABLE(GEN8_4x4_STC_OPTIMIZATION_DISABLE));
}
-static void i8xx_disable_fbc(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- u32 fbc_ctl;
-
- dev_priv->fbc.enabled = false;
-
- /* Disable compression */
- fbc_ctl = I915_READ(FBC_CONTROL);
- if ((fbc_ctl & FBC_CTL_EN) == 0)
- return;
-
- fbc_ctl &= ~FBC_CTL_EN;
- I915_WRITE(FBC_CONTROL, fbc_ctl);
-
- /* Wait for compressing bit to clear */
- if (wait_for((I915_READ(FBC_STATUS) & FBC_STAT_COMPRESSING) == 0, 10)) {
- DRM_DEBUG_KMS("FBC idle timed out\n");
- return;
- }
-
- DRM_DEBUG_KMS("disabled FBC\n");
-}
-
-static void i8xx_enable_fbc(struct drm_crtc *crtc)
-{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_framebuffer *fb = crtc->primary->fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int cfb_pitch;
- int i;
- u32 fbc_ctl;
-
- dev_priv->fbc.enabled = true;
-
- cfb_pitch = dev_priv->fbc.size / FBC_LL_SIZE;
- if (fb->pitches[0] < cfb_pitch)
- cfb_pitch = fb->pitches[0];
-
- /* FBC_CTL wants 32B or 64B units */
- if (IS_GEN2(dev))
- cfb_pitch = (cfb_pitch / 32) - 1;
- else
- cfb_pitch = (cfb_pitch / 64) - 1;
-
- /* Clear old tags */
- for (i = 0; i < (FBC_LL_SIZE / 32) + 1; i++)
- I915_WRITE(FBC_TAG + (i * 4), 0);
-
- if (IS_GEN4(dev)) {
- u32 fbc_ctl2;
-
- /* Set it up... */
- fbc_ctl2 = FBC_CTL_FENCE_DBL | FBC_CTL_IDLE_IMM | FBC_CTL_CPU_FENCE;
- fbc_ctl2 |= FBC_CTL_PLANE(intel_crtc->plane);
- I915_WRITE(FBC_CONTROL2, fbc_ctl2);
- I915_WRITE(FBC_FENCE_OFF, crtc->y);
- }
-
- /* enable it... */
- fbc_ctl = I915_READ(FBC_CONTROL);
- fbc_ctl &= 0x3fff << FBC_CTL_INTERVAL_SHIFT;
- fbc_ctl |= FBC_CTL_EN | FBC_CTL_PERIODIC;
- if (IS_I945GM(dev))
- fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
- fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
- fbc_ctl |= obj->fence_reg;
- I915_WRITE(FBC_CONTROL, fbc_ctl);
-
- DRM_DEBUG_KMS("enabled FBC, pitch %d, yoff %d, plane %c\n",
- cfb_pitch, crtc->y, plane_name(intel_crtc->plane));
-}
-
-static bool i8xx_fbc_enabled(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- return I915_READ(FBC_CONTROL) & FBC_CTL_EN;
-}
-
-static void g4x_enable_fbc(struct drm_crtc *crtc)
-{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_framebuffer *fb = crtc->primary->fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- u32 dpfc_ctl;
-
- dev_priv->fbc.enabled = true;
-
- dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane) | DPFC_SR_EN;
- if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
- dpfc_ctl |= DPFC_CTL_LIMIT_2X;
- else
- dpfc_ctl |= DPFC_CTL_LIMIT_1X;
- dpfc_ctl |= DPFC_CTL_FENCE_EN | obj->fence_reg;
-
- I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
-
- /* enable it... */
- I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
-
- DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
-}
-
-static void g4x_disable_fbc(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- u32 dpfc_ctl;
-
- dev_priv->fbc.enabled = false;
-
- /* Disable compression */
- dpfc_ctl = I915_READ(DPFC_CONTROL);
- if (dpfc_ctl & DPFC_CTL_EN) {
- dpfc_ctl &= ~DPFC_CTL_EN;
- I915_WRITE(DPFC_CONTROL, dpfc_ctl);
-
- DRM_DEBUG_KMS("disabled FBC\n");
- }
-}
-
-static bool g4x_fbc_enabled(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN;
-}
-
-static void sandybridge_blit_fbc_update(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- u32 blt_ecoskpd;
-
- /* Make sure blitter notifies FBC of writes */
-
- /* Blitter is part of Media powerwell on VLV. No impact of
- * his param in other platforms for now */
- gen6_gt_force_wake_get(dev_priv, FORCEWAKE_MEDIA);
-
- blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
- blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY <<
- GEN6_BLITTER_LOCK_SHIFT;
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- blt_ecoskpd &= ~(GEN6_BLITTER_FBC_NOTIFY <<
- GEN6_BLITTER_LOCK_SHIFT);
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- POSTING_READ(GEN6_BLITTER_ECOSKPD);
-
- gen6_gt_force_wake_put(dev_priv, FORCEWAKE_MEDIA);
-}
-
-static void ironlake_enable_fbc(struct drm_crtc *crtc)
-{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_framebuffer *fb = crtc->primary->fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- u32 dpfc_ctl;
-
- dev_priv->fbc.enabled = true;
-
- dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane);
- if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
- dev_priv->fbc.threshold++;
-
- switch (dev_priv->fbc.threshold) {
- case 4:
- case 3:
- dpfc_ctl |= DPFC_CTL_LIMIT_4X;
- break;
- case 2:
- dpfc_ctl |= DPFC_CTL_LIMIT_2X;
- break;
- case 1:
- dpfc_ctl |= DPFC_CTL_LIMIT_1X;
- break;
- }
- dpfc_ctl |= DPFC_CTL_FENCE_EN;
- if (IS_GEN5(dev))
- dpfc_ctl |= obj->fence_reg;
-
- I915_WRITE(ILK_DPFC_FENCE_YOFF, crtc->y);
- I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID);
- /* enable it... */
- I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
-
- if (IS_GEN6(dev)) {
- I915_WRITE(SNB_DPFC_CTL_SA,
- SNB_CPU_FENCE_ENABLE | obj->fence_reg);
- I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
- sandybridge_blit_fbc_update(dev);
- }
-
- DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
-}
-
-static void ironlake_disable_fbc(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- u32 dpfc_ctl;
-
- dev_priv->fbc.enabled = false;
-
- /* Disable compression */
- dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
- if (dpfc_ctl & DPFC_CTL_EN) {
- dpfc_ctl &= ~DPFC_CTL_EN;
- I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl);
-
- DRM_DEBUG_KMS("disabled FBC\n");
- }
-}
-
-static bool ironlake_fbc_enabled(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- return I915_READ(ILK_DPFC_CONTROL) & DPFC_CTL_EN;
-}
-
-static void gen7_enable_fbc(struct drm_crtc *crtc)
-{
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_framebuffer *fb = crtc->primary->fb;
- struct drm_i915_gem_object *obj = intel_fb_obj(fb);
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- u32 dpfc_ctl;
-
- dev_priv->fbc.enabled = true;
-
- dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane);
- if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
- dev_priv->fbc.threshold++;
-
- switch (dev_priv->fbc.threshold) {
- case 4:
- case 3:
- dpfc_ctl |= DPFC_CTL_LIMIT_4X;
- break;
- case 2:
- dpfc_ctl |= DPFC_CTL_LIMIT_2X;
- break;
- case 1:
- dpfc_ctl |= DPFC_CTL_LIMIT_1X;
- break;
- }
-
- dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN;
-
- if (dev_priv->fbc.false_color)
- dpfc_ctl |= FBC_CTL_FALSE_COLOR;
-
- I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
-
- if (IS_IVYBRIDGE(dev)) {
- /* WaFbcAsynchFlipDisableFbcQueue:ivb */
- I915_WRITE(ILK_DISPLAY_CHICKEN1,
- I915_READ(ILK_DISPLAY_CHICKEN1) |
- ILK_FBCQ_DIS);
- } else {
- /* WaFbcAsynchFlipDisableFbcQueue:hsw,bdw */
- I915_WRITE(CHICKEN_PIPESL_1(intel_crtc->pipe),
- I915_READ(CHICKEN_PIPESL_1(intel_crtc->pipe)) |
- HSW_FBCQ_DIS);
- }
-
- I915_WRITE(SNB_DPFC_CTL_SA,
- SNB_CPU_FENCE_ENABLE | obj->fence_reg);
- I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
-
- sandybridge_blit_fbc_update(dev);
-
- DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
-}
-
-bool intel_fbc_enabled(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- return dev_priv->fbc.enabled;
-}
-
-void bdw_fbc_sw_flush(struct drm_device *dev, u32 value)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- if (!IS_GEN8(dev))
- return;
-
- if (!intel_fbc_enabled(dev))
- return;
-
- I915_WRITE(MSG_FBC_REND_STATE, value);
-}
-
-static void intel_fbc_work_fn(struct work_struct *__work)
-{
- struct intel_fbc_work *work =
- container_of(to_delayed_work(__work),
- struct intel_fbc_work, work);
- struct drm_device *dev = work->crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- mutex_lock(&dev->struct_mutex);
- if (work == dev_priv->fbc.fbc_work) {
- /* Double check that we haven't switched fb without cancelling
- * the prior work.
- */
- if (work->crtc->primary->fb == work->fb) {
- dev_priv->display.enable_fbc(work->crtc);
-
- dev_priv->fbc.plane = to_intel_crtc(work->crtc)->plane;
- dev_priv->fbc.fb_id = work->crtc->primary->fb->base.id;
- dev_priv->fbc.y = work->crtc->y;
- }
-
- dev_priv->fbc.fbc_work = NULL;
- }
- mutex_unlock(&dev->struct_mutex);
-
- kfree(work);
-}
-
-static void intel_cancel_fbc_work(struct drm_i915_private *dev_priv)
-{
- if (dev_priv->fbc.fbc_work == NULL)
- return;
-
- DRM_DEBUG_KMS("cancelling pending FBC enable\n");
-
- /* Synchronisation is provided by struct_mutex and checking of
- * dev_priv->fbc.fbc_work, so we can perform the cancellation
- * entirely asynchronously.
- */
- if (cancel_delayed_work(&dev_priv->fbc.fbc_work->work))
- /* tasklet was killed before being run, clean up */
- kfree(dev_priv->fbc.fbc_work);
-
- /* Mark the work as no longer wanted so that if it does
- * wake-up (because the work was already running and waiting
- * for our mutex), it will discover that is no longer
- * necessary to run.
- */
- dev_priv->fbc.fbc_work = NULL;
-}
-
-static void intel_enable_fbc(struct drm_crtc *crtc)
-{
- struct intel_fbc_work *work;
- struct drm_device *dev = crtc->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- if (!dev_priv->display.enable_fbc)
- return;
-
- intel_cancel_fbc_work(dev_priv);
-
- work = kzalloc(sizeof(*work), GFP_KERNEL);
- if (work == NULL) {
- DRM_ERROR("Failed to allocate FBC work structure\n");
- dev_priv->display.enable_fbc(crtc);
- return;
- }
-
- work->crtc = crtc;
- work->fb = crtc->primary->fb;
- INIT_DELAYED_WORK(&work->work, intel_fbc_work_fn);
-
- dev_priv->fbc.fbc_work = work;
-
- /* Delay the actual enabling to let pageflipping cease and the
- * display to settle before starting the compression. Note that
- * this delay also serves a second purpose: it allows for a
- * vblank to pass after disabling the FBC before we attempt
- * to modify the control registers.
- *
- * A more complicated solution would involve tracking vblanks
- * following the termination of the page-flipping sequence
- * and indeed performing the enable as a co-routine and not
- * waiting synchronously upon the vblank.
- *
- * WaFbcWaitForVBlankBeforeEnable:ilk,snb
- */
- schedule_delayed_work(&work->work, msecs_to_jiffies(50));
-}
-
-void intel_disable_fbc(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- intel_cancel_fbc_work(dev_priv);
-
- if (!dev_priv->display.disable_fbc)
- return;
-
- dev_priv->display.disable_fbc(dev);
- dev_priv->fbc.plane = -1;
-}
-
-static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
- enum no_fbc_reason reason)
-{
- if (dev_priv->fbc.no_fbc_reason == reason)
- return false;
-
- dev_priv->fbc.no_fbc_reason = reason;
- return true;
-}
-
-/**
- * intel_update_fbc - enable/disable FBC as needed
- * @dev: the drm_device
- *
- * Set up the framebuffer compression hardware at mode set time. We
- * enable it if possible:
- * - plane A only (on pre-965)
- * - no pixel mulitply/line duplication
- * - no alpha buffer discard
- * - no dual wide
- * - framebuffer <= max_hdisplay in width, max_vdisplay in height
- *
- * We can't assume that any compression will take place (worst case),
- * so the compressed buffer has to be the same size as the uncompressed
- * one. It also must reside (along with the line length buffer) in
- * stolen memory.
- *
- * We need to enable/disable FBC on a global basis.
- */
-void intel_update_fbc(struct drm_device *dev)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_crtc *crtc = NULL, *tmp_crtc;
- struct intel_crtc *intel_crtc;
- struct drm_framebuffer *fb;
- struct drm_i915_gem_object *obj;
- const struct drm_display_mode *adjusted_mode;
- unsigned int max_width, max_height;
-
- if (!HAS_FBC(dev)) {
- set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED);
- return;
- }
-
- if (!i915.powersave) {
- if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
- DRM_DEBUG_KMS("fbc disabled per module param\n");
- return;
- }
-
- /*
- * If FBC is already on, we just have to verify that we can
- * keep it that way...
- * Need to disable if:
- * - more than one pipe is active
- * - changing FBC params (stride, fence, mode)
- * - new fb is too large to fit in compressed buffer
- * - going to an unsupported config (interlace, pixel multiply, etc.)
- */
- for_each_crtc(dev, tmp_crtc) {
- if (intel_crtc_active(tmp_crtc) &&
- to_intel_crtc(tmp_crtc)->primary_enabled) {
- if (crtc) {
- if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
- DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
- goto out_disable;
- }
- crtc = tmp_crtc;
- }
- }
-
- if (!crtc || crtc->primary->fb == NULL) {
- if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT))
- DRM_DEBUG_KMS("no output, disabling\n");
- goto out_disable;
- }
-
- intel_crtc = to_intel_crtc(crtc);
- fb = crtc->primary->fb;
- obj = intel_fb_obj(fb);
- adjusted_mode = &intel_crtc->config.adjusted_mode;
-
- if (i915.enable_fbc < 0) {
- if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
- DRM_DEBUG_KMS("disabled per chip default\n");
- goto out_disable;
- }
- if (!i915.enable_fbc) {
- if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
- DRM_DEBUG_KMS("fbc disabled per module param\n");
- goto out_disable;
- }
- if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ||
- (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) {
- if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
- DRM_DEBUG_KMS("mode incompatible with compression, "
- "disabling\n");
- goto out_disable;
- }
-
- if (INTEL_INFO(dev)->gen >= 8 || IS_HASWELL(dev)) {
- max_width = 4096;
- max_height = 4096;
- } else if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
- max_width = 4096;
- max_height = 2048;
- } else {
- max_width = 2048;
- max_height = 1536;
- }
- if (intel_crtc->config.pipe_src_w > max_width ||
- intel_crtc->config.pipe_src_h > max_height) {
- if (set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE))
- DRM_DEBUG_KMS("mode too large for compression, disabling\n");
- goto out_disable;
- }
- if ((INTEL_INFO(dev)->gen < 4 || HAS_DDI(dev)) &&
- intel_crtc->plane != PLANE_A) {
- if (set_no_fbc_reason(dev_priv, FBC_BAD_PLANE))
- DRM_DEBUG_KMS("plane not A, disabling compression\n");
- goto out_disable;
- }
-
- /* The use of a CPU fence is mandatory in order to detect writes
- * by the CPU to the scanout and trigger updates to the FBC.
- */
- if (obj->tiling_mode != I915_TILING_X ||
- obj->fence_reg == I915_FENCE_REG_NONE) {
- if (set_no_fbc_reason(dev_priv, FBC_NOT_TILED))
- DRM_DEBUG_KMS("framebuffer not tiled or fenced, disabling compression\n");
- goto out_disable;
- }
- if (INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
- to_intel_plane(crtc->primary)->rotation != BIT(DRM_ROTATE_0)) {
- if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
- DRM_DEBUG_KMS("Rotation unsupported, disabling\n");
- goto out_disable;
- }
-
- /* If the kernel debugger is active, always disable compression */
- if (in_dbg_master())
- goto out_disable;
-
- if (i915_gem_stolen_setup_compression(dev, obj->base.size,
- drm_format_plane_cpp(fb->pixel_format, 0))) {
- if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
- DRM_DEBUG_KMS("framebuffer too large, disabling compression\n");
- goto out_disable;
- }
-
- /* If the scanout has not changed, don't modify the FBC settings.
- * Note that we make the fundamental assumption that the fb->obj
- * cannot be unpinned (and have its GTT offset and fence revoked)
- * without first being decoupled from the scanout and FBC disabled.
- */
- if (dev_priv->fbc.plane == intel_crtc->plane &&
- dev_priv->fbc.fb_id == fb->base.id &&
- dev_priv->fbc.y == crtc->y)
- return;
-
- if (intel_fbc_enabled(dev)) {
- /* We update FBC along two paths, after changing fb/crtc
- * configuration (modeswitching) and after page-flipping
- * finishes. For the latter, we know that not only did
- * we disable the FBC at the start of the page-flip
- * sequence, but also more than one vblank has passed.
- *
- * For the former case of modeswitching, it is possible
- * to switch between two FBC valid configurations
- * instantaneously so we do need to disable the FBC
- * before we can modify its control registers. We also
- * have to wait for the next vblank for that to take
- * effect. However, since we delay enabling FBC we can
- * assume that a vblank has passed since disabling and
- * that we can safely alter the registers in the deferred
- * callback.
- *
- * In the scenario that we go from a valid to invalid
- * and then back to valid FBC configuration we have
- * no strict enforcement that a vblank occurred since
- * disabling the FBC. However, along all current pipe
- * disabling paths we do need to wait for a vblank at
- * some point. And we wait before enabling FBC anyway.
- */
- DRM_DEBUG_KMS("disabling active FBC for update\n");
- intel_disable_fbc(dev);
- }
-
- intel_enable_fbc(crtc);
- dev_priv->fbc.no_fbc_reason = FBC_OK;
- return;
-
-out_disable:
- /* Multiple disables should be harmless */
- if (intel_fbc_enabled(dev)) {
- DRM_DEBUG_KMS("unsupported config, disabling FBC\n");
- intel_disable_fbc(dev);
- }
- i915_gem_stolen_cleanup_compression(dev);
-}
static void i915_pineview_get_mem_freq(struct drm_device *dev)
{
@@ -6922,43 +6304,12 @@ void intel_suspend_hw(struct drm_device *dev)
lpt_suspend_hw(dev);
}
-static void intel_init_fbc(struct drm_i915_private *dev_priv)
-{
- if (!HAS_FBC(dev_priv)) {
- dev_priv->fbc.enabled = false;
- return;
- }
-
- if (INTEL_INFO(dev_priv)->gen >= 7) {
- dev_priv->display.fbc_enabled = ironlake_fbc_enabled;
- dev_priv->display.enable_fbc = gen7_enable_fbc;
- dev_priv->display.disable_fbc = ironlake_disable_fbc;
- } else if (INTEL_INFO(dev_priv)->gen >= 5) {
- dev_priv->display.fbc_enabled = ironlake_fbc_enabled;
- dev_priv->display.enable_fbc = ironlake_enable_fbc;
- dev_priv->display.disable_fbc = ironlake_disable_fbc;
- } else if (IS_GM45(dev_priv)) {
- dev_priv->display.fbc_enabled = g4x_fbc_enabled;
- dev_priv->display.enable_fbc = g4x_enable_fbc;
- dev_priv->display.disable_fbc = g4x_disable_fbc;
- } else {
- dev_priv->display.fbc_enabled = i8xx_fbc_enabled;
- dev_priv->display.enable_fbc = i8xx_enable_fbc;
- dev_priv->display.disable_fbc = i8xx_disable_fbc;
-
- /* This value was pulled out of someone's hat */
- I915_WRITE(FBC_CONTROL, 500 << FBC_CTL_INTERVAL_SHIFT);
- }
-
- dev_priv->fbc.enabled = dev_priv->display.fbc_enabled(dev_priv->dev);
-}
-
/* Set up chip specific power management-related functions */
void intel_init_pm(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- intel_init_fbc(dev_priv);
+ intel_fbc_init(dev_priv);
/* For cxsr */
if (IS_PINEVIEW(dev))
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index bc5834b..c18e57d 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1004,7 +1004,7 @@ intel_post_enable_primary(struct drm_crtc *crtc)
hsw_enable_ips(intel_crtc);
mutex_lock(&dev->struct_mutex);
- intel_update_fbc(dev);
+ intel_fbc_update(dev);
mutex_unlock(&dev->struct_mutex);
}
@@ -1017,7 +1017,7 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
mutex_lock(&dev->struct_mutex);
if (dev_priv->fbc.plane == intel_crtc->plane)
- intel_disable_fbc(dev);
+ intel_fbc_disable(dev);
mutex_unlock(&dev->struct_mutex);
/*
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 02/11] drm/i915: Introduce FBC DocBook.
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
2014-12-08 16:09 ` [PATCH 01/11] drm/i915: Move FBC stuff to intel_fbc.c Paulo Zanoni
@ 2014-12-08 16:09 ` Paulo Zanoni
2014-12-08 16:49 ` Daniel Vetter
2014-12-08 16:09 ` [PATCH 03/11] drm/i915: don't try to find crtcs for FBC if it's disabled Paulo Zanoni
` (9 subsequent siblings)
11 siblings, 1 reply; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi
From: Rodrigo Vivi <rodrigo.vivi@intel.com>
No functional changes.
v2 (Paulo): Rebase.
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
Documentation/DocBook/drm.tmpl | 5 ++++
drivers/gpu/drm/i915/intel_fbc.c | 57 ++++++++++++++++++++++++++++++++++------
2 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 85287cb..8b780ab 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -3926,6 +3926,11 @@ int num_ioctls;</synopsis>
!Idrivers/gpu/drm/i915/intel_psr.c
</sect2>
<sect2>
+ <title>Frame Buffer Compression (FBC)</title>
+!Pdrivers/gpu/drm/i915/intel_fbc.c Frame Buffer Compression (FBC)
+!Idrivers/gpu/drm/i915/intel_fbc.c
+ </sect2>
+ <sect2>
<title>DPIO</title>
!Pdrivers/gpu/drm/i915/i915_reg.h DPIO
<table id="dpiox2">
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index f1eeb86..7686573 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -21,20 +21,31 @@
* DEALINGS IN THE SOFTWARE.
*/
-#include "intel_drv.h"
-#include "i915_drv.h"
-
-/* FBC, or Frame Buffer Compression, is a technique employed to compress the
- * framebuffer contents in-memory, aiming at reducing the required bandwidth
+/**
+ * DOC: Frame Buffer Compression (FBC)
+ *
+ * FBC is a technique employed to compress the framebuffer contents
+ * in-memory, aiming at reducing the required bandwidth
* during in-memory transfers and, therefore, reduce the power packet.
*
+ * FBC is primarily a memory power savings technology. That is the major
+ * benefit is to the memory power while displaying the processor graphics
+ * information to the display. FBC works by compressing the amount of memory
+ * used by the display. It means that it is total transparent to user space.
+ *
* The benefits of FBC are mostly visible with solid backgrounds and
- * variation-less patterns.
+ * variation-less patterns. It comes from keeping the memory footprint small
+ * and having fewer memory pages opened and accessed for refreshing the display.
*
- * FBC-related functionality can be enabled by the means of the
- * i915.i915_fbc_enable parameter
+ * i915 is responsible to reserve stolen memory for FBC and configure its
+ * offset on proper register. The hardware takes care of all
+ * compress/decompress. However there are many known cases where we have to
+ * forcibly disable it to allow proper screen updates.
*/
+#include "intel_drv.h"
+#include "i915_drv.h"
+
static void i8xx_fbc_disable(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -318,6 +329,12 @@ static void gen7_fbc_enable(struct drm_crtc *crtc)
DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
}
+/**
+ * intel_fbc_enabled - Is FBC enabled?
+ * @dev: the drm_device
+ *
+ * This function is used to verify the current state of FBC.
+ */
bool intel_fbc_enabled(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -325,6 +342,18 @@ bool intel_fbc_enabled(struct drm_device *dev)
return dev_priv->fbc.enabled;
}
+/**
+ * bdw_fbc_sw_flush - FBC Software Flush for Broadwell.
+ * @dev: the drm_device
+ * @value: Value to be set on MSG_FBC_REND_STATE. Possible values are
+ * FBC_REND_NUKE and FBC_REND_CACHE_CLEAN.
+ *
+ * This function is needed on Broadwell to perform Nuke or Cache clean on
+ * software side over MMIO.
+ * On Broadwell, due a hardware bug, MSG_FBC_REND_STATE stay in a forbidden
+ * address that has a huge risk of causing GPU Hangs if set with LRI on some
+ * command streamers.
+ */
void bdw_fbc_sw_flush(struct drm_device *dev, u32 value)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -429,6 +458,12 @@ static void intel_fbc_enable(struct drm_crtc *crtc)
schedule_delayed_work(&work->work, msecs_to_jiffies(50));
}
+/**
+ * intel_fbc_disable - disable FBC
+ * @dev: the drm_device
+ *
+ * This function disables FBC.
+ */
void intel_fbc_disable(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -643,6 +678,12 @@ out_disable:
i915_gem_stolen_cleanup_compression(dev);
}
+/**
+ * intel_fbc_init - Initialize FBC
+ * @dev_priv: the i915 device
+ *
+ * This function might be called during PM init process.
+ */
void intel_fbc_init(struct drm_i915_private *dev_priv)
{
if (!HAS_FBC(dev_priv)) {
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 02/11] drm/i915: Introduce FBC DocBook.
2014-12-08 16:09 ` [PATCH 02/11] drm/i915: Introduce FBC DocBook Paulo Zanoni
@ 2014-12-08 16:49 ` Daniel Vetter
2014-12-08 14:46 ` [PATCH] " Rodrigo Vivi
2014-12-08 21:48 ` [PATCH 02/11] " Rodrigo Vivi
0 siblings, 2 replies; 41+ messages in thread
From: Daniel Vetter @ 2014-12-08 16:49 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni, Rodrigo Vivi
On Mon, Dec 08, 2014 at 02:09:11PM -0200, Paulo Zanoni wrote:
> From: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> No functional changes.
>
> v2 (Paulo): Rebase.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Some suggestions to polish the documentation a bit below. I've merged
patch 1 right away to avoid further rebase pain.
Thanks, Daniel
> ---
> Documentation/DocBook/drm.tmpl | 5 ++++
> drivers/gpu/drm/i915/intel_fbc.c | 57 ++++++++++++++++++++++++++++++++++------
> 2 files changed, 54 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 85287cb..8b780ab 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -3926,6 +3926,11 @@ int num_ioctls;</synopsis>
> !Idrivers/gpu/drm/i915/intel_psr.c
> </sect2>
> <sect2>
> + <title>Frame Buffer Compression (FBC)</title>
> +!Pdrivers/gpu/drm/i915/intel_fbc.c Frame Buffer Compression (FBC)
> +!Idrivers/gpu/drm/i915/intel_fbc.c
> + </sect2>
> + <sect2>
> <title>DPIO</title>
> !Pdrivers/gpu/drm/i915/i915_reg.h DPIO
> <table id="dpiox2">
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index f1eeb86..7686573 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -21,20 +21,31 @@
> * DEALINGS IN THE SOFTWARE.
> */
>
> -#include "intel_drv.h"
> -#include "i915_drv.h"
> -
> -/* FBC, or Frame Buffer Compression, is a technique employed to compress the
> - * framebuffer contents in-memory, aiming at reducing the required bandwidth
> +/**
> + * DOC: Frame Buffer Compression (FBC)
> + *
> + * FBC is a technique employed to compress the framebuffer contents
> + * in-memory, aiming at reducing the required bandwidth
> * during in-memory transfers and, therefore, reduce the power packet.
The above paragraph imo doesn't make sense. And duplicates what's below,
so just delete it.
> *
> + * FBC is primarily a memory power savings technology. That is the major
> + * benefit is to the memory power while displaying the processor graphics
> + * information to the display. FBC works by compressing the amount of memory
> + * used by the display. It means that it is total transparent to user space.
This reads a bit too much like marketing for my taste ;-) What about:
* FBC tries to save memory bandwidth (and so power consumption) by
* compressing the amount of memory used by the display. It is total
* transparent to user space and completely handled in the kernel.
> + *
> * The benefits of FBC are mostly visible with solid backgrounds and
> - * variation-less patterns.
> + * variation-less patterns. It comes from keeping the memory footprint small
> + * and having fewer memory pages opened and accessed for refreshing the display.
> *
> - * FBC-related functionality can be enabled by the means of the
> - * i915.i915_fbc_enable parameter
> + * i915 is responsible to reserve stolen memory for FBC and configure its
> + * offset on proper register. The hardware takes care of all
^registers
> + * compress/decompress. However there are many known cases where we have to
> + * forcibly disable it to allow proper screen updates.
Mayb add "... using the frontbuffer tracking infrastructure." At least
when that's the case ;-)
> */
>
> +#include "intel_drv.h"
> +#include "i915_drv.h"
> +
> static void i8xx_fbc_disable(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -318,6 +329,12 @@ static void gen7_fbc_enable(struct drm_crtc *crtc)
> DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
> }
>
> +/**
> + * intel_fbc_enabled - Is FBC enabled?
> + * @dev: the drm_device
> + *
> + * This function is used to verify the current state of FBC.
This needs a FIXME: This should be tracked in the plane config eventually
instead of queried at runtime for most callers.
> + */
> bool intel_fbc_enabled(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -325,6 +342,18 @@ bool intel_fbc_enabled(struct drm_device *dev)
> return dev_priv->fbc.enabled;
> }
>
> +/**
> + * bdw_fbc_sw_flush - FBC Software Flush for Broadwell.
> + * @dev: the drm_device
> + * @value: Value to be set on MSG_FBC_REND_STATE. Possible values are
> + * FBC_REND_NUKE and FBC_REND_CACHE_CLEAN.
> + *
> + * This function is needed on Broadwell to perform Nuke or Cache clean on
> + * software side over MMIO.
> + * On Broadwell, due a hardware bug, MSG_FBC_REND_STATE stay in a forbidden
> + * address that has a huge risk of causing GPU Hangs if set with LRI on some
> + * command streamers.
> + */
I guess with the frontbuffer tracking we'll just have invalidate/flush
entry points and this becomes a static function? In that case I wouldn't
bother documenting it - maybe do a comment referencing the wa name if
there is one.
> void bdw_fbc_sw_flush(struct drm_device *dev, u32 value)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -429,6 +458,12 @@ static void intel_fbc_enable(struct drm_crtc *crtc)
> schedule_delayed_work(&work->work, msecs_to_jiffies(50));
> }
>
> +/**
> + * intel_fbc_disable - disable FBC
> + * @dev: the drm_device
> + *
> + * This function disables FBC.
> + */
> void intel_fbc_disable(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -643,6 +678,12 @@ out_disable:
> i915_gem_stolen_cleanup_compression(dev);
> }
>
> +/**
> + * intel_fbc_init - Initialize FBC
> + * @dev_priv: the i915 device
> + *
> + * This function might be called during PM init process.
> + */
> void intel_fbc_init(struct drm_i915_private *dev_priv)
> {
> if (!HAS_FBC(dev_priv)) {
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH] drm/i915: Introduce FBC DocBook.
2014-12-08 16:49 ` Daniel Vetter
@ 2014-12-08 14:46 ` Rodrigo Vivi
2014-12-08 21:48 ` [PATCH 02/11] " Rodrigo Vivi
1 sibling, 0 replies; 41+ messages in thread
From: Rodrigo Vivi @ 2014-12-08 14:46 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi
No functional changes.
v2 (Paulo): Rebase.
v3: Accept Daniel's suggestions:
* remove unclear and duplicated explanation.
* remove marketing like doc and replace by a simple one.
* remove bdw_fbc_sw_flush documentation.
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
Documentation/DocBook/drm.tmpl | 5 +++++
drivers/gpu/drm/i915/intel_fbc.c | 44 ++++++++++++++++++++++++++++++++--------
2 files changed, 40 insertions(+), 9 deletions(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 85287cb..8b780ab 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -3926,6 +3926,11 @@ int num_ioctls;</synopsis>
!Idrivers/gpu/drm/i915/intel_psr.c
</sect2>
<sect2>
+ <title>Frame Buffer Compression (FBC)</title>
+!Pdrivers/gpu/drm/i915/intel_fbc.c Frame Buffer Compression (FBC)
+!Idrivers/gpu/drm/i915/intel_fbc.c
+ </sect2>
+ <sect2>
<title>DPIO</title>
!Pdrivers/gpu/drm/i915/i915_reg.h DPIO
<table id="dpiox2">
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index f1eeb86..4daceae 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -21,20 +21,26 @@
* DEALINGS IN THE SOFTWARE.
*/
-#include "intel_drv.h"
-#include "i915_drv.h"
-
-/* FBC, or Frame Buffer Compression, is a technique employed to compress the
- * framebuffer contents in-memory, aiming at reducing the required bandwidth
- * during in-memory transfers and, therefore, reduce the power packet.
+/**
+ * DOC: Frame Buffer Compression (FBC)
+ *
+ * FBC tries to save memory bandwidth (and so power consumption) by
+ * compressing the amount of memory used by the display. It is total
+ * transparent to user space and completely handled in the kernel.
*
* The benefits of FBC are mostly visible with solid backgrounds and
- * variation-less patterns.
+ * variation-less patterns. It comes from keeping the memory footprint small
+ * and having fewer memory pages opened and accessed for refreshing the display.
*
- * FBC-related functionality can be enabled by the means of the
- * i915.i915_fbc_enable parameter
+ * i915 is responsible to reserve stolen memory for FBC and configure its
+ * offset on proper registers. The hardware takes care of all
+ * compress/decompress. However there are many known cases where we have to
+ * forcibly disable it to allow proper screen updates.
*/
+#include "intel_drv.h"
+#include "i915_drv.h"
+
static void i8xx_fbc_disable(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -318,6 +324,14 @@ static void gen7_fbc_enable(struct drm_crtc *crtc)
DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
}
+/**
+ * intel_fbc_enabled - Is FBC enabled?
+ * @dev: the drm_device
+ *
+ * This function is used to verify the current state of FBC.
+ * FIXME: This should be tracked in the plane config eventually
+ * instead of queried at runtime for most callers.
+ */
bool intel_fbc_enabled(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -429,6 +443,12 @@ static void intel_fbc_enable(struct drm_crtc *crtc)
schedule_delayed_work(&work->work, msecs_to_jiffies(50));
}
+/**
+ * intel_fbc_disable - disable FBC
+ * @dev: the drm_device
+ *
+ * This function disables FBC.
+ */
void intel_fbc_disable(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -643,6 +663,12 @@ out_disable:
i915_gem_stolen_cleanup_compression(dev);
}
+/**
+ * intel_fbc_init - Initialize FBC
+ * @dev_priv: the i915 device
+ *
+ * This function might be called during PM init process.
+ */
void intel_fbc_init(struct drm_i915_private *dev_priv)
{
if (!HAS_FBC(dev_priv)) {
--
1.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 02/11] drm/i915: Introduce FBC DocBook.
2014-12-08 16:49 ` Daniel Vetter
2014-12-08 14:46 ` [PATCH] " Rodrigo Vivi
@ 2014-12-08 21:48 ` Rodrigo Vivi
2014-12-09 9:52 ` Daniel Vetter
1 sibling, 1 reply; 41+ messages in thread
From: Rodrigo Vivi @ 2014-12-08 21:48 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Rodrigo Vivi, intel-gfx, Paulo Zanoni
On Mon, Dec 8, 2014 at 8:49 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Dec 08, 2014 at 02:09:11PM -0200, Paulo Zanoni wrote:
>> From: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>
>> No functional changes.
>>
>> v2 (Paulo): Rebase.
>>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Some suggestions to polish the documentation a bit below. I've merged
> patch 1 right away to avoid further rebase pain.
>
> Thanks, Daniel
>
>> ---
>> Documentation/DocBook/drm.tmpl | 5 ++++
>> drivers/gpu/drm/i915/intel_fbc.c | 57 ++++++++++++++++++++++++++++++++++------
>> 2 files changed, 54 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
>> index 85287cb..8b780ab 100644
>> --- a/Documentation/DocBook/drm.tmpl
>> +++ b/Documentation/DocBook/drm.tmpl
>> @@ -3926,6 +3926,11 @@ int num_ioctls;</synopsis>
>> !Idrivers/gpu/drm/i915/intel_psr.c
>> </sect2>
>> <sect2>
>> + <title>Frame Buffer Compression (FBC)</title>
>> +!Pdrivers/gpu/drm/i915/intel_fbc.c Frame Buffer Compression (FBC)
>> +!Idrivers/gpu/drm/i915/intel_fbc.c
>> + </sect2>
>> + <sect2>
>> <title>DPIO</title>
>> !Pdrivers/gpu/drm/i915/i915_reg.h DPIO
>> <table id="dpiox2">
>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
>> index f1eeb86..7686573 100644
>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>> @@ -21,20 +21,31 @@
>> * DEALINGS IN THE SOFTWARE.
>> */
>>
>> -#include "intel_drv.h"
>> -#include "i915_drv.h"
>> -
>> -/* FBC, or Frame Buffer Compression, is a technique employed to compress the
>> - * framebuffer contents in-memory, aiming at reducing the required bandwidth
>> +/**
>> + * DOC: Frame Buffer Compression (FBC)
>> + *
>> + * FBC is a technique employed to compress the framebuffer contents
>> + * in-memory, aiming at reducing the required bandwidth
>> * during in-memory transfers and, therefore, reduce the power packet.
>
> The above paragraph imo doesn't make sense. And duplicates what's below,
> so just delete it.
>> *
>> + * FBC is primarily a memory power savings technology. That is the major
>> + * benefit is to the memory power while displaying the processor graphics
>> + * information to the display. FBC works by compressing the amount of memory
>> + * used by the display. It means that it is total transparent to user space.
>
> This reads a bit too much like marketing for my taste ;-) What about:
>
> * FBC tries to save memory bandwidth (and so power consumption) by
> * compressing the amount of memory used by the display. It is total
> * transparent to user space and completely handled in the kernel.
>
>> + *
>> * The benefits of FBC are mostly visible with solid backgrounds and
>> - * variation-less patterns.
>> + * variation-less patterns. It comes from keeping the memory footprint small
>> + * and having fewer memory pages opened and accessed for refreshing the display.
>> *
>> - * FBC-related functionality can be enabled by the means of the
>> - * i915.i915_fbc_enable parameter
>> + * i915 is responsible to reserve stolen memory for FBC and configure its
>> + * offset on proper register. The hardware takes care of all
>
> ^registers
>
>> + * compress/decompress. However there are many known cases where we have to
>> + * forcibly disable it to allow proper screen updates.
>
> Mayb add "... using the frontbuffer tracking infrastructure." At least
> when that's the case ;-)
I agree, but I believe the right place is on subsequent patch that
changes to actually using frontbuffer tracking....
>
>> */
>>
>> +#include "intel_drv.h"
>> +#include "i915_drv.h"
>> +
>> static void i8xx_fbc_disable(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -318,6 +329,12 @@ static void gen7_fbc_enable(struct drm_crtc *crtc)
>> DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
>> }
>>
>> +/**
>> + * intel_fbc_enabled - Is FBC enabled?
>> + * @dev: the drm_device
>> + *
>> + * This function is used to verify the current state of FBC.
>
> This needs a FIXME: This should be tracked in the plane config eventually
> instead of queried at runtime for most callers.
>
>> + */
>> bool intel_fbc_enabled(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -325,6 +342,18 @@ bool intel_fbc_enabled(struct drm_device *dev)
>> return dev_priv->fbc.enabled;
>> }
>>
>> +/**
>> + * bdw_fbc_sw_flush - FBC Software Flush for Broadwell.
>> + * @dev: the drm_device
>> + * @value: Value to be set on MSG_FBC_REND_STATE. Possible values are
>> + * FBC_REND_NUKE and FBC_REND_CACHE_CLEAN.
>> + *
>> + * This function is needed on Broadwell to perform Nuke or Cache clean on
>> + * software side over MMIO.
>> + * On Broadwell, due a hardware bug, MSG_FBC_REND_STATE stay in a forbidden
>> + * address that has a huge risk of causing GPU Hangs if set with LRI on some
>> + * command streamers.
>> + */
>
> I guess with the frontbuffer tracking we'll just have invalidate/flush
> entry points and this becomes a static function? In that case I wouldn't
> bother documenting it - maybe do a comment referencing the wa name if
> there is one.
>
>> void bdw_fbc_sw_flush(struct drm_device *dev, u32 value)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -429,6 +458,12 @@ static void intel_fbc_enable(struct drm_crtc *crtc)
>> schedule_delayed_work(&work->work, msecs_to_jiffies(50));
>> }
>>
>> +/**
>> + * intel_fbc_disable - disable FBC
>> + * @dev: the drm_device
>> + *
>> + * This function disables FBC.
>> + */
>> void intel_fbc_disable(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -643,6 +678,12 @@ out_disable:
>> i915_gem_stolen_cleanup_compression(dev);
>> }
>>
>> +/**
>> + * intel_fbc_init - Initialize FBC
>> + * @dev_priv: the i915 device
>> + *
>> + * This function might be called during PM init process.
>> + */
>> void intel_fbc_init(struct drm_i915_private *dev_priv)
>> {
>> if (!HAS_FBC(dev_priv)) {
>> --
>> 2.1.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 02/11] drm/i915: Introduce FBC DocBook.
2014-12-08 21:48 ` [PATCH 02/11] " Rodrigo Vivi
@ 2014-12-09 9:52 ` Daniel Vetter
0 siblings, 0 replies; 41+ messages in thread
From: Daniel Vetter @ 2014-12-09 9:52 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: Rodrigo Vivi, intel-gfx, Paulo Zanoni
On Mon, Dec 08, 2014 at 01:48:01PM -0800, Rodrigo Vivi wrote:
> On Mon, Dec 8, 2014 at 8:49 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Mon, Dec 08, 2014 at 02:09:11PM -0200, Paulo Zanoni wrote:
> >> + * compress/decompress. However there are many known cases where we have to
> >> + * forcibly disable it to allow proper screen updates.
> >
> > Mayb add "... using the frontbuffer tracking infrastructure." At least
> > when that's the case ;-)
>
> I agree, but I believe the right place is on subsequent patch that
> changes to actually using frontbuffer tracking....
Make sense and thanks for the updated doc patch, merged to dinq.
Paulo, can you please make a note to add this to the patch which switches
to frontbuffer tracking?
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 03/11] drm/i915: don't try to find crtcs for FBC if it's disabled
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
2014-12-08 16:09 ` [PATCH 01/11] drm/i915: Move FBC stuff to intel_fbc.c Paulo Zanoni
2014-12-08 16:09 ` [PATCH 02/11] drm/i915: Introduce FBC DocBook Paulo Zanoni
@ 2014-12-08 16:09 ` Paulo Zanoni
2014-12-13 0:53 ` Rodrigo Vivi
2014-12-08 16:09 ` [PATCH 04/11] drm/i915: don't keep reassigning FBC_UNSUPPORTED Paulo Zanoni
` (8 subsequent siblings)
11 siblings, 1 reply; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
.. because it would be a waste of time, so move the place where the
check is done. Also, with this we won't risk printing "more than one
pipe active, disabling compression" or "no output, disabling" when FBC
is actually disabled.
This patch also represents a small behavior difference when using
i915.powersave=0: it is now exactly the same as i915.enable_fbc=0 on
this part of the code.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_fbc.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 7686573..4647d57 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -521,10 +521,16 @@ void intel_fbc_update(struct drm_device *dev)
return;
}
- if (!i915.powersave) {
+ if (i915.enable_fbc < 0) {
+ if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
+ DRM_DEBUG_KMS("disabled per chip default\n");
+ goto out_disable;
+ }
+
+ if (!i915.enable_fbc || !i915.powersave) {
if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
DRM_DEBUG_KMS("fbc disabled per module param\n");
- return;
+ goto out_disable;
}
/*
@@ -559,16 +565,6 @@ void intel_fbc_update(struct drm_device *dev)
obj = intel_fb_obj(fb);
adjusted_mode = &intel_crtc->config.adjusted_mode;
- if (i915.enable_fbc < 0) {
- if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
- DRM_DEBUG_KMS("disabled per chip default\n");
- goto out_disable;
- }
- if (!i915.enable_fbc) {
- if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
- DRM_DEBUG_KMS("fbc disabled per module param\n");
- goto out_disable;
- }
if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ||
(adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) {
if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 03/11] drm/i915: don't try to find crtcs for FBC if it's disabled
2014-12-08 16:09 ` [PATCH 03/11] drm/i915: don't try to find crtcs for FBC if it's disabled Paulo Zanoni
@ 2014-12-13 0:53 ` Rodrigo Vivi
2014-12-15 8:35 ` Daniel Vetter
0 siblings, 1 reply; 41+ messages in thread
From: Rodrigo Vivi @ 2014-12-13 0:53 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> .. because it would be a waste of time, so move the place where the
> check is done. Also, with this we won't risk printing "more than one
> pipe active, disabling compression" or "no output, disabling" when FBC
> is actually disabled.
>
> This patch also represents a small behavior difference when using
> i915.powersave=0: it is now exactly the same as i915.enable_fbc=0 on
> this part of the code.
I always ask myself if we should continue using this i915.powersave here.
I vaguely remember someone complaining when I tried to re-org this
i915.powersave making it a umbrealla for all powersaving features and
the complain was to avoid more than one variable for feature... to
avoid superset of parameters, or something like that.
Or if we keep this here shouldn't we also put this to disable psr?
Anyway this can be another patch. Daniel, please let me know what you
want with this parameters that I can change that later.
So for this patch:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_fbc.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 7686573..4647d57 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -521,10 +521,16 @@ void intel_fbc_update(struct drm_device *dev)
> return;
> }
>
> - if (!i915.powersave) {
> + if (i915.enable_fbc < 0) {
> + if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
> + DRM_DEBUG_KMS("disabled per chip default\n");
> + goto out_disable;
> + }
> +
> + if (!i915.enable_fbc || !i915.powersave) {
> if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
> DRM_DEBUG_KMS("fbc disabled per module param\n");
> - return;
> + goto out_disable;
> }
>
> /*
> @@ -559,16 +565,6 @@ void intel_fbc_update(struct drm_device *dev)
> obj = intel_fb_obj(fb);
> adjusted_mode = &intel_crtc->config.adjusted_mode;
>
> - if (i915.enable_fbc < 0) {
> - if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
> - DRM_DEBUG_KMS("disabled per chip default\n");
> - goto out_disable;
> - }
> - if (!i915.enable_fbc) {
> - if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
> - DRM_DEBUG_KMS("fbc disabled per module param\n");
> - goto out_disable;
> - }
> if ((adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) ||
> (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN)) {
> if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE))
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 03/11] drm/i915: don't try to find crtcs for FBC if it's disabled
2014-12-13 0:53 ` Rodrigo Vivi
@ 2014-12-15 8:35 ` Daniel Vetter
0 siblings, 0 replies; 41+ messages in thread
From: Daniel Vetter @ 2014-12-15 8:35 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Paulo Zanoni
On Fri, Dec 12, 2014 at 04:53:45PM -0800, Rodrigo Vivi wrote:
> On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > .. because it would be a waste of time, so move the place where the
> > check is done. Also, with this we won't risk printing "more than one
> > pipe active, disabling compression" or "no output, disabling" when FBC
> > is actually disabled.
> >
> > This patch also represents a small behavior difference when using
> > i915.powersave=0: it is now exactly the same as i915.enable_fbc=0 on
> > this part of the code.
>
> I always ask myself if we should continue using this i915.powersave here.
> I vaguely remember someone complaining when I tried to re-org this
> i915.powersave making it a umbrealla for all powersaving features and
> the complain was to avoid more than one variable for feature... to
> avoid superset of parameters, or something like that.
>
> Or if we keep this here shouldn't we also put this to disable psr?
>
> Anyway this can be another patch. Daniel, please let me know what you
> want with this parameters that I can change that later.
Imo powersave has outlived its usefulness and it can die. We've long ago
started having specific options for this. So maybe create a patch to mark
it as a debug option and then 1 kernel later kill it?
-Daneil
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 04/11] drm/i915: don't keep reassigning FBC_UNSUPPORTED
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
` (2 preceding siblings ...)
2014-12-08 16:09 ` [PATCH 03/11] drm/i915: don't try to find crtcs for FBC if it's disabled Paulo Zanoni
@ 2014-12-08 16:09 ` Paulo Zanoni
2014-12-13 0:55 ` Rodrigo Vivi
2014-12-08 16:09 ` [PATCH 05/11] drm/i915: change dev_priv->fbc.plane to dev_priv->fbc.crtc Paulo Zanoni
` (7 subsequent siblings)
11 siblings, 1 reply; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
This may save a few picoseconds on !HAS_FBC platforms. And it also
satisfies my OCD.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_fbc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 4647d57..f3d5764 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -516,10 +516,8 @@ void intel_fbc_update(struct drm_device *dev)
const struct drm_display_mode *adjusted_mode;
unsigned int max_width, max_height;
- if (!HAS_FBC(dev)) {
- set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED);
+ if (!HAS_FBC(dev))
return;
- }
if (i915.enable_fbc < 0) {
if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
@@ -684,6 +682,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
{
if (!HAS_FBC(dev_priv)) {
dev_priv->fbc.enabled = false;
+ dev_priv->fbc.no_fbc_reason = FBC_UNSUPPORTED;
return;
}
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 04/11] drm/i915: don't keep reassigning FBC_UNSUPPORTED
2014-12-08 16:09 ` [PATCH 04/11] drm/i915: don't keep reassigning FBC_UNSUPPORTED Paulo Zanoni
@ 2014-12-13 0:55 ` Rodrigo Vivi
2014-12-15 8:37 ` Daniel Vetter
0 siblings, 1 reply; 41+ messages in thread
From: Rodrigo Vivi @ 2014-12-13 0:55 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> This may save a few picoseconds on !HAS_FBC platforms. And it also
> satisfies my OCD.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_fbc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 4647d57..f3d5764 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -516,10 +516,8 @@ void intel_fbc_update(struct drm_device *dev)
> const struct drm_display_mode *adjusted_mode;
> unsigned int max_width, max_height;
>
> - if (!HAS_FBC(dev)) {
> - set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED);
I really don't like this fbc_reasons... I believe a drm_debug here is enough.
but I can try to kill that later.
For this patch: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> + if (!HAS_FBC(dev))
> return;
> - }
>
> if (i915.enable_fbc < 0) {
> if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
> @@ -684,6 +682,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
> {
> if (!HAS_FBC(dev_priv)) {
> dev_priv->fbc.enabled = false;
> + dev_priv->fbc.no_fbc_reason = FBC_UNSUPPORTED;
> return;
> }
>
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 04/11] drm/i915: don't keep reassigning FBC_UNSUPPORTED
2014-12-13 0:55 ` Rodrigo Vivi
@ 2014-12-15 8:37 ` Daniel Vetter
0 siblings, 0 replies; 41+ messages in thread
From: Daniel Vetter @ 2014-12-15 8:37 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Paulo Zanoni
On Fri, Dec 12, 2014 at 04:55:52PM -0800, Rodrigo Vivi wrote:
> On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > This may save a few picoseconds on !HAS_FBC platforms. And it also
> > satisfies my OCD.
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_fbc.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > index 4647d57..f3d5764 100644
> > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > @@ -516,10 +516,8 @@ void intel_fbc_update(struct drm_device *dev)
> > const struct drm_display_mode *adjusted_mode;
> > unsigned int max_width, max_height;
> >
> > - if (!HAS_FBC(dev)) {
> > - set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED);
>
> I really don't like this fbc_reasons... I believe a drm_debug here is enough.
> but I can try to kill that later.
Yeah, I think the original reason for no_fbc_reason was that it's to
reduce dmesg spam since we recheck for every pageflip. So horribly noisy.
Once we have all this restructured to only recheck when something changes
(at modest and when changing planes) and no longer for the on/off stuff we
do around pageflips (only needed on pre-g4x anyway) or when disabling the
primary plane we can go with dmesg debug output.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 05/11] drm/i915: change dev_priv->fbc.plane to dev_priv->fbc.crtc
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
` (3 preceding siblings ...)
2014-12-08 16:09 ` [PATCH 04/11] drm/i915: don't keep reassigning FBC_UNSUPPORTED Paulo Zanoni
@ 2014-12-08 16:09 ` Paulo Zanoni
2014-12-13 0:58 ` Rodrigo Vivi
2014-12-08 16:09 ` [PATCH 06/11] drm/i915: pass which operation triggered the frontbuffer tracking Paulo Zanoni
` (6 subsequent siblings)
11 siblings, 1 reply; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Since the mapping from CRTCs to planes is fixed, looking at the CRTC
is essentially the same as looking at the plane. Also, the next
patches wil start using the frontbuffer_bits macros, and they take the
pipe as the parameter instead of the plane, and this could differ on
gens 2 and 3.
Another nice thing is that we don't risk accidentally initializing
things to PLANE_A if we don't set the value before it is used for the
first time. But this shouldn't be a problem with the current code.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_display.c | 5 ++---
drivers/gpu/drm/i915/intel_fbc.c | 6 +++---
drivers/gpu/drm/i915/intel_sprite.c | 2 +-
4 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b5260bf..9d694f1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -689,7 +689,7 @@ struct i915_fbc {
unsigned long size;
unsigned threshold;
unsigned int fb_id;
- enum plane plane;
+ struct intel_crtc *crtc;
int y;
struct drm_mm_node compressed_fb;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e4789f4..88f3652 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4218,11 +4218,10 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_crtc->pipe;
- int plane = intel_crtc->plane;
intel_crtc_wait_for_pending_flips(crtc);
- if (dev_priv->fbc.plane == plane)
+ if (dev_priv->fbc.crtc == intel_crtc)
intel_fbc_disable(dev);
hsw_disable_ips(intel_crtc);
@@ -11813,7 +11812,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
*/
if (intel_crtc->primary_enabled &&
INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
- dev_priv->fbc.plane == intel_crtc->plane &&
+ dev_priv->fbc.crtc == intel_crtc &&
intel_plane->rotation != BIT(DRM_ROTATE_0)) {
intel_fbc_disable(dev);
}
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index f3d5764..88d00d3 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -383,7 +383,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
if (work->crtc->primary->fb == work->fb) {
dev_priv->display.enable_fbc(work->crtc);
- dev_priv->fbc.plane = to_intel_crtc(work->crtc)->plane;
+ dev_priv->fbc.crtc = to_intel_crtc(work->crtc);
dev_priv->fbc.fb_id = work->crtc->primary->fb->base.id;
dev_priv->fbc.y = work->crtc->y;
}
@@ -474,7 +474,7 @@ void intel_fbc_disable(struct drm_device *dev)
return;
dev_priv->display.disable_fbc(dev);
- dev_priv->fbc.plane = -1;
+ dev_priv->fbc.crtc = NULL;
}
static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
@@ -626,7 +626,7 @@ void intel_fbc_update(struct drm_device *dev)
* cannot be unpinned (and have its GTT offset and fence revoked)
* without first being decoupled from the scanout and FBC disabled.
*/
- if (dev_priv->fbc.plane == intel_crtc->plane &&
+ if (dev_priv->fbc.crtc == intel_crtc &&
dev_priv->fbc.fb_id == fb->base.id &&
dev_priv->fbc.y == crtc->y)
return;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index c18e57d..942daca 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1016,7 +1016,7 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
mutex_lock(&dev->struct_mutex);
- if (dev_priv->fbc.plane == intel_crtc->plane)
+ if (dev_priv->fbc.crtc == intel_crtc)
intel_fbc_disable(dev);
mutex_unlock(&dev->struct_mutex);
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 05/11] drm/i915: change dev_priv->fbc.plane to dev_priv->fbc.crtc
2014-12-08 16:09 ` [PATCH 05/11] drm/i915: change dev_priv->fbc.plane to dev_priv->fbc.crtc Paulo Zanoni
@ 2014-12-13 0:58 ` Rodrigo Vivi
0 siblings, 0 replies; 41+ messages in thread
From: Rodrigo Vivi @ 2014-12-13 0:58 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Since the mapping from CRTCs to planes is fixed, looking at the CRTC
> is essentially the same as looking at the plane. Also, the next
> patches wil start using the frontbuffer_bits macros, and they take the
> pipe as the parameter instead of the plane, and this could differ on
> gens 2 and 3.
>
> Another nice thing is that we don't risk accidentally initializing
> things to PLANE_A if we don't set the value before it is used for the
> first time. But this shouldn't be a problem with the current code.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 5 ++---
> drivers/gpu/drm/i915/intel_fbc.c | 6 +++---
> drivers/gpu/drm/i915/intel_sprite.c | 2 +-
> 4 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b5260bf..9d694f1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -689,7 +689,7 @@ struct i915_fbc {
> unsigned long size;
> unsigned threshold;
> unsigned int fb_id;
> - enum plane plane;
> + struct intel_crtc *crtc;
> int y;
>
> struct drm_mm_node compressed_fb;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e4789f4..88f3652 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4218,11 +4218,10 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> int pipe = intel_crtc->pipe;
> - int plane = intel_crtc->plane;
>
> intel_crtc_wait_for_pending_flips(crtc);
>
> - if (dev_priv->fbc.plane == plane)
> + if (dev_priv->fbc.crtc == intel_crtc)
> intel_fbc_disable(dev);
>
> hsw_disable_ips(intel_crtc);
> @@ -11813,7 +11812,7 @@ intel_commit_primary_plane(struct drm_plane *plane,
> */
> if (intel_crtc->primary_enabled &&
> INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
> - dev_priv->fbc.plane == intel_crtc->plane &&
> + dev_priv->fbc.crtc == intel_crtc &&
> intel_plane->rotation != BIT(DRM_ROTATE_0)) {
> intel_fbc_disable(dev);
> }
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index f3d5764..88d00d3 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -383,7 +383,7 @@ static void intel_fbc_work_fn(struct work_struct *__work)
> if (work->crtc->primary->fb == work->fb) {
> dev_priv->display.enable_fbc(work->crtc);
>
> - dev_priv->fbc.plane = to_intel_crtc(work->crtc)->plane;
> + dev_priv->fbc.crtc = to_intel_crtc(work->crtc);
> dev_priv->fbc.fb_id = work->crtc->primary->fb->base.id;
> dev_priv->fbc.y = work->crtc->y;
> }
> @@ -474,7 +474,7 @@ void intel_fbc_disable(struct drm_device *dev)
> return;
>
> dev_priv->display.disable_fbc(dev);
> - dev_priv->fbc.plane = -1;
> + dev_priv->fbc.crtc = NULL;
> }
>
> static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
> @@ -626,7 +626,7 @@ void intel_fbc_update(struct drm_device *dev)
> * cannot be unpinned (and have its GTT offset and fence revoked)
> * without first being decoupled from the scanout and FBC disabled.
> */
> - if (dev_priv->fbc.plane == intel_crtc->plane &&
> + if (dev_priv->fbc.crtc == intel_crtc &&
> dev_priv->fbc.fb_id == fb->base.id &&
> dev_priv->fbc.y == crtc->y)
> return;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index c18e57d..942daca 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1016,7 +1016,7 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> mutex_lock(&dev->struct_mutex);
> - if (dev_priv->fbc.plane == intel_crtc->plane)
> + if (dev_priv->fbc.crtc == intel_crtc)
> intel_fbc_disable(dev);
> mutex_unlock(&dev->struct_mutex);
>
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 06/11] drm/i915: pass which operation triggered the frontbuffer tracking
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
` (4 preceding siblings ...)
2014-12-08 16:09 ` [PATCH 05/11] drm/i915: change dev_priv->fbc.plane to dev_priv->fbc.crtc Paulo Zanoni
@ 2014-12-08 16:09 ` Paulo Zanoni
2014-12-08 16:53 ` Daniel Vetter
2014-12-08 16:09 ` [PATCH 07/11] drm/i915: also do frontbuffer tracking on pwrites Paulo Zanoni
` (5 subsequent siblings)
11 siblings, 1 reply; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
We want to port FBC to the frontbuffer tracking infrastructure, but
for that we need to know what caused the object invalidation/flush so
we can react accordingly: CPU mmaps need manual, GTT mmaps and
flips don't need handling and ring rendering needs nukes.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
drivers/gpu/drm/i915/i915_gem.c | 10 +++++-----
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h | 11 +++++++----
drivers/gpu/drm/i915/intel_frontbuffer.c | 15 ++++++++++-----
5 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9d694f1..ea3cc81 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -685,6 +685,13 @@ struct intel_context {
struct list_head link;
};
+enum fb_op_origin {
+ ORIGIN_GTT,
+ ORIGIN_CPU,
+ ORIGIN_RENDER,
+ ORIGIN_FLIP,
+};
+
struct i915_fbc {
unsigned long size;
unsigned threshold;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index de241eb..7ef12e8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2321,7 +2321,7 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
list_move_tail(&vma->mm_list, &vm->inactive_list);
}
- intel_fb_obj_flush(obj, true);
+ intel_fb_obj_flush(obj, true, ORIGIN_RENDER);
list_del_init(&obj->ring_list);
@@ -3665,7 +3665,7 @@ i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj)
old_write_domain = obj->base.write_domain;
obj->base.write_domain = 0;
- intel_fb_obj_flush(obj, false);
+ intel_fb_obj_flush(obj, false, ORIGIN_GTT);
trace_i915_gem_object_change_domain(obj,
obj->base.read_domains,
@@ -3688,7 +3688,7 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj,
old_write_domain = obj->base.write_domain;
obj->base.write_domain = 0;
- intel_fb_obj_flush(obj, false);
+ intel_fb_obj_flush(obj, false, ORIGIN_CPU);
trace_i915_gem_object_change_domain(obj,
obj->base.read_domains,
@@ -3745,7 +3745,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
}
if (write)
- intel_fb_obj_invalidate(obj, NULL);
+ intel_fb_obj_invalidate(obj, NULL, ORIGIN_GTT);
trace_i915_gem_object_change_domain(obj,
old_read_domains,
@@ -4072,7 +4072,7 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write)
}
if (write)
- intel_fb_obj_invalidate(obj, NULL);
+ intel_fb_obj_invalidate(obj, NULL, ORIGIN_CPU);
trace_i915_gem_object_change_domain(obj,
old_read_domains,
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 0c25f62..af290e6 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -965,7 +965,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
obj->dirty = 1;
i915_gem_request_assign(&obj->last_write_req, req);
- intel_fb_obj_invalidate(obj, ring);
+ intel_fb_obj_invalidate(obj, ring, ORIGIN_RENDER);
/* update for the implicit flush after a batch */
obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 588b618..633fb9a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -841,13 +841,15 @@ void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
/* intel_frontbuffer.c */
void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
- struct intel_engine_cs *ring);
+ struct intel_engine_cs *ring,
+ enum fb_op_origin origin);
void intel_frontbuffer_flip_prepare(struct drm_device *dev,
unsigned frontbuffer_bits);
void intel_frontbuffer_flip_complete(struct drm_device *dev,
unsigned frontbuffer_bits);
void intel_frontbuffer_flush(struct drm_device *dev,
- unsigned frontbuffer_bits);
+ unsigned frontbuffer_bits,
+ enum fb_op_origin origin);
/**
* intel_frontbuffer_flip - synchronous frontbuffer flip
* @dev: DRM device
@@ -863,10 +865,11 @@ static inline
void intel_frontbuffer_flip(struct drm_device *dev,
unsigned frontbuffer_bits)
{
- intel_frontbuffer_flush(dev, frontbuffer_bits);
+ intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
}
-void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire);
+void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire,
+ enum fb_op_origin origin);
/* intel_audio.c */
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 79f6d72..7bdac69 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -127,6 +127,7 @@ static void intel_mark_fb_busy(struct drm_device *dev,
* intel_fb_obj_invalidate - invalidate frontbuffer object
* @obj: GEM object to invalidate
* @ring: set for asynchronous rendering
+ * @origin: which operation caused the invalidation
*
* This function gets called every time rendering on the given object starts and
* frontbuffer caching (fbc, low refresh rate for DRRS, panel self refresh) must
@@ -135,7 +136,8 @@ static void intel_mark_fb_busy(struct drm_device *dev,
* scheduled.
*/
void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
- struct intel_engine_cs *ring)
+ struct intel_engine_cs *ring,
+ enum fb_op_origin origin)
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -163,6 +165,7 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
* intel_frontbuffer_flush - flush frontbuffer
* @dev: DRM device
* @frontbuffer_bits: frontbuffer plane tracking bits
+ * @origin: which operation caused the invalidation
*
* This function gets called every time rendering on the given planes has
* completed and frontbuffer caching can be started again. Flushes will get
@@ -171,7 +174,8 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
* Can be called without any locks held.
*/
void intel_frontbuffer_flush(struct drm_device *dev,
- unsigned frontbuffer_bits)
+ unsigned frontbuffer_bits,
+ enum fb_op_origin origin)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -199,13 +203,14 @@ void intel_frontbuffer_flush(struct drm_device *dev,
* intel_fb_obj_flush - flush frontbuffer object
* @obj: GEM object to flush
* @retire: set when retiring asynchronous rendering
+ * @origin: which operation caused the invalidation
*
* This function gets called every time rendering on the given object has
* completed and frontbuffer caching can be started again. If @retire is true
* then any delayed flushes will be unblocked.
*/
void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
- bool retire)
+ bool retire, enum fb_op_origin origin)
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -227,7 +232,7 @@ void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
mutex_unlock(&dev_priv->fb_tracking.lock);
}
- intel_frontbuffer_flush(dev, frontbuffer_bits);
+ intel_frontbuffer_flush(dev, frontbuffer_bits, origin);
}
/**
@@ -275,5 +280,5 @@ void intel_frontbuffer_flip_complete(struct drm_device *dev,
dev_priv->fb_tracking.flip_bits &= ~frontbuffer_bits;
mutex_unlock(&dev_priv->fb_tracking.lock);
- intel_frontbuffer_flush(dev, frontbuffer_bits);
+ intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
}
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 06/11] drm/i915: pass which operation triggered the frontbuffer tracking
2014-12-08 16:09 ` [PATCH 06/11] drm/i915: pass which operation triggered the frontbuffer tracking Paulo Zanoni
@ 2014-12-08 16:53 ` Daniel Vetter
2014-12-13 1:05 ` Rodrigo Vivi
0 siblings, 1 reply; 41+ messages in thread
From: Daniel Vetter @ 2014-12-08 16:53 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
On Mon, Dec 08, 2014 at 02:09:15PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> We want to port FBC to the frontbuffer tracking infrastructure, but
> for that we need to know what caused the object invalidation/flush so
> we can react accordingly: CPU mmaps need manual, GTT mmaps and
> flips don't need handling and ring rendering needs nukes.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
> drivers/gpu/drm/i915/i915_gem.c | 10 +++++-----
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 11 +++++++----
> drivers/gpu/drm/i915/intel_frontbuffer.c | 15 ++++++++++-----
> 5 files changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9d694f1..ea3cc81 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -685,6 +685,13 @@ struct intel_context {
> struct list_head link;
> };
>
> +enum fb_op_origin {
> + ORIGIN_GTT,
> + ORIGIN_CPU,
> + ORIGIN_RENDER,
Maybe GPU instead of RENDER since it includes the blitter? Render
typically only means the render ring in gem code. And maybe add an I915_
prefix or so at least to the enum.
Anyway that's it from me with bikesheds, lgtm overall.
-Daniel
> + ORIGIN_FLIP,
> +};
> +
> struct i915_fbc {
> unsigned long size;
> unsigned threshold;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index de241eb..7ef12e8 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2321,7 +2321,7 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
> list_move_tail(&vma->mm_list, &vm->inactive_list);
> }
>
> - intel_fb_obj_flush(obj, true);
> + intel_fb_obj_flush(obj, true, ORIGIN_RENDER);
>
> list_del_init(&obj->ring_list);
>
> @@ -3665,7 +3665,7 @@ i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj)
> old_write_domain = obj->base.write_domain;
> obj->base.write_domain = 0;
>
> - intel_fb_obj_flush(obj, false);
> + intel_fb_obj_flush(obj, false, ORIGIN_GTT);
>
> trace_i915_gem_object_change_domain(obj,
> obj->base.read_domains,
> @@ -3688,7 +3688,7 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj,
> old_write_domain = obj->base.write_domain;
> obj->base.write_domain = 0;
>
> - intel_fb_obj_flush(obj, false);
> + intel_fb_obj_flush(obj, false, ORIGIN_CPU);
>
> trace_i915_gem_object_change_domain(obj,
> obj->base.read_domains,
> @@ -3745,7 +3745,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
> }
>
> if (write)
> - intel_fb_obj_invalidate(obj, NULL);
> + intel_fb_obj_invalidate(obj, NULL, ORIGIN_GTT);
>
> trace_i915_gem_object_change_domain(obj,
> old_read_domains,
> @@ -4072,7 +4072,7 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write)
> }
>
> if (write)
> - intel_fb_obj_invalidate(obj, NULL);
> + intel_fb_obj_invalidate(obj, NULL, ORIGIN_CPU);
>
> trace_i915_gem_object_change_domain(obj,
> old_read_domains,
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 0c25f62..af290e6 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -965,7 +965,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
> obj->dirty = 1;
> i915_gem_request_assign(&obj->last_write_req, req);
>
> - intel_fb_obj_invalidate(obj, ring);
> + intel_fb_obj_invalidate(obj, ring, ORIGIN_RENDER);
>
> /* update for the implicit flush after a batch */
> obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 588b618..633fb9a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -841,13 +841,15 @@ void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
>
> /* intel_frontbuffer.c */
> void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> - struct intel_engine_cs *ring);
> + struct intel_engine_cs *ring,
> + enum fb_op_origin origin);
> void intel_frontbuffer_flip_prepare(struct drm_device *dev,
> unsigned frontbuffer_bits);
> void intel_frontbuffer_flip_complete(struct drm_device *dev,
> unsigned frontbuffer_bits);
> void intel_frontbuffer_flush(struct drm_device *dev,
> - unsigned frontbuffer_bits);
> + unsigned frontbuffer_bits,
> + enum fb_op_origin origin);
> /**
> * intel_frontbuffer_flip - synchronous frontbuffer flip
> * @dev: DRM device
> @@ -863,10 +865,11 @@ static inline
> void intel_frontbuffer_flip(struct drm_device *dev,
> unsigned frontbuffer_bits)
> {
> - intel_frontbuffer_flush(dev, frontbuffer_bits);
> + intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
> }
>
> -void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire);
> +void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire,
> + enum fb_op_origin origin);
>
>
> /* intel_audio.c */
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index 79f6d72..7bdac69 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -127,6 +127,7 @@ static void intel_mark_fb_busy(struct drm_device *dev,
> * intel_fb_obj_invalidate - invalidate frontbuffer object
> * @obj: GEM object to invalidate
> * @ring: set for asynchronous rendering
> + * @origin: which operation caused the invalidation
> *
> * This function gets called every time rendering on the given object starts and
> * frontbuffer caching (fbc, low refresh rate for DRRS, panel self refresh) must
> @@ -135,7 +136,8 @@ static void intel_mark_fb_busy(struct drm_device *dev,
> * scheduled.
> */
> void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> - struct intel_engine_cs *ring)
> + struct intel_engine_cs *ring,
> + enum fb_op_origin origin)
> {
> struct drm_device *dev = obj->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -163,6 +165,7 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> * intel_frontbuffer_flush - flush frontbuffer
> * @dev: DRM device
> * @frontbuffer_bits: frontbuffer plane tracking bits
> + * @origin: which operation caused the invalidation
> *
> * This function gets called every time rendering on the given planes has
> * completed and frontbuffer caching can be started again. Flushes will get
> @@ -171,7 +174,8 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> * Can be called without any locks held.
> */
> void intel_frontbuffer_flush(struct drm_device *dev,
> - unsigned frontbuffer_bits)
> + unsigned frontbuffer_bits,
> + enum fb_op_origin origin)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> @@ -199,13 +203,14 @@ void intel_frontbuffer_flush(struct drm_device *dev,
> * intel_fb_obj_flush - flush frontbuffer object
> * @obj: GEM object to flush
> * @retire: set when retiring asynchronous rendering
> + * @origin: which operation caused the invalidation
> *
> * This function gets called every time rendering on the given object has
> * completed and frontbuffer caching can be started again. If @retire is true
> * then any delayed flushes will be unblocked.
> */
> void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
> - bool retire)
> + bool retire, enum fb_op_origin origin)
> {
> struct drm_device *dev = obj->base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -227,7 +232,7 @@ void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
> mutex_unlock(&dev_priv->fb_tracking.lock);
> }
>
> - intel_frontbuffer_flush(dev, frontbuffer_bits);
> + intel_frontbuffer_flush(dev, frontbuffer_bits, origin);
> }
>
> /**
> @@ -275,5 +280,5 @@ void intel_frontbuffer_flip_complete(struct drm_device *dev,
> dev_priv->fb_tracking.flip_bits &= ~frontbuffer_bits;
> mutex_unlock(&dev_priv->fb_tracking.lock);
>
> - intel_frontbuffer_flush(dev, frontbuffer_bits);
> + intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
> }
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 06/11] drm/i915: pass which operation triggered the frontbuffer tracking
2014-12-08 16:53 ` Daniel Vetter
@ 2014-12-13 1:05 ` Rodrigo Vivi
2014-12-15 8:43 ` Daniel Vetter
0 siblings, 1 reply; 41+ messages in thread
From: Rodrigo Vivi @ 2014-12-13 1:05 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Paulo Zanoni
On Mon, Dec 8, 2014 at 8:53 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Dec 08, 2014 at 02:09:15PM -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> We want to port FBC to the frontbuffer tracking infrastructure, but
>> for that we need to know what caused the object invalidation/flush so
>> we can react accordingly: CPU mmaps need manual, GTT mmaps and
>> flips don't need handling and ring rendering needs nukes.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
>> drivers/gpu/drm/i915/i915_gem.c | 10 +++++-----
>> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
>> drivers/gpu/drm/i915/intel_drv.h | 11 +++++++----
>> drivers/gpu/drm/i915/intel_frontbuffer.c | 15 ++++++++++-----
>> 5 files changed, 30 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 9d694f1..ea3cc81 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -685,6 +685,13 @@ struct intel_context {
>> struct list_head link;
>> };
>>
>> +enum fb_op_origin {
>> + ORIGIN_GTT,
>> + ORIGIN_CPU,
>> + ORIGIN_RENDER,
>
> Maybe GPU instead of RENDER since it includes the blitter? Render
> typically only means the render ring in gem code. And maybe add an I915_
> prefix or so at least to the enum.
I agree with Daniel that RENDER isn't a good name, but also not sure
about GPU... Maybe ENGINE_CS or RINGS...
>
> Anyway that's it from me with bikesheds, lgtm overall.
I bikesheded the bikeshed! :D
Anyway lgtm as well so anyway you prefer to go feel free to use
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> -Daniel
>
>> + ORIGIN_FLIP,
>> +};
>> +
>> struct i915_fbc {
>> unsigned long size;
>> unsigned threshold;
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index de241eb..7ef12e8 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -2321,7 +2321,7 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
>> list_move_tail(&vma->mm_list, &vm->inactive_list);
>> }
>>
>> - intel_fb_obj_flush(obj, true);
>> + intel_fb_obj_flush(obj, true, ORIGIN_RENDER);
>>
>> list_del_init(&obj->ring_list);
>>
>> @@ -3665,7 +3665,7 @@ i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj)
>> old_write_domain = obj->base.write_domain;
>> obj->base.write_domain = 0;
>>
>> - intel_fb_obj_flush(obj, false);
>> + intel_fb_obj_flush(obj, false, ORIGIN_GTT);
>>
>> trace_i915_gem_object_change_domain(obj,
>> obj->base.read_domains,
>> @@ -3688,7 +3688,7 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj,
>> old_write_domain = obj->base.write_domain;
>> obj->base.write_domain = 0;
>>
>> - intel_fb_obj_flush(obj, false);
>> + intel_fb_obj_flush(obj, false, ORIGIN_CPU);
>>
>> trace_i915_gem_object_change_domain(obj,
>> obj->base.read_domains,
>> @@ -3745,7 +3745,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
>> }
>>
>> if (write)
>> - intel_fb_obj_invalidate(obj, NULL);
>> + intel_fb_obj_invalidate(obj, NULL, ORIGIN_GTT);
>>
>> trace_i915_gem_object_change_domain(obj,
>> old_read_domains,
>> @@ -4072,7 +4072,7 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write)
>> }
>>
>> if (write)
>> - intel_fb_obj_invalidate(obj, NULL);
>> + intel_fb_obj_invalidate(obj, NULL, ORIGIN_CPU);
>>
>> trace_i915_gem_object_change_domain(obj,
>> old_read_domains,
>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> index 0c25f62..af290e6 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> @@ -965,7 +965,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
>> obj->dirty = 1;
>> i915_gem_request_assign(&obj->last_write_req, req);
>>
>> - intel_fb_obj_invalidate(obj, ring);
>> + intel_fb_obj_invalidate(obj, ring, ORIGIN_RENDER);
>>
>> /* update for the implicit flush after a batch */
>> obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index 588b618..633fb9a 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -841,13 +841,15 @@ void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
>>
>> /* intel_frontbuffer.c */
>> void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
>> - struct intel_engine_cs *ring);
>> + struct intel_engine_cs *ring,
>> + enum fb_op_origin origin);
>> void intel_frontbuffer_flip_prepare(struct drm_device *dev,
>> unsigned frontbuffer_bits);
>> void intel_frontbuffer_flip_complete(struct drm_device *dev,
>> unsigned frontbuffer_bits);
>> void intel_frontbuffer_flush(struct drm_device *dev,
>> - unsigned frontbuffer_bits);
>> + unsigned frontbuffer_bits,
>> + enum fb_op_origin origin);
>> /**
>> * intel_frontbuffer_flip - synchronous frontbuffer flip
>> * @dev: DRM device
>> @@ -863,10 +865,11 @@ static inline
>> void intel_frontbuffer_flip(struct drm_device *dev,
>> unsigned frontbuffer_bits)
>> {
>> - intel_frontbuffer_flush(dev, frontbuffer_bits);
>> + intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
>> }
>>
>> -void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire);
>> +void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire,
>> + enum fb_op_origin origin);
>>
>>
>> /* intel_audio.c */
>> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
>> index 79f6d72..7bdac69 100644
>> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
>> @@ -127,6 +127,7 @@ static void intel_mark_fb_busy(struct drm_device *dev,
>> * intel_fb_obj_invalidate - invalidate frontbuffer object
>> * @obj: GEM object to invalidate
>> * @ring: set for asynchronous rendering
>> + * @origin: which operation caused the invalidation
>> *
>> * This function gets called every time rendering on the given object starts and
>> * frontbuffer caching (fbc, low refresh rate for DRRS, panel self refresh) must
>> @@ -135,7 +136,8 @@ static void intel_mark_fb_busy(struct drm_device *dev,
>> * scheduled.
>> */
>> void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
>> - struct intel_engine_cs *ring)
>> + struct intel_engine_cs *ring,
>> + enum fb_op_origin origin)
>> {
>> struct drm_device *dev = obj->base.dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -163,6 +165,7 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
>> * intel_frontbuffer_flush - flush frontbuffer
>> * @dev: DRM device
>> * @frontbuffer_bits: frontbuffer plane tracking bits
>> + * @origin: which operation caused the invalidation
>> *
>> * This function gets called every time rendering on the given planes has
>> * completed and frontbuffer caching can be started again. Flushes will get
>> @@ -171,7 +174,8 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
>> * Can be called without any locks held.
>> */
>> void intel_frontbuffer_flush(struct drm_device *dev,
>> - unsigned frontbuffer_bits)
>> + unsigned frontbuffer_bits,
>> + enum fb_op_origin origin)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>>
>> @@ -199,13 +203,14 @@ void intel_frontbuffer_flush(struct drm_device *dev,
>> * intel_fb_obj_flush - flush frontbuffer object
>> * @obj: GEM object to flush
>> * @retire: set when retiring asynchronous rendering
>> + * @origin: which operation caused the invalidation
>> *
>> * This function gets called every time rendering on the given object has
>> * completed and frontbuffer caching can be started again. If @retire is true
>> * then any delayed flushes will be unblocked.
>> */
>> void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
>> - bool retire)
>> + bool retire, enum fb_op_origin origin)
>> {
>> struct drm_device *dev = obj->base.dev;
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> @@ -227,7 +232,7 @@ void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
>> mutex_unlock(&dev_priv->fb_tracking.lock);
>> }
>>
>> - intel_frontbuffer_flush(dev, frontbuffer_bits);
>> + intel_frontbuffer_flush(dev, frontbuffer_bits, origin);
>> }
>>
>> /**
>> @@ -275,5 +280,5 @@ void intel_frontbuffer_flip_complete(struct drm_device *dev,
>> dev_priv->fb_tracking.flip_bits &= ~frontbuffer_bits;
>> mutex_unlock(&dev_priv->fb_tracking.lock);
>>
>> - intel_frontbuffer_flush(dev, frontbuffer_bits);
>> + intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
>> }
>> --
>> 2.1.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 06/11] drm/i915: pass which operation triggered the frontbuffer tracking
2014-12-13 1:05 ` Rodrigo Vivi
@ 2014-12-15 8:43 ` Daniel Vetter
2014-12-15 13:58 ` Paulo Zanoni
0 siblings, 1 reply; 41+ messages in thread
From: Daniel Vetter @ 2014-12-15 8:43 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Paulo Zanoni
On Fri, Dec 12, 2014 at 05:05:41PM -0800, Rodrigo Vivi wrote:
> On Mon, Dec 8, 2014 at 8:53 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Mon, Dec 08, 2014 at 02:09:15PM -0200, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> We want to port FBC to the frontbuffer tracking infrastructure, but
> >> for that we need to know what caused the object invalidation/flush so
> >> we can react accordingly: CPU mmaps need manual, GTT mmaps and
> >> flips don't need handling and ring rendering needs nukes.
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
> >> drivers/gpu/drm/i915/i915_gem.c | 10 +++++-----
> >> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
> >> drivers/gpu/drm/i915/intel_drv.h | 11 +++++++----
> >> drivers/gpu/drm/i915/intel_frontbuffer.c | 15 ++++++++++-----
> >> 5 files changed, 30 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >> index 9d694f1..ea3cc81 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -685,6 +685,13 @@ struct intel_context {
> >> struct list_head link;
> >> };
> >>
> >> +enum fb_op_origin {
> >> + ORIGIN_GTT,
> >> + ORIGIN_CPU,
> >> + ORIGIN_RENDER,
> >
> > Maybe GPU instead of RENDER since it includes the blitter? Render
> > typically only means the render ring in gem code. And maybe add an I915_
> > prefix or so at least to the enum.
>
> I agree with Daniel that RENDER isn't a good name, but also not sure
> about GPU... Maybe ENGINE_CS or RINGS...
ORIGIN_CS sounds good to me. ENGINE is a bit misleading since it's for all
engines - usually we use engine only when we mean a specific one. Paulo,
can you please pick one of the color suggestions and then please resend?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 06/11] drm/i915: pass which operation triggered the frontbuffer tracking
2014-12-15 8:43 ` Daniel Vetter
@ 2014-12-15 13:58 ` Paulo Zanoni
0 siblings, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-15 13:58 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
We want to port FBC to the frontbuffer tracking infrastructure, but
for that we need to know what caused the object invalidation/flush so
we can react accordingly: CPU mmaps need manual, GTT mmaps and
flips don't need handling and ring rendering needs nukes.
v2: - s/ORIGIN_RENDER/ORIGIN_CS/ (Daniel, Rodrigo)
- Fix copy/pasted wrong documentation
- Rebase
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
drivers/gpu/drm/i915/i915_gem.c | 10 +++++-----
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
drivers/gpu/drm/i915/intel_drv.h | 11 +++++++----
drivers/gpu/drm/i915/intel_frontbuffer.c | 15 ++++++++++-----
5 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 09c546c..5de5c77 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -697,6 +697,13 @@ struct intel_context {
struct list_head link;
};
+enum fb_op_origin {
+ ORIGIN_GTT,
+ ORIGIN_CPU,
+ ORIGIN_CS,
+ ORIGIN_FLIP,
+};
+
struct i915_fbc {
unsigned long size;
unsigned threshold;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f0a6a4a..c943748 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2318,7 +2318,7 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj)
list_move_tail(&vma->mm_list, &vma->vm->inactive_list);
}
- intel_fb_obj_flush(obj, true);
+ intel_fb_obj_flush(obj, true, ORIGIN_CS);
list_del_init(&obj->ring_list);
@@ -3676,7 +3676,7 @@ i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj)
old_write_domain = obj->base.write_domain;
obj->base.write_domain = 0;
- intel_fb_obj_flush(obj, false);
+ intel_fb_obj_flush(obj, false, ORIGIN_GTT);
trace_i915_gem_object_change_domain(obj,
obj->base.read_domains,
@@ -3699,7 +3699,7 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj,
old_write_domain = obj->base.write_domain;
obj->base.write_domain = 0;
- intel_fb_obj_flush(obj, false);
+ intel_fb_obj_flush(obj, false, ORIGIN_CPU);
trace_i915_gem_object_change_domain(obj,
obj->base.read_domains,
@@ -3756,7 +3756,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
}
if (write)
- intel_fb_obj_invalidate(obj, NULL);
+ intel_fb_obj_invalidate(obj, NULL, ORIGIN_GTT);
trace_i915_gem_object_change_domain(obj,
old_read_domains,
@@ -4086,7 +4086,7 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write)
}
if (write)
- intel_fb_obj_invalidate(obj, NULL);
+ intel_fb_obj_invalidate(obj, NULL, ORIGIN_CPU);
trace_i915_gem_object_change_domain(obj,
old_read_domains,
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3927d93..4708c17 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -968,7 +968,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
obj->dirty = 1;
i915_gem_request_assign(&obj->last_write_req, req);
- intel_fb_obj_invalidate(obj, ring);
+ intel_fb_obj_invalidate(obj, ring, ORIGIN_CS);
/* update for the implicit flush after a batch */
obj->base.write_domain &= ~I915_GEM_GPU_DOMAINS;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 588b618..633fb9a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -841,13 +841,15 @@ void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
/* intel_frontbuffer.c */
void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
- struct intel_engine_cs *ring);
+ struct intel_engine_cs *ring,
+ enum fb_op_origin origin);
void intel_frontbuffer_flip_prepare(struct drm_device *dev,
unsigned frontbuffer_bits);
void intel_frontbuffer_flip_complete(struct drm_device *dev,
unsigned frontbuffer_bits);
void intel_frontbuffer_flush(struct drm_device *dev,
- unsigned frontbuffer_bits);
+ unsigned frontbuffer_bits,
+ enum fb_op_origin origin);
/**
* intel_frontbuffer_flip - synchronous frontbuffer flip
* @dev: DRM device
@@ -863,10 +865,11 @@ static inline
void intel_frontbuffer_flip(struct drm_device *dev,
unsigned frontbuffer_bits)
{
- intel_frontbuffer_flush(dev, frontbuffer_bits);
+ intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
}
-void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire);
+void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire,
+ enum fb_op_origin origin);
/* intel_audio.c */
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 79f6d72..3d35f31 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -127,6 +127,7 @@ static void intel_mark_fb_busy(struct drm_device *dev,
* intel_fb_obj_invalidate - invalidate frontbuffer object
* @obj: GEM object to invalidate
* @ring: set for asynchronous rendering
+ * @origin: which operation caused the invalidation
*
* This function gets called every time rendering on the given object starts and
* frontbuffer caching (fbc, low refresh rate for DRRS, panel self refresh) must
@@ -135,7 +136,8 @@ static void intel_mark_fb_busy(struct drm_device *dev,
* scheduled.
*/
void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
- struct intel_engine_cs *ring)
+ struct intel_engine_cs *ring,
+ enum fb_op_origin origin)
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -163,6 +165,7 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
* intel_frontbuffer_flush - flush frontbuffer
* @dev: DRM device
* @frontbuffer_bits: frontbuffer plane tracking bits
+ * @origin: which operation caused the flush
*
* This function gets called every time rendering on the given planes has
* completed and frontbuffer caching can be started again. Flushes will get
@@ -171,7 +174,8 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
* Can be called without any locks held.
*/
void intel_frontbuffer_flush(struct drm_device *dev,
- unsigned frontbuffer_bits)
+ unsigned frontbuffer_bits,
+ enum fb_op_origin origin)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -199,13 +203,14 @@ void intel_frontbuffer_flush(struct drm_device *dev,
* intel_fb_obj_flush - flush frontbuffer object
* @obj: GEM object to flush
* @retire: set when retiring asynchronous rendering
+ * @origin: which operation caused the flush
*
* This function gets called every time rendering on the given object has
* completed and frontbuffer caching can be started again. If @retire is true
* then any delayed flushes will be unblocked.
*/
void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
- bool retire)
+ bool retire, enum fb_op_origin origin)
{
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -227,7 +232,7 @@ void intel_fb_obj_flush(struct drm_i915_gem_object *obj,
mutex_unlock(&dev_priv->fb_tracking.lock);
}
- intel_frontbuffer_flush(dev, frontbuffer_bits);
+ intel_frontbuffer_flush(dev, frontbuffer_bits, origin);
}
/**
@@ -275,5 +280,5 @@ void intel_frontbuffer_flip_complete(struct drm_device *dev,
dev_priv->fb_tracking.flip_bits &= ~frontbuffer_bits;
mutex_unlock(&dev_priv->fb_tracking.lock);
- intel_frontbuffer_flush(dev, frontbuffer_bits);
+ intel_frontbuffer_flush(dev, frontbuffer_bits, ORIGIN_FLIP);
}
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 07/11] drm/i915: also do frontbuffer tracking on pwrites
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
` (5 preceding siblings ...)
2014-12-08 16:09 ` [PATCH 06/11] drm/i915: pass which operation triggered the frontbuffer tracking Paulo Zanoni
@ 2014-12-08 16:09 ` Paulo Zanoni
2014-12-08 16:55 ` Daniel Vetter
2014-12-08 16:09 ` [PATCH 08/11] drm/i915: add fronbuffer tracking to FBC Paulo Zanoni
` (4 subsequent siblings)
11 siblings, 1 reply; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
We need this for FBC, and possibly for PSR too.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7ef12e8..b8c906e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1117,6 +1117,10 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
ret = i915_gem_phys_pwrite(obj, args, file);
else
ret = i915_gem_shmem_pwrite(dev, obj, args, file);
+
+ intel_fb_obj_flush(obj, false, ORIGIN_CPU);
+ } else {
+ intel_fb_obj_flush(obj, false, ORIGIN_GTT);
}
out:
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 07/11] drm/i915: also do frontbuffer tracking on pwrites
2014-12-08 16:09 ` [PATCH 07/11] drm/i915: also do frontbuffer tracking on pwrites Paulo Zanoni
@ 2014-12-08 16:55 ` Daniel Vetter
2014-12-13 1:10 ` Rodrigo Vivi
0 siblings, 1 reply; 41+ messages in thread
From: Daniel Vetter @ 2014-12-08 16:55 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
On Mon, Dec 08, 2014 at 02:09:16PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> We need this for FBC, and possibly for PSR too.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Hm, this might fix some cursor update failures if userspace only pwrites
and doesn't follow suite with a cursor ioctl update call. Not sure this
ever happens, but definitely makes sense.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 7ef12e8..b8c906e 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1117,6 +1117,10 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
> ret = i915_gem_phys_pwrite(obj, args, file);
> else
> ret = i915_gem_shmem_pwrite(dev, obj, args, file);
> +
> + intel_fb_obj_flush(obj, false, ORIGIN_CPU);
> + } else {
> + intel_fb_obj_flush(obj, false, ORIGIN_GTT);
> }
>
> out:
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 07/11] drm/i915: also do frontbuffer tracking on pwrites
2014-12-08 16:55 ` Daniel Vetter
@ 2014-12-13 1:10 ` Rodrigo Vivi
2014-12-15 8:39 ` Daniel Vetter
0 siblings, 1 reply; 41+ messages in thread
From: Rodrigo Vivi @ 2014-12-13 1:10 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Paulo Zanoni
wondering if this fixes: https://bugs.freedesktop.org/show_bug.cgi?id=87143
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
On Mon, Dec 8, 2014 at 8:55 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Dec 08, 2014 at 02:09:16PM -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> We need this for FBC, and possibly for PSR too.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Hm, this might fix some cursor update failures if userspace only pwrites
> and doesn't follow suite with a cursor ioctl update call. Not sure this
> ever happens, but definitely makes sense.
> -Daniel
>
>> ---
>> drivers/gpu/drm/i915/i915_gem.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 7ef12e8..b8c906e 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -1117,6 +1117,10 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
>> ret = i915_gem_phys_pwrite(obj, args, file);
>> else
>> ret = i915_gem_shmem_pwrite(dev, obj, args, file);
>> +
>> + intel_fb_obj_flush(obj, false, ORIGIN_CPU);
>> + } else {
>> + intel_fb_obj_flush(obj, false, ORIGIN_GTT);
>> }
>>
>> out:
>> --
>> 2.1.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 08/11] drm/i915: add fronbuffer tracking to FBC
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
` (6 preceding siblings ...)
2014-12-08 16:09 ` [PATCH 07/11] drm/i915: also do frontbuffer tracking on pwrites Paulo Zanoni
@ 2014-12-08 16:09 ` Paulo Zanoni
2014-12-13 1:12 ` [PATCH 6/9] drm/i915: add frontbuffer " Rodrigo Vivi
2014-12-13 1:16 ` [PATCH 08/11] drm/i915: add fronbuffer " Rodrigo Vivi
2014-12-08 16:09 ` [PATCH 09/11] drm/i915: extract intel_fbc_find_crtc() Paulo Zanoni
` (3 subsequent siblings)
11 siblings, 2 replies; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Kill the blt/render tracking we currently have and use the frontbuffer
tracking infrastructure.
Don't enable things by default yet.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 9 +--
drivers/gpu/drm/i915/intel_drv.h | 6 +-
drivers/gpu/drm/i915/intel_fbc.c | 119 ++++++++++++++++++-------------
drivers/gpu/drm/i915/intel_frontbuffer.c | 14 +---
drivers/gpu/drm/i915/intel_ringbuffer.c | 41 +----------
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
6 files changed, 80 insertions(+), 110 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ea3cc81..22285c2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -696,6 +696,7 @@ struct i915_fbc {
unsigned long size;
unsigned threshold;
unsigned int fb_id;
+ unsigned int possible_framebuffer_bits;
struct intel_crtc *crtc;
int y;
@@ -708,14 +709,6 @@ struct i915_fbc {
* possible. */
bool enabled;
- /* On gen8 some rings cannont perform fbc clean operation so for now
- * we are doing this on SW with mmio.
- * This variable works in the opposite information direction
- * of ring->fbc_dirty telling software on frontbuffer tracking
- * to perform the cache clean on sw side.
- */
- bool need_sw_cache_clean;
-
struct intel_fbc_work {
struct delayed_work work;
struct drm_crtc *crtc;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 633fb9a..3a1c3d82 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1071,7 +1071,11 @@ bool intel_fbc_enabled(struct drm_device *dev);
void intel_fbc_update(struct drm_device *dev);
void intel_fbc_init(struct drm_i915_private *dev_priv);
void intel_fbc_disable(struct drm_device *dev);
-void bdw_fbc_sw_flush(struct drm_device *dev, u32 value);
+void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits,
+ enum fb_op_origin origin);
+void intel_fbc_flush(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits, enum fb_op_origin origin);
/* intel_hdmi.c */
void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 88d00d3..03ba43d 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -178,29 +178,10 @@ static bool g4x_fbc_enabled(struct drm_device *dev)
return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN;
}
-static void snb_fbc_blit_update(struct drm_device *dev)
+static void intel_fbc_nuke(struct drm_i915_private *dev_priv)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
- u32 blt_ecoskpd;
-
- /* Make sure blitter notifies FBC of writes */
-
- /* Blitter is part of Media powerwell on VLV. No impact of
- * his param in other platforms for now */
- gen6_gt_force_wake_get(dev_priv, FORCEWAKE_MEDIA);
-
- blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
- blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY <<
- GEN6_BLITTER_LOCK_SHIFT;
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- blt_ecoskpd &= ~(GEN6_BLITTER_FBC_NOTIFY <<
- GEN6_BLITTER_LOCK_SHIFT);
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- POSTING_READ(GEN6_BLITTER_ECOSKPD);
-
- gen6_gt_force_wake_put(dev_priv, FORCEWAKE_MEDIA);
+ I915_WRITE(MSG_FBC_REND_STATE, FBC_REND_NUKE);
+ POSTING_READ(MSG_FBC_REND_STATE);
}
static void ilk_fbc_enable(struct drm_crtc *crtc)
@@ -243,9 +224,10 @@ static void ilk_fbc_enable(struct drm_crtc *crtc)
I915_WRITE(SNB_DPFC_CTL_SA,
SNB_CPU_FENCE_ENABLE | obj->fence_reg);
I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
- snb_fbc_blit_update(dev);
}
+ intel_fbc_nuke(dev_priv);
+
DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
}
@@ -324,7 +306,7 @@ static void gen7_fbc_enable(struct drm_crtc *crtc)
SNB_CPU_FENCE_ENABLE | obj->fence_reg);
I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
- snb_fbc_blit_update(dev);
+ intel_fbc_nuke(dev_priv);
DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
}
@@ -342,31 +324,6 @@ bool intel_fbc_enabled(struct drm_device *dev)
return dev_priv->fbc.enabled;
}
-/**
- * bdw_fbc_sw_flush - FBC Software Flush for Broadwell.
- * @dev: the drm_device
- * @value: Value to be set on MSG_FBC_REND_STATE. Possible values are
- * FBC_REND_NUKE and FBC_REND_CACHE_CLEAN.
- *
- * This function is needed on Broadwell to perform Nuke or Cache clean on
- * software side over MMIO.
- * On Broadwell, due a hardware bug, MSG_FBC_REND_STATE stay in a forbidden
- * address that has a huge risk of causing GPU Hangs if set with LRI on some
- * command streamers.
- */
-void bdw_fbc_sw_flush(struct drm_device *dev, u32 value)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- if (!IS_GEN8(dev))
- return;
-
- if (!intel_fbc_enabled(dev))
- return;
-
- I915_WRITE(MSG_FBC_REND_STATE, value);
-}
-
static void intel_fbc_work_fn(struct work_struct *__work)
{
struct intel_fbc_work *work =
@@ -672,6 +629,63 @@ out_disable:
i915_gem_stolen_cleanup_compression(dev);
}
+void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits,
+ enum fb_op_origin origin)
+{
+ struct drm_device *dev = dev_priv->dev;
+ unsigned int fbc_bits;
+
+ if (origin == ORIGIN_GTT)
+ return;
+
+ if (dev_priv->fbc.enabled)
+ fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe);
+ else if (dev_priv->fbc.fbc_work)
+ fbc_bits = INTEL_FRONTBUFFER_PRIMARY(
+ to_intel_crtc(dev_priv->fbc.fbc_work->crtc)->pipe);
+ else
+ fbc_bits = dev_priv->fbc.possible_framebuffer_bits;
+
+ if ((fbc_bits & frontbuffer_bits) == 0)
+ return;
+
+ intel_fbc_disable(dev);
+}
+
+void intel_fbc_flush(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits,
+ enum fb_op_origin origin)
+{
+ struct drm_device *dev = dev_priv->dev;
+ unsigned int fbc_bits = 0;
+ bool fbc_enabled;
+
+ if (dev_priv->fbc.enabled)
+ fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe);
+ else
+ fbc_bits = dev_priv->fbc.possible_framebuffer_bits;
+
+ /* These are not the planes you are looking for. */
+ if ((frontbuffer_bits & fbc_bits) == 0)
+ return;
+
+ fbc_enabled = intel_fbc_enabled(dev);
+
+ if ((dev_priv->fb_tracking.busy_bits & frontbuffer_bits &
+ fbc_bits) == 0) {
+ if (fbc_enabled) {
+ if (origin == ORIGIN_RENDER || origin == ORIGIN_CPU)
+ intel_fbc_nuke(dev_priv);
+ } else {
+ intel_fbc_update(dev);
+ }
+ } else {
+ if (fbc_enabled)
+ intel_fbc_disable(dev);
+ }
+}
+
/**
* intel_fbc_init - Initialize FBC
* @dev_priv: the i915 device
@@ -680,12 +694,19 @@ out_disable:
*/
void intel_fbc_init(struct drm_i915_private *dev_priv)
{
+ enum pipe pipe;
+
if (!HAS_FBC(dev_priv)) {
dev_priv->fbc.enabled = false;
dev_priv->fbc.no_fbc_reason = FBC_UNSUPPORTED;
return;
}
+ /* TODO: some platforms have FBC tied to a specific plane! */
+ for_each_pipe(dev_priv, pipe)
+ dev_priv->fbc.possible_framebuffer_bits |=
+ INTEL_FRONTBUFFER_PRIMARY(pipe);
+
if (INTEL_INFO(dev_priv)->gen >= 7) {
dev_priv->display.fbc_enabled = ilk_fbc_enabled;
dev_priv->display.enable_fbc = gen7_fbc_enable;
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 7bdac69..47b3409 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -118,8 +118,6 @@ static void intel_mark_fb_busy(struct drm_device *dev,
continue;
intel_increase_pllclock(dev, pipe);
- if (ring && intel_fbc_enabled(dev))
- ring->fbc_dirty = true;
}
}
@@ -159,6 +157,7 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);
intel_psr_invalidate(dev, obj->frontbuffer_bits);
+ intel_fbc_invalidate(dev_priv, obj->frontbuffer_bits, origin);
}
/**
@@ -187,16 +186,7 @@ void intel_frontbuffer_flush(struct drm_device *dev,
intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
intel_psr_flush(dev, frontbuffer_bits);
-
- /*
- * FIXME: Unconditional fbc flushing here is a rather gross hack and
- * needs to be reworked into a proper frontbuffer tracking scheme like
- * psr employs.
- */
- if (dev_priv->fbc.need_sw_cache_clean) {
- dev_priv->fbc.need_sw_cache_clean = false;
- bdw_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN);
- }
+ intel_fbc_flush(dev_priv, frontbuffer_bits, origin);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 83accb7..5982da9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -317,29 +317,6 @@ gen7_render_ring_cs_stall_wa(struct intel_engine_cs *ring)
return 0;
}
-static int gen7_ring_fbc_flush(struct intel_engine_cs *ring, u32 value)
-{
- int ret;
-
- if (!ring->fbc_dirty)
- return 0;
-
- ret = intel_ring_begin(ring, 6);
- if (ret)
- return ret;
- /* WaFbcNukeOn3DBlt:ivb/hsw */
- intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
- intel_ring_emit(ring, MSG_FBC_REND_STATE);
- intel_ring_emit(ring, value);
- intel_ring_emit(ring, MI_STORE_REGISTER_MEM(1) | MI_SRM_LRM_GLOBAL_GTT);
- intel_ring_emit(ring, MSG_FBC_REND_STATE);
- intel_ring_emit(ring, ring->scratch.gtt_offset + 256);
- intel_ring_advance(ring);
-
- ring->fbc_dirty = false;
- return 0;
-}
-
static int
gen7_render_ring_flush(struct intel_engine_cs *ring,
u32 invalidate_domains, u32 flush_domains)
@@ -395,9 +372,6 @@ gen7_render_ring_flush(struct intel_engine_cs *ring,
intel_ring_emit(ring, 0);
intel_ring_advance(ring);
- if (!invalidate_domains && flush_domains)
- return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
-
return 0;
}
@@ -459,9 +433,6 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
if (ret)
return ret;
- if (!invalidate_domains && flush_domains)
- return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
-
return 0;
}
@@ -2279,7 +2250,6 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
u32 invalidate, u32 flush)
{
struct drm_device *dev = ring->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t cmd;
int ret;
@@ -2288,7 +2258,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
return ret;
cmd = MI_FLUSH_DW;
- if (INTEL_INFO(ring->dev)->gen >= 8)
+ if (INTEL_INFO(dev)->gen >= 8)
cmd += 1;
/*
* Bspec vol 1c.3 - blitter engine command streamer:
@@ -2301,7 +2271,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
MI_FLUSH_DW_OP_STOREDW;
intel_ring_emit(ring, cmd);
intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT);
- if (INTEL_INFO(ring->dev)->gen >= 8) {
+ if (INTEL_INFO(dev)->gen >= 8) {
intel_ring_emit(ring, 0); /* upper addr */
intel_ring_emit(ring, 0); /* value */
} else {
@@ -2310,13 +2280,6 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
}
intel_ring_advance(ring);
- if (!invalidate && flush) {
- if (IS_GEN7(dev))
- return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
- else if (IS_BROADWELL(dev))
- dev_priv->fbc.need_sw_cache_clean = true;
- }
-
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 6dbb6f4..7e31ec8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -269,7 +269,6 @@ struct intel_engine_cs {
*/
struct drm_i915_gem_request *outstanding_lazy_request;
bool gpu_caches_dirty;
- bool fbc_dirty;
wait_queue_head_t irq_queue;
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH 6/9] drm/i915: add frontbuffer tracking to FBC
2014-12-08 16:09 ` [PATCH 08/11] drm/i915: add fronbuffer tracking to FBC Paulo Zanoni
@ 2014-12-13 1:12 ` Rodrigo Vivi
2014-12-13 1:16 ` [PATCH 08/11] drm/i915: add fronbuffer " Rodrigo Vivi
1 sibling, 0 replies; 41+ messages in thread
From: Rodrigo Vivi @ 2014-12-13 1:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Kill the blt/render tracking we currently have and use the frontbuffer
tracking infrastructure.
Don't enable things by default yet.
v2: (Rodrigo) Fix small conflict on rebase and typo at subject.
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 9 +--
drivers/gpu/drm/i915/intel_drv.h | 6 +-
drivers/gpu/drm/i915/intel_fbc.c | 107 ++++++++++++++++++++-----------
drivers/gpu/drm/i915/intel_frontbuffer.c | 14 +---
drivers/gpu/drm/i915/intel_ringbuffer.c | 41 +-----------
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
6 files changed, 80 insertions(+), 98 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9efeec2..11a492a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -708,6 +708,7 @@ struct i915_fbc {
unsigned long size;
unsigned threshold;
unsigned int fb_id;
+ unsigned int possible_framebuffer_bits;
struct intel_crtc *crtc;
int y;
@@ -720,14 +721,6 @@ struct i915_fbc {
* possible. */
bool enabled;
- /* On gen8 some rings cannont perform fbc clean operation so for now
- * we are doing this on SW with mmio.
- * This variable works in the opposite information direction
- * of ring->fbc_dirty telling software on frontbuffer tracking
- * to perform the cache clean on sw side.
- */
- bool need_sw_cache_clean;
-
struct intel_fbc_work {
struct delayed_work work;
struct drm_crtc *crtc;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 633fb9a..3a1c3d8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1071,7 +1071,11 @@ bool intel_fbc_enabled(struct drm_device *dev);
void intel_fbc_update(struct drm_device *dev);
void intel_fbc_init(struct drm_i915_private *dev_priv);
void intel_fbc_disable(struct drm_device *dev);
-void bdw_fbc_sw_flush(struct drm_device *dev, u32 value);
+void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits,
+ enum fb_op_origin origin);
+void intel_fbc_flush(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits, enum fb_op_origin origin);
/* intel_hdmi.c */
void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index f658dc0..303470c 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -173,29 +173,10 @@ static bool g4x_fbc_enabled(struct drm_device *dev)
return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN;
}
-static void snb_fbc_blit_update(struct drm_device *dev)
+static void intel_fbc_nuke(struct drm_i915_private *dev_priv)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
- u32 blt_ecoskpd;
-
- /* Make sure blitter notifies FBC of writes */
-
- /* Blitter is part of Media powerwell on VLV. No impact of
- * his param in other platforms for now */
- gen6_gt_force_wake_get(dev_priv, FORCEWAKE_MEDIA);
-
- blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
- blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY <<
- GEN6_BLITTER_LOCK_SHIFT;
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- blt_ecoskpd &= ~(GEN6_BLITTER_FBC_NOTIFY <<
- GEN6_BLITTER_LOCK_SHIFT);
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- POSTING_READ(GEN6_BLITTER_ECOSKPD);
-
- gen6_gt_force_wake_put(dev_priv, FORCEWAKE_MEDIA);
+ I915_WRITE(MSG_FBC_REND_STATE, FBC_REND_NUKE);
+ POSTING_READ(MSG_FBC_REND_STATE);
}
static void ilk_fbc_enable(struct drm_crtc *crtc)
@@ -238,9 +219,10 @@ static void ilk_fbc_enable(struct drm_crtc *crtc)
I915_WRITE(SNB_DPFC_CTL_SA,
SNB_CPU_FENCE_ENABLE | obj->fence_reg);
I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
- snb_fbc_blit_update(dev);
}
+ intel_fbc_nuke(dev_priv);
+
DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
}
@@ -319,7 +301,7 @@ static void gen7_fbc_enable(struct drm_crtc *crtc)
SNB_CPU_FENCE_ENABLE | obj->fence_reg);
I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
- snb_fbc_blit_update(dev);
+ intel_fbc_nuke(dev_priv);
DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
}
@@ -339,19 +321,6 @@ bool intel_fbc_enabled(struct drm_device *dev)
return dev_priv->fbc.enabled;
}
-void bdw_fbc_sw_flush(struct drm_device *dev, u32 value)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- if (!IS_GEN8(dev))
- return;
-
- if (!intel_fbc_enabled(dev))
- return;
-
- I915_WRITE(MSG_FBC_REND_STATE, value);
-}
-
static void intel_fbc_work_fn(struct work_struct *__work)
{
struct intel_fbc_work *work =
@@ -657,6 +626,63 @@ out_disable:
i915_gem_stolen_cleanup_compression(dev);
}
+void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits,
+ enum fb_op_origin origin)
+{
+ struct drm_device *dev = dev_priv->dev;
+ unsigned int fbc_bits;
+
+ if (origin == ORIGIN_GTT)
+ return;
+
+ if (dev_priv->fbc.enabled)
+ fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe);
+ else if (dev_priv->fbc.fbc_work)
+ fbc_bits = INTEL_FRONTBUFFER_PRIMARY(
+ to_intel_crtc(dev_priv->fbc.fbc_work->crtc)->pipe);
+ else
+ fbc_bits = dev_priv->fbc.possible_framebuffer_bits;
+
+ if ((fbc_bits & frontbuffer_bits) == 0)
+ return;
+
+ intel_fbc_disable(dev);
+}
+
+void intel_fbc_flush(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits,
+ enum fb_op_origin origin)
+{
+ struct drm_device *dev = dev_priv->dev;
+ unsigned int fbc_bits = 0;
+ bool fbc_enabled;
+
+ if (dev_priv->fbc.enabled)
+ fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe);
+ else
+ fbc_bits = dev_priv->fbc.possible_framebuffer_bits;
+
+ /* These are not the planes you are looking for. */
+ if ((frontbuffer_bits & fbc_bits) == 0)
+ return;
+
+ fbc_enabled = intel_fbc_enabled(dev);
+
+ if ((dev_priv->fb_tracking.busy_bits & frontbuffer_bits &
+ fbc_bits) == 0) {
+ if (fbc_enabled) {
+ if (origin == ORIGIN_RENDER || origin == ORIGIN_CPU)
+ intel_fbc_nuke(dev_priv);
+ } else {
+ intel_fbc_update(dev);
+ }
+ } else {
+ if (fbc_enabled)
+ intel_fbc_disable(dev);
+ }
+}
+
/**
* intel_fbc_init - Initialize FBC
* @dev_priv: the i915 device
@@ -665,12 +691,19 @@ out_disable:
*/
void intel_fbc_init(struct drm_i915_private *dev_priv)
{
+ enum pipe pipe;
+
if (!HAS_FBC(dev_priv)) {
dev_priv->fbc.enabled = false;
dev_priv->fbc.no_fbc_reason = FBC_UNSUPPORTED;
return;
}
+ /* TODO: some platforms have FBC tied to a specific plane! */
+ for_each_pipe(dev_priv, pipe)
+ dev_priv->fbc.possible_framebuffer_bits |=
+ INTEL_FRONTBUFFER_PRIMARY(pipe);
+
if (INTEL_INFO(dev_priv)->gen >= 7) {
dev_priv->display.fbc_enabled = ilk_fbc_enabled;
dev_priv->display.enable_fbc = gen7_fbc_enable;
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 7bdac69..47b3409 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -118,8 +118,6 @@ static void intel_mark_fb_busy(struct drm_device *dev,
continue;
intel_increase_pllclock(dev, pipe);
- if (ring && intel_fbc_enabled(dev))
- ring->fbc_dirty = true;
}
}
@@ -159,6 +157,7 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);
intel_psr_invalidate(dev, obj->frontbuffer_bits);
+ intel_fbc_invalidate(dev_priv, obj->frontbuffer_bits, origin);
}
/**
@@ -187,16 +186,7 @@ void intel_frontbuffer_flush(struct drm_device *dev,
intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
intel_psr_flush(dev, frontbuffer_bits);
-
- /*
- * FIXME: Unconditional fbc flushing here is a rather gross hack and
- * needs to be reworked into a proper frontbuffer tracking scheme like
- * psr employs.
- */
- if (dev_priv->fbc.need_sw_cache_clean) {
- dev_priv->fbc.need_sw_cache_clean = false;
- bdw_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN);
- }
+ intel_fbc_flush(dev_priv, frontbuffer_bits, origin);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 4702e7b..0d64d72 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -317,29 +317,6 @@ gen7_render_ring_cs_stall_wa(struct intel_engine_cs *ring)
return 0;
}
-static int gen7_ring_fbc_flush(struct intel_engine_cs *ring, u32 value)
-{
- int ret;
-
- if (!ring->fbc_dirty)
- return 0;
-
- ret = intel_ring_begin(ring, 6);
- if (ret)
- return ret;
- /* WaFbcNukeOn3DBlt:ivb/hsw */
- intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
- intel_ring_emit(ring, MSG_FBC_REND_STATE);
- intel_ring_emit(ring, value);
- intel_ring_emit(ring, MI_STORE_REGISTER_MEM(1) | MI_SRM_LRM_GLOBAL_GTT);
- intel_ring_emit(ring, MSG_FBC_REND_STATE);
- intel_ring_emit(ring, ring->scratch.gtt_offset + 256);
- intel_ring_advance(ring);
-
- ring->fbc_dirty = false;
- return 0;
-}
-
static int
gen7_render_ring_flush(struct intel_engine_cs *ring,
u32 invalidate_domains, u32 flush_domains)
@@ -395,9 +372,6 @@ gen7_render_ring_flush(struct intel_engine_cs *ring,
intel_ring_emit(ring, 0);
intel_ring_advance(ring);
- if (!invalidate_domains && flush_domains)
- return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
-
return 0;
}
@@ -459,9 +433,6 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
if (ret)
return ret;
- if (!invalidate_domains && flush_domains)
- return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
-
return 0;
}
@@ -2296,7 +2267,6 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
u32 invalidate, u32 flush)
{
struct drm_device *dev = ring->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t cmd;
int ret;
@@ -2305,7 +2275,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
return ret;
cmd = MI_FLUSH_DW;
- if (INTEL_INFO(ring->dev)->gen >= 8)
+ if (INTEL_INFO(dev)->gen >= 8)
cmd += 1;
/*
* Bspec vol 1c.3 - blitter engine command streamer:
@@ -2318,7 +2288,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
MI_FLUSH_DW_OP_STOREDW;
intel_ring_emit(ring, cmd);
intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT);
- if (INTEL_INFO(ring->dev)->gen >= 8) {
+ if (INTEL_INFO(dev)->gen >= 8) {
intel_ring_emit(ring, 0); /* upper addr */
intel_ring_emit(ring, 0); /* value */
} else {
@@ -2327,13 +2297,6 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
}
intel_ring_advance(ring);
- if (!invalidate && flush) {
- if (IS_GEN7(dev))
- return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
- else if (IS_BROADWELL(dev))
- dev_priv->fbc.need_sw_cache_clean = true;
- }
-
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 6dbb6f4..7e31ec8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -269,7 +269,6 @@ struct intel_engine_cs {
*/
struct drm_i915_gem_request *outstanding_lazy_request;
bool gpu_caches_dirty;
- bool fbc_dirty;
wait_queue_head_t irq_queue;
--
2.1.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 08/11] drm/i915: add fronbuffer tracking to FBC
2014-12-08 16:09 ` [PATCH 08/11] drm/i915: add fronbuffer tracking to FBC Paulo Zanoni
2014-12-13 1:12 ` [PATCH 6/9] drm/i915: add frontbuffer " Rodrigo Vivi
@ 2014-12-13 1:16 ` Rodrigo Vivi
2014-12-15 14:00 ` [PATCH 8/11] drm/i915: add frontbuffer " Paulo Zanoni
1 sibling, 1 reply; 41+ messages in thread
From: Rodrigo Vivi @ 2014-12-13 1:16 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
just a note that I sent a reviwed v2: rebased and with typo fix.
On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Kill the blt/render tracking we currently have and use the frontbuffer
> tracking infrastructure.
>
> Don't enable things by default yet.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 9 +--
> drivers/gpu/drm/i915/intel_drv.h | 6 +-
> drivers/gpu/drm/i915/intel_fbc.c | 119 ++++++++++++++++++-------------
> drivers/gpu/drm/i915/intel_frontbuffer.c | 14 +---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 41 +----------
> drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
> 6 files changed, 80 insertions(+), 110 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ea3cc81..22285c2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -696,6 +696,7 @@ struct i915_fbc {
> unsigned long size;
> unsigned threshold;
> unsigned int fb_id;
> + unsigned int possible_framebuffer_bits;
> struct intel_crtc *crtc;
> int y;
>
> @@ -708,14 +709,6 @@ struct i915_fbc {
> * possible. */
> bool enabled;
>
> - /* On gen8 some rings cannont perform fbc clean operation so for now
> - * we are doing this on SW with mmio.
> - * This variable works in the opposite information direction
> - * of ring->fbc_dirty telling software on frontbuffer tracking
> - * to perform the cache clean on sw side.
> - */
> - bool need_sw_cache_clean;
> -
> struct intel_fbc_work {
> struct delayed_work work;
> struct drm_crtc *crtc;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 633fb9a..3a1c3d82 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1071,7 +1071,11 @@ bool intel_fbc_enabled(struct drm_device *dev);
> void intel_fbc_update(struct drm_device *dev);
> void intel_fbc_init(struct drm_i915_private *dev_priv);
> void intel_fbc_disable(struct drm_device *dev);
> -void bdw_fbc_sw_flush(struct drm_device *dev, u32 value);
> +void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
> + unsigned int frontbuffer_bits,
> + enum fb_op_origin origin);
> +void intel_fbc_flush(struct drm_i915_private *dev_priv,
> + unsigned int frontbuffer_bits, enum fb_op_origin origin);
>
> /* intel_hdmi.c */
> void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 88d00d3..03ba43d 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -178,29 +178,10 @@ static bool g4x_fbc_enabled(struct drm_device *dev)
> return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN;
> }
>
> -static void snb_fbc_blit_update(struct drm_device *dev)
> +static void intel_fbc_nuke(struct drm_i915_private *dev_priv)
> {
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - u32 blt_ecoskpd;
> -
> - /* Make sure blitter notifies FBC of writes */
> -
> - /* Blitter is part of Media powerwell on VLV. No impact of
> - * his param in other platforms for now */
> - gen6_gt_force_wake_get(dev_priv, FORCEWAKE_MEDIA);
> -
> - blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
> - blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY <<
> - GEN6_BLITTER_LOCK_SHIFT;
> - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
> - blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
> - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
> - blt_ecoskpd &= ~(GEN6_BLITTER_FBC_NOTIFY <<
> - GEN6_BLITTER_LOCK_SHIFT);
> - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
> - POSTING_READ(GEN6_BLITTER_ECOSKPD);
> -
> - gen6_gt_force_wake_put(dev_priv, FORCEWAKE_MEDIA);
> + I915_WRITE(MSG_FBC_REND_STATE, FBC_REND_NUKE);
> + POSTING_READ(MSG_FBC_REND_STATE);
> }
>
> static void ilk_fbc_enable(struct drm_crtc *crtc)
> @@ -243,9 +224,10 @@ static void ilk_fbc_enable(struct drm_crtc *crtc)
> I915_WRITE(SNB_DPFC_CTL_SA,
> SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
> - snb_fbc_blit_update(dev);
> }
>
> + intel_fbc_nuke(dev_priv);
> +
> DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
> }
>
> @@ -324,7 +306,7 @@ static void gen7_fbc_enable(struct drm_crtc *crtc)
> SNB_CPU_FENCE_ENABLE | obj->fence_reg);
> I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
>
> - snb_fbc_blit_update(dev);
> + intel_fbc_nuke(dev_priv);
>
> DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
> }
> @@ -342,31 +324,6 @@ bool intel_fbc_enabled(struct drm_device *dev)
> return dev_priv->fbc.enabled;
> }
>
> -/**
> - * bdw_fbc_sw_flush - FBC Software Flush for Broadwell.
> - * @dev: the drm_device
> - * @value: Value to be set on MSG_FBC_REND_STATE. Possible values are
> - * FBC_REND_NUKE and FBC_REND_CACHE_CLEAN.
> - *
> - * This function is needed on Broadwell to perform Nuke or Cache clean on
> - * software side over MMIO.
> - * On Broadwell, due a hardware bug, MSG_FBC_REND_STATE stay in a forbidden
> - * address that has a huge risk of causing GPU Hangs if set with LRI on some
> - * command streamers.
> - */
> -void bdw_fbc_sw_flush(struct drm_device *dev, u32 value)
> -{
> - struct drm_i915_private *dev_priv = dev->dev_private;
> -
> - if (!IS_GEN8(dev))
> - return;
> -
> - if (!intel_fbc_enabled(dev))
> - return;
> -
> - I915_WRITE(MSG_FBC_REND_STATE, value);
> -}
> -
> static void intel_fbc_work_fn(struct work_struct *__work)
> {
> struct intel_fbc_work *work =
> @@ -672,6 +629,63 @@ out_disable:
> i915_gem_stolen_cleanup_compression(dev);
> }
>
> +void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
> + unsigned int frontbuffer_bits,
> + enum fb_op_origin origin)
> +{
> + struct drm_device *dev = dev_priv->dev;
> + unsigned int fbc_bits;
> +
> + if (origin == ORIGIN_GTT)
> + return;
> +
> + if (dev_priv->fbc.enabled)
> + fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe);
> + else if (dev_priv->fbc.fbc_work)
> + fbc_bits = INTEL_FRONTBUFFER_PRIMARY(
> + to_intel_crtc(dev_priv->fbc.fbc_work->crtc)->pipe);
> + else
> + fbc_bits = dev_priv->fbc.possible_framebuffer_bits;
> +
> + if ((fbc_bits & frontbuffer_bits) == 0)
> + return;
> +
> + intel_fbc_disable(dev);
> +}
> +
> +void intel_fbc_flush(struct drm_i915_private *dev_priv,
> + unsigned int frontbuffer_bits,
> + enum fb_op_origin origin)
> +{
> + struct drm_device *dev = dev_priv->dev;
> + unsigned int fbc_bits = 0;
> + bool fbc_enabled;
> +
> + if (dev_priv->fbc.enabled)
> + fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe);
> + else
> + fbc_bits = dev_priv->fbc.possible_framebuffer_bits;
> +
> + /* These are not the planes you are looking for. */
> + if ((frontbuffer_bits & fbc_bits) == 0)
> + return;
> +
> + fbc_enabled = intel_fbc_enabled(dev);
> +
> + if ((dev_priv->fb_tracking.busy_bits & frontbuffer_bits &
> + fbc_bits) == 0) {
> + if (fbc_enabled) {
> + if (origin == ORIGIN_RENDER || origin == ORIGIN_CPU)
> + intel_fbc_nuke(dev_priv);
> + } else {
> + intel_fbc_update(dev);
> + }
> + } else {
> + if (fbc_enabled)
> + intel_fbc_disable(dev);
> + }
> +}
> +
> /**
> * intel_fbc_init - Initialize FBC
> * @dev_priv: the i915 device
> @@ -680,12 +694,19 @@ out_disable:
> */
> void intel_fbc_init(struct drm_i915_private *dev_priv)
> {
> + enum pipe pipe;
> +
> if (!HAS_FBC(dev_priv)) {
> dev_priv->fbc.enabled = false;
> dev_priv->fbc.no_fbc_reason = FBC_UNSUPPORTED;
> return;
> }
>
> + /* TODO: some platforms have FBC tied to a specific plane! */
> + for_each_pipe(dev_priv, pipe)
> + dev_priv->fbc.possible_framebuffer_bits |=
> + INTEL_FRONTBUFFER_PRIMARY(pipe);
> +
> if (INTEL_INFO(dev_priv)->gen >= 7) {
> dev_priv->display.fbc_enabled = ilk_fbc_enabled;
> dev_priv->display.enable_fbc = gen7_fbc_enable;
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index 7bdac69..47b3409 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -118,8 +118,6 @@ static void intel_mark_fb_busy(struct drm_device *dev,
> continue;
>
> intel_increase_pllclock(dev, pipe);
> - if (ring && intel_fbc_enabled(dev))
> - ring->fbc_dirty = true;
> }
> }
>
> @@ -159,6 +157,7 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
> intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);
>
> intel_psr_invalidate(dev, obj->frontbuffer_bits);
> + intel_fbc_invalidate(dev_priv, obj->frontbuffer_bits, origin);
> }
>
> /**
> @@ -187,16 +186,7 @@ void intel_frontbuffer_flush(struct drm_device *dev,
> intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
>
> intel_psr_flush(dev, frontbuffer_bits);
> -
> - /*
> - * FIXME: Unconditional fbc flushing here is a rather gross hack and
> - * needs to be reworked into a proper frontbuffer tracking scheme like
> - * psr employs.
> - */
> - if (dev_priv->fbc.need_sw_cache_clean) {
> - dev_priv->fbc.need_sw_cache_clean = false;
> - bdw_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN);
> - }
> + intel_fbc_flush(dev_priv, frontbuffer_bits, origin);
> }
>
> /**
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 83accb7..5982da9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -317,29 +317,6 @@ gen7_render_ring_cs_stall_wa(struct intel_engine_cs *ring)
> return 0;
> }
>
> -static int gen7_ring_fbc_flush(struct intel_engine_cs *ring, u32 value)
> -{
> - int ret;
> -
> - if (!ring->fbc_dirty)
> - return 0;
> -
> - ret = intel_ring_begin(ring, 6);
> - if (ret)
> - return ret;
> - /* WaFbcNukeOn3DBlt:ivb/hsw */
> - intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> - intel_ring_emit(ring, MSG_FBC_REND_STATE);
> - intel_ring_emit(ring, value);
> - intel_ring_emit(ring, MI_STORE_REGISTER_MEM(1) | MI_SRM_LRM_GLOBAL_GTT);
> - intel_ring_emit(ring, MSG_FBC_REND_STATE);
> - intel_ring_emit(ring, ring->scratch.gtt_offset + 256);
> - intel_ring_advance(ring);
> -
> - ring->fbc_dirty = false;
> - return 0;
> -}
> -
> static int
> gen7_render_ring_flush(struct intel_engine_cs *ring,
> u32 invalidate_domains, u32 flush_domains)
> @@ -395,9 +372,6 @@ gen7_render_ring_flush(struct intel_engine_cs *ring,
> intel_ring_emit(ring, 0);
> intel_ring_advance(ring);
>
> - if (!invalidate_domains && flush_domains)
> - return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
> -
> return 0;
> }
>
> @@ -459,9 +433,6 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
> if (ret)
> return ret;
>
> - if (!invalidate_domains && flush_domains)
> - return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
> -
> return 0;
> }
>
> @@ -2279,7 +2250,6 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
> u32 invalidate, u32 flush)
> {
> struct drm_device *dev = ring->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> uint32_t cmd;
> int ret;
>
> @@ -2288,7 +2258,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
> return ret;
>
> cmd = MI_FLUSH_DW;
> - if (INTEL_INFO(ring->dev)->gen >= 8)
> + if (INTEL_INFO(dev)->gen >= 8)
> cmd += 1;
> /*
> * Bspec vol 1c.3 - blitter engine command streamer:
> @@ -2301,7 +2271,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
> MI_FLUSH_DW_OP_STOREDW;
> intel_ring_emit(ring, cmd);
> intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT);
> - if (INTEL_INFO(ring->dev)->gen >= 8) {
> + if (INTEL_INFO(dev)->gen >= 8) {
> intel_ring_emit(ring, 0); /* upper addr */
> intel_ring_emit(ring, 0); /* value */
> } else {
> @@ -2310,13 +2280,6 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
> }
> intel_ring_advance(ring);
>
> - if (!invalidate && flush) {
> - if (IS_GEN7(dev))
> - return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
> - else if (IS_BROADWELL(dev))
> - dev_priv->fbc.need_sw_cache_clean = true;
> - }
> -
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 6dbb6f4..7e31ec8 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -269,7 +269,6 @@ struct intel_engine_cs {
> */
> struct drm_i915_gem_request *outstanding_lazy_request;
> bool gpu_caches_dirty;
> - bool fbc_dirty;
>
> wait_queue_head_t irq_queue;
>
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* [PATCH 8/11] drm/i915: add frontbuffer tracking to FBC
2014-12-13 1:16 ` [PATCH 08/11] drm/i915: add fronbuffer " Rodrigo Vivi
@ 2014-12-15 14:00 ` Paulo Zanoni
0 siblings, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-15 14:00 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Kill the blt/render tracking we currently have and use the frontbuffer
tracking infrastructure.
Don't enable things by default yet.
v2: (Rodrigo) Fix small conflict on rebase and typo at subject.
v3: (Paulo) Rebase on RENDER_CS change.
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> (v2)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 9 +--
drivers/gpu/drm/i915/intel_drv.h | 6 +-
drivers/gpu/drm/i915/intel_fbc.c | 107 ++++++++++++++++++++-----------
drivers/gpu/drm/i915/intel_frontbuffer.c | 14 +---
drivers/gpu/drm/i915/intel_ringbuffer.c | 41 +-----------
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
6 files changed, 80 insertions(+), 98 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5de5c77..d81ecd7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -708,6 +708,7 @@ struct i915_fbc {
unsigned long size;
unsigned threshold;
unsigned int fb_id;
+ unsigned int possible_framebuffer_bits;
struct intel_crtc *crtc;
int y;
@@ -720,14 +721,6 @@ struct i915_fbc {
* possible. */
bool enabled;
- /* On gen8 some rings cannont perform fbc clean operation so for now
- * we are doing this on SW with mmio.
- * This variable works in the opposite information direction
- * of ring->fbc_dirty telling software on frontbuffer tracking
- * to perform the cache clean on sw side.
- */
- bool need_sw_cache_clean;
-
struct intel_fbc_work {
struct delayed_work work;
struct drm_crtc *crtc;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 633fb9a..3a1c3d82 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1071,7 +1071,11 @@ bool intel_fbc_enabled(struct drm_device *dev);
void intel_fbc_update(struct drm_device *dev);
void intel_fbc_init(struct drm_i915_private *dev_priv);
void intel_fbc_disable(struct drm_device *dev);
-void bdw_fbc_sw_flush(struct drm_device *dev, u32 value);
+void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits,
+ enum fb_op_origin origin);
+void intel_fbc_flush(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits, enum fb_op_origin origin);
/* intel_hdmi.c */
void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index f658dc0..c71b435 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -173,29 +173,10 @@ static bool g4x_fbc_enabled(struct drm_device *dev)
return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN;
}
-static void snb_fbc_blit_update(struct drm_device *dev)
+static void intel_fbc_nuke(struct drm_i915_private *dev_priv)
{
- struct drm_i915_private *dev_priv = dev->dev_private;
- u32 blt_ecoskpd;
-
- /* Make sure blitter notifies FBC of writes */
-
- /* Blitter is part of Media powerwell on VLV. No impact of
- * his param in other platforms for now */
- gen6_gt_force_wake_get(dev_priv, FORCEWAKE_MEDIA);
-
- blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
- blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY <<
- GEN6_BLITTER_LOCK_SHIFT;
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- blt_ecoskpd &= ~(GEN6_BLITTER_FBC_NOTIFY <<
- GEN6_BLITTER_LOCK_SHIFT);
- I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
- POSTING_READ(GEN6_BLITTER_ECOSKPD);
-
- gen6_gt_force_wake_put(dev_priv, FORCEWAKE_MEDIA);
+ I915_WRITE(MSG_FBC_REND_STATE, FBC_REND_NUKE);
+ POSTING_READ(MSG_FBC_REND_STATE);
}
static void ilk_fbc_enable(struct drm_crtc *crtc)
@@ -238,9 +219,10 @@ static void ilk_fbc_enable(struct drm_crtc *crtc)
I915_WRITE(SNB_DPFC_CTL_SA,
SNB_CPU_FENCE_ENABLE | obj->fence_reg);
I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
- snb_fbc_blit_update(dev);
}
+ intel_fbc_nuke(dev_priv);
+
DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
}
@@ -319,7 +301,7 @@ static void gen7_fbc_enable(struct drm_crtc *crtc)
SNB_CPU_FENCE_ENABLE | obj->fence_reg);
I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc->y);
- snb_fbc_blit_update(dev);
+ intel_fbc_nuke(dev_priv);
DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
}
@@ -339,19 +321,6 @@ bool intel_fbc_enabled(struct drm_device *dev)
return dev_priv->fbc.enabled;
}
-void bdw_fbc_sw_flush(struct drm_device *dev, u32 value)
-{
- struct drm_i915_private *dev_priv = dev->dev_private;
-
- if (!IS_GEN8(dev))
- return;
-
- if (!intel_fbc_enabled(dev))
- return;
-
- I915_WRITE(MSG_FBC_REND_STATE, value);
-}
-
static void intel_fbc_work_fn(struct work_struct *__work)
{
struct intel_fbc_work *work =
@@ -657,6 +626,63 @@ out_disable:
i915_gem_stolen_cleanup_compression(dev);
}
+void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits,
+ enum fb_op_origin origin)
+{
+ struct drm_device *dev = dev_priv->dev;
+ unsigned int fbc_bits;
+
+ if (origin == ORIGIN_GTT)
+ return;
+
+ if (dev_priv->fbc.enabled)
+ fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe);
+ else if (dev_priv->fbc.fbc_work)
+ fbc_bits = INTEL_FRONTBUFFER_PRIMARY(
+ to_intel_crtc(dev_priv->fbc.fbc_work->crtc)->pipe);
+ else
+ fbc_bits = dev_priv->fbc.possible_framebuffer_bits;
+
+ if ((fbc_bits & frontbuffer_bits) == 0)
+ return;
+
+ intel_fbc_disable(dev);
+}
+
+void intel_fbc_flush(struct drm_i915_private *dev_priv,
+ unsigned int frontbuffer_bits,
+ enum fb_op_origin origin)
+{
+ struct drm_device *dev = dev_priv->dev;
+ unsigned int fbc_bits = 0;
+ bool fbc_enabled;
+
+ if (dev_priv->fbc.enabled)
+ fbc_bits = INTEL_FRONTBUFFER_PRIMARY(dev_priv->fbc.crtc->pipe);
+ else
+ fbc_bits = dev_priv->fbc.possible_framebuffer_bits;
+
+ /* These are not the planes you are looking for. */
+ if ((frontbuffer_bits & fbc_bits) == 0)
+ return;
+
+ fbc_enabled = intel_fbc_enabled(dev);
+
+ if ((dev_priv->fb_tracking.busy_bits & frontbuffer_bits &
+ fbc_bits) == 0) {
+ if (fbc_enabled) {
+ if (origin == ORIGIN_CS || origin == ORIGIN_CPU)
+ intel_fbc_nuke(dev_priv);
+ } else {
+ intel_fbc_update(dev);
+ }
+ } else {
+ if (fbc_enabled)
+ intel_fbc_disable(dev);
+ }
+}
+
/**
* intel_fbc_init - Initialize FBC
* @dev_priv: the i915 device
@@ -665,12 +691,19 @@ out_disable:
*/
void intel_fbc_init(struct drm_i915_private *dev_priv)
{
+ enum pipe pipe;
+
if (!HAS_FBC(dev_priv)) {
dev_priv->fbc.enabled = false;
dev_priv->fbc.no_fbc_reason = FBC_UNSUPPORTED;
return;
}
+ /* TODO: some platforms have FBC tied to a specific plane! */
+ for_each_pipe(dev_priv, pipe)
+ dev_priv->fbc.possible_framebuffer_bits |=
+ INTEL_FRONTBUFFER_PRIMARY(pipe);
+
if (INTEL_INFO(dev_priv)->gen >= 7) {
dev_priv->display.fbc_enabled = ilk_fbc_enabled;
dev_priv->display.enable_fbc = gen7_fbc_enable;
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 3d35f31..0f5e926 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -118,8 +118,6 @@ static void intel_mark_fb_busy(struct drm_device *dev,
continue;
intel_increase_pllclock(dev, pipe);
- if (ring && intel_fbc_enabled(dev))
- ring->fbc_dirty = true;
}
}
@@ -159,6 +157,7 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);
intel_psr_invalidate(dev, obj->frontbuffer_bits);
+ intel_fbc_invalidate(dev_priv, obj->frontbuffer_bits, origin);
}
/**
@@ -187,16 +186,7 @@ void intel_frontbuffer_flush(struct drm_device *dev,
intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
intel_psr_flush(dev, frontbuffer_bits);
-
- /*
- * FIXME: Unconditional fbc flushing here is a rather gross hack and
- * needs to be reworked into a proper frontbuffer tracking scheme like
- * psr employs.
- */
- if (dev_priv->fbc.need_sw_cache_clean) {
- dev_priv->fbc.need_sw_cache_clean = false;
- bdw_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN);
- }
+ intel_fbc_flush(dev_priv, frontbuffer_bits, origin);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 4702e7b..0d64d72 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -317,29 +317,6 @@ gen7_render_ring_cs_stall_wa(struct intel_engine_cs *ring)
return 0;
}
-static int gen7_ring_fbc_flush(struct intel_engine_cs *ring, u32 value)
-{
- int ret;
-
- if (!ring->fbc_dirty)
- return 0;
-
- ret = intel_ring_begin(ring, 6);
- if (ret)
- return ret;
- /* WaFbcNukeOn3DBlt:ivb/hsw */
- intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
- intel_ring_emit(ring, MSG_FBC_REND_STATE);
- intel_ring_emit(ring, value);
- intel_ring_emit(ring, MI_STORE_REGISTER_MEM(1) | MI_SRM_LRM_GLOBAL_GTT);
- intel_ring_emit(ring, MSG_FBC_REND_STATE);
- intel_ring_emit(ring, ring->scratch.gtt_offset + 256);
- intel_ring_advance(ring);
-
- ring->fbc_dirty = false;
- return 0;
-}
-
static int
gen7_render_ring_flush(struct intel_engine_cs *ring,
u32 invalidate_domains, u32 flush_domains)
@@ -395,9 +372,6 @@ gen7_render_ring_flush(struct intel_engine_cs *ring,
intel_ring_emit(ring, 0);
intel_ring_advance(ring);
- if (!invalidate_domains && flush_domains)
- return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
-
return 0;
}
@@ -459,9 +433,6 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
if (ret)
return ret;
- if (!invalidate_domains && flush_domains)
- return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
-
return 0;
}
@@ -2296,7 +2267,6 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
u32 invalidate, u32 flush)
{
struct drm_device *dev = ring->dev;
- struct drm_i915_private *dev_priv = dev->dev_private;
uint32_t cmd;
int ret;
@@ -2305,7 +2275,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
return ret;
cmd = MI_FLUSH_DW;
- if (INTEL_INFO(ring->dev)->gen >= 8)
+ if (INTEL_INFO(dev)->gen >= 8)
cmd += 1;
/*
* Bspec vol 1c.3 - blitter engine command streamer:
@@ -2318,7 +2288,7 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
MI_FLUSH_DW_OP_STOREDW;
intel_ring_emit(ring, cmd);
intel_ring_emit(ring, I915_GEM_HWS_SCRATCH_ADDR | MI_FLUSH_DW_USE_GTT);
- if (INTEL_INFO(ring->dev)->gen >= 8) {
+ if (INTEL_INFO(dev)->gen >= 8) {
intel_ring_emit(ring, 0); /* upper addr */
intel_ring_emit(ring, 0); /* value */
} else {
@@ -2327,13 +2297,6 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
}
intel_ring_advance(ring);
- if (!invalidate && flush) {
- if (IS_GEN7(dev))
- return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
- else if (IS_BROADWELL(dev))
- dev_priv->fbc.need_sw_cache_clean = true;
- }
-
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 6dbb6f4..7e31ec8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -269,7 +269,6 @@ struct intel_engine_cs {
*/
struct drm_i915_gem_request *outstanding_lazy_request;
bool gpu_caches_dirty;
- bool fbc_dirty;
wait_queue_head_t irq_queue;
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH 09/11] drm/i915: extract intel_fbc_find_crtc()
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
` (7 preceding siblings ...)
2014-12-08 16:09 ` [PATCH 08/11] drm/i915: add fronbuffer tracking to FBC Paulo Zanoni
@ 2014-12-08 16:09 ` Paulo Zanoni
2014-12-13 1:20 ` Rodrigo Vivi
2014-12-08 16:09 ` [PATCH 10/11] drm/i915: HSW+ FBC is tied to pipe A Paulo Zanoni
` (2 subsequent siblings)
11 siblings, 1 reply; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
I want to make this code a little more complicated, so let's extract
the function first.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_fbc.c | 46 +++++++++++++++++++++++++---------------
1 file changed, 29 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 03ba43d..450d0be 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -444,6 +444,32 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
return true;
}
+static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
+{
+ struct drm_device *dev = dev_priv->dev;
+ struct drm_crtc *crtc = NULL, *tmp_crtc;
+
+ for_each_crtc(dev, tmp_crtc) {
+ if (intel_crtc_active(tmp_crtc) &&
+ to_intel_crtc(tmp_crtc)->primary_enabled) {
+ if (crtc) {
+ if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
+ DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
+ return NULL;
+ }
+ crtc = tmp_crtc;
+ }
+ }
+
+ if (!crtc || crtc->primary->fb == NULL) {
+ if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT))
+ DRM_DEBUG_KMS("no output, disabling\n");
+ return NULL;
+ }
+
+ return crtc;
+}
+
/**
* intel_fbc_update - enable/disable FBC as needed
* @dev: the drm_device
@@ -466,7 +492,7 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
void intel_fbc_update(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- struct drm_crtc *crtc = NULL, *tmp_crtc;
+ struct drm_crtc *crtc = NULL;
struct intel_crtc *intel_crtc;
struct drm_framebuffer *fb;
struct drm_i915_gem_object *obj;
@@ -497,23 +523,9 @@ void intel_fbc_update(struct drm_device *dev)
* - new fb is too large to fit in compressed buffer
* - going to an unsupported config (interlace, pixel multiply, etc.)
*/
- for_each_crtc(dev, tmp_crtc) {
- if (intel_crtc_active(tmp_crtc) &&
- to_intel_crtc(tmp_crtc)->primary_enabled) {
- if (crtc) {
- if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
- DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
- goto out_disable;
- }
- crtc = tmp_crtc;
- }
- }
-
- if (!crtc || crtc->primary->fb == NULL) {
- if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT))
- DRM_DEBUG_KMS("no output, disabling\n");
+ crtc = intel_fbc_find_crtc(dev_priv);
+ if (!crtc)
goto out_disable;
- }
intel_crtc = to_intel_crtc(crtc);
fb = crtc->primary->fb;
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 09/11] drm/i915: extract intel_fbc_find_crtc()
2014-12-08 16:09 ` [PATCH 09/11] drm/i915: extract intel_fbc_find_crtc() Paulo Zanoni
@ 2014-12-13 1:20 ` Rodrigo Vivi
0 siblings, 0 replies; 41+ messages in thread
From: Rodrigo Vivi @ 2014-12-13 1:20 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> I want to make this code a little more complicated, so let's extract
> the function first.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_fbc.c | 46 +++++++++++++++++++++++++---------------
> 1 file changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 03ba43d..450d0be 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -444,6 +444,32 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
> return true;
> }
>
> +static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
> +{
> + struct drm_device *dev = dev_priv->dev;
> + struct drm_crtc *crtc = NULL, *tmp_crtc;
> +
> + for_each_crtc(dev, tmp_crtc) {
> + if (intel_crtc_active(tmp_crtc) &&
> + to_intel_crtc(tmp_crtc)->primary_enabled) {
> + if (crtc) {
> + if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
> + DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
> + return NULL;
> + }
> + crtc = tmp_crtc;
> + }
> + }
> +
> + if (!crtc || crtc->primary->fb == NULL) {
> + if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT))
> + DRM_DEBUG_KMS("no output, disabling\n");
> + return NULL;
> + }
> +
> + return crtc;
> +}
> +
> /**
> * intel_fbc_update - enable/disable FBC as needed
> * @dev: the drm_device
> @@ -466,7 +492,7 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
> void intel_fbc_update(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - struct drm_crtc *crtc = NULL, *tmp_crtc;
> + struct drm_crtc *crtc = NULL;
> struct intel_crtc *intel_crtc;
> struct drm_framebuffer *fb;
> struct drm_i915_gem_object *obj;
> @@ -497,23 +523,9 @@ void intel_fbc_update(struct drm_device *dev)
> * - new fb is too large to fit in compressed buffer
> * - going to an unsupported config (interlace, pixel multiply, etc.)
> */
> - for_each_crtc(dev, tmp_crtc) {
> - if (intel_crtc_active(tmp_crtc) &&
> - to_intel_crtc(tmp_crtc)->primary_enabled) {
> - if (crtc) {
> - if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
> - DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
> - goto out_disable;
> - }
> - crtc = tmp_crtc;
> - }
> - }
> -
> - if (!crtc || crtc->primary->fb == NULL) {
> - if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT))
> - DRM_DEBUG_KMS("no output, disabling\n");
> + crtc = intel_fbc_find_crtc(dev_priv);
> + if (!crtc)
> goto out_disable;
> - }
>
> intel_crtc = to_intel_crtc(crtc);
> fb = crtc->primary->fb;
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 10/11] drm/i915: HSW+ FBC is tied to pipe A
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
` (8 preceding siblings ...)
2014-12-08 16:09 ` [PATCH 09/11] drm/i915: extract intel_fbc_find_crtc() Paulo Zanoni
@ 2014-12-08 16:09 ` Paulo Zanoni
2014-12-13 1:23 ` Rodrigo Vivi
2014-12-08 16:09 ` [PATCH 11/11] drm/i915: gen5+ can have FBC with multiple pipes Paulo Zanoni
2014-12-08 16:12 ` [PATCH igt 1/4] lib: add igt_wait() Paulo Zanoni
11 siblings, 1 reply; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
So add code to consider this case.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_fbc.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 450d0be..e8dc1d5 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -446,10 +446,16 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
{
- struct drm_device *dev = dev_priv->dev;
struct drm_crtc *crtc = NULL, *tmp_crtc;
+ enum pipe pipe;
+ bool pipe_a_only = false;
+
+ if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
+ pipe_a_only = true;
+
+ for_each_pipe(dev_priv, pipe) {
+ tmp_crtc = dev_priv->pipe_to_crtc_mapping[pipe];
- for_each_crtc(dev, tmp_crtc) {
if (intel_crtc_active(tmp_crtc) &&
to_intel_crtc(tmp_crtc)->primary_enabled) {
if (crtc) {
@@ -459,6 +465,9 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
}
crtc = tmp_crtc;
}
+
+ if (pipe_a_only)
+ break;
}
if (!crtc || crtc->primary->fb == NULL) {
@@ -714,11 +723,14 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
return;
}
- /* TODO: some platforms have FBC tied to a specific plane! */
- for_each_pipe(dev_priv, pipe)
+ for_each_pipe(dev_priv, pipe) {
dev_priv->fbc.possible_framebuffer_bits |=
INTEL_FRONTBUFFER_PRIMARY(pipe);
+ if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
+ break;
+ }
+
if (INTEL_INFO(dev_priv)->gen >= 7) {
dev_priv->display.fbc_enabled = ilk_fbc_enabled;
dev_priv->display.enable_fbc = gen7_fbc_enable;
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 10/11] drm/i915: HSW+ FBC is tied to pipe A
2014-12-08 16:09 ` [PATCH 10/11] drm/i915: HSW+ FBC is tied to pipe A Paulo Zanoni
@ 2014-12-13 1:23 ` Rodrigo Vivi
2014-12-15 8:41 ` Daniel Vetter
0 siblings, 1 reply; 41+ messages in thread
From: Rodrigo Vivi @ 2014-12-13 1:23 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
I always ask myself if we should just clean the code and remove all
platforms before HSW that always had many fbc issues. So we could make
it simple and just do for pipe A for all platforms.
Anyway, this looks ok for now
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> So add code to consider this case.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_fbc.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 450d0be..e8dc1d5 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -446,10 +446,16 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
>
> static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
> {
> - struct drm_device *dev = dev_priv->dev;
> struct drm_crtc *crtc = NULL, *tmp_crtc;
> + enum pipe pipe;
> + bool pipe_a_only = false;
> +
> + if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
> + pipe_a_only = true;
> +
> + for_each_pipe(dev_priv, pipe) {
> + tmp_crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>
> - for_each_crtc(dev, tmp_crtc) {
> if (intel_crtc_active(tmp_crtc) &&
> to_intel_crtc(tmp_crtc)->primary_enabled) {
> if (crtc) {
> @@ -459,6 +465,9 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
> }
> crtc = tmp_crtc;
> }
> +
> + if (pipe_a_only)
> + break;
> }
>
> if (!crtc || crtc->primary->fb == NULL) {
> @@ -714,11 +723,14 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
> return;
> }
>
> - /* TODO: some platforms have FBC tied to a specific plane! */
> - for_each_pipe(dev_priv, pipe)
> + for_each_pipe(dev_priv, pipe) {
> dev_priv->fbc.possible_framebuffer_bits |=
> INTEL_FRONTBUFFER_PRIMARY(pipe);
>
> + if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
> + break;
> + }
> +
> if (INTEL_INFO(dev_priv)->gen >= 7) {
> dev_priv->display.fbc_enabled = ilk_fbc_enabled;
> dev_priv->display.enable_fbc = gen7_fbc_enable;
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread* Re: [PATCH 10/11] drm/i915: HSW+ FBC is tied to pipe A
2014-12-13 1:23 ` Rodrigo Vivi
@ 2014-12-15 8:41 ` Daniel Vetter
0 siblings, 0 replies; 41+ messages in thread
From: Daniel Vetter @ 2014-12-15 8:41 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx, Paulo Zanoni
On Fri, Dec 12, 2014 at 05:23:24PM -0800, Rodrigo Vivi wrote:
> I always ask myself if we should just clean the code and remove all
> platforms before HSW that always had many fbc issues. So we could make
> it simple and just do for pipe A for all platforms.
With frontbuffer tracking we should be able to get it going on snb+ at
least. Before that workarounds are too horrible really, and pre-g4x is
fbc1 which is an entirely different beast. Unfortuately our code
implements fbc2 like fbc1 so would benefit from a split at least.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH 11/11] drm/i915: gen5+ can have FBC with multiple pipes
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
` (9 preceding siblings ...)
2014-12-08 16:09 ` [PATCH 10/11] drm/i915: HSW+ FBC is tied to pipe A Paulo Zanoni
@ 2014-12-08 16:09 ` Paulo Zanoni
2014-12-09 16:08 ` shuang.he
2014-12-13 1:25 ` Rodrigo Vivi
2014-12-08 16:12 ` [PATCH igt 1/4] lib: add igt_wait() Paulo Zanoni
11 siblings, 2 replies; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:09 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
So allow it.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/intel_fbc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index e8dc1d5..1c22922 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -448,17 +448,19 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
{
struct drm_crtc *crtc = NULL, *tmp_crtc;
enum pipe pipe;
- bool pipe_a_only = false;
+ bool pipe_a_only = false, one_pipe_only = false;
if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
pipe_a_only = true;
+ else if (INTEL_INFO(dev_priv)->gen <= 4)
+ one_pipe_only = true;
for_each_pipe(dev_priv, pipe) {
tmp_crtc = dev_priv->pipe_to_crtc_mapping[pipe];
if (intel_crtc_active(tmp_crtc) &&
to_intel_crtc(tmp_crtc)->primary_enabled) {
- if (crtc) {
+ if (one_pipe_only && crtc) {
if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
return NULL;
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* Re: [PATCH 11/11] drm/i915: gen5+ can have FBC with multiple pipes
2014-12-08 16:09 ` [PATCH 11/11] drm/i915: gen5+ can have FBC with multiple pipes Paulo Zanoni
@ 2014-12-09 16:08 ` shuang.he
2014-12-13 1:25 ` Rodrigo Vivi
1 sibling, 0 replies; 41+ messages in thread
From: shuang.he @ 2014-12-09 16:08 UTC (permalink / raw)
To: shuang.he, intel-gfx, przanoni
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 364/364 364/364
ILK +1 364/366 365/366
SNB -1 448/450 447/450
IVB 497/498 497/498
BYT 289/289 289/289
HSW 563/564 563/564
BDW 417/417 417/417
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
ILK igt_kms_flip_wf_vblank-ts-check DMESG_WARN(1, M26)PASS(5, M26M37) PASS(1, M37)
*SNB igt_gem_concurrent_blit_gtt-rcs-early-read-forked PASS(2, M35M22) FAIL(1, M22)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH 11/11] drm/i915: gen5+ can have FBC with multiple pipes
2014-12-08 16:09 ` [PATCH 11/11] drm/i915: gen5+ can have FBC with multiple pipes Paulo Zanoni
2014-12-09 16:08 ` shuang.he
@ 2014-12-13 1:25 ` Rodrigo Vivi
1 sibling, 0 replies; 41+ messages in thread
From: Rodrigo Vivi @ 2014-12-13 1:25 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
I keep my with to remove support for old gens but anyway:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> So allow it.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/intel_fbc.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index e8dc1d5..1c22922 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -448,17 +448,19 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
> {
> struct drm_crtc *crtc = NULL, *tmp_crtc;
> enum pipe pipe;
> - bool pipe_a_only = false;
> + bool pipe_a_only = false, one_pipe_only = false;
>
> if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)->gen >= 8)
> pipe_a_only = true;
> + else if (INTEL_INFO(dev_priv)->gen <= 4)
> + one_pipe_only = true;
>
> for_each_pipe(dev_priv, pipe) {
> tmp_crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>
> if (intel_crtc_active(tmp_crtc) &&
> to_intel_crtc(tmp_crtc)->primary_enabled) {
> - if (crtc) {
> + if (one_pipe_only && crtc) {
> if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
> DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
> return NULL;
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH igt 1/4] lib: add igt_wait()
2014-12-08 16:09 [PATCH 00/11] FBC improvements + frontbuffer tracking conversion Paulo Zanoni
` (10 preceding siblings ...)
2014-12-08 16:09 ` [PATCH 11/11] drm/i915: gen5+ can have FBC with multiple pipes Paulo Zanoni
@ 2014-12-08 16:12 ` Paulo Zanoni
2014-12-08 16:12 ` [PATCH igt 2/4] tests/kms_fb_crc: call gem_sync() instead of gem_bo_busy() Paulo Zanoni
` (3 more replies)
11 siblings, 4 replies; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Just a little helper for code that needs to wait for a certain
condition to happen. It has the nice advantage that it can survive the
signal helper.
Despite the callers added in this patch, there is another that will go
in a separate patch, and another in a new IGT test file that I plan to
push later.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
lib/igt_aux.c | 18 +-----------------
lib/igt_aux.h | 40 ++++++++++++++++++++++++++++++++++++++++
tests/pm_rpm.c | 28 +++++-----------------------
3 files changed, 46 insertions(+), 40 deletions(-)
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 3051d84..180141e 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -518,29 +518,13 @@ enum igt_runtime_pm_status igt_get_runtime_pm_status(void)
* Waits until for the driver to switch to into the desired runtime PM status,
* with a 10 second timeout.
*
- * Some subtests call this function while the signal helper is active, so we
- * can't assume each usleep() call will sleep for 100ms.
- *
* Returns:
* True if the desired runtime PM status was attained, false if the operation
* timed out.
*/
bool igt_wait_for_pm_status(enum igt_runtime_pm_status status)
{
- struct timeval start, end, diff;
-
- igt_assert(gettimeofday(&start, NULL) == 0);
- do {
- if (igt_get_runtime_pm_status() == status)
- return true;
-
- usleep(100 * 1000);
-
- igt_assert(gettimeofday(&end, NULL) == 0);
- timersub(&end, &start, &diff);
- } while (diff.tv_sec < 10);
-
- return false;
+ return igt_wait(igt_get_runtime_pm_status() == status, 10000, 100);
}
/* Functions with prefix kmstest_ independent of cairo library are pulled out
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 6c83c53..48c495b 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -30,6 +30,7 @@
#include <intel_bufmgr.h>
#include <stdbool.h>
+#include <sys/time.h>
extern drm_intel_bo **trash_bos;
extern int num_trash_bos;
@@ -90,4 +91,43 @@ void intel_require_memory(uint32_t count, uint32_t size, unsigned mode);
#define min(a, b) ((a) < (b) ? (a) : (b))
#define max(a, b) ((a) > (b) ? (a) : (b))
+/**
+ * igt_wait:
+ * @COND: condition to wait
+ * @timeout_ms: timeout in milliseconds
+ * @interval_ms: amount of time we try to sleep between COND checks
+ *
+ * Waits until COND evaluates to true or the timeout passes.
+ *
+ * It is safe to call this macro if the signal helper is active. The only
+ * problem is that the usleep() calls will return early, making us evaluate COND
+ * too often, possibly eating valuable CPU cycles.
+ *
+ * Returns:
+ * True of COND evaluated to true, false otherwise.
+ */
+#define igt_wait(COND, timeout_ms, interval_ms) ({ \
+ struct timeval start_, end_, diff_; \
+ int elapsed_ms_; \
+ bool ret_ = false; \
+ \
+ igt_assert(gettimeofday(&start_, NULL) == 0); \
+ do { \
+ if (COND) { \
+ ret_ = true; \
+ break; \
+ } \
+ \
+ usleep(interval_ms * 1000); \
+ \
+ igt_assert(gettimeofday(&end_, NULL) == 0); \
+ timersub(&end_, &start_, &diff_); \
+ \
+ elapsed_ms_ = diff_.tv_sec * 1000 + \
+ diff_.tv_usec / 1000; \
+ } while (elapsed_ms_ < timeout_ms); \
+ \
+ ret_; \
+})
+
#endif /* IGT_AUX_H */
diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index c120d75..b7f9635 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -153,24 +153,16 @@ static uint64_t get_residency(uint32_t type)
static bool pc8_plus_residency_changed(unsigned int timeout_sec)
{
- unsigned int i;
uint64_t res_pc8, res_pc9, res_pc10;
- int to_sleep = 100 * 1000;
res_pc8 = get_residency(MSR_PC8_RES);
res_pc9 = get_residency(MSR_PC9_RES);
res_pc10 = get_residency(MSR_PC10_RES);
- for (i = 0; i < timeout_sec * 1000 * 1000; i += to_sleep) {
- if (res_pc8 != get_residency(MSR_PC8_RES) ||
- res_pc9 != get_residency(MSR_PC9_RES) ||
- res_pc10 != get_residency(MSR_PC10_RES)) {
- return true;
- }
- usleep(to_sleep);
- }
-
- return false;
+ return igt_wait(res_pc8 != get_residency(MSR_PC8_RES) ||
+ res_pc9 != get_residency(MSR_PC9_RES) ||
+ res_pc10 != get_residency(MSR_PC10_RES),
+ timeout_sec * 1000, 100);
}
static enum pc8_status get_pc8_status(void)
@@ -191,17 +183,7 @@ static enum pc8_status get_pc8_status(void)
static bool wait_for_pc8_status(enum pc8_status status)
{
- int i;
- int hundred_ms = 100 * 1000, ten_s = 10 * 1000 * 1000;
-
- for (i = 0; i < ten_s; i += hundred_ms) {
- if (get_pc8_status() == status)
- return true;
-
- usleep(hundred_ms);
- }
-
- return false;
+ return igt_wait(get_pc8_status() == status, 10000, 100);
}
static bool wait_for_suspended(void)
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH igt 2/4] tests/kms_fb_crc: call gem_sync() instead of gem_bo_busy()
2014-12-08 16:12 ` [PATCH igt 1/4] lib: add igt_wait() Paulo Zanoni
@ 2014-12-08 16:12 ` Paulo Zanoni
2014-12-08 16:12 ` [PATCH igt 3/4] tests/kms_fbc_crc: add wait_for_fbc_enabled() Paulo Zanoni
` (2 subsequent siblings)
3 siblings, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
The way kms_fbc_crc works is that it does an operation that may
trigger/invalidate/update FBC, then it sleeps for 300ms to wait for
FBC to kick in again, then it calls "igt_assert(fbc_enabled())".
This was causing problems where the BLT test would eventually fail in
the fbc_eanbled() assertion.
With the recent FBC move to front buffer rendering tracking, if we
don't call gem_sync() after submitting render and blt commands, it may
take much more than 300ms for FBC to be reenabled:
i915_gem_execbuffer2() indirectly calls intel_fb_obj_invalidate(),
which disables FBC, and then it is only reenabled when
i915_gem_retire_work_handler() happens and indirectly calls
intel_frontbuffer_flush(). Notice that while FBC is not yet enabled,
the screen contents are correct, so this shouldn't really be a "bug".
The gem_sync() call will make sure the long waits don't happen. With
this, 300ms should be much more than enough: either we wait about 50ms
for FBC to be re-enabled - intel_enable_fbc() uses a delayed work - or
it's instantaneous - on the cases where we just do the nuke.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
tests/kms_fbc_crc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 32c3d98..7c3a412 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -122,7 +122,7 @@ static void fill_blt(data_t *data,
intel_batchbuffer_flush(batch);
intel_batchbuffer_free(batch);
- gem_bo_busy(data->drm_fd, handle);
+ gem_sync(data->drm_fd, handle);
}
static void scratch_buf_init(struct igt_buf *buf, drm_intel_bo *bo)
@@ -187,7 +187,7 @@ static void fill_render(data_t *data, uint32_t handle,
intel_batchbuffer_free(batch);
- gem_bo_busy(data->drm_fd, handle);
+ gem_sync(data->drm_fd, handle);
}
static bool fbc_enabled(data_t *data)
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH igt 3/4] tests/kms_fbc_crc: add wait_for_fbc_enabled()
2014-12-08 16:12 ` [PATCH igt 1/4] lib: add igt_wait() Paulo Zanoni
2014-12-08 16:12 ` [PATCH igt 2/4] tests/kms_fb_crc: call gem_sync() instead of gem_bo_busy() Paulo Zanoni
@ 2014-12-08 16:12 ` Paulo Zanoni
2014-12-08 16:12 ` [PATCH igt 4/4] tests/kms_fbc_crc: also gem_sync() on exec_nop() Paulo Zanoni
2014-12-08 16:40 ` [PATCH igt 1/4] lib: add igt_wait() Daniel Vetter
3 siblings, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
The code has a common pattern of "wait 300ms, then check if FBC is
enabled". Most of the time FBC is enabled in either 50ms or 0ms, so
introduce wait_for_fbc_enabled(), which can return much earlier if FBC
is actually enabled before the 300ms timeout.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
tests/kms_fbc_crc.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 7c3a412..354a2b7 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -28,6 +28,7 @@
#include <string.h>
#include "drmtest.h"
+#include "igt_aux.h"
#include "igt_debugfs.h"
#include "igt_kms.h"
#include "intel_chipset.h"
@@ -203,6 +204,11 @@ static bool fbc_enabled(data_t *data)
return strstr(str, "FBC enabled") != NULL;
}
+static bool wait_for_fbc_enabled(data_t *data)
+{
+ return igt_wait(fbc_enabled(data), 300, 30);
+}
+
static void test_crc(data_t *data, enum test_mode mode)
{
uint32_t crtc_id = data->output->config.crtc->crtc_id;
@@ -216,9 +222,8 @@ static void test_crc(data_t *data, enum test_mode mode)
handle = data->handle[1];
igt_assert(drmModePageFlip(data->drm_fd, crtc_id,
data->fb_id[1], 0, NULL) == 0);
- usleep(300000);
- igt_assert(fbc_enabled(data));
+ igt_assert(wait_for_fbc_enabled(data));
}
switch (mode) {
@@ -278,9 +283,7 @@ static void test_crc(data_t *data, enum test_mode mode)
* Allow time for FBC to kick in again if it
* got disabled during dirtyfb or page flip.
*/
- usleep(300000);
-
- igt_assert(fbc_enabled(data));
+ igt_assert(wait_for_fbc_enabled(data));
igt_pipe_crc_start(pipe_crc);
igt_pipe_crc_get_crcs(pipe_crc, 1, &crcs);
@@ -340,9 +343,8 @@ static bool prepare_test(data_t *data, enum test_mode test_mode)
/* scanout = fb[1] */
igt_plane_set_fb(data->primary, &data->fb[1]);
igt_display_commit(display);
- usleep(300000);
- if (!fbc_enabled(data)) {
+ if (!wait_for_fbc_enabled(data)) {
igt_info("FBC not enabled\n");
igt_plane_set_fb(data->primary, NULL);
@@ -390,9 +392,8 @@ static bool prepare_test(data_t *data, enum test_mode test_mode)
/* scanout = fb[0] */
igt_plane_set_fb(data->primary, &data->fb[0]);
igt_display_commit(display);
- usleep(300000);
- igt_assert(fbc_enabled(data));
+ igt_assert(wait_for_fbc_enabled(data));
if (test_mode == TEST_CONTEXT || test_mode == TEST_PAGE_FLIP_AND_CONTEXT) {
/*
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread* [PATCH igt 4/4] tests/kms_fbc_crc: also gem_sync() on exec_nop()
2014-12-08 16:12 ` [PATCH igt 1/4] lib: add igt_wait() Paulo Zanoni
2014-12-08 16:12 ` [PATCH igt 2/4] tests/kms_fb_crc: call gem_sync() instead of gem_bo_busy() Paulo Zanoni
2014-12-08 16:12 ` [PATCH igt 3/4] tests/kms_fbc_crc: add wait_for_fbc_enabled() Paulo Zanoni
@ 2014-12-08 16:12 ` Paulo Zanoni
2014-12-08 16:40 ` [PATCH igt 1/4] lib: add igt_wait() Daniel Vetter
3 siblings, 0 replies; 41+ messages in thread
From: Paulo Zanoni @ 2014-12-08 16:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
When we're doing the context subtest, at the end of prepare_test() we
exec a single nop batch on the front buffer, which invalidates FBC.
With the new frontbuffer tracking scheme it may take a while for FBC
to be reenabled, so we end up failing the first fbc_enabled()
assertion inside test_crc().
Other possible implementations:
- Call gem_sync() at the specific prepare_test() point, not at every
exec_nop() call.
- Change the fbc_enabled() assertion to wait_for_fbc_enabled() and
give it a bigger timeout value.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
tests/kms_fbc_crc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 354a2b7..decb83e 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -155,6 +155,8 @@ static void exec_nop(data_t *data, uint32_t handle, drm_intel_context *context)
intel_batchbuffer_flush_with_context(batch, context);
intel_batchbuffer_free(batch);
+
+ gem_sync(data->drm_fd, handle);
}
static void fill_render(data_t *data, uint32_t handle,
--
2.1.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH igt 1/4] lib: add igt_wait()
2014-12-08 16:12 ` [PATCH igt 1/4] lib: add igt_wait() Paulo Zanoni
` (2 preceding siblings ...)
2014-12-08 16:12 ` [PATCH igt 4/4] tests/kms_fbc_crc: also gem_sync() on exec_nop() Paulo Zanoni
@ 2014-12-08 16:40 ` Daniel Vetter
3 siblings, 0 replies; 41+ messages in thread
From: Daniel Vetter @ 2014-12-08 16:40 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
On Mon, Dec 08, 2014 at 02:12:41PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Just a little helper for code that needs to wait for a certain
> condition to happen. It has the nice advantage that it can survive the
> signal helper.
>
> Despite the callers added in this patch, there is another that will go
> in a separate patch, and another in a new IGT test file that I plan to
> push later.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
A library unit test in lib/tests would be awesome for igt_wait. You can
run them with $ make check. If you're a bit bored, that is ;-)
-Daniel
> ---
> lib/igt_aux.c | 18 +-----------------
> lib/igt_aux.h | 40 ++++++++++++++++++++++++++++++++++++++++
> tests/pm_rpm.c | 28 +++++-----------------------
> 3 files changed, 46 insertions(+), 40 deletions(-)
>
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 3051d84..180141e 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -518,29 +518,13 @@ enum igt_runtime_pm_status igt_get_runtime_pm_status(void)
> * Waits until for the driver to switch to into the desired runtime PM status,
> * with a 10 second timeout.
> *
> - * Some subtests call this function while the signal helper is active, so we
> - * can't assume each usleep() call will sleep for 100ms.
> - *
> * Returns:
> * True if the desired runtime PM status was attained, false if the operation
> * timed out.
> */
> bool igt_wait_for_pm_status(enum igt_runtime_pm_status status)
> {
> - struct timeval start, end, diff;
> -
> - igt_assert(gettimeofday(&start, NULL) == 0);
> - do {
> - if (igt_get_runtime_pm_status() == status)
> - return true;
> -
> - usleep(100 * 1000);
> -
> - igt_assert(gettimeofday(&end, NULL) == 0);
> - timersub(&end, &start, &diff);
> - } while (diff.tv_sec < 10);
> -
> - return false;
> + return igt_wait(igt_get_runtime_pm_status() == status, 10000, 100);
> }
>
> /* Functions with prefix kmstest_ independent of cairo library are pulled out
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 6c83c53..48c495b 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -30,6 +30,7 @@
>
> #include <intel_bufmgr.h>
> #include <stdbool.h>
> +#include <sys/time.h>
>
> extern drm_intel_bo **trash_bos;
> extern int num_trash_bos;
> @@ -90,4 +91,43 @@ void intel_require_memory(uint32_t count, uint32_t size, unsigned mode);
> #define min(a, b) ((a) < (b) ? (a) : (b))
> #define max(a, b) ((a) > (b) ? (a) : (b))
>
> +/**
> + * igt_wait:
> + * @COND: condition to wait
> + * @timeout_ms: timeout in milliseconds
> + * @interval_ms: amount of time we try to sleep between COND checks
> + *
> + * Waits until COND evaluates to true or the timeout passes.
> + *
> + * It is safe to call this macro if the signal helper is active. The only
> + * problem is that the usleep() calls will return early, making us evaluate COND
> + * too often, possibly eating valuable CPU cycles.
> + *
> + * Returns:
> + * True of COND evaluated to true, false otherwise.
> + */
> +#define igt_wait(COND, timeout_ms, interval_ms) ({ \
> + struct timeval start_, end_, diff_; \
> + int elapsed_ms_; \
> + bool ret_ = false; \
> + \
> + igt_assert(gettimeofday(&start_, NULL) == 0); \
> + do { \
> + if (COND) { \
> + ret_ = true; \
> + break; \
> + } \
> + \
> + usleep(interval_ms * 1000); \
> + \
> + igt_assert(gettimeofday(&end_, NULL) == 0); \
> + timersub(&end_, &start_, &diff_); \
> + \
> + elapsed_ms_ = diff_.tv_sec * 1000 + \
> + diff_.tv_usec / 1000; \
> + } while (elapsed_ms_ < timeout_ms); \
> + \
> + ret_; \
> +})
> +
> #endif /* IGT_AUX_H */
> diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> index c120d75..b7f9635 100644
> --- a/tests/pm_rpm.c
> +++ b/tests/pm_rpm.c
> @@ -153,24 +153,16 @@ static uint64_t get_residency(uint32_t type)
>
> static bool pc8_plus_residency_changed(unsigned int timeout_sec)
> {
> - unsigned int i;
> uint64_t res_pc8, res_pc9, res_pc10;
> - int to_sleep = 100 * 1000;
>
> res_pc8 = get_residency(MSR_PC8_RES);
> res_pc9 = get_residency(MSR_PC9_RES);
> res_pc10 = get_residency(MSR_PC10_RES);
>
> - for (i = 0; i < timeout_sec * 1000 * 1000; i += to_sleep) {
> - if (res_pc8 != get_residency(MSR_PC8_RES) ||
> - res_pc9 != get_residency(MSR_PC9_RES) ||
> - res_pc10 != get_residency(MSR_PC10_RES)) {
> - return true;
> - }
> - usleep(to_sleep);
> - }
> -
> - return false;
> + return igt_wait(res_pc8 != get_residency(MSR_PC8_RES) ||
> + res_pc9 != get_residency(MSR_PC9_RES) ||
> + res_pc10 != get_residency(MSR_PC10_RES),
> + timeout_sec * 1000, 100);
> }
>
> static enum pc8_status get_pc8_status(void)
> @@ -191,17 +183,7 @@ static enum pc8_status get_pc8_status(void)
>
> static bool wait_for_pc8_status(enum pc8_status status)
> {
> - int i;
> - int hundred_ms = 100 * 1000, ten_s = 10 * 1000 * 1000;
> -
> - for (i = 0; i < ten_s; i += hundred_ms) {
> - if (get_pc8_status() == status)
> - return true;
> -
> - usleep(hundred_ms);
> - }
> -
> - return false;
> + return igt_wait(get_pc8_status() == status, 10000, 100);
> }
>
> static bool wait_for_suspended(void)
> --
> 2.1.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 41+ messages in thread