[PART1 PATCH v4 7/8] drm/amdgpu: add get clockgating_state method for uvd v5&v6

[StDenis, Tom]

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>