[PATCH 1/2] drm/amdkfd: initialise kfd inside amdgpu_device_init
From: pding Also finalize kfd inside amdgpu_device_fini. kfd device_init needs SRIOV exclusive accessing. Try to gather exclusive accessing to reduce time consuming. Signed-off-by: pding --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c| 5 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 7b7439f..7161af2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1693,6 +1693,8 @@ static int amdgpu_early_init(struct amdgpu_device *adev) if (r) return r; + amdgpu_amdkfd_device_probe(adev); + if (amdgpu_sriov_vf(adev)) { r = amdgpu_virt_request_full_gpu(adev, true); if (r) @@ -1787,6 +1789,7 @@ static int amdgpu_init(struct amdgpu_device *adev) adev->ip_blocks[i].status.hw = true; } + amdgpu_amdkfd_device_init(adev); return 0; } @@ -1854,6 +1857,7 @@ static int amdgpu_fini(struct amdgpu_device *adev) { int i, r; + amdgpu_amdkfd_device_fini(adev); /* need to disable SMC first */ for (i = 0; i < adev->num_ip_blocks; i++) { if (!adev->ip_blocks[i].status.hw) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 3e9760d..c5d2419 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -63,8 +63,6 @@ void amdgpu_driver_unload_kms(struct drm_device *dev) pm_runtime_forbid(dev->dev); } - amdgpu_amdkfd_device_fini(adev); - amdgpu_acpi_fini(adev); amdgpu_device_fini(adev); @@ -170,9 +168,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags) "Error during ACPI methods call\n"); } - amdgpu_amdkfd_device_probe(adev); - amdgpu_amdkfd_device_init(adev); - if (amdgpu_device_is_px(dev)) { pm_runtime_use_autosuspend(dev->dev); pm_runtime_set_autosuspend_delay(dev->dev, 5000); -- 2.9.5 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
release ex mode after hw_init
Hi Felix, Please review. [PATCH 1/2] drm/amdkfd: initialise kfd inside amdgpu_device_init As you suggested, move kfd init/fini inside amdgpu_device_init. Other changes for KFD interfaces are dropped. [PATCH 2/2] drm/amdgpu: release exclusive mode after hw_init ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/2] drm/amdgpu: release exclusive mode after hw_init
From: pding Signed-off-by: pding --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c| 3 --- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 7161af2..6f01dda 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -1790,6 +1790,10 @@ static int amdgpu_init(struct amdgpu_device *adev) } amdgpu_amdkfd_device_init(adev); + + if (amdgpu_sriov_vf(adev)) + amdgpu_virt_release_full_gpu(adev, true); + return 0; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index c5d2419..1d56b5b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -177,9 +177,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned long flags) pm_runtime_put_autosuspend(dev->dev); } - if (amdgpu_sriov_vf(adev)) - amdgpu_virt_release_full_gpu(adev, true); - out: if (r) { /* balance pm_runtime_get_sync in amdgpu_driver_unload_kms */ -- 2.9.5 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amd/dc: Add dc display driver (v2)
Hello Harry Wentland, The patch 4562236b3bc0: "drm/amd/dc: Add dc display driver (v2)" from Sep 12, 2017, leads to the following static checker warning: drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:117 get_reg_field_value_ex() warn: mask and shift to zero drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c 1357 void dce110_link_encoder_enable_hpd(struct link_encoder *enc) 1358 { 1359 struct dce110_link_encoder *enc110 = TO_DCE110_LINK_ENC(enc); 1360 struct dc_context *ctx = enc110->base.ctx; 1361 uint32_t addr = HPD_REG(DC_HPD_CONTROL); 1362 uint32_t hpd_enable = 0; ^^ This is always just zero so the static checker complains: 1363 uint32_t value = dm_read_reg(ctx, addr); 1364 1365 get_reg_field_value(hpd_enable, DC_HPD_CONTROL, DC_HPD_EN); ^^ When we pass zero to here, we know that the answer is always going to be zero. But then we ignore the result from get_reg_field_value() and I'm going to change Smatch so that that also generates a static checker warning. What's going on??? 1366 1367 if (hpd_enable == 0) ^^^ This is true always. 1368 set_reg_field_value(value, 1, DC_HPD_CONTROL, DC_HPD_EN); And we don't use the return for this either? ARgh!! This whole function is a no-op? My Brian is melting??? 1369 } regards, dan carpenter ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: small cleanup in destruct()
Am 06.11.2017 um 08:07 schrieb Dan Carpenter: Static analysis tools get annoyed that we don't indent this if statement. Actually, the if statement isn't required because kfree() can handle NULL pointers just fine. The DCE110STRENC_FROM_STRENC() macro is a wrapper around container_of() but it's basically a no-op or a cast. Anyway, it's not really appropriate here so it should be removed as well. Signed-off-by: Dan Carpenter Acked-by: Christian König --- v2: in v1 I just added a tab diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c index d911590d08bc..4c4bd72d4e40 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c @@ -725,10 +725,8 @@ static void destruct(struct dcn10_resource_pool *pool) } } - for (i = 0; i < pool->base.stream_enc_count; i++) { - if (pool->base.stream_enc[i] != NULL) - kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i])); - } + for (i = 0; i < pool->base.stream_enc_count; i++) + kfree(pool->base.stream_enc[i]); for (i = 0; i < pool->base.audio_count; i++) { if (pool->base.audios[i]) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: resize VRAM BAR for CPU access v5
Am 06.11.2017 um 08:21 schrieb Ding, Pixel: Hi Christian, The latest driver fails to load on SRIOV VF with xen hypervisor driver. +int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev) +{ + u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size); + u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) - 1; + u16 cmd; + int r; + + /* Disable memory decoding while we change the BAR addresses and size */ + pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd); + pci_write_config_word(adev->pdev, PCI_COMMAND, The function above introduces 900ms latency during init in exclusive accessing. + cmd & ~PCI_COMMAND_MEMORY); + Can we bypass disablement of response in memory space here? Any concern or suggestion? Mhm, that was added to be on the extra safe side. In theory we can skip it. Does PCI BAR resize work on SRIOV if you skip this or is that just a NOP anyway? Might be a good idea to just immediately return here when SRIOV is active. Regards, Christian. — Sincerely Yours, Pixel On 02/11/2017, 9:13 PM, "amd-gfx on behalf of Alex Deucher" wrote: On Thu, Nov 2, 2017 at 9:02 AM, Christian König wrote: From: Christian König Try to resize BAR0 to let CPU access all of VRAM. v2: rebased, style cleanups, disable mem decode before resize, handle gmc_v9 as well, round size up to power of two. v3: handle gmc_v6 as well, release and reassign all BARs in the driver. v4: rename new function to amdgpu_device_resize_fb_bar, reenable mem decoding only if all resources are assigned. v5: reorder resource release, return -ENODEV instead of BUG_ON(). Signed-off-by: Christian König Reviewed-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 48 ++ drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 12 ++-- drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 13 ++-- drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 13 ++-- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 14 ++--- 6 files changed, 88 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 2730a75..4f919d3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1854,6 +1854,7 @@ void amdgpu_ttm_placement_from_domain(struct amdgpu_bo *abo, u32 domain); bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo); void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base); void amdgpu_gart_location(struct amdgpu_device *adev, struct amdgpu_mc *mc); +int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev); void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size); int amdgpu_ttm_init(struct amdgpu_device *adev); void amdgpu_ttm_fini(struct amdgpu_device *adev); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 8b33adf..cb3a0ac 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -410,6 +410,9 @@ static int amdgpu_doorbell_init(struct amdgpu_device *adev) return 0; } + if (pci_resource_flags(adev->pdev, 2) & IORESOURCE_UNSET) + return -EINVAL; + /* doorbell bar mapping */ adev->doorbell.base = pci_resource_start(adev->pdev, 2); adev->doorbell.size = pci_resource_len(adev->pdev, 2); @@ -749,6 +752,51 @@ int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev) return r; } +/** + * amdgpu_device_resize_fb_bar - try to resize FB BAR + * + * @adev: amdgpu_device pointer + * + * Try to resize FB BAR to make all VRAM CPU accessible. We try very hard not + * to fail, but if any of the BARs is not accessible after the size we abort + * driver loading by returning -ENODEV. + */ +int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev) +{ + u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size); + u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) - 1; + u16 cmd; + int r; + + /* Disable memory decoding while we change the BAR addresses and size */ + pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd); + pci_write_config_word(adev->pdev, PCI_COMMAND, + cmd & ~PCI_COMMAND_MEMORY); + + /* Free the VRAM and doorbell BAR, we most likely need to move both. */ + amdgpu_doorbell_fini(adev); + if (adev->asic_type >= CHIP_BONAIRE) + pci_release_resource(adev->pdev, 2); + + pci_release_resource(adev->pdev, 0); + + r = pci_resize_resource(adev->pdev, 0, rbar_size); + if (r == -ENOSPC) + DRM_INFO("Not enough PCI address space for a large BAR."); + else if (r && r != -ENOTSUPP) + DRM_ERROR("Problem resizing BAR0 (%d).", r); + + pci_assign_unassigned_bus_resources(
Re: [PATCH] drm/amdgpu:remove debugfs file in amdgpu_device_finish
Am 06.11.2017 um 10:20 schrieb Gary Sun: remove debugfs file in amdgpu_device_finish NAK, the debugfs files are removed automatically by drm_debugfs_cleanup(). So that patch is unnecessary. Regards, Christian. Signed-off-by: Gary Sun --- drivers/gpu/drm/amd/amdgpu/amdgpu.h|1 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++ 2 files changed, 19 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 4f919d3..6cfcb5f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1250,6 +1250,7 @@ struct amdgpu_debugfs { int amdgpu_debugfs_add_files(struct amdgpu_device *adev, const struct drm_info_list *files, unsigned nfiles); +int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev); int amdgpu_debugfs_fence_init(struct amdgpu_device *adev); #if defined(CONFIG_DEBUG_FS) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 7b7439f..ee800ab 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2520,6 +2520,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev) amdgpu_doorbell_fini(adev); amdgpu_pm_sysfs_fini(adev); amdgpu_debugfs_regs_cleanup(adev); + amdgpu_debugfs_cleanup_files(adev); } @@ -3304,6 +3305,23 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev, return 0; } +int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev) +{ + unsigned int i; + + for (i = 0; i < adev->debugfs_count; i++) { +#if defined(CONFIG_DEBUG_FS) + drm_debugfs_remove_files(adev->debugfs[i].files, + adev->debugfs[i].num_files, + adev->ddev->primary); +#endif + adev->debugfs[i].files = NUL; + adev->debugfs[i].num_files = 0; + } + adev->debugfs_count = 0; + return 0; +} + #if defined(CONFIG_DEBUG_FS) static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf, ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/powerplay: suppress KASAN out of bounds warning in vega10_populate_all_memory_levels
Change-Id: I437e3e08cd48943de277c5d3eefdbaf21fd6a489 Signed-off-by: Evan Quan --- drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c index 7079e61..0364a96 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c @@ -1807,6 +1807,10 @@ static int vega10_populate_all_memory_levels(struct pp_hwmgr *hwmgr) mem_channels = (cgs_read_register(hwmgr->device, reg) & DF_CS_AON0_DramBaseAddress0__IntLvNumChan_MASK) >> DF_CS_AON0_DramBaseAddress0__IntLvNumChan__SHIFT; + PP_ASSERT_WITH_CODE(mem_channels < ARRAY_SIZE(channel_number), + "Mem Channel Index Exceeded maximum!", + return -1); + pp_table->NumMemoryChannels = cpu_to_le16(mem_channels); pp_table->MemoryChannelWidth = cpu_to_le16(HBM_MEMORY_CHANNEL_WIDTH * -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: resize VRAM BAR for CPU access v5
What benefits will the larger VRAM bar bring in, or are there new features relying on larger VRAM bar under development? SRIOV wants to leverage this sort of optimization too. I think it’s better to try something which can keep this change for SRIOV and don’t introduce too much latency. — Sincerely Yours, Pixel On 06/11/2017, 5:16 PM, "Christian König" wrote: >Am 06.11.2017 um 08:21 schrieb Ding, Pixel: >> Hi Christian, >> >> The latest driver fails to load on SRIOV VF with xen hypervisor driver. >> >> +int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev) >> +{ >> + u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size); >> + u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) - 1; >> + u16 cmd; >> + int r; >> + >> + /* Disable memory decoding while we change the BAR addresses and size >> */ >> + pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd); >> + pci_write_config_word(adev->pdev, PCI_COMMAND, >> >> The function above introduces 900ms latency during init in exclusive >> accessing. >> >> >> + cmd & ~PCI_COMMAND_MEMORY); >> + >> >> >> Can we bypass disablement of response in memory space here? Any concern or >> suggestion? > >Mhm, that was added to be on the extra safe side. In theory we can skip it. > >Does PCI BAR resize work on SRIOV if you skip this or is that just a NOP >anyway? > >Might be a good idea to just immediately return here when SRIOV is active. > >Regards, >Christian. > >> >> >> — >> Sincerely Yours, >> Pixel >> >> >> >> >> >> >> >> On 02/11/2017, 9:13 PM, "amd-gfx on behalf of Alex Deucher" >> >> wrote: >> >>> On Thu, Nov 2, 2017 at 9:02 AM, Christian König >>> wrote: From: Christian König Try to resize BAR0 to let CPU access all of VRAM. v2: rebased, style cleanups, disable mem decode before resize, handle gmc_v9 as well, round size up to power of two. v3: handle gmc_v6 as well, release and reassign all BARs in the driver. v4: rename new function to amdgpu_device_resize_fb_bar, reenable mem decoding only if all resources are assigned. v5: reorder resource release, return -ENODEV instead of BUG_ON(). Signed-off-by: Christian König >>> Reviewed-by: Alex Deucher >>> --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 48 ++ drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 12 ++-- drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 13 ++-- drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 13 ++-- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 14 ++--- 6 files changed, 88 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 2730a75..4f919d3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1854,6 +1854,7 @@ void amdgpu_ttm_placement_from_domain(struct amdgpu_bo *abo, u32 domain); bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo); void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base); void amdgpu_gart_location(struct amdgpu_device *adev, struct amdgpu_mc *mc); +int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev); void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size); int amdgpu_ttm_init(struct amdgpu_device *adev); void amdgpu_ttm_fini(struct amdgpu_device *adev); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 8b33adf..cb3a0ac 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -410,6 +410,9 @@ static int amdgpu_doorbell_init(struct amdgpu_device *adev) return 0; } + if (pci_resource_flags(adev->pdev, 2) & IORESOURCE_UNSET) + return -EINVAL; + /* doorbell bar mapping */ adev->doorbell.base = pci_resource_start(adev->pdev, 2); adev->doorbell.size = pci_resource_len(adev->pdev, 2); @@ -749,6 +752,51 @@ int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev) return r; } +/** + * amdgpu_device_resize_fb_bar - try to resize FB BAR + * + * @adev: amdgpu_device pointer + * + * Try to resize FB BAR to make all VRAM CPU accessible. We try very hard not + * to fail, but if any of the BARs is not accessible after the size we abort + * driver loading by returning -ENODEV. + */ +int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev) +{ + u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size); + u32 rbar_size = order_base_2(((space
Re: [PATCH] drm/amd/powerplay: suppress KASAN out of bounds warning in vega10_populate_all_memory_levels
Am 06.11.2017 um 10:33 schrieb Evan Quan: Change-Id: I437e3e08cd48943de277c5d3eefdbaf21fd6a489 Signed-off-by: Evan Quan Tested-and-Acked-by: Christian König --- drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c index 7079e61..0364a96 100644 --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c @@ -1807,6 +1807,10 @@ static int vega10_populate_all_memory_levels(struct pp_hwmgr *hwmgr) mem_channels = (cgs_read_register(hwmgr->device, reg) & DF_CS_AON0_DramBaseAddress0__IntLvNumChan_MASK) >> DF_CS_AON0_DramBaseAddress0__IntLvNumChan__SHIFT; + PP_ASSERT_WITH_CODE(mem_channels < ARRAY_SIZE(channel_number), + "Mem Channel Index Exceeded maximum!", + return -1); + pp_table->NumMemoryChannels = cpu_to_le16(mem_channels); pp_table->MemoryChannelWidth = cpu_to_le16(HBM_MEMORY_CHANNEL_WIDTH * ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[bug report] drm/amd/display : add high part address calculation for underlay
Hello Shirish S, The patch 4d3e00dad80a: "drm/amd/display : add high part address calculation for underlay" from Oct 19, 2017, leads to the following static checker warning: drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:1838 fill_plane_attributes_from_fb() warn: cast after binop drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c 1831 } else { 1832 awidth = ALIGN(fb->width, 64); 1833 plane_state->address.type = PLN_ADDR_TYPE_VIDEO_PROGRESSIVE; 1834 plane_state->address.video_progressive.luma_addr.low_part 1835 = lower_32_bits(fb_location); 1836 plane_state->address.video_progressive.luma_addr.high_part 1837 = upper_32_bits(fb_location); 1838 chroma_addr = fb_location + (u64)(awidth * fb->height); ^ This cast is a no-op. fb_location is a u64. awidth and fb->height are both unsigned int. Perhaps you meant to do: chroma_addr = fb_location + ((u64)awidth * fb->height); Or maybe just remove the cast? It doesn't help readability, because if it did I wouldn't be so confused. 1839 plane_state->address.video_progressive.chroma_addr.low_part 1840 = lower_32_bits(chroma_addr); 1841 plane_state->address.video_progressive.chroma_addr.high_part 1842 = upper_32_bits(chroma_addr); 1843 plane_state->plane_size.video.luma_size.x = 0; 1844 plane_state->plane_size.video.luma_size.y = 0; 1845 plane_state->plane_size.video.luma_size.width = awidth; 1846 plane_state->plane_size.video.luma_size.height = fb->height; 1847 /* TODO: unhardcode */ 1848 plane_state->plane_size.video.luma_pitch = awidth; regards, dan carpenter ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: resize VRAM BAR for CPU access v5
What benefits will the larger VRAM bar bring in, Well the obvious one that the CPU can access all of VRAM. There are some games/workloads which rely quite a bit on this and it simplifies MM largely when you don't need to shuffle buffers around for CPU access. or are there new features relying on larger VRAM bar under development? Not that I know of. I think it’s better to try something which can keep this change for SRIOV and don’t introduce too much latency. Agree on that, but reprogramming the PCI BAR is something which takes quite a bunch of time. That's why I asked if it is sufficient if you comment out this one pci_write_config_word() and it works? If yes than we can just skip that write, but I have doubts that this will be sufficient. On the other hand in an SRIOV environment the host can resize the BAR before starting the client, so that whole stuff shouldn't be necessary in the first place. Regards, Christian. Am 06.11.2017 um 10:34 schrieb Ding, Pixel: What benefits will the larger VRAM bar bring in, or are there new features relying on larger VRAM bar under development? SRIOV wants to leverage this sort of optimization too. I think it’s better to try something which can keep this change for SRIOV and don’t introduce too much latency. — Sincerely Yours, Pixel On 06/11/2017, 5:16 PM, "Christian König" wrote: Am 06.11.2017 um 08:21 schrieb Ding, Pixel: Hi Christian, The latest driver fails to load on SRIOV VF with xen hypervisor driver. +int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev) +{ + u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size); + u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) - 1; + u16 cmd; + int r; + + /* Disable memory decoding while we change the BAR addresses and size */ + pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd); + pci_write_config_word(adev->pdev, PCI_COMMAND, The function above introduces 900ms latency during init in exclusive accessing. + cmd & ~PCI_COMMAND_MEMORY); + Can we bypass disablement of response in memory space here? Any concern or suggestion? Mhm, that was added to be on the extra safe side. In theory we can skip it. Does PCI BAR resize work on SRIOV if you skip this or is that just a NOP anyway? Might be a good idea to just immediately return here when SRIOV is active. Regards, Christian. — Sincerely Yours, Pixel On 02/11/2017, 9:13 PM, "amd-gfx on behalf of Alex Deucher" wrote: On Thu, Nov 2, 2017 at 9:02 AM, Christian König wrote: From: Christian König Try to resize BAR0 to let CPU access all of VRAM. v2: rebased, style cleanups, disable mem decode before resize, handle gmc_v9 as well, round size up to power of two. v3: handle gmc_v6 as well, release and reassign all BARs in the driver. v4: rename new function to amdgpu_device_resize_fb_bar, reenable mem decoding only if all resources are assigned. v5: reorder resource release, return -ENODEV instead of BUG_ON(). Signed-off-by: Christian König Reviewed-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 48 ++ drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 12 ++-- drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 13 ++-- drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 13 ++-- drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 14 ++--- 6 files changed, 88 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 2730a75..4f919d3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1854,6 +1854,7 @@ void amdgpu_ttm_placement_from_domain(struct amdgpu_bo *abo, u32 domain); bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo); void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, u64 base); void amdgpu_gart_location(struct amdgpu_device *adev, struct amdgpu_mc *mc); +int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev); void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size); int amdgpu_ttm_init(struct amdgpu_device *adev); void amdgpu_ttm_fini(struct amdgpu_device *adev); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 8b33adf..cb3a0ac 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -410,6 +410,9 @@ static int amdgpu_doorbell_init(struct amdgpu_device *adev) return 0; } + if (pci_resource_flags(adev->pdev, 2) & IORESOURCE_UNSET) + return -EINVAL; + /* doorbell bar mapping */ adev->doorbell.base = pci_resource_start(adev->pdev, 2); adev->doorbell.size = pci_resource_len(adev->pdev, 2); @@ -749,6 +752,51 @@ int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev
Re: [PATCH umr] Skip ahead if PDE entry is actually a PTE entry. (v2)
Am 04.11.2017 um 18:15 schrieb Tom St Denis: Signed-off-by: Tom St Denis Still not perfect, but good enough for now. Patch is Tested-by: Christian König . I think you need to rework the VM walking a bit, cause we need to support the T bit as well in the future and your code make a few assumptions which doesn't allow that. Regards, Christian. (v2) Don't print out PDE entries with PTE bit set --- src/lib/read_vram.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/lib/read_vram.c b/src/lib/read_vram.c index 0df48dadec12..51823d71021e 100644 --- a/src/lib/read_vram.c +++ b/src/lib/read_vram.c @@ -509,7 +509,7 @@ static int umr_access_vram_ai(struct umr_asic *asic, uint32_t vmid, pde_fields.system= (pde_entry >> 1) & 1; pde_fields.cache = (pde_entry >> 2) & 1; pde_fields.pte = (pde_entry >> 54) & 1; - if (memcmp(&pde_fields, &pde_array[pde_cnt], sizeof pde_fields) && asic->options.verbose) + if (!pde_fields.pte && memcmp(&pde_fields, &pde_array[pde_cnt], sizeof pde_fields) && asic->options.verbose) fprintf(stderr, "[VERBOSE]: %s PDE%d=0x%016llx, VA=0x%012llx, PBA==0x%012llx, V=%d, S=%d, C=%d, P=%d\n", &indentation[12-pde_cnt*3], pde_cnt, @@ -522,6 +522,11 @@ static int umr_access_vram_ai(struct umr_asic *asic, uint32_t vmid, (int)pde_fields.pte); memcpy(&pde_array[pde_cnt++], &pde_fields, sizeof pde_fields); +if (pde_fields.pte) { + pte_entry = pde_entry; + goto pde_is_pte; + } + if (!pde_fields.system) pde_fields.pte_base_addr -= vm_fb_offset; @@ -539,6 +544,7 @@ static int umr_access_vram_ai(struct umr_asic *asic, uint32_t vmid, return -1; // decode PTE values +pde_is_pte: pte_fields.page_base_addr = pte_entry & 0xFF000ULL; pte_fields.fragment = (pte_entry >> 7) & 0x1F; pte_fields.system = (pte_entry >> 1) & 1; ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu:remove debugfs file in amdgpu_device_finish
remove debugfs file in amdgpu_device_finish Signed-off-by: Gary Sun --- drivers/gpu/drm/amd/amdgpu/amdgpu.h|1 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++ 2 files changed, 19 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 4f919d3..6cfcb5f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1250,6 +1250,7 @@ struct amdgpu_debugfs { int amdgpu_debugfs_add_files(struct amdgpu_device *adev, const struct drm_info_list *files, unsigned nfiles); +int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev); int amdgpu_debugfs_fence_init(struct amdgpu_device *adev); #if defined(CONFIG_DEBUG_FS) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 7b7439f..ee800ab 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2520,6 +2520,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev) amdgpu_doorbell_fini(adev); amdgpu_pm_sysfs_fini(adev); amdgpu_debugfs_regs_cleanup(adev); + amdgpu_debugfs_cleanup_files(adev); } @@ -3304,6 +3305,23 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev, return 0; } +int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev) +{ + unsigned int i; + + for (i = 0; i < adev->debugfs_count; i++) { +#if defined(CONFIG_DEBUG_FS) + drm_debugfs_remove_files(adev->debugfs[i].files, + adev->debugfs[i].num_files, + adev->ddev->primary); +#endif + adev->debugfs[i].files = NUL; + adev->debugfs[i].num_files = 0; + } + adev->debugfs_count = 0; + return 0; +} + #if defined(CONFIG_DEBUG_FS) static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf, -- 1.7.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
AMD, please run Smatch on your driver
Linux-next was offline for the last month and the AMD drm driver went through major changes. Anyway, I'm a bit overwhelmed by the number of warnings and I'm not going to be able to go through them all so I'm just sending them to you unfiltered. Part of the problem is that I'm not running the released version of Smatch myself. That has two effects. 1) The released version is crappier than I had imagined. 2) I get *way* more warnings than you see which is overwhelming... So this is mostly my fault and I will try to do better. Here are the current warnings from Friday's linux-next, lightly edited. I know that everyone hates a big dump of static checker warnings... Speaking of being ignored, I sent a fix for this one back in August but never heard back: drivers/gpu/drm/amd/amdgpu/ci_dpm.c:4553 ci_set_mc_special_registers() error: buffer overflow 'table->mc_reg_address' 16 <= 16 https://lists.freedesktop.org/archives/amd-gfx/2017-August/012333.html So this is partly your fault as well because if you cleaned up static checker warnings little by little, then they wouldn't pile up like this. Eventually, everyone is going to have to start running Smatch for themselves because it scales better than relying on me to do it. regards, dan carpenter drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:2224 amdgpu_device_init() warn: 'adev->rio_mem' was not released on error drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:2395 amdgpu_device_init() warn: 'adev->rio_mem' was not released on error drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3373 amdgpu_debugfs_regs_write() warn: 'mutex:&adev->pm.mutex' is sometimes locked here and sometimes unlocked. drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3377 amdgpu_debugfs_regs_write() warn: 'mutex:&adev->pm.mutex' is sometimes locked here and sometimes unlocked. drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3771 amdgpu_debugfs_gpr_read() error: buffer overflow 'data' 1024 <= 4095 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:155 amdgpu_driver_load_kms() warn: we tested 'r' before and it was 'false' drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c:689 amdgpu_gem_op_ioctl() warn: should 'robj->tbo.mem.page_alignment << 12' be a 64 bit type? drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:196 amdgpu_cs_parser_init() warn: 'mutex:&p->ctx->lock' is sometimes locked here and sometimes unlocked. drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:674 amdgpu_cs_parser_bos() warn: we tested 'r' before and it was 'false' drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:755 amdgpu_cs_parser_fini() warn: 'mutex:&parser->ctx->lock' is sometimes locked here and sometimes unlocked. drivers/gpu/drm/amd/amdgpu/atombios_i2c.c:72 amdgpu_atombios_i2c_process_i2c_ch() warn: impossible condition '(num > 255) => (0-255 > 255)' drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c:217 amdgpu_queue_mgr_map() warn: variable dereferenced before check 'mgr' (see line 215) drivers/gpu/drm/amd/amdgpu/kv_dpm.c:1618 kv_get_acp_boot_level() warn: always true condition '(table->entries[i]->clk >= 0) => (0-u32max >= 0)' drivers/gpu/drm/amd/amdgpu/ci_dpm.c:4560 ci_set_mc_special_registers() error: buffer overflow 'table->mc_reg_address' 16 <= 16 drivers/gpu/drm/amd/amdgpu/ci_dpm.c:5065 ci_request_link_speed_change_before_state_change() warn: missing break? reassigning 'pi->force_pcie_gen' drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:5256 gfx_v7_0_get_cu_info() error: buffer overflow 'cu_info->bitmap' 4 <= 4 drivers/gpu/drm/amd/amdgpu/si.c:1288 si_common_early_init() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/dce_v6_0.c:3026 dce_v6_0_pageflip_irq() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/si_dpm.c:6242 si_request_link_speed_change_before_state_change() warn: missing break? reassigning 'si_pi->force_pcie_gen' drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c:5222 gfx_v8_0_pre_soft_reset() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c:7105 gfx_v8_0_get_cu_info() error: buffer overflow 'cu_info->bitmap' 4 <= 4 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:3077 gfx_v9_0_soft_reset() warn: we tested 'grbm_soft_reset' before and it was 'true' drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:3644 gfx_v9_0_ring_emit_ib_gfx() warn: inconsistent indenting drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:4457 gfx_v9_0_get_cu_info() error: buffer overflow 'cu_info->bitmap' 4 <= 4 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c:605 amdgpu_cgs_lock_grbm_idx() warn: 'mutex:&adev->grbm_idx_mutex' is sometimes locked here and sometimes unlocked. drivers/gpu/drm/amd/amdgpu/../scheduler/gpu_scheduler.c:696 amd_sched_init() warn: call of 'kthread_create_on_node' with non-constant format argument drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/tonga_smumgr.c:3128 tonga_set_mc_special_registers() error: buffer overflow 'table->mc_reg_address' 16 <= 16 drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/polaris10_smumgr.c:916 polaris10_calculate_sclk_params() warn: should 'clock << table->SclkFcwRangeTable[sclk_setting->PllRange].postdiv' be a 64 bit type? drivers/gpu/drm/amd/amdgp
[PATCH] drm/amd/display: checking for NULL instead of IS_ERR()
backlight_device_register() never returns NULL, it returns error pointers on error so the check here is wrong. Signed-off-by: Dan Carpenter diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 33a15a1d818c..75f9383f5b9b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -1296,7 +1296,7 @@ amdgpu_dm_register_backlight_device(struct amdgpu_display_manager *dm) &amdgpu_dm_backlight_ops, &props); - if (NULL == dm->backlight_dev) + if (IS_ERR(dm->backlight_dev)) DRM_ERROR("DM: Backlight registration failed!\n"); else DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n", bl_name); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/display: remove some unneeded code
We assign "v_init = asic_blank_start;" a few lines earlier so there is no need to do it again inside the if statements. Also "v_init" is unsigned so it can't be less than zero. Signed-off-by: Dan Carpenter diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c index 1994865d4351..c7333cdf1802 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c @@ -236,13 +236,10 @@ static void tgn10_program_timing( if (tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT || tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT_MST || tg->dlg_otg_param.signal == SIGNAL_TYPE_EDP) { - v_init = asic_blank_start; start_point = 1; if (patched_crtc_timing.flags.INTERLACE == 1) field_num = 1; } - if (v_init < 0) - v_init = 0; v_fp2 = 0; if (tg->dlg_otg_param.vstartup_start > asic_blank_end) v_fp2 = tg->dlg_otg_param.vstartup_start > asic_blank_end; ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: checking for NULL instead of IS_ERR()
Am 06.11.2017 um 12:43 schrieb Dan Carpenter: backlight_device_register() never returns NULL, it returns error pointers on error so the check here is wrong. Signed-off-by: Dan Carpenter Acked-by: Christian König diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 33a15a1d818c..75f9383f5b9b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -1296,7 +1296,7 @@ amdgpu_dm_register_backlight_device(struct amdgpu_display_manager *dm) &amdgpu_dm_backlight_ops, &props); - if (NULL == dm->backlight_dev) + if (IS_ERR(dm->backlight_dev)) DRM_ERROR("DM: Backlight registration failed!\n"); else DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n", bl_name); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: remove some unneeded code
Am 06.11.2017 um 12:44 schrieb Dan Carpenter: We assign "v_init = asic_blank_start;" a few lines earlier so there is no need to do it again inside the if statements. Also "v_init" is unsigned so it can't be less than zero. Signed-off-by: Dan Carpenter Acked-by: Christian König diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c index 1994865d4351..c7333cdf1802 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c @@ -236,13 +236,10 @@ static void tgn10_program_timing( if (tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT || tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT_MST || tg->dlg_otg_param.signal == SIGNAL_TYPE_EDP) { - v_init = asic_blank_start; start_point = 1; if (patched_crtc_timing.flags.INTERLACE == 1) field_num = 1; } - if (v_init < 0) - v_init = 0; v_fp2 = 0; if (tg->dlg_otg_param.vstartup_start > asic_blank_end) v_fp2 = tg->dlg_otg_param.vstartup_start > asic_blank_end; ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH umr] Tidy up skipping PDE=>PTE mappings
Should fix the indentation now when a PDE entry is actually a PTE. Signed-off-by: Tom St Denis --- src/lib/read_vram.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/lib/read_vram.c b/src/lib/read_vram.c index 51823d71021e..89d55ff1bef6 100644 --- a/src/lib/read_vram.c +++ b/src/lib/read_vram.c @@ -509,20 +509,20 @@ static int umr_access_vram_ai(struct umr_asic *asic, uint32_t vmid, pde_fields.system= (pde_entry >> 1) & 1; pde_fields.cache = (pde_entry >> 2) & 1; pde_fields.pte = (pde_entry >> 54) & 1; - if (!pde_fields.pte && memcmp(&pde_fields, &pde_array[pde_cnt], sizeof pde_fields) && asic->options.verbose) - fprintf(stderr, "[VERBOSE]: %s PDE%d=0x%016llx, VA=0x%012llx, PBA==0x%012llx, V=%d, S=%d, C=%d, P=%d\n", - &indentation[12-pde_cnt*3], - pde_cnt, - (unsigned long long)pde_entry, - (unsigned long long)address & va_mask, - (unsigned long long)pde_fields.pte_base_addr, - (int)pde_fields.valid, - (int)pde_fields.system, - (int)pde_fields.cache, - (int)pde_fields.pte); - memcpy(&pde_array[pde_cnt++], &pde_fields, sizeof pde_fields); - - if (pde_fields.pte) { + if (!pde_fields.pte) { + if (memcmp(&pde_fields, &pde_array[pde_cnt], sizeof pde_fields) && asic->options.verbose) + fprintf(stderr, "[VERBOSE]: %s PDE%d=0x%016llx, VA=0x%012llx, PBA==0x%012llx, V=%d, S=%d, C=%d, P=%d\n", + &indentation[12-pde_cnt*3], + pde_cnt, + (unsigned long long)pde_entry, + (unsigned long long)address & va_mask, + (unsigned long long)pde_fields.pte_base_addr, + (int)pde_fields.valid, + (int)pde_fields.system, + (int)pde_fields.cache, + (int)pde_fields.pte); + memcpy(&pde_array[pde_cnt++], &pde_fields, sizeof pde_fields); + } else { pte_entry = pde_entry; goto pde_is_pte; } -- 2.12.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
radeon_dp_aux_transfer_native: 74 callbacks suppressed
Hi all, commit 92c177b7947d9c889ea7b024871445015ea74221 Author: Lyude Date: Wed Feb 22 16:34:53 2017 -0500 drm/radeon/dp_auxch: Ratelimit aux transfer debug messages does more harm than good in my opinion. Since this commit, I see several occurrences of the following message in my kernel log daily: radeon_dp_aux_transfer_native: 74 callbacks suppressed I never got to see the "callback" in question though, not even once, as this is a debug message which is off by default. Before the change, I would not get any such message in the kernel log (as I would expect when everything works as intended.) Does this debug message really have any practical value? If not, the easiest solution would be to simply drop it: --- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c +++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c @@ -168,8 +168,10 @@ radeon_dp_aux_transfer_native(struct drm goto done; } if (tmp & AUX_RX_ERROR_FLAGS) { - DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero: %08x\n", - tmp); + /* +* aux transfers always fail with non-zero status flags when +* there's nothing connected on the port +*/ ret = -EIO; goto done; } I can resend this as a formal patch if you agree with this solution. The actual cause of the problem is that ___ratelimit() prints its message at KERN_WARNING level regardless of the level of the message being suppressed. This makes ratelimiting debug, info or notice messages awkward. Looks like a design overlook to me, maybe it should be fixed, but that's a much bigger and intrusive change than the proposal above. -- Jean Delvare SUSE L3 Support ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Did my graphics card get damaged?
Hi John, On Sun, Nov 5, 2017 at 11:53 PM, Bridgman, John wrote: > For clarity, are you saying that when you go back to whatever distro you had > installed on the machine previously it is still not booting correctly ? Indeed. Even worse, nothing boots correctly as the machine does not get to/complete POST. > The only problems I see in the xorg log are the following lines at the end, > but not sure if they are serious or not. > > [ 205.542] (WW) RADEON(0): flip queue failed: Invalid argument > [ 205.542] (WW) RADEON(0): Page flip failed: Invalid argument > [ 205.544] (WW) RADEON(0): flip queue failed: Invalid argument > [ 205.544] (WW) RADEON(0): Page flip failed: Invalid argument Neither do I. I hope someone here might have an idea. > If it makes you feel any better I just ran into a similar problem installing > latest Ubuntu 17.10 on a newer laptop - no POST, no display at all. Haven't > had time to mess with it to figure out what the problem is but will try to > make time. Sorry about that. Vladimir ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Did my graphics card get damaged?
On 06/11/17 09:57 AM, Vladimir Klebanov wrote: > On Sun, Nov 5, 2017 at 11:53 PM, Bridgman, John wrote: > >> The only problems I see in the xorg log are the following lines at the end, >> but not sure if they are serious or not. >> >> [ 205.542] (WW) RADEON(0): flip queue failed: Invalid argument >> [ 205.542] (WW) RADEON(0): Page flip failed: Invalid argument >> [ 205.544] (WW) RADEON(0): flip queue failed: Invalid argument >> [ 205.544] (WW) RADEON(0): Page flip failed: Invalid argument > > Neither do I. I hope someone here might have an idea. They shouldn't be serious. It's extremely unlikely that the failure to POST is directly related to them. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: release ex mode after hw_init
[+Oded] Hi Pixel, Thanks, this looks a lot cleaner. The series is Reviewed-by: Felix Kuehling . Oded, are you OK with this as well? Regards, Felix On 2017-11-06 03:18 AM, Pixel Ding wrote: > Hi Felix, > > Please review. > > [PATCH 1/2] drm/amdkfd: initialise kfd inside amdgpu_device_init > As you suggested, move kfd init/fini inside amdgpu_device_init. > Other changes for KFD interfaces are dropped. > > [PATCH 2/2] drm/amdgpu: release exclusive mode after hw_init > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/3] ASoC: amd: Report accurate hw_ptr during dma
On Fri, Nov 03, 2017 at 04:35:43PM -0400, Alex Deucher wrote: > Signed-off-by: Vijendar Mukunda > Signed-off-by: Akshu Agrawal > Reviewed-on: https://chromium-review.googlesource.com/659699 > Commit-Ready: Akshu Agrawal > Tested-by: Akshu Agrawal > Reviewed-by: Jason Clinton > Reviewed-on: https://chromium-review.googlesource.com/676627 These two URLs are different, what was being reviewed here? What is Commit-Ready supposed to mean? signature.asc Description: PGP signature ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: radeon_dp_aux_transfer_native: 74 callbacks suppressed
The main reason I added this was because the radeon driver's hotplugging paths for DP do a ton of unnessecary probing, and because the driver usually also checks all connectors every time there's a hotplug (there isn't much of a good reason for this, it's just an old driver) it's not at all difficult to get well more then 70 callbacks from this that end up filling the kernel log with useless information (all the messages mean is that for some reason or another, a DP aux transaction fails which usually just means the port is disconnected). Ideally: We should be printing the first error code that the i2c handlers for radeon return in addition to the suppressed messages (and this itself should not be suppressed iirc), since that's almost always the only piece of information that matters. I wouldn't drop the message entirely though unless we're printing something from DRM. Additionally, we should also just fix this ratelimit macro anyway since it's intended purpose is not to print anything when debugging isn't enabled. What do you think Alex? On Mon, 2017-11-06 at 10:21 +0100, Jean Delvare wrote: > Hi all, > > commit 92c177b7947d9c889ea7b024871445015ea74221 > Author: Lyude > Date: Wed Feb 22 16:34:53 2017 -0500 > > drm/radeon/dp_auxch: Ratelimit aux transfer debug messages > > does more harm than good in my opinion. Since this commit, I see > several occurrences of the following message in my kernel log daily: > > radeon_dp_aux_transfer_native: 74 callbacks suppressed > > I never got to see the "callback" in question though, not even once, as > this is a debug message which is off by default. Before the change, I > would not get any such message in the kernel log (as I would expect > when everything works as intended.) > > Does this debug message really have any practical value? If not, the > easiest solution would be to simply drop it: > > --- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c > +++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c > @@ -168,8 +168,10 @@ radeon_dp_aux_transfer_native(struct drm > goto done; > } > if (tmp & AUX_RX_ERROR_FLAGS) { > - DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero: > %08x\n", > - tmp); > + /* > + * aux transfers always fail with non-zero status flags > when > + * there's nothing connected on the port > + */ > ret = -EIO; > goto done; > } > > I can resend this as a formal patch if you agree with this solution. > > The actual cause of the problem is that ___ratelimit() prints its > message at KERN_WARNING level regardless of the level of the message > being suppressed. This makes ratelimiting debug, info or notice > messages awkward. Looks like a design overlook to me, maybe it should > be fixed, but that's a much bigger and intrusive change than the > proposal above. > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/2] dma-buf: keep only not signaled fence in reservation_object_add_shared_replace v2
Quoting Christian König (2017-10-30 14:59:03) > From: Christian König > > The amdgpu issue to also need signaled fences in the reservation objects > should be fixed by now. > > Optimize the list by keeping only the not signaled yet fences around. > > v2: temporary put the signaled fences at the end of the new container > > Signed-off-by: Christian König > --- > drivers/dma-buf/reservation.c | 36 ++-- > 1 file changed, 26 insertions(+), 10 deletions(-) > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c > index b44d9d7db347..6fc794576997 100644 > --- a/drivers/dma-buf/reservation.c > +++ b/drivers/dma-buf/reservation.c > @@ -145,8 +145,8 @@ reservation_object_add_shared_replace(struct > reservation_object *obj, > struct reservation_object_list *fobj, > struct dma_fence *fence) > { > - unsigned i; > struct dma_fence *old_fence = NULL; > + unsigned i, j, k; > > dma_fence_get(fence); > > @@ -162,9 +162,7 @@ reservation_object_add_shared_replace(struct > reservation_object *obj, > * references from the old struct are carried over to > * the new. > */ > - fobj->shared_count = old->shared_count; > - > - for (i = 0; i < old->shared_count; ++i) { > + for (i = 0, j = 0, k = fobj->shared_max; i < old->shared_count; ++i) { > struct dma_fence *check; > > check = rcu_dereference_protected(old->shared[i], > @@ -172,10 +170,18 @@ reservation_object_add_shared_replace(struct > reservation_object *obj, > > if (!old_fence && check->context == fence->context) { > old_fence = check; > - RCU_INIT_POINTER(fobj->shared[i], fence); > - } else > - RCU_INIT_POINTER(fobj->shared[i], check); > + RCU_INIT_POINTER(fobj->shared[j++], fence); > + } else if (!dma_fence_is_signaled(check)) { > + RCU_INIT_POINTER(fobj->shared[j++], check); > + } else { > + /* > +* Temporary save the signaled fences at the end of > the > +* new container > +*/ > + RCU_INIT_POINTER(fobj->shared[--k], check); > + } > } > + fobj->shared_count = j; > if (!old_fence) { > RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence); > fobj->shared_count++; > @@ -192,10 +198,20 @@ reservation_object_add_shared_replace(struct > reservation_object *obj, > write_seqcount_end(&obj->seq); > preempt_enable(); > > - if (old) > - kfree_rcu(old, rcu); > - > dma_fence_put(old_fence); We don't need to keep old_fence (local var) around any more in this scheme. By construction, the new shared_list is large enough to hold all the old_fences+1, so you can use a loop like j = 0; k = fobj->shared_max; RCU_INIT_POINTER(fobj->shared[j++], fence); for (i = 0; i < old->shared_count; ++i) { struct dma_fence *check; check = rcu_dereference_protected(old->shared[i], reservation_object_held(obj)); if (check->context == fence->context || dma_fence_is_signaled(check)) RCU_INIT_POINTER(fobj->shared[--k], check); else RCU_INIT_POINTER(fobj->shared[j++], check); } fobj->shared_count = j; > + > + if (!old) > + return; > + > + /* Drop the references to the signaled fences */ > + for (i = k; i < fobj->shared_max; ++i) { > + struct dma_fence *f; > + > + f = rcu_dereference_protected(fobj->shared[i], > + reservation_object_held(obj)); > + dma_fence_put(f); > + } > + kfree_rcu(old, rcu); > } Fwiw, this fixes a big problem with the lack of pruning of the reservation_object, so \o/ -Chris ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/2] dma-buf: try to replace a signaled fence in reservation_object_add_shared_inplace
Quoting Christian König (2017-10-30 14:59:04) > From: Christian König > > The amdgpu issue to also need signaled fences in the reservation objects > should > be fixed by now. > > Optimize the handling by replacing a signaled fence when adding a new > shared one. > > Signed-off-by: Christian König > --- > drivers/dma-buf/reservation.c | 18 +++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c > index 6fc794576997..a3928ce9f311 100644 > --- a/drivers/dma-buf/reservation.c > +++ b/drivers/dma-buf/reservation.c > @@ -104,7 +104,8 @@ reservation_object_add_shared_inplace(struct > reservation_object *obj, > struct reservation_object_list *fobj, > struct dma_fence *fence) > { > - u32 i; > + struct dma_fence *signaled = NULL; > + u32 i, signaled_idx; > > dma_fence_get(fence); > > @@ -126,17 +127,28 @@ reservation_object_add_shared_inplace(struct > reservation_object *obj, > dma_fence_put(old_fence); > return; > } > + > + if (!signaled && dma_fence_is_signaled(old_fence)) { > + signaled = old_fence; > + signaled_idx = i; > + } How much do we care about only keeping one fence per-ctx here? You could rearrange this to break on old_fence->context == fence->context || dma_fence_is_signaled(old_fence) then everyone can use the final block. -Chris ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 3/3] ASoC: rt5645: Wait for 400msec before concluding on value of RT5645_VENDOR_ID2
On Fri, Nov 03, 2017 at 04:35:45PM -0400, Alex Deucher wrote: > Minimum time required between power On of codec and read > of RT5645_VENDOR_ID2 is 400msec. We should wait and attempt > before erroring out. So the description says we have to wait 400ms before attempting a read... > BUG=b:66978383 What does this mean? > @@ -3786,6 +3789,15 @@ static int rt5645_i2c_probe(struct i2c_client *i2c, > } > regmap_read(regmap, RT5645_VENDOR_ID2, &val); > > + /* > + * Read for 400msec, as it is the interval required between > + * read and power On. > + */ > + while (val != RT5645_DEVICE_ID && val != RT5650_DEVICE_ID && --timeout) > { > + msleep(1); > + regmap_read(regmap, RT5645_VENDOR_ID2, &val); > + } > + ...but what we actually do is try to read up to 400 times starting well before that 400ms is up. This directly contradicts what the commit message said we needed, may take a lot longer if the chip misbehaves on the I2C bus while it's not ready (which wouldn't be that much of a surprise), might lead to us reporting before the chip is really stable (if the read happens to work while the chip isn't yet stable) and could cause lots of noise on the console if the I2C controller gets upset. What are we actually waiting for here? If we really need 400ms of dead reckoning time (which is a lot for a modern chip, that feels more like a VMID ramp) then it's going to be safer to just do that. signature.asc Description: PGP signature ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: linux-next: Tree for Nov 6 (amdgpu_virt.c)
On 11/05/2017 11:30 PM, Stephen Rothwell wrote: > Hi all, > > Changes since 20171103: > on i386, when CONFIG_MODULES is not set: CC drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o In file included from ../arch/x86/include/asm/atomic.h:5:0, from ../include/linux/atomic.h:5, from ../drivers/gpu/drm/amd/amdgpu/amdgpu.h:31, from ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:24: ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c: In function 'amdgpu_virt_init_data_exchange': ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing pointer to incomplete type if (THIS_MODULE->version != NULL) ^ ../include/linux/compiler.h:58:30: note: in definition of macro '__trace_if' if (__builtin_constant_p(!!(cond)) ? !!(cond) : \ ^ ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of macro 'if' if (THIS_MODULE->version != NULL) ^ ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing pointer to incomplete type if (THIS_MODULE->version != NULL) ^ ../include/linux/compiler.h:58:42: note: in definition of macro '__trace_if' if (__builtin_constant_p(!!(cond)) ? !!(cond) : \ ^ ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of macro 'if' if (THIS_MODULE->version != NULL) ^ ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing pointer to incomplete type if (THIS_MODULE->version != NULL) ^ ../include/linux/compiler.h:69:16: note: in definition of macro '__trace_if' __r = !!(cond); \ ^ ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of macro 'if' if (THIS_MODULE->version != NULL) ^ ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:332:29: error: dereferencing pointer to incomplete type strcpy(str, THIS_MODULE->version); ^ ../scripts/Makefile.build:314: recipe for target 'drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o' failed make[5]: *** [drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o] Error 1 Reported-by: Randy Dunlap -- ~Randy ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: linux-next: Tree for Nov 6 (amdgpu_virt.c)
On Mon, Nov 6, 2017 at 12:20 PM, Randy Dunlap wrote: > On 11/05/2017 11:30 PM, Stephen Rothwell wrote: >> Hi all, >> >> Changes since 20171103: >> > > on i386, when CONFIG_MODULES is not set: > > CC drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o > In file included from ../arch/x86/include/asm/atomic.h:5:0, > from ../include/linux/atomic.h:5, > from ../drivers/gpu/drm/amd/amdgpu/amdgpu.h:31, > from ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:24: > ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c: In function > 'amdgpu_virt_init_data_exchange': > ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing > pointer to incomplete type > if (THIS_MODULE->version != NULL) > ^ > ../include/linux/compiler.h:58:30: note: in definition of macro '__trace_if' > if (__builtin_constant_p(!!(cond)) ? !!(cond) : \ > ^ > ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of > macro 'if' > if (THIS_MODULE->version != NULL) > ^ > ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing > pointer to incomplete type > if (THIS_MODULE->version != NULL) > ^ > ../include/linux/compiler.h:58:42: note: in definition of macro '__trace_if' > if (__builtin_constant_p(!!(cond)) ? !!(cond) : \ > ^ > ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of > macro 'if' > if (THIS_MODULE->version != NULL) > ^ > ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing > pointer to incomplete type > if (THIS_MODULE->version != NULL) > ^ > ../include/linux/compiler.h:69:16: note: in definition of macro '__trace_if' >__r = !!(cond); \ > ^ > ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of > macro 'if' > if (THIS_MODULE->version != NULL) > ^ > ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:332:29: error: dereferencing > pointer to incomplete type > strcpy(str, THIS_MODULE->version); > ^ > ../scripts/Makefile.build:314: recipe for target > 'drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o' failed > make[5]: *** [drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o] Error 1 > > > > Reported-by: Randy Dunlap Thanks. Fixed in: https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next&id=e477e940dad1836c6f6d23353e424665b9316b6e Alex ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/2] drm/amdgpu: use multipipe compute policy on non PL11 asics
Sorry my mail client seems to have blown up. My reply got cut off, here is the full version: On 2017-11-06 01:49 AM, Chunming Zhou wrote: > Hi Andres, > Hi David, > With your this patch, OCLperf hung. Is this on all ASICs or just a specific one? > > Could you explain more? > > If I am correctly, the difference of with and without this patch is > setting first two queue or setting all queues of pipe0 to queue_bitmap. It is slightly different. With this patch we will also use the first two queues of all pipes, not just pipe 0; Pre-patch: |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-| Post-patch: |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-| 1100 1100 1100 1100 What this means is that we are allowing real multithreading for compute. Jobs on different pipes allow for parallel execution of work. Jobs on the same pipe (but different queues) use timeslicing to share the hardware. > > Then UMD can use different number queue to submit command for compute > selected by amdgpu_queue_mgr_map. > > I checked amdgpu_queue_mgr_map implementation, CS_IOCTL can map user > ring to different hw ring depending on busy or idle, right? Yes, when a queue is first used, amdgpu_queue_mgr_map will decide what the mapping is for a usermode ring to a kernel ring id. > If yes, I see a bug in it, which will result in our sched_fence not > work. Our sched fence assumes the job will be executed in order, your > mapping queue breaks this. I think here you mean that work will execute out of order because it will go to different rings? That should not happen, since the id mapping is permanent on a per-context basis. Once a mapping is decided, it will be cached for this context so that we keep execution order guarantees. See the id-caching code in amdgpu_queue_mgr.c for reference. As long as the usermode keeps submitting work to the same ring, it will all be executed in order (all in the same ring). There is no change in this guarantee compared to pre-patch. Note that even before this patch amdgpu_queue_mgr_map has been using an LRU policy for a long time now. Regards, Andres On Mon, Nov 6, 2017 at 12:44 PM, Andres Rodriguez wrote: > > > On 2017-11-06 01:49 AM, Chunming Zhou wrote: >> >> Hi Andres, >> > > Hi David, > >> With your this patch, OCLperf hung. > > > Is this on all ASICs or just a specific one? > >> >> Could you explain more? >> >> If I am correctly, the difference of with and without this patch is >> setting first two queue or setting all queues of pipe0 to queue_bitmap. > > > It is slightly different. With this patch we will also use the first two > queues of all pipes, not just pipe 0; > > Pre-patch: > > |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-| > > > Post-patch: > > |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-| > 1100 1100 1100 1100 > > What this means is that we are allowing real multithreading for compute. > Jobs on different pipes allow for parallel execution of work. Jobs on the > same pipe (but different queues) use timeslicing to share the hardware. > > > >> >> Then UMD can use different number queue to submit command for compute >> selected by amdgpu_queue_mgr_map. >> >> I checked amdgpu_queue_mgr_map implementation, CS_IOCTL can map user ring >> to different hw ring depending on busy or idle, right? >> >> If yes, I see a bug in it, which will result in our sched_fence not work. >> Our sched fence assumes the job will be executed in order, your mapping >> queue breaks this. >> >> >> Regards, >> >> David Zhou >> >> >> On 2017年09月27日 00:22, Andres Rodriguez wrote: >>> >>> A performance regression for OpenCL tests on Polaris11 had this feature >>> disabled for all asics. >>> >>> Instead, disable it selectively on the affected asics. >>> >>> Signed-off-by: Andres Rodriguez >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 14 -- >>> 1 file changed, 12 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c >>> index 4f6c68f..3d76e76 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c >>> @@ -109,9 +109,20 @@ void amdgpu_gfx_parse_disable_cu(unsigned *mask, >>> unsigned max_se, unsigned max_s >>> } >>> } >>> +static bool amdgpu_gfx_is_multipipe_capable(struct amdgpu_device *adev) >>> +{ >>> +/* FIXME: spreading the queues across pipes causes perf regressions >>> + * on POLARIS11 compute workloads */ >>> +if (adev->asic_type == CHIP_POLARIS11) >>> +return false; >>> + >>> +return adev->gfx.mec.num_mec > 1; >>> +} >>> + >>> void amdgpu_gfx_compute_queue_acquire(struct amdgpu_device *adev) >>> { >>> int i, queue, pipe, mec; >>> +bool multipipe_policy = amdgpu_gfx_is_multipipe_capable(adev); >>> /* policy for amdgpu compute queue ownership */ >>> for (i = 0; i < AMDGPU_MAX_COMPUTE_QUEUES; ++i) { >>
Re: radeon_dp_aux_transfer_native: 74 callbacks suppressed
Additionally, we should also just fix this ratelimit macro anyway since it's intended purpose is not to print anything when debugging isn't enabled. What do you think Alex? Well, at least I think that the described behavior of the macro is a bug which should be fixed. But I think that is independent of the problem that radeon is a bit flaky about DP aux transfers. Regards, Christian. Am 06.11.2017 um 17:17 schrieb Lyude Paul: The main reason I added this was because the radeon driver's hotplugging paths for DP do a ton of unnessecary probing, and because the driver usually also checks all connectors every time there's a hotplug (there isn't much of a good reason for this, it's just an old driver) it's not at all difficult to get well more then 70 callbacks from this that end up filling the kernel log with useless information (all the messages mean is that for some reason or another, a DP aux transaction fails which usually just means the port is disconnected). Ideally: We should be printing the first error code that the i2c handlers for radeon return in addition to the suppressed messages (and this itself should not be suppressed iirc), since that's almost always the only piece of information that matters. I wouldn't drop the message entirely though unless we're printing something from DRM. Additionally, we should also just fix this ratelimit macro anyway since it's intended purpose is not to print anything when debugging isn't enabled. What do you think Alex? On Mon, 2017-11-06 at 10:21 +0100, Jean Delvare wrote: Hi all, commit 92c177b7947d9c889ea7b024871445015ea74221 Author: Lyude Date: Wed Feb 22 16:34:53 2017 -0500 drm/radeon/dp_auxch: Ratelimit aux transfer debug messages does more harm than good in my opinion. Since this commit, I see several occurrences of the following message in my kernel log daily: radeon_dp_aux_transfer_native: 74 callbacks suppressed I never got to see the "callback" in question though, not even once, as this is a debug message which is off by default. Before the change, I would not get any such message in the kernel log (as I would expect when everything works as intended.) Does this debug message really have any practical value? If not, the easiest solution would be to simply drop it: --- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c +++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c @@ -168,8 +168,10 @@ radeon_dp_aux_transfer_native(struct drm goto done; } if (tmp & AUX_RX_ERROR_FLAGS) { - DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero: %08x\n", - tmp); + /* +* aux transfers always fail with non-zero status flags when +* there's nothing connected on the port +*/ ret = -EIO; goto done; } I can resend this as a formal patch if you agree with this solution. The actual cause of the problem is that ___ratelimit() prints its message at KERN_WARNING level regardless of the level of the message being suppressed. This makes ratelimiting debug, info or notice messages awkward. Looks like a design overlook to me, maybe it should be fixed, but that's a much bigger and intrusive change than the proposal above. ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH umr] Skip ahead if PDE entry is actually a PTE entry. (v2)
On 06/11/17 05:01 AM, Christian König wrote: Am 04.11.2017 um 18:15 schrieb Tom St Denis: Signed-off-by: Tom St Denis Still not perfect, but good enough for now. Patch is Tested-by: Christian König . I think you need to rework the VM walking a bit, cause we need to support the T bit as well in the future and your code make a few assumptions which doesn't allow that. Doesn't the T bit imply V=0 which means the page isn't backed by memory. Not much umr could do about that other than to print out the T bit. Tom ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH umr] Skip ahead if PDE entry is actually a PTE entry. (v2)
Am 06.11.2017 um 19:28 schrieb Tom St Denis: On 06/11/17 05:01 AM, Christian König wrote: Am 04.11.2017 um 18:15 schrieb Tom St Denis: Signed-off-by: Tom St Denis Still not perfect, but good enough for now. Patch is Tested-by: Christian König . I think you need to rework the VM walking a bit, cause we need to support the T bit as well in the future and your code make a few assumptions which doesn't allow that. Doesn't the T bit imply V=0 which means the page isn't backed by memory. Not much umr could do about that other than to print out the T bit. No, the T bit means translate further. In other words it is the counter part of the P bit and means that a PTE should be handled as a PDE. But for this to have meaning you also need to handle the fragment size as well (Now I have you totally confused, haven't I? :). Cheers, Christian. Tom ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH umr] Skip ahead if PDE entry is actually a PTE entry. (v2)
On 06/11/17 01:34 PM, Christian König wrote: Am 06.11.2017 um 19:28 schrieb Tom St Denis: On 06/11/17 05:01 AM, Christian König wrote: Am 04.11.2017 um 18:15 schrieb Tom St Denis: Signed-off-by: Tom St Denis Still not perfect, but good enough for now. Patch is Tested-by: Christian König . I think you need to rework the VM walking a bit, cause we need to support the T bit as well in the future and your code make a few assumptions which doesn't allow that. Doesn't the T bit imply V=0 which means the page isn't backed by memory. Not much umr could do about that other than to print out the T bit. No, the T bit means translate further. In other words it is the counter part of the P bit and means that a PTE should be handled as a PDE. But for this to have meaning you also need to handle the fragment size as well (Now I have you totally confused, haven't I? :). Yes :-) I thought fragment size was more for hinting to the cache controller and not actually part of the VM decoding. Also the PI docs say T => "Tiled (PRT)" and from what I gather that just means the page is valid but might not be backed so instead of raising a page fault you raise a new fault that the application (?) handles accordingly. There's an 'F' bit that is labeled "translate further". Reading section 8 of said document seems to indicate you're confusing bits F and T or my doc is wildly out of date (or we're talking about different IP revisions) Tom ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH umr] Skip ahead if PDE entry is actually a PTE entry. (v2)
Am 06.11.2017 um 19:39 schrieb Tom St Denis: On 06/11/17 01:34 PM, Christian König wrote: Am 06.11.2017 um 19:28 schrieb Tom St Denis: On 06/11/17 05:01 AM, Christian König wrote: Am 04.11.2017 um 18:15 schrieb Tom St Denis: Signed-off-by: Tom St Denis Still not perfect, but good enough for now. Patch is Tested-by: Christian König . I think you need to rework the VM walking a bit, cause we need to support the T bit as well in the future and your code make a few assumptions which doesn't allow that. Doesn't the T bit imply V=0 which means the page isn't backed by memory. Not much umr could do about that other than to print out the T bit. No, the T bit means translate further. In other words it is the counter part of the P bit and means that a PTE should be handled as a PDE. But for this to have meaning you also need to handle the fragment size as well (Now I have you totally confused, haven't I? :). Yes :-) I thought fragment size was more for hinting to the cache controller and not actually part of the VM decoding. Correct, our hardware devs are unfortunately using the same name for two different things. The fragment size in the PTE means what you described above and controls the L1 TLB settings. The fragment size in the PDE controls how big the PTE entries will be. So with the default fragment size of zero in the PDE we have 512 PTEs resulting in a 4K page table. Also the PI docs say T => "Tiled (PRT)" and from what I gather that just means the page is valid but might not be backed so instead of raising a page fault you raise a new fault that the application (?) handles accordingly. There's an 'F' bit that is labeled "translate further". Reading section 8 of said document seems to indicate you're confusing bits F and T or my doc is wildly out of date (or we're talking about different IP revisions) Sorry my fault. I was just confusing the F and T bits. Christian. Tom ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] amdgpu/dc: fix non-ansi function decls.
From: Dave Airlie smatch reported: drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce80/command_table_helper_dce80.c:351:71: warning: non-ANSI function declaration of function 'dal_cmd_tbl_helper_dce80_get_table' drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce110/command_table_helper_dce110.c:361:72: warning: non-ANSI function declaration of function 'dal_cmd_tbl_helper_dce110_get_table' drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce112/command_table_helper_dce112.c:415:72: warning: non-ANSI function declaration of function 'dal_cmd_tbl_helper_dce112_get_table' drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce112/command_table_helper2_dce112.c:415:73: warning: non-ANSI function declaration of function 'dal_cmd_tbl_helper_dce112_get_table2' drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_surface.c:148:34: warning: non-ANSI function declaration of function 'dc_create_gamma' drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_surface.c:178:50: warning: non-ANSI function declaration of function 'dc_create_transfer_func' This fixes them. Signed-off-by: Dave Airlie --- .../gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c | 2 +- .../gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c | 2 +- .../gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c | 2 +- .../gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c| 2 +- drivers/gpu/drm/amd/display/dc/core/dc_surface.c | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c b/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c index 8049320..ca24154 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c +++ b/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c @@ -358,7 +358,7 @@ static const struct command_table_helper command_table_helper_funcs = { * const struct command_table_helper **h - [out] struct of functions * */ -const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table() +const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table(void) { return &command_table_helper_funcs; } diff --git a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c index d342cde..0237ae5 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c +++ b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c @@ -412,7 +412,7 @@ static const struct command_table_helper command_table_helper_funcs = { * const struct command_table_helper **h - [out] struct of functions * */ -const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table2() +const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table2(void) { return &command_table_helper_funcs; } diff --git a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c index 48e5996..452034f 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c +++ b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c @@ -412,7 +412,7 @@ static const struct command_table_helper command_table_helper_funcs = { * const struct command_table_helper **h - [out] struct of functions * */ -const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table() +const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table(void) { return &command_table_helper_funcs; } diff --git a/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c b/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c index 295e16e..8b30b55 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c +++ b/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c @@ -348,7 +348,7 @@ static const struct command_table_helper command_table_helper_funcs = { dal_cmd_table_helper_encoder_mode_bp_to_atom, }; -const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table() +const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table(void) { return &command_table_helper_funcs; } diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c index 5aa2270..ade5b8e 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c @@ -145,7 +145,7 @@ void dc_gamma_release(struct dc_gamma **gamma) *gamma = NULL; } -struct dc_gamma *dc_create_gamma() +struct dc_gamma *dc_create_gamma(void) { struct dc_gamma *gamma = kzalloc(sizeof(*gamma), GFP_KERNEL); @@ -175,7 +175,7 @@ void dc_transfer_func_release(struct dc_transfer_func
[PATCH] amdgpu/dc: handle allocation failures in dc_commit_planes_to_stream.
From: Dave Airlie Reported-by smatch: drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:966 dc_commit_planes_to_stream() error: potential null dereference 'flip_addr'. (kcalloc returns null) drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:968 dc_commit_planes_to_stream() error: potential null dereference 'plane_info'. (kcalloc returns null) drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:978 dc_commit_planes_to_stream() error: potential null dereference 'scaling_info'. (kcalloc returns null) Signed-off-by: Dave Airlie --- drivers/gpu/drm/amd/display/dc/core/dc.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index a71392f..ce3c57b 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -952,6 +952,14 @@ bool dc_commit_planes_to_stream( scaling_info = kcalloc(MAX_SURFACES, sizeof(struct dc_scaling_info), GFP_KERNEL); + if (!flip_addr || !plane_info || !scaling_info) { + kfree(flip_addr); + kfree(plane_info); + kfree(scaling_info); + kfree(stream_update); + return false; + } + memset(updates, 0, sizeof(updates)); stream_update->src = dc_stream->src; -- 2.9.5 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] amdgpu/dc: fix indentation warning from smatch.
From: Dave Airlie This fixes all the current smatch: warn: inconsistent indenting Signed-off-by: Dave Airlie --- drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c | 2 +- .../gpu/drm/amd/display/dc/bios/command_table2.c | 18 ++--- drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c | 2 +- drivers/gpu/drm/amd/display/dc/core/dc_stream.c| 4 +-- drivers/gpu/drm/amd/display/dc/dce/dce_audio.c | 26 +-- drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c | 16 ++-- .../amd/display/dc/dce110/dce110_hw_sequencer.c| 30 +++--- .../display/dc/dce120/dce120_timing_generator.c| 7 +++-- .../gpu/drm/amd/display/dc/dcn10/dcn10_dpp_cm.c| 2 +- .../dc/i2caux/dce110/i2c_hw_engine_dce110.c| 30 +++--- 10 files changed, 68 insertions(+), 69 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c index 43e9a99..1ee1717 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c +++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c @@ -1373,7 +1373,7 @@ static enum bp_result get_firmware_info_v3_1( bp->cmd_tbl.get_smu_clock_info(bp) * 10; } -return BP_RESULT_OK; + return BP_RESULT_OK; } static enum bp_result bios_parser_get_encoder_cap_info( diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table2.c b/drivers/gpu/drm/amd/display/dc/bios/command_table2.c index 64eab35..ba68693 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/command_table2.c +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table2.c @@ -373,15 +373,15 @@ static void init_set_crtc_timing(struct bios_parser *bp) uint32_t dtd_version = BIOS_CMD_TABLE_PARA_REVISION(setcrtc_usingdtdtiming); - switch (dtd_version) { - case 3: - bp->cmd_tbl.set_crtc_timing = - set_crtc_using_dtd_timing_v3; - break; - default: - bp->cmd_tbl.set_crtc_timing = NULL; - break; - } + switch (dtd_version) { + case 3: + bp->cmd_tbl.set_crtc_timing = + set_crtc_using_dtd_timing_v3; + break; + default: + bp->cmd_tbl.set_crtc_timing = NULL; + break; + } } static enum bp_result set_crtc_using_dtd_timing_v3( diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c index e151523..3dce35e 100644 --- a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c +++ b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c @@ -1155,7 +1155,7 @@ static unsigned int dcn_find_normalized_clock_vdd_Level( unsigned factor = (ddr4_dram_factor_single_Channel * dc->dcn_soc->number_of_channels); if (clocks_in_khz > dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9*100/factor) { - vdd_level = dcn_bw_v_max0p91; + vdd_level = dcn_bw_v_max0p91; BREAK_TO_DEBUGGER(); } else if (clocks_in_khz > dc->dcn_soc->fabric_and_dram_bandwidth_vnom0p8*100/factor) { vdd_level = dcn_bw_v_max0p9; diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c index 572b885..de04b95 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c @@ -182,11 +182,11 @@ bool dc_stream_set_cursor_attributes( if (NULL == stream) { dm_error("DC: dc_stream is NULL!\n"); - return false; + return false; } if (NULL == attributes) { dm_error("DC: attributes is NULL!\n"); - return false; + return false; } if (attributes->address.quad_part == 0) { diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c index 526ec5c..d882adf 100644 --- a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c @@ -179,19 +179,19 @@ static void check_audio_bandwidth_hdmi( /* Number of Audio Packets (multiplied by 10) per Line (for 8 ch number * of Audio samples per line multiplied by 10 - Layout 1) */ -samples /= 32; -samples *= crtc_info->v_active; -/*Number of samples multiplied by 10, per second */ -samples *= crtc_info->refresh_rate; -/*Number of Audio samples per second */ -samples /= 10; - -/* @todo do it after deep color is implemented - * 8xx - deep color bandwidth scaling - * Extra bandwidth is avaliable in deep color b
Re: [PATCH] amdgpu/dc: fix non-ansi function decls.
Am 06.11.2017 um 20:17 schrieb Dave Airlie: From: Dave Airlie smatch reported: drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce80/command_table_helper_dce80.c:351:71: warning: non-ANSI function declaration of function 'dal_cmd_tbl_helper_dce80_get_table' drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce110/command_table_helper_dce110.c:361:72: warning: non-ANSI function declaration of function 'dal_cmd_tbl_helper_dce110_get_table' drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce112/command_table_helper_dce112.c:415:72: warning: non-ANSI function declaration of function 'dal_cmd_tbl_helper_dce112_get_table' drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce112/command_table_helper2_dce112.c:415:73: warning: non-ANSI function declaration of function 'dal_cmd_tbl_helper_dce112_get_table2' drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_surface.c:148:34: warning: non-ANSI function declaration of function 'dc_create_gamma' drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_surface.c:178:50: warning: non-ANSI function declaration of function 'dc_create_transfer_func' This fixes them. Signed-off-by: Dave Airlie --- .../gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c | 2 +- .../gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c | 2 +- .../gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c | 2 +- .../gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c| 2 +- drivers/gpu/drm/amd/display/dc/core/dc_surface.c | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c b/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c index 8049320..ca24154 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c +++ b/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c @@ -358,7 +358,7 @@ static const struct command_table_helper command_table_helper_funcs = { * const struct command_table_helper **h - [out] struct of functions * */ -const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table() +const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table(void) { return &command_table_helper_funcs; } BTW may I ask what those accessors functions are good for? That looks just like a LOC increase to me. Why not export the command_table_helper_funcs directly? Regards, Christian. diff --git a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c index d342cde..0237ae5 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c +++ b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c @@ -412,7 +412,7 @@ static const struct command_table_helper command_table_helper_funcs = { * const struct command_table_helper **h - [out] struct of functions * */ -const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table2() +const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table2(void) { return &command_table_helper_funcs; } diff --git a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c index 48e5996..452034f 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c +++ b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c @@ -412,7 +412,7 @@ static const struct command_table_helper command_table_helper_funcs = { * const struct command_table_helper **h - [out] struct of functions * */ -const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table() +const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table(void) { return &command_table_helper_funcs; } diff --git a/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c b/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c index 295e16e..8b30b55 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c +++ b/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c @@ -348,7 +348,7 @@ static const struct command_table_helper command_table_helper_funcs = { dal_cmd_table_helper_encoder_mode_bp_to_atom, }; -const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table() +const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table(void) { return &command_table_helper_funcs; } diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c index 5aa2270..ade5b8e 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c @@ -145,7 +145,7 @@ void dc_gamma_release(struct dc_gamma **ga
[PATCH 2/2] drm/amdkfd: Use order_base_2 to get log2 of buffes sizes
Replace (ffs(size) - 1) with order_base_2(size) as a more straight forward way to get log2 of buffer sizes. Signed-off-by: Felix Kuehling --- drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 6 +++--- drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c index efed6ef..7aa57ab 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c @@ -183,7 +183,7 @@ static int update_mqd(struct mqd_manager *mm, void *mqd, * Calculating queue size which is log base 2 of actual queue size -1 * dwords and another -1 for ffs */ - m->cp_hqd_pq_control |= ffs(q->queue_size / 4) - 1 - 1; + m->cp_hqd_pq_control |= order_base_2(q->queue_size / 4) - 1; m->cp_hqd_pq_base_lo = lower_32_bits((uint64_t)q->queue_address >> 8); m->cp_hqd_pq_base_hi = upper_32_bits((uint64_t)q->queue_address >> 8); m->cp_hqd_pq_rptr_report_addr_lo = lower_32_bits((uint64_t)q->read_ptr); @@ -208,7 +208,7 @@ static int update_mqd_sdma(struct mqd_manager *mm, void *mqd, struct cik_sdma_rlc_registers *m; m = get_sdma_mqd(mqd); - m->sdma_rlc_rb_cntl = (ffs(q->queue_size / 4) - 1) + m->sdma_rlc_rb_cntl = order_base_2(q->queue_size / 4) << SDMA0_RLC0_RB_CNTL__RB_SIZE__SHIFT | q->vmid << SDMA0_RLC0_RB_CNTL__RB_VMID__SHIFT | 1 << SDMA0_RLC0_RB_CNTL__RPTR_WRITEBACK_ENABLE__SHIFT | @@ -349,7 +349,7 @@ static int update_mqd_hiq(struct mqd_manager *mm, void *mqd, * Calculating queue size which is log base 2 of actual queue * size -1 dwords */ - m->cp_hqd_pq_control |= ffs(q->queue_size / 4) - 1 - 1; + m->cp_hqd_pq_control |= order_base_2(q->queue_size / 4) - 1; m->cp_hqd_pq_base_lo = lower_32_bits((uint64_t)q->queue_address >> 8); m->cp_hqd_pq_base_hi = upper_32_bits((uint64_t)q->queue_address >> 8); m->cp_hqd_pq_rptr_report_addr_lo = lower_32_bits((uint64_t)q->read_ptr); diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c index 85e1b67..2ba7cea 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c @@ -121,7 +121,7 @@ static int __update_mqd(struct mqd_manager *mm, void *mqd, m->cp_hqd_pq_control = 5 << CP_HQD_PQ_CONTROL__RPTR_BLOCK_SIZE__SHIFT | atc_bit << CP_HQD_PQ_CONTROL__PQ_ATC__SHIFT | mtype << CP_HQD_PQ_CONTROL__MTYPE__SHIFT; - m->cp_hqd_pq_control |= ffs(q->queue_size / 4) - 1 - 1; + m->cp_hqd_pq_control |= order_base_2(q->queue_size / 4) - 1; pr_debug("cp_hqd_pq_control 0x%x\n", m->cp_hqd_pq_control); m->cp_hqd_pq_base_lo = lower_32_bits((uint64_t)q->queue_address >> 8); @@ -151,7 +151,7 @@ static int __update_mqd(struct mqd_manager *mm, void *mqd, * is safe, giving a maximum field value of 0xA. */ m->cp_hqd_eop_control |= min(0xA, - ffs(q->eop_ring_buffer_size / 4) - 1 - 1); + order_base_2(q->eop_ring_buffer_size / 4) - 1); m->cp_hqd_eop_base_addr_lo = lower_32_bits(q->eop_ring_buffer_address >> 8); m->cp_hqd_eop_base_addr_hi = @@ -287,7 +287,7 @@ static int update_mqd_sdma(struct mqd_manager *mm, void *mqd, struct vi_sdma_mqd *m; m = get_sdma_mqd(mqd); - m->sdmax_rlcx_rb_cntl = (ffs(q->queue_size / 4) - 1) + m->sdmax_rlcx_rb_cntl = order_base_2(q->queue_size / 4) << SDMA0_RLC0_RB_CNTL__RB_SIZE__SHIFT | q->vmid << SDMA0_RLC0_RB_CNTL__RB_VMID__SHIFT | 1 << SDMA0_RLC0_RB_CNTL__RPTR_WRITEBACK_ENABLE__SHIFT | -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/2] drm/amdkfd: Hardware DWORD size is 4 bytes
Don't use sizeof(uint32_t) or similar types for hardware or firmware DWORD size. The hardware and firmware don't care about Linux types. Signed-off-by: Felix Kuehling --- drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 14 +- drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c| 2 +- drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 10 -- drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c | 9 - drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c | 2 +- 5 files changed, 15 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c index c407f6b..afb26f2 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c @@ -95,7 +95,7 @@ static int dbgdev_diq_submit_ib(struct kfd_dbgdev *dbgdev, ib_packet->bitfields3.ib_base_hi = largep->u.high_part; ib_packet->control = (1 << 23) | (1 << 31) | - ((size_in_bytes / sizeof(uint32_t)) & 0xf); + ((size_in_bytes / 4) & 0xf); ib_packet->bitfields5.pasid = pasid; @@ -126,8 +126,7 @@ static int dbgdev_diq_submit_ib(struct kfd_dbgdev *dbgdev, rm_packet->header.opcode = IT_RELEASE_MEM; rm_packet->header.type = PM4_TYPE_3; - rm_packet->header.count = sizeof(struct pm4__release_mem) / - sizeof(unsigned int) - 2; + rm_packet->header.count = sizeof(struct pm4__release_mem) / 4 - 2; rm_packet->bitfields2.event_type = CACHE_FLUSH_AND_INV_TS_EVENT; rm_packet->bitfields2.event_index = @@ -652,8 +651,7 @@ static int dbgdev_wave_control_diq(struct kfd_dbgdev *dbgdev, packets_vec[0].header.opcode = IT_SET_UCONFIG_REG; packets_vec[0].header.type = PM4_TYPE_3; packets_vec[0].bitfields2.reg_offset = - GRBM_GFX_INDEX / (sizeof(uint32_t)) - - USERCONFIG_REG_BASE; + GRBM_GFX_INDEX / 4 - USERCONFIG_REG_BASE; packets_vec[0].bitfields2.insert_vmid = 0; packets_vec[0].reg_data[0] = reg_gfx_index.u32All; @@ -661,8 +659,7 @@ static int dbgdev_wave_control_diq(struct kfd_dbgdev *dbgdev, packets_vec[1].header.count = 1; packets_vec[1].header.opcode = IT_SET_CONFIG_REG; packets_vec[1].header.type = PM4_TYPE_3; - packets_vec[1].bitfields2.reg_offset = SQ_CMD / (sizeof(uint32_t)) - - AMD_CONFIG_REG_BASE; + packets_vec[1].bitfields2.reg_offset = SQ_CMD / 4 - AMD_CONFIG_REG_BASE; packets_vec[1].bitfields2.vmid_shift = SQ_CMD_VMID_OFFSET; packets_vec[1].bitfields2.insert_vmid = 1; @@ -678,8 +675,7 @@ static int dbgdev_wave_control_diq(struct kfd_dbgdev *dbgdev, packets_vec[2].ordinal1 = packets_vec[0].ordinal1; packets_vec[2].bitfields2.reg_offset = - GRBM_GFX_INDEX / (sizeof(uint32_t)) - - USERCONFIG_REG_BASE; + GRBM_GFX_INDEX / 4 - USERCONFIG_REG_BASE; packets_vec[2].bitfields2.insert_vmid = 0; packets_vec[2].reg_data[0] = reg_gfx_index.u32All; diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c index 8b0c064..5dc6567 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c @@ -218,7 +218,7 @@ static int acquire_packet_buffer(struct kernel_queue *kq, rptr = *kq->rptr_kernel; wptr = *kq->wptr_kernel; queue_address = (unsigned int *)kq->pq_kernel_addr; - queue_size_dwords = kq->queue->properties.queue_size / sizeof(uint32_t); + queue_size_dwords = kq->queue->properties.queue_size / 4; pr_debug("rptr: %d\n", rptr); pr_debug("wptr: %d\n", wptr); diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c index 9873929..efed6ef 100644 --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c @@ -154,7 +154,7 @@ static int load_mqd(struct mqd_manager *mm, void *mqd, uint32_t pipe_id, { /* AQL write pointer counts in 64B packets, PM4/CP counts in dwords. */ uint32_t wptr_shift = (p->format == KFD_QUEUE_FORMAT_AQL ? 4 : 0); - uint32_t wptr_mask = (uint32_t)((p->queue_size / sizeof(uint32_t)) - 1); + uint32_t wptr_mask = (uint32_t)((p->queue_size / 4) - 1); return mm->dev->kfd2kgd->hqd_load(mm->dev->kgd, mqd, pipe_id, queue_id, (uint32_t __user *)p->write_ptr, @@ -183,8 +183,7 @@ static int update_mqd(struct mqd_manager *mm, void *mqd, * Calculating queue size which is log base 2 of actual queue size -1 * dwords and another -1 for ffs */ - m->cp_hqd_pq_control |= ffs(q->queue_size / s
Re: [PATCH] drm/amd/display: small cleanup in destruct()
On Mon, Nov 6, 2017 at 3:44 AM, Christian König wrote: > Am 06.11.2017 um 08:07 schrieb Dan Carpenter: >> >> Static analysis tools get annoyed that we don't indent this if >> statement. Actually, the if statement isn't required because kfree() >> can handle NULL pointers just fine. The DCE110STRENC_FROM_STRENC() >> macro is a wrapper around container_of() but it's basically a no-op or a >> cast. Anyway, it's not really appropriate here so it should be removed >> as well. >> >> Signed-off-by: Dan Carpenter > > > Acked-by: Christian König Applied. thanks! Alex > >> --- >> v2: in v1 I just added a tab >> >> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c >> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c >> index d911590d08bc..4c4bd72d4e40 100644 >> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c >> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c >> @@ -725,10 +725,8 @@ static void destruct(struct dcn10_resource_pool >> *pool) >> } >> } >> - for (i = 0; i < pool->base.stream_enc_count; i++) { >> - if (pool->base.stream_enc[i] != NULL) >> - kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i])); >> - } >> + for (i = 0; i < pool->base.stream_enc_count; i++) >> + kfree(pool->base.stream_enc[i]); >> for (i = 0; i < pool->base.audio_count; i++) { >> if (pool->base.audios[i]) > > > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: checking for NULL instead of IS_ERR()
On Mon, Nov 6, 2017 at 6:51 AM, Christian König wrote: > Am 06.11.2017 um 12:43 schrieb Dan Carpenter: >> >> backlight_device_register() never returns NULL, it returns error >> pointers on error so the check here is wrong. >> >> Signed-off-by: Dan Carpenter > > > Acked-by: Christian König Applied. thanks! Alex > >> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> index 33a15a1d818c..75f9383f5b9b 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> @@ -1296,7 +1296,7 @@ amdgpu_dm_register_backlight_device(struct >> amdgpu_display_manager *dm) >> &amdgpu_dm_backlight_ops, >> &props); >> - if (NULL == dm->backlight_dev) >> + if (IS_ERR(dm->backlight_dev)) >> DRM_ERROR("DM: Backlight registration failed!\n"); >> else >> DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n", >> bl_name); > > > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: remove some unneeded code
On Mon, Nov 6, 2017 at 6:59 AM, Christian König wrote: > Am 06.11.2017 um 12:44 schrieb Dan Carpenter: >> >> We assign "v_init = asic_blank_start;" a few lines earlier so there is >> no need to do it again inside the if statements. Also "v_init" is >> unsigned so it can't be less than zero. >> >> Signed-off-by: Dan Carpenter > > > Acked-by: Christian König > Applied. thanks! Alex >> >> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c >> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c >> index 1994865d4351..c7333cdf1802 100644 >> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c >> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c >> @@ -236,13 +236,10 @@ static void tgn10_program_timing( >> if (tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT || >> tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT_MST >> || >> tg->dlg_otg_param.signal == SIGNAL_TYPE_EDP) { >> - v_init = asic_blank_start; >> start_point = 1; >> if (patched_crtc_timing.flags.INTERLACE == 1) >> field_num = 1; >> } >> - if (v_init < 0) >> - v_init = 0; >> v_fp2 = 0; >> if (tg->dlg_otg_param.vstartup_start > asic_blank_end) >> v_fp2 = tg->dlg_otg_param.vstartup_start > asic_blank_end; >> ___ >> amd-gfx mailing list >> amd-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/1] drm/amd/powerplay: initialize a variable before using it
On Sat, Nov 4, 2017 at 8:21 AM, Nicolas Iooss wrote: > On Sun, Sep 3, 2017 at 2:00 PM, Nicolas Iooss > wrote: >> >> Function vega10_apply_state_adjust_rules() only initializes >> stable_pstate_sclk_dpm_percentage when >> data->registry_data.stable_pstate_sclk_dpm_percentage is not between 1 >> and 100. The variable is then used to compute stable_pstate_sclk, which >> therefore uses an uninitialized value. >> >> Fix this by initializing stable_pstate_sclk_dpm_percentage to >> data->registry_data.stable_pstate_sclk_dpm_percentage. >> >> This issue has been found while building the kernel with clang. The >> compiler reported a -Wsometimes-uninitialized warning. >> >> Fixes: f83a9991648b ("drm/amd/powerplay: add Vega10 powerplay support (v5)") >> Signed-off-by: Nicolas Iooss >> --- >> drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c >> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c >> index 197174e562d2..c8d28f78cd47 100644 >> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c >> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c >> @@ -3043,6 +3043,8 @@ static int vega10_apply_state_adjust_rules(struct >> pp_hwmgr *hwmgr, >> >> if (phm_cap_enabled(hwmgr->platform_descriptor.platformCaps, >> PHM_PlatformCaps_StablePState)) { >> + stable_pstate_sclk_dpm_percentage = >> + >> data->registry_data.stable_pstate_sclk_dpm_percentage; >> PP_ASSERT_WITH_CODE( >> >> data->registry_data.stable_pstate_sclk_dpm_percentage >= 1 && >> >> data->registry_data.stable_pstate_sclk_dpm_percentage <= 100, >> -- >> 2.14.1 > > Hello, > I have not received any comment on the above patch that I sent two > months ago, and the issue which is fixed by it still exists in today's > linux-next code [1]. Could you please review this patch? Reviewed and applied. Sorry for missing this. Feel free to ping sooner if it looks like something slipped through the cracks. Thanks, Alex > > Thanks, > Nicolas > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c#n3137 > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 2/2] drm/amdkfd: Use order_base_2 to get log2 of buffes sizes
> -Original Message- > From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf > Of Felix Kuehling > Sent: Monday, November 06, 2017 2:52 PM > To: amd-gfx@lists.freedesktop.org; oded.gab...@gmail.com > Cc: Kuehling, Felix > Subject: [PATCH 2/2] drm/amdkfd: Use order_base_2 to get log2 of buffes > sizes > > Replace (ffs(size) - 1) with order_base_2(size) as a more straight > forward way to get log2 of buffer sizes. > > Signed-off-by: Felix Kuehling Series is: Reviewed-by: Alex Deucher > --- > drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 6 +++--- > drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c | 6 +++--- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c > b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c > index efed6ef..7aa57ab 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c > @@ -183,7 +183,7 @@ static int update_mqd(struct mqd_manager *mm, > void *mqd, >* Calculating queue size which is log base 2 of actual queue size -1 >* dwords and another -1 for ffs >*/ > - m->cp_hqd_pq_control |= ffs(q->queue_size / 4) - 1 - 1; > + m->cp_hqd_pq_control |= order_base_2(q->queue_size / 4) - 1; > m->cp_hqd_pq_base_lo = lower_32_bits((uint64_t)q- > >queue_address >> 8); > m->cp_hqd_pq_base_hi = upper_32_bits((uint64_t)q- > >queue_address >> 8); > m->cp_hqd_pq_rptr_report_addr_lo = lower_32_bits((uint64_t)q- > >read_ptr); > @@ -208,7 +208,7 @@ static int update_mqd_sdma(struct mqd_manager > *mm, void *mqd, > struct cik_sdma_rlc_registers *m; > > m = get_sdma_mqd(mqd); > - m->sdma_rlc_rb_cntl = (ffs(q->queue_size / 4) - 1) > + m->sdma_rlc_rb_cntl = order_base_2(q->queue_size / 4) > << SDMA0_RLC0_RB_CNTL__RB_SIZE__SHIFT | > q->vmid << > SDMA0_RLC0_RB_CNTL__RB_VMID__SHIFT | > 1 << > SDMA0_RLC0_RB_CNTL__RPTR_WRITEBACK_ENABLE__SHIFT | > @@ -349,7 +349,7 @@ static int update_mqd_hiq(struct mqd_manager > *mm, void *mqd, >* Calculating queue size which is log base 2 of actual queue >* size -1 dwords >*/ > - m->cp_hqd_pq_control |= ffs(q->queue_size / 4) - 1 - 1; > + m->cp_hqd_pq_control |= order_base_2(q->queue_size / 4) - 1; > m->cp_hqd_pq_base_lo = lower_32_bits((uint64_t)q- > >queue_address >> 8); > m->cp_hqd_pq_base_hi = upper_32_bits((uint64_t)q- > >queue_address >> 8); > m->cp_hqd_pq_rptr_report_addr_lo = lower_32_bits((uint64_t)q- > >read_ptr); > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c > b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c > index 85e1b67..2ba7cea 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c > @@ -121,7 +121,7 @@ static int __update_mqd(struct mqd_manager *mm, > void *mqd, > m->cp_hqd_pq_control = 5 << > CP_HQD_PQ_CONTROL__RPTR_BLOCK_SIZE__SHIFT | > atc_bit << CP_HQD_PQ_CONTROL__PQ_ATC__SHIFT > | > mtype << CP_HQD_PQ_CONTROL__MTYPE__SHIFT; > - m->cp_hqd_pq_control |= ffs(q->queue_size / 4) - 1 - 1; > + m->cp_hqd_pq_control |= order_base_2(q->queue_size / 4) - 1; > pr_debug("cp_hqd_pq_control 0x%x\n", m->cp_hqd_pq_control); > > m->cp_hqd_pq_base_lo = lower_32_bits((uint64_t)q- > >queue_address >> 8); > @@ -151,7 +151,7 @@ static int __update_mqd(struct mqd_manager *mm, > void *mqd, >* is safe, giving a maximum field value of 0xA. >*/ > m->cp_hqd_eop_control |= min(0xA, > - ffs(q->eop_ring_buffer_size / 4) - 1 - 1); > + order_base_2(q->eop_ring_buffer_size / 4) - 1); > m->cp_hqd_eop_base_addr_lo = > lower_32_bits(q->eop_ring_buffer_address >> 8); > m->cp_hqd_eop_base_addr_hi = > @@ -287,7 +287,7 @@ static int update_mqd_sdma(struct mqd_manager > *mm, void *mqd, > struct vi_sdma_mqd *m; > > m = get_sdma_mqd(mqd); > - m->sdmax_rlcx_rb_cntl = (ffs(q->queue_size / 4) - 1) > + m->sdmax_rlcx_rb_cntl = order_base_2(q->queue_size / 4) > << SDMA0_RLC0_RB_CNTL__RB_SIZE__SHIFT | > q->vmid << SDMA0_RLC0_RB_CNTL__RB_VMID__SHIFT | > 1 << > SDMA0_RLC0_RB_CNTL__RPTR_WRITEBACK_ENABLE__SHIFT | > -- > 2.7.4 > > ___ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 3/3] amdgpu/dc: Fix missing null checks in amdgpu_dm.c
From smatch: error: we previously assumed X could be null Signed-off-by: Ernst Sjöstrand --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 2301589e4cc3..d036178c2241 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -432,8 +432,10 @@ static int amdgpu_dm_init(struct amdgpu_device *adev) if (adev->dm.dc) DRM_INFO("Display Core initialized!\n"); - else + else { DRM_INFO("Display Core failed to initialize!\n"); + goto error; + } INIT_WORK(&adev->dm.mst_hotplug_work, hotplug_notify_work_func); @@ -2263,7 +2265,7 @@ decide_crtc_timing_for_drm_display_mode(struct drm_display_mode *drm_mode, } } -static void create_fake_sink(struct amdgpu_dm_connector *aconnector) +static int create_fake_sink(struct amdgpu_dm_connector *aconnector) { struct dc_sink *sink = NULL; struct dc_sink_init_data sink_init_data = { 0 }; @@ -2272,14 +2274,18 @@ static void create_fake_sink(struct amdgpu_dm_connector *aconnector) sink_init_data.sink_signal = aconnector->dc_link->connector_signal; sink = dc_sink_create(&sink_init_data); - if (!sink) + if (!sink) { DRM_ERROR("Failed to create sink!\n"); + return -1; + } sink->sink_signal = SIGNAL_TYPE_VIRTUAL; aconnector->fake_enable = true; aconnector->dc_sink = sink; aconnector->dc_link->local_sink = sink; + + return 0; } static struct dc_stream_state * @@ -2313,7 +2319,8 @@ create_stream_for_sink(struct amdgpu_dm_connector *aconnector, if (aconnector->mst_port) goto stream_create_fail; - create_fake_sink(aconnector); + if (create_fake_sink(aconnector)) + goto stream_create_fail; } stream = dc_create_stream_for_sink(aconnector->dc_sink); -- 2.14.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More smatch fixes
I saw the previous smatch fixes and thought I would try to fix a few. Since the other patches were applied to agd5f/amd-staging-drm-next I continued there. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/3] amdgpu/dc: Fix potential null dereference in amdgpu_dm.c
Signed-off-by: Ernst Sjöstrand --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index e6bfa9f30900..2301589e4cc3 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -2413,6 +2413,8 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc) return NULL; state = kzalloc(sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; __drm_atomic_helper_crtc_duplicate_state(crtc, &state->base); @@ -3443,6 +3445,8 @@ create_i2c(struct ddc_service *ddc_service, struct amdgpu_i2c_adapter *i2c; i2c = kzalloc(sizeof(struct amdgpu_i2c_adapter), GFP_KERNEL); + if (!i2c) + return NULL; i2c->base.owner = THIS_MODULE; i2c->base.class = I2C_CLASS_DDC; i2c->base.dev.parent = &adev->pdev->dev; @@ -3473,6 +3477,11 @@ static int amdgpu_dm_connector_init(struct amdgpu_display_manager *dm, DRM_DEBUG_DRIVER("%s()\n", __func__); i2c = create_i2c(link->ddc, link->link_index, &res); + if (!i2c) { + DRM_ERROR("Failed to create i2c adapter data\n"); + return -1; + } + aconnector->i2c = i2c; res = i2c_add_adapter(&i2c->base); -- 2.14.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/3] amdgpu/dc: fix more indentation warnings
More "warn: inconsistent indenting" fixes from smatch. Signed-off-by: Ernst Sjöstrand --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 10 +- .../gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 67cad46f9f15..e6bfa9f30900 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -517,7 +517,7 @@ static int detect_mst_link_for_all_connectors(struct drm_device *dev) drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); list_for_each_entry(connector, &dev->mode_config.connector_list, head) { - aconnector = to_amdgpu_dm_connector(connector); + aconnector = to_amdgpu_dm_connector(connector); if (aconnector->dc_link->type == dc_connection_mst_branch) { DRM_DEBUG_DRIVER("DM_MST: starting TM on aconnector: %p [id: %d]\n", aconnector, aconnector->base.base.id); @@ -4754,10 +4754,10 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, goto fail; } -/* Run this here since we want to validate the streams we created */ -ret = drm_atomic_helper_check_planes(dev, state); -if (ret) -goto fail; + /* Run this here since we want to validate the streams we created */ + ret = drm_atomic_helper_check_planes(dev, state); + if (ret) + goto fail; /* Check scaling and underscan changes*/ /*TODO Removed scaling changes validation due to inability to commit diff --git a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c b/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c index 0c4bbc10510d..81f9f3e34c10 100644 --- a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c +++ b/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c @@ -318,7 +318,7 @@ static void process_channel_reply( REG_GET(AUX_SW_DATA, AUX_SW_DATA, &aux_sw_data_val); -reply->data[i] = aux_sw_data_val; + reply->data[i] = aux_sw_data_val; ++i; } -- 2.14.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: resize VRAM BAR for CPU access v5
It doesn’t work without that write. Meanwhile, this functionality doesn’t change BAR0 and BAR2 when operating on VF, just as you said it’s not necessary for VF. So I think we can skip the whole stuff. Please help to review the patch coming later. — Sincerely Yours, Pixel On 06/11/2017, 5:52 PM, "Christian König" wrote: >> What benefits will the larger VRAM bar bring in, >Well the obvious one that the CPU can access all of VRAM. There are some >games/workloads which rely quite a bit on this and it simplifies MM >largely when you don't need to shuffle buffers around for CPU access. > >> or are there new features relying on larger VRAM bar under development? >Not that I know of. > >> I think it’s better to try something which can keep this change for SRIOV >> and don’t introduce too much latency. >Agree on that, but reprogramming the PCI BAR is something which takes >quite a bunch of time. > >That's why I asked if it is sufficient if you comment out this one >pci_write_config_word() and it works? > >If yes than we can just skip that write, but I have doubts that this >will be sufficient. > >On the other hand in an SRIOV environment the host can resize the BAR >before starting the client, so that whole stuff shouldn't be necessary >in the first place. > >Regards, >Christian. > >Am 06.11.2017 um 10:34 schrieb Ding, Pixel: >> What benefits will the larger VRAM bar bring in, or are there new features >> relying on larger VRAM bar under development? SRIOV wants to leverage this >> sort of optimization too. >> >> I think it’s better to try something which can keep this change for SRIOV >> and don’t introduce too much latency. >> >> — >> Sincerely Yours, >> Pixel >> >> >> >> >> >> >> >> On 06/11/2017, 5:16 PM, "Christian König" >> wrote: >> >>> Am 06.11.2017 um 08:21 schrieb Ding, Pixel: Hi Christian, The latest driver fails to load on SRIOV VF with xen hypervisor driver. +int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev) +{ + u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size); + u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) - 1; + u16 cmd; + int r; + + /* Disable memory decoding while we change the BAR addresses and size */ + pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd); + pci_write_config_word(adev->pdev, PCI_COMMAND, The function above introduces 900ms latency during init in exclusive accessing. + cmd & ~PCI_COMMAND_MEMORY); + Can we bypass disablement of response in memory space here? Any concern or suggestion? >>> Mhm, that was added to be on the extra safe side. In theory we can skip it. >>> >>> Does PCI BAR resize work on SRIOV if you skip this or is that just a NOP >>> anyway? >>> >>> Might be a good idea to just immediately return here when SRIOV is active. >>> >>> Regards, >>> Christian. >>> — Sincerely Yours, Pixel On 02/11/2017, 9:13 PM, "amd-gfx on behalf of Alex Deucher" wrote: > On Thu, Nov 2, 2017 at 9:02 AM, Christian König > wrote: >> From: Christian König >> >> Try to resize BAR0 to let CPU access all of VRAM. >> >> v2: rebased, style cleanups, disable mem decode before resize, >> handle gmc_v9 as well, round size up to power of two. >> v3: handle gmc_v6 as well, release and reassign all BARs in the driver. >> v4: rename new function to amdgpu_device_resize_fb_bar, >> reenable mem decoding only if all resources are assigned. >> v5: reorder resource release, return -ENODEV instead of BUG_ON(). >> >> Signed-off-by: Christian König > Reviewed-by: Alex Deucher > >> --- >>drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 + >>drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 48 >> ++ >>drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 12 ++-- >>drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 13 ++-- >>drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 13 ++-- >>drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 14 ++--- >>6 files changed, 88 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> index 2730a75..4f919d3 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> @@ -1854,6 +1854,7 @@ void amdgpu_ttm_placement_from_domain(struct >> amdgpu_bo *abo, u32 domain); >>bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo); >>void amdgpu_vram_location(struct amdgpu_device *adev, struct >> amdgpu_mc *mc, u64 base); >>void amdgpu_gart_location(struct amdgpu_device *adev, struct >> amdgpu_mc *mc); >> +int amdgpu_device_resize_fb_bar(struct amdg
[PATCH 2/2] drm/amd/scheduler: remove fence callback when context is killed
Otherwise there could be page protection. Change-Id: I1f6c81002fb2ba21c17cdc14fdde86579b28374e Signed-off-by: Chunming Zhou --- drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c index 310904042dfc..e38d55961a9f 100644 --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c @@ -243,6 +243,12 @@ void amd_sched_entity_fini(struct amd_gpu_scheduler *sched, amd_sched_fence_scheduled(s_fence); kcl_dma_fence_set_error(&s_fence->finished, -ESRCH); amd_sched_fence_finished(s_fence); + if (s_fence->parent) { + dma_fence_remove_callback(s_fence->parent, + &s_fence->cb); + dma_fence_put(s_fence->parent); + s_fence->parent = NULL; + } dma_fence_put(&s_fence->finished); sched->ops->free_job(job); } -- 2.14.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/2] drm/amd/scheduler: fix page protection of cb
We must remove the fence callback. Change-Id: I5d58a3a43b82fefd6c211c4128b0c9187c191e7f Signed-off-by: Chunming Zhou --- drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c index af5b2c50abac..310904042dfc 100644 --- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c +++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c @@ -231,6 +231,13 @@ void amd_sched_entity_fini(struct amd_gpu_scheduler *sched, */ kcl_kthread_park(sched->thread); kcl_kthread_unpark(sched->thread); + if (entity->dependency) { + dma_fence_remove_callback(entity->dependency, + &entity->cb); + dma_fence_put(entity->dependency); + entity->dependency = NULL; + } + while ((job = to_amd_sched_job(spsc_queue_pop(&entity->job_queue { struct amd_sched_fence *s_fence = job->s_fence; amd_sched_fence_scheduled(s_fence); -- 2.14.1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: bypass FB resizing for SRIOV VF
From: pding It introduces 900ms latency in exclusive mode which causes failure of driver loading. Host can resize the BAR before guest staring, so the resizing is not necessary here. Signed-off-by: pding --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 6f01dda..bf2b008 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -768,6 +768,10 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev) u16 cmd; int r; + /* Bypass for VF */ + if (amdgpu_sriov_vf(adev)) + return 0; + /* Disable memory decoding while we change the BAR addresses and size */ pci_read_config_word(adev->pdev, PCI_COMMAND, &cmd); pci_write_config_word(adev->pdev, PCI_COMMAND, -- 2.9.5 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/2] drm/amdgpu: use multipipe compute policy on non PL11 asics
Then snychronization should have no problem, it maybe relate to multipipe hw setting issue. Regards, David Zhou From: Andres Rodriguez Sent: Tuesday, November 7, 2017 2:00:57 AM To: Zhou, David(ChunMing); amd-gfx list Cc: Deucher, Alexander Subject: Re: [PATCH 1/2] drm/amdgpu: use multipipe compute policy on non PL11 asics Sorry my mail client seems to have blown up. My reply got cut off, here is the full version: On 2017-11-06 01:49 AM, Chunming Zhou wrote: > Hi Andres, > Hi David, > With your this patch, OCLperf hung. Is this on all ASICs or just a specific one? > > Could you explain more? > > If I am correctly, the difference of with and without this patch is > setting first two queue or setting all queues of pipe0 to queue_bitmap. It is slightly different. With this patch we will also use the first two queues of all pipes, not just pipe 0; Pre-patch: |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-| Post-patch: |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-| 1100 1100 1100 1100 What this means is that we are allowing real multithreading for compute. Jobs on different pipes allow for parallel execution of work. Jobs on the same pipe (but different queues) use timeslicing to share the hardware. > > Then UMD can use different number queue to submit command for compute > selected by amdgpu_queue_mgr_map. > > I checked amdgpu_queue_mgr_map implementation, CS_IOCTL can map user > ring to different hw ring depending on busy or idle, right? Yes, when a queue is first used, amdgpu_queue_mgr_map will decide what the mapping is for a usermode ring to a kernel ring id. > If yes, I see a bug in it, which will result in our sched_fence not > work. Our sched fence assumes the job will be executed in order, your > mapping queue breaks this. I think here you mean that work will execute out of order because it will go to different rings? That should not happen, since the id mapping is permanent on a per-context basis. Once a mapping is decided, it will be cached for this context so that we keep execution order guarantees. See the id-caching code in amdgpu_queue_mgr.c for reference. As long as the usermode keeps submitting work to the same ring, it will all be executed in order (all in the same ring). There is no change in this guarantee compared to pre-patch. Note that even before this patch amdgpu_queue_mgr_map has been using an LRU policy for a long time now. Regards, Andres On Mon, Nov 6, 2017 at 12:44 PM, Andres Rodriguez wrote: > > > On 2017-11-06 01:49 AM, Chunming Zhou wrote: >> >> Hi Andres, >> > > Hi David, > >> With your this patch, OCLperf hung. > > > Is this on all ASICs or just a specific one? > >> >> Could you explain more? >> >> If I am correctly, the difference of with and without this patch is >> setting first two queue or setting all queues of pipe0 to queue_bitmap. > > > It is slightly different. With this patch we will also use the first two > queues of all pipes, not just pipe 0; > > Pre-patch: > > |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-| > > > Post-patch: > > |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-| > 1100 1100 1100 1100 > > What this means is that we are allowing real multithreading for compute. > Jobs on different pipes allow for parallel execution of work. Jobs on the > same pipe (but different queues) use timeslicing to share the hardware. > > > >> >> Then UMD can use different number queue to submit command for compute >> selected by amdgpu_queue_mgr_map. >> >> I checked amdgpu_queue_mgr_map implementation, CS_IOCTL can map user ring >> to different hw ring depending on busy or idle, right? >> >> If yes, I see a bug in it, which will result in our sched_fence not work. >> Our sched fence assumes the job will be executed in order, your mapping >> queue breaks this. >> >> >> Regards, >> >> David Zhou >> >> >> On 2017年09月27日 00:22, Andres Rodriguez wrote: >>> >>> A performance regression for OpenCL tests on Polaris11 had this feature >>> disabled for all asics. >>> >>> Instead, disable it selectively on the affected asics. >>> >>> Signed-off-by: Andres Rodriguez >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 14 -- >>> 1 file changed, 12 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c >>> index 4f6c68f..3d76e76 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c >>> @@ -109,9 +109,20 @@ void amdgpu_gfx_parse_disable_cu(unsigned *mask, >>> unsigned max_se, unsigned max_s >>> } >>> } >>> +static bool amdgpu_gfx_is_multipipe_capable(struct amdgpu_device *adev) >>> +{ >>> +/* FIXME: spreading the queues across pipes causes perf regressions >>> + * on POLARIS11 compute workloads */ >>> +if (adev->asic_type == CHIP_POLARIS11) >>> +return false; >>>
[PATCH] drm/amd/display: fix static checker warning
From: Shirish S This patch fixes static checker warning of "warn: cast after binop" introduced by 4d3e00dad80a: "drm/amd/display : add high part address calculation for underlay" Signed-off-by: Shirish S --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index a87e5ac..e1bdf5e 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -1827,7 +1827,7 @@ static int fill_plane_attributes_from_fb(struct amdgpu_device *adev, = lower_32_bits(fb_location); plane_state->address.video_progressive.luma_addr.high_part = upper_32_bits(fb_location); - chroma_addr = fb_location + (u64)(awidth * fb->height); + chroma_addr = fb_location + (u64)awidth * fb->height; plane_state->address.video_progressive.chroma_addr.low_part = lower_32_bits(chroma_addr); plane_state->address.video_progressive.chroma_addr.high_part @@ -2959,7 +2959,7 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane, = lower_32_bits(afb->address); plane_state->address.video_progressive.luma_addr.high_part = upper_32_bits(afb->address); - chroma_addr = afb->address + (u64)(awidth * new_state->fb->height); + chroma_addr = afb->address + (u64)awidth * new_state->fb->height; plane_state->address.video_progressive.chroma_addr.low_part = lower_32_bits(chroma_addr); plane_state->address.video_progressive.chroma_addr.high_part -- 2.7.4 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/2] drm/amdgpu: use multipipe compute policy on non PL11 asics
Do you have any work actually going into multiple pipes? My understanding is that opencl will only use one queue at a time (but I'm not really certain about that). What you can also check is if the app works correctly when it executed on pipe0, and if it hangs on pipe 1+. I removed all the locations where pipe0 was hardcoded in the open driver, but it is possible it is still hardcoded somewhere on the closed stack. Regards, Andres On Nov 6, 2017 10:19 PM, "Zhou, David(ChunMing)" wrote: > Then snychronization should have no problem, it maybe relate to multipipe > hw setting issue. > > > Regards, > > David Zhou > -- > *From:* Andres Rodriguez > *Sent:* Tuesday, November 7, 2017 2:00:57 AM > *To:* Zhou, David(ChunMing); amd-gfx list > *Cc:* Deucher, Alexander > *Subject:* Re: [PATCH 1/2] drm/amdgpu: use multipipe compute policy on > non PL11 asics > > Sorry my mail client seems to have blown up. My reply got cut off, > here is the full version: > > > > On 2017-11-06 01:49 AM, Chunming Zhou wrote: > > Hi Andres, > > > Hi David, > > > With your this patch, OCLperf hung. > Is this on all ASICs or just a specific one? > > > > > Could you explain more? > > > > If I am correctly, the difference of with and without this patch is > > setting first two queue or setting all queues of pipe0 to queue_bitmap. > It is slightly different. With this patch we will also use the first > two queues of all pipes, not just pipe 0; > > Pre-patch: > > |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-| > > > Post-patch: > > |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-| > 1100 1100 1100 1100 > > What this means is that we are allowing real multithreading for > compute. Jobs on different pipes allow for parallel execution of work. > Jobs on the same pipe (but different queues) use timeslicing to share > the hardware. > > > > > > Then UMD can use different number queue to submit command for compute > > selected by amdgpu_queue_mgr_map. > > > > I checked amdgpu_queue_mgr_map implementation, CS_IOCTL can map user > > ring to different hw ring depending on busy or idle, right? > Yes, when a queue is first used, amdgpu_queue_mgr_map will decide what > the mapping is for a usermode ring to a kernel ring id. > > > If yes, I see a bug in it, which will result in our sched_fence not > > work. Our sched fence assumes the job will be executed in order, your > > mapping queue breaks this. > > I think here you mean that work will execute out of order because it > will go to different rings? > > That should not happen, since the id mapping is permanent on a > per-context basis. Once a mapping is decided, it will be cached for > this context so that we keep execution order guarantees. See the > id-caching code in amdgpu_queue_mgr.c for reference. > > As long as the usermode keeps submitting work to the same ring, it > will all be executed in order (all in the same ring). There is no > change in this guarantee compared to pre-patch. Note that even before > this patch amdgpu_queue_mgr_map has been using an LRU policy for a > long time now. > > Regards, > Andres > > On Mon, Nov 6, 2017 at 12:44 PM, Andres Rodriguez > wrote: > > > > > > On 2017-11-06 01:49 AM, Chunming Zhou wrote: > >> > >> Hi Andres, > >> > > > > Hi David, > > > >> With your this patch, OCLperf hung. > > > > > > Is this on all ASICs or just a specific one? > > > >> > >> Could you explain more? > >> > >> If I am correctly, the difference of with and without this patch is > >> setting first two queue or setting all queues of pipe0 to queue_bitmap. > > > > > > It is slightly different. With this patch we will also use the first two > > queues of all pipes, not just pipe 0; > > > > Pre-patch: > > > > |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-| > > > > > > Post-patch: > > > > |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-| > > 1100 1100 1100 1100 > > > > What this means is that we are allowing real multithreading for compute. > > Jobs on different pipes allow for parallel execution of work. Jobs on the > > same pipe (but different queues) use timeslicing to share the hardware. > > > > > > > >> > >> Then UMD can use different number queue to submit command for compute > >> selected by amdgpu_queue_mgr_map. > >> > >> I checked amdgpu_queue_mgr_map implementation, CS_IOCTL can map user > ring > >> to different hw ring depending on busy or idle, right? > >> > >> If yes, I see a bug in it, which will result in our sched_fence not > work. > >> Our sched fence assumes the job will be executed in order, your mapping > >> queue breaks this. > >> > >> > >> Regards, > >> > >> David Zhou > >> > >> > >> On 2017年09月27日 00:22, Andres Rodriguez wrote: > >>> > >>> A performance regression for OpenCL tests on Polaris11 had this feature > >>> disabled for all asics. > >>> > >>> Instead, disable it selectively on the affected asics. > >>> > >>> Signed-off-by: Andres Rodriguez > >>
[PATCH 2/2] drm/amdgpu: use irq-safe lock for adev->ring_lru_list_lock
From: pding This lock is used during register accessing in SRIOV guest since KIQ uses general ring submission (amdgpu_ring_commit). The register accessing could happen both in irq enabled and irq disabled cases. Always use irq-safe lock. Signed-off-by: pding --- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c index e5ece1f..d7997b5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c @@ -401,11 +401,12 @@ int amdgpu_ring_lru_get(struct amdgpu_device *adev, int type, bool lru_pipe_order, struct amdgpu_ring **ring) { struct amdgpu_ring *entry; + unsigned long flags; /* List is sorted in LRU order, find first entry corresponding * to the desired HW IP */ *ring = NULL; - spin_lock(&adev->ring_lru_list_lock); + spin_lock_irqsave(&adev->ring_lru_list_lock, flags); list_for_each_entry(entry, &adev->ring_lru_list, lru_list) { if (entry->funcs->type != type) continue; @@ -430,7 +431,7 @@ int amdgpu_ring_lru_get(struct amdgpu_device *adev, int type, if (*ring) amdgpu_ring_lru_touch_locked(adev, *ring); - spin_unlock(&adev->ring_lru_list_lock); + spin_unlock_irqrestore(&adev->ring_lru_list_lock, flags); if (!*ring) { DRM_ERROR("Ring LRU contains no entries for ring type:%d\n", type); @@ -450,9 +451,11 @@ int amdgpu_ring_lru_get(struct amdgpu_device *adev, int type, */ void amdgpu_ring_lru_touch(struct amdgpu_device *adev, struct amdgpu_ring *ring) { - spin_lock(&adev->ring_lru_list_lock); + unsigned long flags; + + spin_lock_irqsave(&adev->ring_lru_list_lock, flags); amdgpu_ring_lru_touch_locked(adev, ring); - spin_unlock(&adev->ring_lru_list_lock); + spin_unlock_irqrestore(&adev->ring_lru_list_lock, flags); } /* -- 2.9.5 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] drm/amdgpu:remove debugfs file in amdgpu_device_finish
Hi Christian, The patch is for driver re- initialize feature, not for driver exit or rmmod. When the driver initialize failed at some point, the re- initialize feature will do some little clean and then try to initialize driver again, then it will re-register some registered debugfs , so it will fail. Regards, Gary -Original Message- From: Koenig, Christian Sent: Monday, November 06, 2017 5:26 PM To: Sun, Gary ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu:remove debugfs file in amdgpu_device_finish Am 06.11.2017 um 10:20 schrieb Gary Sun: > remove debugfs file in amdgpu_device_finish NAK, the debugfs files are removed automatically by drm_debugfs_cleanup(). So that patch is unnecessary. Regards, Christian. > > Signed-off-by: Gary Sun > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h|1 + > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++ > 2 files changed, 19 insertions(+), 0 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index 4f919d3..6cfcb5f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -1250,6 +1250,7 @@ struct amdgpu_debugfs { > int amdgpu_debugfs_add_files(struct amdgpu_device *adev, >const struct drm_info_list *files, >unsigned nfiles); > +int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev); > int amdgpu_debugfs_fence_init(struct amdgpu_device *adev); > > #if defined(CONFIG_DEBUG_FS) > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 7b7439f..ee800ab 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -2520,6 +2520,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev) > amdgpu_doorbell_fini(adev); > amdgpu_pm_sysfs_fini(adev); > amdgpu_debugfs_regs_cleanup(adev); > + amdgpu_debugfs_cleanup_files(adev); > } > > > @@ -3304,6 +3305,23 @@ int amdgpu_debugfs_add_files(struct amdgpu_device > *adev, > return 0; > } > > +int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev) { > + unsigned int i; > + > + for (i = 0; i < adev->debugfs_count; i++) { #if > +defined(CONFIG_DEBUG_FS) > + drm_debugfs_remove_files(adev->debugfs[i].files, > + adev->debugfs[i].num_files, > + adev->ddev->primary); > +#endif > + adev->debugfs[i].files = NUL; > + adev->debugfs[i].num_files = 0; > + } > + adev->debugfs_count = 0; > + return 0; > +} > + > #if defined(CONFIG_DEBUG_FS) > > static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user > *buf, ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 1/2] drm/amdgpu: use irq-safe lock for kiq->ring_lock
From: pding This lock is used during register accessing in SRIOV guest. The register accessing could happen both in irq enabled and irq disabled cases. Always use irq-safe lock. Signed-off-by: pding --- drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c index 9a73918..733c64c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c @@ -120,18 +120,19 @@ void amdgpu_virt_init_setting(struct amdgpu_device *adev) uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg) { signed long r; + unsigned long flags; uint32_t val, seq; struct amdgpu_kiq *kiq = &adev->gfx.kiq; struct amdgpu_ring *ring = &kiq->ring; BUG_ON(!ring->funcs->emit_rreg); - spin_lock(&kiq->ring_lock); + spin_lock_irqsave(&kiq->ring_lock, flags); amdgpu_ring_alloc(ring, 32); amdgpu_ring_emit_rreg(ring, reg); amdgpu_fence_emit_polling(ring, &seq); amdgpu_ring_commit(ring); - spin_unlock(&kiq->ring_lock); + spin_unlock_irqrestore(&kiq->ring_lock, flags); r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT); if (r < 1) { @@ -146,18 +147,19 @@ uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg) void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v) { signed long r; + unsigned long flags; uint32_t seq; struct amdgpu_kiq *kiq = &adev->gfx.kiq; struct amdgpu_ring *ring = &kiq->ring; BUG_ON(!ring->funcs->emit_wreg); - spin_lock(&kiq->ring_lock); + spin_lock_irqsave(&kiq->ring_lock, flags); amdgpu_ring_alloc(ring, 32); amdgpu_ring_emit_wreg(ring, reg, v); amdgpu_fence_emit_polling(ring, &seq); amdgpu_ring_commit(ring); - spin_unlock(&kiq->ring_lock); + spin_unlock_irqrestore(&kiq->ring_lock, flags); r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT); if (r < 1) -- 2.9.5 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu:remove debugfs file in amdgpu_device_finish
Hi Gary, not sure what driver re-initialize feature you are talking about, but the last time I tried to re-initialize the driver it deadlocks in the modeset code because of some DC problem. It's probably a good idea to fix that first, but in general please explain further what are you working on. Regards, Christian. Am 07.11.2017 um 08:23 schrieb Sun, Gary: Hi Christian, The patch is for driver re- initialize feature, not for driver exit or rmmod. When the driver initialize failed at some point, the re- initialize feature will do some little clean and then try to initialize driver again, then it will re-register some registered debugfs , so it will fail. Regards, Gary -Original Message- From: Koenig, Christian Sent: Monday, November 06, 2017 5:26 PM To: Sun, Gary ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH] drm/amdgpu:remove debugfs file in amdgpu_device_finish Am 06.11.2017 um 10:20 schrieb Gary Sun: remove debugfs file in amdgpu_device_finish NAK, the debugfs files are removed automatically by drm_debugfs_cleanup(). So that patch is unnecessary. Regards, Christian. Signed-off-by: Gary Sun --- drivers/gpu/drm/amd/amdgpu/amdgpu.h|1 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 18 ++ 2 files changed, 19 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 4f919d3..6cfcb5f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1250,6 +1250,7 @@ struct amdgpu_debugfs { int amdgpu_debugfs_add_files(struct amdgpu_device *adev, const struct drm_info_list *files, unsigned nfiles); +int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev); int amdgpu_debugfs_fence_init(struct amdgpu_device *adev); #if defined(CONFIG_DEBUG_FS) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 7b7439f..ee800ab 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2520,6 +2520,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev) amdgpu_doorbell_fini(adev); amdgpu_pm_sysfs_fini(adev); amdgpu_debugfs_regs_cleanup(adev); + amdgpu_debugfs_cleanup_files(adev); } @@ -3304,6 +3305,23 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev, return 0; } +int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev) { + unsigned int i; + + for (i = 0; i < adev->debugfs_count; i++) { #if +defined(CONFIG_DEBUG_FS) + drm_debugfs_remove_files(adev->debugfs[i].files, + adev->debugfs[i].num_files, + adev->ddev->primary); +#endif + adev->debugfs[i].files = NUL; + adev->debugfs[i].num_files = 0; + } + adev->debugfs_count = 0; + return 0; +} + #if defined(CONFIG_DEBUG_FS) static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf, ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx