RE: [PATCH] drm/amd/powerplay: ack the SMUToHost interrupt on receive

["Quan, Evan" <Evan.Quan@xxxxxxx>]

[AMD Official Use Only - Internal Distribution Only]

Just setting the ACK bit is enough and no need to touch other bits.
It's aligned with windows and confirmed by my local test.

Thanks,
Evan
-----Original Message-----
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: Tuesday, May 12, 2020 9:05 PM
To: Quan, Evan <Evan.Quan@xxxxxxx>
Cc: amd-gfx list <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>
Subject: Re: [PATCH] drm/amd/powerplay: ack the SMUToHost interrupt on receive

On Tue, May 12, 2020 at 7:14 AM Evan Quan <evan.quan@xxxxxxx> wrote:
>
> There will be no further interrupt without proper ack for current one.
>
> Change-Id: Iad5adcaf7dd5c3a773b3d93ee0922a424dba8ac8
> Signed-off-by: Evan Quan <evan.quan@xxxxxxx>
> ---
>  drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index 80d6c296a599..beec4ae0b1d6 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1535,6 +1535,7 @@ static int smu_v11_0_irq_process(struct amdgpu_device *adev,
>          */
>         uint32_t ctxid = entry->src_data[0];
>         struct smu_context *smu = &adev->smu;
> +       uint32_t data;
>
>         if (client_id == SOC15_IH_CLIENTID_THM) {
>                 switch (src_id) {
> @@ -1576,6 +1577,11 @@ static int smu_v11_0_irq_process(struct amdgpu_device *adev,
>                 orderly_poweroff(true);
>         } else if (client_id == SOC15_IH_CLIENTID_MP1) {
>                 if (src_id == 0xfe) {
> +                       /* ACK SMUToHost interrupt */
> +                       data = RREG32_SOC15(MP1, 0, mmMP1_SMN_IH_SW_INT_CTRL);
> +                       data = REG_SET_FIELD(0,
> + MP1_SMN_IH_SW_INT_CTRL, INT_ACK, 1);

Did you intend to clear the register here before setting the bit or just set the ACK bit?  With that clarified:
Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>

> +                       WREG32_SOC15(MP1, 0, mmMP1_SMN_IH_SW_INT_CTRL,
> + data);
> +
>                         switch (ctxid) {
>                         case 0x3:
>                                 dev_dbg(adev->dev, "Switched to AC
> mode!\n");
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cev
> an.quan%40amd.com%7Cdd256ffb1af04cd76e7208d7f675247c%7C3dd8961fe4884e6
> 08e11a82d994e183d%7C0%7C0%7C637248855296590946&amp;sdata=vqgWfjcX9Sqqb
> We7plBmWhDVfAuyvwhNABovaafHwiE%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx