[Intel-gfx] [PATCH v5 09/35] drm/i915: Force MMIO flips when scheduler enabled

[Lankhorst, Maarten]

Hey,


Jesse Barnes schreef op vr 19-02-2016 om 12:01 [-0800]:
> On 02/19/2016 11:53 AM, Ville Syrjälä wrote:
> > On Fri, Feb 19, 2016 at 11:28:05AM -0800, Jesse Barnes wrote:
> >> On 02/18/2016 06:26 AM, John.C.Harrison at Intel.com wrote:
> >>> From: John Harrison <John.C.Harrison at Intel.com>
> >>>
> >>> MMIO flips are the preferred mechanism now but more importantly, pipe
> >>> based flips cause issues for the scheduler. Specifically, submitting
> >>> work to the rings around the side of the scheduler could cause that
> >>> work to be lost if the scheduler generates a pre-emption event on that
> >>> ring.
> >>>
> >>> For: VIZ-1587
> >>> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/intel_display.c | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>> index 6e12ed7..731d20a 100644
> >>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> @@ -46,6 +46,7 @@
> >>>  #include <linux/dma_remapping.h>
> >>>  #include <linux/reservation.h>
> >>>  #include <linux/dma-buf.h>
> >>> +#include "i915_scheduler.h"
> >>>  
> >>>  /* Primary plane formats for gen <= 3 */
> >>>  static const uint32_t i8xx_primary_formats[] = {
> >>> @@ -11330,6 +11331,8 @@ static bool use_mmio_flip(struct intel_engine_cs *ring,
> >>>  		return true;
> >>>  	else if (i915.enable_execlists)
> >>>  		return true;
> >>> +	else if (i915_scheduler_is_enabled(ring->dev))
> >>> +		return true;
> >>>  	else if (obj->base.dma_buf &&
> >>>  		 !reservation_object_test_signaled_rcu(obj->base.dma_buf->resv,
> >>>  						       false))
> >>>
> >>
> >> Why can't we use mmio flips unconditionally?  Maarten or Ville?
> > 
> > We do when execlists are used, which is always on gen9+. So I guess I'm
> > missing the point of this patch. For gen5+ we could also do it trivially.
> 
> didn't check if the scheduler is also enabled for gen8 (I guess it would be nice, that would cover BDW and BSW).

I have a patch that would enable support for mmio flips on all
platforms, but because of Chris Wilson's objections I still didn't force
mmio flips by default.

> > 
> > For older platforms it'd require a bit of work since we'd need to
> > complete the flips from the vblank interrupt. Well, we actually do
> > that already even with CS flips on those platforms, but we do look
> > at the flip pending interrupt to figure out if CS already issued
> > the flip or not. So that part would need changing.
It was easy to fix in a way similar to that.

> > I also think we should switch to using the vblank interrupt for this
> > stuff on all platforms, mainly since the flip done interrupt is somewhat
> > broken on at least BDW (no idea if it got fixed in SKL or later), and
> > doing things in more than one way certainly won't decrease our bug count.
> 
> Yeah that's probably the way to go; I haven't checked the behavior on SKL either.

---------------------------------------------------------------------
Intel International B.V.
Registered in The Netherlands under number 34098535
Statutory seat: Haarlemmermeer
Registered address: Capronilaan 37, 1119NG Schiphol-Rijk

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.