[PATCH 2/2] drm/radeon: remove bapm callbacks

[Deucher, Alexander]

> -----Original Message-----
> From: Lucas Stach [mailto:dev at lynxeye.de]
> Sent: Thursday, April 23, 2015 5:28 PM
> To: Deucher, Alexander; Koenig, Christian; David Airlie
> Cc: dri-devel at lists.freedesktop.org
> Subject: [PATCH 2/2] drm/radeon: remove bapm callbacks
> 
> Trying to disable BAPM on battery power does not fix the problematic
> Trinity mobile parts. Fixing those probably need a more complex solution,
> like doing a complete reinit of the DPM state. This will take more work
> and most likely it will not be possible to map this to a single callback.
> 
> Even worse trying to change the BAPM state mid flight is breaking BAPM
> on Richland mobile parts that are otherwise completely stable with BAPM
> enabled.

Do you have any examples of this?  This was added to fix bapm stability on mobile parts and did help on a number of systems.  Bapm is disabled by default since it is so problematic on a lot of systems.  Ripping all of this out doesn't seem like it will really help anything.

Alex

> 
> As there are no users of this hook anymore we can just remove the calling
> infrastructure.
> 
> Signed-off-by: Lucas Stach <dev at lynxeye.de>
> ---
> I've did some pretty extensive tests with one of the problematic Trinity
> mobile parts. I was able to figure out a stable config with BAPM enabled
> for this, but unfortunately the power consumption rose quite a bit. So on
> Trinity one has to disable BAPM while on battery power. I found a sequence

Can you elaborate on this?  This seems to contradict what the patch does.

> to enable/disable BAPM mid flight without hanging the GPU, but the SMC
> seemed to be stuck after that, as no DPM power level changes were
> happening
> anymore.
> ---
>  drivers/gpu/drm/radeon/radeon.h      |  2 --
>  drivers/gpu/drm/radeon/radeon_asic.c |  1 -
>  drivers/gpu/drm/radeon/radeon_asic.h |  1 -
>  drivers/gpu/drm/radeon/radeon_pm.c   |  4 ----
>  drivers/gpu/drm/radeon/trinity_dpm.c | 11 -----------
>  5 files changed, 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon.h
> b/drivers/gpu/drm/radeon/radeon.h
> index 33d5a4f..93659d2 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -1980,7 +1980,6 @@ struct radeon_asic {
>  		int (*force_performance_level)(struct radeon_device *rdev,
> enum radeon_dpm_forced_level level);
>  		bool (*vblank_too_short)(struct radeon_device *rdev);
>  		void (*powergate_uvd)(struct radeon_device *rdev, bool
> gate);
> -		void (*enable_bapm)(struct radeon_device *rdev, bool
> enable);
>  		void (*fan_ctrl_set_mode)(struct radeon_device *rdev, u32
> mode);
>  		u32 (*fan_ctrl_get_mode)(struct radeon_device *rdev);
>  		int (*set_fan_speed_percent)(struct radeon_device *rdev,
> u32 speed);
> @@ -2949,7 +2948,6 @@ static inline void radeon_ring_write(struct
> radeon_ring *ring, uint32_t v)
>  #define radeon_dpm_force_performance_level(rdev, l) rdev->asic-
> >dpm.force_performance_level((rdev), (l))
>  #define radeon_dpm_vblank_too_short(rdev) rdev->asic-
> >dpm.vblank_too_short((rdev))
>  #define radeon_dpm_powergate_uvd(rdev, g) rdev->asic-
> >dpm.powergate_uvd((rdev), (g))
> -#define radeon_dpm_enable_bapm(rdev, e) rdev->asic-
> >dpm.enable_bapm((rdev), (e))
> 
>  /* Common functions */
>  /* AGP */
> diff --git a/drivers/gpu/drm/radeon/radeon_asic.c
> b/drivers/gpu/drm/radeon/radeon_asic.c
> index 2a33e38..fa11abc 100644
> --- a/drivers/gpu/drm/radeon/radeon_asic.c
> +++ b/drivers/gpu/drm/radeon/radeon_asic.c
> @@ -1818,7 +1818,6 @@ static struct radeon_asic trinity_asic = {
>  		.print_power_state = &trinity_dpm_print_power_state,
>  		.debugfs_print_current_performance_level =
> &trinity_dpm_debugfs_print_current_performance_level,
>  		.force_performance_level =
> &trinity_dpm_force_performance_level,
> -		.enable_bapm = &trinity_dpm_enable_bapm,
>  	},
>  	.pflip = {
>  		.page_flip = &evergreen_page_flip,
> diff --git a/drivers/gpu/drm/radeon/radeon_asic.h
> b/drivers/gpu/drm/radeon/radeon_asic.h
> index 85d76da..605be51 100644
> --- a/drivers/gpu/drm/radeon/radeon_asic.h
> +++ b/drivers/gpu/drm/radeon/radeon_asic.h
> @@ -673,7 +673,6 @@ void
> trinity_dpm_debugfs_print_current_performance_level(struct
> radeon_device *r
>  							 struct seq_file *m);
>  int trinity_dpm_force_performance_level(struct radeon_device *rdev,
>  					enum radeon_dpm_forced_level
> level);
> -void trinity_dpm_enable_bapm(struct radeon_device *rdev, bool enable);
> 
>  /* DCE6 - SI */
>  void dce6_bandwidth_update(struct radeon_device *rdev);
> diff --git a/drivers/gpu/drm/radeon/radeon_pm.c
> b/drivers/gpu/drm/radeon/radeon_pm.c
> index c1ba83a..a78d146 100644
> --- a/drivers/gpu/drm/radeon/radeon_pm.c
> +++ b/drivers/gpu/drm/radeon/radeon_pm.c
> @@ -74,10 +74,6 @@ void radeon_pm_acpi_event_handler(struct
> radeon_device *rdev)
>  			rdev->pm.dpm.ac_power = true;
>  		else
>  			rdev->pm.dpm.ac_power = false;
> -		if (rdev->family == CHIP_ARUBA) {
> -			if (rdev->asic->dpm.enable_bapm)
> -				radeon_dpm_enable_bapm(rdev, rdev-
> >pm.dpm.ac_power);
> -		}
>  		mutex_unlock(&rdev->pm.mutex);
>          } else if (rdev->pm.pm_method == PM_METHOD_PROFILE) {
>  		if (rdev->pm.profile == PM_PROFILE_AUTO) {
> diff --git a/drivers/gpu/drm/radeon/trinity_dpm.c
> b/drivers/gpu/drm/radeon/trinity_dpm.c
> index 38dacb7..e473455 100644
> --- a/drivers/gpu/drm/radeon/trinity_dpm.c
> +++ b/drivers/gpu/drm/radeon/trinity_dpm.c
> @@ -1069,17 +1069,6 @@ static void trinity_update_requested_ps(struct
> radeon_device *rdev,
>  	pi->requested_rps.ps_priv = &pi->requested_ps;
>  }
> 
> -void trinity_dpm_enable_bapm(struct radeon_device *rdev, bool enable)
> -{
> -	struct trinity_power_info *pi = trinity_get_pi(rdev);
> -
> -	if (pi->enable_bapm) {
> -		trinity_acquire_mutex(rdev);
> -		trinity_dpm_bapm_enable(rdev, enable);
> -		trinity_release_mutex(rdev);
> -	}
> -}
> -
>  int trinity_dpm_enable(struct radeon_device *rdev)
>  {
>  	struct trinity_power_info *pi = trinity_get_pi(rdev);
> --
> 2.1.0