Re: [PATCH 00/32] Separating vcn power management by instance

[Boyuan Zhang <Boyuan.Zhang@amd.com>]

On 2024-10-22 02:25, Christian König wrote:
> Patches #1-#5, #7, #8, #32 are Acked-by: Christian König 
> <christian.koenig@amd.com>


Added Acked-by, and ready to be submitted.


>
> Patches #9 - #19, #27 are Reviewed-by: Christian König 
> <christian.koenig@amd.com>


Added Reviewed-by, and ready to be submitted.


>
> Patch #6 the drm/amdgpu prefix is missing from the subject line, apart 
> from that the patch is Reviewed-by: Christian König 
> <christian.koenig@amd.com>


Fixed in new patch-set just submitted (patch 06/29), and added Reviewed-by.


>
> For patches #20-#26 I'm not sure if those won't break the driver in 
> between. Alex what do you think?


Need Alex's input.


>
> Patches #28 and #29 look good to me as well, but I leave the review to 
> Sunil he wrote that code and should know it best.


Patches #28 and #29 has been dropped in the new patch set based on 
discussion with Sunil, will be submitted later on once his work is done.


>
> Patch #30:
>
> +    int ret = 0;
>
> ...
> +    ret = SOC15_WAIT_ON_RREG(VCN, inst, mmUVD_STATUS, UVD_STATUS__IDLE,
>
>
> That will get you a warning for an unneeded local variable 
> initialization from the automated checkers.
>
> The init was only necessary because we previously had the loop over 
> all instances here.


Fixed in new patch-set just submitted (patch 28/29)


>
> Patch #31:
>
> int inst = ip_block->instance;
> int ret = 1;
>
> if (adev->vcn.harvest_config & (1 << inst))
>     return ret;
>
> ret &= (RREG32_SOC15(VCN, inst, mmUVD_STATUS) == UVD_STATUS__IDLE);
>
> return ret;
>
> That code looks really strange now, maybe drop the local variable ret.


Patch #31 has been dropped in the new patch set based on discussion with 
Sunil, will be submitted later on once his work is done.


>
> Regards,
> Christian
>
> Am 17.10.24 um 15:20 schrieb boyuan.zhang@amd.com:
>> From: Boyuan Zhang <boyuan.zhang@amd.com>
>>
>> Previously, all vcn instance will be powered on/off at the same time
>> even only one of the instance requests power status change. This 
>> patch set
>> enables vcn to ONLY power on/off the instance that requires power status
>> change. Other vcn instances will remain the original power status.
>>
>> v4:
>> code polishing and minor fixes.
>>
>> v3:
>> move all of the per instance variables from struct amdgpu_vcn to
>> struct amdgpu_vcn_inst. (patch 10 - 11)
>>
>> update amdgpu_device_ip_set_powergating_state() to take the instance 
>> as a
>> new parameter, remove the duplicated function in v2. (patch 19)
>>
>> update all amdgpu_vcn_* helpers to handle vcn instance. All functions
>> are now only handle the given vcn instance. (patch 20 - 26)
>>
>> update all vcn ip callback functions to handle vcn instance. All 
>> functions
>> are now only handle the given vcn instance. (patch 27 - 32)
>>
>>
>> v2:
>> complete re-work for all PM changes as suggested-by Christian König and
>> Alex Deucher. Adding instance to all existing functions, instead of 
>> create
>> new functions. Remove all duplicated PM functions in previous patch set.
>> Use a new logic to track instance for ip_block with same type as
>> suggested by Alex. Also, fix wrong ip block index and remove 
>> redundant logic
>> suggested by Christian. Finally rebase all patches based on Sunil's 
>> ip block
>> changes.
>>
>> Patch 1-6 are SMU changes to only power ON/OFF given VCN instance.
>>
>> Patch 7-8 pass ip_block instead of adev pointer for 
>> set_powergating_state,
>> set_clockgating_state, and is_idle
>>
>> Patch 9 is to track VCN instance in VCN ip_block.
>>
>> Patch 10 move all of the per instance variables from struct 
>> amdgpu_vcn to
>> struct amdgpu_vcn_inst.
>>
>> Patch 11  VCN change to separate gating status for each VCN instance.
>>
>> Patch 12-17 are to handle ip callback functions separately for each
>> VCN instance, so that only the given instance will be powered on/off.
>>
>> Patch 18 is VCN change to handle idle work separately for each VCN 
>> instance.
>>
>> Patch 19 is to set powergating state by VCN instance in amdgpu_vcn.
>>
>> Patch 20-26 update all amdgpu_vcn_* helpers to handle vcn instance. 
>> All functions
>> are now only handle the given vcn instance.
>>
>> Patch 27-32 update all vcn ip callback functions to handle vcn 
>> instance. All functions
>> are now only handle the given vcn instance.
>>
>> Boyuan Zhang (32):
>>    drm/amd/pm: add inst to dpm_set_vcn_enable
>>    drm/amd/pm: power up or down vcn by instance
>>    drm/amd/pm: add inst to smu_dpm_set_vcn_enable
>>    drm/amd/pm: add inst to set_powergating_by_smu
>>    drm/amd/pm: add inst to dpm_set_powergating_by_smu
>>    add inst to amdgpu_dpm_enable_vcn
>>    drm/amdgpu: pass ip_block in set_powergating_state
>>    drm/amdgpu: pass ip_block in set_clockgating_state
>>    drm/amdgpu: track instances of the same IP block
>>    drm/amdgpu: move per inst variables to amdgpu_vcn_inst
>>    drm/amdgpu/vcn: separate gating state by instance
>>    drm/amdgpu: power vcn 2_5 by instance
>>    drm/amdgpu: power vcn 3_0 by instance
>>    drm/amdgpu: power vcn 4_0 by instance
>>    drm/amdgpu: power vcn 4_0_3 by instance
>>    drm/amdgpu: power vcn 4_0_5 by instance
>>    drm/amdgpu: power vcn 5_0_0 by instance
>>    drm/amdgpu/vcn: separate idle work by instance
>>    drm/amdgpu: set powergating state by vcn instance
>>    drm/amdgpu: early_init for each vcn instance
>>    drm/amdgpu: sw_init for each vcn instance
>>    drm/amdgpu: sw_fini for each vcn instance
>>    drm/amdgpu: hw_init for each vcn instance
>>    drm/amdgpu: suspend for each vcn instance
>>    drm/amdgpu: resume for each vcn instance
>>    drm/amdgpu: setup_ucode for each vcn instance
>>    drm/amdgpu: set funcs for each vcn instance
>>    drm/amdgpu: print_ip_state for each vcn instance
>>    drm/amdgpu: dump_ip_state for each vcn instance
>>    drm/amdgpu: wait_for_idle for each vcn instance
>>    drm/amdgpu: is_idle for each vcn instance
>>    drm/amdgpu: set_powergating for each vcn instance
>>
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h           |    4 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c       |   22 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c    |    5 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c    |   41 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c |   24 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c       |    4 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_isp.c       |    6 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c      |    4 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c       |    4 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c       |    4 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c       |    4 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c       |  338 +++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h       |   24 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c      |    6 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vpe.c       |   14 +-
>>   drivers/gpu/drm/amd/amdgpu/cik.c              |    6 +-
>>   drivers/gpu/drm/amd/amdgpu/cik_ih.c           |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/cik_sdma.c         |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/cz_ih.c            |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/dce_v10_0.c        |    6 +-
>>   drivers/gpu/drm/amd/amdgpu/dce_v11_0.c        |    6 +-
>>   drivers/gpu/drm/amd/amdgpu/dce_v6_0.c         |    6 +-
>>   drivers/gpu/drm/amd/amdgpu/dce_v8_0.c         |    6 +-
>>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c        |   16 +-
>>   drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c        |   12 +-
>>   drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c        |   12 +-
>>   drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c         |   14 +-
>>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c         |   12 +-
>>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c         |   16 +-
>>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c         |   14 +-
>>   drivers/gpu/drm/amd/amdgpu/gfx_v9_4_3.c       |   12 +-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c        |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c        |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v12_0.c        |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c         |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c         |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c         |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c         |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/iceland_ih.c       |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/ih_v6_0.c          |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/ih_v6_1.c          |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/ih_v7_0.c          |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c        |    2 +-
>>   drivers/gpu/drm/amd/amdgpu/jpeg_v2_0.c        |   20 +-
>>   drivers/gpu/drm/amd/amdgpu/jpeg_v2_5.c        |   20 +-
>>   drivers/gpu/drm/amd/amdgpu/jpeg_v3_0.c        |   20 +-
>>   drivers/gpu/drm/amd/amdgpu/jpeg_v4_0.c        |   20 +-
>>   drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_3.c      |   18 +-
>>   drivers/gpu/drm/amd/amdgpu/jpeg_v4_0_5.c      |   20 +-
>>   drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c      |   20 +-
>>   drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c       |    2 +-
>>   drivers/gpu/drm/amd/amdgpu/navi10_ih.c        |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/nv.c               |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c        |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c        |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c        |   16 +-
>>   drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c      |   14 +-
>>   drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c        |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c        |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c        |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c        |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/si.c               |    6 +-
>>   drivers/gpu/drm/amd/amdgpu/si_dma.c           |   14 +-
>>   drivers/gpu/drm/amd/amdgpu/si_ih.c            |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/soc15.c            |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/soc21.c            |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/soc24.c            |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/tonga_ih.c         |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/uvd_v3_1.c         |   10 +-
>>   drivers/gpu/drm/amd/amdgpu/uvd_v4_2.c         |   14 +-
>>   drivers/gpu/drm/amd/amdgpu/uvd_v5_0.c         |   23 +-
>>   drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c         |   25 +-
>>   drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c         |    4 +-
>>   drivers/gpu/drm/amd/amdgpu/vce_v2_0.c         |   16 +-
>>   drivers/gpu/drm/amd/amdgpu/vce_v3_0.c         |   20 +-
>>   drivers/gpu/drm/amd/amdgpu/vce_v4_0.c         |   12 +-
>>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c         |  140 +-
>>   drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c         |  124 +-
>>   drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c         | 1154 ++++++++---------
>>   drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c         |  909 +++++++------
>>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c         |  801 ++++++------
>>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c       |  727 +++++------
>>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c       |  782 ++++++-----
>>   drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c       |  690 +++++-----
>>   drivers/gpu/drm/amd/amdgpu/vega10_ih.c        |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/vega20_ih.c        |    8 +-
>>   drivers/gpu/drm/amd/amdgpu/vi.c               |    8 +-
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |    6 +-
>>   drivers/gpu/drm/amd/include/amd_shared.h      |    6 +-
>>   .../gpu/drm/amd/include/kgd_pp_interface.h    |    4 +-
>>   drivers/gpu/drm/amd/pm/amdgpu_dpm.c           |   55 +-
>>   drivers/gpu/drm/amd/pm/inc/amdgpu_dpm.h       |    3 +-
>>   drivers/gpu/drm/amd/pm/legacy-dpm/kv_dpm.c    |   18 +-
>>   drivers/gpu/drm/amd/pm/legacy-dpm/si_dpm.c    |    6 +-
>>   .../gpu/drm/amd/pm/powerplay/amd_powerplay.c  |   10 +-
>>   .../drm/amd/pm/powerplay/hwmgr/smu10_hwmgr.c  |    6 +-
>>   .../powerplay/hwmgr/smu7_clockpowergating.c   |   12 +-
>>   .../drm/amd/pm/powerplay/hwmgr/smu8_hwmgr.c   |   12 +-
>>   .../drm/amd/pm/powerplay/hwmgr/vega20_hwmgr.c |    6 +-
>>   drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     |   65 +-
>>   drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h |    4 +-
>>   drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h  |    3 +-
>>   drivers/gpu/drm/amd/pm/swsmu/inc/smu_v14_0.h  |    3 +-
>>   .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c |    4 +-
>>   .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   |    4 +-
>>   .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   |   24 +-
>>   .../gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c  |    4 +-
>>   .../gpu/drm/amd/pm/swsmu/smu12/renoir_ppt.c   |    4 +-
>>   .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c    |   19 +-
>>   .../drm/amd/pm/swsmu/smu13/smu_v13_0_5_ppt.c  |    4 +-
>>   .../drm/amd/pm/swsmu/smu13/yellow_carp_ppt.c  |    4 +-
>>   .../gpu/drm/amd/pm/swsmu/smu14/smu_v14_0.c    |   38 +-
>>   112 files changed, 3442 insertions(+), 3433 deletions(-)
>>
>