Re: [PATCH 3/3] drm/amd/pp: Add gfx power status check support on Rv/Vega12

[Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 7195 bytes --]

If you use them enough for debugging that you think it's better to have
them in tree, feel free to commit them.  Maybe just add a note that they
are just for debugging and add a note about the race condition.

Alex


On Thu, Aug 2, 2018 at 6:22 AM, Zhu, Rex <Rex.Zhu-5C7GfCeVMHo@public.gmane.org> wrote:

> Currently, there is no use case except for debug.
>
> So I will keep the patches in my local.
>
> Thanks.
>
>
>
> Best Regards
>
> Rex
>
>
>
>
> ------------------------------
> *From:* Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> *Sent:* Wednesday, August 1, 2018 11:13 PM
> *To:* Zhu, Rex
> *Cc:* Deucher, Alexander; Quan, Evan; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>
> *Subject:* Re: [PATCH 3/3] drm/amd/pp: Add gfx power status check support
> on Rv/Vega12
>
> Does anything use it?  Might be useful for debugging, but I guess you'd
> probably use something like umr in that case.
>
> Alex
>
> On Tue, Jul 31, 2018 at 9:15 PM, Zhu, Rex <Rex.Zhu-5C7GfCeVMHo@public.gmane.org> wrote:
>
> Hi Alex,
>
>
> Is it necessary to export an interface as "is_gfx_on" in powerplay to
> amdgpu?
>
> It can show the current power state of gfx ip.
>
>
>
> Best Regards
>
> Rex
>
>
> ------------------------------
> *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Zhu,
> Rex <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> *Sent:* Monday, July 30, 2018 11:39 AM
> *To:* Quan, Evan; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>
> *Subject:* Re: [PATCH 3/3] drm/amd/pp: Add gfx power status check support
> on Rv/Vega12
>
>
> > It means user was told the gfx is on but actually it may switches to
> gfxoff already.
>
>
> Correct.
>
> So if the gfx is off, user should disable gfx off feature before they
> read/write the registers via mmio.
>
> if gfx is on, that is not mean the gfx off feature is disabled. it
> doesn't mean it is safe to visit the registers through mmio.
>
>
> Best Regards
>
> Rex
>
> ------------------------------
> *From:* Quan, Evan
> *Sent:* Monday, July 30, 2018 9:31 AM
> *To:* Zhu, Rex; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Subject:* Re: [PATCH 3/3] drm/amd/pp: Add gfx power status check support
> on Rv/Vega12
>
>
> Since the gfxoff is always in dynamic switch, i do not think the status
> reported to user is reliable. It means user was told the gfx is on but
> actually it may switches to gfxoff already.
>
>
> Regards,
>
> Evan
>
>
> ------------------------------
> *From:* amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Rex
> Zhu <rex.zhu-5C7GfCeVMHo@public.gmane.org>
> *Sent:* Sunday, July 29, 2018 7:42:27 PM
> *To:* amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> *Cc:* Zhu, Rex
> *Subject:* [PATCH 3/3] drm/amd/pp: Add gfx power status check support on
> Rv/Vega12
>
> As gfx off is supported on Rv/Vega12, so this check is
> helpful and necessary when visit gfx regesiter via mmio.
>
> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c  | 10 +++++++
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c | 31
> +++++++++++++++++++++-
>  2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> index 26d130a..da1be82 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> @@ -296,6 +296,15 @@ static bool smu10_is_gfx_on(struct pp_hwmgr *hwmgr)
>          return false;
>  }
>
> +static bool smu10_is_hw_ip_on(struct pp_hwmgr *hwmgr,
> +                               enum amd_ip_block_type client)
> +{
> +       if (client == AMD_IP_BLOCK_TYPE_GFX)
> +               return smu10_is_gfx_on(hwmgr);
> +       else /* for other ip, to do */
> +               return true;
> +}
> +
>  static int smu10_disable_gfx_off(struct pp_hwmgr *hwmgr)
>  {
>          struct smu10_hwmgr *smu10_data = (struct smu10_hwmgr
> *)(hwmgr->backend);
> @@ -1187,6 +1196,7 @@ static void smu10_powergate_vcn(struct pp_hwmgr
> *hwmgr, bool bgate)
>          .smus_notify_pwe = smu10_smus_notify_pwe,
>          .display_clock_voltage_request = smu10_display_clock_voltage_re
> quest,
>          .powergate_gfx = smu10_gfx_off_control,
> +       .is_hw_ip_on = smu10_is_hw_ip_on,
>  };
>
>  int smu10_init_function_pointers(struct pp_hwmgr *hwmgr)
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> index 0789d64..ee44300 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> @@ -45,7 +45,7 @@
>  #include "ppinterrupt.h"
>  #include "pp_overdriver.h"
>  #include "pp_thermal.h"
> -
> +#include "soc15_common.h"
>
>  static int vega12_force_clock_level(struct pp_hwmgr *hwmgr,
>                  enum pp_clock_type type, uint32_t mask);
> @@ -2320,6 +2320,34 @@ static int vega12_get_thermal_temperature_range(struct
> pp_hwmgr *hwmgr,
>          return 0;
>  }
>
> +/* GFX HW Power Status can be queried from bits [1:0] of
> MP1_SMN_EXT_SCRATCH0
> + * 2'b00   GFX is OFF
> + * 2'b01   Transitioning out of GFXOFF state
> + * 2'b10   GFX is ON
> + * 2'b11   Transitioning into GFXOFF state
> +*/
> +
> +static bool vega12_is_gfx_on(struct pp_hwmgr *hwmgr)
> +{
> +       uint32_t tmp;
> +       struct amdgpu_device *adev = hwmgr->adev;
> +
> +       tmp = RREG32_SOC15(MP1, 0, mmMP1_SMN_EXT_SCRATCH0);
> +       if (tmp == 1 || tmp == 2)
> +               return true;
> +       else
> +               return false;
> +}
> +
> +static bool vega12_is_hw_ip_on(struct pp_hwmgr *hwmgr,
> +                               enum amd_ip_block_type client)
> +{
> +       if (client == AMD_IP_BLOCK_TYPE_GFX)
> +               return vega12_is_gfx_on(hwmgr);
> +       else /* for other ip, to do */
> +               return true;
> +}
> +
>  static int vega12_enable_gfx_off(struct pp_hwmgr *hwmgr)
>  {
>          struct vega12_hwmgr *data =
> @@ -2402,6 +2430,7 @@ static int vega12_gfx_off_control(struct pp_hwmgr
> *hwmgr, bool enable)
>          .register_irq_handlers = smu9_register_irq_handlers,
>          .start_thermal_controller = vega12_start_thermal_controller,
>          .powergate_gfx = vega12_gfx_off_control,
> +       .is_hw_ip_on = vega12_is_hw_ip_on,
>  };
>
>  int vega12_hwmgr_init(struct pp_hwmgr *hwmgr)
> --
> 1.9.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> amd-gfx Info Page - freedesktop.org
> <https://lists.freedesktop.org/mailman/listinfo/amd-gfx>
> lists.freedesktop.org
> Subscribing to amd-gfx: Subscribe to amd-gfx by filling out the following
> form. Use of all freedesktop.org lists is subject to our Code of Conduct.
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 16964 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx