[PATCH 1/3] drm/amdgpu: Optimize a function called by every IB sheduling

[Xie, AlexBin]

HI Christian,


Too late. The code has been committed.


I don't have strong opinion about where the code should be. But if we put this code in VM, there will be one extra array index operation because the VM bug is ring related. VM manager need to maintain an array to manage this information.


In the amdgpu_ring structure, there is already information like vm_inv_eng and vmhub. Those are VM related information too. So this one extra information is not new.


Thanks,

Alex Bin

________________________________
From: Christian König <deathsimple at vodafone.de>
Sent: Wednesday, May 31, 2017 2:57 AM
To: Xie, AlexBin; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH 1/3] drm/amdgpu: Optimize a function called by every IB sheduling

Am 30.05.2017 um 23:47 schrieb Alex Xie:
>    Move several if statements and a loop statment from
>    run time to initialization time.

Yeah, that's exactly what I've suggested before as well.

Just keep the code inside amdgpu_vm.c (and the variable inside
amdgpu_vm_manager), since this isn't related to ring management at all.

Regards,
Christian.

>
> Signed-off-by: Alex Xie <AlexBin.Xie at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 33 ++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  6 ++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c   | 28 +--------------------------
>   3 files changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index 6a85db0..7d95435 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -153,6 +153,36 @@ void amdgpu_ring_undo(struct amdgpu_ring *ring)
>   }
>
>   /**
> + * amdgpu_ring_check_compute_vm_bug - check whether this ring has compute vm bug
> + *
> + * @adev: amdgpu_device pointer
> + * @ring: amdgpu_ring structure holding ring information
> + */
> +static void amdgpu_ring_check_compute_vm_bug(struct amdgpu_device *adev,
> +                                     struct amdgpu_ring *ring)
> +{
> +     const struct amdgpu_ip_block *ip_block;
> +
> +     ring->has_compute_vm_bug = false;
> +
> +     if (ring->funcs->type != AMDGPU_RING_TYPE_COMPUTE)
> +             /* only compute rings */
> +             return;
> +
> +     ip_block = amdgpu_get_ip_block(adev, AMD_IP_BLOCK_TYPE_GFX);
> +     if (!ip_block)
> +             return;
> +
> +     /* Compute ring has a VM bug for GFX version < 7.
> +           And compute ring has a VM bug for GFX 8 MEC firmware version < 673.*/
> +     if (ip_block->version->major <= 7) {
> +             ring->has_compute_vm_bug = true;
> +     } else if (ip_block->version->major == 8)
> +             if (adev->gfx.mec_fw_version < 673)
> +                     ring->has_compute_vm_bug = true;
> +}
> +
> +/**
>    * amdgpu_ring_init - init driver ring struct.
>    *
>    * @adev: amdgpu_device pointer
> @@ -257,6 +287,9 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring,
>        if (amdgpu_debugfs_ring_init(adev, ring)) {
>                DRM_ERROR("Failed to register debugfs file for rings !\n");
>        }
> +
> +     amdgpu_ring_check_compute_vm_bug(adev, ring);
> +
>        return 0;
>   }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index a9223a8..334307e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -185,6 +185,7 @@ struct amdgpu_ring {
>        u64                     cond_exe_gpu_addr;
>        volatile u32            *cond_exe_cpu_addr;
>        unsigned                vm_inv_eng;
> +     bool                    has_compute_vm_bug;
>   #if defined(CONFIG_DEBUG_FS)
>        struct dentry *ent;
>   #endif
> @@ -207,4 +208,9 @@ static inline void amdgpu_ring_clear_ring(struct amdgpu_ring *ring)
>
>   }
>
> +static inline bool amdgpu_ring_has_compute_vm_bug(struct amdgpu_ring *ring)
> +{
> +     return ring->has_compute_vm_bug;
> +}
> +
>   #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index b2384b8..7a323f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -656,32 +656,6 @@ static int amdgpu_vm_alloc_reserved_vmid(struct amdgpu_device *adev,
>        return r;
>   }
>
> -static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring *ring)
> -{
> -     struct amdgpu_device *adev = ring->adev;
> -     const struct amdgpu_ip_block *ip_block;
> -
> -     if (ring->funcs->type != AMDGPU_RING_TYPE_COMPUTE)
> -             /* only compute rings */
> -             return false;
> -
> -     ip_block = amdgpu_get_ip_block(adev, AMD_IP_BLOCK_TYPE_GFX);
> -     if (!ip_block)
> -             return false;
> -
> -     if (ip_block->version->major <= 7) {
> -             /* gfx7 has no workaround */
> -             return true;
> -     } else if (ip_block->version->major == 8) {
> -             if (adev->gfx.mec_fw_version >= 673)
> -                     /* gfx8 is fixed in MEC firmware 673 */
> -                     return false;
> -             else
> -                     return true;
> -     }
> -     return false;
> -}
> -
>   bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>                                  struct amdgpu_job *job)
>   {
> @@ -691,7 +665,7 @@ bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
>        struct amdgpu_vm_id *id;
>        bool gds_switch_needed;
>        bool vm_flush_needed = job->vm_needs_flush ||
> -             amdgpu_vm_ring_has_compute_vm_bug(ring);
> +             amdgpu_ring_has_compute_vm_bug(ring);
>
>        if (job->vm_id == 0)
>                return false;


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20170531/4ecac9ee/attachment.html>