Re: [PATCH] drm/amd/display: Change dc_set_power_state() to bool instead of int

[Harry Wentland <harry.wentland@amd.com>]

On 2023-09-25 14:24, Mario Limonciello wrote:
> DC code is reused by other OSes and so Linux return codes don't
> make sense.  Change dc_set_power_state() to boolean and add a wrapper
> dm_set_power_state() to return a Linux error code for the memory
> allocation failure.
> 
> Suggested-by: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 11 ++++++++---
>  drivers/gpu/drm/amd/display/dc/core/dc.c          |  8 ++++----
>  drivers/gpu/drm/amd/display/dc/dc.h               |  2 +-
>  3 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 58609a8cb49d..f06136a0bba9 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2638,6 +2638,11 @@ static void hpd_rx_irq_work_suspend(struct amdgpu_display_manager *dm)
>  	}
>  }
>  
> +static int dm_set_power_state(struct dc *dc, enum dc_acpi_cm_power_state power_state)
> +{
> +	return dc_set_power_state(dc, power_state) ? 0 : -ENOMEM;
> +}
> +
>  static int dm_suspend(void *handle)
>  {
>  	struct amdgpu_device *adev = handle;
> @@ -2671,7 +2676,7 @@ static int dm_suspend(void *handle)
>  
>  	hpd_rx_irq_work_suspend(dm);
>  
> -	return dc_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D3);
> +	return dm_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D3);
>  }
>  
>  struct amdgpu_dm_connector *
> @@ -2865,7 +2870,7 @@ static int dm_resume(void *handle)
>  		if (r)
>  			DRM_ERROR("DMUB interface failed to initialize: status=%d\n", r);
>  
> -		r = dc_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
> +		r = dm_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
>  		if (r)
>  			return r;
>  
> @@ -2917,7 +2922,7 @@ static int dm_resume(void *handle)
>  	}
>  
>  	/* power on hardware */
> -	r = dc_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
> +	r = dm_set_power_state(dm->dc, DC_ACPI_CM_POWER_STATE_D0);
>  	if (r)
>  		return r;
>  
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 3b060e08707d..cb3cb2db90ee 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -4723,7 +4723,7 @@ void dc_power_down_on_boot(struct dc *dc)
>  		dc->hwss.power_down_on_boot(dc);
>  }
>  
> -int dc_set_power_state(
> +bool dc_set_power_state(
>  	struct dc *dc,
>  	enum dc_acpi_cm_power_state power_state)
>  {
> @@ -4731,7 +4731,7 @@ int dc_set_power_state(
>  	struct display_mode_lib *dml;
>  
>  	if (!dc->current_state)
> -		return 0;
> +		return true;
>  
>  	switch (power_state) {
>  	case DC_ACPI_CM_POWER_STATE_D0:
> @@ -4758,7 +4758,7 @@ int dc_set_power_state(
>  
>  		ASSERT(dml);
>  		if (!dml)
> -			return -ENOMEM;
> +			return false;
>  
>  		/* Preserve refcount */
>  		refcount = dc->current_state->refcount;
> @@ -4777,7 +4777,7 @@ int dc_set_power_state(
>  		break;
>  	}
>  
> -	return 0;
> +	return true;
>  }
>  
>  void dc_resume(struct dc *dc)
> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
> index d30de81b4779..b140eb240ad7 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
> @@ -2330,7 +2330,7 @@ void dc_notify_vsync_int_state(struct dc *dc, struct dc_stream_state *stream, bo
>  
>  /* Power Interfaces */
>  
> -int dc_set_power_state(
> +bool dc_set_power_state(
>  		struct dc *dc,
>  		enum dc_acpi_cm_power_state power_state);
>  void dc_resume(struct dc *dc);