Re: [PATCH] drm/amdgu/vce_v3: start vce block before ring test
Am 05.02.19 um 10:21 schrieb S, Shirish: > On 2/4/2019 9:00 PM, Liu, Leo wrote: >> On 2/4/19 7:49 AM, Koenig, Christian wrote: >>> Am 04.02.19 um 13:44 schrieb S, Shirish: vce ring test fails during resume since mmVCE_RB_RPTR* is not intitalized/updated. Hence start vce block before ring test. >>> Mhm, I wonder why this ever worked. But yeah, same problem seems to >>> exits for VCE 2 as well. >>> >>> Leo any comment on this? >> UVD and VCE start function were at hw_init originally from the bring up >> on all the HW. And later the DPM developer moved them to >> set_powergating_state() for some reason. >> >> @Shirish, are you sure the vce_v3_0_start() is not there? >> >> Just simply adding it back to hw_init, might break the DPM logic, so >> please make sure. > Sure Leo, i will check and get back. I've just double checked the code and at least of hand this patch looks incorrect to me. Submitting commands to the ring should automatically calls amdgpu_vce_ring_begin_use() and so ungate the power and clocks and start the engine. This is actually vital for the ring test, so this patch is a clear NAK. Christian. > > Regards, > > Shirish S > >> Thanks, >> >> Leo >> >> >>> Thanks, >>> Christian. >>> Signed-off-by: Shirish S --- * vce_v4_0.c's hw_init sequence already has this change. drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c index 6ec65cf1..d809c10 100644 --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c @@ -469,6 +469,10 @@ static int vce_v3_0_hw_init(void *handle) int r, i; struct amdgpu_device *adev = (struct amdgpu_device *)handle; + r = vce_v3_0_start(adev); + if (r) + return r; + vce_v3_0_override_vce_clock_gating(adev, true); amdgpu_asic_set_vce_clocks(adev, 1, 1); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgu/vce_v3: start vce block before ring test
On 2/4/2019 9:00 PM, Liu, Leo wrote: > On 2/4/19 7:49 AM, Koenig, Christian wrote: >> Am 04.02.19 um 13:44 schrieb S, Shirish: >>> vce ring test fails during resume since mmVCE_RB_RPTR* >>> is not intitalized/updated. >>> >>> Hence start vce block before ring test. >> Mhm, I wonder why this ever worked. But yeah, same problem seems to >> exits for VCE 2 as well. >> >> Leo any comment on this? > UVD and VCE start function were at hw_init originally from the bring up > on all the HW. And later the DPM developer moved them to > set_powergating_state() for some reason. > > @Shirish, are you sure the vce_v3_0_start() is not there? > > Just simply adding it back to hw_init, might break the DPM logic, so > please make sure. Sure Leo, i will check and get back. Regards, Shirish S > > Thanks, > > Leo > > >> Thanks, >> Christian. >> >>> Signed-off-by: Shirish S >>> --- >>> * vce_v4_0.c's hw_init sequence already has this change. >>> >>> drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 4 >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c >>> b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c >>> index 6ec65cf1..d809c10 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c >>> @@ -469,6 +469,10 @@ static int vce_v3_0_hw_init(void *handle) >>> int r, i; >>> struct amdgpu_device *adev = (struct amdgpu_device *)handle; >>> >>> + r = vce_v3_0_start(adev); >>> + if (r) >>> + return r; >>> + >>> vce_v3_0_override_vce_clock_gating(adev, true); >>> >>> amdgpu_asic_set_vce_clocks(adev, 1, 1); -- Regards, Shirish S ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgu/vce_v3: start vce block before ring test
On 2/4/19 7:49 AM, Koenig, Christian wrote: > Am 04.02.19 um 13:44 schrieb S, Shirish: >> vce ring test fails during resume since mmVCE_RB_RPTR* >> is not intitalized/updated. >> >> Hence start vce block before ring test. > Mhm, I wonder why this ever worked. But yeah, same problem seems to > exits for VCE 2 as well. > > Leo any comment on this? UVD and VCE start function were at hw_init originally from the bring up on all the HW. And later the DPM developer moved them to set_powergating_state() for some reason. @Shirish, are you sure the vce_v3_0_start() is not there? Just simply adding it back to hw_init, might break the DPM logic, so please make sure. Thanks, Leo > > Thanks, > Christian. > >> Signed-off-by: Shirish S >> --- >> * vce_v4_0.c's hw_init sequence already has this change. >> >>drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 4 >>1 file changed, 4 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c >> b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c >> index 6ec65cf1..d809c10 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c >> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c >> @@ -469,6 +469,10 @@ static int vce_v3_0_hw_init(void *handle) >> int r, i; >> struct amdgpu_device *adev = (struct amdgpu_device *)handle; >> >> +r = vce_v3_0_start(adev); >> +if (r) >> +return r; >> + >> vce_v3_0_override_vce_clock_gating(adev, true); >> >> amdgpu_asic_set_vce_clocks(adev, 1, 1); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgu/vce_v3: start vce block before ring test
Am 04.02.19 um 13:44 schrieb S, Shirish: > vce ring test fails during resume since mmVCE_RB_RPTR* > is not intitalized/updated. > > Hence start vce block before ring test. Mhm, I wonder why this ever worked. But yeah, same problem seems to exits for VCE 2 as well. Leo any comment on this? Thanks, Christian. > > Signed-off-by: Shirish S > --- > * vce_v4_0.c's hw_init sequence already has this change. > > drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c > b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c > index 6ec65cf1..d809c10 100644 > --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c > @@ -469,6 +469,10 @@ static int vce_v3_0_hw_init(void *handle) > int r, i; > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > > + r = vce_v3_0_start(adev); > + if (r) > + return r; > + > vce_v3_0_override_vce_clock_gating(adev, true); > > amdgpu_asic_set_vce_clocks(adev, 1, 1); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
