[PATCH 2/4] drm/amd/display: Set minimum requirement for using PSR-SU on Rembrandt
Limonciello, Mario
Mario.Limonciello at amd.com
Fri Jun 23 19:32:07 UTC 2023
[AMD Official Use Only - General]
> -----Original Message-----
> From: Li, Sun peng (Leo) <Sunpeng.Li at amd.com>
> Sent: Friday, June 23, 2023 2:27 PM
> To: Limonciello, Mario <Mario.Limonciello at amd.com>; amd-
> gfx at lists.freedesktop.org
> Cc: Lin, Tsung-hua (Ryan) <Tsung-hua.Lin at amd.com>; Rossi, Marc
> <Marc.Rossi at amd.com>; Wang, Sean <sean.ns.wang at amd.com>; Mahfooz,
> Hamza <Hamza.Mahfooz at amd.com>
> 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 at amd.com>
> > Cc: Marc Rossi <Marc.Rossi at amd.com>
> > Cc: Hamza Mahfooz <Hamza.Mahfooz at amd.com>
> > Cc: Tsung-hua (Ryan) Lin <Tsung-hua.Lin at amd.com>
> > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2443
> > Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> > ---
> > 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;
More information about the amd-gfx
mailing list