Re: [Intel-gfx] [PATCH 3/3] drm/i915: Make module param for MMIO flip selection as tristate

[Chris Wilson]

I was thinking this patch should be more like

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3201495..ab9b5f7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2060,7 +2060,7 @@ struct i915_params {
bool reset;
bool disable_display;
bool disable_vtd_wa;
-   bool use_mmio_flip;
+   int use_mmio_flip;
 };
 extern struct i915_params i915 __read_mostly;
 
diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index e0d44df..6d7c580 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -158,5 +158,5 @@ module_param_named(enable_cmd_parser, 
i915.enable_cmd_parser, int, 0600);
 MODULE_PARM_DESC(enable_cmd_parser,
 Enable command parsing (1=enabled [default], 0=disabled));
 
-module_param_named(use_mmio_flip, i915.use_mmio_flip, bool, 0600);
-MODULE_PARM_DESC(use_mmio_flip, use MMIO flips (default: false));
+module_param_named(use_mmio_flip, i915.use_mmio_flip, int, 0600);
+MODULE_PARM_DESC(use_mmio_flip, use MMIO flips (-1=never, 0=driver discretion 
[default], 1=always));
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ac93ae4..b6c8fce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9207,24 +9207,24 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
return 0;
 }
 
-static bool intel_use_mmio_flip(struct drm_device *dev)
+static bool use_mmio_flip(struct intel_engine_cs *ring,
+ struct drm_i915_gem_object *obj)
 {
-   /* If module parameter is disabled, use CS flips.
-* Otherwise, use MMIO flips starting from Gen5.
-* This is not being used for older platforms, because
+   /* This is not being used for older platforms, because
 * non-availability of flip done interrupt forces us to use
 * CS flips. Older platforms derive flip done using some clever
 * tricks involving the flip_pending status bits and vblank irqs.
 * So using MMIO flips there would disrupt this mechanism.
 */
-
-   if (i915.use_mmio_flip == 0)
+   if (INTEL_INFO(dev)-gen  5)
return false;
 
-   if (INTEL_INFO(dev)-gen = 5)
+   if (i915.use_mmio_flip  0)
+   return false;
+   else if (i915.use_mmio_flip  0)
return true;
else
-   return false;
+   return ring != obj-ring;
 }
 
 static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
@@ -9290,9 +9290,9 @@ void intel_notify_mmio_flip(struct intel_engine_cs *ring)
struct intel_mmio_flip *mmio_flip;
 
mmio_flip = intel_crtc-mmio_flip;
-
if (mmio_flip-seqno == 0)
continue;
+
if (ring-id != mmio_flip-ring_id)
continue;
 
@@ -9306,26 +9306,25 @@ void intel_notify_mmio_flip(struct intel_engine_cs 
*ring)
 }
 
 static int intel_queue_mmio_flip(struct drm_device *dev,
-   struct drm_crtc *crtc,
-   struct drm_framebuffer *fb,
-   struct drm_i915_gem_object *obj,
-   struct intel_engine_cs *ring,
-   uint32_t flags)
+struct drm_crtc *crtc,
+struct drm_framebuffer *fb,
+struct drm_i915_gem_object *obj,
+struct intel_engine_cs *ring,
+uint32_t flags)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
unsigned long irq_flags;
int ret;
 
-   if (WARN_ON(intel_crtc-mmio_flip.seqno)) {
-   ret = -EBUSY;
-   goto err;
-   }
+   if (WARN_ON(intel_crtc-mmio_flip.seqno))
+   return -EBUSY;
 
ret = intel_postpone_flip(obj);
-   if (ret  0) {
-   goto err;
-   } else if (ret == 0) {
+   if (ret  0)
+   return ret;
+
+   if (ret == 0) {
intel_do_mmio_flip(intel_crtc);
return 0;
}
@@ -9340,8 +9339,6 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
 */
intel_notify_mmio_flip(obj-ring);
return 0;
-err:
-   return ret;
 }
 
 static int intel_default_queue_flip(struct drm_device *dev,
@@ -9529,7 +9526,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
i915_gem_obj_ggtt_offset(obj) + intel_crtc-dspaddr_offset;
work-enable_stall_check = true;
 
-   ret = dev_priv-display.queue_flip(dev, crtc, fb, obj, ring, 
page_flip_flags);
+   if (use_mmio_flip(ring, obj))
+   ret = intel_queue_mmio_flip(ring, obj)(dev, crtc, fb, obj, 
ring, page_flip_flags);
+   else
+

[Intel-gfx] [PATCH 3/3] drm/i915: Make module param for MMIO flip selection as tristate

[sourab . gupta]

From: Sourab Gupta sourab.gu...@intel.com

This patch enhances the module parameter, 'use_mmio_flip' which
enables MMIO flips, to make it tristate. The values being-
0: Force CS flip
1: Force MMIO flip (Gen5+)
1: Driver discretion is applied while selecting CS vs MMIO flip.

For Valleyview, this driver selection happens based on the idleness
of Blitter and Video engines. The Blitter and Video engines are in
the same power well. So, if both are idle, we can use MMIO flips,
Otherwise, we can use the BCS flips.
This usecase can be modified and/or enhanced to cover more platforms.

Signed-off-by: Sourab Gupta sourab.gu...@intel.com
---
 drivers/gpu/drm/i915/i915_params.c   |  4 +++-
 drivers/gpu/drm/i915/intel_display.c | 42 ++--
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index e0d44df..9becd1e 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -159,4 +159,6 @@ MODULE_PARM_DESC(enable_cmd_parser,
 Enable command parsing (1=enabled [default], 0=disabled));
 
 module_param_named(use_mmio_flip, i915.use_mmio_flip, bool, 0600);
-MODULE_PARM_DESC(use_mmio_flip, use MMIO flips (default: false));
+MODULE_PARM_DESC(use_mmio_flip, use MMIO page flips
+   (0 force CS, 1:force mmio, 1: driver selection)
+   (default: 0));
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 9dda965..a3d38d2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9185,22 +9185,50 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
 
 static bool intel_use_mmio_flip(struct drm_device *dev)
 {
-   /* If module parameter is disabled, use CS flips.
-* Otherwise, use MMIO flips starting from Gen5.
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   bool use_mmio_flip = false;
+
+   /* If module parameter is 0, force CS flip.
+* If module parameter is 1, force MMIO flip starting from Gen5.
 * This is not being used for older platforms, because
 * non-availability of flip done interrupt forces us to use
 * CS flips. Older platforms derive flip done using some clever
 * tricks involving the flip_pending status bits and vblank irqs.
 * So using MMIO flips there would disrupt this mechanism.
+* If module parameter is  1, driver discretion is applied for
+* selection of CS vs MMIO flip.
 */
 
if (i915.use_mmio_flip == 0)
-   return false;
+   use_mmio_flip = false;
 
-   if (INTEL_INFO(dev)-gen = 5)
-   return true;
-   else
-   return false;
+   if (i915.use_mmio_flip == 1) {
+   if (INTEL_INFO(dev)-gen = 5)
+   use_mmio_flip = true;
+   else
+   use_mmio_flip = false;
+   }
+
+   if (i915.use_mmio_flip  1) {
+   /* For Valleyview, Blitter and Video engines are in the same
+* power well. So, if both are idle, we can use MMIO flips,
+* Otherwise, we can use the BCS flips.
+* We use the parameter 'request_list' to determine the idleness
+* of the engine.
+*/
+   if (IS_VALLEYVIEW(dev)) {
+   struct intel_engine_cs *bcs_ring = dev_priv-ring[BCS];
+   struct intel_engine_cs *vcs_ring = dev_priv-ring[VCS];
+
+   if (list_empty(bcs_ring-request_list) 
+   list_empty(vcs_ring-request_list))
+   use_mmio_flip = true;
+   else
+   use_mmio_flip = false;
+   } else
+   use_mmio_flip = false;
+   }
+   return use_mmio_flip;
 }
 
 static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
-- 
1.8.5.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx