drm/amdgpu: remove unused power control for vcn

Submitted by Leo Liu on July 26, 2017, 7:07 p.m.

Details

Message ID 1501096057-10197-1-git-send-email-leo.liu@amd.com
State New
Headers show
Series "drm/amdgpu: remove unused power control for vcn" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Leo Liu July 26, 2017, 7:07 p.m.
The power control for vcn has been moved to firmware, kernel'll spins 
"amdgpu: [powerplay] pp_dpm_powergate_uvd was not implemented", each
time when application runs, so let's remove it.

Signed-off-by: Leo Liu <leo.liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 23 +----------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  1 -
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c   |  4 ++--
 3 files changed, 3 insertions(+), 25 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 09190fa..4186f45 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -207,29 +207,8 @@  static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
 		container_of(work, struct amdgpu_device, vcn.idle_work.work);
 	unsigned fences = amdgpu_fence_count_emitted(&adev->vcn.ring_dec);
 
-	if (fences == 0) {
-		if (adev->pm.dpm_enabled) {
-			amdgpu_dpm_enable_uvd(adev, false);
-		} else {
-			amdgpu_asic_set_uvd_clocks(adev, 0, 0);
-		}
-	} else {
+	if (fences != 0)
 		schedule_delayed_work(&adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
-	}
-}
-
-void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
-{
-	struct amdgpu_device *adev = ring->adev;
-	bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
-
-	if (set_clocks) {
-		if (adev->pm.dpm_enabled) {
-			amdgpu_dpm_enable_uvd(adev, true);
-		} else {
-			amdgpu_asic_set_uvd_clocks(adev, 53300, 40000);
-		}
-	}
 }
 
 void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index d50ba06..930903d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -65,7 +65,6 @@  int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
 int amdgpu_vcn_sw_fini(struct amdgpu_device *adev);
 int amdgpu_vcn_suspend(struct amdgpu_device *adev);
 int amdgpu_vcn_resume(struct amdgpu_device *adev);
-void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring);
 void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring);
 
 int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring);
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index 21e7b88..94814f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -1122,7 +1122,7 @@  static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
 	.insert_start = vcn_v1_0_dec_ring_insert_start,
 	.insert_end = vcn_v1_0_dec_ring_insert_end,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
-	.begin_use = amdgpu_vcn_ring_begin_use,
+	.begin_use = NULL,
 	.end_use = amdgpu_vcn_ring_end_use,
 };
 
@@ -1148,7 +1148,7 @@  static const struct amdgpu_ring_funcs vcn_v1_0_enc_ring_vm_funcs = {
 	.insert_nop = amdgpu_ring_insert_nop,
 	.insert_end = vcn_v1_0_enc_ring_insert_end,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
-	.begin_use = amdgpu_vcn_ring_begin_use,
+	.begin_use = NULL,
 	.end_use = amdgpu_vcn_ring_end_use,
 };
 

Comments

> -----Original Message-----

> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf

> Of Leo Liu

> Sent: Wednesday, July 26, 2017 3:08 PM

> To: amd-gfx@lists.freedesktop.org

> Cc: Liu, Leo

> Subject: [PATCH] drm/amdgpu: remove unused power control for vcn

> 

> The power control for vcn has been moved to firmware, kernel'll spins

> "amdgpu: [powerplay] pp_dpm_powergate_uvd was not implemented",

> each

> time when application runs, so let's remove it.

> 

> Signed-off-by: Leo Liu <leo.liu@amd.com>


Does firmware cover powergating/clockgating as well or just dpm?  I think we may still need to handle clock and powergating  manually.

Alex

> ---

>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 23 +----------------------

>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  1 -

>  drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c   |  4 ++--

>  3 files changed, 3 insertions(+), 25 deletions(-)

> 

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c

> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c

> index 09190fa..4186f45 100644

> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c

> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c

> @@ -207,29 +207,8 @@ static void amdgpu_vcn_idle_work_handler(struct

> work_struct *work)

>  		container_of(work, struct amdgpu_device,

> vcn.idle_work.work);

>  	unsigned fences = amdgpu_fence_count_emitted(&adev-

> >vcn.ring_dec);

> 

> -	if (fences == 0) {

> -		if (adev->pm.dpm_enabled) {

> -			amdgpu_dpm_enable_uvd(adev, false);

> -		} else {

> -			amdgpu_asic_set_uvd_clocks(adev, 0, 0);

> -		}

> -	} else {

> +	if (fences != 0)

>  		schedule_delayed_work(&adev->vcn.idle_work,

> VCN_IDLE_TIMEOUT);

> -	}

> -}

> -

> -void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)

> -{

> -	struct amdgpu_device *adev = ring->adev;

> -	bool set_clocks = !cancel_delayed_work_sync(&adev-

> >vcn.idle_work);

> -

> -	if (set_clocks) {

> -		if (adev->pm.dpm_enabled) {

> -			amdgpu_dpm_enable_uvd(adev, true);

> -		} else {

> -			amdgpu_asic_set_uvd_clocks(adev, 53300, 40000);

> -		}

> -	}

>  }

> 

>  void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h

> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h

> index d50ba06..930903d 100644

> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h

> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h

> @@ -65,7 +65,6 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev);

>  int amdgpu_vcn_sw_fini(struct amdgpu_device *adev);

>  int amdgpu_vcn_suspend(struct amdgpu_device *adev);

>  int amdgpu_vcn_resume(struct amdgpu_device *adev);

> -void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring);

>  void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring);

> 

>  int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring);

> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c

> b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c

> index 21e7b88..94814f0 100644

> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c

> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c

> @@ -1122,7 +1122,7 @@ static const struct amdgpu_ring_funcs

> vcn_v1_0_dec_ring_vm_funcs = {

>  	.insert_start = vcn_v1_0_dec_ring_insert_start,

>  	.insert_end = vcn_v1_0_dec_ring_insert_end,

>  	.pad_ib = amdgpu_ring_generic_pad_ib,

> -	.begin_use = amdgpu_vcn_ring_begin_use,

> +	.begin_use = NULL,

>  	.end_use = amdgpu_vcn_ring_end_use,

>  };

> 

> @@ -1148,7 +1148,7 @@ static const struct amdgpu_ring_funcs

> vcn_v1_0_enc_ring_vm_funcs = {

>  	.insert_nop = amdgpu_ring_insert_nop,

>  	.insert_end = vcn_v1_0_enc_ring_insert_end,

>  	.pad_ib = amdgpu_ring_generic_pad_ib,

> -	.begin_use = amdgpu_vcn_ring_begin_use,

> +	.begin_use = NULL,

>  	.end_use = amdgpu_vcn_ring_end_use,

>  };

> 

> --

> 2.7.4

> 

> _______________________________________________

> amd-gfx mailing list

> amd-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Am 27.07.2017 um 04:44 schrieb Deucher, Alexander:
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
>> Of Leo Liu
>> Sent: Wednesday, July 26, 2017 3:08 PM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Liu, Leo
>> Subject: [PATCH] drm/amdgpu: remove unused power control for vcn
>>
>> The power control for vcn has been moved to firmware, kernel'll spins
>> "amdgpu: [powerplay] pp_dpm_powergate_uvd was not implemented",
>> each
>> time when application runs, so let's remove it.
>>
>> Signed-off-by: Leo Liu <leo.liu@amd.com>
> Does firmware cover powergating/clockgating as well or just dpm?  I think we may still need to handle clock and powergating  manually.

Additional to that you could remove even more when that isn't used any 
more. E.g. the whole idle_work and end_use() function can now go away as 
well.

Christian.

>
> Alex
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 23 +----------------------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  1 -
>>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c   |  4 ++--
>>   3 files changed, 3 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> index 09190fa..4186f45 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
>> @@ -207,29 +207,8 @@ static void amdgpu_vcn_idle_work_handler(struct
>> work_struct *work)
>>   		container_of(work, struct amdgpu_device,
>> vcn.idle_work.work);
>>   	unsigned fences = amdgpu_fence_count_emitted(&adev-
>>> vcn.ring_dec);
>> -	if (fences == 0) {
>> -		if (adev->pm.dpm_enabled) {
>> -			amdgpu_dpm_enable_uvd(adev, false);
>> -		} else {
>> -			amdgpu_asic_set_uvd_clocks(adev, 0, 0);
>> -		}
>> -	} else {
>> +	if (fences != 0)
>>   		schedule_delayed_work(&adev->vcn.idle_work,
>> VCN_IDLE_TIMEOUT);
>> -	}
>> -}
>> -
>> -void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
>> -{
>> -	struct amdgpu_device *adev = ring->adev;
>> -	bool set_clocks = !cancel_delayed_work_sync(&adev-
>>> vcn.idle_work);
>> -
>> -	if (set_clocks) {
>> -		if (adev->pm.dpm_enabled) {
>> -			amdgpu_dpm_enable_uvd(adev, true);
>> -		} else {
>> -			amdgpu_asic_set_uvd_clocks(adev, 53300, 40000);
>> -		}
>> -	}
>>   }
>>
>>   void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> index d50ba06..930903d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
>> @@ -65,7 +65,6 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
>>   int amdgpu_vcn_sw_fini(struct amdgpu_device *adev);
>>   int amdgpu_vcn_suspend(struct amdgpu_device *adev);
>>   int amdgpu_vcn_resume(struct amdgpu_device *adev);
>> -void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring);
>>   void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring);
>>
>>   int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> index 21e7b88..94814f0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> @@ -1122,7 +1122,7 @@ static const struct amdgpu_ring_funcs
>> vcn_v1_0_dec_ring_vm_funcs = {
>>   	.insert_start = vcn_v1_0_dec_ring_insert_start,
>>   	.insert_end = vcn_v1_0_dec_ring_insert_end,
>>   	.pad_ib = amdgpu_ring_generic_pad_ib,
>> -	.begin_use = amdgpu_vcn_ring_begin_use,
>> +	.begin_use = NULL,
>>   	.end_use = amdgpu_vcn_ring_end_use,
>>   };
>>
>> @@ -1148,7 +1148,7 @@ static const struct amdgpu_ring_funcs
>> vcn_v1_0_enc_ring_vm_funcs = {
>>   	.insert_nop = amdgpu_ring_insert_nop,
>>   	.insert_end = vcn_v1_0_enc_ring_insert_end,
>>   	.pad_ib = amdgpu_ring_generic_pad_ib,
>> -	.begin_use = amdgpu_vcn_ring_begin_use,
>> +	.begin_use = NULL,
>>   	.end_use = amdgpu_vcn_ring_end_use,
>>   };
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On 07/26/2017 10:44 PM, Deucher, Alexander wrote:
> > -----Original Message-----
> > From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> > Of Leo Liu
> > Sent: Wednesday, July 26, 2017 3:08 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Liu, Leo
> > Subject: [PATCH] drm/amdgpu: remove unused power control for vcn
> >
> > The power control for vcn has been moved to firmware, kernel'll spins
> > "amdgpu: [powerplay] pp_dpm_powergate_uvd was not implemented",
> > each
> > time when application runs, so let's remove it.
> >
> > Signed-off-by: Leo Liu <leo.liu@amd.com>
>
> Does firmware cover powergating/clockgating as well or just dpm?
No. dpm only.

> I think we may still need to handle clock and powergating  manually.
I will keep the function and comment it out till pg/cg, since it's 
annoying to spin that error message.

The new patch will follow shortly.

Leo

>
> Alex
>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 23 +----------------------
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  1 -
> >  drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c   |  4 ++--
> >  3 files changed, 3 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> > index 09190fa..4186f45 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> > @@ -207,29 +207,8 @@ static void amdgpu_vcn_idle_work_handler(struct
> > work_struct *work)
> >                container_of(work, struct amdgpu_device,
> > vcn.idle_work.work);
> >        unsigned fences = amdgpu_fence_count_emitted(&adev-
> > >vcn.ring_dec);
> >
> > -     if (fences == 0) {
> > -             if (adev->pm.dpm_enabled) {
> > -                     amdgpu_dpm_enable_uvd(adev, false);
> > -             } else {
> > -                     amdgpu_asic_set_uvd_clocks(adev, 0, 0);
> > -             }
> > -     } else {
> > +     if (fences != 0)
> > schedule_delayed_work(&adev->vcn.idle_work,
> > VCN_IDLE_TIMEOUT);
> > -     }
> > -}
> > -
> > -void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
> > -{
> > -     struct amdgpu_device *adev = ring->adev;
> > -     bool set_clocks = !cancel_delayed_work_sync(&adev-
> > >vcn.idle_work);
> > -
> > -     if (set_clocks) {
> > -             if (adev->pm.dpm_enabled) {
> > -                     amdgpu_dpm_enable_uvd(adev, true);
> > -             } else {
> > -                     amdgpu_asic_set_uvd_clocks(adev, 53300, 40000);
> > -             }
> > -     }
> >  }
> >
> >  void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> > index d50ba06..930903d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> > @@ -65,7 +65,6 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev);
> >  int amdgpu_vcn_sw_fini(struct amdgpu_device *adev);
> >  int amdgpu_vcn_suspend(struct amdgpu_device *adev);
> >  int amdgpu_vcn_resume(struct amdgpu_device *adev);
> > -void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring);
> >  void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring);
> >
> >  int amdgpu_vcn_dec_ring_test_ring(struct amdgpu_ring *ring);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> > b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> > index 21e7b88..94814f0 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> > @@ -1122,7 +1122,7 @@ static const struct amdgpu_ring_funcs
> > vcn_v1_0_dec_ring_vm_funcs = {
> >        .insert_start = vcn_v1_0_dec_ring_insert_start,
> >        .insert_end = vcn_v1_0_dec_ring_insert_end,
> >        .pad_ib = amdgpu_ring_generic_pad_ib,
> > -     .begin_use = amdgpu_vcn_ring_begin_use,
> > +     .begin_use = NULL,
> >        .end_use = amdgpu_vcn_ring_end_use,
> >  };
> >
> > @@ -1148,7 +1148,7 @@ static const struct amdgpu_ring_funcs
> > vcn_v1_0_enc_ring_vm_funcs = {
> >        .insert_nop = amdgpu_ring_insert_nop,
> >        .insert_end = vcn_v1_0_enc_ring_insert_end,
> >        .pad_ib = amdgpu_ring_generic_pad_ib,
> > -     .begin_use = amdgpu_vcn_ring_begin_use,
> > +     .begin_use = NULL,
> >        .end_use = amdgpu_vcn_ring_end_use,
> >  };
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx