| Message ID | 1384785284-1435-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Nov 18, 2013 at 04:34:44PM +0200, Mika Kuoppala wrote: > Large parts of hw initialization is behind per gen > clock gating functions. Including workarounds. > > Call intel_modeset_init_hw after reset so that we > set these up correctly. > > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index c2e00ed..2908f7f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -798,6 +798,8 @@ int i915_reset(struct drm_device *dev) > drm_irq_uninstall(dev); > drm_irq_install(dev); > intel_hpd_init(dev); > + > + intel_modeset_init_hw(dev); Currently the idea is that w/as which get nuked by the gt reset should be put into the respective ring_init function in intel_ringbuffer.c. This disdinction is important since init_clock_gating gets called fairly early in the resume/load sequence before most of the gem stuff is set up. And the w/a in the ring_init functions are carefully ordered wrt the ring (re) enabling. So which bit/register is the culprit here? -Daniel > } else { > mutex_unlock(&dev->struct_mutex); > } > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter <daniel@ffwll.ch> writes: Hi Daniel, > On Mon, Nov 18, 2013 at 04:34:44PM +0200, Mika Kuoppala wrote: >> Large parts of hw initialization is behind per gen >> clock gating functions. Including workarounds. >> >> Call intel_modeset_init_hw after reset so that we >> set these up correctly. >> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >> index c2e00ed..2908f7f 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.c >> +++ b/drivers/gpu/drm/i915/i915_drv.c >> @@ -798,6 +798,8 @@ int i915_reset(struct drm_device *dev) >> drm_irq_uninstall(dev); >> drm_irq_install(dev); >> intel_hpd_init(dev); >> + >> + intel_modeset_init_hw(dev); > > Currently the idea is that w/as which get nuked by the gt reset should be > put into the respective ring_init function in intel_ringbuffer.c. This > disdinction is important since init_clock_gating gets called fairly early > in the resume/load sequence before most of the gem stuff is set up. And > the w/a in the ring_init functions are carefully ordered wrt the ring (re) > enabling. > > So which bit/register is the culprit here? Here is output from the test on ivb, after drv_hangman: FAIL WaDisableEarlyCull:ivb OK WaDisableBackToBackFlipFix:ivb FAIL WaDisablePSDDualDispatchEnable:ivb FAIL WaDisableRHWOptimizationForRenderHang:ivb FAIL WaApplyL3ControlAndL3ChickenMode:ivb OK WaForceL3Serialization:ivb OK WaDisableRCZUnitClockGating:ivb OK WaCatErrorRejectionIssue:ivb FAIL WaVSRefCountFullforceMissDisable:ivb FAIL WaDisable4x2SubspanOptimization:ivb 10 workarounds tested, 4 passed, 6 failed Test assertion failure function main, file drv_workarounds.c:119: Failed assertion: check_workarounds(&ivb_workarounds[0], "ivb") == 0 Test can be found in here: https://github.com/mkuoppal/intel-gpu-tools/tree/was -Mika
On Wed, Nov 20, 2013 at 12:47 PM, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote: > FAIL WaDisableEarlyCull:ivb > OK WaDisableBackToBackFlipFix:ivb > FAIL WaDisablePSDDualDispatchEnable:ivb > FAIL WaDisableRHWOptimizationForRenderHang:ivb > FAIL WaApplyL3ControlAndL3ChickenMode:ivb > OK WaForceL3Serialization:ivb > OK WaDisableRCZUnitClockGating:ivb > OK WaCatErrorRejectionIssue:ivb > FAIL WaVSRefCountFullforceMissDisable:ivb > FAIL WaDisable4x2SubspanOptimization:ivb Looks like just a bunch of render related registers, so could make sense to move them. I guess the bigger question is what happens if we use per-ring reset. Do we loose all the same register settings as for a full reset or is there some fancy split? Can you please check this out with your per-ring reset patches if possible? Thanks, Daniel
Hi 2013/11/20 Mika Kuoppala <mika.kuoppala@linux.intel.com>: > Daniel Vetter <daniel@ffwll.ch> writes: > > Hi Daniel, > >> On Mon, Nov 18, 2013 at 04:34:44PM +0200, Mika Kuoppala wrote: >>> Large parts of hw initialization is behind per gen >>> clock gating functions. Including workarounds. >>> >>> Call intel_modeset_init_hw after reset so that we >>> set these up correctly. >>> >>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> >>> --- >>> drivers/gpu/drm/i915/i915_drv.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >>> index c2e00ed..2908f7f 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.c >>> +++ b/drivers/gpu/drm/i915/i915_drv.c >>> @@ -798,6 +798,8 @@ int i915_reset(struct drm_device *dev) >>> drm_irq_uninstall(dev); >>> drm_irq_install(dev); >>> intel_hpd_init(dev); >>> + >>> + intel_modeset_init_hw(dev); >> >> Currently the idea is that w/as which get nuked by the gt reset should be >> put into the respective ring_init function in intel_ringbuffer.c. This >> disdinction is important since init_clock_gating gets called fairly early >> in the resume/load sequence before most of the gem stuff is set up. And >> the w/a in the ring_init functions are carefully ordered wrt the ring (re) >> enabling. >> >> So which bit/register is the culprit here? > > Here is output from the test on ivb, after drv_hangman: > > FAIL WaDisableEarlyCull:ivb > OK WaDisableBackToBackFlipFix:ivb > FAIL WaDisablePSDDualDispatchEnable:ivb > FAIL WaDisableRHWOptimizationForRenderHang:ivb > FAIL WaApplyL3ControlAndL3ChickenMode:ivb > OK WaForceL3Serialization:ivb > OK WaDisableRCZUnitClockGating:ivb > OK WaCatErrorRejectionIssue:ivb > FAIL WaVSRefCountFullforceMissDisable:ivb > FAIL WaDisable4x2SubspanOptimization:ivb > 10 workarounds tested, 4 passed, 6 failed > Test assertion failure function main, file drv_workarounds.c:119: > Failed assertion: check_workarounds(&ivb_workarounds[0], "ivb") == 0 > > Test can be found in here: > https://github.com/mkuoppal/intel-gpu-tools/tree/was I really like the idea of having a way to test bits that should always be set to specific values, but I think that if we do this in IGT we'll always be out of sync with the Kernel. We change these WAs way too often, and I'm sure we'll always forget to keep IGT in sync. One of the things I have discussed with Daniel was to create some sort of data structure inside the Kernel where we explain which bits of which registers should always have specific values, and which function/stage should be responsible for setting the value. Then, at init_clock_gating or ring_init or other places, our code would parse this data structure and appropriately fix whatever is needed. As a bonus, we would also be able to write functions that check if the values in HW match the values on our data structure. The big benefit of this model is that with a single change in the data structure we'd guarantee that both the function that applies the WA and the function that checks the WA are in sync. And if we need to expose the WA checker to IGT, we could maybe write a debugfs file that triggers the check. That said, this was just an initial idea we discussed and I did not develop more concrete plans regarding what exactly the data structure should look like. I also have no plans to implement this soon. So if you think it's a good idea, feel free to take the idea, improve it and implement if you want. Thanks, Paulo > > -Mika > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index c2e00ed..2908f7f 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -798,6 +798,8 @@ int i915_reset(struct drm_device *dev) drm_irq_uninstall(dev); drm_irq_install(dev); intel_hpd_init(dev); + + intel_modeset_init_hw(dev); } else { mutex_unlock(&dev->struct_mutex); }
Large parts of hw initialization is behind per gen clock gating functions. Including workarounds. Call intel_modeset_init_hw after reset so that we set these up correctly. Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 2 ++ 1 file changed, 2 insertions(+)