[PATCH] drm/amd/pm: fix return value in aldebaran_set_mp1_state()

[Xu, Feifei]

[AMD Official Use Only]

PP_MP1_STATE_NONE should be valid while other 2 are not for Aldebaran for now. It depends on whether driver want to throw out error if the msg fall out of the 4 cases.
Benefit of handling all the cases will be catching potential bugs easily. But the handling of these 4 and others are the same in both driver and PMFW - which should skip and return 0.
So I am ok with the simplify code logic. Will take your suggestion which return 0 for default. Thanks.

Thanks,
Feifei

-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar at amd.com>
Sent: Thursday, May 20, 2021 7:19 PM
To: Xu, Feifei <Feifei.Xu at amd.com>; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH] drm/amd/pm: fix return value in aldebaran_set_mp1_state()

This now handles all of the states. Those states are not valid (and therefore not handled) for Aldebaran. If the intent is to skip handling of any other state, may be just return 0 for default or skip default altogether so that it falls through to return 0 for any other state.

In any case,

Reviewed-by: Lijo Lazar <lijo.lazar at amd.com>

On 5/20/2021 3:20 PM, Feifei Xu wrote:
> We should just return error in invalid case. For valid but not
> implemented one, do nothing and return 0. Otherwise resume will abort
> because of the wrong return value.
>
> Signed-off-by: Feifei Xu <Feifei.Xu at amd.com>
> ---
>   drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index 5d04a1dfdfd8..5fcfd8e1a548 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -1781,13 +1781,15 @@ static int aldebaran_set_mp1_state(struct smu_context *smu,
>                                  enum pp_mp1_state mp1_state)
>   {
>       switch (mp1_state) {
> +     case PP_MP1_STATE_NONE:
> +     case PP_MP1_STATE_RESET:
> +     case PP_MP1_STATE_SHUTDOWN:
> +             return 0;
>       case PP_MP1_STATE_UNLOAD:
>               return smu_cmn_set_mp1_state(smu, mp1_state);
>       default:
>               return -EINVAL;
>       }
> -
> -     return 0;
>   }
>
>   static const struct pptable_funcs aldebaran_ppt_funcs = {
>

--
Thanks,
Lijo