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