diff mbox

drm/i915: deferring the HSW IPS enable at plane switch

Message ID 1390135633-10465-1-git-send-email-ramalingam.c@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ramalingam C Jan. 19, 2014, 12:47 p.m. UTC
To remove the wait_for_vblank from the plane switch execution path,
this change implements a function which will add a delayed work to
defer the IPS enable.

The delay is nothing but frame length, which is calculated based on
vrefresh of the hwmode. i.e IPS enable will be scheduled after a frame.
If mode is not set during the plane switch (__unlikely__), delay will
fallback to a default value 100mSec (can handle the FPS >= 10).

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   58 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |    8 +++++
 drivers/gpu/drm/i915/intel_sprite.c  |    3 +-
 3 files changed, 67 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Jan. 21, 2014, 1:15 p.m. UTC | #1
On Sun, Jan 19, 2014 at 1:47 PM, Ramalingam C <ramalingam.c@intel.com> wrote:
> To remove the wait_for_vblank from the plane switch execution path,
> this change implements a function which will add a delayed work to
> defer the IPS enable.
>
> The delay is nothing but frame length, which is calculated based on
> vrefresh of the hwmode. i.e IPS enable will be scheduled after a frame.
> If mode is not set during the plane switch (__unlikely__), delay will
> fallback to a default value 100mSec (can handle the FPS >= 10).
>
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>

We have a general need to do abitrary stuff in both irq and process
context on future vblanks. Chris started with such infrastructure
already:

http://lists.freedesktop.org/archives/intel-gfx/2012-April/016557.html

We've had a fair bit of discussion about where to go to with all this,
but atm this is stalled. I also think we should have this bit of
infrastructure generally available for drivers in drm_vblank.c.

The other thing to keep in mind is to make this fit with Ville's plan
for nuclear pageflips. I think in the end the modeset code should only
fire up the pipe itself (i.e. we'll display a nice black) and enabling
pipes, sprites and cursors should happen with the nuclear pageflip
code. Otherwise we need to duplicate this logic, which usually means
twice the bugs.
-Daniel
Ville Syrjala Jan. 22, 2014, 7:10 p.m. UTC | #2
On Tue, Jan 21, 2014 at 02:15:51PM +0100, Daniel Vetter wrote:
> On Sun, Jan 19, 2014 at 1:47 PM, Ramalingam C <ramalingam.c@intel.com> wrote:
> > To remove the wait_for_vblank from the plane switch execution path,
> > this change implements a function which will add a delayed work to
> > defer the IPS enable.
> >
> > The delay is nothing but frame length, which is calculated based on
> > vrefresh of the hwmode. i.e IPS enable will be scheduled after a frame.
> > If mode is not set during the plane switch (__unlikely__), delay will
> > fallback to a default value 100mSec (can handle the FPS >= 10).
> >
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> 
> We have a general need to do abitrary stuff in both irq and process
> context on future vblanks. Chris started with such infrastructure
> already:
> 
> http://lists.freedesktop.org/archives/intel-gfx/2012-April/016557.html
> 
> We've had a fair bit of discussion about where to go to with all this,
> but atm this is stalled. I also think we should have this bit of
> infrastructure generally available for drivers in drm_vblank.c.
> 
> The other thing to keep in mind is to make this fit with Ville's plan
> for nuclear pageflips. I think in the end the modeset code should only
> fire up the pipe itself (i.e. we'll display a nice black) and enabling
> pipes, sprites and cursors should happen with the nuclear pageflip
> code. Otherwise we need to duplicate this logic, which usually means
> twice the bugs.

Another thing we'd need there is a similar mechanism but for GPU. So get
a callback when your target seqno has passed. I have something along
those lines in the atomic branch but maybe it could be generalized.

For FBC I'd more or less like to have both since I need actually first
wait for a seqno, and then for the vblank. But maybe I can make it work
with just the current wait_seqno. Didn't actually try yet.

Also my watermark update code has another vblank work mechanism which
actually looks at the frame counter to make sure work gets executed
after the target frame count has passed. Maybe there's another
possibility for unification here. I'll need to post those patches first
though.
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e77d4b8..9aa66d5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -52,6 +52,7 @@  static void ironlake_pch_clock_get(struct intel_crtc *crtc,
 static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
 			  int x, int y, struct drm_framebuffer *old_fb);
 
+#define IPS_DEFAULT_SCHEDULED_DELAY_MSECS	100	/* For >=10FPS */
 
 typedef struct {
 	int	min, max;
@@ -3452,6 +3453,53 @@  void hsw_enable_ips(struct intel_crtc *crtc)
 	}
 }
 
+void intel_ips_deferred_work_fn(struct work_struct *__work)
+{
+	struct intel_ips_deferred_work *work = container_of(
+		to_delayed_work(__work), struct intel_ips_deferred_work,
+								delayed_work);
+	struct intel_crtc *intel_crtc = to_intel_crtc(work->crtc);
+
+	hsw_enable_ips(intel_crtc);
+	kfree(work);
+	intel_crtc->ips_deferred_work = NULL;
+}
+
+int hsw_deferred_ips_enable(struct drm_crtc *crtc)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_ips_deferred_work *work;
+	int delay = 0;
+
+	/* Canceling any previous pending IPS deferred work, if any */
+	if (intel_crtc->ips_deferred_work) {
+		work = intel_crtc->ips_deferred_work;
+		cancel_delayed_work_sync(&work->delayed_work);
+		kfree(work);
+		intel_crtc->ips_deferred_work = NULL;
+	}
+
+	/* Awaits unnecessary load to wq */
+	if (!intel_crtc->primary_enabled)
+		return -EPERM;
+
+	work = kzalloc(sizeof(*work), GFP_KERNEL);
+	if (work == NULL)
+		return -ENOMEM;
+
+	work->crtc = crtc;
+	INIT_DELAYED_WORK(&work->delayed_work, intel_ips_deferred_work_fn);
+	intel_crtc->ips_deferred_work = work;
+
+	/* If mode is set, frame length is calculated, else default delay
+	 * is used */
+	delay = crtc->hwmode.vrefresh ? (1000 / crtc->hwmode.vrefresh) :
+					IPS_DEFAULT_SCHEDULED_DELAY_MSECS;
+	schedule_delayed_work(&work->delayed_work, msecs_to_jiffies(delay));
+
+	return 0;
+}
+
 void hsw_disable_ips(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
@@ -8218,6 +8266,7 @@  static void intel_crtc_destroy(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct intel_unpin_work *work;
+	struct intel_ips_deferred_work *ips_deferred_work;
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev->event_lock, flags);
@@ -8230,6 +8279,13 @@  static void intel_crtc_destroy(struct drm_crtc *crtc)
 		kfree(work);
 	}
 
+	if (intel_crtc->ips_deferred_work) {
+		ips_deferred_work = intel_crtc->ips_deferred_work;
+		cancel_delayed_work_sync(&(ips_deferred_work->delayed_work));
+		kfree(ips_deferred_work);
+		intel_crtc->ips_deferred_work = NULL;
+	}
+
 	intel_crtc_cursor_set(crtc, NULL, 0, 0, 0);
 
 	drm_crtc_cleanup(crtc);
@@ -10193,6 +10249,8 @@  static void intel_crtc_init(struct drm_device *dev, int pipe)
 		intel_crtc->plane = !pipe;
 	}
 
+	intel_crtc->ips_deferred_work = NULL;
+
 	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
 	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
 	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8754db9..aad4cf5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -347,6 +347,8 @@  struct intel_crtc {
 
 	atomic_t unpin_work_count;
 
+	struct intel_ips_deferred_work *ips_deferred_work;
+
 	/* Display surface base address adjustement for pageflips. Note that on
 	 * gen4+ this only adjusts up to a tile, offsets within a tile are
 	 * handled in the hw itself (with the TILEOFF register). */
@@ -524,6 +526,11 @@  intel_get_crtc_for_plane(struct drm_device *dev, int plane)
 	return dev_priv->plane_to_crtc_mapping[plane];
 }
 
+struct intel_ips_deferred_work {
+	struct delayed_work delayed_work;
+	struct drm_crtc *crtc;
+};
+
 struct intel_unpin_work {
 	struct work_struct work;
 	struct drm_crtc *crtc;
@@ -707,6 +714,7 @@  ironlake_check_encoder_dotclock(const struct intel_crtc_config *pipe_config,
 bool intel_crtc_active(struct drm_crtc *crtc);
 void hsw_enable_ips(struct intel_crtc *crtc);
 void hsw_disable_ips(struct intel_crtc *crtc);
+int hsw_deferred_ips_enable(struct drm_crtc *crtc);
 void intel_display_set_init_power(struct drm_device *dev, bool enable);
 int valleyview_get_vco(struct drm_i915_private *dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index fe4de89..e2c71c8 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -541,8 +541,7 @@  intel_enable_primary(struct drm_crtc *crtc)
 	 * versa.
 	 */
 	if (intel_crtc->config.ips_enabled) {
-		intel_wait_for_vblank(dev, intel_crtc->pipe);
-		hsw_enable_ips(intel_crtc);
+		hsw_deferred_ips_enable(crtc);
 	}
 
 	mutex_lock(&dev->struct_mutex);