On Wed, Jan 28, 2015 at 05:54:14PM +0200, Mika Kuoppala wrote: > Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> writes: > > > On Wed, Jan 28, 2015 at 03:28:56PM +0200, Mika Kuoppala wrote: > >> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > >> > >> > On Wed, Jan 28, 2015 at 02:43:25PM +0200, Mika Kuoppala wrote: > >> >> commit 05a2fb157e44a53c79133805d30eaada43911941 > >> >> Author: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> > >> >> Date: Mon Jan 19 16:20:43 2015 +0200 > >> >> > >> >> drm/i915: Consolidate forcewake code > >> >> > >> >> introduced domain handling where each domain has it's own posting > >> >> read registers. This changed the forcewake sequence on 'put' side when > >> >> there is multiple domains as there would be extra read between the domain > >> >> puts. Any posting read should be enough to flush all the changes. > >> >> > >> >> Do a posting read only once, at the end of the sequence and for > >> >> the first domain. Like it was before. > >> >> > >> >> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > >> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> > >> > > >> > fwiw, I would argue that the posting read in _get() is superfluous as we > >> > will serialise the fw with not only the ack, but any subsequent mmio. > >> > > >> > On the _put() side we do want to flush the write so that the hw can > >> > power down as early as possible. So just kill the posting read from _get > >> > and otherwise drop the patch. :) > >> > >> Yes, both put/get patches should be dropped. I posted a patch removing > >> the posting read on get side and with your explanations in commit message. > >> > >> This all starts to make so much sense that some gen is bound to break ;) > > > > IIRC the posting read from same cache line actually fixed real bugs. So > > I'm a bit worried about dropping them. But I suppose it's possible only > > the _put side was important for those bugs. > > I found these: > > commit 6af2d180f82151cf3d58952e35a4f96e45bc453a > Author: Daniel Vetter <daniel.vetter@xxxxxxxx> > Date: Thu Jul 26 16:24:50 2012 +0200 > > drm/i915: fix forcewake related hangs on snb > > commit 8dee3eea3ccd3b6c00a8d3a08dd715d6adf737dd > Author: Ben Widawsky <ben@xxxxxxxxxxxx> > Date: Sat Sep 1 22:59:50 2012 -0700 > > drm/i915: Never read FORCEWAKE > > https://bugs.freedesktop.org/show_bug.cgi?id=51738 > https://bugs.freedesktop.org/show_bug.cgi?id=52424 > > The snb here seems to survive gem_dummy_reloc_loop and > gem_ring_sync_loop in here with the get side posting removed. Note that we kept the once associated with #52424, but judging by my comments in #51738 the posting read is just a band aid anyway as a full mb() itself was not adequate. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
