On Fri, Sep 04, 2015 at 04:53:28PM +0200, Daniel Vetter wrote: > On Fri, Sep 04, 2015 at 03:18:14PM +0300, Mika Kuoppala wrote: > > Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> writes: > > > > > Daniel Vetter <daniel@xxxxxxxx> writes: > > > > > >> On Fri, Sep 04, 2015 at 11:40:26AM +0300, Mika Kuoppala wrote: > > >>> Daniel Vetter <daniel@xxxxxxxx> writes: > > >>> > > >>> > On Thu, Sep 03, 2015 at 04:51:45PM -0300, Paulo Zanoni wrote: > > >>> >> From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > >>> >> > > >>> >> Delay the expensive read on the FPGA_DBG register from once per mmio to > > >>> >> once per forcewake section when we are doing the general wellbeing > > >>> >> check rather than the targetted error detection. This almost reduces > > >>> >> the overhead of the debug facility (for example when submitting execlists) > > >>> >> to zero whilst keeping the debug checks around. > > >>> >> > > >>> >> v2: Enable one-shot mmio debugging from the interrupt check as well as a > > >>> >> safeguard to catch invalid display writes from outside the powerwell. > > >>> >> v3 (from Paulo): rebase since gen9 addition and intel_uncore_check_errors > > >>> >> removal > > >>> >> > > >>> >> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > >>> >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > >>> >> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> > > >>> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > > >>> > > > >>> > I'm unclear how this interacts (or how it sould interact) with patch 2: > > >>> > Forcwake is mostly for GT registers, but patch 2 also tries to optimize > > >>> > forcwake for GT registers. Do we really need both? > > >>> > > >>> Assuming the hardware detects access to unpowered domains and > > >>> to unregistered ranges by setting this bit, I would say that patch 2 > > >>> is not needed. One could argue that patch 2 is somewhat harmful as > > >>> current register access pattern affects the detection. > > >>> > > >>> Also the commit message in patch 2 is not valid wrt the code. > > >>> > > >>> With skl, the debug bit seems to decay with time, instead of being > > >>> sticky. So in there we could argue that in patch 4/4, the reading > > >>> should be done before (and after) the forcewake scope. > > >> > > >> Do we know where the bits decay? Could it be that the firmware (dmc) does > > >> something with it, or maybe it gets reset when we change display power > > >> wells? > > > > > > Now when trying to actually measure the decay time, I can't reproduce > > > the same behaviour anymore. Now it is sticky. Up until the display > > > power off clears the register without explicit clearing write. > > > > > > Now I just wonder why I saw decay in just less than millisecond, > > > few weeks back. I am willing to blame my imperfect test setup on this, > > > and something just cleared it behind my back. > > > > > > > Well, apparently this is in display power well, like Daniel > > noted. Which means that if display is off, the write won't > > stick. (but stays in some pci write buffer for 0.5usecs? > > until it vanishes, thus the decay) > > > > Test setup was otherwise water proof, but if one does > > skl debugging from console and bdw debugging through > > ssh, the display power well states are quite different... > > Hm, doesn't that mean that we can't do delayed checking since what we > really want is spot unclaimed register writes when the power well is off? > At least it was really useful to catch the oddball runtime pm management > bug. So perhaps we indeed want the range-based filter and not this patch > here? Would it also make sense to enable/disable the checks from the power well enable/disable hooks, so that we would minimize the overhead when the power well is actually enabled? -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
