[AMD Official Use Only - General] > -----Original Message----- > From: Li, Sun peng (Leo) <Sunpeng.Li@xxxxxxx> > Sent: Friday, June 23, 2023 2:27 PM > To: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; amd- > gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: Lin, Tsung-hua (Ryan) <Tsung-hua.Lin@xxxxxxx>; Rossi, Marc > <Marc.Rossi@xxxxxxx>; Wang, Sean <sean.ns.wang@xxxxxxx>; Mahfooz, > Hamza <Hamza.Mahfooz@xxxxxxx> > Subject: Re: [PATCH 2/4] drm/amd/display: Set minimum requirement for > using PSR-SU on Rembrandt > > > > > On 6/22/23 14:25, Mario Limonciello wrote: > > A number of parade TCONs are causing system hangs when utilized with > > older DMUB firmware and PSR-SU. Some changes have been introduced into > > DMUB firmware to add resilience against these failures. > > > > Don't allow running PSR-SU unless on the newer firmware. > > > > Cc: Sean Wang <sean.ns.wang@xxxxxxx> > > Cc: Marc Rossi <Marc.Rossi@xxxxxxx> > > Cc: Hamza Mahfooz <Hamza.Mahfooz@xxxxxxx> > > Cc: Tsung-hua (Ryan) Lin <Tsung-hua.Lin@xxxxxxx> > > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2443 > > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > > --- > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c | 3 ++- > > drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c | 7 +++++++ > > drivers/gpu/drm/amd/display/dc/dc_dmub_srv.h | 1 + > > drivers/gpu/drm/amd/display/dmub/dmub_srv.h | 2 ++ > > drivers/gpu/drm/amd/display/dmub/src/dmub_dcn31.c | 5 +++++ > > drivers/gpu/drm/amd/display/dmub/src/dmub_dcn31.h | 2 ++ > > drivers/gpu/drm/amd/display/dmub/src/dmub_srv.c | 10 ++++++---- > > 7 files changed, 25 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c > > index d647f68fd563..4f61d4f257cd 100644 > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c > > @@ -24,6 +24,7 @@ > > */ > > > > #include "amdgpu_dm_psr.h" > > +#include "dc_dmub_srv.h" > > #include "dc.h" > > #include "dm_helpers.h" > > #include "amdgpu_dm.h" > > @@ -50,7 +51,7 @@ static bool link_supports_psrsu(struct dc_link *link) > > !link->dpcd_caps.psr_info.psr2_su_y_granularity_cap) > > return false; > > > > - return true; > > + return dc_dmub_check_min_version(dc->ctx->dmub_srv->dmub); > > } > > > > /* > > diff --git a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > > index c52c40b16387..c753c6f30dd7 100644 > > --- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > > +++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.c > > @@ -1011,3 +1011,10 @@ void dc_send_update_cursor_info_to_dmu( > > dm_execute_dmub_cmd_list(pCtx->stream->ctx, 2, cmd, > DM_DMUB_WAIT_TYPE_WAIT); > > } > > } > > + > > +bool dc_dmub_check_min_version(struct dmub_srv *srv) > > +{ > > + if (!srv->hw_funcs.is_psrsu_supported) > > + return true; > > + return srv->hw_funcs.is_psrsu_supported(srv); > > +} > > diff --git a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.h > b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.h > > index a5196a9292b3..099f94b6107c 100644 > > --- a/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.h > > +++ b/drivers/gpu/drm/amd/display/dc/dc_dmub_srv.h > > @@ -86,4 +86,5 @@ void dc_dmub_setup_subvp_dmub_command(struct > dc *dc, struct dc_state *context, b > > void dc_dmub_srv_log_diagnostic_data(struct dc_dmub_srv > *dc_dmub_srv); > > > > void dc_send_update_cursor_info_to_dmu(struct pipe_ctx *pCtx, uint8_t > pipe_idx); > > +bool dc_dmub_check_min_version(struct dmub_srv *srv); > > #endif /* _DMUB_DC_SRV_H_ */ > > diff --git a/drivers/gpu/drm/amd/display/dmub/dmub_srv.h > b/drivers/gpu/drm/amd/display/dmub/dmub_srv.h > > index 2a66a305679a..4585e0419da6 100644 > > --- a/drivers/gpu/drm/amd/display/dmub/dmub_srv.h > > +++ b/drivers/gpu/drm/amd/display/dmub/dmub_srv.h > > @@ -367,6 +367,8 @@ struct dmub_srv_hw_funcs { > > > > bool (*is_supported)(struct dmub_srv *dmub); > > > > + bool (*is_psrsu_supported)(struct dmub_srv *dmub); > > + > > bool (*is_hw_init)(struct dmub_srv *dmub); > > > > void (*enable_dmub_boot_options)(struct dmub_srv *dmub, > > diff --git a/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn31.c > b/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn31.c > > index ebf7aeec4029..c8445d474107 100644 > > --- a/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn31.c > > +++ b/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn31.c > > @@ -302,6 +302,11 @@ bool dmub_dcn31_is_supported(struct dmub_srv > *dmub) > > return supported; > > } > > > > +bool dmub_dcn31_is_psrsu_supported(struct dmub_srv *dmub) > > +{ > > + return dmub->fw_version >= DMUB_FW_VERSION(4, 0, 58); > > +} > > + > > void dmub_dcn31_set_gpint(struct dmub_srv *dmub, > > union dmub_gpint_data_register reg) > > { > > diff --git a/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn31.h > b/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn31.h > > index 7d5c10ee539b..89c5a948b67d 100644 > > --- a/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn31.h > > +++ b/drivers/gpu/drm/amd/display/dmub/src/dmub_dcn31.h > > @@ -221,6 +221,8 @@ bool dmub_dcn31_is_hw_init(struct dmub_srv > *dmub); > > > > bool dmub_dcn31_is_supported(struct dmub_srv *dmub); > > > > +bool dmub_dcn31_is_psrsu_supported(struct dmub_srv *dmub); > > + > > void dmub_dcn31_set_gpint(struct dmub_srv *dmub, > > union dmub_gpint_data_register reg); > > > > diff --git a/drivers/gpu/drm/amd/display/dmub/src/dmub_srv.c > b/drivers/gpu/drm/amd/display/dmub/src/dmub_srv.c > > index 9e9a6a44a7ac..fd5b043dd32d 100644 > > --- a/drivers/gpu/drm/amd/display/dmub/src/dmub_srv.c > > +++ b/drivers/gpu/drm/amd/display/dmub/src/dmub_srv.c > > @@ -226,14 +226,16 @@ static bool dmub_srv_hw_setup(struct > dmub_srv *dmub, enum dmub_asic asic) > > case DMUB_ASIC_DCN314: > > case DMUB_ASIC_DCN315: > > case DMUB_ASIC_DCN316: > > - if (asic == DMUB_ASIC_DCN314) > > + if (asic == DMUB_ASIC_DCN314) { > > dmub->regs_dcn31 = &dmub_srv_dcn314_regs; > > - else if (asic == DMUB_ASIC_DCN315) > > + funcs->is_psrsu_supported = > dmub_dcn314_is_psrsu_supported; > > + } else if (asic == DMUB_ASIC_DCN315) { > > dmub->regs_dcn31 = &dmub_srv_dcn315_regs; > > - else if (asic == DMUB_ASIC_DCN316) > > + } else if (asic == DMUB_ASIC_DCN316) { > > dmub->regs_dcn31 = &dmub_srv_dcn316_regs; > > - else > > + } else { > > dmub->regs_dcn31 = &dmub_srv_dcn31_regs; > > + } > > Should these hunks be rolled into 3/4? dmub_dcn314_is_psrsu_supported is > defined there. > Ah yup; I mixed the two up when I split the patch in half. Will fix, thanks. > Thanks, > Leo > > funcs->reset = dmub_dcn31_reset; > > funcs->reset_release = dmub_dcn31_reset_release; > > funcs->backdoor_load = dmub_dcn31_backdoor_load;
