[PATCH 00/25] Fix FBC for real

[PATCH 00/25] Fix FBC for real

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

This series rewrites the FBC code to actually work. It utilizes the
hardware tracking/nuking as much as possible, eg. relying on hardware
nuke on flip when possible.

I also introduce the generic ring and vblank notifier gizmos which could
be used for various other things. I already included a patch to convert
the IPS enable to be asynchronous by using the vblank notifier. Other
users for thse could be mmio flips, watermark programming, atomic
gamma/color correction (single buffered registers all) updates from vblank
interrupt, etc.

There's also a rather big behavioural change that FBC now stays enabled
even when multiple pipes are active. FBC just automatially migrates to
the primary plane where it's deemed most beneficial. We do that
determination by looking at the rate at which the plane is pulling data
(pixel rate * cpp). That seems like a reasonable choice all else being
equal. Eventually we might want to adjust the FBC score based on
nuke/invalidate frequency as well.

The locking now has a new fbc.mutex. I had to split the page flip code
apart a bit to accomondate. I'm not entirely sure if it wouldn't be
better to just keep struct_mutex locked all through there, but then
I'd need rework the locking in the fbc code to not take struct_mutex
when called from the page flip code. And then I'd have to start
questioning whether fbc.mutex has any point existing.

I pushed the lot here:
git://gitorious.org/vsyrjala/linux.git fbc_update_thing_14

Ville Syrjälä (25):
  drm/i915: Add ring_notify mechanism
  drm/i915: Add vblank notify mechanism
  drm/i915: Name the IPS bits
  drm/i915: Use vblank notifier for IPS
  drm/i915: Reogranize page flip code for fbc
  drm/i915: Move ilk_pipe_pixel_rate() earlier to avoid forward
    declaration
  drm/i915: Reorganize intel_update_fbc()
  drm/i915: Check panel fitting state before enabling fbc
  drm/i915: Reject fbc on g4x when sprites are enabled
  drm/i915: Check pixel format for fbc
  drm/i915: Remove dblscan flag from fbc1 check
  drm/i915: Don't claim fbc as possible if the obj size exceeds stolen
    size
  drm/i915: Use low level funciton to disable fbc at init/resume
  drm/i915: Move fbc function prototypes got intel_drv.h
  drm/i915: Move fbc state into dev_priv.fbc
  drm/i915: Rewrite fbc
  drm/i915: Reduce dmesg spam from FBC enable
  drm/i915: Add i915_fbc_info debugfs file
  drm/i915: Implement LRI based FBC tracking
  drm/i915: Use LRI based FBC render tracking for ILK
  drm/i915: Reorder i915_gem_execbuffer_move_to_gpu() and
    i915_switch_context()
  drm/i915: Flush caches for scanout during cpu->gtt move
  drm/i915: Nuke FBC from SW_FINISH ioctl
  drm/i915: Pimp fbc render/blitter tracking
  drm/i915: Enable fbc for ilk+ by default

 drivers/gpu/drm/i915/i915_debugfs.c        |   60 +-
 drivers/gpu/drm/i915/i915_drv.c            |    3 +
 drivers/gpu/drm/i915/i915_drv.h            |   44 +-
 drivers/gpu/drm/i915/i915_gem.c            |   23 +-
 drivers/gpu/drm/i915/i915_gem_context.c    |   14 +
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   40 +-
 drivers/gpu/drm/i915/i915_irq.c            |   12 +
 drivers/gpu/drm/i915/i915_reg.h            |    3 +-
 drivers/gpu/drm/i915/i915_suspend.c        |    5 +-
 drivers/gpu/drm/i915/intel_display.c       |  399 ++++++++--
 drivers/gpu/drm/i915/intel_drv.h           |   30 +-
 drivers/gpu/drm/i915/intel_pm.c            | 1099 +++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_ringbuffer.c    |  154 +++-
 drivers/gpu/drm/i915/intel_ringbuffer.h    |   26 +-
 drivers/gpu/drm/i915/intel_sprite.c        |   15 +-
 15 files changed, 1483 insertions(+), 444 deletions(-)

-- 
1.8.5.5

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

[PATCH 01/25] drm/i915: Add ring_notify mechanism

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Proveide a ring notify mechanism where you can ask for a callback when a
specific seqno has been passed.

Can be used for FBC and mmio flips at least.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         | 10 ++++
 drivers/gpu/drm/i915/i915_irq.c         |  4 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 89 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.h | 17 +++++++
 4 files changed, 120 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d857f58..a5e62cb 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2521,6 +2521,8 @@ static void i915_gem_reset_ring_status(struct drm_i915_private *dev_priv,
 static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
 					struct intel_engine_cs *ring)
 {
+	struct intel_ring_notify *notify, *next;
+
 	while (!list_empty(&ring->active_list)) {
 		struct drm_i915_gem_object *obj;
 
@@ -2552,6 +2554,14 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
 	kfree(ring->preallocated_lazy_request);
 	ring->preallocated_lazy_request = NULL;
 	ring->outstanding_lazy_seqno = 0;
+
+	spin_lock_irq(&ring->lock);
+	list_for_each_entry_safe(notify, next, &ring->notify_list, list) {
+		intel_ring_notify_complete(notify);
+		/* FIXME should we notify at reset? */
+		notify->notify(notify);
+	}
+	spin_unlock_irq(&ring->lock);
 }
 
 void i915_gem_restore_fences(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1c1ec22..218f011 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1221,6 +1221,10 @@ static void notify_ring(struct drm_device *dev,
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		intel_notify_mmio_flip(ring);
 
+	spin_lock(&ring->lock);
+	intel_ring_notify_check(ring);
+	spin_unlock(&ring->lock);
+
 	wake_up_all(&ring->irq_queue);
 	i915_queue_hangcheck(dev);
 }
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index b96edaf..31321ae 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1447,6 +1447,9 @@ static int intel_init_ring_buffer(struct drm_device *dev,
 
 	init_waitqueue_head(&ring->irq_queue);
 
+	INIT_LIST_HEAD(&ring->notify_list);
+	spin_lock_init(&ring->lock);
+
 	if (I915_NEED_GFX_HWS(dev)) {
 		ret = init_status_page(ring);
 		if (ret)
@@ -2407,3 +2410,89 @@ intel_stop_ring_buffer(struct intel_engine_cs *ring)
 
 	stop_ring(ring);
 }
+
+void intel_ring_notify_complete(struct intel_ring_notify *notify)
+{
+	struct intel_engine_cs *ring = notify->ring;
+
+	ring->irq_put(ring);
+	list_del(&notify->list);
+	notify->ring = NULL;
+}
+
+void intel_ring_notify_check(struct intel_engine_cs *ring)
+{
+	struct intel_ring_notify *notify, *next;
+	u32 seqno;
+
+	assert_spin_locked(&ring->lock);
+
+	if (list_empty(&ring->notify_list))
+		return;
+
+	seqno = ring->get_seqno(ring, false);
+
+	list_for_each_entry_safe(notify, next, &ring->notify_list, list) {
+		if (i915_seqno_passed(seqno, notify->seqno)) {
+			intel_ring_notify_complete(notify);
+			notify->notify(notify);
+		}
+	}
+}
+
+int intel_ring_notify_add(struct intel_engine_cs *ring,
+			  struct intel_ring_notify *notify)
+{
+	unsigned long irqflags;
+	int ret;
+
+	lockdep_assert_held(&ring->dev->struct_mutex);
+
+	if (WARN_ON(notify->ring != NULL || notify->seqno == 0))
+		return -EINVAL;
+
+	if (i915_seqno_passed(ring->get_seqno(ring, true), notify->seqno))
+		goto notify_immediately;
+
+	ret = i915_gem_check_olr(ring, notify->seqno);
+	if (ret)
+		return ret;
+
+	if (WARN_ON(!ring->irq_get(ring)))
+		goto notify_immediately;
+
+	spin_lock_irqsave(&ring->lock, irqflags);
+	notify->ring = ring;
+	list_add_tail(&notify->list, &ring->notify_list);
+	/* check again in case we just missed it */
+	intel_ring_notify_check(ring);
+	spin_unlock_irqrestore(&ring->lock, irqflags);
+
+	return 0;
+
+ notify_immediately:
+	spin_lock_irqsave(&ring->lock, irqflags);
+	notify->notify(notify);
+	spin_unlock_irqrestore(&ring->lock, irqflags);
+
+	return 0;
+}
+
+bool intel_ring_notify_pending(struct intel_ring_notify *notify)
+{
+	return notify->ring != NULL;
+}
+
+void intel_ring_notify_cancel(struct intel_ring_notify *notify)
+{
+	struct intel_engine_cs *ring = ACCESS_ONCE(notify->ring);
+	unsigned long irqflags;
+
+	if (!ring)
+		return;
+
+	spin_lock_irqsave(&ring->lock, irqflags);
+	if (notify->ring)
+		intel_ring_notify_complete(notify);
+	spin_unlock_irqrestore(&ring->lock, irqflags);
+}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index e72017b..273abf3 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -79,6 +79,13 @@ struct intel_ringbuffer {
 	u32 last_retired_head;
 };
 
+struct intel_ring_notify {
+	void (*notify)(struct intel_ring_notify *notify);
+	struct intel_engine_cs *ring;
+	struct list_head list;
+	u32 seqno;
+};
+
 struct  intel_engine_cs {
 	const char	*name;
 	enum intel_ring_id {
@@ -217,6 +224,9 @@ struct  intel_engine_cs {
 	 * to encode the command length in the header).
 	 */
 	u32 (*get_cmd_length_mask)(u32 cmd_header);
+
+	struct list_head notify_list;
+	spinlock_t lock;
 };
 
 static inline bool
@@ -335,6 +345,13 @@ static inline void i915_trace_irq_get(struct intel_engine_cs *ring, u32 seqno)
 		ring->trace_irq_seqno = seqno;
 }
 
+void intel_ring_notify_complete(struct intel_ring_notify *notify);
+void intel_ring_notify_check(struct intel_engine_cs *ring);
+int __must_check intel_ring_notify_add(struct intel_engine_cs *ring,
+				       struct intel_ring_notify *notify);
+bool intel_ring_notify_pending(struct intel_ring_notify *notify);
+void intel_ring_notify_cancel(struct intel_ring_notify *notify);
+
 /* DRI warts */
 int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size);
 
-- 
1.8.5.5

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

Re: [PATCH 01/25] drm/i915: Add ring_notify mechanism

[Daniel Vetter]

On Wed, Jun 18, 2014 at 08:58:34PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Proveide a ring notify mechanism where you can ask for a callback when a
> specific seqno has been passed.
> 
> Can be used for FBC and mmio flips at least.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Hm, most of the callbacks I can image need some process context anyway due
to at least grabbing a mutex or something similar. And we support lockless
seqno waits. So with that I'd lean towards offloading this into work items
and just doing a lockless wait in the overall sequence since doing full
callback-based programming is hard. But I haven't looked at the users at
all here tbh.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_gem.c         | 10 ++++
>  drivers/gpu/drm/i915/i915_irq.c         |  4 ++
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 89 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.h | 17 +++++++
>  4 files changed, 120 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index d857f58..a5e62cb 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2521,6 +2521,8 @@ static void i915_gem_reset_ring_status(struct drm_i915_private *dev_priv,
>  static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
>  					struct intel_engine_cs *ring)
>  {
> +	struct intel_ring_notify *notify, *next;
> +
>  	while (!list_empty(&ring->active_list)) {
>  		struct drm_i915_gem_object *obj;
>  
> @@ -2552,6 +2554,14 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
>  	kfree(ring->preallocated_lazy_request);
>  	ring->preallocated_lazy_request = NULL;
>  	ring->outstanding_lazy_seqno = 0;
> +
> +	spin_lock_irq(&ring->lock);
> +	list_for_each_entry_safe(notify, next, &ring->notify_list, list) {
> +		intel_ring_notify_complete(notify);
> +		/* FIXME should we notify at reset? */
> +		notify->notify(notify);
> +	}
> +	spin_unlock_irq(&ring->lock);
>  }
>  
>  void i915_gem_restore_fences(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 1c1ec22..218f011 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1221,6 +1221,10 @@ static void notify_ring(struct drm_device *dev,
>  	if (drm_core_check_feature(dev, DRIVER_MODESET))
>  		intel_notify_mmio_flip(ring);
>  
> +	spin_lock(&ring->lock);
> +	intel_ring_notify_check(ring);
> +	spin_unlock(&ring->lock);
> +
>  	wake_up_all(&ring->irq_queue);
>  	i915_queue_hangcheck(dev);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index b96edaf..31321ae 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1447,6 +1447,9 @@ static int intel_init_ring_buffer(struct drm_device *dev,
>  
>  	init_waitqueue_head(&ring->irq_queue);
>  
> +	INIT_LIST_HEAD(&ring->notify_list);
> +	spin_lock_init(&ring->lock);
> +
>  	if (I915_NEED_GFX_HWS(dev)) {
>  		ret = init_status_page(ring);
>  		if (ret)
> @@ -2407,3 +2410,89 @@ intel_stop_ring_buffer(struct intel_engine_cs *ring)
>  
>  	stop_ring(ring);
>  }
> +
> +void intel_ring_notify_complete(struct intel_ring_notify *notify)
> +{
> +	struct intel_engine_cs *ring = notify->ring;
> +
> +	ring->irq_put(ring);
> +	list_del(&notify->list);
> +	notify->ring = NULL;
> +}
> +
> +void intel_ring_notify_check(struct intel_engine_cs *ring)
> +{
> +	struct intel_ring_notify *notify, *next;
> +	u32 seqno;
> +
> +	assert_spin_locked(&ring->lock);
> +
> +	if (list_empty(&ring->notify_list))
> +		return;
> +
> +	seqno = ring->get_seqno(ring, false);
> +
> +	list_for_each_entry_safe(notify, next, &ring->notify_list, list) {
> +		if (i915_seqno_passed(seqno, notify->seqno)) {
> +			intel_ring_notify_complete(notify);
> +			notify->notify(notify);
> +		}
> +	}
> +}
> +
> +int intel_ring_notify_add(struct intel_engine_cs *ring,
> +			  struct intel_ring_notify *notify)
> +{
> +	unsigned long irqflags;
> +	int ret;
> +
> +	lockdep_assert_held(&ring->dev->struct_mutex);
> +
> +	if (WARN_ON(notify->ring != NULL || notify->seqno == 0))
> +		return -EINVAL;
> +
> +	if (i915_seqno_passed(ring->get_seqno(ring, true), notify->seqno))
> +		goto notify_immediately;
> +
> +	ret = i915_gem_check_olr(ring, notify->seqno);
> +	if (ret)
> +		return ret;
> +
> +	if (WARN_ON(!ring->irq_get(ring)))
> +		goto notify_immediately;
> +
> +	spin_lock_irqsave(&ring->lock, irqflags);
> +	notify->ring = ring;
> +	list_add_tail(&notify->list, &ring->notify_list);
> +	/* check again in case we just missed it */
> +	intel_ring_notify_check(ring);
> +	spin_unlock_irqrestore(&ring->lock, irqflags);
> +
> +	return 0;
> +
> + notify_immediately:
> +	spin_lock_irqsave(&ring->lock, irqflags);
> +	notify->notify(notify);
> +	spin_unlock_irqrestore(&ring->lock, irqflags);
> +
> +	return 0;
> +}
> +
> +bool intel_ring_notify_pending(struct intel_ring_notify *notify)
> +{
> +	return notify->ring != NULL;
> +}
> +
> +void intel_ring_notify_cancel(struct intel_ring_notify *notify)
> +{
> +	struct intel_engine_cs *ring = ACCESS_ONCE(notify->ring);
> +	unsigned long irqflags;
> +
> +	if (!ring)
> +		return;
> +
> +	spin_lock_irqsave(&ring->lock, irqflags);
> +	if (notify->ring)
> +		intel_ring_notify_complete(notify);
> +	spin_unlock_irqrestore(&ring->lock, irqflags);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index e72017b..273abf3 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -79,6 +79,13 @@ struct intel_ringbuffer {
>  	u32 last_retired_head;
>  };
>  
> +struct intel_ring_notify {
> +	void (*notify)(struct intel_ring_notify *notify);
> +	struct intel_engine_cs *ring;
> +	struct list_head list;
> +	u32 seqno;
> +};
> +
>  struct  intel_engine_cs {
>  	const char	*name;
>  	enum intel_ring_id {
> @@ -217,6 +224,9 @@ struct  intel_engine_cs {
>  	 * to encode the command length in the header).
>  	 */
>  	u32 (*get_cmd_length_mask)(u32 cmd_header);
> +
> +	struct list_head notify_list;
> +	spinlock_t lock;
>  };
>  
>  static inline bool
> @@ -335,6 +345,13 @@ static inline void i915_trace_irq_get(struct intel_engine_cs *ring, u32 seqno)
>  		ring->trace_irq_seqno = seqno;
>  }
>  
> +void intel_ring_notify_complete(struct intel_ring_notify *notify);
> +void intel_ring_notify_check(struct intel_engine_cs *ring);
> +int __must_check intel_ring_notify_add(struct intel_engine_cs *ring,
> +				       struct intel_ring_notify *notify);
> +bool intel_ring_notify_pending(struct intel_ring_notify *notify);
> +void intel_ring_notify_cancel(struct intel_ring_notify *notify);
> +
>  /* DRI warts */
>  int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size);
>  
> -- 
> 1.8.5.5
> 
> _______________________________________________
> 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

[PATCH 02/25] drm/i915: Add vblank notify mechanism

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a vblank notify mechanism where you can ask for a callback when a
specific frame counter value has been passed.

This could be used for various things like FBC, IPS, watermarks,
and updating single buffered display registers from the interrupt
handler (eg. gamma).

As gen2 doesn't have a hardware frame counter we use the software vblank
counter drm core procvides. This is rather racy, but for something like
FBC it doesn't matter too much. For gen2 we could just scheudle the FBC
enable happen a frame later than on other gens. That should paper over
the races sufficiently.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c      |   8 +++
 drivers/gpu/drm/i915/intel_display.c | 132 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  16 +++++
 3 files changed, 156 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 218f011..a908a55 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1736,7 +1736,15 @@ static bool intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
 	if (!drm_handle_vblank(dev, pipe))
 		return false;
 
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return true;
+
 	crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
+
+	spin_lock(&crtc->lock);
+	intel_vblank_notify_check(crtc);
+	spin_unlock(&crtc->lock);
+
 	wake_up(&crtc->vbl_wait);
 
 	return true;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5e8e711..be3ee69 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11499,6 +11499,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 
 	init_waitqueue_head(&intel_crtc->vbl_wait);
 
+	spin_lock_init(&intel_crtc->lock);
+	INIT_LIST_HEAD(&intel_crtc->vblank_notify_list);
+
 	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;
@@ -13051,3 +13054,132 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
 		err_printf(m, "  VSYNC: %08x\n", error->transcoder[i].vsync);
 	}
 }
+
+/* is a after b? */
+static bool vbl_count_after_eq(struct drm_device *dev, u32 a, u32 b)
+{
+	u32 mask = dev->max_vblank_count;
+
+	/* now hardware counter on gen2 */
+	if (mask == 0)
+		mask = -1;
+
+	mask &= (mask >> 1);
+
+	return !((a - b) & mask);
+}
+
+static void intel_vblank_notify_complete(struct intel_vblank_notify *notify)
+{
+	struct intel_crtc *crtc = notify->crtc;
+	struct drm_device *dev = crtc->base.dev;
+
+	assert_spin_locked(&crtc->lock);
+
+	drm_vblank_put(dev, crtc->pipe);
+	list_del(&notify->list);
+	notify->crtc = NULL;
+}
+
+void intel_vblank_notify_check(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_vblank_crtc *vblank =
+		&dev->vblank[drm_crtc_index(&crtc->base)];
+	struct intel_vblank_notify *notify, *next;
+	u32 vbl_count;
+
+	assert_spin_locked(&crtc->lock);
+
+	if (list_empty(&crtc->vblank_notify_list))
+		return;
+
+	/* no hardware frame counter on gen2 */
+	if (dev->max_vblank_count == 0)
+		vbl_count = atomic_read(&vblank->count);
+	else
+		vbl_count = dev->driver->get_vblank_counter(dev, crtc->pipe);
+
+	list_for_each_entry_safe(notify, next, &crtc->vblank_notify_list, list) {
+		if (!vbl_count_after_eq(dev, vbl_count, notify->vbl_count))
+			continue;
+
+		intel_vblank_notify_complete(notify);
+		notify->notify(notify);
+	}
+}
+
+int intel_vblank_notify_add(struct intel_crtc *crtc,
+			    struct intel_vblank_notify *notify)
+{
+	struct drm_device *dev = crtc->base.dev;
+	unsigned long irqflags;
+	u32 vbl_count;
+	int ret;
+
+	if (WARN_ON(notify->crtc))
+		return -EINVAL;
+
+	ret = drm_vblank_get(dev, crtc->pipe);
+	if (ret)
+		return ret;
+
+	spin_lock_irqsave(&crtc->lock, irqflags);
+
+	notify->crtc = crtc;
+	list_add(&notify->list, &crtc->vblank_notify_list);
+
+	/* no hardware frame counter on gen2 */
+	if (dev->max_vblank_count == 0) {
+		struct drm_vblank_crtc *vblank =
+			&dev->vblank[drm_crtc_index(&crtc->base)];
+
+		vbl_count = atomic_read(&vblank->count);
+	} else {
+		vbl_count = dev->driver->get_vblank_counter(dev, crtc->pipe);
+	}
+
+	if (vbl_count_after_eq(dev, vbl_count, notify->vbl_count)) {
+		intel_vblank_notify_complete(notify);
+		notify->notify(notify);
+	}
+
+	spin_unlock_irqrestore(&crtc->lock, irqflags);
+
+	return 0;
+}
+
+bool intel_vblank_notify_pending(struct intel_vblank_notify *notify)
+{
+	return notify->crtc != NULL;
+}
+
+void intel_vblank_notify_cancel(struct intel_vblank_notify *notify)
+{
+	struct intel_crtc *crtc = ACCESS_ONCE(notify->crtc);
+	unsigned long irqflags;
+
+	if (!crtc)
+		return;
+
+	spin_lock_irqsave(&crtc->lock, irqflags);
+	if (notify->crtc)
+		intel_vblank_notify_complete(notify);
+	spin_unlock_irqrestore(&crtc->lock, irqflags);
+}
+
+u32 intel_crtc_vbl_count_rel_to_abs(struct intel_crtc *crtc, u32 rel)
+{
+	struct drm_device *dev = crtc->base.dev;
+
+	/* now hardware counter on gen2 */
+	if (dev->max_vblank_count == 0) {
+		struct drm_vblank_crtc *vblank =
+			&dev->vblank[drm_crtc_index(&crtc->base)];
+
+		return atomic_read(&vblank->count) + rel;
+	}
+
+	return (dev->driver->get_vblank_counter(dev, crtc->pipe) + rel) &
+		dev->max_vblank_count;
+}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ab5962b..c93626b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -358,6 +358,13 @@ struct intel_pipe_wm {
 	bool sprites_scaled;
 };
 
+struct intel_vblank_notify {
+	void (*notify)(struct intel_vblank_notify *notify);
+	struct intel_crtc *crtc;
+	struct list_head list;
+	u32 vbl_count;
+};
+
 struct intel_mmio_flip {
 	u32 seqno;
 	u32 ring_id;
@@ -417,6 +424,9 @@ struct intel_crtc {
 
 	int scanline_offset;
 	struct intel_mmio_flip mmio_flip;
+
+	struct list_head vblank_notify_list;
+	spinlock_t lock;
 };
 
 struct intel_plane_wm_parameters {
@@ -811,6 +821,12 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_config *pipe_config);
 int intel_format_to_fourcc(int format);
 void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
+int intel_vblank_notify_add(struct intel_crtc *crtc,
+			    struct intel_vblank_notify *notify);
+void intel_vblank_notify_cancel(struct intel_vblank_notify *notify);
+bool intel_vblank_notify_pending(struct intel_vblank_notify *notify);
+void intel_vblank_notify_check(struct intel_crtc *crtc);
+u32 intel_crtc_vbl_count_rel_to_abs(struct intel_crtc *crtc, u32 rel);
 
 
 /* intel_dp.c */
-- 
1.8.5.5

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

Re: [PATCH 02/25] drm/i915: Add vblank notify mechanism

[Daniel Vetter]

On Wed, Jun 18, 2014 at 08:58:35PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add a vblank notify mechanism where you can ask for a callback when a
> specific frame counter value has been passed.
> 
> This could be used for various things like FBC, IPS, watermarks,
> and updating single buffered display registers from the interrupt
> handler (eg. gamma).
> 
> As gen2 doesn't have a hardware frame counter we use the software vblank
> counter drm core procvides. This is rather racy, but for something like
> FBC it doesn't matter too much. For gen2 we could just scheudle the FBC
> enable happen a frame later than on other gens. That should paper over
> the races sufficiently.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Chris submitted a very similar patch for vblank work items, same review
still applies: This should be moved into drm_irq.c
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_irq.c      |   8 +++
>  drivers/gpu/drm/i915/intel_display.c | 132 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  16 +++++
>  3 files changed, 156 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 218f011..a908a55 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1736,7 +1736,15 @@ static bool intel_pipe_handle_vblank(struct drm_device *dev, enum pipe pipe)
>  	if (!drm_handle_vblank(dev, pipe))
>  		return false;
>  
> +	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +		return true;
> +
>  	crtc = to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
> +
> +	spin_lock(&crtc->lock);
> +	intel_vblank_notify_check(crtc);
> +	spin_unlock(&crtc->lock);
> +
>  	wake_up(&crtc->vbl_wait);
>  
>  	return true;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5e8e711..be3ee69 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11499,6 +11499,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  
>  	init_waitqueue_head(&intel_crtc->vbl_wait);
>  
> +	spin_lock_init(&intel_crtc->lock);
> +	INIT_LIST_HEAD(&intel_crtc->vblank_notify_list);
> +
>  	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;
> @@ -13051,3 +13054,132 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
>  		err_printf(m, "  VSYNC: %08x\n", error->transcoder[i].vsync);
>  	}
>  }
> +
> +/* is a after b? */
> +static bool vbl_count_after_eq(struct drm_device *dev, u32 a, u32 b)
> +{
> +	u32 mask = dev->max_vblank_count;
> +
> +	/* now hardware counter on gen2 */
> +	if (mask == 0)
> +		mask = -1;
> +
> +	mask &= (mask >> 1);
> +
> +	return !((a - b) & mask);
> +}
> +
> +static void intel_vblank_notify_complete(struct intel_vblank_notify *notify)
> +{
> +	struct intel_crtc *crtc = notify->crtc;
> +	struct drm_device *dev = crtc->base.dev;
> +
> +	assert_spin_locked(&crtc->lock);
> +
> +	drm_vblank_put(dev, crtc->pipe);
> +	list_del(&notify->list);
> +	notify->crtc = NULL;
> +}
> +
> +void intel_vblank_notify_check(struct intel_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_vblank_crtc *vblank =
> +		&dev->vblank[drm_crtc_index(&crtc->base)];
> +	struct intel_vblank_notify *notify, *next;
> +	u32 vbl_count;
> +
> +	assert_spin_locked(&crtc->lock);
> +
> +	if (list_empty(&crtc->vblank_notify_list))
> +		return;
> +
> +	/* no hardware frame counter on gen2 */
> +	if (dev->max_vblank_count == 0)
> +		vbl_count = atomic_read(&vblank->count);
> +	else
> +		vbl_count = dev->driver->get_vblank_counter(dev, crtc->pipe);
> +
> +	list_for_each_entry_safe(notify, next, &crtc->vblank_notify_list, list) {
> +		if (!vbl_count_after_eq(dev, vbl_count, notify->vbl_count))
> +			continue;
> +
> +		intel_vblank_notify_complete(notify);
> +		notify->notify(notify);
> +	}
> +}
> +
> +int intel_vblank_notify_add(struct intel_crtc *crtc,
> +			    struct intel_vblank_notify *notify)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	unsigned long irqflags;
> +	u32 vbl_count;
> +	int ret;
> +
> +	if (WARN_ON(notify->crtc))
> +		return -EINVAL;
> +
> +	ret = drm_vblank_get(dev, crtc->pipe);
> +	if (ret)
> +		return ret;
> +
> +	spin_lock_irqsave(&crtc->lock, irqflags);
> +
> +	notify->crtc = crtc;
> +	list_add(&notify->list, &crtc->vblank_notify_list);
> +
> +	/* no hardware frame counter on gen2 */
> +	if (dev->max_vblank_count == 0) {
> +		struct drm_vblank_crtc *vblank =
> +			&dev->vblank[drm_crtc_index(&crtc->base)];
> +
> +		vbl_count = atomic_read(&vblank->count);
> +	} else {
> +		vbl_count = dev->driver->get_vblank_counter(dev, crtc->pipe);
> +	}
> +
> +	if (vbl_count_after_eq(dev, vbl_count, notify->vbl_count)) {
> +		intel_vblank_notify_complete(notify);
> +		notify->notify(notify);
> +	}
> +
> +	spin_unlock_irqrestore(&crtc->lock, irqflags);
> +
> +	return 0;
> +}
> +
> +bool intel_vblank_notify_pending(struct intel_vblank_notify *notify)
> +{
> +	return notify->crtc != NULL;
> +}
> +
> +void intel_vblank_notify_cancel(struct intel_vblank_notify *notify)
> +{
> +	struct intel_crtc *crtc = ACCESS_ONCE(notify->crtc);
> +	unsigned long irqflags;
> +
> +	if (!crtc)
> +		return;
> +
> +	spin_lock_irqsave(&crtc->lock, irqflags);
> +	if (notify->crtc)
> +		intel_vblank_notify_complete(notify);
> +	spin_unlock_irqrestore(&crtc->lock, irqflags);
> +}
> +
> +u32 intel_crtc_vbl_count_rel_to_abs(struct intel_crtc *crtc, u32 rel)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +
> +	/* now hardware counter on gen2 */
> +	if (dev->max_vblank_count == 0) {
> +		struct drm_vblank_crtc *vblank =
> +			&dev->vblank[drm_crtc_index(&crtc->base)];
> +
> +		return atomic_read(&vblank->count) + rel;
> +	}
> +
> +	return (dev->driver->get_vblank_counter(dev, crtc->pipe) + rel) &
> +		dev->max_vblank_count;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ab5962b..c93626b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -358,6 +358,13 @@ struct intel_pipe_wm {
>  	bool sprites_scaled;
>  };
>  
> +struct intel_vblank_notify {
> +	void (*notify)(struct intel_vblank_notify *notify);
> +	struct intel_crtc *crtc;
> +	struct list_head list;
> +	u32 vbl_count;
> +};
> +
>  struct intel_mmio_flip {
>  	u32 seqno;
>  	u32 ring_id;
> @@ -417,6 +424,9 @@ struct intel_crtc {
>  
>  	int scanline_offset;
>  	struct intel_mmio_flip mmio_flip;
> +
> +	struct list_head vblank_notify_list;
> +	spinlock_t lock;
>  };
>  
>  struct intel_plane_wm_parameters {
> @@ -811,6 +821,12 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
>  				 struct intel_crtc_config *pipe_config);
>  int intel_format_to_fourcc(int format);
>  void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
> +int intel_vblank_notify_add(struct intel_crtc *crtc,
> +			    struct intel_vblank_notify *notify);
> +void intel_vblank_notify_cancel(struct intel_vblank_notify *notify);
> +bool intel_vblank_notify_pending(struct intel_vblank_notify *notify);
> +void intel_vblank_notify_check(struct intel_crtc *crtc);
> +u32 intel_crtc_vbl_count_rel_to_abs(struct intel_crtc *crtc, u32 rel);
>  
>  
>  /* intel_dp.c */
> -- 
> 1.8.5.5
> 
> _______________________________________________
> 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

[PATCH 03/25] drm/i915: Name the IPS bits

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Kill the magic numbers from the IPS code and give a name to the
IPS pcode control bit.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      | 1 +
 drivers/gpu/drm/i915/intel_display.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3488567..78310c1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5551,6 +5551,7 @@ enum punit_power_well {
 #define   GEN6_ENCODE_RC6_VID(mv)		(((mv) - 245) / 5)
 #define   GEN6_DECODE_RC6_VID(vids)		(((vids) * 5) + 245)
 #define   DISPLAY_IPS_CONTROL			0x19
+#define     IPS_PCODE_CONTROL			(1<<30)
 #define GEN6_PCODE_DATA				0x138128
 #define   GEN6_PCODE_FREQ_IA_RATIO_SHIFT	8
 #define   GEN6_PCODE_FREQ_RING_RATIO_SHIFT	16
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index be3ee69..af7fd96 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3788,7 +3788,8 @@ void hsw_enable_ips(struct intel_crtc *crtc)
 	assert_plane_enabled(dev_priv, crtc->plane);
 	if (IS_BROADWELL(dev)) {
 		mutex_lock(&dev_priv->rps.hw_lock);
-		WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0xc0000000));
+		WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL,
+						IPS_ENABLE | IPS_PCODE_CONTROL));
 		mutex_unlock(&dev_priv->rps.hw_lock);
 		/* Quoting Art Runyan: "its not safe to expect any particular
 		 * value in IPS_CTL bit 31 after enabling IPS through the
-- 
1.8.5.5

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

[PATCH 04/25] drm/i915: Use vblank notifier for IPS

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

IPS enable must be delayed one frame after the planes have been enabled.
Currently there's a blocking vblank wait in the path. Replace that with
a vblank notify so that it can be done asynchronously. The disable path
must remain synchronous.

TODO: see if the ips state checking can be fixed, for now it's disabled

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |   2 +
 drivers/gpu/drm/i915/i915_drv.h      |  12 ++++
 drivers/gpu/drm/i915/intel_display.c | 110 +++++++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_drv.h     |   8 +--
 4 files changed, 94 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 7ae4e2a..f046a3c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -537,6 +537,8 @@ static int i915_drm_freeze(struct drm_device *dev)
 		}
 		drm_modeset_unlock_all(dev);
 
+		intel_ips_cleanup(dev);
+
 		intel_modeset_suspend_hw(dev);
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0640071..4fd8be9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1331,6 +1331,13 @@ struct intel_pipe_crc {
 	wait_queue_head_t wq;
 };
 
+struct intel_vblank_notify {
+	void (*notify)(struct intel_vblank_notify *notify);
+	struct intel_crtc *crtc;
+	struct list_head list;
+	u32 vbl_count;
+};
+
 struct drm_i915_private {
 	struct drm_device *dev;
 	struct kmem_cache *slab;
@@ -1405,6 +1412,11 @@ struct drm_i915_private {
 	u32 hpd_event_bits;
 	struct timer_list hotplug_reenable_timer;
 
+	struct {
+		struct intel_crtc *crtc;
+		struct work_struct work;
+		struct intel_vblank_notify notify;
+	} hsw_ips;
 	struct i915_fbc fbc;
 	struct i915_drrs drrs;
 	struct intel_opregion opregion;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index af7fd96..e672fed 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3774,38 +3774,70 @@ static void intel_disable_planes(struct drm_crtc *crtc)
 	}
 }
 
+static void bdw_ips_work(struct work_struct *work)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(work, struct drm_i915_private, hsw_ips.work);
+	struct intel_crtc *crtc = ACCESS_ONCE(dev_priv->hsw_ips.crtc);
+
+	if (!crtc)
+		return;
+
+	drm_modeset_lock(&crtc->base.mutex, NULL);
+
+	if (WARN_ON(!crtc->config.ips_enabled))
+		goto unlock;
+
+	/*
+	 * IPS must have been disabled in the meantime,
+	 * and may not re-enabled, at least quite so soon.
+	 */
+	if (!crtc->primary_enabled ||
+	    intel_vblank_notify_pending(&dev_priv->hsw_ips.notify))
+		goto unlock;
+
+	assert_plane_enabled(dev_priv, crtc->plane);
+
+	mutex_lock(&dev_priv->rps.hw_lock);
+	WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL,
+					IPS_ENABLE | IPS_PCODE_CONTROL));
+	mutex_unlock(&dev_priv->rps.hw_lock);
+	/* Quoting Art Runyan: "its not safe to expect any particular
+	 * value in IPS_CTL bit 31 after enabling IPS through the
+	 * mailbox." Moreover, the mailbox may return a bogus state,
+	 * so we need to just enable it and continue on.
+	 */
+
+ unlock:
+	drm_modeset_unlock(&crtc->base.mutex);
+}
+
+static void hsw_ips_notify(struct intel_vblank_notify *notify)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(notify, struct drm_i915_private, hsw_ips.notify);
+
+	if (IS_BROADWELL(dev_priv->dev))
+		schedule_work(&dev_priv->hsw_ips.work);
+	else
+		I915_WRITE(IPS_CTL, IPS_ENABLE);
+}
+
 void hsw_enable_ips(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	lockdep_assert_held(&crtc->base.mutex.mutex.base);
+
+	WARN_ON(intel_vblank_notify_pending(&dev_priv->hsw_ips.notify));
+
 	if (!crtc->config.ips_enabled)
 		return;
 
-	/* We can only enable IPS after we enable a plane and wait for a vblank */
-	intel_wait_for_vblank(dev, crtc->pipe);
+	dev_priv->hsw_ips.crtc = crtc;
 
-	assert_plane_enabled(dev_priv, crtc->plane);
-	if (IS_BROADWELL(dev)) {
-		mutex_lock(&dev_priv->rps.hw_lock);
-		WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL,
-						IPS_ENABLE | IPS_PCODE_CONTROL));
-		mutex_unlock(&dev_priv->rps.hw_lock);
-		/* Quoting Art Runyan: "its not safe to expect any particular
-		 * value in IPS_CTL bit 31 after enabling IPS through the
-		 * mailbox." Moreover, the mailbox may return a bogus state,
-		 * so we need to just enable it and continue on.
-		 */
-	} else {
-		I915_WRITE(IPS_CTL, IPS_ENABLE);
-		/* The bit only becomes 1 in the next vblank, so this wait here
-		 * is essentially intel_wait_for_vblank. If we don't have this
-		 * and don't wait for vblanks until the end of crtc_enable, then
-		 * the HW state readout code will complain that the expected
-		 * IPS_CTL value is not the one we read. */
-		if (wait_for(I915_READ_NOTRACE(IPS_CTL) & IPS_ENABLE, 50))
-			DRM_ERROR("Timed out waiting for IPS enable\n");
-	}
+	intel_vblank_notify_add(crtc, &dev_priv->hsw_ips.notify);
 }
 
 void hsw_disable_ips(struct intel_crtc *crtc)
@@ -3813,9 +3845,13 @@ void hsw_disable_ips(struct intel_crtc *crtc)
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	lockdep_assert_held(&crtc->base.mutex.mutex.base);
+
 	if (!crtc->config.ips_enabled)
 		return;
 
+	intel_vblank_notify_cancel(&dev_priv->hsw_ips.notify);
+
 	assert_plane_enabled(dev_priv, crtc->plane);
 	if (IS_BROADWELL(dev)) {
 		mutex_lock(&dev_priv->rps.hw_lock);
@@ -3833,6 +3869,22 @@ void hsw_disable_ips(struct intel_crtc *crtc)
 	intel_wait_for_vblank(dev, crtc->pipe);
 }
 
+static void intel_ips_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	dev_priv->hsw_ips.notify.notify = hsw_ips_notify;
+	INIT_WORK(&dev_priv->hsw_ips.work, bdw_ips_work);
+}
+
+void intel_ips_cleanup(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	intel_vblank_notify_cancel(&dev_priv->hsw_ips.notify);
+	cancel_work_sync(&dev_priv->hsw_ips.work);
+}
+
 /** Loads the palette/gamma unit for the CRTC with the prepared values */
 static void intel_crtc_load_lut(struct drm_crtc *crtc)
 {
@@ -7613,10 +7665,6 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	if (intel_display_power_enabled(dev_priv, pfit_domain))
 		ironlake_get_pfit_config(crtc, pipe_config);
 
-	if (IS_HASWELL(dev))
-		pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
-			(I915_READ(IPS_CTL) & IPS_ENABLE);
-
 	pipe_config->pixel_multiplier = 1;
 
 	return true;
@@ -10207,10 +10255,6 @@ intel_pipe_config_compare(struct drm_device *dev,
 		PIPE_CONF_CHECK_I(pch_pfit.size);
 	}
 
-	/* BDW+ don't expose a synchronous way to read the state */
-	if (IS_HASWELL(dev))
-		PIPE_CONF_CHECK_I(ips_enabled);
-
 	PIPE_CONF_CHECK_I(double_wide);
 
 	PIPE_CONF_CHECK_I(shared_dpll);
@@ -12274,6 +12318,8 @@ void intel_modeset_init(struct drm_device *dev)
 		      INTEL_INFO(dev)->num_pipes,
 		      INTEL_INFO(dev)->num_pipes > 1 ? "s" : "");
 
+	intel_ips_init(dev);
+
 	for_each_pipe(pipe) {
 		intel_crtc_init(dev, pipe);
 		for_each_sprite(pipe, sprite) {
@@ -12808,6 +12854,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
 
 	mutex_unlock(&dev->struct_mutex);
 
+	intel_ips_cleanup(dev);
+
 	/* flush any delayed tasks or pending work */
 	flush_scheduled_work();
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c93626b..9a8eb0e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -358,13 +358,6 @@ struct intel_pipe_wm {
 	bool sprites_scaled;
 };
 
-struct intel_vblank_notify {
-	void (*notify)(struct intel_vblank_notify *notify);
-	struct intel_crtc *crtc;
-	struct list_head list;
-	u32 vbl_count;
-};
-
 struct intel_mmio_flip {
 	u32 seqno;
 	u32 ring_id;
@@ -813,6 +806,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);
+void intel_ips_cleanup(struct drm_device *dev);
 void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);
 enum intel_display_power_domain
 intel_display_port_power_domain(struct intel_encoder *intel_encoder);
-- 
1.8.5.5

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

[PATCH 05/25] drm/i915: Reogranize page flip code for fbc

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The locking needs to be adjusted to accomodate the upcoming fbc.mutex,
so shuffle around the prep steps before actally issuing the flip.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 47 ++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e672fed..295bba7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9515,22 +9515,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	if (ret)
 		goto cleanup;
 
-	/* Reference the objects for the scheduled work. */
-	drm_gem_object_reference(&work->old_fb_obj->base);
-	drm_gem_object_reference(&obj->base);
-
-	crtc->primary->fb = fb;
-
-	work->pending_flip_obj = obj;
-
-	work->enable_stall_check = true;
-
-	atomic_inc(&intel_crtc->unpin_work_count);
-	intel_crtc->reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
-
-	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
-		work->flip_count = I915_READ(PIPE_FLIPCOUNT_GM45(intel_crtc->pipe)) + 1;
-
 	if (IS_VALLEYVIEW(dev)) {
 		ring = &dev_priv->ring[BCS];
 	} else if (INTEL_INFO(dev)->gen >= 7) {
@@ -9543,7 +9527,24 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
 	ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
 	if (ret)
-		goto cleanup_pending;
+		goto cleanup_unlock;
+
+	atomic_inc(&intel_crtc->unpin_work_count);
+
+	crtc->primary->fb = fb;
+
+	/* Reference the objects for the scheduled work. */
+	drm_gem_object_reference(&work->old_fb_obj->base);
+	drm_gem_object_reference(&obj->base);
+
+	work->pending_flip_obj = obj;
+
+	work->enable_stall_check = true;
+
+	intel_crtc->reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
+
+	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
+		work->flip_count = I915_READ(PIPE_FLIPCOUNT_GM45(intel_crtc->pipe)) + 1;
 
 	work->gtt_offset =
 		i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset;
@@ -9555,7 +9556,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, ring,
 				page_flip_flags);
 	if (ret)
-		goto cleanup_unpin;
+		goto cleanup_all;
 
 	intel_disable_fbc(dev);
 	intel_mark_fb_busy(obj, NULL);
@@ -9565,13 +9566,13 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
 	return 0;
 
-cleanup_unpin:
-	intel_unpin_fb_obj(obj);
-cleanup_pending:
-	atomic_dec(&intel_crtc->unpin_work_count);
-	crtc->primary->fb = old_fb;
+cleanup_all:
 	drm_gem_object_unreference(&work->old_fb_obj->base);
 	drm_gem_object_unreference(&obj->base);
+	crtc->primary->fb = old_fb;
+	atomic_dec(&intel_crtc->unpin_work_count);
+	intel_unpin_fb_obj(obj);
+cleanup_unlock:
 	mutex_unlock(&dev->struct_mutex);
 
 cleanup:
-- 
1.8.5.5

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

[PATCH 06/25] drm/i915: Move ilk_pipe_pixel_rate() earlier to avoid forward declaration

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Just shuffle code around, no functional change.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 62 ++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2043c4b..4732c8d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -437,6 +437,37 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
 	return true;
 }
 
+static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
+				    struct drm_crtc *crtc)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	uint32_t pixel_rate;
+
+	pixel_rate = intel_crtc->config.adjusted_mode.crtc_clock;
+
+	/* We only use IF-ID interlacing. If we ever use PF-ID we'll need to
+	 * adjust the pixel_rate here. */
+
+	if (intel_crtc->config.pch_pfit.enabled) {
+		uint64_t pipe_w, pipe_h, pfit_w, pfit_h;
+		uint32_t pfit_size = intel_crtc->config.pch_pfit.size;
+
+		pipe_w = intel_crtc->config.pipe_src_w;
+		pipe_h = intel_crtc->config.pipe_src_h;
+		pfit_w = (pfit_size >> 16) & 0xFFFF;
+		pfit_h = pfit_size & 0xFFFF;
+		if (pipe_w < pfit_w)
+			pipe_w = pfit_w;
+		if (pipe_h < pfit_h)
+			pipe_h = pfit_h;
+
+		pixel_rate = div_u64((uint64_t) pixel_rate * pipe_w * pipe_h,
+				     pfit_w * pfit_h);
+	}
+
+	return pixel_rate;
+}
+
 /**
  * intel_update_fbc - enable/disable FBC as needed
  * @dev: the drm_device
@@ -1651,37 +1682,6 @@ static void i845_update_wm(struct drm_crtc *unused_crtc)
 	I915_WRITE(FW_BLC, fwater_lo);
 }
 
-static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
-				    struct drm_crtc *crtc)
-{
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	uint32_t pixel_rate;
-
-	pixel_rate = intel_crtc->config.adjusted_mode.crtc_clock;
-
-	/* We only use IF-ID interlacing. If we ever use PF-ID we'll need to
-	 * adjust the pixel_rate here. */
-
-	if (intel_crtc->config.pch_pfit.enabled) {
-		uint64_t pipe_w, pipe_h, pfit_w, pfit_h;
-		uint32_t pfit_size = intel_crtc->config.pch_pfit.size;
-
-		pipe_w = intel_crtc->config.pipe_src_w;
-		pipe_h = intel_crtc->config.pipe_src_h;
-		pfit_w = (pfit_size >> 16) & 0xFFFF;
-		pfit_h = pfit_size & 0xFFFF;
-		if (pipe_w < pfit_w)
-			pipe_w = pfit_w;
-		if (pipe_h < pfit_h)
-			pipe_h = pfit_h;
-
-		pixel_rate = div_u64((uint64_t) pixel_rate * pipe_w * pipe_h,
-				     pfit_w * pfit_h);
-	}
-
-	return pixel_rate;
-}
-
 /* latency must be in 0.1us units. */
 static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
 			       uint32_t latency)
-- 
1.8.5.5

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

[PATCH 07/25] drm/i915: Reorganize intel_update_fbc()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

FBC1 and FBC2 behave a bit differently when it comes to multiple pipes.
FBC1 supposedly must be disabled when multiple pipes are enabled, but FBC2
can be enabled. To make this more easier to read simply split the code
along the FBC1 vs. FBC2 lines.

On FBC2 we get to pick one of the primary planes for compression if
multiple are enabled. For now just pick the one with highest pixel rate.

Since this also means that many of no_fbc_reason values no longer makes
sense since we track that on a per-device basic, but the reasons are
actually per-CRTC. Simply replace all the per-CRTC no_fbc_reason values
with a single new FBC_UNSUPPORTED_CONFIG reason. In case someone still
wants more debug information, all the per-CRTC reasons for disabling
FBC will now print a DRM_DEBUG() message.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  16 +--
 drivers/gpu/drm/i915/i915_drv.h     |   6 +-
 drivers/gpu/drm/i915/intel_pm.c     | 265 +++++++++++++++++++++++++-----------
 3 files changed, 189 insertions(+), 98 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 76c2572..91c64f9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1468,24 +1468,12 @@ static int i915_fbc_status(struct seq_file *m, void *unused)
 		case FBC_UNSUPPORTED:
 			seq_puts(m, "unsupported by this chipset");
 			break;
-		case FBC_NO_OUTPUT:
-			seq_puts(m, "no outputs");
+		case FBC_UNSUPPORTED_CONFIG:
+			seq_puts(m, "unsupported pipe/plane config");
 			break;
 		case FBC_STOLEN_TOO_SMALL:
 			seq_puts(m, "not enough stolen memory");
 			break;
-		case FBC_UNSUPPORTED_MODE:
-			seq_puts(m, "mode not supported");
-			break;
-		case FBC_MODE_TOO_LARGE:
-			seq_puts(m, "mode too large");
-			break;
-		case FBC_BAD_PLANE:
-			seq_puts(m, "FBC unsupported on plane");
-			break;
-		case FBC_NOT_TILED:
-			seq_puts(m, "scanout buffer not tiled");
-			break;
 		case FBC_MULTIPLE_PIPES:
 			seq_puts(m, "multiple pipes are enabled");
 			break;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4fd8be9..bc6d2cb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -617,12 +617,8 @@ struct i915_fbc {
 	enum no_fbc_reason {
 		FBC_OK, /* FBC is enabled */
 		FBC_UNSUPPORTED, /* FBC is not supported by this chipset */
-		FBC_NO_OUTPUT, /* no outputs enabled to compress */
+		FBC_UNSUPPORTED_CONFIG, /* pipe/plane config not suitable for FBC */
 		FBC_STOLEN_TOO_SMALL, /* not enough space for buffers */
-		FBC_UNSUPPORTED_MODE, /* interlace or doublescanned mode */
-		FBC_MODE_TOO_LARGE, /* mode too large for compression */
-		FBC_BAD_PLANE, /* fbc not supported on plane */
-		FBC_NOT_TILED, /* buffer not tiled */
 		FBC_MULTIPLE_PIPES, /* more than one pipe active */
 		FBC_MODULE_PARAM,
 		FBC_CHIP_DEFAULT, /* disabled by default on this chip */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4732c8d..07fa149 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -468,6 +468,179 @@ static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
 	return pixel_rate;
 }
 
+static bool intel_fbc1_possible(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_framebuffer *fb = crtc->base.primary->fb;
+	struct drm_i915_gem_object *obj;
+
+	if (!IS_GEN4(dev) && crtc->plane != PLANE_A) {
+		DRM_DEBUG("FBC pipe %c, plane %c: no FBC on this plane\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane));
+		return false;
+	}
+
+	if (!intel_crtc_active(&crtc->base)) {
+		DRM_DEBUG("FBC pipe %c, plane %c: pipe disabled\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane));
+		return false;
+	}
+
+	if (crtc->config.pipe_src_w > 2048 ||
+	    crtc->config.pipe_src_h > 1536) {
+		DRM_DEBUG("FBC pipe %c, plane %c: mode (%dx%d) too large\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane),
+			  crtc->config.pipe_src_w, crtc->config.pipe_src_h);
+		return false;
+	}
+
+	if (crtc->config.adjusted_mode.flags & (DRM_MODE_FLAG_INTERLACE |
+						DRM_MODE_FLAG_DBLSCAN) ||
+	    crtc->config.double_wide) {
+		DRM_DEBUG("FBC pipe %c, plane %c: mode incompatible\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane));
+		return false;
+	}
+
+	if (!crtc->primary_enabled) {
+		DRM_DEBUG("FBC pipe %c, plane %c: primary plane disabled\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane));
+		return false;
+	}
+
+	obj = to_intel_framebuffer(fb)->obj;
+
+	if (obj->tiling_mode != I915_TILING_X ||
+	    obj->fence_reg == I915_FENCE_REG_NONE) {
+		DRM_DEBUG("FBC pipe %c, plane %c: framebuffer not tiled or fenced\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane));
+		return false;
+	}
+
+	return true;
+}
+
+static bool intel_fbc2_possible(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_framebuffer *fb = crtc->base.primary->fb;
+	struct drm_i915_gem_object *obj;
+	int max_width, max_height;
+
+	if ((IS_HASWELL(dev) || INTEL_INFO(dev)->gen >= 8) &&
+	    crtc->plane != PLANE_A) {
+		DRM_DEBUG("FBC pipe %c, plane %c: no FBC on this plane\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane));
+		return false;
+	}
+
+	if (!intel_crtc_active(&crtc->base)) {
+		DRM_DEBUG("FBC pipe %c, plane %c: pipe disabled\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane));
+		return false;
+	}
+
+	if (IS_HASWELL(dev) || INTEL_INFO(dev)->gen >= 8) {
+		max_width = 4096;
+		max_height = 4096;
+	} else {
+		max_width = 4096;
+		max_height = 2048;
+	}
+
+	if (crtc->config.pipe_src_w > max_width ||
+	    crtc->config.pipe_src_h > max_height) {
+		DRM_DEBUG("FBC pipe %c, plane %c: mode (%dx%d) too large\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane),
+			  crtc->config.pipe_src_w, crtc->config.pipe_src_h);
+		return false;
+	}
+
+	if (!crtc->primary_enabled) {
+		DRM_DEBUG("FBC pipe %c, plane %c: primary plane disabled\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane));
+		return false;
+	}
+
+	obj = to_intel_framebuffer(fb)->obj;
+
+	if (obj->tiling_mode != I915_TILING_X ||
+	    obj->fence_reg == I915_FENCE_REG_NONE) {
+		DRM_DEBUG("FBC pipe %c, plane %c: framebuffer not tiled or fenced\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane));
+		return false;
+	}
+
+	return true;
+}
+
+static struct intel_crtc *intel_fbc1_pick_crtc(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *tmp_crtc, *crtc = NULL;
+
+	list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, base.head) {
+		if (!tmp_crtc->active)
+			continue;
+
+		if (crtc) {
+			if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
+				DRM_DEBUG_KMS("more than one pipe active, disabling FBC\n");
+			return NULL;
+		}
+
+		crtc = tmp_crtc;
+	}
+
+	if (!crtc || !intel_fbc1_possible(crtc)) {
+		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_CONFIG))
+			DRM_DEBUG_KMS("no suitable plane, disabling FBC\n");
+		return NULL;
+	}
+
+	return crtc;
+}
+
+static struct intel_crtc *intel_fbc2_pick_crtc(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *tmp_crtc, *crtc = NULL;
+	unsigned int pixel_rate = 0;
+
+	list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, base.head) {
+		unsigned int tmp_pixel_rate;
+
+		if (!intel_fbc2_possible(tmp_crtc))
+			continue;
+
+		/*
+		 * If we have multiple possibilities, prefer
+		 * the pipe with the highest pixel rate.
+		 *
+		 * FIXME: is this a good heuristic?
+		 *
+		 * FIXME: We should have something like ilk_pipe_pixel_rate()
+		 * for GMCH platforms too, but here it doesn't matter since
+		 * we anyway disallow FBC with panel fitting on g4x.
+		 */
+		if (HAS_PCH_SPLIT(dev))
+			tmp_pixel_rate = ilk_pipe_pixel_rate(dev, &tmp_crtc->base);
+		else
+			tmp_pixel_rate = tmp_crtc->config.adjusted_mode.crtc_clock;
+
+		if (pixel_rate < tmp_pixel_rate) {
+			crtc = tmp_crtc;
+			pixel_rate = tmp_pixel_rate;
+		}
+	}
+
+	if (!crtc)
+		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_CONFIG))
+			DRM_DEBUG_KMS("no suitable plane, disabling FBC\n");
+
+	return crtc;
+}
+
 /**
  * intel_update_fbc - enable/disable FBC as needed
  * @dev: the drm_device
@@ -490,13 +663,9 @@ static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
 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 intel_crtc *crtc;
 	struct drm_framebuffer *fb;
-	struct intel_framebuffer *intel_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);
@@ -509,39 +678,6 @@ void intel_update_fbc(struct drm_device *dev)
 		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;
-	intel_fb = to_intel_framebuffer(fb);
-	obj = intel_fb->obj;
-	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");
@@ -552,52 +688,23 @@ void intel_update_fbc(struct drm_device *dev)
 			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;
-	}
+	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5)
+		crtc = intel_fbc2_pick_crtc(dev);
+	else
+		crtc = intel_fbc1_pick_crtc(dev);
 
-	/* 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");
+	if (!crtc)
 		goto out_disable;
-	}
+
+	fb = crtc->base.primary->fb;
+	obj = to_intel_framebuffer(fb)->obj;
 
 	/* If the kernel debugger is active, always disable compression */
 	if (in_dbg_master())
 		goto out_disable;
 
-	if (i915_gem_stolen_setup_compression(dev, intel_fb->obj->base.size)) {
+	if (i915_gem_stolen_setup_compression(dev, obj->base.size)) {
 		if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
 			DRM_DEBUG_KMS("framebuffer too large, disabling compression\n");
 		goto out_disable;
@@ -608,9 +715,9 @@ void intel_update_fbc(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.plane == crtc->plane &&
 	    dev_priv->fbc.fb_id == fb->base.id &&
-	    dev_priv->fbc.y == crtc->y)
+	    dev_priv->fbc.y == crtc->base.y)
 		return;
 
 	if (intel_fbc_enabled(dev)) {
@@ -641,7 +748,7 @@ void intel_update_fbc(struct drm_device *dev)
 		intel_disable_fbc(dev);
 	}
 
-	intel_enable_fbc(crtc);
+	intel_enable_fbc(&crtc->base);
 	dev_priv->fbc.no_fbc_reason = FBC_OK;
 	return;
 
-- 
1.8.5.5

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

[PATCH 08/25] drm/i915: Check panel fitting state before enabling fbc

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

WaFbcOnlyForNativeModeOnLFP:ctg says we need to disable fbc when panel
fitting is enabled on g4x.

On ilk we need to disable fbc if panel fitter is downscaling. Upscaling
is apparently OK.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 07fa149..cdadf6e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -556,6 +556,32 @@ static bool intel_fbc2_possible(struct intel_crtc *crtc)
 		return false;
 	}
 
+	if (IS_G4X(dev)) {
+		/* WaFbcOnlyForNativeModeOnLFP:ctg */
+		if (crtc->config.gmch_pfit.control) {
+			DRM_DEBUG("FBC pipe %c, plane %c: panel fitter enabled\n",
+				  pipe_name(crtc->pipe), plane_name(crtc->plane));
+			return false;
+		}
+	}
+
+	if (IS_GEN5(dev) && crtc->config.pch_pfit.enabled) {
+		uint32_t pfit_w, pfit_h;
+
+		pfit_w = crtc->config.pch_pfit.size >> 16;
+		pfit_h = crtc->config.pch_pfit.size & 0xFFFF;
+
+		/* FBC not allowed with panel fitter downscaling */
+		if (crtc->config.pipe_src_w > pfit_w ||
+		    crtc->config.pipe_src_h > pfit_h) {
+			DRM_DEBUG("FBC pipe %c, plane %c: panel fitter downscaling\n",
+				  pipe_name(crtc->pipe), plane_name(crtc->plane));
+			return false;
+		}
+
+		/* TODO: PF-ID is also disallowed */
+	}
+
 	if (!crtc->primary_enabled) {
 		DRM_DEBUG("FBC pipe %c, plane %c: primary plane disabled\n",
 			  pipe_name(crtc->pipe), plane_name(crtc->plane));
-- 
1.8.5.5

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

[PATCH 09/25] drm/i915: Reject fbc on g4x when sprites are enabled

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

WaFbcDisabledForOverlaySprite:ctg says we need to disallow fbc when
sprites are enabled.

We don't currently expose the sprites on g4x, but maybe we will later.
Add the appropriate check to fbc2_possible(). We will also need to
adjust the sprite code to turn off fbc prior to enabling the sprite,
but let's leave that until we actually get g4x sprite support.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index cdadf6e..b09d1a2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -523,6 +523,7 @@ static bool intel_fbc1_possible(struct intel_crtc *crtc)
 static bool intel_fbc2_possible(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_framebuffer *fb = crtc->base.primary->fb;
 	struct drm_i915_gem_object *obj;
 	int max_width, max_height;
@@ -563,6 +564,14 @@ static bool intel_fbc2_possible(struct intel_crtc *crtc)
 				  pipe_name(crtc->pipe), plane_name(crtc->plane));
 			return false;
 		}
+
+		/* WaFbcDisabledForOverlaySprite:ctg */
+		/* FIXME need to figure this out nicer */
+		if (I915_READ(DVSCNTR(crtc->pipe)) & DVS_ENABLE) {
+			DRM_DEBUG("FBC pipe %c, plane %c: video sprite enabled\n",
+				  pipe_name(crtc->pipe), plane_name(crtc->plane));
+			return false;
+		}
 	}
 
 	if (IS_GEN5(dev) && crtc->config.pch_pfit.enabled) {
-- 
1.8.5.5

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

[PATCH 10/25] drm/i915: Check pixel format for fbc

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

fbc is only possible with certain pixel formats. Check for those before
enabling fbc.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 50 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b09d1a2..0fcc0c6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -508,6 +508,32 @@ static bool intel_fbc1_possible(struct intel_crtc *crtc)
 		return false;
 	}
 
+	/*
+	 * Planes A & B don't support alpha, so
+	 * the "A" formats and "X" formats are
+	 * one and the same.
+	 */
+	switch (fb->pixel_format) {
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_ARGB8888:
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_ABGR8888:
+		break;
+	case DRM_FORMAT_XRGB1555:
+	case DRM_FORMAT_ARGB1555:
+	case DRM_FORMAT_RGB565:
+		/* 16bpp not supported on gen2 */
+		if (!IS_GEN2(dev))
+			break;
+
+		/* fall through */
+	default:
+		DRM_DEBUG("FBC pipe %c, plane %c: framebuffer format (%s) unsupported\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane),
+			  drm_get_format_name(fb->pixel_format));
+		return false;
+	}
+
 	obj = to_intel_framebuffer(fb)->obj;
 
 	if (obj->tiling_mode != I915_TILING_X ||
@@ -597,6 +623,30 @@ static bool intel_fbc2_possible(struct intel_crtc *crtc)
 		return false;
 	}
 
+	/*
+	 * Primary planes don't support alpha, so
+	 * the "A" formats and "X" formats are
+	 * one and the same.
+	 */
+	switch (fb->pixel_format) {
+	case DRM_FORMAT_XRGB8888:
+	case DRM_FORMAT_ARGB8888:
+	case DRM_FORMAT_XBGR8888:
+	case DRM_FORMAT_ABGR8888:
+		break;
+	case DRM_FORMAT_RGB565:
+		/* WaFbcOnly1to1Ratio:ctg */
+		if (!IS_G4X(dev))
+			break;
+
+		/* fall through */
+	default:
+		DRM_DEBUG("FBC pipe %c, plane %c: framebuffer format (%s) unsupported\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane),
+			  drm_get_format_name(fb->pixel_format));
+		return false;
+	}
+
 	obj = to_intel_framebuffer(fb)->obj;
 
 	if (obj->tiling_mode != I915_TILING_X ||
-- 
1.8.5.5

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

[PATCH 11/25] drm/i915: Remove dblscan flag from fbc1 check

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We don't do double scan anyway so don't bother checking it in the fbc
code.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0fcc0c6..f3ef14a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -494,8 +494,7 @@ static bool intel_fbc1_possible(struct intel_crtc *crtc)
 		return false;
 	}
 
-	if (crtc->config.adjusted_mode.flags & (DRM_MODE_FLAG_INTERLACE |
-						DRM_MODE_FLAG_DBLSCAN) ||
+	if (crtc->config.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE ||
 	    crtc->config.double_wide) {
 		DRM_DEBUG("FBC pipe %c, plane %c: mode incompatible\n",
 			  pipe_name(crtc->pipe), plane_name(crtc->plane));
-- 
1.8.5.5

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

[PATCH 12/25] drm/i915: Don't claim fbc as possible if the obj size exceeds stolen size

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

If the scanout object exceeds the size of the stolen memory, there's no
way we can ever enable fbc with it. Just say no.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f3ef14a..777167e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -471,6 +471,7 @@ static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
 static bool intel_fbc1_possible(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_framebuffer *fb = crtc->base.primary->fb;
 	struct drm_i915_gem_object *obj;
 
@@ -535,6 +536,12 @@ static bool intel_fbc1_possible(struct intel_crtc *crtc)
 
 	obj = to_intel_framebuffer(fb)->obj;
 
+	if (obj->base.size > dev_priv->gtt.stolen_size) {
+		DRM_DEBUG("FBC pipe %c, plane %c: framebuffer too big for stolen\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane));
+		return false;
+	}
+
 	if (obj->tiling_mode != I915_TILING_X ||
 	    obj->fence_reg == I915_FENCE_REG_NONE) {
 		DRM_DEBUG("FBC pipe %c, plane %c: framebuffer not tiled or fenced\n",
@@ -648,6 +655,12 @@ static bool intel_fbc2_possible(struct intel_crtc *crtc)
 
 	obj = to_intel_framebuffer(fb)->obj;
 
+	if (obj->base.size > dev_priv->gtt.stolen_size) {
+		DRM_DEBUG("FBC pipe %c, plane %c: framebuffer too big for stolen\n",
+			  pipe_name(crtc->pipe), plane_name(crtc->plane));
+		return false;
+	}
+
 	if (obj->tiling_mode != I915_TILING_X ||
 	    obj->fence_reg == I915_FENCE_REG_NONE) {
 		DRM_DEBUG("FBC pipe %c, plane %c: framebuffer not tiled or fenced\n",
-- 
1.8.5.5

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

[PATCH 13/25] drm/i915: Use low level funciton to disable fbc at init/resume

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The hardware state might not match the software state here, so just
call the low level fbc disable hook to disable it in case the BIOS
left it enabled.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_suspend.c  | 5 +++--
 drivers/gpu/drm/i915/intel_display.c | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 043123c..9088d9ae 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -288,8 +288,9 @@ static void i915_restore_display(struct drm_device *dev)
 		I915_WRITE(PP_CONTROL, dev_priv->regfile.savePP_CONTROL);
 	}
 
-	/* only restore FBC info on the platform that supports FBC*/
-	intel_disable_fbc(dev);
+	/* disable FBC in case someone left it on */
+	if (dev_priv->display.disable_fbc)
+		dev_priv->display.disable_fbc(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 295bba7..53ed4a9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12342,7 +12342,8 @@ 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);
+	if (dev_priv->display.disable_fbc)
+		dev_priv->display.disable_fbc(dev);
 
 	drm_modeset_lock_all(dev);
 	intel_modeset_setup_hw_state(dev, false);
-- 
1.8.5.5

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

[PATCH 14/25] drm/i915: Move fbc function prototypes got intel_drv.h

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

fbc function prototypes are spread around in to headers. Collect them to
a single place.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  | 2 --
 drivers/gpu/drm/i915/intel_drv.h | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bc6d2cb..c92292a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2605,8 +2605,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 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/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9a8eb0e..7a48eda 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -959,6 +959,7 @@ void intel_update_sprite_watermarks(struct drm_plane *plane,
 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_disable_fbc(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);
-- 
1.8.5.5

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

[PATCH 15/25] drm/i915: Move fbc state into dev_priv.fbc

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  10 +--
 drivers/gpu/drm/i915/intel_display.c |   3 +-
 drivers/gpu/drm/i915/intel_drv.h     |   1 +
 drivers/gpu/drm/i915/intel_pm.c      | 130 +++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_sprite.c  |   2 +-
 5 files changed, 81 insertions(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c92292a..e88baee 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -410,7 +410,7 @@ struct dpll;
 
 struct drm_i915_display_funcs {
 	bool (*fbc_enabled)(struct drm_device *dev);
-	void (*enable_fbc)(struct drm_crtc *crtc);
+	void (*enable_fbc)(struct drm_device *dev);
 	void (*disable_fbc)(struct drm_device *dev);
 	int (*get_display_clock_speed)(struct drm_device *dev);
 	int (*get_fifo_size)(struct drm_device *dev, int plane);
@@ -600,14 +600,16 @@ struct intel_context {
 };
 
 struct i915_fbc {
+	struct intel_crtc *crtc;
 	unsigned long size;
-	unsigned int fb_id;
-	enum plane plane;
-	int y;
+	uint32_t pixel_format;
+	int fence_reg, pitch, y;
 
 	struct drm_mm_node *compressed_fb;
 	struct drm_mm_node *compressed_llb;
 
+	struct drm_i915_gem_object *obj;
+
 	struct intel_fbc_work {
 		struct delayed_work work;
 		struct drm_crtc *crtc;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 53ed4a9..beead24 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4010,7 +4010,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
 
 	intel_crtc_wait_for_pending_flips(crtc);
 
-	if (dev_priv->fbc.plane == plane)
+	if (dev_priv->fbc.crtc == intel_crtc)
 		intel_disable_fbc(dev);
 
 	hsw_disable_ips(intel_crtc);
@@ -12320,6 +12320,7 @@ void intel_modeset_init(struct drm_device *dev)
 		      INTEL_INFO(dev)->num_pipes > 1 ? "s" : "");
 
 	intel_ips_init(dev);
+	intel_fbc_init(dev);
 
 	for_each_pipe(pipe) {
 		intel_crtc_init(dev, pipe);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7a48eda..1da1f7b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -961,6 +961,7 @@ void intel_pm_setup(struct drm_device *dev);
 bool intel_fbc_enabled(struct drm_device *dev);
 void intel_disable_fbc(struct drm_device *dev);
 void intel_update_fbc(struct drm_device *dev);
+void intel_fbc_init(struct drm_device *dev);
 void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
 void intel_gpu_ips_teardown(void);
 int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 777167e..6b22e40 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -88,21 +88,17 @@ static void i8xx_disable_fbc(struct drm_device *dev)
 	DRM_DEBUG_KMS("disabled FBC\n");
 }
 
-static void i8xx_enable_fbc(struct drm_crtc *crtc)
+static void i8xx_enable_fbc(struct drm_device *dev)
 {
-	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_framebuffer *fb = crtc->primary->fb;
-	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
-	struct drm_i915_gem_object *obj = intel_fb->obj;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc *crtc = dev_priv->fbc.crtc;
 	int cfb_pitch;
 	int i;
 	u32 fbc_ctl;
 
 	cfb_pitch = dev_priv->fbc.size / FBC_LL_SIZE;
-	if (fb->pitches[0] < cfb_pitch)
-		cfb_pitch = fb->pitches[0];
+	if (dev_priv->fbc.pitch < cfb_pitch)
+		cfb_pitch = dev_priv->fbc.pitch;
 
 	/* FBC_CTL wants 32B or 64B units */
 	if (IS_GEN2(dev))
@@ -119,9 +115,9 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc)
 
 		/* 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);
+		fbc_ctl2 |= FBC_CTL_PLANE(crtc->plane);
 		I915_WRITE(FBC_CONTROL2, fbc_ctl2);
-		I915_WRITE(FBC_FENCE_OFF, crtc->y);
+		I915_WRITE(FBC_FENCE_OFF, dev_priv->fbc.y);
 	}
 
 	/* enable it... */
@@ -131,11 +127,11 @@ static void i8xx_enable_fbc(struct drm_crtc *crtc)
 	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;
+	fbc_ctl |= dev_priv->fbc.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));
+		      cfb_pitch, dev_priv->fbc.y, plane_name(crtc->plane));
 }
 
 static bool i8xx_fbc_enabled(struct drm_device *dev)
@@ -145,29 +141,25 @@ static bool i8xx_fbc_enabled(struct drm_device *dev)
 	return I915_READ(FBC_CONTROL) & FBC_CTL_EN;
 }
 
-static void g4x_enable_fbc(struct drm_crtc *crtc)
+static void g4x_enable_fbc(struct drm_device *dev)
 {
-	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_framebuffer *fb = crtc->primary->fb;
-	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
-	struct drm_i915_gem_object *obj = intel_fb->obj;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc *crtc = dev_priv->fbc.crtc;
 	u32 dpfc_ctl;
 
-	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_PLANE(crtc->plane) | DPFC_SR_EN;
+	if (drm_format_plane_cpp(dev_priv->fbc.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;
+	dpfc_ctl |= DPFC_CTL_FENCE_EN | dev_priv->fbc.fence_reg;
 
-	I915_WRITE(DPFC_FENCE_YOFF, crtc->y);
+	I915_WRITE(DPFC_FENCE_YOFF, dev_priv->fbc.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));
+	DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(crtc->plane));
 }
 
 static void g4x_disable_fbc(struct drm_device *dev)
@@ -217,38 +209,35 @@ static void sandybridge_blit_fbc_update(struct drm_device *dev)
 	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_MEDIA);
 }
 
-static void ironlake_enable_fbc(struct drm_crtc *crtc)
+static void ironlake_enable_fbc(struct drm_device *dev)
 {
-	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_framebuffer *fb = crtc->primary->fb;
-	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
-	struct drm_i915_gem_object *obj = intel_fb->obj;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc *crtc = dev_priv->fbc.crtc;
 	u32 dpfc_ctl;
 
-	dpfc_ctl = DPFC_CTL_PLANE(intel_crtc->plane);
-	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
+	dpfc_ctl = DPFC_CTL_PLANE(crtc->plane);
+	if (drm_format_plane_cpp(dev_priv->fbc.pixel_format, 0) == 2)
 		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
 	else
 		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
 	dpfc_ctl |= DPFC_CTL_FENCE_EN;
 	if (IS_GEN5(dev))
-		dpfc_ctl |= obj->fence_reg;
+		dpfc_ctl |= dev_priv->fbc.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);
+	I915_WRITE(ILK_DPFC_FENCE_YOFF, dev_priv->fbc.y);
+	I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(dev_priv->fbc.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_CPU_FENCE_ENABLE | dev_priv->fbc.fence_reg);
+		I915_WRITE(DPFC_CPU_FENCE_OFFSET, dev_priv->fbc.y);
 		sandybridge_blit_fbc_update(dev);
 	}
 
-	DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
+	DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(crtc->plane));
 }
 
 static void ironlake_disable_fbc(struct drm_device *dev)
@@ -273,18 +262,14 @@ static bool ironlake_fbc_enabled(struct drm_device *dev)
 	return I915_READ(ILK_DPFC_CONTROL) & DPFC_CTL_EN;
 }
 
-static void gen7_enable_fbc(struct drm_crtc *crtc)
+static void gen7_enable_fbc(struct drm_device *dev)
 {
-	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_framebuffer *fb = crtc->primary->fb;
-	struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
-	struct drm_i915_gem_object *obj = intel_fb->obj;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc *crtc = dev_priv->fbc.crtc;
 	u32 dpfc_ctl;
 
-	dpfc_ctl = IVB_DPFC_CTL_PLANE(intel_crtc->plane);
-	if (drm_format_plane_cpp(fb->pixel_format, 0) == 2)
+	dpfc_ctl = IVB_DPFC_CTL_PLANE(crtc->plane);
+	if (drm_format_plane_cpp(dev_priv->fbc.pixel_format, 0) == 2)
 		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
 	else
 		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
@@ -299,18 +284,18 @@ static void gen7_enable_fbc(struct drm_crtc *crtc)
 			   ILK_FBCQ_DIS);
 	} else {
 		/* WaFbcAsynchFlipDisableFbcQueue:hsw,bdw */
-		I915_WRITE(CHICKEN_PIPESL_1(intel_crtc->pipe),
-			   I915_READ(CHICKEN_PIPESL_1(intel_crtc->pipe)) |
+		I915_WRITE(CHICKEN_PIPESL_1(crtc->pipe),
+			   I915_READ(CHICKEN_PIPESL_1(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_CPU_FENCE_ENABLE | dev_priv->fbc.fence_reg);
+	I915_WRITE(DPFC_CPU_FENCE_OFFSET, dev_priv->fbc.y);
 
 	sandybridge_blit_fbc_update(dev);
 
-	DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(intel_crtc->plane));
+	DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(crtc->plane));
 }
 
 bool intel_fbc_enabled(struct drm_device *dev)
@@ -323,6 +308,21 @@ bool intel_fbc_enabled(struct drm_device *dev)
 	return dev_priv->display.fbc_enabled(dev);
 }
 
+static void intel_fbc_update_params(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_framebuffer *fb = crtc->base.primary->fb;
+	struct drm_i915_gem_object *obj = to_intel_framebuffer(fb)->obj;
+
+	dev_priv->fbc.crtc = crtc;
+	dev_priv->fbc.pixel_format = fb->pixel_format;
+	dev_priv->fbc.pitch = fb->pitches[0];
+	dev_priv->fbc.obj = obj;
+	dev_priv->fbc.fence_reg = obj->fence_reg;
+	dev_priv->fbc.y = crtc->base.y;
+}
+
 static void intel_fbc_work_fn(struct work_struct *__work)
 {
 	struct intel_fbc_work *work =
@@ -337,11 +337,8 @@ static void intel_fbc_work_fn(struct work_struct *__work)
 		 * 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;
+			intel_fbc_update_params(to_intel_crtc(work->crtc));
+			dev_priv->display.enable_fbc(dev);
 		}
 
 		dev_priv->fbc.fbc_work = NULL;
@@ -388,7 +385,8 @@ static void intel_enable_fbc(struct drm_crtc *crtc)
 	work = kzalloc(sizeof(*work), GFP_KERNEL);
 	if (work == NULL) {
 		DRM_ERROR("Failed to allocate FBC work structure\n");
-		dev_priv->display.enable_fbc(crtc);
+		intel_fbc_update_params(to_intel_crtc(crtc));
+		dev_priv->display.enable_fbc(dev);
 		return;
 	}
 
@@ -424,7 +422,11 @@ void intel_disable_fbc(struct drm_device *dev)
 		return;
 
 	dev_priv->display.disable_fbc(dev);
-	dev_priv->fbc.plane = -1;
+	dev_priv->fbc.crtc = NULL;
+	dev_priv->fbc.pixel_format = 0;
+	dev_priv->fbc.pitch = 0;
+	dev_priv->fbc.fence_reg = I915_FENCE_REG_NONE;
+	dev_priv->fbc.y = 0;
 }
 
 static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
@@ -812,8 +814,11 @@ void intel_update_fbc(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 == crtc->plane &&
-	    dev_priv->fbc.fb_id == fb->base.id &&
+	if (dev_priv->fbc.crtc == crtc &&
+	    dev_priv->fbc.obj == obj &&
+	    dev_priv->fbc.pixel_format == fb->pixel_format &&
+	    dev_priv->fbc.pitch == fb->pitches[0] &&
+	    dev_priv->fbc.fence_reg == obj->fence_reg &&
 	    dev_priv->fbc.y == crtc->base.y)
 		return;
 
@@ -858,6 +863,13 @@ out_disable:
 	i915_gem_stolen_cleanup_compression(dev);
 }
 
+void intel_fbc_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	dev_priv->fbc.fence_reg = I915_FENCE_REG_NONE;
+}
+
 static void i915_pineview_get_mem_freq(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 404335d..e04ca5d 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -711,7 +711,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_disable_fbc(dev);
 	mutex_unlock(&dev->struct_mutex);
 
-- 
1.8.5.5

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

[PATCH 16/25] drm/i915: Rewrite fbc

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Redesign the fbc code to deal with multiple pipes better, fix the
locking problems, and in general try to utilize the hardware front
buffer tracking features to their full extent.

For each crtc we will compute a pending fbc score as (pixel_rate * cpp).
At appropriate times (after a flip, on the next vblank after set_base,
etc.) the pending score will get promoted to and active score. Based on
the active score of all crtcs we then pick which crtc is best candidate
for fbc.

Note that this now allows FBC to be enabled when multiple pipes are
active. That seems have been always intended as supported on FBC2, but
the FBC1 docs are a bit more conservative about this. But that might
be just an oversight in the docs. The code now allows FBC with multiple
active pipes even on FBC1, and thus far I've not seen any ill effects
from this on my 855.

The locking is now handled by a new fbc.mutex which protects most of
dev_priv->fbc and the hardware state. dev_priv->fbc.obj is used to track
the front buffer for render/blitter tracking so it's protected by
struct_mutex.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |   1 +
 drivers/gpu/drm/i915/i915_drv.h      |  13 +-
 drivers/gpu/drm/i915/intel_display.c | 105 +++++-
 drivers/gpu/drm/i915/intel_drv.h     |  18 +-
 drivers/gpu/drm/i915/intel_pm.c      | 665 +++++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_sprite.c  |  15 +-
 6 files changed, 569 insertions(+), 248 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f046a3c..9d97adb 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -538,6 +538,7 @@ static int i915_drm_freeze(struct drm_device *dev)
 		drm_modeset_unlock_all(dev);
 
 		intel_ips_cleanup(dev);
+		intel_fbc_cleanup(dev);
 
 		intel_modeset_suspend_hw(dev);
 	}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e88baee..a0e615e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -600,21 +600,24 @@ struct intel_context {
 };
 
 struct i915_fbc {
+	/* protects most fbc state */
+	struct mutex mutex;
+
 	struct intel_crtc *crtc;
 	unsigned long size;
 	uint32_t pixel_format;
+	unsigned int score;
 	int fence_reg, pitch, y;
 
 	struct drm_mm_node *compressed_fb;
 	struct drm_mm_node *compressed_llb;
 
+	/* for render tracking, protected by struct_mutex */
 	struct drm_i915_gem_object *obj;
 
-	struct intel_fbc_work {
-		struct delayed_work work;
-		struct drm_crtc *crtc;
-		struct drm_framebuffer *fb;
-	} *fbc_work;
+	struct intel_ring_notify notify;
+
+	struct work_struct enable_work, update_work;
 
 	enum no_fbc_reason {
 		FBC_OK, /* FBC is enabled */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index beead24..e4cacd9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2746,13 +2746,37 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 		intel_crtc->config.pipe_src_h = adjusted_mode->crtc_vdisplay;
 	}
 
-	dev_priv->display.update_primary_plane(crtc, fb, x, y);
-
 	old_fb = crtc->primary->fb;
 	crtc->primary->fb = fb;
 	crtc->x = x;
 	crtc->y = y;
 
+	mutex_lock(&dev_priv->fbc.mutex);
+	intel_fbc_pre_page_flip(intel_crtc);
+
+	dev_priv->display.update_primary_plane(crtc, fb, x, y);
+
+	/*
+	 * Now we can update fbc.obj for render/blitter tracking.
+	 * The flip will nuke so no need for seqno waits here.
+	 */
+	if (intel_crtc == intel_fbc_best_crtc(dev)) {
+		/*
+		 * fbc should not be enabled unless it's
+		 * on the current crtc thanks to
+		 * intel_fbc_pre_page_flip().
+		 */
+		WARN_ON(dev_priv->fbc.crtc != NULL &&
+			dev_priv->fbc.crtc != intel_crtc);
+
+		mutex_lock(&dev->struct_mutex);
+		intel_fbc_update_object(dev, to_intel_framebuffer(fb)->obj);
+		mutex_unlock(&dev->struct_mutex);
+	}
+
+	intel_fbc_post_page_flip(intel_crtc, true);
+	mutex_unlock(&dev_priv->fbc.mutex);
+
 	if (old_fb) {
 		if (intel_crtc->active && old_fb != fb)
 			intel_wait_for_vblank(dev, intel_crtc->pipe);
@@ -2762,7 +2786,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 	}
 
 	mutex_lock(&dev->struct_mutex);
-	intel_update_fbc(dev);
 	intel_edp_psr_update(dev);
 	mutex_unlock(&dev->struct_mutex);
 
@@ -3994,8 +4017,11 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc)
 
 	hsw_enable_ips(intel_crtc);
 
+	mutex_lock(&dev_priv->fbc.mutex);
+	intel_fbc_post_plane_enable(intel_crtc);
+	mutex_unlock(&dev_priv->fbc.mutex);
+
 	mutex_lock(&dev->struct_mutex);
-	intel_update_fbc(dev);
 	intel_edp_psr_update(dev);
 	mutex_unlock(&dev->struct_mutex);
 }
@@ -4010,8 +4036,9 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
 
 	intel_crtc_wait_for_pending_flips(crtc);
 
-	if (dev_priv->fbc.crtc == intel_crtc)
-		intel_disable_fbc(dev);
+	mutex_lock(&dev_priv->fbc.mutex);
+	intel_fbc_disable(intel_crtc);
+	mutex_unlock(&dev_priv->fbc.mutex);
 
 	hsw_disable_ips(intel_crtc);
 
@@ -4287,8 +4314,11 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	intel_crtc->active = false;
 	intel_update_watermarks(crtc);
 
+	mutex_lock(&dev_priv->fbc.mutex);
+	intel_fbc_schedule_update(dev);
+	mutex_unlock(&dev_priv->fbc.mutex);
+
 	mutex_lock(&dev->struct_mutex);
-	intel_update_fbc(dev);
 	intel_edp_psr_update(dev);
 	mutex_unlock(&dev->struct_mutex);
 }
@@ -4335,8 +4365,11 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	intel_crtc->active = false;
 	intel_update_watermarks(crtc);
 
+	mutex_lock(&dev_priv->fbc.mutex);
+	intel_fbc_schedule_update(dev);
+	mutex_unlock(&dev_priv->fbc.mutex);
+
 	mutex_lock(&dev->struct_mutex);
-	intel_update_fbc(dev);
 	intel_edp_psr_update(dev);
 	mutex_unlock(&dev->struct_mutex);
 }
@@ -4887,8 +4920,11 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	intel_crtc->active = false;
 	intel_update_watermarks(crtc);
 
+	mutex_lock(&dev_priv->fbc.mutex);
+	intel_fbc_schedule_update(dev);
+	mutex_unlock(&dev_priv->fbc.mutex);
+
 	mutex_lock(&dev->struct_mutex);
-	intel_update_fbc(dev);
 	intel_edp_psr_update(dev);
 	mutex_unlock(&dev->struct_mutex);
 }
@@ -8924,8 +8960,6 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 	intel_unpin_fb_obj(work->old_fb_obj);
 	drm_gem_object_unreference(&work->pending_flip_obj->base);
 	drm_gem_object_unreference(&work->old_fb_obj->base);
-
-	intel_update_fbc(dev);
 	mutex_unlock(&dev->struct_mutex);
 
 	BUG_ON(atomic_read(&to_intel_crtc(work->crtc)->unpin_work_count) == 0);
@@ -8973,6 +9007,8 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
 
 	queue_work(dev_priv->wq, &work->work);
 
+	intel_fbc_finish_page_flip(intel_crtc);
+
 	trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj);
 }
 
@@ -9531,8 +9567,17 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
 	atomic_inc(&intel_crtc->unpin_work_count);
 
+	mutex_unlock(&dev->struct_mutex);
+
 	crtc->primary->fb = fb;
 
+	mutex_lock(&dev_priv->fbc.mutex);
+	intel_fbc_pre_page_flip(intel_crtc);
+
+	ret = i915_mutex_lock_interruptible(dev);
+	if (ret)
+		goto cleanup_fbc;
+
 	/* Reference the objects for the scheduled work. */
 	drm_gem_object_reference(&work->old_fb_obj->base);
 	drm_gem_object_reference(&obj->base);
@@ -9558,10 +9603,28 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	if (ret)
 		goto cleanup_all;
 
-	intel_disable_fbc(dev);
+	/*
+	 * Now we can update fbc.obj for render/blitter tracking.
+	 * The flip will nuke so no need for seqno waits here.
+	 */
+	if (intel_crtc == intel_fbc_best_crtc(dev)) {
+		/*
+		 * fbc should not be enabled unless it's
+		 * on the current crtc thanks to
+		 * intel_fbc_pre_page_flip().
+		 */
+		WARN_ON(dev_priv->fbc.crtc != NULL &&
+			dev_priv->fbc.crtc != intel_crtc);
+
+		intel_fbc_update_object(dev, obj);
+	}
+
 	intel_mark_fb_busy(obj, NULL);
 	mutex_unlock(&dev->struct_mutex);
 
+	intel_fbc_post_page_flip(intel_crtc, false);
+	mutex_unlock(&dev_priv->fbc.mutex);
+
 	trace_i915_flip_request(intel_crtc->plane, obj);
 
 	return 0;
@@ -9569,7 +9632,20 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 cleanup_all:
 	drm_gem_object_unreference(&work->old_fb_obj->base);
 	drm_gem_object_unreference(&obj->base);
+	mutex_unlock(&dev->struct_mutex);
+
+cleanup_fbc:
+	/*
+	 * Restore fb before doing the fbc cleanup so that
+	 * fbc.pending_score gets correctly computed.
+	 */
 	crtc->primary->fb = old_fb;
+	/* This should get fbc back on track, eventually */
+	intel_fbc_disable(intel_crtc);
+	intel_fbc_post_plane_enable(intel_crtc);
+	mutex_unlock(&dev_priv->fbc.mutex);
+
+	mutex_lock(&dev->struct_mutex);
 	atomic_dec(&intel_crtc->unpin_work_count);
 	intel_unpin_fb_obj(obj);
 cleanup_unlock:
@@ -11548,6 +11624,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	spin_lock_init(&intel_crtc->lock);
 	INIT_LIST_HEAD(&intel_crtc->vblank_notify_list);
 
+	intel_fbc_crtc_init(intel_crtc);
+
 	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;
@@ -12849,8 +12927,6 @@ void intel_modeset_cleanup(struct drm_device *dev)
 		intel_increase_pllclock(crtc);
 	}
 
-	intel_disable_fbc(dev);
-
 	intel_disable_gt_powersave(dev);
 
 	ironlake_teardown_rc6(dev);
@@ -12858,6 +12934,7 @@ void intel_modeset_cleanup(struct drm_device *dev)
 	mutex_unlock(&dev->struct_mutex);
 
 	intel_ips_cleanup(dev);
+	intel_fbc_cleanup(dev);
 
 	/* flush any delayed tasks or pending work */
 	flush_scheduled_work();
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1da1f7b..52e95f1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -420,6 +420,11 @@ struct intel_crtc {
 
 	struct list_head vblank_notify_list;
 	spinlock_t lock;
+
+	struct {
+		struct intel_vblank_notify notify;
+		unsigned int score, pending_score;
+	} fbc;
 };
 
 struct intel_plane_wm_parameters {
@@ -959,9 +964,18 @@ void intel_update_sprite_watermarks(struct drm_plane *plane,
 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_disable_fbc(struct drm_device *dev);
-void intel_update_fbc(struct drm_device *dev);
 void intel_fbc_init(struct drm_device *dev);
+void intel_fbc_crtc_init(struct intel_crtc *crtc);
+void intel_fbc_cleanup(struct drm_device *dev);
+void intel_fbc_pre_page_flip(struct intel_crtc *crtc);
+void intel_fbc_post_page_flip(struct intel_crtc *crtc, bool use_vblank_notify);
+void intel_fbc_finish_page_flip(struct intel_crtc *crtc);
+void intel_fbc_update_object(struct drm_device *dev,
+			     struct drm_i915_gem_object *obj);
+void intel_fbc_post_plane_enable(struct intel_crtc *crtc);
+void intel_fbc_disable(struct intel_crtc *crtc);
+void intel_fbc_schedule_update(struct drm_device *dev);
+struct intel_crtc *intel_fbc_best_crtc(struct drm_device *dev);
 void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
 void intel_gpu_ips_teardown(void);
 int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6b22e40..dd3f807 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -308,6 +308,28 @@ bool intel_fbc_enabled(struct drm_device *dev)
 	return dev_priv->display.fbc_enabled(dev);
 }
 
+void intel_fbc_update_object(struct drm_device *dev,
+			     struct drm_i915_gem_object *obj)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	lockdep_assert_held(&dev_priv->fbc.mutex);
+	lockdep_assert_held(&dev->struct_mutex);
+
+	intel_ring_notify_cancel(&dev_priv->fbc.notify);
+
+	if (obj == dev_priv->fbc.obj)
+		return;
+
+	if (obj)
+		drm_gem_object_reference(&obj->base);
+
+	if (dev_priv->fbc.obj)
+		drm_gem_object_unreference(&dev_priv->fbc.obj->base);
+
+	dev_priv->fbc.obj = obj;
+}
+
 static void intel_fbc_update_params(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
@@ -316,117 +338,55 @@ static void intel_fbc_update_params(struct intel_crtc *crtc)
 	struct drm_i915_gem_object *obj = to_intel_framebuffer(fb)->obj;
 
 	dev_priv->fbc.crtc = crtc;
+	dev_priv->fbc.score = crtc->fbc.score;
 	dev_priv->fbc.pixel_format = fb->pixel_format;
 	dev_priv->fbc.pitch = fb->pitches[0];
-	dev_priv->fbc.obj = obj;
 	dev_priv->fbc.fence_reg = obj->fence_reg;
 	dev_priv->fbc.y = crtc->base.y;
-}
-
-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) {
-			intel_fbc_update_params(to_intel_crtc(work->crtc));
-			dev_priv->display.enable_fbc(dev);
-		}
-
-		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;
+	dev_priv->fbc.no_fbc_reason = FBC_OK;
 }
 
-static void intel_enable_fbc(struct drm_crtc *crtc)
+static void __intel_fbc_disable(struct intel_crtc *crtc)
 {
-	struct intel_fbc_work *work;
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_device *dev;
+	struct drm_i915_private *dev_priv;
 
-	if (!dev_priv->display.enable_fbc)
+	if (!crtc)
 		return;
 
-	intel_cancel_fbc_work(dev_priv);
+	dev = crtc->base.dev;
+	dev_priv = dev->dev_private;
 
-	work = kzalloc(sizeof(*work), GFP_KERNEL);
-	if (work == NULL) {
-		DRM_ERROR("Failed to allocate FBC work structure\n");
-		intel_fbc_update_params(to_intel_crtc(crtc));
-		dev_priv->display.enable_fbc(dev);
-		return;
-	}
+	lockdep_assert_held(&dev_priv->fbc.mutex);
 
-	work->crtc = crtc;
-	work->fb = crtc->primary->fb;
-	INIT_DELAYED_WORK(&work->work, intel_fbc_work_fn);
+	if (dev_priv->fbc.crtc != crtc)
+		return;
 
-	dev_priv->fbc.fbc_work = work;
+	dev_priv->display.disable_fbc(dev);
+	dev_priv->fbc.crtc = NULL;
+	dev_priv->fbc.score = 0;
+	dev_priv->fbc.pixel_format = 0;
+	dev_priv->fbc.pitch = 0;
+	dev_priv->fbc.fence_reg = I915_FENCE_REG_NONE;
+	dev_priv->fbc.y = 0;
 
-	/* 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));
+	mutex_lock(&dev->struct_mutex);
+	intel_fbc_update_object(dev, NULL);
+	mutex_unlock(&dev->struct_mutex);
 }
 
-void intel_disable_fbc(struct drm_device *dev)
+void intel_fbc_disable(struct intel_crtc *crtc)
 {
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	intel_cancel_fbc_work(dev_priv);
+	lockdep_assert_held(&dev_priv->fbc.mutex);
 
-	if (!dev_priv->display.disable_fbc)
-		return;
+	intel_vblank_notify_cancel(&crtc->fbc.notify);
+	crtc->fbc.score = crtc->fbc.pending_score = 0;
 
-	dev_priv->display.disable_fbc(dev);
-	dev_priv->fbc.crtc = NULL;
-	dev_priv->fbc.pixel_format = 0;
-	dev_priv->fbc.pitch = 0;
-	dev_priv->fbc.fence_reg = I915_FENCE_REG_NONE;
-	dev_priv->fbc.y = 0;
+	__intel_fbc_disable(crtc);
 }
 
 static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
@@ -673,201 +633,464 @@ static bool intel_fbc2_possible(struct intel_crtc *crtc)
 	return true;
 }
 
-static struct intel_crtc *intel_fbc1_pick_crtc(struct drm_device *dev)
+static void intel_fbc_update_pending_score(struct intel_crtc *crtc)
 {
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *tmp_crtc, *crtc = NULL;
+	unsigned int pixel_rate, cpp;
+	bool ok;
 
-	list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, base.head) {
-		if (!tmp_crtc->active)
-			continue;
+	lockdep_assert_held(&crtc->base.mutex.mutex.base);
+	lockdep_assert_held(&dev_priv->fbc.mutex);
 
-		if (crtc) {
-			if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
-				DRM_DEBUG_KMS("more than one pipe active, disabling FBC\n");
-			return NULL;
-		}
+	WARN_ON(intel_vblank_notify_pending(&crtc->fbc.notify));
 
-		crtc = tmp_crtc;
-	}
+	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5)
+		ok = intel_fbc2_possible(crtc);
+	else
+		ok = intel_fbc1_possible(crtc);
 
-	if (!crtc || !intel_fbc1_possible(crtc)) {
-		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_CONFIG))
-			DRM_DEBUG_KMS("no suitable plane, disabling FBC\n");
-		return NULL;
+	if (!ok) {
+		crtc->fbc.pending_score = 0;
+		return;
 	}
 
-	return crtc;
+	/*
+	 * If we have multiple possibilities, prefer the pipe with
+	 * the highest data rate (pixel_rate * cpp).
+	 *
+	 * FIXME: is this a good heuristic?
+	 *
+	 * FIXME: We should have something like ilk_pipe_pixel_rate()
+	 * for GMCH platforms too, but here it doesn't matter since
+	 * we anyway disallow FBC with panel fitting on g4x.
+	 */
+	if (HAS_PCH_SPLIT(dev))
+		pixel_rate = ilk_pipe_pixel_rate(dev, &crtc->base);
+	else
+		pixel_rate = crtc->config.adjusted_mode.crtc_clock;
+
+	cpp = drm_format_plane_cpp(crtc->base.primary->fb->pixel_format, 0);
+
+	crtc->fbc.pending_score = pixel_rate * cpp;
 }
 
-static struct intel_crtc *intel_fbc2_pick_crtc(struct drm_device *dev)
+static void intel_fbc_schedule_enable(struct intel_crtc *crtc)
 {
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *tmp_crtc, *crtc = NULL;
-	unsigned int pixel_rate = 0;
+	struct drm_i915_gem_object *obj =
+		to_intel_framebuffer(crtc->base.primary->fb)->obj;
+	int ret;
 
-	list_for_each_entry(tmp_crtc, &dev->mode_config.crtc_list, base.head) {
-		unsigned int tmp_pixel_rate;
+	lockdep_assert_held(&crtc->base.mutex.mutex.base);
+	lockdep_assert_held(&dev_priv->fbc.mutex);
 
-		if (!intel_fbc2_possible(tmp_crtc))
-			continue;
+	WARN_ON(dev_priv->fbc.crtc == crtc);
 
+	/*
+	 * Must disable fbc before touching the compressed
+	 * buffer, or most fbc paramerers.
+	 *
+	 * Note that there might be a pending ring notify
+	 * still but fbc mutex will prevent it from doign stuff.
+	 *
+	 * Use the low level function to avoid clearing fbc.obj
+	 * to skip the seqno wait in case the crtc changes but
+	 * the obj doesn't.
+	 */
+	dev_priv->display.disable_fbc(dev);
+
+	mutex_lock(&dev->struct_mutex);
+
+	ret = i915_gem_stolen_setup_compression(dev, obj->base.size);
+	if (ret) {
 		/*
-		 * If we have multiple possibilities, prefer
-		 * the pipe with the highest pixel rate.
-		 *
-		 * FIXME: is this a good heuristic?
-		 *
-		 * FIXME: We should have something like ilk_pipe_pixel_rate()
-		 * for GMCH platforms too, but here it doesn't matter since
-		 * we anyway disallow FBC with panel fitting on g4x.
+		 * FIXME what now? maybe clear the score for
+		 * the current crtc so we might try again on
+		 * another crtc?
 		 */
-		if (HAS_PCH_SPLIT(dev))
-			tmp_pixel_rate = ilk_pipe_pixel_rate(dev, &tmp_crtc->base);
-		else
-			tmp_pixel_rate = tmp_crtc->config.adjusted_mode.crtc_clock;
+		i915_gem_stolen_cleanup_compression(dev);
+		mutex_unlock(&dev->struct_mutex);
+		__intel_fbc_disable(dev_priv->fbc.crtc);
+		return;
+	}
 
-		if (pixel_rate < tmp_pixel_rate) {
-			crtc = tmp_crtc;
-			pixel_rate = tmp_pixel_rate;
+	/*
+	 * If the object changes, we must make sure we wait for
+	 * all previous writes to land before enabling fbc.
+	 */
+	if (dev_priv->fbc.obj != obj) {
+		struct intel_engine_cs *ring = NULL;
+
+		intel_fbc_update_object(dev, obj);
+		dev_priv->fbc.notify.seqno = obj->last_write_seqno;
+		if (dev_priv->fbc.notify.seqno != 0)
+			ring = obj->ring;
+
+		if (ring) {
+			ret = intel_ring_notify_add(ring, &dev_priv->fbc.notify);
+			if (ret) {
+				/* FIXME what now? */
+				i915_gem_stolen_cleanup_compression(dev);
+				mutex_unlock(&dev->struct_mutex);
+				__intel_fbc_disable(dev_priv->fbc.crtc);
+				return;
+			}
 		}
 	}
 
-	if (!crtc)
-		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_CONFIG))
-			DRM_DEBUG_KMS("no suitable plane, disabling FBC\n");
+	mutex_unlock(&dev->struct_mutex);
+
+	intel_fbc_update_params(crtc);
 
-	return crtc;
+	/*
+	 * If the object changed add a new ring notification.
+	 * If the object didn't change we either wait for the
+	 * already pending ring notification, or we just directly
+	 * enable fbc (as the seqno must have passed already).
+	 */
+	if (!intel_ring_notify_pending(&dev_priv->fbc.notify))
+		dev_priv->display.enable_fbc(dev);
 }
 
-/**
- * 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 intel_crtc *intel_fbc_best_crtc(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *crtc;
-	struct drm_framebuffer *fb;
-	struct drm_i915_gem_object *obj;
+	struct intel_crtc *crtc, *best_crtc;
+
+	lockdep_assert_held(&dev_priv->fbc.mutex);
 
 	if (!HAS_FBC(dev)) {
 		set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED);
-		return;
+		return NULL;
 	}
 
 	if (!i915.powersave) {
 		if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM))
 			DRM_DEBUG_KMS("fbc disabled per module param\n");
-		return;
+		return NULL;
 	}
 
 	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;
+			DRM_DEBUG_KMS("fbc disabled per chip default\n");
+		return NULL;
 	}
+
 	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;
+		return NULL;
 	}
 
-	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5)
-		crtc = intel_fbc2_pick_crtc(dev);
-	else
-		crtc = intel_fbc1_pick_crtc(dev);
+	/* prefer to maintain the status quo if scores are equal */
+	best_crtc = dev_priv->fbc.crtc;
 
-	if (!crtc)
-		goto out_disable;
+	for_each_intel_crtc(dev_priv->dev, crtc) {
+		if (crtc->fbc.score == 0)
+			continue;
 
-	fb = crtc->base.primary->fb;
-	obj = to_intel_framebuffer(fb)->obj;
+		if (!best_crtc || crtc->fbc.score > best_crtc->fbc.score)
+			best_crtc = crtc;
+	}
 
-	/* If the kernel debugger is active, always disable compression */
-	if (in_dbg_master())
-		goto out_disable;
+	WARN_ON(best_crtc && best_crtc->fbc.score == 0);
 
-	if (i915_gem_stolen_setup_compression(dev, obj->base.size)) {
-		if (set_no_fbc_reason(dev_priv, FBC_STOLEN_TOO_SMALL))
-			DRM_DEBUG_KMS("framebuffer too large, disabling compression\n");
-		goto out_disable;
-	}
+	if (!best_crtc)
+		if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_CONFIG))
+			DRM_DEBUG_KMS("no suitable plane, disabling FBC\n");
+
+	return best_crtc;
+}
 
-	/* 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.
+static void intel_fbc_disable_full(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	__intel_fbc_disable(dev_priv->fbc.crtc);
+	mutex_lock(&dev->struct_mutex);
+	i915_gem_stolen_cleanup_compression(dev);
+	mutex_unlock(&dev->struct_mutex);
+}
+
+void intel_fbc_schedule_update(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *best_crtc;
+
+	lockdep_assert_held(&dev_priv->fbc.mutex);
+
+	best_crtc = intel_fbc_best_crtc(dev);
+
+	if (best_crtc == NULL)
+		intel_fbc_disable_full(dev);
+	else if (best_crtc != dev_priv->fbc.crtc)
+		schedule_work(&dev_priv->fbc.update_work);
+}
+
+static bool intel_fbc_need_reinit(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+	struct drm_framebuffer *fb = crtc->base.primary->fb;
+	struct drm_i915_gem_object *obj = to_intel_framebuffer(fb)->obj;
+
+	return crtc != dev_priv->fbc.crtc ||
+		obj->base.size > dev_priv->fbc.size ||
+		drm_format_plane_cpp(fb->pixel_format, 0) !=
+		drm_format_plane_cpp(dev_priv->fbc.pixel_format, 0) ||
+		fb->pitches[0] != dev_priv->fbc.pitch;
+}
+
+void intel_fbc_pre_page_flip(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *best_crtc;
+
+	lockdep_assert_held(&crtc->base.mutex.mutex.base);
+	lockdep_assert_held(&dev_priv->fbc.mutex);
+
+	WARN_ON(crtc->fbc.score == 0 && dev_priv->fbc.crtc == crtc);
+
+	intel_vblank_notify_cancel(&crtc->fbc.notify);
+	intel_fbc_update_pending_score(crtc);
+
+	/*
+	 * If fbc was already possible we can update immediately,
+	 * otherwise we will wait until the flip is finished.
+	 */
+	if (crtc->fbc.score != 0)
+		crtc->fbc.score = crtc->fbc.pending_score;
+
+	/*
+	 * Disable fbc if we're not (yet) capable, or if
+	 * we just need a full disable+enable reinit.
+	 */
+	if (crtc->fbc.score == 0 || intel_fbc_need_reinit(crtc))
+		__intel_fbc_disable(crtc);
+
+	best_crtc = intel_fbc_best_crtc(dev);
+
+	/*
+	 * Since the current crtc is the best, we're going to update
+	 * fbc.obj to the current fb (usually a bit later when we have
+	 * the protection of struct_mutex), Before doing that however,
+	 * we must kick out the previous owner.
 	 */
-	if (dev_priv->fbc.crtc == crtc &&
-	    dev_priv->fbc.obj == obj &&
-	    dev_priv->fbc.pixel_format == fb->pixel_format &&
-	    dev_priv->fbc.pitch == fb->pitches[0] &&
-	    dev_priv->fbc.fence_reg == obj->fence_reg &&
-	    dev_priv->fbc.y == crtc->base.y)
+	if (crtc == best_crtc && crtc != dev_priv->fbc.crtc)
+		__intel_fbc_disable(dev_priv->fbc.crtc);
+
+	if (dev_priv->fbc.crtc == crtc) {
+		/* We still have fbc, so just update the fence. */
+		intel_fbc_update_params(crtc);
+		dev_priv->display.enable_fbc(dev);
+	}
+}
+
+void intel_fbc_post_page_flip(struct intel_crtc *crtc,
+			      bool use_vblank_notify)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *best_crtc;
+
+	lockdep_assert_held(&crtc->base.mutex.mutex.base);
+	lockdep_assert_held(&dev_priv->fbc.mutex);
+
+	WARN_ON(intel_vblank_notify_pending(&crtc->fbc.notify));
+
+	best_crtc = intel_fbc_best_crtc(dev);
+
+	/* already handled in intel_fbc_pre_pageflip() */
+	if (crtc == dev_priv->fbc.crtc) {
+		WARN_ON(crtc->fbc.pending_score != crtc->fbc.score ||
+			crtc != best_crtc);
 		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.
+	if (crtc->fbc.pending_score != crtc->fbc.score) {
+		int ret;
+
+		WARN_ON(crtc->fbc.pending_score == 0 ||
+			crtc->fbc.score != 0);
+
+		/* will be handled by intel_fbc_finish_page_flip() */
+		if (!use_vblank_notify)
+			return;
+
+		/*
+		 * fbc wasn't possible previosuly, so must wait for the
+		 * next vblank before fbc.score can be updated.
 		 */
-		DRM_DEBUG_KMS("disabling active FBC for update\n");
-		intel_disable_fbc(dev);
+		crtc->fbc.notify.vbl_count = intel_crtc_vbl_count_rel_to_abs(crtc, 1);
+		ret = intel_vblank_notify_add(crtc, &crtc->fbc.notify);
+		WARN_ON(ret != 0);
+		return;
 	}
 
-	intel_enable_fbc(&crtc->base);
-	dev_priv->fbc.no_fbc_reason = FBC_OK;
-	return;
+	/*
+	 * If we're the best but not currently active,
+	 * perform the full fbc enable. We have the crtc->mutex
+	 * so no need to schedule a work for this.
+	 * But maybe we want the work anyway to avoid doing
+	 * so much work in the page flip ioctl?
+	 *
+	 * If someone else is better suited for the task,
+	 * schedule a full update work.
+	 */
+	if (best_crtc == crtc)
+		intel_fbc_schedule_enable(crtc);
+	else
+		intel_fbc_schedule_update(dev);
+}
+
+void intel_fbc_finish_page_flip(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+
+	WARN_ON(intel_vblank_notify_pending(&crtc->fbc.notify));
 
-out_disable:
-	/* Multiple disables should be harmless */
-	if (intel_fbc_enabled(dev)) {
-		DRM_DEBUG_KMS("unsupported config, disabling FBC\n");
-		intel_disable_fbc(dev);
+	if (crtc->fbc.score != crtc->fbc.pending_score) {
+		WARN_ON(crtc->fbc.pending_score == 0);
+		crtc->fbc.score = crtc->fbc.pending_score;
+		schedule_work(&dev_priv->fbc.update_work);
 	}
-	i915_gem_stolen_cleanup_compression(dev);
 }
 
+/*
+ * Update fbc.pending_score, and schedule an update of
+ * fbc.score for the crtc. This should only be called
+ * when fbc wasn't previosuly possible, ie. after
+ * enabling the primary plane.
+ */
+void intel_fbc_post_plane_enable(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	lockdep_assert_held(&crtc->base.mutex.mutex.base);
+	lockdep_assert_held(&dev_priv->fbc.mutex);
+
+	WARN_ON(intel_vblank_notify_pending(&crtc->fbc.notify));
+
+	WARN_ON(crtc->fbc.score != 0 || dev_priv->fbc.crtc == crtc);
+
+	intel_fbc_update_pending_score(crtc);
+	if (crtc->fbc.pending_score != 0)
+		intel_fbc_post_page_flip(crtc, true);
+}
+
+static void intel_fbc_vblank_notify(struct intel_vblank_notify *notify)
+{
+	struct intel_crtc *crtc =
+		container_of(notify, struct intel_crtc, fbc.notify);
+	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
+
+	WARN_ON(crtc->fbc.score != 0 ||
+		crtc->fbc.score == crtc->fbc.pending_score);
+
+	crtc->fbc.score = crtc->fbc.pending_score;
+	schedule_work(&dev_priv->fbc.update_work);
+}
+
+static void intel_fbc_enable_work(struct work_struct *work)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(work, struct drm_i915_private, fbc.enable_work);
+
+	mutex_lock(&dev_priv->fbc.mutex);
+
+	if (dev_priv->fbc.crtc &&
+	    !intel_ring_notify_pending(&dev_priv->fbc.notify))
+		dev_priv->display.enable_fbc(dev_priv->dev);
+
+	mutex_unlock(&dev_priv->fbc.mutex);
+}
+
+static void intel_fbc_ring_notify(struct intel_ring_notify *notify)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(notify, struct drm_i915_private, fbc.notify);
+
+	schedule_work(&dev_priv->fbc.enable_work);
+}
+
+void intel_fbc_crtc_init(struct intel_crtc *crtc)
+{
+	crtc->fbc.notify.notify = intel_fbc_vblank_notify;
+}
+
+static void intel_fbc_update_work(struct work_struct *work)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(work, struct drm_i915_private, fbc.update_work);
+	struct drm_device *dev = dev_priv->dev;
+	struct intel_crtc *crtc, *best_crtc;
+
+	if (in_dbg_master())
+		return;
+
+	mutex_lock(&dev_priv->fbc.mutex);
+
+	crtc = intel_fbc_best_crtc(dev);
+
+	if (crtc == NULL)
+		intel_fbc_disable_full(dev);
+	else if (crtc == dev_priv->fbc.crtc)
+		/* nothing to do */
+		crtc = NULL;
+
+	mutex_unlock(&dev_priv->fbc.mutex);
+
+	if (!crtc)
+		return;
+
+	drm_modeset_lock(&crtc->base.mutex, NULL);
+	mutex_lock(&dev_priv->fbc.mutex);
+
+	/* check that things didn't change when we dropped fbc.mutex */
+	best_crtc = intel_fbc_best_crtc(dev);
+	if (crtc == best_crtc && crtc != dev_priv->fbc.crtc)
+		intel_fbc_schedule_enable(crtc);
+
+	mutex_unlock(&dev_priv->fbc.mutex);
+	drm_modeset_unlock(&crtc->base.mutex);
+}
+
+/* to be called before crtc init */
 void intel_fbc_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	dev_priv->fbc.fence_reg = I915_FENCE_REG_NONE;
+
+	mutex_init(&dev_priv->fbc.mutex);
+	dev_priv->fbc.notify.notify = intel_fbc_ring_notify;
+	INIT_WORK(&dev_priv->fbc.enable_work, intel_fbc_enable_work);
+	INIT_WORK(&dev_priv->fbc.update_work, intel_fbc_update_work);
+}
+
+/* to be called after all crtcs are disabled */
+void intel_fbc_cleanup(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *crtc;
+
+	mutex_lock(&dev_priv->fbc.mutex);
+	for_each_intel_crtc(dev, crtc)
+		intel_fbc_disable(crtc);
+	mutex_unlock(&dev_priv->fbc.mutex);
+	cancel_work_sync(&dev_priv->fbc.update_work);
+
+	intel_ring_notify_cancel(&dev_priv->fbc.notify);
+	cancel_work_sync(&dev_priv->fbc.enable_work);
+
+	WARN_ON(dev_priv->fbc.crtc != NULL);
+
+	mutex_lock(&dev->struct_mutex);
+	i915_gem_stolen_cleanup_compression(dev);
+	mutex_unlock(&dev->struct_mutex);
 }
 
 static void i915_pineview_get_mem_freq(struct drm_device *dev)
@@ -6808,6 +7031,8 @@ void intel_init_pm(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	mutex_init(&dev_priv->fbc.mutex);
+
 	if (HAS_FBC(dev)) {
 		if (INTEL_INFO(dev)->gen >= 7) {
 			dev_priv->display.fbc_enabled = ironlake_fbc_enabled;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index e04ca5d..4509bda 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -688,6 +688,7 @@ static void
 intel_post_enable_primary(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
 	/*
@@ -698,9 +699,9 @@ intel_post_enable_primary(struct drm_crtc *crtc)
 	 */
 	hsw_enable_ips(intel_crtc);
 
-	mutex_lock(&dev->struct_mutex);
-	intel_update_fbc(dev);
-	mutex_unlock(&dev->struct_mutex);
+	mutex_lock(&dev_priv->fbc.mutex);
+	intel_fbc_post_plane_enable(intel_crtc);
+	mutex_unlock(&dev_priv->fbc.mutex);
 }
 
 static void
@@ -710,10 +711,10 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	mutex_lock(&dev->struct_mutex);
-	if (dev_priv->fbc.crtc == intel_crtc)
-		intel_disable_fbc(dev);
-	mutex_unlock(&dev->struct_mutex);
+	mutex_lock(&dev_priv->fbc.mutex);
+	intel_fbc_disable(intel_crtc);
+	intel_fbc_schedule_update(dev);
+	mutex_unlock(&dev_priv->fbc.mutex);
 
 	/*
 	 * FIXME IPS should be fine as long as one plane is
-- 
1.8.5.5

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

[PATCH 17/25] drm/i915: Reduce dmesg spam from FBC enable

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Only print the FBC enabled debug message when FBC wasn't already
enabled. Now we call the .enable_fbc() function every time we need to
update the CPU fence during page flips, so dmesg gets considerable
amounts of pointless spam.

Also unify the format of the message across all gens.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index dd3f807..6de283b 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -96,6 +96,9 @@ static void i8xx_enable_fbc(struct drm_device *dev)
 	int i;
 	u32 fbc_ctl;
 
+	if ((I915_READ(FBC_CONTROL) & FBC_CTL_EN) == 0)
+		DRM_DEBUG_KMS("enabling FBC on plane %c\n", plane_name(crtc->plane));
+
 	cfb_pitch = dev_priv->fbc.size / FBC_LL_SIZE;
 	if (dev_priv->fbc.pitch < cfb_pitch)
 		cfb_pitch = dev_priv->fbc.pitch;
@@ -129,9 +132,6 @@ static void i8xx_enable_fbc(struct drm_device *dev)
 	fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
 	fbc_ctl |= dev_priv->fbc.fence_reg;
 	I915_WRITE(FBC_CONTROL, fbc_ctl);
-
-	DRM_DEBUG_KMS("enabled FBC, pitch %d, yoff %d, plane %c\n",
-		      cfb_pitch, dev_priv->fbc.y, plane_name(crtc->plane));
 }
 
 static bool i8xx_fbc_enabled(struct drm_device *dev)
@@ -147,6 +147,9 @@ static void g4x_enable_fbc(struct drm_device *dev)
 	struct intel_crtc *crtc = dev_priv->fbc.crtc;
 	u32 dpfc_ctl;
 
+	if ((I915_READ(DPFC_CONTROL) & DPFC_CTL_EN) == 0)
+		DRM_DEBUG_KMS("enabling FBC on plane %c\n", plane_name(crtc->plane));
+
 	dpfc_ctl = DPFC_CTL_PLANE(crtc->plane) | DPFC_SR_EN;
 	if (drm_format_plane_cpp(dev_priv->fbc.pixel_format, 0) == 2)
 		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
@@ -158,8 +161,6 @@ static void g4x_enable_fbc(struct drm_device *dev)
 
 	/* enable it... */
 	I915_WRITE(DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
-
-	DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(crtc->plane));
 }
 
 static void g4x_disable_fbc(struct drm_device *dev)
@@ -215,6 +216,9 @@ static void ironlake_enable_fbc(struct drm_device *dev)
 	struct intel_crtc *crtc = dev_priv->fbc.crtc;
 	u32 dpfc_ctl;
 
+	if ((I915_READ(ILK_DPFC_CONTROL) & DPFC_CTL_EN) == 0)
+		DRM_DEBUG_KMS("enabling FBC on plane %c\n", plane_name(crtc->plane));
+
 	dpfc_ctl = DPFC_CTL_PLANE(crtc->plane);
 	if (drm_format_plane_cpp(dev_priv->fbc.pixel_format, 0) == 2)
 		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
@@ -236,8 +240,6 @@ static void ironlake_enable_fbc(struct drm_device *dev)
 		I915_WRITE(DPFC_CPU_FENCE_OFFSET, dev_priv->fbc.y);
 		sandybridge_blit_fbc_update(dev);
 	}
-
-	DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(crtc->plane));
 }
 
 static void ironlake_disable_fbc(struct drm_device *dev)
@@ -268,6 +270,9 @@ static void gen7_enable_fbc(struct drm_device *dev)
 	struct intel_crtc *crtc = dev_priv->fbc.crtc;
 	u32 dpfc_ctl;
 
+	if ((I915_READ(ILK_DPFC_CONTROL) & DPFC_CTL_EN) == 0)
+		DRM_DEBUG_KMS("enabling fbc on plane %c\n", plane_name(crtc->plane));
+
 	dpfc_ctl = IVB_DPFC_CTL_PLANE(crtc->plane);
 	if (drm_format_plane_cpp(dev_priv->fbc.pixel_format, 0) == 2)
 		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
@@ -294,8 +299,6 @@ static void gen7_enable_fbc(struct drm_device *dev)
 	I915_WRITE(DPFC_CPU_FENCE_OFFSET, dev_priv->fbc.y);
 
 	sandybridge_blit_fbc_update(dev);
-
-	DRM_DEBUG_KMS("enabled fbc on plane %c\n", plane_name(crtc->plane));
 }
 
 bool intel_fbc_enabled(struct drm_device *dev)
-- 
1.8.5.5

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

[PATCH 18/25] drm/i915: Add i915_fbc_info debugfs file

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a debugfs file which shows the fbc state in all its glory.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 44 +++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 91c64f9..1eacbae 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1444,6 +1444,49 @@ static int i915_drpc_info(struct seq_file *m, void *unused)
 		return ironlake_drpc_info(m);
 }
 
+static int i915_fbc_info(struct seq_file *m, void *unused)
+{
+	struct drm_info_node *node = m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *crtc;
+
+	intel_runtime_pm_get(dev_priv);
+
+	mutex_lock(&dev_priv->fbc.mutex);
+
+	seq_printf(m, "FBC state:\n"
+		   "supported = %d\n"
+		   "enabled in hardware = %d\n"
+		   "compressed buffer size = %lu\n"
+		   "score = %d\n"
+		   "plane = %c\n"
+		   "pixel_format = %s\n"
+		   "pitch = %d\n"
+		   "fence_reg = %d\n"
+		   "y = %d\n"
+		   "obj = %p\n",
+		   HAS_FBC(dev), intel_fbc_enabled(dev),
+		   dev_priv->fbc.size, dev_priv->fbc.score,
+		   dev_priv->fbc.crtc ?
+		   plane_name(dev_priv->fbc.crtc->plane) : '?',
+		   dev_priv->fbc.pixel_format ?
+		   drm_get_format_name(dev_priv->fbc.pixel_format) : "?",
+		   dev_priv->fbc.pitch, dev_priv->fbc.fence_reg,
+		   dev_priv->fbc.y, dev_priv->fbc.obj);
+
+	for_each_intel_crtc(dev, crtc)
+		seq_printf(m, "plane %c: score = %d, pending_score = %d\n",
+			   plane_name(crtc->plane), crtc->fbc.score,
+			   crtc->fbc.pending_score);
+
+	mutex_unlock(&dev_priv->fbc.mutex);
+
+	intel_runtime_pm_put(dev_priv);
+
+	return 0;
+}
+
 static int i915_fbc_status(struct seq_file *m, void *unused)
 {
 	struct drm_info_node *node = m->private;
@@ -3807,6 +3850,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_emon_status", i915_emon_status, 0},
 	{"i915_ring_freq_table", i915_ring_freq_table, 0},
 	{"i915_gfxec", i915_gfxec, 0},
+	{"i915_fbc_info", i915_fbc_info, 0},
 	{"i915_fbc_status", i915_fbc_status, 0},
 	{"i915_ips_status", i915_ips_status, 0},
 	{"i915_sr_status", i915_sr_status, 0},
-- 
1.8.5.5

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

[PATCH v6 19/25] drm/i915: Implement LRI based FBC tracking

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

As per the SNB and HSW PM guides, we should enable FBC render/blitter
tracking only during batches targetting the front buffer.

On SNB we must also update the FBC render tracking address whenever it
changes. And since the register in question is stored in the context,
we need to make sure we reload it with correct data after context
switches.

On IVB/HSW we use the render nuke mechanism, so no render tracking
address updates are needed. Hoever on the blitter side we need to
enable the blitter tracking like on SNB, and in addition we need
to issue the cache clean messages, which we already did.

v2: Introduce intel_fb_obj_has_fbc()
    Fix crtc locking around crtc->fb access
    Drop a hunk that was included by accident in v1
    Set fbc_address_dirty=false not true after emitting the LRI
v3: Now that fbc hangs on to the fb intel_fb_obj_has_fbc() doesn't
    need to upset lockdep anymore
v4: Use |= instead of = to update fbc_address_dirty
v5: |= for fbc_dirty too, kill fbc_obj variable, pack the
    intel_ringbuffer dirty bits using bitfields, skip ILK_FBC_RT_BASE
    write on SNB+, kill sandybridge_blit_fbc_update(), reorganize
    code to make future ILK FBC RT LRI support easier
v6: Rework based on new fbc.obj handling

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c    |  7 ++++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 29 +++++++++++++++
 drivers/gpu/drm/i915/intel_display.c       |  2 -
 drivers/gpu/drm/i915/intel_drv.h           |  1 +
 drivers/gpu/drm/i915/intel_pm.c            | 33 ++---------------
 drivers/gpu/drm/i915/intel_ringbuffer.c    | 59 +++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  6 ++-
 7 files changed, 102 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 3ffe308..1aab053 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -588,6 +588,13 @@ mi_set_context(struct intel_engine_cs *ring,
 
 	intel_ring_advance(ring);
 
+	/*
+	 * FBC RT address is stored in the context, so we may have just
+	 * restored it to an old value. Make sure we emit a new LRI
+	 * to update the address.
+	 */
+	ring->fbc_address_dirty = true;
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 93d7f72..5a91dae 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -955,6 +955,33 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
 }
 
 static void
+i915_gem_execbuffer_mark_fbc_dirty(struct intel_engine_cs *ring,
+				   struct list_head *vmas)
+{
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
+	struct i915_vma *vma;
+	u32 fbc_address = -1;
+
+	list_for_each_entry(vma, vmas, exec_list) {
+		struct drm_i915_gem_object *obj = vma->obj;
+
+		if (obj->base.pending_write_domain &&
+		    obj == dev_priv->fbc.obj) {
+			WARN_ON(fbc_address != -1 &&
+				fbc_address != i915_gem_obj_ggtt_offset(obj));
+			fbc_address = i915_gem_obj_ggtt_offset(obj);
+		}
+	}
+
+	/* need to nuke/cache_clean on IVB+? */
+	ring->fbc_dirty |= fbc_address != -1;
+
+	/* need to update FBC tracking? */
+	ring->fbc_address_dirty |= fbc_address != ring->fbc_address;
+	ring->fbc_address = fbc_address;
+}
+
+static void
 i915_gem_execbuffer_move_to_active(struct list_head *vmas,
 				   struct intel_engine_cs *ring)
 {
@@ -1322,6 +1349,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	else
 		exec_start += i915_gem_obj_offset(batch_obj, vm);
 
+	i915_gem_execbuffer_mark_fbc_dirty(ring, &eb->vmas);
+
 	ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas);
 	if (ret)
 		goto err;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e4cacd9..3011c35 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8923,8 +8923,6 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
 			continue;
 
 		intel_increase_pllclock(crtc);
-		if (ring && intel_fbc_enabled(dev))
-			ring->fbc_dirty = true;
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 52e95f1..e9de8b1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -732,6 +732,7 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev);
 int intel_pch_rawclk(struct drm_device *dev);
 int valleyview_cur_cdclk(struct drm_i915_private *dev_priv);
 void intel_mark_busy(struct drm_device *dev);
+bool intel_fb_obj_has_fbc(struct drm_i915_gem_object *obj);
 void intel_mark_fb_busy(struct drm_i915_gem_object *obj,
 			struct intel_engine_cs *ring);
 void intel_mark_idle(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6de283b..a5d3d9f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -185,31 +185,6 @@ static bool g4x_fbc_enabled(struct drm_device *dev)
 	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_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -229,8 +204,9 @@ static void ironlake_enable_fbc(struct drm_device *dev)
 		dpfc_ctl |= dev_priv->fbc.fence_reg;
 
 	I915_WRITE(ILK_DPFC_FENCE_YOFF, dev_priv->fbc.y);
-	I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(dev_priv->fbc.obj) |
-		   ILK_FBC_RT_VALID);
+	if (IS_GEN5(dev))
+		I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(dev_priv->fbc.obj) |
+			   ILK_FBC_RT_VALID);
 	/* enable it... */
 	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
 
@@ -238,7 +214,6 @@ static void ironlake_enable_fbc(struct drm_device *dev)
 		I915_WRITE(SNB_DPFC_CTL_SA,
 			   SNB_CPU_FENCE_ENABLE | dev_priv->fbc.fence_reg);
 		I915_WRITE(DPFC_CPU_FENCE_OFFSET, dev_priv->fbc.y);
-		sandybridge_blit_fbc_update(dev);
 	}
 }
 
@@ -297,8 +272,6 @@ static void gen7_enable_fbc(struct drm_device *dev)
 	I915_WRITE(SNB_DPFC_CTL_SA,
 		   SNB_CPU_FENCE_ENABLE | dev_priv->fbc.fence_reg);
 	I915_WRITE(DPFC_CPU_FENCE_OFFSET, dev_priv->fbc.y);
-
-	sandybridge_blit_fbc_update(dev);
 }
 
 bool intel_fbc_enabled(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 31321ae..d19738a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -69,6 +69,57 @@ void __intel_ring_advance(struct intel_engine_cs *ring)
 	ring->write_tail(ring, ringbuf->tail);
 }
 
+static int gen5_render_fbc_tracking(struct intel_engine_cs *ring)
+{
+	int ret;
+
+	if (!ring->fbc_address_dirty)
+		return 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+
+	intel_ring_emit(ring, MI_NOOP);
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+	intel_ring_emit(ring, ILK_FBC_RT_BASE);
+	if (ring->fbc_address != -1)
+		intel_ring_emit(ring, ring->fbc_address |
+				SNB_FBC_FRONT_BUFFER | ILK_FBC_RT_VALID);
+	else
+		intel_ring_emit(ring, 0);
+	intel_ring_advance(ring);
+
+	ring->fbc_address_dirty = false;
+
+	return 0;
+}
+
+static int gen6_blt_fbc_tracking(struct intel_engine_cs *ring)
+{
+	int ret;
+
+	if (!ring->fbc_address_dirty)
+		return 0;
+
+	ret = intel_ring_begin(ring, 4);
+	if (ret)
+		return ret;
+
+	intel_ring_emit(ring, MI_NOOP);
+	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+	intel_ring_emit(ring, GEN6_BLITTER_ECOSKPD);
+	if (ring->fbc_address != -1)
+		intel_ring_emit(ring, _MASKED_BIT_ENABLE(GEN6_BLITTER_FBC_NOTIFY));
+	else
+		intel_ring_emit(ring, _MASKED_BIT_DISABLE(GEN6_BLITTER_FBC_NOTIFY));
+	intel_ring_advance(ring);
+
+	ring->fbc_address_dirty = false;
+
+	return 0;
+}
+
 static int
 gen2_render_ring_flush(struct intel_engine_cs *ring,
 		       u32	invalidate_domains,
@@ -274,6 +325,9 @@ gen6_render_ring_flush(struct intel_engine_cs *ring,
 	intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
 
+	if (invalidate_domains)
+		return gen5_render_fbc_tracking(ring);
+
 	return 0;
 }
 
@@ -316,6 +370,7 @@ static int gen7_ring_fbc_flush(struct intel_engine_cs *ring, u32 value)
 	intel_ring_advance(ring);
 
 	ring->fbc_dirty = false;
+
 	return 0;
 }
 
@@ -1938,7 +1993,9 @@ static int gen6_ring_flush(struct intel_engine_cs *ring,
 	}
 	intel_ring_advance(ring);
 
-	if (IS_GEN7(dev) && !invalidate && flush)
+	if (invalidate)
+		return gen6_blt_fbc_tracking(ring);
+	else if (flush && IS_GEN7(dev))
 		return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 273abf3..80406ab 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -176,8 +176,10 @@ struct  intel_engine_cs {
 	 */
 	struct drm_i915_gem_request *preallocated_lazy_request;
 	u32 outstanding_lazy_seqno;
-	bool gpu_caches_dirty;
-	bool fbc_dirty;
+	u32 fbc_address;
+	bool gpu_caches_dirty:1;
+	bool fbc_dirty:1;
+	bool fbc_address_dirty:1;
 
 	wait_queue_head_t irq_queue;
 
-- 
1.8.5.5

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

[PATCH v3 20/25] drm/i915: Use LRI based FBC render tracking for ILK

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

ILK should work pretty much the same as SNB, except it
doesn't have the blitter, so we only care about render tracking.

v2: Rebased against earlier changes
v3: Rebased due to fbc.obj changes

Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         | 2 +-
 drivers/gpu/drm/i915/intel_pm.c         | 3 ---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 5 ++++-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 78310c1..f0caab3 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1503,7 +1503,7 @@ enum punit_power_well {
 #define ILK_DPFC_CHICKEN	0x43224
 #define ILK_FBC_RT_BASE		0x2128
 #define   ILK_FBC_RT_VALID	(1<<0)
-#define   SNB_FBC_FRONT_BUFFER	(1<<1)
+#define   ILK_FBC_FRONT_BUFFER	(1<<1)
 
 #define ILK_DISPLAY_CHICKEN1	0x42000
 #define   ILK_FBCQ_DIS		(1<<22)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a5d3d9f..3bde3ce 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -204,9 +204,6 @@ static void ironlake_enable_fbc(struct drm_device *dev)
 		dpfc_ctl |= dev_priv->fbc.fence_reg;
 
 	I915_WRITE(ILK_DPFC_FENCE_YOFF, dev_priv->fbc.y);
-	if (IS_GEN5(dev))
-		I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(dev_priv->fbc.obj) |
-			   ILK_FBC_RT_VALID);
 	/* enable it... */
 	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index d19738a..dbfa280 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -85,7 +85,7 @@ static int gen5_render_fbc_tracking(struct intel_engine_cs *ring)
 	intel_ring_emit(ring, ILK_FBC_RT_BASE);
 	if (ring->fbc_address != -1)
 		intel_ring_emit(ring, ring->fbc_address |
-				SNB_FBC_FRONT_BUFFER | ILK_FBC_RT_VALID);
+				ILK_FBC_FRONT_BUFFER | ILK_FBC_RT_VALID);
 	else
 		intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
@@ -201,6 +201,9 @@ gen4_render_ring_flush(struct intel_engine_cs *ring,
 	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
 
+	if (invalidate_domains && IS_GEN5(dev))
+		return gen5_render_fbc_tracking(ring);
+
 	return 0;
 }
 
-- 
1.8.5.5

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

[PATCH v2 21/25] drm/i915: Reorder i915_gem_execbuffer_move_to_gpu() and i915_switch_context()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The FBC RT address is stored in the context, and thus needs to be
rewritten after a context switch before any batches are run. We emit
the LRI to update the FBC RT address when we call the ring ->flush
function to invalidate the caches.

When a context switch is being performed we currently do the
invalidate before the context switch, which means we're not updating
the FBC RT address correctly. Move the i915_gem_execbuffer_move_to_gpu()
call to happen after i915_switch_context() to make the LRI happen
after the MI_SET_CONTEXT.

v2: Rebased

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 5a91dae..7a328f6 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1351,11 +1351,11 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 
 	i915_gem_execbuffer_mark_fbc_dirty(ring, &eb->vmas);
 
-	ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas);
+	ret = i915_switch_context(ring, ctx);
 	if (ret)
 		goto err;
 
-	ret = i915_switch_context(ring, ctx);
+	ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas);
 	if (ret)
 		goto err;
 
-- 
1.8.5.5

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

[PATCH 22/25] drm/i915: Flush caches for scanout during cpu->gtt move

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Flush the caches when moving a scanout buffer from CPU to GTT domain.
This allows us to move a scanout buffer to CPU write domain, do some
writes, and move it back to the GTT read domain. The display will then
see the correct data. In addition we still need to do the dirtyfb
ioctl to nuke FBC if that's enabled.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a5e62cb..0ce66da 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3613,7 +3613,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
 		return ret;
 
 	i915_gem_object_retire(obj);
-	i915_gem_object_flush_cpu_write_domain(obj, false);
+	i915_gem_object_flush_cpu_write_domain(obj, obj->pin_display);
 
 	/* Serialise direct access to this object with the barriers for
 	 * coherent writes from the GPU, by effectively invalidating the
-- 
1.8.5.5

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

[PATCH v5 23/25] drm/i915: Nuke FBC from SW_FINISH ioctl

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

FBC host modification tracking only works through GTT mmaps, so any
direct CPU access needs to manually nuke the compressed framebuffer
on modifications. Do the nuking from the SW_FINISH ioctl.

v2: nuke from SW_FINISH insted of DIRTYFB ioctl
v3: Call intel_fbc_nuke() only when pin_display is true
v4: Don't oops if dev_priv->fbc.fb is NULL
v5: Rework based on fbc.mutex and fbc.obj

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c  |  8 ++++++++
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c  | 31 +++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0ce66da..a1753dc 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1434,6 +1434,7 @@ int
 i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
 			 struct drm_file *file)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_sw_finish *args = data;
 	struct drm_i915_gem_object *obj;
 	int ret = 0;
@@ -1457,6 +1458,13 @@ i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
 	drm_gem_object_unreference(&obj->base);
 unlock:
 	mutex_unlock(&dev->struct_mutex);
+
+	if (ret == 0) {
+		mutex_lock(&dev_priv->fbc.mutex);
+		intel_fbc_nuke(obj);
+		mutex_unlock(&dev_priv->fbc.mutex);
+	}
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e9de8b1..161639b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -977,6 +977,7 @@ void intel_fbc_post_plane_enable(struct intel_crtc *crtc);
 void intel_fbc_disable(struct intel_crtc *crtc);
 void intel_fbc_schedule_update(struct drm_device *dev);
 struct intel_crtc *intel_fbc_best_crtc(struct drm_device *dev);
+void intel_fbc_nuke(struct drm_i915_gem_object *obj);
 void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
 void intel_gpu_ips_teardown(void);
 int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3bde3ce..f3d76aa 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -362,6 +362,37 @@ void intel_fbc_disable(struct intel_crtc *crtc)
 	__intel_fbc_disable(crtc);
 }
 
+void intel_fbc_nuke(struct drm_i915_gem_object *obj)
+{
+	struct drm_device *dev = obj->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *crtc = dev_priv->fbc.crtc;
+	unsigned int pending_score;
+	int ret;
+
+	lockdep_assert_held(&dev_priv->fbc.mutex);
+
+	if (dev_priv->fbc.obj != obj)
+		return;
+
+	if (!crtc)
+		return;
+
+	WARN_ON(crtc->fbc.pending_score == 0);
+
+	pending_score = crtc->fbc.pending_score;
+	intel_fbc_disable(crtc);
+	crtc->fbc.pending_score = pending_score;
+
+	/*
+	 * Must wait until the next vblank before re-enabling
+	 * otherwise the nuking won't actually happen.
+	 */
+	crtc->fbc.notify.vbl_count = intel_crtc_vbl_count_rel_to_abs(crtc, 1);
+	ret = intel_vblank_notify_add(crtc, &crtc->fbc.notify);
+	WARN_ON(ret != 0);
+}
+
 static bool set_no_fbc_reason(struct drm_i915_private *dev_priv,
 			      enum no_fbc_reason reason)
 {
-- 
1.8.5.5

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

[PATCH 24/25] drm/i915: Pimp fbc render/blitter tracking

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The FBC RT address is saved as part of the context, so track it
the same way in the driver to skip pointless LRIs. Also try
make sure we re-emit the render/blitter tracking stuff on driver
load and gpu reset.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            |  1 +
 drivers/gpu/drm/i915/i915_gem.c            |  3 +++
 drivers/gpu/drm/i915/i915_gem_context.c    | 21 ++++++++++++++-------
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 27 +++++++++++++++++----------
 drivers/gpu/drm/i915/intel_ringbuffer.c    | 17 ++++++++++-------
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  7 +++++--
 6 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a0e615e..c6401ba 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -587,6 +587,7 @@ struct i915_ctx_hang_stats {
 #define DEFAULT_CONTEXT_ID 0
 struct intel_context {
 	struct kref ref;
+	unsigned long fbc_address;
 	int id;
 	bool is_initialized;
 	uint8_t remap_slice;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a1753dc..3fac2c6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2563,6 +2563,9 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
 	ring->preallocated_lazy_request = NULL;
 	ring->outstanding_lazy_seqno = 0;
 
+	/* current state unknown so force a FBC RT address update */
+	ring->fbc_address = I915_FBC_RT_RESET;
+
 	spin_lock_irq(&ring->lock);
 	list_for_each_entry_safe(notify, next, &ring->notify_list, list) {
 		intel_ring_notify_complete(notify);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 1aab053..c5e50f0 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -273,6 +273,9 @@ __create_hw_context(struct drm_device *dev,
 	 * is no remap info, it will be a NOP. */
 	ctx->remap_slice = (1 << NUM_L3_SLICES(dev)) - 1;
 
+	/* force a re-emit FBC RT address on first use */
+	ctx->fbc_address = I915_FBC_RT_RESET;
+
 	return ctx;
 
 err_out:
@@ -359,6 +362,7 @@ err_destroy:
 void i915_gem_context_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_context *ctx;
 	int i;
 
 	/* Prevent the hardware from restoring the last context (which hung) on
@@ -386,6 +390,13 @@ void i915_gem_context_reset(struct drm_device *dev)
 		i915_gem_context_reference(dctx);
 		ring->last_context = dctx;
 	}
+
+	/*
+	 * FBC RT address update(s) may have been lost,
+	 * just have everyone re-emit them.
+	 */
+	list_for_each_entry(ctx, &dev_priv->context_list, link)
+		ctx->fbc_address = I915_FBC_RT_RESET;
 }
 
 int i915_gem_context_init(struct drm_device *dev)
@@ -588,13 +599,6 @@ mi_set_context(struct intel_engine_cs *ring,
 
 	intel_ring_advance(ring);
 
-	/*
-	 * FBC RT address is stored in the context, so we may have just
-	 * restored it to an old value. Make sure we emit a new LRI
-	 * to update the address.
-	 */
-	ring->fbc_address_dirty = true;
-
 	return ret;
 }
 
@@ -667,6 +671,9 @@ static int do_switch(struct intel_engine_cs *ring,
 	if (ret)
 		goto unpin_out;
 
+	if ((hw_flags & MI_RESTORE_INHIBIT) == 0)
+		ring->fbc_address = to->fbc_address;
+
 	for (i = 0; i < MAX_L3_SLICES; i++) {
 		if (!(to->remap_slice & (1<<i)))
 			continue;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 7a328f6..046f62b 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -956,29 +956,33 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file,
 
 static void
 i915_gem_execbuffer_mark_fbc_dirty(struct intel_engine_cs *ring,
+				   struct intel_context *ctx,
 				   struct list_head *vmas)
 {
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 	struct i915_vma *vma;
-	u32 fbc_address = -1;
+	unsigned long fbc_address = I915_FBC_RT_NONE;
 
 	list_for_each_entry(vma, vmas, exec_list) {
 		struct drm_i915_gem_object *obj = vma->obj;
 
 		if (obj->base.pending_write_domain &&
 		    obj == dev_priv->fbc.obj) {
-			WARN_ON(fbc_address != -1 &&
-				fbc_address != i915_gem_obj_ggtt_offset(obj));
-			fbc_address = i915_gem_obj_ggtt_offset(obj);
+			WARN_ON(fbc_address != I915_FBC_RT_NONE &&
+				fbc_address != vma->node.start);
+			fbc_address = vma->node.start;
 		}
 	}
 
-	/* need to nuke/cache_clean on IVB+? */
-	ring->fbc_dirty |= fbc_address != -1;
-
 	/* need to update FBC tracking? */
-	ring->fbc_address_dirty |= fbc_address != ring->fbc_address;
-	ring->fbc_address = fbc_address;
+
+	/* simple yes/no is sufficient for !RCS */
+	if (ring->id != RCS && fbc_address != I915_FBC_RT_NONE)
+		ring->pending_fbc_address = 0;
+	else
+		ring->pending_fbc_address = fbc_address;
+
+	ring->fbc_dirty |= fbc_address != I915_FBC_RT_NONE;
 }
 
 static void
@@ -1349,7 +1353,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	else
 		exec_start += i915_gem_obj_offset(batch_obj, vm);
 
-	i915_gem_execbuffer_mark_fbc_dirty(ring, &eb->vmas);
+	i915_gem_execbuffer_mark_fbc_dirty(ring, ctx, &eb->vmas);
 
 	ret = i915_switch_context(ring, ctx);
 	if (ret)
@@ -1359,6 +1363,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	if (ret)
 		goto err;
 
+	if (HAS_HW_CONTEXTS(dev) && ring->id == RCS)
+		ctx->fbc_address = ring->fbc_address;
+
 	if (ring == &dev_priv->ring[RCS] &&
 	    mode != dev_priv->relative_constants_mode) {
 		ret = intel_ring_begin(ring, 4);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index dbfa280..6655eb5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -73,7 +73,7 @@ static int gen5_render_fbc_tracking(struct intel_engine_cs *ring)
 {
 	int ret;
 
-	if (!ring->fbc_address_dirty)
+	if (ring->fbc_address == ring->pending_fbc_address)
 		return 0;
 
 	ret = intel_ring_begin(ring, 4);
@@ -83,14 +83,14 @@ static int gen5_render_fbc_tracking(struct intel_engine_cs *ring)
 	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
 	intel_ring_emit(ring, ILK_FBC_RT_BASE);
-	if (ring->fbc_address != -1)
-		intel_ring_emit(ring, ring->fbc_address |
+	if (ring->pending_fbc_address != I915_FBC_RT_NONE)
+		intel_ring_emit(ring, ring->pending_fbc_address |
 				ILK_FBC_FRONT_BUFFER | ILK_FBC_RT_VALID);
 	else
 		intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
 
-	ring->fbc_address_dirty = false;
+	ring->fbc_address = ring->pending_fbc_address;
 
 	return 0;
 }
@@ -99,7 +99,7 @@ static int gen6_blt_fbc_tracking(struct intel_engine_cs *ring)
 {
 	int ret;
 
-	if (!ring->fbc_address_dirty)
+	if (ring->fbc_address == ring->pending_fbc_address)
 		return 0;
 
 	ret = intel_ring_begin(ring, 4);
@@ -109,13 +109,13 @@ static int gen6_blt_fbc_tracking(struct intel_engine_cs *ring)
 	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
 	intel_ring_emit(ring, GEN6_BLITTER_ECOSKPD);
-	if (ring->fbc_address != -1)
+	if (ring->pending_fbc_address != I915_FBC_RT_NONE)
 		intel_ring_emit(ring, _MASKED_BIT_ENABLE(GEN6_BLITTER_FBC_NOTIFY));
 	else
 		intel_ring_emit(ring, _MASKED_BIT_DISABLE(GEN6_BLITTER_FBC_NOTIFY));
 	intel_ring_advance(ring);
 
-	ring->fbc_address_dirty = false;
+	ring->fbc_address = ring->pending_fbc_address;
 
 	return 0;
 }
@@ -1505,6 +1505,9 @@ static int intel_init_ring_buffer(struct drm_device *dev,
 
 	init_waitqueue_head(&ring->irq_queue);
 
+	/* current state unknown so force a FBC RT address update */
+	ring->fbc_address = I915_FBC_RT_RESET;
+
 	INIT_LIST_HEAD(&ring->notify_list);
 	spin_lock_init(&ring->lock);
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 80406ab..b8f9fd2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -175,11 +175,14 @@ struct  intel_engine_cs {
 	 * Do we have some not yet emitted requests outstanding?
 	 */
 	struct drm_i915_gem_request *preallocated_lazy_request;
+
+#define I915_FBC_RT_NONE 1 /* FBC RT disabled */
+#define I915_FBC_RT_RESET 2 /* force re-emit of FBC RT */
+	unsigned long fbc_address, pending_fbc_address;
+
 	u32 outstanding_lazy_seqno;
-	u32 fbc_address;
 	bool gpu_caches_dirty:1;
 	bool fbc_dirty:1;
-	bool fbc_address_dirty:1;
 
 	wait_queue_head_t irq_queue;
 
-- 
1.8.5.5

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

[PATCH 25/25] drm/i915: Enable fbc for ilk+ by default

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Things should work just fine now for FBC2 platforms. However on g4x we
don't have any code to set up the FBC watermarks so I'm hesitant to
enable it there by default. FBC1 platforms could use more testing, and
also my 855 seems to experience some GPU+system hangs occasionally when
FBC is enabled. So FBC1 seems to need further investigations.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f3d76aa..ecfb6ab 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -776,7 +776,7 @@ struct intel_crtc *intel_fbc_best_crtc(struct drm_device *dev)
 		return NULL;
 	}
 
-	if (i915.enable_fbc < 0) {
+	if (i915.enable_fbc < 0 && INTEL_INFO(dev)->gen < 5) {
 		if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT))
 			DRM_DEBUG_KMS("fbc disabled per chip default\n");
 		return NULL;
-- 
1.8.5.5

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

Re: [PATCH 00/25] Fix FBC for real

[Daniel Vetter]

On Wed, Jun 18, 2014 at 08:58:33PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> This series rewrites the FBC code to actually work. It utilizes the
> hardware tracking/nuking as much as possible, eg. relying on hardware
> nuke on flip when possible.
> 
> I also introduce the generic ring and vblank notifier gizmos which could
> be used for various other things. I already included a patch to convert
> the IPS enable to be asynchronous by using the vblank notifier. Other
> users for thse could be mmio flips, watermark programming, atomic
> gamma/color correction (single buffered registers all) updates from vblank
> interrupt, etc.
> 
> There's also a rather big behavioural change that FBC now stays enabled
> even when multiple pipes are active. FBC just automatially migrates to
> the primary plane where it's deemed most beneficial. We do that
> determination by looking at the rate at which the plane is pulling data
> (pixel rate * cpp). That seems like a reasonable choice all else being
> equal. Eventually we might want to adjust the FBC score based on
> nuke/invalidate frequency as well.
> 
> The locking now has a new fbc.mutex. I had to split the page flip code
> apart a bit to accomondate. I'm not entirely sure if it wouldn't be
> better to just keep struct_mutex locked all through there, but then
> I'd need rework the locking in the fbc code to not take struct_mutex
> when called from the page flip code. And then I'd have to start
> questioning whether fbc.mutex has any point existing.
> 
> I pushed the lot here:
> git://gitorious.org/vsyrjala/linux.git fbc_update_thing_14

Just very cursory look at it, but this conflicts badly (well the
integration with the driver) with my frontbuffer tracking stuff. My idea
was to essentially hide all the fbc integration points we have sprinkled
all over behind that one and maybe even switch to manual invalidation for
gt rendering with a cpu register write and ditch all the complexity we
have with injecting cmds into rings right now.

Other integration points would be an fbc enable called from crtc_enable to
check the basics (like screen size to allocate the fbc backing buffers).
And then an additional function called every time we update the primary
buffer to check for e.g. pixel depth and stuff like that. Post-flip
updates would exclusively happen through the frontbuffer tracking scheme.

Just dumping my high-level thoughts.
-Daniel

> 
> Ville Syrjälä (25):
>   drm/i915: Add ring_notify mechanism
>   drm/i915: Add vblank notify mechanism
>   drm/i915: Name the IPS bits
>   drm/i915: Use vblank notifier for IPS
>   drm/i915: Reogranize page flip code for fbc
>   drm/i915: Move ilk_pipe_pixel_rate() earlier to avoid forward
>     declaration
>   drm/i915: Reorganize intel_update_fbc()
>   drm/i915: Check panel fitting state before enabling fbc
>   drm/i915: Reject fbc on g4x when sprites are enabled
>   drm/i915: Check pixel format for fbc
>   drm/i915: Remove dblscan flag from fbc1 check
>   drm/i915: Don't claim fbc as possible if the obj size exceeds stolen
>     size
>   drm/i915: Use low level funciton to disable fbc at init/resume
>   drm/i915: Move fbc function prototypes got intel_drv.h
>   drm/i915: Move fbc state into dev_priv.fbc
>   drm/i915: Rewrite fbc
>   drm/i915: Reduce dmesg spam from FBC enable
>   drm/i915: Add i915_fbc_info debugfs file
>   drm/i915: Implement LRI based FBC tracking
>   drm/i915: Use LRI based FBC render tracking for ILK
>   drm/i915: Reorder i915_gem_execbuffer_move_to_gpu() and
>     i915_switch_context()
>   drm/i915: Flush caches for scanout during cpu->gtt move
>   drm/i915: Nuke FBC from SW_FINISH ioctl
>   drm/i915: Pimp fbc render/blitter tracking
>   drm/i915: Enable fbc for ilk+ by default
> 
>  drivers/gpu/drm/i915/i915_debugfs.c        |   60 +-
>  drivers/gpu/drm/i915/i915_drv.c            |    3 +
>  drivers/gpu/drm/i915/i915_drv.h            |   44 +-
>  drivers/gpu/drm/i915/i915_gem.c            |   23 +-
>  drivers/gpu/drm/i915/i915_gem_context.c    |   14 +
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   40 +-
>  drivers/gpu/drm/i915/i915_irq.c            |   12 +
>  drivers/gpu/drm/i915/i915_reg.h            |    3 +-
>  drivers/gpu/drm/i915/i915_suspend.c        |    5 +-
>  drivers/gpu/drm/i915/intel_display.c       |  399 ++++++++--
>  drivers/gpu/drm/i915/intel_drv.h           |   30 +-
>  drivers/gpu/drm/i915/intel_pm.c            | 1099 +++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_ringbuffer.c    |  154 +++-
>  drivers/gpu/drm/i915/intel_ringbuffer.h    |   26 +-
>  drivers/gpu/drm/i915/intel_sprite.c        |   15 +-
>  15 files changed, 1483 insertions(+), 444 deletions(-)
> 
> -- 
> 1.8.5.5
> 
> _______________________________________________
> 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

Re: [PATCH 00/25] Fix FBC for real

[Chris Wilson]

On Wed, Jun 18, 2014 at 10:03:27PM +0200, Daniel Vetter wrote:
> Just very cursory look at it, but this conflicts badly (well the
> integration with the driver) with my frontbuffer tracking stuff. My idea
> was to essentially hide all the fbc integration points we have sprinkled
> all over behind that one and maybe even switch to manual invalidation for
> gt rendering with a cpu register write and ditch all the complexity we
> have with injecting cmds into rings right now.

I'm inclined to agree. I think it would be (on the GEM side) much
simpler and be a much better base to build upon.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 00/25] Fix FBC for real

[Ville Syrjälä]

On Wed, Jun 18, 2014 at 10:03:27PM +0200, Daniel Vetter wrote:
> On Wed, Jun 18, 2014 at 08:58:33PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > This series rewrites the FBC code to actually work. It utilizes the
> > hardware tracking/nuking as much as possible, eg. relying on hardware
> > nuke on flip when possible.
> > 
> > I also introduce the generic ring and vblank notifier gizmos which could
> > be used for various other things. I already included a patch to convert
> > the IPS enable to be asynchronous by using the vblank notifier. Other
> > users for thse could be mmio flips, watermark programming, atomic
> > gamma/color correction (single buffered registers all) updates from vblank
> > interrupt, etc.
> > 
> > There's also a rather big behavioural change that FBC now stays enabled
> > even when multiple pipes are active. FBC just automatially migrates to
> > the primary plane where it's deemed most beneficial. We do that
> > determination by looking at the rate at which the plane is pulling data
> > (pixel rate * cpp). That seems like a reasonable choice all else being
> > equal. Eventually we might want to adjust the FBC score based on
> > nuke/invalidate frequency as well.
> > 
> > The locking now has a new fbc.mutex. I had to split the page flip code
> > apart a bit to accomondate. I'm not entirely sure if it wouldn't be
> > better to just keep struct_mutex locked all through there, but then
> > I'd need rework the locking in the fbc code to not take struct_mutex
> > when called from the page flip code. And then I'd have to start
> > questioning whether fbc.mutex has any point existing.
> > 
> > I pushed the lot here:
> > git://gitorious.org/vsyrjala/linux.git fbc_update_thing_14
> 
> Just very cursory look at it, but this conflicts badly (well the
> integration with the driver) with my frontbuffer tracking stuff. My idea
> was to essentially hide all the fbc integration points we have sprinkled
> all over behind that one and maybe even switch to manual invalidation for
> gt rendering with a cpu register write and ditch all the complexity we
> have with injecting cmds into rings right now.

Well yeah if you want to kill the hardware tracking entirely the end
result ought to have less code, especially since psr needs the software
tracking anyway. Although on certain (older) platforms you can't really
turn off the hardware tracking, so the software tracking is somewhat
pointless.

Anyways, I think you're going to have to find another sucker to do that
work if you want to go that way. I don't think I'll have time for major
redesigns until maybe next year.

-- 
Ville Syrjälä
Intel OTC