Maybe add a comment to that effect. With that added, this patch is: Reviewed-by: Alex Deucher <alexander.deucher at amd.com> From: StDenis, Tom Sent: Wednesday, August 03, 2016 12:31 PM To: Deucher, Alexander; amd-gfx at lists.freedesktop.org Subject: Re: [PATCH 2/6] drm/amd/amdgpu: Fix VCE CG order and resume defaults The logic is a bit confusing at first. "gated" means that SW CG is enabled not that the clocks are turned off. When I sampled the registers in the original configuration the clocks were reading 100% when the core was active. It would read 0% when idle but only because power was gated. With the logic the way it is when the core is active I see many of the clocks throttle as they should. Tom ________________________________ From: Deucher, Alexander Sent: Wednesday, August 3, 2016 12:28 To: 'Tom St Denis'; amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> Cc: StDenis, Tom Subject: RE: [PATCH 2/6] drm/amd/amdgpu: Fix VCE CG order and resume defaults > -----Original Message----- > From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf > Of Tom St Denis > Sent: Wednesday, August 03, 2016 11:52 AM > To: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> > Cc: StDenis, Tom > Subject: [PATCH 2/6] drm/amd/amdgpu: Fix VCE CG order and resume > defaults > > CG was being enabled in reverse sense from dpm/powerplay. > Also fix the default CLK_EN signal to enable all of the blocks. > > Signed-off-by: Tom St Denis <tom.stdenis at amd.com<mailto:tom.stdenis at amd.com>> > --- > drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c > b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c > index 26fb606e2d3e..93604bb78257 100644 > --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c > @@ -130,7 +130,7 @@ static void vce_v3_0_set_vce_sw_clock_gating(struct > amdgpu_device *adev, > /* Set Override to disable Clock Gating */ > vce_v3_0_override_vce_clock_gating(adev, true); > > - if (!gated) { > + if (gated) { > /* Force CLOCK ON for VCE_CLOCK_GATING_B, > * {*_FORCE_ON, *_FORCE_OFF} = {1, 0} This doesn't make sense to me unless I'm mis-understanding the specific CG feature. Forcing the clocks on, implies ungating, not gating. In this case if forcing the clocks on part of the fw controlled clockgating configuration? If so, please add a comment to that effect in the function. Alex > * VREG can be FORCE ON or set to Dynamic, but can't be OFF > @@ -153,7 +153,7 @@ static void vce_v3_0_set_vce_sw_clock_gating(struct > amdgpu_device *adev, > /* set VCE_UENC_CLOCK_GATING_2 */ > tmp = data = RREG32(mmVCE_UENC_CLOCK_GATING_2); > data |= 0x2; > - data &= ~0x2; > + data &= ~0x00010000; > if (tmp != data) > WREG32(mmVCE_UENC_CLOCK_GATING_2, data); > > @@ -538,7 +538,7 @@ static void vce_v3_0_mc_resume(struct > amdgpu_device *adev, int idx) > WREG32_P(mmVCE_CLOCK_GATING_A, 0, ~(1 << 16)); > WREG32_P(mmVCE_UENC_CLOCK_GATING, 0x1FF000, > ~0xFF9FF000); > WREG32_P(mmVCE_UENC_REG_CLOCK_GATING, 0x3F, ~0x3F); > - WREG32(mmVCE_CLOCK_GATING_B, 0xf7); > + WREG32(mmVCE_CLOCK_GATING_B, 0x1FF); > > WREG32(mmVCE_LMI_CTRL, 0x00398000); > WREG32_P(mmVCE_LMI_CACHE_CTRL, 0x0, ~0x1); > -- > 2.9.2 > > _______________________________________________ > amd-gfx mailing list > amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20160803/4313fecd/attachment.html>
