Hi Alex,

We don't need to set bypass mode for uvd.
For the issue uvd clock stay in high clock when uvd is idle, just need to set 
the default uvd clock to 100MHz when initialize.

Please Review the attached patch.

Best Regards
Rex

-----Original Message-----
From: Deucher, Alexander 
Sent: Friday, November 04, 2016 9:43 PM
To: Zhu, Rex; [email protected]
Subject: RE: [PATCH] drm/amdgpu: set bypass mode when uvd is idle.

> -----Original Message-----
> From: Zhu, Rex
> Sent: Thursday, November 03, 2016 11:18 PM
> To: Deucher, Alexander; [email protected]
> Subject: RE: [PATCH] drm/amdgpu: set bypass mode when uvd is idle.
> 
> >>>Is there any harm in just always putting it into bypass mode or 
> >>>does it
> interact badly with PG?  Presumably it does (otherwise we wouldn't 
> need this patch), it would be good to
> >>>note why.
> 
> Rex: when UVD PG enabled, DCLK/VCLK will be turn off when uvd is 
> idle(DCLK=OFF). If we set bypass mode=1, dclk/vclk will be bypassed to 
> an external ‘Bypass’ clock(DCLK = 100MHz)
> 
> So it is unnecessary to set bypass mode when PG enabled.
> 
> +uvd_v5_0_set_bypass_mode(adev, !enable);
> This change is because tom's commit
> 72cb64c1f6a3a8129af341e90418a687c4971a40
> Fix the sequence of UVD powergate function in smu7_clockgating.c.
> 

Thanks for clarifying.  Can you update the commit message to include these 
details?  With that:
Reviewed-by: Alex Deucher <[email protected]>

> 
> 
> 
> Best Regards
> Rex
> 
> -----Original Message-----
> From: Deucher, Alexander
> Sent: Thursday, November 03, 2016 10:21 PM
> To: Zhu, Rex; [email protected]
> Cc: Zhu, Rex
> Subject: RE: [PATCH] drm/amdgpu: set bypass mode when uvd is idle.
> 
> > -----Original Message-----
> > From: amd-gfx [mailto:[email protected]] On 
> > Behalf Of Rex Zhu
> > Sent: Thursday, November 03, 2016 4:14 AM
> > To: [email protected]
> > Cc: Zhu, Rex
> > Subject: [PATCH] drm/amdgpu: set bypass mode when uvd is idle.
> >
> > Change-Id: If44f8e91d14f5ec7d4067691684866ef8d77724a
> > Signed-off-by: Rex Zhu <[email protected]>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c | 3 ++- 
> > drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c | 3 ++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> > b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> > index 95303e2..0a6a0e7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c
> > @@ -745,7 +745,8 @@ static int uvd_v5_0_set_clockgating_state(void
> > *handle,
> >     bool enable = (state == AMD_CG_STATE_GATE) ? true : false;
> >     static int curstate = -1;
> >
> > -   uvd_v5_0_set_bypass_mode(adev, enable);
> > +   if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
> > +           uvd_v5_0_set_bypass_mode(adev, !enable);
> 
> Is there any harm in just always putting it into bypass mode or does 
> it interact badly with PG?  Presumably it does (otherwise we wouldn't 
> need this patch), it would be good to note why.
> 
> Alex
> 
> >
> >     if (!(adev->cg_flags & AMD_CG_SUPPORT_UVD_MGCG))
> >             return 0;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> > b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> > index a339b5c..b64829fe 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
> > @@ -955,7 +955,8 @@ static int uvd_v6_0_set_clockgating_state(void
> > *handle,
> >     struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> >     bool enable = (state == AMD_CG_STATE_GATE) ? true : false;
> >
> > -   uvd_v6_0_set_bypass_mode(adev, enable);
> > +   if (!(adev->pg_flags & AMD_PG_SUPPORT_UVD))
> > +           uvd_v6_0_set_bypass_mode(adev, !enable);
> >
> >     if (!(adev->cg_flags & AMD_CG_SUPPORT_UVD_MGCG))
> >             return 0;
> > --
> > 1.9.1
> >
> > _______________________________________________
> > amd-gfx mailing list
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Attachment: 0001-drm-amdgpu-not-need-to-set-bypassmode-for-uvd5.0-uvd.patch
Description: 0001-drm-amdgpu-not-need-to-set-bypassmode-for-uvd5.0-uvd.patch

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to