Am 12.06.2018 um 20:37 schrieb Andrey Grodzovsky:
> Move all instnaces of this check into a function in amdgpu_gmc.h
> Rename the original function to a more proper name.
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
There seems to be two more candidates for that cleanup:
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c:Â Â Â if
(adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c:Â Â Â Â Â Â } else if
(adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&
With those two fixed as well the patch is Reviewed-by: Christian König
<christian.koenig at amd.com>.
Thanks,
Christian.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 11 ++++++-----
> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 15 +++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 ++++----------------
> 3 files changed, 25 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 12f0d18..f98be69 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -31,6 +31,7 @@
> #include <drm/drm_syncobj.h>
> #include "amdgpu.h"
> #include "amdgpu_trace.h"
> +#include "amdgpu_gmc.h"
>
> static int amdgpu_cs_user_fence_chunk(struct amdgpu_cs_parser *p,
> struct drm_amdgpu_cs_chunk_fence *data,
> @@ -302,7 +303,7 @@ static void amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev,
> *max_bytes = us_to_bytes(adev, adev->mm_stats.accum_us);
>
> /* Do the same for visible VRAM if half of it is free */
> - if (adev->gmc.visible_vram_size < adev->gmc.real_vram_size) {
> + if (!amdgpu_gmc_vram_full_visible(&adev->gmc)) {
> u64 total_vis_vram = adev->gmc.visible_vram_size;
> u64 used_vis_vram =
> amdgpu_vram_mgr_vis_usage(&adev->mman.bdev.man[TTM_PL_VRAM]);
> @@ -359,7 +360,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
> * to move it. Don't move anything if the threshold is zero.
> */
> if (p->bytes_moved < p->bytes_moved_threshold) {
> - if (adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&
> + if (!amdgpu_gmc_vram_full_visible(&adev->gmc) &&
> (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)) {
> /* And don't move a CPU_ACCESS_REQUIRED BO to limited
> * visible VRAM if we've depleted our allowance to do
> @@ -381,7 +382,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
> r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
>
> p->bytes_moved += ctx.bytes_moved;
> - if (adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&
> + if (!amdgpu_gmc_vram_full_visible(&adev->gmc) &&
> amdgpu_bo_in_cpu_visible_vram(bo))
> p->bytes_moved_vis += ctx.bytes_moved;
>
> @@ -434,8 +435,8 @@ static bool amdgpu_cs_try_evict(struct amdgpu_cs_parser *p,
>
> /* Good we can try to move this BO somewhere else */
> update_bytes_moved_vis =
> - adev->gmc.visible_vram_size < adev->gmc.real_vram_size &&
> - amdgpu_bo_in_cpu_visible_vram(bo);
> + !amdgpu_gmc_vram_full_visible(&adev->gmc) &&
> + amdgpu_bo_in_cpu_visible_vram(bo);
> amdgpu_ttm_placement_from_domain(bo, other);
> r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
> p->bytes_moved += ctx.bytes_moved;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> index 893c249..6cb4948 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
> @@ -109,4 +109,19 @@ struct amdgpu_gmc {
> const struct amdgpu_gmc_funcs *gmc_funcs;
> };
>
> +/**
> + * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
> + *
> + * @adev: amdgpu_device pointer
> + *
> + * Returns:
> + * True if full VRAM is visible through the BAR
> + */
> +static inline bool amdgpu_gmc_vram_full_visible(struct amdgpu_gmc *gmc)
> +{
> + WARN_ON(gmc->real_vram_size < gmc->visible_vram_size);
> +
> + return (gmc->real_vram_size == gmc->visible_vram_size);
> +}
> +
> #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 7f03f8c..6d22942 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -33,6 +33,7 @@
> #include "amdgpu.h"
> #include "amdgpu_trace.h"
> #include "amdgpu_amdkfd.h"
> +#include "amdgpu_gmc.h"
>
> /**
> * DOC: GPUVM
> @@ -669,19 +670,6 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
> }
>
> /**
> - * amdgpu_vm_is_large_bar - Check if BAR is large enough
> - *
> - * @adev: amdgpu_device pointer
> - *
> - * Returns:
> - * True if BAR is large enough.
> - */
> -static bool amdgpu_vm_is_large_bar(struct amdgpu_device *adev)
> -{
> - return (adev->gmc.real_vram_size == adev->gmc.visible_vram_size);
> -}
> -
> -/**
> * amdgpu_vm_flush - hardware flush the vm
> *
> * @ring: ring to use for flush
> @@ -2579,7 +2567,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> }
> DRM_DEBUG_DRIVER("VM update mode is %s\n",
> vm->use_cpu_for_update ? "CPU" : "SDMA");
> - WARN_ONCE((vm->use_cpu_for_update & !amdgpu_vm_is_large_bar(adev)),
> + WARN_ONCE((vm->use_cpu_for_update & !amdgpu_gmc_vram_full_visible(&adev->gmc)),
> "CPU update of VM recommended only for large BAR system\n");
> vm->last_update = NULL;
>
> @@ -2699,7 +2687,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> vm->pte_support_ats = pte_support_ats;
> DRM_DEBUG_DRIVER("VM update mode is %s\n",
> vm->use_cpu_for_update ? "CPU" : "SDMA");
> - WARN_ONCE((vm->use_cpu_for_update & !amdgpu_vm_is_large_bar(adev)),
> + WARN_ONCE((vm->use_cpu_for_update & !amdgpu_gmc_vram_full_visible(&adev->gmc)),
> "CPU update of VM recommended only for large BAR system\n");
>
> if (vm->pasid) {
> @@ -2877,7 +2865,7 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
> */
> #ifdef CONFIG_X86_64
> if (amdgpu_vm_update_mode == -1) {
> - if (amdgpu_vm_is_large_bar(adev))
> + if (amdgpu_gmc_vram_full_visible(&adev->gmc))
> adev->vm_manager.vm_update_mode =
> AMDGPU_VM_USE_CPU_FOR_COMPUTE;
> else
