Re: [PATCH 4/7] accel/ivpu: Gracefully shutdown NPU before reset

[Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>]

On 05.02.2024 09:39, Jacek Lawrynowicz wrote:
> On 26.01.2024 19:23, Jeffrey Hugo wrote:
>> On 1/26/2024 5:28 AM, Jacek Lawrynowicz wrote:
>>> From: "Wachowski, Karol" <karol.wachowski@intel.com>
>>>
>>> Replace forceful disable of power domains with requests to disable
>>> TOP NOC CPU_CTRL and HOSTIF_L2CACHE through QREQN.
>>>
>>> In case of failure retry multiple times following HAS sequence of
>>> checking both QACCEPN and QDENYN registers.
>>>
>>> This fixes VPU hangs with PCODE released in January 2024 onwards.
>>>
>>> Fixes: 3f7c0634926d ("accel/ivpu/37xx: Fix hangs related to MMIO reset")
>>> Signed-off-by: Wachowski, Karol <karol.wachowski@intel.com>
>>> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@linux.intel.com>
>>> ---
>>>   drivers/accel/ivpu/ivpu_hw_37xx.c | 122 +++++++++++++++---------------
>>>   1 file changed, 60 insertions(+), 62 deletions(-)
>>>

...

>>>   static void ivpu_boot_no_snoop_enable(struct ivpu_device *vdev)
>>>   {
>>>       u32 val = REGV_RD32(VPU_37XX_HOST_IF_TCU_PTW_OVERRIDES);
>>> @@ -618,19 +617,18 @@ static int ivpu_hw_37xx_info_init(struct ivpu_device *vdev)
>>>     static int ivpu_hw_37xx_reset(struct ivpu_device *vdev)
>>>   {
>>> -    int ret = 0;
>>> +    int retries = 100;
>>>   -    if (ivpu_boot_pwr_domain_disable(vdev)) {
>>> -        ivpu_err(vdev, "Failed to disable power domain\n");
>>> -        ret = -EIO;
>>> -    }
>>> +    while (ivpu_boot_host_ss_top_noc_cpu_ctrl_disable(vdev) && --retries > 0)
>>> +        ivpu_warn(vdev, "Retrying to disable CPU control, retries left: %d\n", retries);
>>>   -    if (ivpu_pll_disable(vdev)) {
>>> -        ivpu_err(vdev, "Failed to disable PLL\n");
>>> -        ret = -EIO;
>>> -    }
>>> +    while (ivpu_boot_host_ss_top_noc_hostif_l2cache_disable(vdev) && --retries > 0)
>>> +        ivpu_warn(vdev, "Retrying to disable HostIf L2 Cache, retries left: %d\n", retries);
>>>   -    return ret;
>>> +    while (ivpu_pll_disable(vdev) && --retries > 0)
>>> +        ivpu_warn(vdev, "Retrying to disable PLL, retries left: %d\n", retries);
>>> +
>>> +    return retries > 0 ? 0 : -EIO;
>>
>> It seems weird that retries is never reset between operations.  Why is that?
> 
> This is intentional.
> Retries are shared among all operations as we don't exacly know max number of retries for each of them.

We found a better solution to our stability issues. I will drop this patch and submit a new one.