On Tue, Sep 5, 2023 at 2:30 AM Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
>
> Instead of each implementation doing this more or less correctly
> move taking the reset lock at a higher level.
>
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 9 +++++++++
> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 6 +-----
> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 5 -----
> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 5 -----
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 6 +-----
> 5 files changed, 11 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 15814cb801e7..c24252304d48 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -589,8 +589,17 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
> !adev->mman.buffer_funcs_enabled ||
> !adev->ib_pool_ready || amdgpu_in_reset(adev) ||
> !ring->sched.ready) {
> +
> + /*
> + * A GPU reset should flush all TLBs anyway, so no need to do
> + * this while one is ongoing.
> + */
> + if(!down_read_trylock(&adev->reset_domain->sem))
space between the if and (.
With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
> + return;
> +
> adev->gmc.gmc_funcs->flush_gpu_tlb(adev, vmid, vmhub,
> flush_type);
> + up_read(&adev->reset_domain->sem);
> return;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index 40d432d46469..302279497d67 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -51,8 +51,6 @@
> #include "athub_v2_0.h"
> #include "athub_v2_1.h"
>
> -#include "amdgpu_reset.h"
> -
> static int gmc_v10_0_ecc_interrupt_state(struct amdgpu_device *adev,
> struct amdgpu_irq_src *src,
> unsigned int type,
> @@ -264,11 +262,9 @@ static void gmc_v10_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
> * Directly use kiq to do the vm invalidation instead
> */
> if (adev->gfx.kiq[0].ring.sched.ready && !adev->enable_mes &&
> - (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev)) &&
> - down_read_trylock(&adev->reset_domain->sem)) {
> + (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev))) {
> amdgpu_virt_kiq_reg_write_reg_wait(adev, req, ack, inv_req,
> 1 << vmid);
> - up_read(&adev->reset_domain->sem);
> return;
> }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index fa3586efacd2..998f6ee60b78 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -33,7 +33,6 @@
> #include "amdgpu_ucode.h"
> #include "amdgpu_amdkfd.h"
> #include "amdgpu_gem.h"
> -#include "amdgpu_reset.h"
>
> #include "bif/bif_4_1_d.h"
> #include "bif/bif_4_1_sh_mask.h"
> @@ -430,9 +429,6 @@ static void gmc_v7_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
> u32 mask = 0x0;
> int vmid;
>
> - if(!down_read_trylock(&adev->reset_domain->sem))
> - return;
> -
> for (vmid = 1; vmid < 16; vmid++) {
> u32 tmp = RREG32(mmATC_VMID0_PASID_MAPPING + vmid);
>
> @@ -443,7 +439,6 @@ static void gmc_v7_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
>
> WREG32(mmVM_INVALIDATE_REQUEST, mask);
> RREG32(mmVM_INVALIDATE_RESPONSE);
> - up_read(&adev->reset_domain->sem);
> }
>
> /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index ffcd79d28b9a..8dcd9b13673c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -31,7 +31,6 @@
> #include "amdgpu_ucode.h"
> #include "amdgpu_amdkfd.h"
> #include "amdgpu_gem.h"
> -#include "amdgpu_reset.h"
>
> #include "gmc/gmc_8_1_d.h"
> #include "gmc/gmc_8_1_sh_mask.h"
> @@ -620,9 +619,6 @@ static void gmc_v8_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
> u32 mask = 0x0;
> int vmid;
>
> - if(!down_read_trylock(&adev->reset_domain->sem))
> - return;
> -
> for (vmid = 1; vmid < 16; vmid++) {
> u32 tmp = RREG32(mmATC_VMID0_PASID_MAPPING + vmid);
>
> @@ -633,7 +629,6 @@ static void gmc_v8_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
>
> WREG32(mmVM_INVALIDATE_REQUEST, mask);
> RREG32(mmVM_INVALIDATE_RESPONSE);
> - up_read(&adev->reset_domain->sem);
> }
>
> /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 94ba16536fc2..c5df8f052f3f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -64,8 +64,6 @@
> #include "amdgpu_ras.h"
> #include "amdgpu_xgmi.h"
>
> -#include "amdgpu_reset.h"
> -
> /* add these here since we already include dce12 headers and these are for DCN */
> #define mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION 0x055d
> #define mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION_BASE_IDX 2
> @@ -849,8 +847,7 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
> * as GFXOFF under bare metal
> */
> if (adev->gfx.kiq[0].ring.sched.ready &&
> - (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev)) &&
> - down_read_trylock(&adev->reset_domain->sem)) {
> + (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev))) {
> uint32_t req = hub->vm_inv_eng0_req + hub->eng_distance * eng;
> uint32_t ack = hub->vm_inv_eng0_ack + hub->eng_distance * eng;
>
> @@ -860,7 +857,6 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
> amdgpu_virt_kiq_reg_write_reg_wait(adev, req, ack,
> inv_req2, 1 << vmid);
>
> - up_read(&adev->reset_domain->sem);
> return;
> }
>
> --
> 2.34.1
>
