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?
Thanks,
Rui
