On Fri, Oct 21, 2016 at 04:00:10PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
>
> > On Fri, Oct 21, 2016 at 02:16:46PM +0200, Arkadiusz Hiler wrote:
> >> On Wed, Aug 24, 2016 at 05:03:11PM +0100, Matthew Auld wrote:
> >> > The comment which documents the proper usage of the *_FW family of macros makes
> >> > reference to intel_uncore_forcewake_irq{unlock, lock}, which is just
> >> > confusing, seeing as such a set of functions don't even exist and never have
> >> > for that matter(according to git). Let's fix that by replacing them with
> >> > intel_uncore_forcewake_{get, put}.
> >> >
> >> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >> > Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> >> Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx>
> >
> > The downside is that this now doesn't mention the locking required to
> > prevent machine hangs on some platforms.
>
> "intel_uncore_forcewake_get will acquire forcewake reference and also
> take a uncore.lock to guarantee explicit access by one thread only. As
> some registers don't need forcewake held, intel_uncore_forcewake_{get,put}
> can be omitted. If you do so, be warned that on some gens (gen7),
> concurrent access to the same cacheline by multiple cpu threads with the gpu
> can risk a system hang. You need to grab uncore spinlock explicitly to
> guard against this."
>
> Would that be accurate addition?
intel_uncore_forcewake_get() doesn't acquire the spinlock for you, just
for itself.
The full sequence would be
spin_lock_irq(&dev_priv->uncore.lock);
intel_uncore_forcewake_get__locked()
...
intel_uncore_forcewake_put__locked()
spin_unlock_irq(&dev_priv->uncore.lock);
We very rarely do that either (a) presuming that we are serialised by
some other lock, (b) don't care because it is safe or (c) completely
forgotten about the risks.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
