drm/amdgu/vce_v3: start vce block before ring test

Submitted by S, Shirish on Feb. 4, 2019, 12:44 p.m.

Details

Message ID 1549284230-32705-1-git-send-email-shirish.s@amd.com
State New
Series "drm/amdgu/vce_v3: start vce block before ring test"
Headers show

Commit Message

S, Shirish Feb. 4, 2019, 12:44 p.m.
vce ring test fails during resume since mmVCE_RB_RPTR*
is not intitalized/updated.

Hence start vce block before ring test.

Signed-off-by: Shirish S <shirish.s@amd.com>
---
* 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(+)

Patch hide | download patch | download mbox

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, 10000, 10000);

Comments

Koenig, Christian Feb. 4, 2019, 12:49 p.m.
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 <shirish.s@amd.com>

> ---

> * 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, 10000, 10000);
Liu, Leo Feb. 4, 2019, 3:30 p.m.
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 <shirish.s@amd.com>

>> ---

>> * 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, 10000, 10000);
S, Shirish Feb. 5, 2019, 9:21 a.m.
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 <shirish.s@amd.com>

>>> ---

>>> * 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, 10000, 10000);


-- 
Regards,
Shirish S
Koenig, Christian Feb. 5, 2019, 10:57 a.m.
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 <shirish.s@amd.com>

>>>> ---

>>>> * 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, 10000, 10000);