* [PATCH 0/2] drm/i915: IVB MI_DISPLAY_FLIP cacheline trick v2
@ 2014-02-11 17:52 ville.syrjala
2014-02-11 17:52 ` [PATCH 1/2] drm/i915: Add intel_ring_cachline_align() ville.syrjala
2014-02-11 17:52 ` [PATCH v3 2/2] drm/i915: Prevent MI_DISPLAY_FLIP straddling two cachelines on IVB ville.syrjala
0 siblings, 2 replies; 6+ messages in thread
From: ville.syrjala @ 2014-02-11 17:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Enrico Tagliavini, Bjoern C, Alexandru DAMIAN
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
BSpec tells us that the entire MI_DISPLAY_FLIP packet must be contained
within a single cacheline on IVB. This series achieves that.
Changes since my original patch [1]:
* Move the logic into a new intel_ring_begin_cacheline_safe() function
(as suggested by Daniel).
* Actually handle the case when the ring would wrap due to the extra
dwords. With the original patch, the MI_DISPLAY_FLIP packet might still
end up straddling two cachelines in this case.
Changes since v1 [2]:
* Just do the simple cacheline alignment trick Chris suggested
[1] https://bugs.freedesktop.org/show_bug.cgi?id=74053#
[2] http://lists.freedesktop.org/archives/intel-gfx/2014-February/039921.html
Ville Syrjälä (2):
drm/i915: Add intel_ring_cachline_align()
drm/i915: Prevent MI_DISPLAY_FLIP straddling two cachelines on IVB
drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++
drivers/gpu/drm/i915/intel_ringbuffer.c | 21 +++++++++++++++++++++
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
3 files changed, 36 insertions(+)
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] drm/i915: Add intel_ring_cachline_align()
2014-02-11 17:52 [PATCH 0/2] drm/i915: IVB MI_DISPLAY_FLIP cacheline trick v2 ville.syrjala
@ 2014-02-11 17:52 ` ville.syrjala
2014-02-11 17:52 ` [PATCH v3 2/2] drm/i915: Prevent MI_DISPLAY_FLIP straddling two cachelines on IVB ville.syrjala
1 sibling, 0 replies; 6+ messages in thread
From: ville.syrjala @ 2014-02-11 17:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Enrico Tagliavini, Bjoern C, Alexandru DAMIAN
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
intel_ring_cachline_align() emits MI_NOOPs until the ring tail is
aligned to a cacheline boundary.
Cc: Bjoern C <lkml@call-home.ch>
Cc: Alexandru DAMIAN <alexandru.damian@intel.com>
Cc: Enrico Tagliavini <enrico.tagliavini@gmail.com>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 21 +++++++++++++++++++++
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
2 files changed, 22 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ba686d7..8c1c0bc 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1638,6 +1638,27 @@ int intel_ring_begin(struct intel_ring_buffer *ring,
return 0;
}
+/* Align the ring tail to a cacheline boundary */
+int intel_ring_cacheline_align(struct intel_ring_buffer *ring)
+{
+ int num_dwords = (64 - (ring->tail & 63)) / sizeof(uint32_t);
+ int ret;
+
+ if (num_dwords == 0)
+ return 0;
+
+ ret = intel_ring_begin(ring, num_dwords);
+ if (ret)
+ return ret;
+
+ while (num_dwords--)
+ intel_ring_emit(ring, MI_NOOP);
+
+ intel_ring_advance(ring);
+
+ return 0;
+}
+
void intel_ring_init_seqno(struct intel_ring_buffer *ring, u32 seqno)
{
struct drm_i915_private *dev_priv = ring->dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 38c757e..08b91c6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -235,6 +235,7 @@ intel_write_status_page(struct intel_ring_buffer *ring,
void intel_cleanup_ring_buffer(struct intel_ring_buffer *ring);
int __must_check intel_ring_begin(struct intel_ring_buffer *ring, int n);
+int __must_check intel_ring_cacheline_align(struct intel_ring_buffer *ring);
static inline void intel_ring_emit(struct intel_ring_buffer *ring,
u32 data)
{
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v3 2/2] drm/i915: Prevent MI_DISPLAY_FLIP straddling two cachelines on IVB
2014-02-11 17:52 [PATCH 0/2] drm/i915: IVB MI_DISPLAY_FLIP cacheline trick v2 ville.syrjala
2014-02-11 17:52 ` [PATCH 1/2] drm/i915: Add intel_ring_cachline_align() ville.syrjala
@ 2014-02-11 17:52 ` ville.syrjala
2014-02-11 20:19 ` Chris Wilson
1 sibling, 1 reply; 6+ messages in thread
From: ville.syrjala @ 2014-02-11 17:52 UTC (permalink / raw)
To: intel-gfx; +Cc: Enrico Tagliavini, Bjoern C, Alexandru DAMIAN
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
According to BSpec the entire MI_DISPLAY_FLIP packet must be contained
in a single cacheline. Make sure that happens.
v2: Use intel_ring_begin_cacheline_safe()
v3: Use intel_ring_cacheline_align() (Chris)
Cc: Bjoern C <lkml@call-home.ch>
Cc: Alexandru DAMIAN <alexandru.damian@intel.com>
Cc: Enrico Tagliavini <enrico.tagliavini@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74053
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0c25310..37527d4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8609,6 +8609,20 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
if (ring->id == RCS)
len += 6;
+ /*
+ * BSpec MI_DISPLAY_FLIP for IVB:
+ * "The full packet must be contained within the same cache line."
+ *
+ * Currently the LRI+SRM+MI_DISPLAY_FLIP all fit within the same
+ * cacheline, if we ever start emitting more commands before
+ * the MI_DISPLAY_FLIP we may need to first emit everything else,
+ * then do the cacheline alignment, and finally emit the
+ * MI_DISPLAY_FLIP.
+ */
+ ret = intel_ring_cacheline_align(ring);
+ if (ret)
+ goto err_unpin;
+
ret = intel_ring_begin(ring, len);
if (ret)
goto err_unpin;
--
1.8.3.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] drm/i915: Prevent MI_DISPLAY_FLIP straddling two cachelines on IVB
2014-02-11 17:52 ` [PATCH v3 2/2] drm/i915: Prevent MI_DISPLAY_FLIP straddling two cachelines on IVB ville.syrjala
@ 2014-02-11 20:19 ` Chris Wilson
2014-02-11 20:34 ` Ville Syrjälä
2014-02-11 22:01 ` Daniel Vetter
0 siblings, 2 replies; 6+ messages in thread
From: Chris Wilson @ 2014-02-11 20:19 UTC (permalink / raw)
To: ville.syrjala; +Cc: Enrico Tagliavini, intel-gfx, Alexandru DAMIAN, Bjoern C
On Tue, Feb 11, 2014 at 07:52:06PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> According to BSpec the entire MI_DISPLAY_FLIP packet must be contained
> in a single cacheline. Make sure that happens.
>
> v2: Use intel_ring_begin_cacheline_safe()
> v3: Use intel_ring_cacheline_align() (Chris)
>
> Cc: Bjoern C <lkml@call-home.ch>
> Cc: Alexandru DAMIAN <alexandru.damian@intel.com>
> Cc: Enrico Tagliavini <enrico.tagliavini@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74053
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
I would have used intel_ring_align_cacheline() as it seems more natural
for me to say...
Both,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] drm/i915: Prevent MI_DISPLAY_FLIP straddling two cachelines on IVB
2014-02-11 20:19 ` Chris Wilson
@ 2014-02-11 20:34 ` Ville Syrjälä
2014-02-11 22:01 ` Daniel Vetter
1 sibling, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2014-02-11 20:34 UTC (permalink / raw)
To: Chris Wilson, intel-gfx, Enrico Tagliavini, Bjoern C,
Alexandru DAMIAN
On Tue, Feb 11, 2014 at 08:19:13PM +0000, Chris Wilson wrote:
> On Tue, Feb 11, 2014 at 07:52:06PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > According to BSpec the entire MI_DISPLAY_FLIP packet must be contained
> > in a single cacheline. Make sure that happens.
> >
> > v2: Use intel_ring_begin_cacheline_safe()
> > v3: Use intel_ring_cacheline_align() (Chris)
> >
> > Cc: Bjoern C <lkml@call-home.ch>
> > Cc: Alexandru DAMIAN <alexandru.damian@intel.com>
> > Cc: Enrico Tagliavini <enrico.tagliavini@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74053
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> I would have used intel_ring_align_cacheline() as it seems more natural
> for me to say...
I had it as intel_ring_align_to_cacheline(), but then changed it when I
saw Daniel's suggestion for the name. So you can blame Daniel for this ;)
But I have no emotional attachment to the chosen name. So if you prefer
we rename it, I'm fine with that.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] drm/i915: Prevent MI_DISPLAY_FLIP straddling two cachelines on IVB
2014-02-11 20:19 ` Chris Wilson
2014-02-11 20:34 ` Ville Syrjälä
@ 2014-02-11 22:01 ` Daniel Vetter
1 sibling, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2014-02-11 22:01 UTC (permalink / raw)
To: Chris Wilson, ville.syrjala, intel-gfx, Enrico Tagliavini,
Bjoern C, Alexandru DAMIAN
On Tue, Feb 11, 2014 at 08:19:13PM +0000, Chris Wilson wrote:
> On Tue, Feb 11, 2014 at 07:52:06PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > According to BSpec the entire MI_DISPLAY_FLIP packet must be contained
> > in a single cacheline. Make sure that happens.
> >
> > v2: Use intel_ring_begin_cacheline_safe()
> > v3: Use intel_ring_cacheline_align() (Chris)
> >
> > Cc: Bjoern C <lkml@call-home.ch>
> > Cc: Alexandru DAMIAN <alexandru.damian@intel.com>
> > Cc: Enrico Tagliavini <enrico.tagliavini@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74053
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> I would have used intel_ring_align_cacheline() as it seems more natural
> for me to say...
>
> Both,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Both merged to -fixes with cc: stable slapped on them.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-11 22:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-11 17:52 [PATCH 0/2] drm/i915: IVB MI_DISPLAY_FLIP cacheline trick v2 ville.syrjala
2014-02-11 17:52 ` [PATCH 1/2] drm/i915: Add intel_ring_cachline_align() ville.syrjala
2014-02-11 17:52 ` [PATCH v3 2/2] drm/i915: Prevent MI_DISPLAY_FLIP straddling two cachelines on IVB ville.syrjala
2014-02-11 20:19 ` Chris Wilson
2014-02-11 20:34 ` Ville Syrjälä
2014-02-11 22:01 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.