[bug report] drm/amdgpu/gfx11: add a mutex for the gfx semaphore

Dan Carpenter dan.carpenter at linaro.org
Tue Aug 20 12:55:13 UTC 2024


Hello Alex Deucher,

Commit 76acba7b7f12 ("drm/amdgpu/gfx11: add a mutex for the gfx
semaphore") from Jul 12, 2024 (linux-next), leads to the following
Smatch static checker warning:

	drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c:4778 gfx_v11_0_request_gfx_index_mutex()
	warn: inconsistent returns '&adev->gfx.reset_sem_mutex'.

drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
    4745 int gfx_v11_0_request_gfx_index_mutex(struct amdgpu_device *adev,
    4746                                       bool req)
    4747 {
    4748         u32 i, tmp, val;
    4749 
    4750         if (req)
    4751                 mutex_lock(&adev->gfx.reset_sem_mutex);

Could we move this lock into the caller?

    4752         for (i = 0; i < adev->usec_timeout; i++) {
    4753                 /* Request with MeId=2, PipeId=0 */
    4754                 tmp = REG_SET_FIELD(0, CP_GFX_INDEX_MUTEX, REQUEST, req);
    4755                 tmp = REG_SET_FIELD(tmp, CP_GFX_INDEX_MUTEX, CLIENTID, 4);
    4756                 WREG32_SOC15(GC, 0, regCP_GFX_INDEX_MUTEX, tmp);
    4757 
    4758                 val = RREG32_SOC15(GC, 0, regCP_GFX_INDEX_MUTEX);
    4759                 if (req) {
    4760                         if (val == tmp)
    4761                                 break;
    4762                 } else {
    4763                         tmp = REG_SET_FIELD(tmp, CP_GFX_INDEX_MUTEX,
    4764                                             REQUEST, 1);
    4765 
    4766                         /* unlocked or locked by firmware */
    4767                         if (val != tmp)
    4768                                 break;
    4769                 }
    4770                 udelay(1);
    4771         }
    4772         if (!req)
    4773                 mutex_unlock(&adev->gfx.reset_sem_mutex);
    4774 
    4775         if (i >= adev->usec_timeout)
    4776                 return -EINVAL;

Either way this error path needs an unlock.

	if (i >= adev->usec_timeout) {
		if (req)
			mutex_unlock(&adev->gfx.reset_sem_mutex);
		return -EINVAL;
	}

But I really think it would be nicer in the caller.

    4777 
--> 4778         return 0;
    4779 }

regards,
dan carpenter


More information about the amd-gfx mailing list