|
Okay. I submitted a PSDB. Meanwhile, I got the answer from FW and SQ HW contact that nothing bad will happen on GFX9 by writing 1 to TRAP_EN.
Regards,
Yong
From: Kuehling, Felix <Felix.Kuehling@xxxxxxx>
Sent: Thursday, November 7, 2019 4:07 PM To: Zhao, Yong <Yong.Zhao@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx> Subject: Re: [PATCH] drm/amdkfd: Use kernel queue v9 functions for v10 (ver2) Are you sure that setting the SQ_SHADER_TBA_HI__TRAP_EN bit on GFXv9 is
completely harmless? If the field is not defined, maybe setting the bit makes the address invalid. It's probably worth running that through a PSDB, which would cover Vega10, Vega20 and Arcturus. If it actually works, the patch is Reviewed-by: Felix Kuehling <Felix.Kuehling@xxxxxxx> Regards, Felix On 2019-11-07 15:34, Zhao, Yong wrote: > The kernel queue functions for v9 and v10 are the same except > pm_map_process_v* which have small difference, so they should be reused. > This eliminates the need of reapplying several patches which were > applied on v9 but not on v10, such as bigger GWS and more than 2 > SDMA engine support which were introduced on Arcturus. > > Change-Id: I2d385961e3c884db14e30b5afc98d0d9e4cb1802 > Signed-off-by: Yong Zhao <Yong.Zhao@xxxxxxx> > --- > drivers/gpu/drm/amd/amdkfd/Makefile | 1 - > drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 4 +- > drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h | 1 - > .../gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c | 317 ------------------ > .../gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c | 16 +- > .../gpu/drm/amd/amdkfd/kfd_packet_manager.c | 4 +- > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 4 - > 7 files changed, 14 insertions(+), 333 deletions(-) > delete mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c > > diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile b/drivers/gpu/drm/amd/amdkfd/Makefile > index 48155060a57c..017a8b7156da 100644 > --- a/drivers/gpu/drm/amd/amdkfd/Makefile > +++ b/drivers/gpu/drm/amd/amdkfd/Makefile > @@ -41,7 +41,6 @@ AMDKFD_FILES := $(AMDKFD_PATH)/kfd_module.o \ > $(AMDKFD_PATH)/kfd_kernel_queue_cik.o \ > $(AMDKFD_PATH)/kfd_kernel_queue_vi.o \ > $(AMDKFD_PATH)/kfd_kernel_queue_v9.o \ > - $(AMDKFD_PATH)/kfd_kernel_queue_v10.o \ > $(AMDKFD_PATH)/kfd_packet_manager.o \ > $(AMDKFD_PATH)/kfd_process_queue_manager.o \ > $(AMDKFD_PATH)/kfd_device_queue_manager.o \ > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c > index 11d244891393..0d966408ea87 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c > @@ -332,12 +332,10 @@ struct kernel_queue *kernel_queue_init(struct kfd_dev *dev, > case CHIP_RAVEN: > case CHIP_RENOIR: > case CHIP_ARCTURUS: > - kernel_queue_init_v9(&kq->ops_asic_specific); > - break; > case CHIP_NAVI10: > case CHIP_NAVI12: > case CHIP_NAVI14: > - kernel_queue_init_v10(&kq->ops_asic_specific); > + kernel_queue_init_v9(&kq->ops_asic_specific); > break; > default: > WARN(1, "Unexpected ASIC family %u", > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h > index 365fc674fea4..a7116a939029 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h > @@ -102,6 +102,5 @@ struct kernel_queue { > void kernel_queue_init_cik(struct kernel_queue_ops *ops); > void kernel_queue_init_vi(struct kernel_queue_ops *ops); > void kernel_queue_init_v9(struct kernel_queue_ops *ops); > -void kernel_queue_init_v10(struct kernel_queue_ops *ops); > > #endif /* KFD_KERNEL_QUEUE_H_ */ > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c > deleted file mode 100644 > index bfd6221acae9..000000000000 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c > +++ /dev/null > @@ -1,317 +0,0 @@ > -/* > - * Copyright 2018 Advanced Micro Devices, Inc. > - * > - * Permission is hereby granted, free of charge, to any person obtaining a > - * copy of this software and associated documentation files (the "Software"), > - * to deal in the Software without restriction, including without limitation > - * the rights to use, copy, modify, merge, publish, distribute, sublicense, > - * and/or sell copies of the Software, and to permit persons to whom the > - * Software is furnished to do so, subject to the following conditions: > - * > - * The above copyright notice and this permission notice shall be included in > - * all copies or substantial portions of the Software. > - * > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR > - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > - * OTHER DEALINGS IN THE SOFTWARE. > - * > - */ > - > -#include "kfd_kernel_queue.h" > -#include "kfd_device_queue_manager.h" > -#include "kfd_pm4_headers_ai.h" > -#include "kfd_pm4_opcodes.h" > -#include "gc/gc_10_1_0_sh_mask.h" > - > -static bool initialize_v10(struct kernel_queue *kq, struct kfd_dev *dev, > - enum kfd_queue_type type, unsigned int queue_size); > -static void uninitialize_v10(struct kernel_queue *kq); > -static void submit_packet_v10(struct kernel_queue *kq); > - > -void kernel_queue_init_v10(struct kernel_queue_ops *ops) > -{ > - ops->initialize = initialize_v10; > - ops->uninitialize = uninitialize_v10; > - ops->submit_packet = submit_packet_v10; > -} > - > -static bool initialize_v10(struct kernel_queue *kq, struct kfd_dev *dev, > - enum kfd_queue_type type, unsigned int queue_size) > -{ > - int retval; > - > - retval = kfd_gtt_sa_allocate(dev, PAGE_SIZE, &kq->eop_mem); > - if (retval != 0) > - return false; > - > - kq->eop_gpu_addr = kq->eop_mem->gpu_addr; > - kq->eop_kernel_addr = kq->eop_mem->cpu_ptr; > - > - memset(kq->eop_kernel_addr, 0, PAGE_SIZE); > - > - return true; > -} > - > -static void uninitialize_v10(struct kernel_queue *kq) > -{ > - kfd_gtt_sa_free(kq->dev, kq->eop_mem); > -} > - > -static void submit_packet_v10(struct kernel_queue *kq) > -{ > - *kq->wptr64_kernel = kq->pending_wptr64; > - write_kernel_doorbell64(kq->queue->properties.doorbell_ptr, > - kq->pending_wptr64); > -} > - > -static int pm_map_process_v10(struct packet_manager *pm, > - uint32_t *buffer, struct qcm_process_device *qpd) > -{ > - struct pm4_mes_map_process *packet; > - uint64_t vm_page_table_base_addr = qpd->page_table_base; > - > - packet = (struct pm4_mes_map_process *)buffer; > - memset(buffer, 0, sizeof(struct pm4_mes_map_process)); > - > - packet->header.u32All = pm_build_pm4_header(IT_MAP_PROCESS, > - sizeof(struct pm4_mes_map_process)); > - packet->bitfields2.diq_enable = (qpd->is_debug) ? 1 : 0; > - packet->bitfields2.process_quantum = 1; > - packet->bitfields2.pasid = qpd->pqm->process->pasid; > - packet->bitfields14.gds_size = qpd->gds_size; > - packet->bitfields14.num_gws = qpd->num_gws; > - packet->bitfields14.num_oac = qpd->num_oac; > - packet->bitfields14.sdma_enable = 1; > - > - packet->bitfields14.num_queues = (qpd->is_debug) ? 0 : qpd->queue_count; > - > - packet->sh_mem_config = qpd->sh_mem_config; > - packet->sh_mem_bases = qpd->sh_mem_bases; > - if (qpd->tba_addr) { > - packet->sq_shader_tba_lo = lower_32_bits(qpd->tba_addr >> 8); > - packet->sq_shader_tba_hi = (1 << SQ_SHADER_TBA_HI__TRAP_EN__SHIFT) | > - upper_32_bits(qpd->tba_addr >> 8); > - packet->sq_shader_tma_lo = lower_32_bits(qpd->tma_addr >> 8); > - packet->sq_shader_tma_hi = upper_32_bits(qpd->tma_addr >> 8); > - } > - > - packet->gds_addr_lo = lower_32_bits(qpd->gds_context_area); > - packet->gds_addr_hi = upper_32_bits(qpd->gds_context_area); > - > - packet->vm_context_page_table_base_addr_lo32 = > - lower_32_bits(vm_page_table_base_addr); > - packet->vm_context_page_table_base_addr_hi32 = > - upper_32_bits(vm_page_table_base_addr); > - > - return 0; > -} > - > -static int pm_runlist_v10(struct packet_manager *pm, uint32_t *buffer, > - uint64_t ib, size_t ib_size_in_dwords, bool chain) > -{ > - struct pm4_mes_runlist *packet; > - > - int concurrent_proc_cnt = 0; > - struct kfd_dev *kfd = pm->dqm->dev; > - > - /* Determine the number of processes to map together to HW: > - * it can not exceed the number of VMIDs available to the > - * scheduler, and it is determined by the smaller of the number > - * of processes in the runlist and kfd module parameter > - * hws_max_conc_proc. > - * Note: the arbitration between the number of VMIDs and > - * hws_max_conc_proc has been done in > - * kgd2kfd_device_init(). > - */ > - concurrent_proc_cnt = min(pm->dqm->processes_count, > - kfd->max_proc_per_quantum); > - > - > - packet = (struct pm4_mes_runlist *)buffer; > - > - memset(buffer, 0, sizeof(struct pm4_mes_runlist)); > - packet->header.u32All = pm_build_pm4_header(IT_RUN_LIST, > - sizeof(struct pm4_mes_runlist)); > - > - packet->bitfields4.ib_size = ib_size_in_dwords; > - packet->bitfields4.chain = chain ? 1 : 0; > - packet->bitfields4.offload_polling = 0; > - packet->bitfields4.valid = 1; > - packet->bitfields4.process_cnt = concurrent_proc_cnt; > - packet->ordinal2 = lower_32_bits(ib); > - packet->ib_base_hi = upper_32_bits(ib); > - > - return 0; > -} > - > -static int pm_map_queues_v10(struct packet_manager *pm, uint32_t *buffer, > - struct queue *q, bool is_static) > -{ > - struct pm4_mes_map_queues *packet; > - bool use_static = is_static; > - > - packet = (struct pm4_mes_map_queues *)buffer; > - memset(buffer, 0, sizeof(struct pm4_mes_map_queues)); > - > - packet->header.u32All = pm_build_pm4_header(IT_MAP_QUEUES, > - sizeof(struct pm4_mes_map_queues)); > - packet->bitfields2.num_queues = 1; > - packet->bitfields2.queue_sel = > - queue_sel__mes_map_queues__map_to_hws_determined_queue_slots_vi; > - > - packet->bitfields2.engine_sel = > - engine_sel__mes_map_queues__compute_vi; > - packet->bitfields2.queue_type = > - queue_type__mes_map_queues__normal_compute_vi; > - > - switch (q->properties.type) { > - case KFD_QUEUE_TYPE_COMPUTE: > - if (use_static) > - packet->bitfields2.queue_type = > - queue_type__mes_map_queues__normal_latency_static_queue_vi; > - break; > - case KFD_QUEUE_TYPE_DIQ: > - packet->bitfields2.queue_type = > - queue_type__mes_map_queues__debug_interface_queue_vi; > - break; > - case KFD_QUEUE_TYPE_SDMA: > - case KFD_QUEUE_TYPE_SDMA_XGMI: > - packet->bitfields2.engine_sel = q->properties.sdma_engine_id + > - engine_sel__mes_map_queues__sdma0_vi; > - use_static = false; /* no static queues under SDMA */ > - break; > - default: > - WARN(1, "queue type %d\n", q->properties.type); > - return -EINVAL; > - } > - packet->bitfields3.doorbell_offset = > - q->properties.doorbell_off; > - > - packet->mqd_addr_lo = > - lower_32_bits(q->gart_mqd_addr); > - > - packet->mqd_addr_hi = > - upper_32_bits(q->gart_mqd_addr); > - > - packet->wptr_addr_lo = > - lower_32_bits((uint64_t)q->properties.write_ptr); > - > - packet->wptr_addr_hi = > - upper_32_bits((uint64_t)q->properties.write_ptr); > - > - return 0; > -} > - > -static int pm_unmap_queues_v10(struct packet_manager *pm, uint32_t *buffer, > - enum kfd_queue_type type, > - enum kfd_unmap_queues_filter filter, > - uint32_t filter_param, bool reset, > - unsigned int sdma_engine) > -{ > - struct pm4_mes_unmap_queues *packet; > - > - packet = (struct pm4_mes_unmap_queues *)buffer; > - memset(buffer, 0, sizeof(struct pm4_mes_unmap_queues)); > - > - packet->header.u32All = pm_build_pm4_header(IT_UNMAP_QUEUES, > - sizeof(struct pm4_mes_unmap_queues)); > - switch (type) { > - case KFD_QUEUE_TYPE_COMPUTE: > - case KFD_QUEUE_TYPE_DIQ: > - packet->bitfields2.engine_sel = > - engine_sel__mes_unmap_queues__compute; > - break; > - case KFD_QUEUE_TYPE_SDMA: > - case KFD_QUEUE_TYPE_SDMA_XGMI: > - packet->bitfields2.engine_sel = > - engine_sel__mes_unmap_queues__sdma0 + sdma_engine; > - break; > - default: > - WARN(1, "queue type %d\n", type); > - break; > - } > - > - if (reset) > - packet->bitfields2.action =""> > - action__mes_unmap_queues__reset_queues; > - else > - packet->bitfields2.action =""> > - action__mes_unmap_queues__preempt_queues; > - > - switch (filter) { > - case KFD_UNMAP_QUEUES_FILTER_SINGLE_QUEUE: > - packet->bitfields2.queue_sel = > - queue_sel__mes_unmap_queues__perform_request_on_specified_queues; > - packet->bitfields2.num_queues = 1; > - packet->bitfields3b.doorbell_offset0 = filter_param; > - break; > - case KFD_UNMAP_QUEUES_FILTER_BY_PASID: > - packet->bitfields2.queue_sel = > - queue_sel__mes_unmap_queues__perform_request_on_pasid_queues; > - packet->bitfields3a.pasid = filter_param; > - break; > - case KFD_UNMAP_QUEUES_FILTER_ALL_QUEUES: > - packet->bitfields2.queue_sel = > - queue_sel__mes_unmap_queues__unmap_all_queues; > - break; > - case KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES: > - /* in this case, we do not preempt static queues */ > - packet->bitfields2.queue_sel = > - queue_sel__mes_unmap_queues__unmap_all_non_static_queues; > - break; > - default: > - WARN(1, "filter %d\n", filter); > - break; > - } > - > - return 0; > - > -} > - > -static int pm_query_status_v10(struct packet_manager *pm, uint32_t *buffer, > - uint64_t fence_address, uint32_t fence_value) > -{ > - struct pm4_mes_query_status *packet; > - > - packet = (struct pm4_mes_query_status *)buffer; > - memset(buffer, 0, sizeof(struct pm4_mes_query_status)); > - > - > - packet->header.u32All = pm_build_pm4_header(IT_QUERY_STATUS, > - sizeof(struct pm4_mes_query_status)); > - > - packet->bitfields2.context_id = 0; > - packet->bitfields2.interrupt_sel = > - interrupt_sel__mes_query_status__completion_status; > - packet->bitfields2.command = > - command__mes_query_status__fence_only_after_write_ack; > - > - packet->addr_hi = upper_32_bits((uint64_t)fence_address); > - packet->addr_lo = lower_32_bits((uint64_t)fence_address); > - packet->data_hi = upper_32_bits((uint64_t)fence_value); > - packet->data_lo = lower_32_bits((uint64_t)fence_value); > - > - return 0; > -} > - > -const struct packet_manager_funcs kfd_v10_pm_funcs = { > - .map_process = pm_map_process_v10, > - .runlist = pm_runlist_v10, > - .set_resources = pm_set_resources_vi, > - .map_queues = pm_map_queues_v10, > - .unmap_queues = pm_unmap_queues_v10, > - .query_status = pm_query_status_v10, > - .release_mem = NULL, > - .map_process_size = sizeof(struct pm4_mes_map_process), > - .runlist_size = sizeof(struct pm4_mes_runlist), > - .set_resources_size = sizeof(struct pm4_mes_set_resources), > - .map_queues_size = sizeof(struct pm4_mes_map_queues), > - .unmap_queues_size = sizeof(struct pm4_mes_unmap_queues), > - .query_status_size = sizeof(struct pm4_mes_query_status), > - .release_mem_size = 0, > -}; > - > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c > index f0e4910a8865..9e0eaf446bab 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c > @@ -25,6 +25,7 @@ > #include "kfd_device_queue_manager.h" > #include "kfd_pm4_headers_ai.h" > #include "kfd_pm4_opcodes.h" > +#include "gc/gc_10_1_0_sh_mask.h" > > static bool initialize_v9(struct kernel_queue *kq, struct kfd_dev *dev, > enum kfd_queue_type type, unsigned int queue_size) > @@ -85,10 +86,17 @@ static int pm_map_process_v9(struct packet_manager *pm, > > packet->sh_mem_config = qpd->sh_mem_config; > packet->sh_mem_bases = qpd->sh_mem_bases; > - packet->sq_shader_tba_lo = lower_32_bits(qpd->tba_addr >> 8); > - packet->sq_shader_tba_hi = upper_32_bits(qpd->tba_addr >> 8); > - packet->sq_shader_tma_lo = lower_32_bits(qpd->tma_addr >> 8); > - packet->sq_shader_tma_hi = upper_32_bits(qpd->tma_addr >> 8); > + if (qpd->tba_addr) { > + packet->sq_shader_tba_lo = lower_32_bits(qpd->tba_addr >> 8); > + /* On GFX9, unlike GFX10, bit TRAP_EN of SQ_SHADER_TBA_HI is > + * not defined, so setting it won't do any harm. > + */ > + packet->sq_shader_tba_hi = upper_32_bits(qpd->tba_addr >> 8) > + | 1 << SQ_SHADER_TBA_HI__TRAP_EN__SHIFT; > + > + packet->sq_shader_tma_lo = lower_32_bits(qpd->tma_addr >> 8); > + packet->sq_shader_tma_hi = upper_32_bits(qpd->tma_addr >> 8); > + } > > packet->gds_addr_lo = lower_32_bits(qpd->gds_context_area); > packet->gds_addr_hi = upper_32_bits(qpd->gds_context_area); > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c > index 83ef4b3dd2fb..700be4f80867 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c > @@ -241,12 +241,10 @@ int pm_init(struct packet_manager *pm, struct device_queue_manager *dqm) > case CHIP_RAVEN: > case CHIP_RENOIR: > case CHIP_ARCTURUS: > - pm->pmf = &kfd_v9_pm_funcs; > - break; > case CHIP_NAVI10: > case CHIP_NAVI12: > case CHIP_NAVI14: > - pm->pmf = &kfd_v10_pm_funcs; > + pm->pmf = &kfd_v9_pm_funcs; > break; > default: > WARN(1, "Unexpected ASIC family %u", > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > index 62db4d20ed32..f2078bfb8ae1 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h > @@ -977,7 +977,6 @@ struct packet_manager_funcs { > > extern const struct packet_manager_funcs kfd_vi_pm_funcs; > extern const struct packet_manager_funcs kfd_v9_pm_funcs; > -extern const struct packet_manager_funcs kfd_v10_pm_funcs; > > int pm_init(struct packet_manager *pm, struct device_queue_manager *dqm); > void pm_uninit(struct packet_manager *pm); > @@ -996,9 +995,6 @@ void pm_release_ib(struct packet_manager *pm); > > /* Following PM funcs can be shared among VI and AI */ > unsigned int pm_build_pm4_header(unsigned int opcode, size_t packet_size); > -int pm_set_resources_vi(struct packet_manager *pm, uint32_t *buffer, > - struct scheduling_resources *res); > - > > uint64_t kfd_get_number_elems(struct kfd_dev *kfd); > |
_______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
