| Message ID | 1307646877-1979-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 14 Jun 2011 00:14:11 -0700, Kenneth Graunke <kenneth@whitecape.org> wrote: > On 06/09/2011 12:14 PM, Jesse Barnes wrote: > > Found a couple more problems: > > 1) MI_DISPLAY_FLIP should take a '1' in the last byte to indicate > > length on IVB > > 2) apprently only the BLT ring version of the command actually causes > > interrupts to be generated > > > > With this patch, modetest -v works on my test platform. Clearly it's in > > need of more splitting though. The case statement should be split into > > per-chipset flip command generation. > > > > Signed-off-by: Jesse Barnes<jbarnes@virtuousgeek.org> > > Reviewed-and-tested-by: Kenneth Graunke <kenneth@whitecape.org> But it's not the neatest patch in the world ;-) Using struct intel_ring_buffer *ring for all generation and switching between blt/render on demand should be neater. Also we should be able to do pageflips from either the render or blt ring on gen7, right? That will be useful to avoid a synchronisation point. -Chris
On Tue, 14 Jun 2011 00:29:08 -0700 Kenneth Graunke <kenneth@whitecape.org> wrote: > On 06/13/2011 03:22 PM, Chris Wilson wrote: > > On Tue, 14 Jun 2011 00:14:11 -0700, Kenneth Graunke<kenneth@whitecape.org> wrote: > >> On 06/09/2011 12:14 PM, Jesse Barnes wrote: > >>> Found a couple more problems: > >>> 1) MI_DISPLAY_FLIP should take a '1' in the last byte to indicate > >>> length on IVB > >>> 2) apprently only the BLT ring version of the command actually causes > >>> interrupts to be generated > >>> > >>> With this patch, modetest -v works on my test platform. Clearly it's in > >>> need of more splitting though. The case statement should be split into > >>> per-chipset flip command generation. > >>> > >>> Signed-off-by: Jesse Barnes<jbarnes@virtuousgeek.org> > >> > >> Reviewed-and-tested-by: Kenneth Graunke<kenneth@whitecape.org> > > > > But it's not the neatest patch in the world ;-) > > True :) > > > Using struct intel_ring_buffer *ring for all generation and switching > > between blt/render on demand should be neater. Also we should be able > > to do pageflips from either the render or blt ring on gen7, right? That > > will be useful to avoid a synchronisation point. > > -Chris > > I thought so, from reading the docs. But it doesn't seem to work from > the render ring... Yeah, it's just an ugly hack. So far the hw guys are confused why the render ring doesn't work and the blit ring does, all else being equal. I'll clean up the patch and re-post, and follow up with them and see if we can root cause this issue either way. Thanks,
On 06/13/2011 03:22 PM, Chris Wilson wrote: > On Tue, 14 Jun 2011 00:14:11 -0700, Kenneth Graunke<kenneth@whitecape.org> wrote: >> On 06/09/2011 12:14 PM, Jesse Barnes wrote: >>> Found a couple more problems: >>> 1) MI_DISPLAY_FLIP should take a '1' in the last byte to indicate >>> length on IVB >>> 2) apprently only the BLT ring version of the command actually causes >>> interrupts to be generated >>> >>> With this patch, modetest -v works on my test platform. Clearly it's in >>> need of more splitting though. The case statement should be split into >>> per-chipset flip command generation. >>> >>> Signed-off-by: Jesse Barnes<jbarnes@virtuousgeek.org> >> >> Reviewed-and-tested-by: Kenneth Graunke<kenneth@whitecape.org> > > But it's not the neatest patch in the world ;-) True :) > Using struct intel_ring_buffer *ring for all generation and switching > between blt/render on demand should be neater. Also we should be able > to do pageflips from either the render or blt ring on gen7, right? That > will be useful to avoid a synchronisation point. > -Chris I thought so, from reading the docs. But it doesn't seem to work from the render ring... --Kenneth
On Mon, 13 Jun 2011 16:00:53 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Yeah, it's just an ugly hack.
Let's also put a default: return -ENODEV in there for future sanity.
-Chris
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 81a9059..37d0087 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6343,9 +6343,11 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, /* Offset into the new buffer for cases of shared fbs between CRTCs */ offset = crtc->y * fb->pitch + crtc->x * fb->bits_per_pixel/8; - ret = BEGIN_LP_RING(4); - if (ret) - goto cleanup_objs; + if (!IS_IVYBRIDGE(dev)) { + ret = BEGIN_LP_RING(4); + if (ret) + goto cleanup_objs; + } /* Block clients from rendering to the new back buffer until * the flip occurs and the object is no longer visible. @@ -6390,7 +6392,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, break; case 6: - case 7: OUT_RING(MI_DISPLAY_FLIP | MI_DISPLAY_FLIP_PLANE(intel_crtc->plane)); OUT_RING(fb->pitch | obj->tiling_mode); @@ -6400,8 +6401,18 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, pipesrc = I915_READ(PIPESRC(pipe)) & 0x0fff0fff; OUT_RING(pf | pipesrc); break; + case 7: + intel_ring_begin(&dev_priv->ring[BCS], 4); + intel_ring_emit(&dev_priv->ring[BCS], (MI_DISPLAY_FLIP_I915 | (intel_crtc->plane << 19))); + intel_ring_emit(&dev_priv->ring[BCS], (fb->pitch | obj->tiling_mode)); + intel_ring_emit(&dev_priv->ring[BCS], (obj->gtt_offset)); + intel_ring_emit(&dev_priv->ring[BCS], (MI_NOOP)); + intel_ring_advance(&dev_priv->ring[BCS]); + break; } - ADVANCE_LP_RING(); + + if (!IS_IVYBRIDGE(dev)) + ADVANCE_LP_RING(); mutex_unlock(&dev->struct_mutex);
Found a couple more problems: 1) MI_DISPLAY_FLIP should take a '1' in the last byte to indicate length on IVB 2) apprently only the BLT ring version of the command actually causes interrupts to be generated With this patch, modetest -v works on my test platform. Clearly it's in need of more splitting though. The case statement should be split into per-chipset flip command generation. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/intel_display.c | 21 ++++++++++++++++----- 1 files changed, 16 insertions(+), 5 deletions(-)