Re: [PATCH 04/11] drm/amd/display: Handle SMU msg response

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 26, 2020 at 12:19 PM Eryk Brol <eryk.brol@xxxxxxx> wrote:
>
> From: Yongqiang Sun <yongqiang.sun@xxxxxxx>
>
> [Why]
> SMU may return error code to driver, but driver only check if response
> is OK.
>
> [How]
> Check SMU response instead of reg_wait, assert in case of reponse isn't
> OK.

Will you ever get concurrent calls to these interfaces or do you
already have a higher level lock to prevent that?  You need to make
sure you don't have multiple threads using these interfaces at the
same time or you'll need locking to protect the message, param, and
response registers.

Alex

>
> Signed-off-by: Yongqiang Sun <yongqiang.sun@xxxxxxx>
> Reviewed-by: Tony Cheng <Tony.Cheng@xxxxxxx>
> Acked-by: Eryk Brol <eryk.brol@xxxxxxx>
> ---
>  .../dc/clk_mgr/dcn10/rv1_clk_mgr_vbios_smu.c  | 39 +++++++++++++++++-
>  .../dc/clk_mgr/dcn21/rn_clk_mgr_vbios_smu.c   | 40 ++++++++++++++++++-
>  2 files changed, 75 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr_vbios_smu.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr_vbios_smu.c
> index c320b7af7d34..dbc7cde00433 100644
> --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr_vbios_smu.c
> +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn10/rv1_clk_mgr_vbios_smu.c
> @@ -26,6 +26,7 @@
>  #include "core_types.h"
>  #include "clk_mgr_internal.h"
>  #include "reg_helper.h"
> +#include <linux/delay.h>
>
>  #define MAX_INSTANCE   5
>  #define MAX_SEGMENT            5
> @@ -68,10 +69,42 @@ static const struct IP_BASE MP1_BASE  = { { { { 0x00016000, 0, 0, 0, 0 } },
>  #define VBIOSSMC_MSG_SetDispclkFreq           0x4
>  #define VBIOSSMC_MSG_SetDprefclkFreq          0x5
>
> +#define VBIOSSMC_Status_BUSY                      0x0
> +#define VBIOSSMC_Result_OK                        0x1
> +#define VBIOSSMC_Result_Failed                    0xFF
> +#define VBIOSSMC_Result_UnknownCmd                0xFE
> +#define VBIOSSMC_Result_CmdRejectedPrereq         0xFD
> +#define VBIOSSMC_Result_CmdRejectedBusy           0xFC
> +
> +/*
> + * Function to be used instead of REG_WAIT macro because the wait ends when
> + * the register is NOT EQUAL to zero, and because the translation in msg_if.h
> + * won't work with REG_WAIT.
> + */
> +static uint32_t rv1_smu_wait_for_response(struct clk_mgr_internal *clk_mgr, unsigned int delay_us, unsigned int max_retries)
> +{
> +       uint32_t res_val = VBIOSSMC_Status_BUSY;
> +
> +       do {
> +               res_val = REG_READ(MP1_SMN_C2PMSG_91);
> +               if (res_val != VBIOSSMC_Status_BUSY)
> +                       break;
> +
> +               if (delay_us >= 1000)
> +                       msleep(delay_us/1000);
> +               else if (delay_us > 0)
> +                       udelay(delay_us);
> +       } while (max_retries--);
> +
> +       return res_val;
> +}
> +
>  int rv1_vbios_smu_send_msg_with_param(struct clk_mgr_internal *clk_mgr, unsigned int msg_id, unsigned int param)
>  {
> +       uint32_t result;
> +
>         /* First clear response register */
> -       REG_WRITE(MP1_SMN_C2PMSG_91, 0);
> +       REG_WRITE(MP1_SMN_C2PMSG_91, VBIOSSMC_Status_BUSY);
>
>         /* Set the parameter register for the SMU message, unit is Mhz */
>         REG_WRITE(MP1_SMN_C2PMSG_83, param);
> @@ -79,7 +112,9 @@ int rv1_vbios_smu_send_msg_with_param(struct clk_mgr_internal *clk_mgr, unsigned
>         /* Trigger the message transaction by writing the message ID */
>         REG_WRITE(MP1_SMN_C2PMSG_67, msg_id);
>
> -       REG_WAIT(MP1_SMN_C2PMSG_91, CONTENT, 1, 10, 200000);
> +       result = rv1_smu_wait_for_response(clk_mgr, 10, 1000);
> +
> +       ASSERT(result == VBIOSSMC_Result_OK);
>
>         /* Actual dispclk set is returned in the parameter register */
>         return REG_READ(MP1_SMN_C2PMSG_83);
> diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr_vbios_smu.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr_vbios_smu.c
> index 6878aedf1d3e..d2facbb114d3 100644
> --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr_vbios_smu.c
> +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr_vbios_smu.c
> @@ -26,6 +26,7 @@
>  #include "core_types.h"
>  #include "clk_mgr_internal.h"
>  #include "reg_helper.h"
> +#include <linux/delay.h>
>
>  #include "renoir_ip_offset.h"
>
> @@ -53,10 +54,43 @@
>  #define VBIOSSMC_MSG_EnableTmdp48MHzRefclkPwrDown 0xD
>  #define VBIOSSMC_MSG_UpdatePmeRestore                    0xE
>
> +#define VBIOSSMC_Status_BUSY                      0x0
> +#define VBIOSSMC_Result_OK                        0x1
> +#define VBIOSSMC_Result_Failed                    0xFF
> +#define VBIOSSMC_Result_UnknownCmd                0xFE
> +#define VBIOSSMC_Result_CmdRejectedPrereq         0xFD
> +#define VBIOSSMC_Result_CmdRejectedBusy           0xFC
> +
> +/*
> + * Function to be used instead of REG_WAIT macro because the wait ends when
> + * the register is NOT EQUAL to zero, and because the translation in msg_if.h
> + * won't work with REG_WAIT.
> + */
> +static uint32_t rn_smu_wait_for_response(struct clk_mgr_internal *clk_mgr, unsigned int delay_us, unsigned int max_retries)
> +{
> +       uint32_t res_val = VBIOSSMC_Status_BUSY;
> +
> +       do {
> +               res_val = REG_READ(MP1_SMN_C2PMSG_91);
> +               if (res_val != VBIOSSMC_Status_BUSY)
> +                       break;
> +
> +               if (delay_us >= 1000)
> +                       msleep(delay_us/1000);
> +               else if (delay_us > 0)
> +                       udelay(delay_us);
> +       } while (max_retries--);
> +
> +       return res_val;
> +}
> +
> +
>  int rn_vbios_smu_send_msg_with_param(struct clk_mgr_internal *clk_mgr, unsigned int msg_id, unsigned int param)
>  {
> +       uint32_t result;
> +
>         /* First clear response register */
> -       REG_WRITE(MP1_SMN_C2PMSG_91, 0);
> +       REG_WRITE(MP1_SMN_C2PMSG_91, VBIOSSMC_Status_BUSY);
>
>         /* Set the parameter register for the SMU message, unit is Mhz */
>         REG_WRITE(MP1_SMN_C2PMSG_83, param);
> @@ -64,7 +98,9 @@ int rn_vbios_smu_send_msg_with_param(struct clk_mgr_internal *clk_mgr, unsigned
>         /* Trigger the message transaction by writing the message ID */
>         REG_WRITE(MP1_SMN_C2PMSG_67, msg_id);
>
> -       REG_WAIT(MP1_SMN_C2PMSG_91, CONTENT, 1, 10, 200000);
> +       result = rn_smu_wait_for_response(clk_mgr, 10, 1000);
> +
> +       ASSERT(result == VBIOSSMC_Result_OK);
>
>         /* Actual dispclk set is returned in the parameter register */
>         return REG_READ(MP1_SMN_C2PMSG_83);
> --
> 2.25.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux