Am 14.04.2017 um 00:04 schrieb Andres Rodriguez: > Use an LRU policy to map usermode rings to HW compute queues. > > Most compute clients use one queue, and usually the first queue > available. This results in poor pipe/queue work distribution when > multiple compute apps are running. In most cases pipe 0 queue 0 is > the only queue that gets used. > > In order to better distribute work across multiple HW queues, we adopt > a policy to map the usermode ring ids to the LRU HW queue. > > This fixes a large majority of multi-app compute workloads sharing the > same HW queue, even though 7 other queues are available. > > v2: use ring->funcs->type instead of ring->hw_ip > v3: remove amdgpu_queue_mapper_funcs > v4: change ring_lru_list_lock to spinlock, grab only once in lru_get() > > Signed-off-by: Andres Rodriguez <andresx7 at gmail.com> Reviewed-by: Christian König <christian.koenig at amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++ > drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c | 38 +++++++++++++++- > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 63 +++++++++++++++++++++++++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 4 ++ > 5 files changed, 110 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index 1d9053f..a9b7a61 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -1617,40 +1617,43 @@ struct amdgpu_device { > int num_ip_blocks; > struct mutex mn_lock; > DECLARE_HASHTABLE(mn_hash, 7); > > /* tracking pinned memory */ > u64 vram_pin_size; > u64 invisible_pin_size; > u64 gart_pin_size; > > /* amdkfd interface */ > struct kfd_dev *kfd; > > struct amdgpu_virt virt; > > /* link all shadow bo */ > struct list_head shadow_list; > struct mutex shadow_list_lock; > /* link all gtt */ > spinlock_t gtt_list_lock; > struct list_head gtt_list; > + /* keep an lru list of rings by HW IP */ > + struct list_head ring_lru_list; > + spinlock_t ring_lru_list_lock; > > /* record hw reset is performed */ > bool has_hw_reset; > > }; > > static inline struct amdgpu_device *amdgpu_ttm_adev(struct ttm_bo_device *bdev) > { > return container_of(bdev, struct amdgpu_device, mman.bdev); > } > > bool amdgpu_device_is_px(struct drm_device *dev); > int amdgpu_device_init(struct amdgpu_device *adev, > struct drm_device *ddev, > struct pci_dev *pdev, > uint32_t flags); > void amdgpu_device_fini(struct amdgpu_device *adev); > int amdgpu_gpu_wait_for_idle(struct amdgpu_device *adev); > > uint32_t amdgpu_mm_rreg(struct amdgpu_device *adev, uint32_t reg, > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 724b4c1..2acceef 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -1865,40 +1865,43 @@ int amdgpu_device_init(struct amdgpu_device *adev, > > amdgpu_check_arguments(adev); > > /* Registers mapping */ > /* TODO: block userspace mapping of io register */ > spin_lock_init(&adev->mmio_idx_lock); > spin_lock_init(&adev->smc_idx_lock); > spin_lock_init(&adev->pcie_idx_lock); > spin_lock_init(&adev->uvd_ctx_idx_lock); > spin_lock_init(&adev->didt_idx_lock); > spin_lock_init(&adev->gc_cac_idx_lock); > spin_lock_init(&adev->audio_endpt_idx_lock); > spin_lock_init(&adev->mm_stats.lock); > > INIT_LIST_HEAD(&adev->shadow_list); > mutex_init(&adev->shadow_list_lock); > > INIT_LIST_HEAD(&adev->gtt_list); > spin_lock_init(&adev->gtt_list_lock); > > + INIT_LIST_HEAD(&adev->ring_lru_list); > + spin_lock_init(&adev->ring_lru_list_lock); > + > if (adev->asic_type >= CHIP_BONAIRE) { > adev->rmmio_base = pci_resource_start(adev->pdev, 5); > adev->rmmio_size = pci_resource_len(adev->pdev, 5); > } else { > adev->rmmio_base = pci_resource_start(adev->pdev, 2); > adev->rmmio_size = pci_resource_len(adev->pdev, 2); > } > > adev->rmmio = ioremap(adev->rmmio_base, adev->rmmio_size); > if (adev->rmmio == NULL) { > return -ENOMEM; > } > DRM_INFO("register mmio base: 0x%08X\n", (uint32_t)adev->rmmio_base); > DRM_INFO("register mmio size: %u\n", (unsigned)adev->rmmio_size); > > if (adev->asic_type >= CHIP_BONAIRE) > /* doorbell bar mapping */ > amdgpu_doorbell_init(adev); > > /* io port mapping */ > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c > index 3e9ac80..054d750 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c > @@ -74,40 +74,74 @@ static int amdgpu_identity_map(struct amdgpu_device *adev, > *out_ring = &adev->gfx.compute_ring[ring]; > break; > case AMDGPU_HW_IP_DMA: > *out_ring = &adev->sdma.instance[ring].ring; > break; > case AMDGPU_HW_IP_UVD: > *out_ring = &adev->uvd.ring; > break; > case AMDGPU_HW_IP_VCE: > *out_ring = &adev->vce.ring[ring]; > break; > default: > *out_ring = NULL; > DRM_ERROR("unknown HW IP type: %d\n", mapper->hw_ip); > return -EINVAL; > } > > return amdgpu_update_cached_map(mapper, ring, *out_ring); > } > > +static enum amdgpu_ring_type amdgpu_hw_ip_to_ring_type(int hw_ip) > +{ > + switch (hw_ip) { > + case AMDGPU_HW_IP_GFX: > + return AMDGPU_RING_TYPE_GFX; > + case AMDGPU_HW_IP_COMPUTE: > + return AMDGPU_RING_TYPE_COMPUTE; > + case AMDGPU_HW_IP_DMA: > + return AMDGPU_RING_TYPE_SDMA; > + case AMDGPU_HW_IP_UVD: > + return AMDGPU_RING_TYPE_UVD; > + case AMDGPU_HW_IP_VCE: > + return AMDGPU_RING_TYPE_VCE; > + default: > + 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 ring_type = amdgpu_hw_ip_to_ring_type(mapper->hw_ip); > + > + r = amdgpu_ring_lru_get(adev, ring_type, 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 > * @mgr: amdgpu_queue_mgr structure holding queue information > * > * Initialize the the selected @mgr (all asics). > * > * Returns 0 on success, error on failure. > */ > int amdgpu_queue_mgr_init(struct amdgpu_device *adev, > struct amdgpu_queue_mgr *mgr) > { > int i, r; > > if (!adev || !mgr) > return -EINVAL; > > memset(mgr, 0, sizeof(*mgr)); > > @@ -195,36 +229,38 @@ int amdgpu_queue_mgr_map(struct amdgpu_device *adev, > return -EINVAL; > } > > if (ring >= ip_num_rings) { > DRM_ERROR("Ring index:%d exceeds maximum:%d for ip:%d\n", > ring, ip_num_rings, hw_ip); > return -EINVAL; > } > > mutex_lock(&mapper->lock); > > *out_ring = amdgpu_get_cached_map(mapper, ring); > if (*out_ring) { > /* cache hit */ > r = 0; > goto out_unlock; > } > > switch (mapper->hw_ip) { > case AMDGPU_HW_IP_GFX: > - case AMDGPU_HW_IP_COMPUTE: > case AMDGPU_HW_IP_DMA: > case AMDGPU_HW_IP_UVD: > case AMDGPU_HW_IP_VCE: > r = amdgpu_identity_map(adev, mapper, ring, out_ring); > break; > + case AMDGPU_HW_IP_COMPUTE: > + r = amdgpu_lru_map(adev, mapper, ring, out_ring); > + break; > default: > *out_ring = NULL; > r = -EINVAL; > DRM_ERROR("unknown HW IP type: %d\n", mapper->hw_ip); > } > > out_unlock: > mutex_unlock(&mapper->lock); > return r; > } > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > index 12fc815..2b452b0 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c > @@ -163,40 +163,42 @@ void amdgpu_ring_generic_pad_ib(struct amdgpu_ring *ring, struct amdgpu_ib *ib) > * @ring: amdgpu_ring structure holding ring information > * > * Update the wptr (write pointer) to tell the GPU to > * execute new commands on the ring buffer (all asics). > */ > void amdgpu_ring_commit(struct amdgpu_ring *ring) > { > uint32_t count; > > /* We pad to match fetch size */ > count = ring->funcs->align_mask + 1 - > (ring->wptr & ring->funcs->align_mask); > count %= ring->funcs->align_mask + 1; > ring->funcs->insert_nop(ring, count); > > mb(); > amdgpu_ring_set_wptr(ring); > > if (ring->funcs->end_use) > ring->funcs->end_use(ring); > + > + amdgpu_ring_lru_touch(ring->adev, ring); > } > > /** > * amdgpu_ring_undo - reset the wptr > * > * @ring: amdgpu_ring structure holding ring information > * > * Reset the driver's copy of the wptr (all asics). > */ > void amdgpu_ring_undo(struct amdgpu_ring *ring) > { > ring->wptr = ring->wptr_old; > > if (ring->funcs->end_use) > ring->funcs->end_use(ring); > } > > /** > * amdgpu_ring_init - init driver ring struct. > * > @@ -281,40 +283,42 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring, > amdgpu_sched_hw_submission); > > ring->buf_mask = (ring->ring_size / 4) - 1; > ring->ptr_mask = ring->funcs->support_64bit_ptrs ? > 0xffffffffffffffff : ring->buf_mask; > /* Allocate ring buffer */ > if (ring->ring_obj == NULL) { > r = amdgpu_bo_create_kernel(adev, ring->ring_size, PAGE_SIZE, > AMDGPU_GEM_DOMAIN_GTT, > &ring->ring_obj, > &ring->gpu_addr, > (void **)&ring->ring); > if (r) { > dev_err(adev->dev, "(%d) ring create failed\n", r); > return r; > } > amdgpu_ring_clear_ring(ring); > } > > ring->max_dw = max_dw; > + INIT_LIST_HEAD(&ring->lru_list); > + amdgpu_ring_lru_touch(adev, ring); > > if (amdgpu_debugfs_ring_init(adev, ring)) { > DRM_ERROR("Failed to register debugfs file for rings !\n"); > } > return 0; > } > > /** > * amdgpu_ring_fini - tear down the driver ring struct. > * > * @adev: amdgpu_device pointer > * @ring: amdgpu_ring structure holding ring information > * > * Tear down the driver information for the selected ring (all asics). > */ > void amdgpu_ring_fini(struct amdgpu_ring *ring) > { > ring->ready = false; > > if (ring->funcs->support_64bit_ptrs) { > @@ -322,40 +326,99 @@ void amdgpu_ring_fini(struct amdgpu_ring *ring) > amdgpu_wb_free_64bit(ring->adev, ring->fence_offs); > amdgpu_wb_free_64bit(ring->adev, ring->rptr_offs); > amdgpu_wb_free_64bit(ring->adev, ring->wptr_offs); > } else { > amdgpu_wb_free(ring->adev, ring->cond_exe_offs); > amdgpu_wb_free(ring->adev, ring->fence_offs); > amdgpu_wb_free(ring->adev, ring->rptr_offs); > amdgpu_wb_free(ring->adev, ring->wptr_offs); > } > > > amdgpu_bo_free_kernel(&ring->ring_obj, > &ring->gpu_addr, > (void **)&ring->ring); > > amdgpu_debugfs_ring_fini(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); > +} > + > +/** > + * amdgpu_ring_lru_get - get the least recently used ring for a HW IP block > + * > + * @adev: amdgpu_device pointer > + * @type: amdgpu_ring_type enum > + * @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) > +{ > + 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; > + } > + } > + 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; > +} > + > +/** > + * amdgpu_ring_lru_touch - mark a ring as recently being used > + * > + * @adev: amdgpu_device pointer > + * @ring: ring to touch > + * > + * Move @ring to the tail of the lru list > + */ > +void amdgpu_ring_lru_touch(struct amdgpu_device *adev, struct amdgpu_ring *ring) > +{ > + spin_lock(&adev->ring_lru_list_lock); > + amdgpu_ring_lru_touch_locked(adev, ring); > + spin_unlock(&adev->ring_lru_list_lock); > +} > + > /* > * Debugfs info > */ > #if defined(CONFIG_DEBUG_FS) > > /* Layout of file is 12 bytes consisting of > * - rptr > * - wptr > * - driver's copy of wptr > * > * followed by n-words of ring data > */ > static ssize_t amdgpu_debugfs_ring_read(struct file *f, char __user *buf, > size_t size, loff_t *pos) > { > struct amdgpu_ring *ring = file_inode(f)->i_private; > int r, i; > uint32_t value, result, early[3]; > > if (*pos & 3 || size & 3) > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > index 4de0d83..3967f7b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h > @@ -132,40 +132,41 @@ struct amdgpu_ring_funcs { > void (*insert_nop)(struct amdgpu_ring *ring, uint32_t count); > void (*insert_end)(struct amdgpu_ring *ring); > /* pad the indirect buffer to the necessary number of dw */ > void (*pad_ib)(struct amdgpu_ring *ring, struct amdgpu_ib *ib); > unsigned (*init_cond_exec)(struct amdgpu_ring *ring); > void (*patch_cond_exec)(struct amdgpu_ring *ring, unsigned offset); > /* note usage for clock and power gating */ > void (*begin_use)(struct amdgpu_ring *ring); > void (*end_use)(struct amdgpu_ring *ring); > void (*emit_switch_buffer) (struct amdgpu_ring *ring); > void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags); > void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg); > void (*emit_wreg)(struct amdgpu_ring *ring, uint32_t reg, uint32_t val); > }; > > struct amdgpu_ring { > struct amdgpu_device *adev; > const struct amdgpu_ring_funcs *funcs; > struct amdgpu_fence_driver fence_drv; > struct amd_gpu_scheduler sched; > + struct list_head lru_list; > > struct amdgpu_bo *ring_obj; > volatile uint32_t *ring; > unsigned rptr_offs; > u64 wptr; > u64 wptr_old; > unsigned ring_size; > unsigned max_dw; > int count_dw; > uint64_t gpu_addr; > uint64_t ptr_mask; > uint32_t buf_mask; > bool ready; > u32 idx; > u32 me; > u32 pipe; > u32 queue; > struct amdgpu_bo *mqd_obj; > uint64_t mqd_gpu_addr; > void *mqd_ptr; > @@ -179,29 +180,32 @@ struct amdgpu_ring { > unsigned cond_exe_offs; > u64 cond_exe_gpu_addr; > volatile u32 *cond_exe_cpu_addr; > unsigned vm_inv_eng; > #if defined(CONFIG_DEBUG_FS) > struct dentry *ent; > #endif > }; > > int amdgpu_ring_is_valid_index(struct amdgpu_device *adev, > int hw_ip, int ring); > int amdgpu_ring_alloc(struct amdgpu_ring *ring, unsigned ndw); > void amdgpu_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count); > 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_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); > +void amdgpu_ring_lru_touch(struct amdgpu_device *adev, struct amdgpu_ring *ring); > static inline void amdgpu_ring_clear_ring(struct amdgpu_ring *ring) > { > int i = 0; > while (i <= ring->buf_mask) > ring->ring[i++] = ring->funcs->nop; > > } > > #endif