Re: [PATCH] drm/amd/display: fix wrong plane_count check in dm_gpureset_toggle_interrupts()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch introduces a critical boolean logic error in `dm_gpureset_toggle_interrupts` that breaks DCN interrupt toggling during GPU reset.
--

--- Patch [1]: [PATCH] drm/amd/display: fix wrong plane_count check in dm_gpureset_toggle_interrupts() ---
commit 019af4e190dd8d0cacc73ad6b2cdd860d90cbb1b
Author: Lu Yao <yaolu@kylinos.cn>
drm/amd/display: fix wrong plane_count check in dm_gpureset_toggle_interrupts()

The else-if branch in dm_gpureset_toggle_interrupts() condition was incorrectly
checking plane_count != 0, which the author states is the same as the
preceding if-branch and can never be true in the else path.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index f8d8132ea50ed..e3d490d27171f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1587,7 +1587,7 @@ static void dm_gpureset_toggle_interrupts(struct amdgpu_device *adev,
>  			if (!dc_interrupt_set(adev->dm.dc, irq_source, enable))
>  				drm_warn(adev_to_drm(adev), "Failed to %sable vblank interrupt\n", enable ? "en" : "dis");
>  
> -		} else if (acrtc && state->stream_status[i].plane_count != 0) {
> +		} else if (acrtc && state->stream_status[i].plane_count == 0) {

[Severity: High]
Does this change unintentionally break interrupt toggling for DCN hardware?

The commit message states that the preceding if-branch already checks for
plane_count != 0, making it impossible to be true here. However, looking at
the preceding if-condition in dm_gpureset_toggle_interrupts():

		if (acrtc && state->stream_status[i].plane_count != 0 &&
		    amdgpu_ip_version(adev, DCE_HWIP, 0) == 0) {

The if-branch also requires amdgpu_ip_version(adev, DCE_HWIP, 0) == 0. For
DCN hardware, this IP version check evaluates to false, which causes the
execution to fall through to this else-if branch even when plane_count != 0.

With this patch changing the check to plane_count == 0, won't DCN streams
with active planes incorrectly skip toggling VUPDATE_NO_LOCK during a GPU
reset?

>  			/* DCN only needs to toggle VUPDATE_NO_LOCK */
>  			rc = amdgpu_dm_crtc_set_vupdate_irq(&acrtc->base, enable);
>  			if (rc)
>  				drm_warn(adev_to_drm(adev), "Failed to %sable vupdate interrupt\n",
>  					 enable ? "en" : "dis");
>  		}
>  	}
>  
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723015503.32122-1-yaolu@kylinos.cn?part=1