<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Arial,Helvetica,sans-serif;" dir="ltr"> 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 <div style="color: rgb(0, 0, 0);"> <div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr">From: Koenig, Christian Sent: Monday, January 9, 2017 09:45 To: Huang, Ray; StDenis, Tom Cc: Deucher, Alexander; amd-gfx@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 <div> </div> </div> </div> <div class="PlainText">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 </div> </div> </div> </body> </html>