| Message ID | 20240820143909.1933483-1-alexander.deucher@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd/gfx11: move the gfx mutex into the caller | expand |
On Tue, Aug 20, 2024 at 10:39:09AM -0400, Alex Deucher wrote: > Otherwise we can fail to drop the software mutex when > we fail to take the hardware mutex. > > Fixes: 76acba7b7f12 ("drm/amdgpu/gfx11: add a mutex for the gfx semaphore") > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> Thanks so much! Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org> regards, dan carpenter
On 8/20/2024 8:09 PM, Alex Deucher wrote: > Otherwise we can fail to drop the software mutex when > we fail to take the hardware mutex. > > Fixes: 76acba7b7f12 ("drm/amdgpu/gfx11: add a mutex for the gfx semaphore") > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c > index 5704ad25a49d6..aa7fdece8ad42 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c > @@ -4747,8 +4747,6 @@ int gfx_v11_0_request_gfx_index_mutex(struct amdgpu_device *adev, > { > u32 i, tmp, val; > > - if (req) > - mutex_lock(&adev->gfx.reset_sem_mutex); > for (i = 0; i < adev->usec_timeout; i++) { > /* Request with MeId=2, PipeId=0 */ > tmp = REG_SET_FIELD(0, CP_GFX_INDEX_MUTEX, REQUEST, req); > @@ -4769,8 +4767,6 @@ int gfx_v11_0_request_gfx_index_mutex(struct amdgpu_device *adev, > } > udelay(1); > } > - if (!req) > - mutex_unlock(&adev->gfx.reset_sem_mutex); > > if (i >= adev->usec_timeout) > return -EINVAL; > @@ -4818,8 +4814,10 @@ static int gfx_v11_0_soft_reset(void *handle) > mutex_unlock(&adev->srbm_mutex); > > /* Try to acquire the gfx mutex before access to CP_VMID_RESET */ > + mutex_lock(&adev->gfx.reset_sem_mutex); > r = gfx_v11_0_request_gfx_index_mutex(adev, true); > if (r) { > + mutex_unlock(&adev->gfx.reset_sem_mutex); > DRM_ERROR("Failed to acquire the gfx mutex during soft reset\n"); > return r; > } > @@ -4834,6 +4832,7 @@ static int gfx_v11_0_soft_reset(void *handle) > > /* release the gfx mutex */ > r = gfx_v11_0_request_gfx_index_mutex(adev, false); > + mutex_unlock(&adev->gfx.reset_sem_mutex); > if (r) { > DRM_ERROR("Failed to release the gfx mutex during soft reset\n"); > return r; By moving locking and unlocking of the mutex to the gfx_v11_0_soft_reset function (the caller). This ensures that the mutex is always unlocked, regardless of whether (indicated by if (i >= adev->usec_timeout)) succeeds or fails. Acked-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c index 5704ad25a49d6..aa7fdece8ad42 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c @@ -4747,8 +4747,6 @@ int gfx_v11_0_request_gfx_index_mutex(struct amdgpu_device *adev, { u32 i, tmp, val; - if (req) - mutex_lock(&adev->gfx.reset_sem_mutex); for (i = 0; i < adev->usec_timeout; i++) { /* Request with MeId=2, PipeId=0 */ tmp = REG_SET_FIELD(0, CP_GFX_INDEX_MUTEX, REQUEST, req); @@ -4769,8 +4767,6 @@ int gfx_v11_0_request_gfx_index_mutex(struct amdgpu_device *adev, } udelay(1); } - if (!req) - mutex_unlock(&adev->gfx.reset_sem_mutex); if (i >= adev->usec_timeout) return -EINVAL; @@ -4818,8 +4814,10 @@ static int gfx_v11_0_soft_reset(void *handle) mutex_unlock(&adev->srbm_mutex); /* Try to acquire the gfx mutex before access to CP_VMID_RESET */ + mutex_lock(&adev->gfx.reset_sem_mutex); r = gfx_v11_0_request_gfx_index_mutex(adev, true); if (r) { + mutex_unlock(&adev->gfx.reset_sem_mutex); DRM_ERROR("Failed to acquire the gfx mutex during soft reset\n"); return r; } @@ -4834,6 +4832,7 @@ static int gfx_v11_0_soft_reset(void *handle) /* release the gfx mutex */ r = gfx_v11_0_request_gfx_index_mutex(adev, false); + mutex_unlock(&adev->gfx.reset_sem_mutex); if (r) { DRM_ERROR("Failed to release the gfx mutex during soft reset\n"); return r;
Otherwise we can fail to drop the software mutex when we fail to take the hardware mutex. Fixes: 76acba7b7f12 ("drm/amdgpu/gfx11: add a mutex for the gfx semaphore") Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)