Re: [PATCH 09/13] drm/i915/bdw: poll semaphores

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Intel GFX <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 09/13] drm/i915/bdw: poll semaphores
Date: Wed, 30 Apr 2014 13:53:24 +0300	[thread overview]
Message-ID: <20140430105324.GH18465@intel.com> (raw)
In-Reply-To: <1398808360-3674-10-git-send-email-benjamin.widawsky@intel.com>

On Tue, Apr 29, 2014 at 02:52:36PM -0700, Ben Widawsky wrote:
> As Ville points out, it's possible/probable we don't actually need this.
> Potentially, this validates the letter of the spec, and not the spirit.
> 
> Ville:
> > I discussed this on irc w/ Ben, and I was suggesting we don't need to
> > poll. Polling apparently can be used as a workaround for certain
> > hardware issues, but it looks like those issues shouldn't affect us,
> > for the momemnt at least. So my suggestion was to try w/o polling
> > first (since there could be some power cost to polling) and add the
> > poll bit if problems arise.

I had another look at bspec and there seems to be a bit more text about
the signal mode stuff since the last time I looked. But it still looks
like signal mode should be OK for production hardware.

One of the workarounds says that !RCS rings can lose the PPGTT
page directory information when becoming idle while waiting for a
semaphore. The workaround is either reprogamming the PPGTT
after semaphore wait or disabling the idle message. We already
disable the idle message for RCS for some other reason but not
for the other rings. Doing that for all rings seems like the easier
option here. This seems to apply to production hardware.

There's another idle message disable w/a (using another bit which
maybe disables more rc states?) but that seems to affect only
pre-production hardware. Here the other option here is to use
polling instead.

> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 31b1f3c..e7748ef 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -842,6 +842,7 @@ gen8_ring_sync(struct intel_ring_buffer *waiter,
>  
>  	intel_ring_emit(waiter, MI_SEMAPHORE_WAIT |
>  				MI_SEMAPHORE_GLOBAL_GTT |
> +				MI_SEMAPHORE_POLL |
>  				MI_SEMAPHORE_SAD_GTE_SDD);
>  	intel_ring_emit(waiter, seqno);
>  	intel_ring_emit(waiter,
> -- 
> 1.9.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC