On 17.03.2017 19:52, Andres Rodriguez wrote:
> Depending on usage patterns, the current LRU policy may create a
> non-injective mapping between userspace ring ids and kernel rings.
>
> This behaviour is undesired as apps that attempt to fill all HW blocks
> would be unable to reach some of them.
>
> This change forces the LRU policy to create bijective mappings only.
>
> Signed-off-by: Andres Rodriguez <andresx7 at gmail.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c | 15 ++++++++++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 33 +++++++++++++++++++++------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 4 ++--
> 3 files changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
> index 054d750..2cffb0e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c
> @@ -108,24 +108,35 @@ static enum amdgpu_ring_type amdgpu_hw_ip_to_ring_type(int hw_ip)
> DRM_ERROR("Invalid HW IP specified %d\n", hw_ip);
> return -1;
> }
> }
>
> static int amdgpu_lru_map(struct amdgpu_device *adev,
> struct amdgpu_queue_mapper *mapper,
> int user_ring,
> struct amdgpu_ring **out_ring)
> {
> - int r;
> + int r, i;
> int ring_type = amdgpu_hw_ip_to_ring_type(mapper->hw_ip);
> + int ring_blacklist[AMDGPU_MAX_RINGS];
> + struct amdgpu_ring *ring;
> +
> + for (i = 0; i < AMDGPU_MAX_RINGS; i++) {
> + ring = mapper->queue_map[i];
> + if (!ring)
> + ring_blacklist[i] = -1;
> + else
> + ring_blacklist[i] = ring->idx;
> + }
Given how ring_blacklist is used, I'd suggest to "compress" its entries
instead of introducing -1 gaps.
The rest of the patch looks good to me.
Cheers,
Nicolai
>
> - r = amdgpu_ring_lru_get(adev, ring_type, out_ring);
> + r = amdgpu_ring_lru_get(adev, ring_type, ring_blacklist,
> + AMDGPU_MAX_RINGS, out_ring);
> if (r)
> return r;
>
> return amdgpu_update_cached_map(mapper, user_ring, *out_ring);
> }
>
> /**
> * amdgpu_queue_mgr_init - init an amdgpu_queue_mgr struct
> *
> * @adev: amdgpu_device pointer
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index ca41b3a..0db07b0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -393,46 +393,65 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring)
> ring->adev->rings[ring->idx] = NULL;
> }
>
> static void amdgpu_ring_lru_touch_locked(struct amdgpu_device *adev,
> struct amdgpu_ring *ring)
> {
> /* list_move_tail handles the case where ring isn't part of the list */
> list_move_tail(&ring->lru_list, &adev->ring_lru_list);
> }
>
> +static bool amdgpu_ring_is_blacklisted(struct amdgpu_ring *ring,
> + int *blacklist, int num_blacklist)
> +{
> + int i;
> +
> + for (i = 0; i < num_blacklist; i++) {
> + if (ring->idx == blacklist[i])
> + return true;
> + }
> +
> + return false;
> +}
> +
> /**
> * amdgpu_ring_lru_get - get the least recently used ring for a HW IP block
> *
> * @adev: amdgpu_device pointer
> * @type: amdgpu_ring_type enum
> + * @blacklist: blacklisted ring ids array
> + * @num_blacklist: number of entries in @blacklist
> * @ring: output ring
> *
> * Retrieve the amdgpu_ring structure for the least recently used ring of
> * a specific IP block (all asics).
> * Returns 0 on success, error on failure.
> */
> -int amdgpu_ring_lru_get(struct amdgpu_device *adev, int type,
> - struct amdgpu_ring **ring)
> +int amdgpu_ring_lru_get(struct amdgpu_device *adev, int type, int *blacklist,
> + int num_blacklist, struct amdgpu_ring **ring)
> {
> struct amdgpu_ring *entry;
>
> /* List is sorted in LRU order, find first entry corresponding
> * to the desired HW IP */
> *ring = NULL;
> spin_lock(&adev->ring_lru_list_lock);
> list_for_each_entry(entry, &adev->ring_lru_list, lru_list) {
> - if (entry->funcs->type == type) {
> - *ring = entry;
> - amdgpu_ring_lru_touch_locked(adev, *ring);
> - break;
> - }
> + if (entry->funcs->type != type)
> + continue;
> +
> + if (amdgpu_ring_is_blacklisted(entry, blacklist, num_blacklist))
> + continue;
> +
> + *ring = entry;
> + amdgpu_ring_lru_touch_locked(adev, *ring);
> + break;
> }
> spin_unlock(&adev->ring_lru_list_lock);
>
> if (!*ring) {
> DRM_ERROR("Ring LRU contains no entries for ring type:%d\n", type);
> return -EINVAL;
> }
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 2c801a5..811d76a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -195,15 +195,15 @@ void amdgpu_ring_generic_pad_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib);
> void amdgpu_ring_commit(struct amdgpu_ring *ring);
> void amdgpu_ring_undo(struct amdgpu_ring *ring);
> int amdgpu_ring_priority_get(struct amdgpu_ring *ring,
> enum amd_sched_priority priority);
> void amdgpu_ring_priority_put(struct amdgpu_ring *ring,
> enum amd_sched_priority priority);
> int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring,
> unsigned ring_size, struct amdgpu_irq_src *irq_src,
> unsigned irq_type);
> void amdgpu_ring_fini(struct amdgpu_ring *ring);
> -int amdgpu_ring_lru_get(struct amdgpu_device *adev, int hw_ip,
> - struct amdgpu_ring **ring);
> +int amdgpu_ring_lru_get(struct amdgpu_device *adev, int type, int *blacklist,
> + int num_blacklist, struct amdgpu_ring **ring);
> void amdgpu_ring_lru_touch(struct amdgpu_device *adev, struct amdgpu_ring *ring);
>
> #endif
>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
