[Intel-gfx] [PATCH] drm/i915: setup workarounds on reset

[Paulo Zanoni]

Hi

2013/11/20 Mika Kuoppala <mika.kuoppala at linux.intel.com>:
> Daniel Vetter <daniel at 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 at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni