[PATCH v2 2/2] drm/amdgpu/pm: Use macro definitions in the smu IH process function

Ma, Jun majun at amd.com
Thu Jan 25 07:45:16 UTC 2024


Hi Kevin,

On 1/23/2024 8:08 PM, Wang, Yang(Kevin) wrote:
> [AMD Official Use Only - General]
> 
> HI Jun,
> 
> I don't think it's necessary to delete these definitions in smu driver_if.h.
> Adding a prefix can be used to distinguish definitions in the driver and can also make it easier for us to track problems. E.g: SMU_IH_INTERRUPT_ID_TO_DRIVER
> And definitions in the smu_cmn.h file will cause definition conflicts in all subsequent driver if files, we try to avoid modifying the driver_if file and have kept synchronization with pmfw.
> 
In general, we do need to follow the above rules.
But smu_v13_0_irq_process() is a common function used by other asics.
Defining these macros in the corresponding smu driver_if.h will cause the
compile error.

Regards,
Ma Jun
> Best Regards,
> Kevin
> 
> -----Original Message-----
> From: Ma, Jun <Jun.Ma2 at amd.com>
> Sent: Tuesday, January 23, 2024 4:13 PM
> To: amd-gfx at lists.freedesktop.org
> Cc: Lazar, Lijo <Lijo.Lazar at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>; Deucher, Alexander <Alexander.Deucher at amd.com>; Wang, Yang(Kevin) <KevinYang.Wang at amd.com>; Ma, Jun <Jun.Ma2 at amd.com>
> Subject: [PATCH v2 2/2] drm/amdgpu/pm: Use macro definitions in the smu IH process function
> 
> Replace the hard-coded numbers with macro definition
> 
> Signed-off-by: Ma Jun <Jun.Ma2 at amd.com>
> ---
>  .../pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h | 11 -----------  .../pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_6.h |  4 ----  .../pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h | 11 -----------
>  drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c     | 10 +++++-----
>  drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c     | 14 +++++++-------
>  drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c     |  2 +-
>  drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h             | 10 ++++++++++
>  7 files changed, 23 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h
> index b114d14fc053..91229b2dadb5 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h
> @@ -1618,15 +1618,4 @@ typedef struct {
>  #define TABLE_WIFIBAND                12
>  #define TABLE_COUNT                   13
> 
> -//IH Interupt ID
> -#define IH_INTERRUPT_ID_TO_DRIVER                   0xFE
> -#define IH_INTERRUPT_CONTEXT_ID_BACO                0x2
> -#define IH_INTERRUPT_CONTEXT_ID_AC                  0x3
> -#define IH_INTERRUPT_CONTEXT_ID_DC                  0x4
> -#define IH_INTERRUPT_CONTEXT_ID_AUDIO_D0            0x5
> -#define IH_INTERRUPT_CONTEXT_ID_AUDIO_D3            0x6
> -#define IH_INTERRUPT_CONTEXT_ID_THERMAL_THROTTLING  0x7
> -#define IH_INTERRUPT_CONTEXT_ID_FAN_ABNORMAL        0x8
> -#define IH_INTERRUPT_CONTEXT_ID_FAN_RECOVERY        0x9
> -
>  #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_6.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_6.h
> index ced348d2e8bb..957b177414a9 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_6.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_6.h
> @@ -209,10 +209,6 @@ typedef struct {
>    float    minPsmVoltage[30];
>  } AvfsDebugTableXcd_t;
> 
> -// Defines used for IH-based thermal interrupts to GFX driver - A/X only
> -#define IH_INTERRUPT_ID_TO_DRIVER                   0xFE
> -#define IH_INTERRUPT_CONTEXT_ID_THERMAL_THROTTLING  0x7
> -
>  //thermal over-temp mask defines for IH interrupt to host
>  #define THROTTLER_PROCHOT_BIT           0
>  #define THROTTLER_PPT_BIT               1
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h
> index 8b1496f8ce58..33937c1d984f 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h
> @@ -1608,15 +1608,4 @@ typedef struct {
>  #define TABLE_WIFIBAND                12
>  #define TABLE_COUNT                   13
> 
> -//IH Interupt ID
> -#define IH_INTERRUPT_ID_TO_DRIVER                   0xFE
> -#define IH_INTERRUPT_CONTEXT_ID_BACO                0x2
> -#define IH_INTERRUPT_CONTEXT_ID_AC                  0x3
> -#define IH_INTERRUPT_CONTEXT_ID_DC                  0x4
> -#define IH_INTERRUPT_CONTEXT_ID_AUDIO_D0            0x5
> -#define IH_INTERRUPT_CONTEXT_ID_AUDIO_D3            0x6
> -#define IH_INTERRUPT_CONTEXT_ID_THERMAL_THROTTLING  0x7
> -#define IH_INTERRUPT_CONTEXT_ID_FAN_ABNORMAL        0x8
> -#define IH_INTERRUPT_CONTEXT_ID_FAN_RECOVERY        0x9
> -
>  #endif
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> index fbeb31bf9e48..ddf435cdddfa 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/smu_v11_0.c
> @@ -1432,24 +1432,24 @@ static int smu_v11_0_irq_process(struct amdgpu_device *adev,
>                 dev_emerg(adev->dev, "ERROR: System is going to shutdown due to GPU HW CTF!\n");
>                 orderly_poweroff(true);
>         } else if (client_id == SOC15_IH_CLIENTID_MP1) {
> -               if (src_id == 0xfe) {
> +               if (src_id == IH_INTERRUPT_ID_TO_DRIVER) {
>                         /* ACK SMUToHost interrupt */
>                         data = RREG32_SOC15(MP1, 0, mmMP1_SMN_IH_SW_INT_CTRL);
>                         data = REG_SET_FIELD(data, MP1_SMN_IH_SW_INT_CTRL, INT_ACK, 1);
>                         WREG32_SOC15(MP1, 0, mmMP1_SMN_IH_SW_INT_CTRL, data);
> 
>                         switch (ctxid) {
> -                       case 0x3:
> +                       case IH_INTERRUPT_CONTEXT_ID_AC:
>                                 dev_dbg(adev->dev, "Switched to AC mode!\n");
>                                 schedule_work(&smu->interrupt_work);
>                                 adev->pm.ac_power = true;
>                                 break;
> -                       case 0x4:
> +                       case IH_INTERRUPT_CONTEXT_ID_DC:
>                                 dev_dbg(adev->dev, "Switched to DC mode!\n");
>                                 schedule_work(&smu->interrupt_work);
>                                 adev->pm.ac_power = false;
>                                 break;
> -                       case 0x7:
> +                       case IH_INTERRUPT_CONTEXT_ID_THERMAL_THROTTLING:
>                                 /*
>                                  * Increment the throttle interrupt counter
>                                  */
> @@ -1508,7 +1508,7 @@ int smu_v11_0_register_irq_handler(struct smu_context *smu)
>                 return ret;
> 
>         ret = amdgpu_irq_add_id(adev, SOC15_IH_CLIENTID_MP1,
> -                               0xfe,
> +                               IH_INTERRUPT_ID_TO_DRIVER,
>                                 irq_src);
>         if (ret)
>                 return ret;
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> index 1db74c0b5257..9277c5dff5e4 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c
> @@ -1368,24 +1368,24 @@ static int smu_v13_0_irq_process(struct amdgpu_device *adev,
>                 dev_emerg(adev->dev, "ERROR: System is going to shutdown due to GPU HW CTF!\n");
>                 orderly_poweroff(true);
>         } else if (client_id == SOC15_IH_CLIENTID_MP1) {
> -               if (src_id == 0xfe) {
> +               if (src_id == IH_INTERRUPT_ID_TO_DRIVER) {
>                         /* ACK SMUToHost interrupt */
>                         data = RREG32_SOC15(MP1, 0, regMP1_SMN_IH_SW_INT_CTRL);
>                         data = REG_SET_FIELD(data, MP1_SMN_IH_SW_INT_CTRL, INT_ACK, 1);
>                         WREG32_SOC15(MP1, 0, regMP1_SMN_IH_SW_INT_CTRL, data);
> 
>                         switch (ctxid) {
> -                       case 0x3:
> +                       case IH_INTERRUPT_CONTEXT_ID_AC:
>                                 dev_dbg(adev->dev, "Switched to AC mode!\n");
>                                 smu_v13_0_ack_ac_dc_interrupt(smu);
>                                 adev->pm.ac_power = true;
>                                 break;
> -                       case 0x4:
> +                       case IH_INTERRUPT_CONTEXT_ID_DC:
>                                 dev_dbg(adev->dev, "Switched to DC mode!\n");
>                                 smu_v13_0_ack_ac_dc_interrupt(smu);
>                                 adev->pm.ac_power = false;
>                                 break;
> -                       case 0x7:
> +                       case IH_INTERRUPT_CONTEXT_ID_THERMAL_THROTTLING:
>                                 /*
>                                  * Increment the throttle interrupt counter
>                                  */
> @@ -1398,7 +1398,7 @@ static int smu_v13_0_irq_process(struct amdgpu_device *adev,
>                                         schedule_work(&smu->throttling_logging_work);
> 
>                                 break;
> -                       case 0x8:
> +                       case IH_INTERRUPT_CONTEXT_ID_FAN_ABNORMAL:
>                                 high = smu->thermal_range.software_shutdown_temp +
>                                         smu->thermal_range.software_shutdown_temp_offset;
>                                 high = min_t(typeof(high),
> @@ -1415,7 +1415,7 @@ static int smu_v13_0_irq_process(struct amdgpu_device *adev,
>                                 data = data & (~THM_THERMAL_INT_CTRL__THERM_TRIGGER_MASK_MASK);
>                                 WREG32_SOC15(THM, 0, regTHM_THERMAL_INT_CTRL, data);
>                                 break;
> -                       case 0x9:
> +                       case IH_INTERRUPT_CONTEXT_ID_FAN_RECOVERY:
>                                 high = min_t(typeof(high),
>                                              SMU_THERMAL_MAXIMUM_ALERT_TEMP,
>                                              smu->thermal_range.software_shutdown_temp);
> @@ -1476,7 +1476,7 @@ int smu_v13_0_register_irq_handler(struct smu_context *smu)
>                 return ret;
> 
>         ret = amdgpu_irq_add_id(adev, SOC15_IH_CLIENTID_MP1,
> -                               0xfe,
> +                               IH_INTERRUPT_ID_TO_DRIVER,
>                                 irq_src);
>         if (ret)
>                 return ret;
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
> index 4894f7ee737b..9a8b7fd6995d 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c
> @@ -892,7 +892,7 @@ int smu_v14_0_register_irq_handler(struct smu_context *smu)
>         // TODO: THM related
> 
>         ret = amdgpu_irq_add_id(adev, SOC15_IH_CLIENTID_MP1,
> -                               0xfe,
> +                               IH_INTERRUPT_ID_TO_DRIVER,
>                                 irq_src);
>         if (ret)
>                 return ret;
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> index cc590e27d88a..9d5d1afe4137 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> @@ -30,6 +30,16 @@
>  #define FDO_PWM_MODE_STATIC  1
>  #define FDO_PWM_MODE_STATIC_RPM 5
> 
> +#define IH_INTERRUPT_ID_TO_DRIVER                   0xFE
> +#define IH_INTERRUPT_CONTEXT_ID_BACO                0x2
> +#define IH_INTERRUPT_CONTEXT_ID_AC                  0x3
> +#define IH_INTERRUPT_CONTEXT_ID_DC                  0x4
> +#define IH_INTERRUPT_CONTEXT_ID_AUDIO_D0            0x5
> +#define IH_INTERRUPT_CONTEXT_ID_AUDIO_D3            0x6
> +#define IH_INTERRUPT_CONTEXT_ID_THERMAL_THROTTLING  0x7
> +#define IH_INTERRUPT_CONTEXT_ID_FAN_ABNORMAL        0x8
> +#define IH_INTERRUPT_CONTEXT_ID_FAN_RECOVERY        0x9
> +
>  extern const int link_speed[];
> 
>  /* Helper to Convert from PCIE Gen 1/2/3/4/5/6 to 0.1 GT/s speed units */
> --
> 2.34.1
> 


More information about the amd-gfx mailing list