[PATCH v2] drm/amdgpu: Need to set the baco cap before baco reset

Alex Deucher alexdeucher at gmail.com
Thu May 23 03:54:21 UTC 2019


On Wed, May 22, 2019 at 11:48 PM Alex Deucher <alexdeucher at gmail.com> wrote:
>
> On Wed, May 22, 2019 at 11:27 PM Emily Deng <Emily.Deng at amd.com> wrote:
> >
>
> Please include a patch description.
>
> > Signed-off-by: Emily Deng <Emily.Deng at amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c         | 10 +++++++++-
> >  drivers/gpu/drm/amd/include/kgd_pp_interface.h     |  1 +
> >  drivers/gpu/drm/amd/powerplay/amd_powerplay.c      | 16 ++++++++++++++++
> >  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  1 +
> >  .../amd/powerplay/hwmgr/vega10_processpptables.c   | 22 ++++++++++++++++++++++
> >  .../amd/powerplay/hwmgr/vega10_processpptables.h   |  1 +
> >  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h          |  1 +
> >  7 files changed, 51 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index d6286ed..5288763 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -2605,7 +2605,15 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> >         /* check if we need to reset the asic
> >          *  E.g., driver was not cleanly unloaded previously, etc.
> >          */
> > -       if (!amdgpu_sriov_vf(adev) && amdgpu_asic_need_reset_on_init(adev)) {
> > +       if (amdgpu_passthrough(adev) && amdgpu_asic_need_reset_on_init(adev)) {
>
> This will change the current behavior on baremetal.
>
> > +               if (adev->powerplay.pp_funcs && adev->powerplay.pp_funcs->set_asic_baco_cap) {
> > +                       r = adev->powerplay.pp_funcs->set_asic_baco_cap(adev->powerplay.pp_handle);
> > +                       if (r) {
> > +                               dev_err(adev->dev, "set baco capability failed\n");
> > +                               goto failed;
> > +                       }
> > +               }
> > +
>
> This will also change the current behavior on bare metal.
>
> I think you may want to rework this a bit otherwise this change won't
> really make any difference due to this patch:
> https://cgit.freedesktop.org/~agd5f/linux/commit/?h=amd-staging-drm-next&id=60ae2cd5aec94dc6459bdee5c610bb5c76a1d0ae
> We need to avoid breaking module reload (modeprobe amdgpu; modprobe -r
> amdgpu; modeprobe amdgpu).
> I think it would be cleaner to call set_asic_baco_cap() in
> hwmgr_early_init() and then return true in soc15_need_reset_on_init()
> is it's passthrough mode.  then everything should just work as is.

Assuming you set the set_asic_baco_cap callbacks for vega20 and vega12
as well. Otherwise, you'll have to limit it to vega10 which starts to
get messy IMHO.

Alex

>
> >                 r = amdgpu_asic_reset(adev);
> >                 if (r) {
> >                         dev_err(adev->dev, "asic reset on init failed\n");
> > diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> > index 2b579ba..0dcc18d 100644
> > --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> > +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> > @@ -285,6 +285,7 @@ struct amd_pm_funcs {
> >         int (*set_hard_min_fclk_by_freq)(void *handle, uint32_t clock);
> >         int (*set_min_deep_sleep_dcefclk)(void *handle, uint32_t clock);
> >         int (*get_asic_baco_capability)(void *handle, bool *cap);
> > +       int (*set_asic_baco_cap)(void *handle);
> >         int (*get_asic_baco_state)(void *handle, int *state);
> >         int (*set_asic_baco_state)(void *handle, int state);
> >         int (*get_ppfeature_status)(void *handle, char *buf);
> > diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > index bea1587..9856760 100644
> > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > @@ -1404,6 +1404,21 @@ static int pp_set_active_display_count(void *handle, uint32_t count)
> >         return ret;
> >  }
> >
> > +static int pp_set_asic_baco_cap(void *handle)
> > +{
> > +       struct pp_hwmgr *hwmgr = handle;
> > +
> > +       if (!hwmgr)
> > +               return -EINVAL;
> > +
> > +       if (!hwmgr->pm_en || !hwmgr->hwmgr_func->set_asic_baco_cap)
> > +               return 0;
> > +
> > +       hwmgr->hwmgr_func->set_asic_baco_cap(hwmgr);
> > +
> > +       return 0;
> > +}
> > +
> >  static int pp_get_asic_baco_capability(void *handle, bool *cap)
> >  {
> >         struct pp_hwmgr *hwmgr = handle;
> > @@ -1546,6 +1561,7 @@ static const struct amd_pm_funcs pp_dpm_funcs = {
> >         .set_hard_min_dcefclk_by_freq = pp_set_hard_min_dcefclk_by_freq,
> >         .set_hard_min_fclk_by_freq = pp_set_hard_min_fclk_by_freq,
> >         .get_asic_baco_capability = pp_get_asic_baco_capability,
> > +       .set_asic_baco_cap = pp_set_asic_baco_cap,
> >         .get_asic_baco_state = pp_get_asic_baco_state,
> >         .set_asic_baco_state = pp_set_asic_baco_state,
> >         .get_ppfeature_status = pp_get_ppfeature_status,
> > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> > index ed6c638..8dc23eb 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> > @@ -5171,6 +5171,7 @@ static const struct pp_hwmgr_func vega10_hwmgr_funcs = {
> >         .odn_edit_dpm_table = vega10_odn_edit_dpm_table,
> >         .get_performance_level = vega10_get_performance_level,
> >         .get_asic_baco_capability = smu9_baco_get_capability,
> > +       .set_asic_baco_cap = vega10_baco_set_cap,
>
> Any chance you can add callbacks for vega12 and 20 as well?
>
> >         .get_asic_baco_state = smu9_baco_get_state,
> >         .set_asic_baco_state = vega10_baco_set_state,
> >         .enable_mgpu_fan_boost = vega10_enable_mgpu_fan_boost,
> > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c
> > index b6767d7..8fdeb23 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.c
> > @@ -1371,3 +1371,25 @@ int vega10_get_powerplay_table_entry(struct pp_hwmgr *hwmgr,
> >
> >         return result;
> >  }
> > +
> > +int vega10_baco_set_cap(struct pp_hwmgr *hwmgr)
> > +{
> > +       int result = 0;
> > +       const ATOM_Vega10_POWERPLAYTABLE *powerplay_table;
> > +
> > +       powerplay_table = get_powerplay_table(hwmgr);
> > +
> > +       PP_ASSERT_WITH_CODE((powerplay_table != NULL),
> > +               "Missing PowerPlay Table!", return -1);
> > +
> > +       result = check_powerplay_tables(hwmgr, powerplay_table);
> > +
> > +       PP_ASSERT_WITH_CODE((result == 0),
> > +                           "check_powerplay_tables failed", return result);
> > +
> > +       set_hw_cap(
> > +                       hwmgr,
> > +                       0 != (le32_to_cpu(powerplay_table->ulPlatformCaps) & ATOM_VEGA10_PP_PLATFORM_CAP_BACO),
> > +                       PHM_PlatformCaps_BACO);
> > +       return result;
> > +}
> > \ No newline at end of file
>
> Missing newline.
>
> Alex
>
> > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.h b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.h
> > index d83ed2a..da5fbec 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.h
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_processpptables.h
> > @@ -59,4 +59,5 @@ extern int vega10_get_number_of_powerplay_table_entries(struct pp_hwmgr *hwmgr);
> >  extern int vega10_get_powerplay_table_entry(struct pp_hwmgr *hwmgr, uint32_t entry_index,
> >                 struct pp_power_state *power_state, int (*call_back_func)(struct pp_hwmgr *, void *,
> >                                 struct pp_power_state *, void *, uint32_t));
> > +extern int vega10_baco_set_cap(struct pp_hwmgr *hwmgr);
> >  #endif
> > diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > index bac3d85..14480ae 100644
> > --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > @@ -339,6 +339,7 @@ struct pp_hwmgr_func {
> >         int (*set_hard_min_dcefclk_by_freq)(struct pp_hwmgr *hwmgr, uint32_t clock);
> >         int (*set_hard_min_fclk_by_freq)(struct pp_hwmgr *hwmgr, uint32_t clock);
> >         int (*get_asic_baco_capability)(struct pp_hwmgr *hwmgr, bool *cap);
> > +       int (*set_asic_baco_cap)(struct pp_hwmgr *hwmgr);
> >         int (*get_asic_baco_state)(struct pp_hwmgr *hwmgr, enum BACO_STATE *state);
> >         int (*set_asic_baco_state)(struct pp_hwmgr *hwmgr, enum BACO_STATE state);
> >         int (*get_ppfeature_status)(struct pp_hwmgr *hwmgr, char *buf);
> > --
> > 2.7.4
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx


More information about the amd-gfx mailing list