On Fri, May 16, 2014 at 12:34:08PM +0000, Gupta, Sourab wrote:
> On Thu, 2014-05-15 at 12:27 +0000, Ville Syrjälä wrote:
> > On Thu, May 15, 2014 at 11:47:37AM +0530, sourab.gupta@xxxxxxxxx wrote:
> > > From: Sourab Gupta <sourab.gupta@xxxxxxxxx>
> > >
> > > Using MMIO based flips on Gen5+ for Media power well residency optimization.
> > > The blitter ring is currently being used just for command streamer based
> > > flip calls. For pure 3D workloads, with MMIO flips, there will be no use
> > > of blitter ring and this will ensure the 100% residency for Media well.
> > >
> > > In a subsequent patch, we can make the selection between CS vs MMIO flip
> > > based on a module parameter to give more testing coverage.
> > >
> > > v2: The MMIO flips now use the interrupt driven mechanism for issuing the
> > > flips when target seqno is reached. (Incorporating Ville's idea)
> > >
> > > v3: Rebasing on latest code. Code restructuring after incorporating
> > > Damien's comments
> > >
> > > v4: Addressing Ville's review comments
> > > -general cleanup
> > > -updating only base addr instead of calling update_primary_plane
> > > -extending patch for gen5+ platforms
> > >
> > > Signed-off-by: Sourab Gupta <sourab.gupta@xxxxxxxxx>
> > > Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/i915_dma.c | 1 +
> > > drivers/gpu/drm/i915/i915_drv.h | 6 ++
> > > drivers/gpu/drm/i915/i915_gem.c | 2 +-
> > > drivers/gpu/drm/i915/i915_irq.c | 2 +
> > > drivers/gpu/drm/i915/intel_display.c | 115 +++++++++++++++++++++++++++++++++++
> > > drivers/gpu/drm/i915/intel_drv.h | 6 ++
> > > 6 files changed, 131 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > > index 46f1dec..672c28f 100644
> > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > @@ -1570,6 +1570,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> > > spin_lock_init(&dev_priv->backlight_lock);
> > > spin_lock_init(&dev_priv->uncore.lock);
> > > spin_lock_init(&dev_priv->mm.object_stat_lock);
> > > + spin_lock_init(&dev_priv->mmio_flip_lock);
> > > dev_priv->ring_index = 0;
> > > mutex_init(&dev_priv->dpio_lock);
> > > mutex_init(&dev_priv->modeset_restore_lock);
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 4006dfe..38c0820 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1543,6 +1543,8 @@ struct drm_i915_private {
> > > struct i915_ums_state ums;
> > > /* the indicator for dispatch video commands on two BSD rings */
> > > int ring_index;
> > > + /* protects the mmio flip data */
> > > + spinlock_t mmio_flip_lock;
> > > };
> > >
> > > static inline struct drm_i915_private *to_i915(const struct drm_device *dev)
> > > @@ -2209,6 +2211,7 @@ bool i915_gem_retire_requests(struct drm_device *dev);
> > > void i915_gem_retire_requests_ring(struct intel_ring_buffer *ring);
> > > int __must_check i915_gem_check_wedge(struct i915_gpu_error *error,
> > > bool interruptible);
> > > +int i915_gem_check_olr(struct intel_ring_buffer *ring, u32 seqno);
> > > static inline bool i915_reset_in_progress(struct i915_gpu_error *error)
> > > {
> > > return unlikely(atomic_read(&error->reset_counter)
> > > @@ -2580,6 +2583,9 @@ int i915_reg_read_ioctl(struct drm_device *dev, void *data,
> > > int i915_get_reset_stats_ioctl(struct drm_device *dev, void *data,
> > > struct drm_file *file);
> > >
> > > +void intel_notify_mmio_flip(struct drm_device *dev,
> > > + struct intel_ring_buffer *ring);
> > > +
> > > /* overlay */
> > > extern struct intel_overlay_error_state *intel_overlay_capture_error_state(struct drm_device *dev);
> > > extern void intel_overlay_print_error_state(struct drm_i915_error_state_buf *e,
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index fa5b5ab..5b4e953 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -975,7 +975,7 @@ i915_gem_check_wedge(struct i915_gpu_error *error,
> > > * Compare seqno against outstanding lazy request. Emit a request if they are
> > > * equal.
> > > */
> > > -static int
> > > +int
> > > i915_gem_check_olr(struct intel_ring_buffer *ring, u32 seqno)
> > > {
> > > int ret;
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > index b10fbde..a353693 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -1084,6 +1084,8 @@ static void notify_ring(struct drm_device *dev,
> > >
> > > trace_i915_gem_request_complete(ring);
> > >
> > > + intel_notify_mmio_flip(dev, ring);
> > > +
> >
> > Hmm. How badly is this going to explode with UMS?
>
> Hi Ville,
> It seems there would be a small race between the page filp done intr and
> the flip done interrupt from previous set base. But it seems to be the
> case for CS flips also. In both cases, once we do the
> mark_page_flip_active, there may be a window in which page flip intr
> from previous set base may arrive.
> Have we interpreted the race correctly? Or are we missing something
> here?
Yes. See here for my patches to fix the mmio vs. CS race:
http://lists.freedesktop.org/archives/intel-gfx/2014-April/043759.html
Feel free to review that stuff if you have a bit of time.
I've not had time to think about the mmio vs. mmio case yet. Perhaps my
patches would fix that too?
>
> Also, notify_mmio_flip is being called from notify_ring function.
> Not sure of the scenario in which it may explode with UMS. Can you
> please elaborate more.
With UMS we have no modeset structures (drm_crtcs and whatnot). So the
crtc list walk will probably explode.
Hmm. I guess we could just init all the mode_config lists even w/ UMS,
so that the code will just see an empty list. Does anyone see any
problems with that?
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
