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

[Huang Rui <ray.huang-5C7GfCeVMHo@public.gmane.org>]

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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx