Yes, holding pm.mutex will prevent PP/DPM from entering/leaving PG states and is used as a means to probe power/clock related signals reliably (except for VCE clock signals which aren't part of an AON tile). You just can't take it from a "set" state function since the higher up API will take it. Tom ________________________________ From: Koenig, Christian Sent: Monday, January 9, 2017 09:45 To: Huang, Ray; StDenis, Tom Cc: Deucher, Alexander; amd-gfx at lists.freedesktop.org; Zhu, Rex; Mao, David; Fu, Ping; Zhang, Hawking; Kuehling, Felix Subject: Re: [PART1 PATCH v4 7/8] drm/amdgpu: add get clockgating_state method for uvd v5&v6 Am 09.01.2017 um 15:46 schrieb Huang Rui: > On Mon, Jan 09, 2017 at 07:29:00PM +0800, StDenis, Tom wrote: >> Yup it's held by both amdgpu_dpm_enable_uvd() and amdgpu_dpm_enable_vce() >> >> Tom >> > <snip> > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c b/drivers/gpu/drm/amd/ >> amdgpu/uvd_v5_0.c >>> index 03a35d9..e647d3e 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c >>> @@ -781,16 +781,48 @@ static int uvd_v5_0_set_powergating_state(void *handle, >>> * the smc and the hw blocks >>> */ >>> struct amdgpu_device *adev = (struct amdgpu_device *)handle; >>> + int ret = 0; >>> >>> if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD)) >>> return 0; >>> >>> + mutex_lock(&adev->pm.mutex); >> Might be that I'm wrong, but didn't Tom said the mutex is taken anyway >> when this function is called? >> >> If that's true we would certainly run into problem when we try to >> acquire it again. >> > OK, I see. Actually, pm.mutex is already held on upper layer call > (amdgpu_dpm_enable_uvd). So we should not hold it again here. > >>> + >>> +static void uvd_v5_0_get_clockgating_state(void *handle, u32 *flags) >>> +{ >>> + struct amdgpu_device *adev = (struct amdgpu_device *)handle; >>> + int data; >>> + >>> + mutex_lock(&adev->pm.mutex); >>> + > We just need keep pm.mutex here in set_clockgating_state, that's enough. > > Christian, Tom, am I right? Yes, I think so. Christian. > > Thanks, > Rui -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20170109/ff0221ed/attachment-0001.html>