[PATCH 2/2] drm/display/dp_mst: Fix drm_dp_mst_add_affected_dsc_crtcs() return code
Looks like that we're accidentally dropping a pretty important return code
here. For some reason, we just return -EINVAL if we fail to get the MST
topology state. This is wrong: error codes are important and should never
be squashed without being handled, which here seems to have the potential
to cause a deadlock.
Signed-off-by: Lyude Paul
Fixes: 8ec046716ca8 ("drm/dp_mst: Add helper to trigger modeset on affected DSC
MST CRTCs")
Cc: # v5.6+
---
drivers/gpu/drm/display/drm_dp_mst_topology.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c
b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index ecd22c038c8c0..51a46689cda70 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -5186,7 +5186,7 @@ int drm_dp_mst_add_affected_dsc_crtcs(struct
drm_atomic_state *state, struct drm
mst_state = drm_atomic_get_mst_topology_state(state, mgr);
if (IS_ERR(mst_state))
- return -EINVAL;
+ return PTR_ERR(mst_state);
list_for_each_entry(pos, &mst_state->payloads, next) {
--
2.37.3
[PATCH 1/2] drm/amdgpu/mst: Stop ignoring error codes and deadlocking
It appears that amdgpu makes the mistake of completely ignoring the return
values from the DP MST helpers, and instead just returns a simple
true/false. In this case, it seems to have come back to bite us because as
a result of simply returning false from
compute_mst_dsc_configs_for_state(), amdgpu had no way of telling when a
deadlock happened from these helpers. This could definitely result in some
kernel splats.
Signed-off-by: Lyude Paul
Fixes: 8c20a1ed9b4f ("drm/amd/display: MST DSC compute fair share")
Cc: Harry Wentland
Cc: # v5.6+
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 18 +--
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 107 ++
.../display/amdgpu_dm/amdgpu_dm_mst_types.h | 12 +-
3 files changed, 73 insertions(+), 64 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 0db2a88cd4d7b..6f76b2c84cdb5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6462,7 +6462,7 @@ static int dm_update_mst_vcpi_slots_for_dsc(struct
drm_atomic_state *state,
struct drm_connector_state *new_con_state;
struct amdgpu_dm_connector *aconnector;
struct dm_connector_state *dm_conn_state;
- int i, j;
+ int i, j, ret;
int vcpi, pbn_div, pbn, slot_num = 0;
for_each_new_connector_in_state(state, connector, new_con_state, i) {
@@ -6509,8 +6509,11 @@ static int dm_update_mst_vcpi_slots_for_dsc(struct
drm_atomic_state *state,
dm_conn_state->pbn = pbn;
dm_conn_state->vcpi_slots = slot_num;
- drm_dp_mst_atomic_enable_dsc(state, aconnector->port,
dm_conn_state->pbn,
-false);
+ ret = drm_dp_mst_atomic_enable_dsc(state,
aconnector->port,
+ dm_conn_state->pbn,
false);
+ if (ret != 0)
+ return ret;
+
continue;
}
@@ -9523,10 +9526,9 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
#if defined(CONFIG_DRM_AMD_DC_DCN)
if (dc_resource_is_dsc_encoding_supported(dc)) {
- if (!pre_validate_dsc(state, &dm_state, vars)) {
- ret = -EINVAL;
+ ret = pre_validate_dsc(state, &dm_state, vars);
+ if (ret != 0)
goto fail;
- }
}
#endif
@@ -9621,9 +9623,9 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
}
#if defined(CONFIG_DRM_AMD_DC_DCN)
- if (!compute_mst_dsc_configs_for_state(state,
dm_state->context, vars)) {
+ ret = compute_mst_dsc_configs_for_state(state,
dm_state->context, vars);
+ if (ret) {
DRM_DEBUG_DRIVER("compute_mst_dsc_configs_for_state()
failed\n");
- ret = -EINVAL;
goto fail;
}
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 6ff96b4bdda5c..30bc2e5058b70 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -864,25 +864,25 @@ static bool try_disable_dsc(struct drm_atomic_state
*state,
return true;
}
-static bool compute_mst_dsc_configs_for_link(struct drm_atomic_state *state,
-struct dc_state *dc_state,
-struct dc_link *dc_link,
-struct dsc_mst_fairness_vars *vars,
-struct drm_dp_mst_topology_mgr
*mgr,
-int *link_vars_start_index)
+static int compute_mst_dsc_configs_for_link(struct drm_atomic_state *state,
+ struct dc_state *dc_state,
+ struct dc_link *dc_link,
+ struct dsc_mst_fairness_vars *vars,
+ struct drm_dp_mst_topology_mgr *mgr,
+ int *link_vars_start_index)
{
struct dc_stream_state *stream;
struct dsc_mst_fairness_params params[MAX_PIPES];
struct amdgpu_dm_connector *aconnector;
struct drm_dp_mst_topology_state *mst_state =
drm_atomic_get_mst_topology_state(state, mgr);
int count = 0;
- int i, k;
+ int i, k, ret;
bool debugfs_overwrite = false;
memset(params, 0, sizeof(params));
if (IS_ERR(mst_state))
- return false;
+ return PTR_ERR(mst_state);
mst_state->pbn_div =
[PATCH 0/2] MST deadlocking fixes
Some deadlock related fixes for amdgpu and DRM, spurred by: https://gitlab.freedesktop.org/drm/amd/-/issues/2171 Unfortunately these don't fully fix the problem yet, but I'm getting there! Lyude Paul (2): drm/amdgpu/mst: Stop ignoring error codes and deadlocking drm/display/dp_mst: Fix drm_dp_mst_add_affected_dsc_crtcs() return code .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 18 +-- .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 107 ++ .../display/amdgpu_dm/amdgpu_dm_mst_types.h | 12 +- drivers/gpu/drm/display/drm_dp_mst_topology.c | 2 +- 4 files changed, 74 insertions(+), 65 deletions(-) -- 2.37.3
[pull] amdgpu, amdkfd, radeon, drm drm-next-6.2
Hi Dave, Daniel, New stuff for 6.2. The following changes since commit 9abf2313adc1ca1b6180c508c25f22f9395cc780: Linux 6.1-rc1 (2022-10-16 15:36:24 -0700) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-next-6.2-2022-11-04 for you to fetch changes up to fcf00f8d29f2fc6bf00531a1447be28b99073cc3: drm/amdkfd: Remove skiping userptr buffer mapping when mmu notifier marks it as invalid (2022-11-04 16:05:54 -0400) amd-drm-next-6.2-2022-11-04: amdgpu: - Add TMZ support for GC 11.0.1 - More IP version check conversions - Mode2 reset fixes for sienna cichlid - SMU 13.x fixes - RAS enablement on MP 13.x - Replace kmap with kmap_local_page() - Misc Clang warning fixes - SR-IOV fixes for GC 11.x - PCI AER fix - DCN 3.2.x commit sequence rework - SDMA 4.x doorbell fix - Expose additional new GC 11.x firmware versions - Misc code cleanups - S0i3 fixes - More DC FPU cleanup - Add more DC kerneldoc - Misc spelling and grammer fixes - DCN 3.1.x fixes - Plane modifier fix - MCA RAS enablement - Secure display locking fix - RAS TA rework - RAS EEPROM fixes - Fail suspend if eviction fails - Drop AMD specific DSC workarounds in favor of drm EDID quirks - SR-IOV suspend/resume fixes - Enable DCN support for ARM - Enable secure display on DCN 2.1 amdkfd: - Cache size fixes for GC 10.3.x - kfd_dev struct cleanup - GC11.x CWSR trap handler fix - Userptr fixes - Warning fixes radeon: - Replace kmap with kmap_local_page() UAPI: - Expose additional new GC 11.x firmware versions via the existing INFO query drm: - Add some new EDID DSC quirks Alan Liu (3): drm/amd/display: Implement secure display on DCN21 drm/amd/display: Drop struct crc_region and reuse struct rect drm/amdgpu: Move the mutex_lock to protect the return status of securedisplay command buffer Alex Deucher (6): drm/amdgpu: convert vega20_ih.c to IP version checks drm/amdgpu: convert amdgpu_amdkfd_gpuvm.c to IP version checks drm/amdgpu: fix sdma doorbell init ordering on APUs drm/amdgpu/gfx9: set gfx.funcs in early init drm/amdgpu/gfx10: set gfx.funcs in early init drm/amdgpu/gfx11: set gfx.funcs in early init Alvin Lee (4): drm/amd/display: Don't return false if no stream drm/amd/display: Remove optimization for VRR updates drm/amd/display: Enable timing sync on DCN32 drm/amd/display: Don't enable ODM + MPO Anthony Koo (2): drm/amd/display: Document part of the DMUB cmd drm/amd/display: [FW Promotion] Release 0.0.141.0 Ao Zhong (3): drm/amd/display: move remaining FPU code to dml folder drm/amd/display: move remaining FPU code to dml folder drm/amd/display: add DCN support for ARM64 Aric Cyr (4): drm/amd/display: 3.2.208 drm/amd/display: Fix SDR visual confirm drm/amd/display: 3.2.209 drm/amd/display: 3.2.210 Arunpravin Paneer Selvam (1): drm/amdgpu: Fix for BO move issue Asher Song (1): drm/amdgpu: Revert "drm/amdgpu: getting fan speed pwm for vega10 properly" Bhawanpreet Lakha (1): drm/amd/display: Fix HDCP 1.X 1A-04 failing Candice Li (5): drm/amdgpu: Optimize RAS TA initialization and TA unload funcs drm/amdgpu: Optimize TA load/unload/invoke debugfs interfaces drm/amdgpu: Update ras eeprom support for smu v13_0_0 and v13_0_10 drm/amdgpu: Add EEPROM I2C address support for ip discovery drm/amdgpu: Enable GFX RAS feature for gfx v11_0_3 Charlene Liu (2): drm/amd/display: Update DML formula drm/amd/display: Fix null pointer issues found in emulation Chengming Gui (1): drm/amdgpu: fix pstate setting issue Danijel Slivka (1): drm/amdgpu: set vm_update_mode=0 as default for Sienna Cichlid in SRIOV case David Francis (1): drm/amd: Add IMU fw version to fw version queries Deming Wang (1): drm/amdkfd: use vma_lookup() instead of find_vma() Dillon Varone (4): drm/amd/display: Update latencies on DCN321 drm/amd/display: Set memclk levels to be at least 1 for dcn32 drm/amd/display: Reinit DPG when exiting dynamic ODM drm/amd/display: Check validation passed after applying pipe split changes Dmytro Laktyushkin (1): drm/amd/display: correctly populate dcn315 clock table Eric Bernstein (1): drm/amd/display: Include virtual signal to set k1 and k2 values Evan Quan (3): drm/amd/pm: fulfill SMU13.0.0 cstate control interface drm/amd/pm: fulfill SMU13.0.7 cstate control interface drm/amd/pm: disable cstate feature for gpu reset scenario Fabio M. De Francesco (2): drm/radeon: Replace kmap() with kmap_local_page() drm/amd/amdgpu: Replace kmap() with kmap_local_page() Fangzhi Zuo (3): drm/amd/display: Add UHBR135 and UHBR20 into debugfs drm/amd/display: Ignore Cable ID Featur
Re: Coverity: kfd_parse_subtype_cache(): Memory - corruptions
On 2022-11-04 15:41, coverity-bot wrote:
Hello!
This is an experimental semi-automated report about issues detected by
Coverity from a scan of next-20221104 as part of the linux-next scan project:
https://scan.coverity.com/projects/linux-next-weekly-scan
You're getting this email because you were associated with the identified
lines of code (noted below) that were touched by commits:
Fri Dec 8 23:08:59 2017 -0500
3a87177eb141 ("drm/amdkfd: Add topology support for dGPUs")
Coverity reported the following:
*** CID 1527133: Memory - corruptions (OVERRUN)
drivers/gpu/drm/amd/amdkfd/kfd_crat.c:1113 in kfd_parse_subtype_cache()
1107props->cache_size = cache->cache_size;
1108props->cacheline_size = cache->cache_line_size;
1109props->cachelines_per_tag =
cache->lines_per_tag;
1110props->cache_assoc = cache->associativity;
props->cache_latency = cache->cache_latency;
1112
vvv CID 1527133: Memory - corruptions (OVERRUN)
vvv Overrunning array "cache->sibling_map" of 32 bytes by passing it to a function
which accesses it at byte offset 63 using argument "64UL". [Note: The source code
implementation of the function has been overridden by a builtin model.]
1113memcpy(props->sibling_map, cache->sibling_map,
1114sizeof(props->sibling_map));
1115
1116/* set the sibling_map_size as 32 for CRAT from
ACPI */
1117props->sibling_map_size = CRAT_SIBLINGMAP_SIZE;
1118
If this is a false positive, please let us know so we can mark it as
such, or teach the Coverity rules to be smarter. If not, please make
sure fixes get into linux-next. :) For patches fixing this, please
include these lines (but double-check the "Fixes" first):
Reported-by: coverity-bot
Addresses-Coverity-ID: 1527133 ("Memory - corruptions")
Fixes: 3a87177eb141 ("drm/amdkfd: Add topology support for dGPUs")
I'm not sure why this suddenly appeared after 5 years, but the read
over-run looks legit:
I think this was introduced by a more recent patch that was in fact
meant to fix an array overrun on HW that is outgrowing the CRAT sibling
map size:
commit 0938fbeb6f53fc44bc9b19784dee28496e68ba0c
Author: Ma Jun
Date: Wed Nov 2 15:53:26 2022 +0800
drm/amdkfd: Fix the warning of array-index-out-of-bounds
For some GPUs with more CUs, the original sibling_map[32]
in struct crat_subtype_cache is not enough
to save the cache information when create the VCRAT table,
so skip filling the struct crat_subtype_cache info instead
fill struct kfd_cache_properties directly to fix this problem.
Signed-off-by: Ma Jun
Reviewed-by: Felix Kuehling
Signed-off-by: Alex Deucher
I added Ma Jun to the email.
Regards,
Felix
struct crat_subtype_cache {
...
uint8_t sibling_map[CRAT_SIBLINGMAP_SIZE];
#define CRAT_SIBLINGMAP_SIZE32
struct kfd_cache_properties {
...
uint8_t sibling_map[CACHE_SIBLINGMAP_SIZE];
#define CACHE_SIBLINGMAP_SIZE 64
Thanks for your attention!
Coverity: kfd_parse_subtype_cache(): Memory - corruptions
Hello!
This is an experimental semi-automated report about issues detected by
Coverity from a scan of next-20221104 as part of the linux-next scan project:
https://scan.coverity.com/projects/linux-next-weekly-scan
You're getting this email because you were associated with the identified
lines of code (noted below) that were touched by commits:
Fri Dec 8 23:08:59 2017 -0500
3a87177eb141 ("drm/amdkfd: Add topology support for dGPUs")
Coverity reported the following:
*** CID 1527133: Memory - corruptions (OVERRUN)
drivers/gpu/drm/amd/amdkfd/kfd_crat.c:1113 in kfd_parse_subtype_cache()
1107props->cache_size = cache->cache_size;
1108props->cacheline_size = cache->cache_line_size;
1109props->cachelines_per_tag =
cache->lines_per_tag;
1110props->cache_assoc = cache->associativity;
props->cache_latency = cache->cache_latency;
1112
vvv CID 1527133: Memory - corruptions (OVERRUN)
vvv Overrunning array "cache->sibling_map" of 32 bytes by passing it to a
function which accesses it at byte offset 63 using argument "64UL". [Note: The
source code implementation of the function has been overridden by a builtin
model.]
1113memcpy(props->sibling_map, cache->sibling_map,
1114sizeof(props->sibling_map));
1115
1116/* set the sibling_map_size as 32 for CRAT from
ACPI */
1117props->sibling_map_size = CRAT_SIBLINGMAP_SIZE;
1118
If this is a false positive, please let us know so we can mark it as
such, or teach the Coverity rules to be smarter. If not, please make
sure fixes get into linux-next. :) For patches fixing this, please
include these lines (but double-check the "Fixes" first):
Reported-by: coverity-bot
Addresses-Coverity-ID: 1527133 ("Memory - corruptions")
Fixes: 3a87177eb141 ("drm/amdkfd: Add topology support for dGPUs")
I'm not sure why this suddenly appeared after 5 years, but the read
over-run looks legit:
struct crat_subtype_cache {
...
uint8_t sibling_map[CRAT_SIBLINGMAP_SIZE];
#define CRAT_SIBLINGMAP_SIZE32
struct kfd_cache_properties {
...
uint8_t sibling_map[CACHE_SIBLINGMAP_SIZE];
#define CACHE_SIBLINGMAP_SIZE 64
Thanks for your attention!
--
Coverity-bot
[linux-next:master] BUILD SUCCESS WITH WARNING 0cdb3579f1ee4c1e55acf8dfb0697b660067b1f8
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 0cdb3579f1ee4c1e55acf8dfb0697b660067b1f8 Add linux-next specific files for 20221104 Warning reports: https://lore.kernel.org/oe-kbuild-all/202211041320.coq8eelj-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211041654.zcupre9o-...@intel.com Warning: (recently discovered and may have been fixed) drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:4878: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:5044:24: warning: implicit conversion from 'enum ' to 'enum dc_status' [-Wenum-conversion] drivers/gpu/drm/msm/hdmi/hdmi.c:244:1: warning: 'static' is not at beginning of declaration [-Wold-style-declaration] drivers/gpu/drm/msm/hdmi/hdmi.c:251:1: warning: 'static' is not at beginning of declaration [-Wold-style-declaration] Warning ids grouped by kconfigs: gcc_recent_errors |-- alpha-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc.c:warning:This-comment-starts-with-but-isn-t-a-kernel-doc-comment.-Refer-Documentation-doc-guide-kernel-doc.rst | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link_dp.c:warning:implicit-conversion-from-enum-anonymous-to-enum-dc_status | `-- drivers-gpu-drm-msm-hdmi-hdmi.c:warning:static-is-not-at-beginning-of-declaration |-- arc-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc.c:warning:This-comment-starts-with-but-isn-t-a-kernel-doc-comment.-Refer-Documentation-doc-guide-kernel-doc.rst | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link_dp.c:warning:implicit-conversion-from-enum-anonymous-to-enum-dc_status | `-- drivers-gpu-drm-msm-hdmi-hdmi.c:warning:static-is-not-at-beginning-of-declaration |-- arc-randconfig-r036-20221104 | `-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link_dp.c:warning:implicit-conversion-from-enum-anonymous-to-enum-dc_status |-- arc-randconfig-r043-20221104 | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc.c:warning:This-comment-starts-with-but-isn-t-a-kernel-doc-comment.-Refer-Documentation-doc-guide-kernel-doc.rst | `-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link_dp.c:warning:implicit-conversion-from-enum-anonymous-to-enum-dc_status |-- arm-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc.c:warning:This-comment-starts-with-but-isn-t-a-kernel-doc-comment.-Refer-Documentation-doc-guide-kernel-doc.rst | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link_dp.c:warning:implicit-conversion-from-enum-anonymous-to-enum-dc_status | `-- drivers-gpu-drm-msm-hdmi-hdmi.c:warning:static-is-not-at-beginning-of-declaration |-- arm-defconfig | `-- drivers-gpu-drm-msm-hdmi-hdmi.c:warning:static-is-not-at-beginning-of-declaration |-- arm64-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc.c:warning:This-comment-starts-with-but-isn-t-a-kernel-doc-comment.-Refer-Documentation-doc-guide-kernel-doc.rst | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link_dp.c:warning:implicit-conversion-from-enum-anonymous-to-enum-dc_status | `-- drivers-gpu-drm-msm-hdmi-hdmi.c:warning:static-is-not-at-beginning-of-declaration |-- i386-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc.c:warning:This-comment-starts-with-but-isn-t-a-kernel-doc-comment.-Refer-Documentation-doc-guide-kernel-doc.rst | `-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link_dp.c:warning:implicit-conversion-from-enum-anonymous-to-enum-dc_status |-- ia64-allmodconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc.c:warning:This-comment-starts-with-but-isn-t-a-kernel-doc-comment.-Refer-Documentation-doc-guide-kernel-doc.rst | `-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link_dp.c:warning:implicit-conversion-from-enum-anonymous-to-enum-dc_status |-- ia64-buildonly-randconfig-r006-20221104 | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc.c:warning:This-comment-starts-with-but-isn-t-a-kernel-doc-comment.-Refer-Documentation-doc-guide-kernel-doc.rst | `-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link_dp.c:warning:implicit-conversion-from-enum-anonymous-to-enum-dc_status |-- m68k-allmodconfig | `-- drivers-gpu-drm-msm-hdmi-hdmi.c:warning:static-is-not-at-beginning-of-declaration |-- m68k-allyesconfig | `-- drivers-gpu-drm-msm-hdmi-hdmi.c:warning:static-is-not-at-beginning-of-declaration |-- mips-allyesconfig | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc.c:warning:This-comment-starts-with-but-isn-t-a-kernel-doc-comment.-Refer-Documentation-doc-guide-kernel-doc.rst | |-- drivers-gpu-drm-amd-amdgpu-..-display-dc-core-dc_link_dp.c:warning:implicit-conversion-from-enum-anonymous-to-enum-dc_status | `-- drivers-gpu-drm-msm-hdmi-hdmi.c:warning:static-is-not-at-beginning-o
Re: [PATCH 1/2] drm/radeon: Using unsigned long instead of unsigned to fix possible overflow
On Fri, Nov 4, 2022 at 6:05 AM Hanjun Guo wrote:
>
> VBIOSImageOffset in struct UEFI_ACPI_VFCT is ULONG (unsigned long),
> but it will be assigned to "unsigned offset", so use unsigned long
> instead of unsigned for the offset, to avoid possible overflow.
ULONG in atombios is 32 bits so an int should be fine.
Alex
>
> Signed-off-by: Hanjun Guo
> ---
> drivers/gpu/drm/radeon/radeon_bios.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_bios.c
> b/drivers/gpu/drm/radeon/radeon_bios.c
> index 3312165..520d1d6 100644
> --- a/drivers/gpu/drm/radeon/radeon_bios.c
> +++ b/drivers/gpu/drm/radeon/radeon_bios.c
> @@ -611,7 +611,7 @@ static bool radeon_acpi_vfct_bios(struct radeon_device
> *rdev)
> struct acpi_table_header *hdr;
> acpi_size tbl_size;
> UEFI_ACPI_VFCT *vfct;
> - unsigned offset;
> + unsigned long offset;
>
> if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
> return false;
> --
> 1.7.12.4
>
Re: [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)
Hi Matthew, Rafael, On 10/27/22 14:09, Rafael J. Wysocki wrote: > On Thu, Oct 27, 2022 at 12:37 PM Hans de Goede wrote: >> >> Hi, >> >> On 10/27/22 11:52, Matthew Garrett wrote: >>> On Thu, Oct 27, 2022 at 11:39:38AM +0200, Hans de Goede wrote: >>> The *only* behavior which actually is new in 6.1 is the native GPU drivers now doing the equivalent of: if (acpi_video_get_backlight_type() != acpi_backlight_native) return; In their backlight register paths (i), which is causing the native backlight to disappear on your custom laptop setup and on Chromebooks (with the Chromebooks case being already solved I hope.). >>> >>> It's causing the backlight control to vanish on any machine that isn't >>> ((acpi_video || vendor interface) || !acpi). Most machines that fall >>> into that are either weird or Chromebooks or old, but there are machines >>> that fall into that. >> >> I acknowledge that their are machines that fall into this category, >> but I expect / hope there to be so few of them that we can just DMI >> quirk our way out if this. >> >> I believe the old group to be small because: >> >> 1. Generally speaking the "native" control method is usually not >> present on the really old (pre ACPI video spec) mobile GPUs. >> >> 2. On most old laptops I would still expect there to be a vendor >> interface too, and if both get registered standard desktop environments >> will prefer the vendor one, so then we need a native DMI quirk to >> disable the vendor interface anyways and we already have a bunch of >> those, so some laptops in this group are already covered by DMI quirks. >> >> And a fix for the Chromebook case is already in Linus' tree, which >> just leaves the weird case, of which there will hopefully be only >> a few. >> >> I do share your worry that this might break some machines, but >> the only way to really find out is to get this code out there >> I'm afraid. >> >> I have just written a blog post asking for people to check if >> their laptop might be affected; and to report various details >> to me of their laptop is affected: >> >> https://hansdegoede.dreamwidth.org/26548.html >> >> Lets wait and see how this goes. If I get (too) many reports then >> I will send a revert of the addition of the: >> >> if (acpi_video_get_backlight_type() != acpi_backlight_native) >> return; >> >> check to the i915 / radeon / amd / nouveau drivers. >> >> (And if I only get a couple of reports I will probably just submit >> DMI quirks for the affected models). > > Sounds reasonable to me, FWIW. I have received quite a few test reports as a result of my blogpost (and of the blogpost's mention in an arstechnica article). Long story short, Matthew, you are right. Quite a few laptop models will end up with an empty /sys/class/backlight because of the native backlight class devices no longer registering when acpi_video_backlight_use_native() returns false. I will submit a patch-set later today to fix this (by making cpi_video_backlight_use_native() always return true for now). More detailed summary/analysis of the received test reports: -30 unaffected models -The following laptop models: Acer Aspire 1640 Apple MacBook 2.1 Apple MacBook 4.1 Apple MacBook Pro 7.1 (uses nv_backligh instead of intel_backlight!) HP Compaq nc6120 IBM ThinkPad X40 System76 Starling Star1 All only have a native intel_backlight interface and the heuristics from acpi_video_get_backlight_type() return acpi_backlight_vendor there causing the changes in 6.1 to not register native backlights when acpi_video_backlight_use_native() returns false resulting in an empty /sys/class/backlight, breaking users ability to control their laptop panel's brightness. I will submit a patch to always make acpi_video_backlight_use_native() return true for now to work around this for 6.1. I do plan to try to re-introduce that change again later. First I need to change the heuristics to still native on more models so that on models where the native backlight is the only (working) entry they will return native. -The Dell N1410 has acpi_video support and acpi_osi_is_win8() returns false so acpi_video_get_backlight_type() returns acpi_video, but acpi_video fails to register a backlight device due to a_BCM eval error. The intel_backlight interface works fine, but this model is going to need a DMI-use-native-quirk to avoid intel_backlight disappearing when acpi_video_backlight_use_native() is changed back. -The following laptop models actually use a vendor backlight control method, while also having a native backlight entry under /sys/class/backlight: Asus EeePC 901 -> native backlight confirmed to also work Dell Latitude D610 -> native backlight confirmed to work better then vendor Sony Vaio PCG-FRV3 -> native backlight not tested Note these will keep working the same as before in 6.1, independent of the revert. I've tracked these seperatel
Re: [PATCH] drm/amdgpu: Inherit coherence flags on dmabuf import
Am 04.11.22 um 16:40 schrieb Felix Kuehling:
When importing dmabufs from other amdgpu devices, inherit the coherence
flags from the exported BO. This ensures correct MTYPEs in the GPU PTEs
for multi-GPU mappings of shared BOs.
Fixes: 763fbebb20e1 ("drm/amdgpu: Set MTYPE in PTE based on BO flags")
Tested-by: Gang Ba
Signed-off-by: Felix Kuehling
Acked-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 7bd8e33b14be..271e30e34d93 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -328,7 +328,9 @@ amdgpu_dma_buf_create_obj(struct drm_device *dev, struct
dma_buf *dma_buf)
if (dma_buf->ops == &amdgpu_dmabuf_ops) {
struct amdgpu_bo *other = gem_to_amdgpu_bo(dma_buf->priv);
- flags |= other->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC;
+ flags |= other->flags & (AMDGPU_GEM_CREATE_CPU_GTT_USWC |
+AMDGPU_GEM_CREATE_COHERENT |
+AMDGPU_GEM_CREATE_UNCACHED);
}
ret = amdgpu_gem_object_create(adev, dma_buf->size, PAGE_SIZE,
[PATCH] drm/amdgpu: Inherit coherence flags on dmabuf import
When importing dmabufs from other amdgpu devices, inherit the coherence
flags from the exported BO. This ensures correct MTYPEs in the GPU PTEs
for multi-GPU mappings of shared BOs.
Fixes: 763fbebb20e1 ("drm/amdgpu: Set MTYPE in PTE based on BO flags")
Tested-by: Gang Ba
Signed-off-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 7bd8e33b14be..271e30e34d93 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -328,7 +328,9 @@ amdgpu_dma_buf_create_obj(struct drm_device *dev, struct
dma_buf *dma_buf)
if (dma_buf->ops == &amdgpu_dmabuf_ops) {
struct amdgpu_bo *other = gem_to_amdgpu_bo(dma_buf->priv);
- flags |= other->flags & AMDGPU_GEM_CREATE_CPU_GTT_USWC;
+ flags |= other->flags & (AMDGPU_GEM_CREATE_CPU_GTT_USWC |
+AMDGPU_GEM_CREATE_COHERENT |
+AMDGPU_GEM_CREATE_UNCACHED);
}
ret = amdgpu_gem_object_create(adev, dma_buf->size, PAGE_SIZE,
--
2.32.0
Re: [PATCH] drm/amdgpu: wait till Gfxoff allow signal completes during suspend
On Fri, Nov 4, 2022 at 12:17 AM Harsh Jain wrote:
>
> change guarantees that gfxoff is allowed before moving further in
> s2idle sequence to add more reliablity about gfxoff in amdgpu IP's
> suspend flow
>
> Signed-off-by: Harsh Jain
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 222d3d7ea076..5b2afe348c23 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -581,11 +581,11 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device *adev,
> bool enable)
>
> if (adev->gfx.gfx_off_req_count == 0 &&
> !adev->gfx.gfx_off_state) {
> - /* If going to s2idle, no need to wait */
> - if (adev->in_s0ix)
> - delay = GFX_OFF_NO_DELAY;
I think we can leave this hunk. The 0 delay is still fine even if if flush.
> schedule_delayed_work(&adev->gfx.gfx_off_delay_work,
> delay);
> + /* If going to s2idle, no need to wait */
> + if (adev->in_s0ix)
> +
> flush_delayed_work(&adev->gfx.gfx_off_delay_work);
> }
> } else {
> if (adev->gfx.gfx_off_req_count == 0) {
> --
> 2.25.1
>
Re: [PATCH] Revert "drm/amdgpu: Revert "drm/amdgpu: getting fan speed pwm for vega10 properly""
On Fri, Nov 4, 2022 at 5:30 AM Asher Song wrote:
>
> This reverts commit 97370f1826eb7ee6880e09ee1eaafe28232cabc6.
>
> The origin patch "drm/amdgpu: getting fan speed pwm for vega10 properly"
> works fine. Test failure is caused by test case self.
Instead of reverting the revert, can you just reapply the original
patch and amend the commit message with this statement?
Either way:
Acked-by: Alex Deucher
Alex
>
> Signed-off-by: Asher Song
> ---
> .../amd/pm/powerplay/hwmgr/vega10_thermal.c | 25 +--
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
> b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
> index dad3e3741a4e..190af79f3236 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
> @@ -67,22 +67,21 @@ int vega10_fan_ctrl_get_fan_speed_info(struct pp_hwmgr
> *hwmgr,
> int vega10_fan_ctrl_get_fan_speed_pwm(struct pp_hwmgr *hwmgr,
> uint32_t *speed)
> {
> - uint32_t current_rpm;
> - uint32_t percent = 0;
> -
> - if (hwmgr->thermal_controller.fanInfo.bNoFan)
> - return 0;
> + struct amdgpu_device *adev = hwmgr->adev;
> + uint32_t duty100, duty;
> + uint64_t tmp64;
>
> - if (vega10_get_current_rpm(hwmgr, ¤t_rpm))
> - return -1;
> + duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL1),
> + CG_FDO_CTRL1, FMAX_DUTY100);
> + duty = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_THERMAL_STATUS),
> + CG_THERMAL_STATUS, FDO_PWM_DUTY);
>
> - if (hwmgr->thermal_controller.
> - advanceFanControlParameters.usMaxFanRPM != 0)
> - percent = current_rpm * 255 /
> - hwmgr->thermal_controller.
> - advanceFanControlParameters.usMaxFanRPM;
> + if (!duty100)
> + return -EINVAL;
>
> - *speed = MIN(percent, 255);
> + tmp64 = (uint64_t)duty * 255;
> + do_div(tmp64, duty100);
> + *speed = MIN((uint32_t)tmp64, 255);
>
> return 0;
> }
> --
> 2.25.1
>
[PATCH v1] drm/amd/display: Have risk for memory exhaustion
In dcn*_clock_source_create when dcn*_clk_src_construct fails allocated clk_src needs release. A local attack could use this to cause memory exhaustion. Signed-off-by: LongJun Tang --- drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c | 1 + drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c | 1 + drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c | 1 + drivers/gpu/drm/amd/display/dc/dcn303/dcn303_resource.c | 1 + drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c | 1 + drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c | 1 + drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c | 1 + drivers/gpu/drm/amd/display/dc/dcn321/dcn321_resource.c | 1 + 8 files changed, 8 insertions(+) diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c index 020f512e9690..9b7e786bd4a2 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c @@ -1323,6 +1323,7 @@ static struct clock_source *dcn30_clock_source_create( return &clk_src->base; } + kfree(clk_src); BREAK_TO_DEBUGGER(); return NULL; } diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c index f04595b750ab..7c1225046544 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c @@ -1288,6 +1288,7 @@ static struct clock_source *dcn301_clock_source_create( return &clk_src->base; } + kfree(clk_src); BREAK_TO_DEBUGGER(); return NULL; } diff --git a/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c b/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c index b925b6ddde5a..73ae1146dad5 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c @@ -458,6 +458,7 @@ static struct clock_source *dcn302_clock_source_create(struct dc_context *ctx, s return &clk_src->base; } + kfree(clk_src); BREAK_TO_DEBUGGER(); return NULL; } diff --git a/drivers/gpu/drm/amd/display/dc/dcn303/dcn303_resource.c b/drivers/gpu/drm/amd/display/dc/dcn303/dcn303_resource.c index 527d5c902878..0ea97eeec5a6 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn303/dcn303_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn303/dcn303_resource.c @@ -425,6 +425,7 @@ static struct clock_source *dcn303_clock_source_create(struct dc_context *ctx, s return &clk_src->base; } + kfree(clk_src); BREAK_TO_DEBUGGER(); return NULL; } diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c index fddc21a5a04c..b02aa8874efb 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c @@ -1625,6 +1625,7 @@ static struct clock_source *dcn31_clock_source_create( return &clk_src->base; } + kfree(clk_src); BREAK_TO_DEBUGGER(); return NULL; } diff --git a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c index 58746c437554..b2ff29e5f93c 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c @@ -1623,6 +1623,7 @@ static struct clock_source *dcn31_clock_source_create( return &clk_src->base; } + kfree(clk_src); BREAK_TO_DEBUGGER(); return NULL; } diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c index a88dd7b3d1c1..71730bb0 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c @@ -829,6 +829,7 @@ static struct clock_source *dcn32_clock_source_create( return &clk_src->base; } + kfree(clk_src); BREAK_TO_DEBUGGER(); return NULL; } diff --git a/drivers/gpu/drm/amd/display/dc/dcn321/dcn321_resource.c b/drivers/gpu/drm/amd/display/dc/dcn321/dcn321_resource.c index 61087f2385a9..d3980fc243c9 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn321/dcn321_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn321/dcn321_resource.c @@ -828,6 +828,7 @@ static struct clock_source *dcn321_clock_source_create( return &clk_src->base; } + kfree(clk_src); BREAK_TO_DEBUGGER(); return NULL; } -- 2.17.1 No virus found Checked by Hillstone Network AntiVirus
Re: [PATCH] drm/amd/amdgpu: Enable gfx pipe1 and fix related issues
Am 04.11.22 um 12:19 schrieb Emily Deng:
Starting from SIENNA CICHLID asic supports two gfx pipes, enabling
two graphics queues for performance concern.
Signed-off-by: Emily Deng
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +-
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 42 -
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 331aa191910c..0072f36b44d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -33,7 +33,7 @@
container_of((e), struct amdgpu_ctx_entity, entity)
const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = {
- [AMDGPU_HW_IP_GFX] = 1,
+ [AMDGPU_HW_IP_GFX] = 2,
Again, please completely drop this. That change absolutely makes no
sense at all.
Regards,
Christian.
[AMDGPU_HW_IP_COMPUTE] = 4,
[AMDGPU_HW_IP_DMA] = 2,
[AMDGPU_HW_IP_UVD] = 1,
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 49d34c7bbf20..bbf18060611e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -4606,7 +4606,7 @@ static int gfx_v10_0_sw_init(void *handle)
case IP_VERSION(10, 3, 3):
case IP_VERSION(10, 3, 7):
adev->gfx.me.num_me = 1;
- adev->gfx.me.num_pipe_per_me = 1;
+ adev->gfx.me.num_pipe_per_me = 2;
adev->gfx.me.num_queue_per_pipe = 1;
adev->gfx.mec.num_mec = 2;
adev->gfx.mec.num_pipe_per_mec = 4;
@@ -6008,6 +6008,24 @@ static int gfx_v10_0_cp_gfx_load_microcode(struct
amdgpu_device *adev)
return 0;
}
+static int gfx_v10_0_wait_for_idle(void *handle)
+{
+ unsigned i;
+ u32 tmp;
+ struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+ for (i = 0; i < adev->usec_timeout; i++) {
+ /* read MC_STATUS */
+ tmp = RREG32_SOC15(GC, 0, mmGRBM_STATUS) &
+ GRBM_STATUS__GUI_ACTIVE_MASK;
+
+ if (!REG_GET_FIELD(tmp, GRBM_STATUS, GUI_ACTIVE))
+ return 0;
+ udelay(1);
+ }
+ return -ETIMEDOUT;
+}
+
static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)
{
struct amdgpu_ring *ring;
@@ -6069,7 +6087,7 @@ static int gfx_v10_0_cp_gfx_start(struct amdgpu_device
*adev)
amdgpu_ring_write(ring, 0x8000);
amdgpu_ring_commit(ring);
-
+ gfx_v10_0_wait_for_idle(adev);
/* submit cs packet to copy state 0 to next available state */
if (adev->gfx.num_gfx_rings > 1) {
/* maximum supported gfx ring is 2 */
@@ -7404,24 +7422,6 @@ static bool gfx_v10_0_is_idle(void *handle)
return true;
}
-static int gfx_v10_0_wait_for_idle(void *handle)
-{
- unsigned i;
- u32 tmp;
- struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-
- for (i = 0; i < adev->usec_timeout; i++) {
- /* read MC_STATUS */
- tmp = RREG32_SOC15(GC, 0, mmGRBM_STATUS) &
- GRBM_STATUS__GUI_ACTIVE_MASK;
-
- if (!REG_GET_FIELD(tmp, GRBM_STATUS, GUI_ACTIVE))
- return 0;
- udelay(1);
- }
- return -ETIMEDOUT;
-}
-
static int gfx_v10_0_soft_reset(void *handle)
{
u32 grbm_soft_reset = 0;
@@ -8466,7 +8466,7 @@ static void gfx_v10_0_ring_emit_hdp_flush(struct
amdgpu_ring *ring)
}
reg_mem_engine = 0;
} else {
- ref_and_mask = nbio_hf_reg->ref_and_mask_cp0;
+ ref_and_mask = nbio_hf_reg->ref_and_mask_cp0 << ring->pipe;
reg_mem_engine = 1; /* pfp */
}
[PATCH] drm/amd/amdgpu: Enable gfx pipe1 and fix related issues
Starting from SIENNA CICHLID asic supports two gfx pipes, enabling
two graphics queues for performance concern.
Signed-off-by: Emily Deng
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +-
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 42 -
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 331aa191910c..0072f36b44d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -33,7 +33,7 @@
container_of((e), struct amdgpu_ctx_entity, entity)
const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = {
- [AMDGPU_HW_IP_GFX] = 1,
+ [AMDGPU_HW_IP_GFX] = 2,
[AMDGPU_HW_IP_COMPUTE] = 4,
[AMDGPU_HW_IP_DMA] = 2,
[AMDGPU_HW_IP_UVD] = 1,
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 49d34c7bbf20..bbf18060611e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -4606,7 +4606,7 @@ static int gfx_v10_0_sw_init(void *handle)
case IP_VERSION(10, 3, 3):
case IP_VERSION(10, 3, 7):
adev->gfx.me.num_me = 1;
- adev->gfx.me.num_pipe_per_me = 1;
+ adev->gfx.me.num_pipe_per_me = 2;
adev->gfx.me.num_queue_per_pipe = 1;
adev->gfx.mec.num_mec = 2;
adev->gfx.mec.num_pipe_per_mec = 4;
@@ -6008,6 +6008,24 @@ static int gfx_v10_0_cp_gfx_load_microcode(struct
amdgpu_device *adev)
return 0;
}
+static int gfx_v10_0_wait_for_idle(void *handle)
+{
+ unsigned i;
+ u32 tmp;
+ struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+ for (i = 0; i < adev->usec_timeout; i++) {
+ /* read MC_STATUS */
+ tmp = RREG32_SOC15(GC, 0, mmGRBM_STATUS) &
+ GRBM_STATUS__GUI_ACTIVE_MASK;
+
+ if (!REG_GET_FIELD(tmp, GRBM_STATUS, GUI_ACTIVE))
+ return 0;
+ udelay(1);
+ }
+ return -ETIMEDOUT;
+}
+
static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)
{
struct amdgpu_ring *ring;
@@ -6069,7 +6087,7 @@ static int gfx_v10_0_cp_gfx_start(struct amdgpu_device
*adev)
amdgpu_ring_write(ring, 0x8000);
amdgpu_ring_commit(ring);
-
+ gfx_v10_0_wait_for_idle(adev);
/* submit cs packet to copy state 0 to next available state */
if (adev->gfx.num_gfx_rings > 1) {
/* maximum supported gfx ring is 2 */
@@ -7404,24 +7422,6 @@ static bool gfx_v10_0_is_idle(void *handle)
return true;
}
-static int gfx_v10_0_wait_for_idle(void *handle)
-{
- unsigned i;
- u32 tmp;
- struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-
- for (i = 0; i < adev->usec_timeout; i++) {
- /* read MC_STATUS */
- tmp = RREG32_SOC15(GC, 0, mmGRBM_STATUS) &
- GRBM_STATUS__GUI_ACTIVE_MASK;
-
- if (!REG_GET_FIELD(tmp, GRBM_STATUS, GUI_ACTIVE))
- return 0;
- udelay(1);
- }
- return -ETIMEDOUT;
-}
-
static int gfx_v10_0_soft_reset(void *handle)
{
u32 grbm_soft_reset = 0;
@@ -8466,7 +8466,7 @@ static void gfx_v10_0_ring_emit_hdp_flush(struct
amdgpu_ring *ring)
}
reg_mem_engine = 0;
} else {
- ref_and_mask = nbio_hf_reg->ref_and_mask_cp0;
+ ref_and_mask = nbio_hf_reg->ref_and_mask_cp0 << ring->pipe;
reg_mem_engine = 1; /* pfp */
}
--
2.36.1
[PATCH] drm/amdgpu: add vram reservation logic based on vram_usagebyfirmware_v2_2
Move TMR region from top of FB to 2MB for FFBM, so we need to reserve TMR region
firstly to make sure TMR can be allocated at 2MB
Signed-off-by: Tong Liu01
---
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 106 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 52 +
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 6 +
drivers/gpu/drm/amd/include/atomfirmware.h| 56 -
4 files changed, 192 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index b81b77a9efa6..65cf23818f4e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -101,39 +101,99 @@ void amdgpu_atomfirmware_scratch_regs_init(struct
amdgpu_device *adev)
}
}
+static int amdgpu_atomfirmware_allocate_fb_v2_1(struct amdgpu_device *adev,
+ struct vram_usagebyfirmware_v2_1 *firmware_usage_v2_1,
+ int *usage_bytes)
+{
+ uint32_t start_addr, size;
+
+ DRM_DEBUG("atom firmware v2_1 requested %08x %dkb fw %dkb drv\n",
+ le32_to_cpu(firmware_usage_v2_1->start_address_in_kb),
+ le16_to_cpu(firmware_usage_v2_1->used_by_firmware_in_kb),
+ le16_to_cpu(firmware_usage_v2_1->used_by_driver_in_kb));
+
+ start_addr = le32_to_cpu(firmware_usage_v2_1->start_address_in_kb);
+ size = le16_to_cpu(firmware_usage_v2_1->used_by_firmware_in_kb);
+
+ if ((uint32_t)(start_addr & ATOM_VRAM_OPERATION_FLAGS_MASK) ==
+ (uint32_t)(ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATION <<
+ ATOM_VRAM_OPERATION_FLAGS_SHIFT)) {
+ /* Firmware request VRAM reservation for SR-IOV */
+ adev->mman.fw_vram_usage_start_offset = (start_addr &
+ (~ATOM_VRAM_OPERATION_FLAGS_MASK)) << 10;
+ adev->mman.fw_vram_usage_size = size << 10;
+ /* Use the default scratch size */
+ usage_bytes = 0;
+ } else {
+ usage_bytes =
+ le16_to_cpu(firmware_usage_v2_1->used_by_driver_in_kb)
<< 10;
+ }
+ return 0;
+}
+
+static int amdgpu_atomfirmware_allocate_fb_v2_2(struct amdgpu_device *adev,
+ struct vram_usagebyfirmware_v2_2 *firmware_usage_v2_2,
+ int *usage_bytes)
+{
+ uint32_t fw_start_addr, fw_size, drv_start_addr, drv_size;
+
+ DRM_DEBUG("atom requested fw start at %08x %dkb and drv start at %08x
%dkb\n",
+ le32_to_cpu(firmware_usage_v2_2->fw_region_start_address_in_kb),
+ le16_to_cpu(firmware_usage_v2_2->used_by_firmware_in_kb),
+
le32_to_cpu(firmware_usage_v2_2->driver_region0_start_address_in_kb),
+ le32_to_cpu(firmware_usage_v2_2->used_by_driver_region0_in_kb));
+
+ fw_start_addr =
le32_to_cpu(firmware_usage_v2_2->fw_region_start_address_in_kb);
+ fw_size = le16_to_cpu(firmware_usage_v2_2->used_by_firmware_in_kb);
+
+ drv_start_addr =
le32_to_cpu(firmware_usage_v2_2->driver_region0_start_address_in_kb);
+ drv_size =
le32_to_cpu(firmware_usage_v2_2->used_by_driver_region0_in_kb);
+
+ if ((uint32_t)(fw_start_addr & (ATOM_VRAM_BLOCK_NEEDS_NO_RESERVATION <<
30)) == 0) {
+ /* Firmware request VRAM reservation for SR-IOV */
+ adev->mman.fw_vram_usage_start_offset = (fw_start_addr &
+ (~ATOM_VRAM_OPERATION_FLAGS_MASK)) << 10;
+ adev->mman.fw_vram_usage_size = fw_size << 10;
+ }
+
+ if ((uint32_t)(drv_start_addr & (ATOM_VRAM_BLOCK_NEEDS_NO_RESERVATION
<< 30)) == 0) {
+ /* driver request VRAM reservation for SR-IOV */
+ adev->mman.drv_vram_usage_start_offset = (drv_start_addr &
+ (~ATOM_VRAM_OPERATION_FLAGS_MASK)) << 10;
+ adev->mman.drv_vram_usage_size = drv_size << 10;
+ }
+
+ usage_bytes = 0;
+ return 0;
+}
+
int amdgpu_atomfirmware_allocate_fb_scratch(struct amdgpu_device *adev)
{
struct atom_context *ctx = adev->mode_info.atom_context;
int index =
get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
vram_usagebyfirmware);
- struct vram_usagebyfirmware_v2_1 *firmware_usage;
- uint32_t start_addr, size;
+ struct vram_usagebyfirmware_v2_1 *firmware_usage_v2_1;
+ struct vram_usagebyfirmware_v2_2 *firmware_usage_v2_2;
uint16_t data_offset;
+ uint8_t frev, crev;
int usage_bytes = 0;
- if (amdgpu_atom_parse_data_header(ctx, index, NULL, NULL, NULL,
&data_offset)) {
- firmware_usage = (struct vram_usagebyfirmware_v2_1 *)(ctx->bios
+ data_offset);
- DRM_DEBUG("atom firmware requested %08x %dkb fw %dkb drv\n",
- le32_to_cpu(firmware_usage->start_address_in_kb),
-
Re: [PATCH v3 23/23] drm/fb-helper: Clarify use of last_close and output_poll_changed
On 11/3/22 16:14, Thomas Zimmermann wrote: > Clarify documentation in the use of struct drm_driver.last_close and > struct drm_mode_config_funcs.output_poll_changed. Those callbacks should > not be said for fbdev implementations on top of struct drm_client_funcs. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH v3 20/23] drm/fb-helper: Set flag in struct drm_fb_helper for leaking physical addresses
On 11/3/22 16:14, Thomas Zimmermann wrote: > Uncouple the parameter drm_leak_fbdev_smem from the implementation by > setting a flag in struct drm_fb_helper. This will help to move the > generic fbdev emulation into its own source file, while keeping the > parameter in drm_fb_helper.c. No functional changes. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Javier Martinez Canillas -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
[PATCH 1/2] drm/radeon: Using unsigned long instead of unsigned to fix possible overflow
VBIOSImageOffset in struct UEFI_ACPI_VFCT is ULONG (unsigned long),
but it will be assigned to "unsigned offset", so use unsigned long
instead of unsigned for the offset, to avoid possible overflow.
Signed-off-by: Hanjun Guo
---
drivers/gpu/drm/radeon/radeon_bios.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/radeon/radeon_bios.c
b/drivers/gpu/drm/radeon/radeon_bios.c
index 3312165..520d1d6 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -611,7 +611,7 @@ static bool radeon_acpi_vfct_bios(struct radeon_device
*rdev)
struct acpi_table_header *hdr;
acpi_size tbl_size;
UEFI_ACPI_VFCT *vfct;
- unsigned offset;
+ unsigned long offset;
if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
return false;
--
1.7.12.4
[PATCH 2/2] drm/radeon: Add the missed acpi_put_table() to fix memory leak
When the radeon driver reads the bios information from ACPI
table in radeon_acpi_vfct_bios(), it misses to call acpi_put_table()
to release the ACPI memory after the init, so add acpi_put_table()
properly to fix the memory leak.
Fixes: 268ba0a99f89 ("drm/radeon: implement ACPI VFCT vbios fetch (v3)")
Signed-off-by: Hanjun Guo
---
drivers/gpu/drm/radeon/radeon_bios.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_bios.c
b/drivers/gpu/drm/radeon/radeon_bios.c
index 520d1d6..16730c1 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -612,13 +612,14 @@ static bool radeon_acpi_vfct_bios(struct radeon_device
*rdev)
acpi_size tbl_size;
UEFI_ACPI_VFCT *vfct;
unsigned long offset;
+ bool r = false;
if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
return false;
tbl_size = hdr->length;
if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
DRM_ERROR("ACPI VFCT table present but broken (too short
#1)\n");
- return false;
+ goto out;
}
vfct = (UEFI_ACPI_VFCT *)hdr;
@@ -631,13 +632,13 @@ static bool radeon_acpi_vfct_bios(struct radeon_device
*rdev)
offset += sizeof(VFCT_IMAGE_HEADER);
if (offset > tbl_size) {
DRM_ERROR("ACPI VFCT image header truncated\n");
- return false;
+ goto out;
}
offset += vhdr->ImageLength;
if (offset > tbl_size) {
DRM_ERROR("ACPI VFCT image truncated\n");
- return false;
+ goto out;
}
if (vhdr->ImageLength &&
@@ -649,15 +650,18 @@ static bool radeon_acpi_vfct_bios(struct radeon_device
*rdev)
rdev->bios = kmemdup(&vbios->VbiosContent,
vhdr->ImageLength,
GFP_KERNEL);
+ if (rdev->bios)
+ r = true;
- if (!rdev->bios)
- return false;
- return true;
+ goto out;
}
}
DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
- return false;
+
+out:
+ acpi_put_table(hdr);
+ return r;
}
#else
static inline bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
--
1.7.12.4
RE: [PATCH] drm/amdgpu: wait till Gfxoff allow signal completes during suspend
Please find my reply inline below
>> -Original Message-
>> From: amd-gfx On Behalf Of
>> Harsh Jain
>> Sent: Friday, November 4, 2022 12:17 PM
>> To: Deucher, Alexander
>> Cc: Jain, Harsh ; amd-gfx@lists.freedesktop.org
>> Subject: [PATCH] drm/amdgpu: wait till Gfxoff allow signal completes
>> during suspend
>>
>> change guarantees that gfxoff is allowed before moving further in
>> s2idle sequence to add more reliablity about gfxoff in amdgpu IP's
>> suspend flow
>>
>> Signed-off-by: Harsh Jain
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> index 222d3d7ea076..5b2afe348c23 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> @@ -581,11 +581,11 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device
>> *adev, bool enable)
>>
>> if (adev->gfx.gfx_off_req_count == 0 &&
>> !adev->gfx.gfx_off_state) {
>> -/* If going to s2idle, no need to wait */
>> -if (adev->in_s0ix)
>> -delay = GFX_OFF_NO_DELAY;
>> schedule_delayed_work(&adev-
>> >gfx.gfx_off_delay_work,
>>delay);
>> +/* If going to s2idle, no need to wait */
>> +if (adev->in_s0ix)
>> +flush_delayed_work(&adev-
>> >gfx.gfx_off_delay_work);
>[Quan, Evan] This seems a little weird. Can you just call
>amdgpu_device_delay_enable_gfx_off directly for s0ix case?
>
>Evan
[Harsh Jain] amdgpu_device_delay_enable_gfx_off is a static function in
amdgpu_device file.
It's better to keep it static otherwise it would create a new api for gfxoff
control for which current api amdgpu_gfx_off_ctrl is available.
Thanks
>> }
>> } else {
>> if (adev->gfx.gfx_off_req_count == 0) {
>> --
>> 2.25.1
Re: [PATCH] drm/amdgpu: add vram reservation logic based on vram_usagebyfirmware_v2_2
Hi Esther,
ok, that works as well.
I would just move the handling of version of the firmware table into
separate functions. That should make it easier to understand what's
going on here.
Thanks,
Christian.
Am 04.11.22 um 10:34 schrieb Liu01, Tong (Esther):
[AMD Official Use Only - General]
Hi Christian,
Based on VBIOS's comment " driver can allocate their own reservation region as long
as it does not overlap firwmare's reservation region". Our understanding is that
they are not mutual exclusive. So we create extra parameters to record drv reservation
request.
Kind regards,
Esther
-Original Message-
From: Koenig, Christian
Sent: 2022年11月4日星期五 下午5:09
To: Liu01, Tong (Esther) ; amd-gfx@lists.freedesktop.org
Cc: Andrey Grodzovsky ; Quan, Evan ; Chen, Horace ;
Tuikov, Luben ; Deucher, Alexander ; Xiao, Jack ;
Zhang, Hawking ; Liu, Monk ; Xu, Feifei ; Wang, Yang(Kevin)
Subject: Re: [PATCH] drm/amdgpu: add vram reservation logic based on
vram_usagebyfirmware_v2_2
Am 04.11.22 um 09:52 schrieb Tong Liu01:
Move TMR region from top of FB to 2MB for FFBM, so we need to reserve
TMR region firstly to make sure TMR can be allocated at 2MB
If I understand it correctly the two methods are mutual exclusive. So I think
you can just extend the existing function in the amdgpu TTM code instead of
adding a new one.
Signed-off-by: Tong Liu01
---
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 84 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 52
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 6 ++
drivers/gpu/drm/amd/include/atomfirmware.h| 56 +++--
4 files changed, 171 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index b81b77a9efa6..f577b1d151d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -106,34 +106,74 @@ int amdgpu_atomfirmware_allocate_fb_scratch(struct
amdgpu_device *adev)
struct atom_context *ctx = adev->mode_info.atom_context;
int index =
get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
vram_usagebyfirmware);
- struct vram_usagebyfirmware_v2_1 *firmware_usage;
- uint32_t start_addr, size;
+ struct vram_usagebyfirmware_v2_1 *firmware_usage_v2_1;
+ struct vram_usagebyfirmware_v2_2 *firmware_usage_v2_2;
+ uint32_t start_addr, size, fw_start_addr, fw_size, drv_addr, drv_size;
uint16_t data_offset;
+ uint8_t frev, crev;
int usage_bytes = 0;
- if (amdgpu_atom_parse_data_header(ctx, index, NULL, NULL, NULL, &data_offset)) {
- firmware_usage = (struct vram_usagebyfirmware_v2_1 *)(ctx->bios
+ data_offset);
- DRM_DEBUG("atom firmware requested %08x %dkb fw %dkb drv\n",
- le32_to_cpu(firmware_usage->start_address_in_kb),
- le16_to_cpu(firmware_usage->used_by_firmware_in_kb),
- le16_to_cpu(firmware_usage->used_by_driver_in_kb));
-
- start_addr = le32_to_cpu(firmware_usage->start_address_in_kb);
- size = le16_to_cpu(firmware_usage->used_by_firmware_in_kb);
-
- if ((uint32_t)(start_addr & ATOM_VRAM_OPERATION_FLAGS_MASK) ==
- (uint32_t)(ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATION
<<
- ATOM_VRAM_OPERATION_FLAGS_SHIFT)) {
- /* Firmware request VRAM reservation for SR-IOV */
- adev->mman.fw_vram_usage_start_offset = (start_addr &
- (~ATOM_VRAM_OPERATION_FLAGS_MASK)) << 10;
- adev->mman.fw_vram_usage_size = size << 10;
- /* Use the default scratch size */
+ if (amdgpu_atom_parse_data_header(ctx, index, NULL, &frev, &crev,
&data_offset)) {
+ if (frev == 2 && crev == 1) {
+ firmware_usage_v2_1 =
+ (struct vram_usagebyfirmware_v2_1 *)(ctx->bios
+ data_offset);
+ DRM_DEBUG("atom firmware v2_1 requested %08x %dkb fw %dkb
drv\n",
+ le32_to_cpu(firmware_usage_v2_1->start_address_in_kb),
+
le16_to_cpu(firmware_usage_v2_1->used_by_firmware_in_kb),
+
le16_to_cpu(firmware_usage_v2_1->used_by_driver_in_kb));
+
+ start_addr =
le32_to_cpu(firmware_usage_v2_1->start_address_in_kb);
+ size =
le16_to_cpu(firmware_usage_v2_1->used_by_firmware_in_kb);
+
+ if ((uint32_t)(start_addr &
ATOM_VRAM_OPERATION_FLAGS_MASK) ==
+
(uint32_t)(ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATION <<
+ ATOM_VRAM_OPERATION_FLAGS_SHIFT)) {
+ /* Firmware
RE: [PATCH] drm/amd/amdgpu: Enable gfx pipe1 and fix related issues
[AMD Official Use Only - General]
>-Original Message-
>From: Christian König
>Sent: Friday, November 4, 2022 5:26 PM
>To: Deng, Emily ; amd-gfx@lists.freedesktop.org;
>Michel Dänzer
>Subject: Re: [PATCH] drm/amd/amdgpu: Enable gfx pipe1 and fix related
>issues
>
>Am 04.11.22 um 01:32 schrieb Emily Deng:
>> Starting from SIENNA CICHLID asic supports two gfx pipes, enabling two
>> graphics queues for performance concern.
>
>With the printk still in the patch I assume that this is just a debugging
>patch?
>
Sorry, will delete it.
>>
>> Signed-off-by: Emily Deng
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +-
>> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 43 +
>> 2 files changed, 23 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> index 331aa191910c..0072f36b44d1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> @@ -33,7 +33,7 @@
>> container_of((e), struct amdgpu_ctx_entity, entity)
>>
>> const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = {
>> -[AMDGPU_HW_IP_GFX] = 1,
>> +[AMDGPU_HW_IP_GFX] = 2,
>
>That's an absolutely clear NAK and as far as I can see also unnecessary.
>
>We don't want to expose the GFX queues as separate queues to userspace.
>
>Instead the queues have separate priorities which userspace can select.
>
>Regards,
>Christian.
>
>> [AMDGPU_HW_IP_COMPUTE] = 4,
>> [AMDGPU_HW_IP_DMA] = 2,
>> [AMDGPU_HW_IP_UVD] = 1,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> index 49d34c7bbf20..9219cd29acd3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> @@ -4606,7 +4606,7 @@ static int gfx_v10_0_sw_init(void *handle)
>> case IP_VERSION(10, 3, 3):
>> case IP_VERSION(10, 3, 7):
>> adev->gfx.me.num_me = 1;
>> -adev->gfx.me.num_pipe_per_me = 1;
>> +adev->gfx.me.num_pipe_per_me = 2;
>> adev->gfx.me.num_queue_per_pipe = 1;
>> adev->gfx.mec.num_mec = 2;
>> adev->gfx.mec.num_pipe_per_mec = 4; @@ -6008,6 +6008,25
>@@ static
>> int gfx_v10_0_cp_gfx_load_microcode(struct amdgpu_device *adev)
>> return 0;
>> }
>>
>> +static int gfx_v10_0_wait_for_idle(void *handle) {
>> +unsigned i;
>> +u32 tmp;
>> +struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> +
>> +for (i = 0; i < adev->usec_timeout; i++) {
>> +/* read MC_STATUS */
>> +tmp = RREG32_SOC15(GC, 0, mmGRBM_STATUS) &
>> +GRBM_STATUS__GUI_ACTIVE_MASK;
>> +
>> +if (!REG_GET_FIELD(tmp, GRBM_STATUS, GUI_ACTIVE))
>> +return 0;
>> +udelay(1);
>> +}
>> +printk("Emily:gfx_v10_0_wait_for_idle\n");
>> +return -ETIMEDOUT;
>> +}
>> +
>> static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)
>> {
>> struct amdgpu_ring *ring;
>> @@ -6069,7 +6088,7 @@ static int gfx_v10_0_cp_gfx_start(struct
>amdgpu_device *adev)
>> amdgpu_ring_write(ring, 0x8000);
>>
>> amdgpu_ring_commit(ring);
>> -
>> +gfx_v10_0_wait_for_idle(adev);
>> /* submit cs packet to copy state 0 to next available state */
>> if (adev->gfx.num_gfx_rings > 1) {
>> /* maximum supported gfx ring is 2 */ @@ -7404,24 +7423,6
>@@
>> static bool gfx_v10_0_is_idle(void *handle)
>> return true;
>> }
>>
>> -static int gfx_v10_0_wait_for_idle(void *handle) -{
>> -unsigned i;
>> -u32 tmp;
>> -struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>> -
>> -for (i = 0; i < adev->usec_timeout; i++) {
>> -/* read MC_STATUS */
>> -tmp = RREG32_SOC15(GC, 0, mmGRBM_STATUS) &
>> -GRBM_STATUS__GUI_ACTIVE_MASK;
>> -
>> -if (!REG_GET_FIELD(tmp, GRBM_STATUS, GUI_ACTIVE))
>> -return 0;
>> -udelay(1);
>> -}
>> -return -ETIMEDOUT;
>> -}
>> -
>> static int gfx_v10_0_soft_reset(void *handle)
>> {
>> u32 grbm_soft_reset = 0;
>> @@ -8466,7 +8467,7 @@ static void gfx_v10_0_ring_emit_hdp_flush(struct
>amdgpu_ring *ring)
>> }
>> reg_mem_engine = 0;
>> } else {
>> -ref_and_mask = nbio_hf_reg->ref_and_mask_cp0;
>> +ref_and_mask = nbio_hf_reg->ref_and_mask_cp0 << ring-
>>pipe;
>> reg_mem_engine = 1; /* pfp */
>> }
>>
RE: [PATCH] drm/amdgpu: add vram reservation logic based on vram_usagebyfirmware_v2_2
[AMD Official Use Only - General]
Hi Christian,
Based on VBIOS's comment " driver can allocate their own reservation region as
long as it does not overlap firwmare's reservation region". Our understanding
is that they are not mutual exclusive. So we create extra parameters to record
drv reservation request.
Kind regards,
Esther
-Original Message-
From: Koenig, Christian
Sent: 2022年11月4日星期五 下午5:09
To: Liu01, Tong (Esther) ; amd-gfx@lists.freedesktop.org
Cc: Andrey Grodzovsky ; Quan, Evan
; Chen, Horace ; Tuikov, Luben
; Deucher, Alexander ; Xiao,
Jack ; Zhang, Hawking ; Liu, Monk
; Xu, Feifei ; Wang, Yang(Kevin)
Subject: Re: [PATCH] drm/amdgpu: add vram reservation logic based on
vram_usagebyfirmware_v2_2
Am 04.11.22 um 09:52 schrieb Tong Liu01:
> Move TMR region from top of FB to 2MB for FFBM, so we need to reserve
> TMR region firstly to make sure TMR can be allocated at 2MB
If I understand it correctly the two methods are mutual exclusive. So I think
you can just extend the existing function in the amdgpu TTM code instead of
adding a new one.
>
> Signed-off-by: Tong Liu01
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 84 ++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 52
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 6 ++
> drivers/gpu/drm/amd/include/atomfirmware.h| 56 +++--
> 4 files changed, 171 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> index b81b77a9efa6..f577b1d151d9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
> @@ -106,34 +106,74 @@ int amdgpu_atomfirmware_allocate_fb_scratch(struct
> amdgpu_device *adev)
> struct atom_context *ctx = adev->mode_info.atom_context;
> int index =
> get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
> vram_usagebyfirmware);
> - struct vram_usagebyfirmware_v2_1 *firmware_usage;
> - uint32_t start_addr, size;
> + struct vram_usagebyfirmware_v2_1 *firmware_usage_v2_1;
> + struct vram_usagebyfirmware_v2_2 *firmware_usage_v2_2;
> + uint32_t start_addr, size, fw_start_addr, fw_size, drv_addr, drv_size;
> uint16_t data_offset;
> + uint8_t frev, crev;
> int usage_bytes = 0;
>
> - if (amdgpu_atom_parse_data_header(ctx, index, NULL, NULL, NULL,
> &data_offset)) {
> - firmware_usage = (struct vram_usagebyfirmware_v2_1 *)(ctx->bios
> + data_offset);
> - DRM_DEBUG("atom firmware requested %08x %dkb fw %dkb drv\n",
> - le32_to_cpu(firmware_usage->start_address_in_kb),
> - le16_to_cpu(firmware_usage->used_by_firmware_in_kb),
> - le16_to_cpu(firmware_usage->used_by_driver_in_kb));
> -
> - start_addr = le32_to_cpu(firmware_usage->start_address_in_kb);
> - size = le16_to_cpu(firmware_usage->used_by_firmware_in_kb);
> -
> - if ((uint32_t)(start_addr & ATOM_VRAM_OPERATION_FLAGS_MASK) ==
> - (uint32_t)(ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATION
> <<
> - ATOM_VRAM_OPERATION_FLAGS_SHIFT)) {
> - /* Firmware request VRAM reservation for SR-IOV */
> - adev->mman.fw_vram_usage_start_offset = (start_addr &
> - (~ATOM_VRAM_OPERATION_FLAGS_MASK)) << 10;
> - adev->mman.fw_vram_usage_size = size << 10;
> - /* Use the default scratch size */
> + if (amdgpu_atom_parse_data_header(ctx, index, NULL, &frev, &crev,
> &data_offset)) {
> + if (frev == 2 && crev == 1) {
> + firmware_usage_v2_1 =
> + (struct vram_usagebyfirmware_v2_1 *)(ctx->bios
> + data_offset);
> + DRM_DEBUG("atom firmware v2_1 requested %08x %dkb fw
> %dkb drv\n",
> + le32_to_cpu(firmware_usage_v2_1->start_address_in_kb),
> +
> le16_to_cpu(firmware_usage_v2_1->used_by_firmware_in_kb),
> +
> le16_to_cpu(firmware_usage_v2_1->used_by_driver_in_kb));
> +
> + start_addr =
> le32_to_cpu(firmware_usage_v2_1->start_address_in_kb);
> + size =
> le16_to_cpu(firmware_usage_v2_1->used_by_firmware_in_kb);
> +
> + if ((uint32_t)(start_addr &
> ATOM_VRAM_OPERATION_FLAGS_MASK) ==
> +
> (uint32_t)(ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATION <<
> + ATOM_VRAM_OPERATION_FLAGS_SHIFT)) {
> + /* Firmware request VRAM reservation for SR-IOV
> */
> + adev->mman.fw_vram_usage_start_offset =
> (start_addr &
> + (~ATOM_VRAM_OPERATIO
[PATCH] Revert "drm/amdgpu: Revert "drm/amdgpu: getting fan speed pwm for vega10 properly""
This reverts commit 97370f1826eb7ee6880e09ee1eaafe28232cabc6.
The origin patch "drm/amdgpu: getting fan speed pwm for vega10 properly" works
fine. Test failure is caused by test case self.
Signed-off-by: Asher Song
---
.../amd/pm/powerplay/hwmgr/vega10_thermal.c | 25 +--
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
index dad3e3741a4e..190af79f3236 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/vega10_thermal.c
@@ -67,22 +67,21 @@ int vega10_fan_ctrl_get_fan_speed_info(struct pp_hwmgr
*hwmgr,
int vega10_fan_ctrl_get_fan_speed_pwm(struct pp_hwmgr *hwmgr,
uint32_t *speed)
{
- uint32_t current_rpm;
- uint32_t percent = 0;
-
- if (hwmgr->thermal_controller.fanInfo.bNoFan)
- return 0;
+ struct amdgpu_device *adev = hwmgr->adev;
+ uint32_t duty100, duty;
+ uint64_t tmp64;
- if (vega10_get_current_rpm(hwmgr, ¤t_rpm))
- return -1;
+ duty100 = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_FDO_CTRL1),
+ CG_FDO_CTRL1, FMAX_DUTY100);
+ duty = REG_GET_FIELD(RREG32_SOC15(THM, 0, mmCG_THERMAL_STATUS),
+ CG_THERMAL_STATUS, FDO_PWM_DUTY);
- if (hwmgr->thermal_controller.
- advanceFanControlParameters.usMaxFanRPM != 0)
- percent = current_rpm * 255 /
- hwmgr->thermal_controller.
- advanceFanControlParameters.usMaxFanRPM;
+ if (!duty100)
+ return -EINVAL;
- *speed = MIN(percent, 255);
+ tmp64 = (uint64_t)duty * 255;
+ do_div(tmp64, duty100);
+ *speed = MIN((uint32_t)tmp64, 255);
return 0;
}
--
2.25.1
Re: [PATCH 1/1] drm/amdgpu: Unlock bo_list_mutex after error handling
Am 03.11.22 um 15:59 schrieb Philip Yang:
Get below kernel WARNING backtrace when pressing ctrl-C to kill kfdtest
application.
If amdgpu_cs_parser_bos returns error after taking bo_list_mutex, as
caller amdgpu_cs_ioctl will not unlock bo_list_mutex, this generates the
kernel WARNING.
Add unlock bo_list_mutex after amdgpu_cs_parser_bos error handling to
cleanup bo_list userptr bo.
WARNING: kfdtest/2930 still has locks held!
1 lock held by kfdtest/2930:
(&list->bo_list_mutex){+.+.}-{3:3}, at: amdgpu_cs_ioctl+0xce5/0x1f10 [amdgpu]
stack backtrace:
dump_stack_lvl+0x44/0x57
get_signal+0x79f/0xd00
arch_do_signal_or_restart+0x36/0x7b0
exit_to_user_mode_prepare+0xfd/0x1b0
syscall_exit_to_user_mode+0x19/0x40
do_syscall_64+0x40/0x80
Signed-off-by: Philip Yang
Good catch, patch is Reviewed-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 1bbd39b3b0fc..d371000a5727 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -992,6 +992,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
kvfree(e->user_pages);
e->user_pages = NULL;
}
+ mutex_unlock(&p->bo_list->bo_list_mutex);
return r;
}
Re: [PATCH] drm/amd/amdgpu: Enable gfx pipe1 and fix related issues
Am 04.11.22 um 01:32 schrieb Emily Deng:
Starting from SIENNA CICHLID asic supports two gfx pipes, enabling
two graphics queues for performance concern.
With the printk still in the patch I assume that this is just a
debugging patch?
Signed-off-by: Emily Deng
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 2 +-
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 43 +
2 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 331aa191910c..0072f36b44d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -33,7 +33,7 @@
container_of((e), struct amdgpu_ctx_entity, entity)
const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM] = {
- [AMDGPU_HW_IP_GFX] = 1,
+ [AMDGPU_HW_IP_GFX] = 2,
That's an absolutely clear NAK and as far as I can see also unnecessary.
We don't want to expose the GFX queues as separate queues to userspace.
Instead the queues have separate priorities which userspace can select.
Regards,
Christian.
[AMDGPU_HW_IP_COMPUTE] = 4,
[AMDGPU_HW_IP_DMA] = 2,
[AMDGPU_HW_IP_UVD] = 1,
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 49d34c7bbf20..9219cd29acd3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -4606,7 +4606,7 @@ static int gfx_v10_0_sw_init(void *handle)
case IP_VERSION(10, 3, 3):
case IP_VERSION(10, 3, 7):
adev->gfx.me.num_me = 1;
- adev->gfx.me.num_pipe_per_me = 1;
+ adev->gfx.me.num_pipe_per_me = 2;
adev->gfx.me.num_queue_per_pipe = 1;
adev->gfx.mec.num_mec = 2;
adev->gfx.mec.num_pipe_per_mec = 4;
@@ -6008,6 +6008,25 @@ static int gfx_v10_0_cp_gfx_load_microcode(struct
amdgpu_device *adev)
return 0;
}
+static int gfx_v10_0_wait_for_idle(void *handle)
+{
+ unsigned i;
+ u32 tmp;
+ struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+
+ for (i = 0; i < adev->usec_timeout; i++) {
+ /* read MC_STATUS */
+ tmp = RREG32_SOC15(GC, 0, mmGRBM_STATUS) &
+ GRBM_STATUS__GUI_ACTIVE_MASK;
+
+ if (!REG_GET_FIELD(tmp, GRBM_STATUS, GUI_ACTIVE))
+ return 0;
+ udelay(1);
+ }
+ printk("Emily:gfx_v10_0_wait_for_idle\n");
+ return -ETIMEDOUT;
+}
+
static int gfx_v10_0_cp_gfx_start(struct amdgpu_device *adev)
{
struct amdgpu_ring *ring;
@@ -6069,7 +6088,7 @@ static int gfx_v10_0_cp_gfx_start(struct amdgpu_device
*adev)
amdgpu_ring_write(ring, 0x8000);
amdgpu_ring_commit(ring);
-
+ gfx_v10_0_wait_for_idle(adev);
/* submit cs packet to copy state 0 to next available state */
if (adev->gfx.num_gfx_rings > 1) {
/* maximum supported gfx ring is 2 */
@@ -7404,24 +7423,6 @@ static bool gfx_v10_0_is_idle(void *handle)
return true;
}
-static int gfx_v10_0_wait_for_idle(void *handle)
-{
- unsigned i;
- u32 tmp;
- struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-
- for (i = 0; i < adev->usec_timeout; i++) {
- /* read MC_STATUS */
- tmp = RREG32_SOC15(GC, 0, mmGRBM_STATUS) &
- GRBM_STATUS__GUI_ACTIVE_MASK;
-
- if (!REG_GET_FIELD(tmp, GRBM_STATUS, GUI_ACTIVE))
- return 0;
- udelay(1);
- }
- return -ETIMEDOUT;
-}
-
static int gfx_v10_0_soft_reset(void *handle)
{
u32 grbm_soft_reset = 0;
@@ -8466,7 +8467,7 @@ static void gfx_v10_0_ring_emit_hdp_flush(struct
amdgpu_ring *ring)
}
reg_mem_engine = 0;
} else {
- ref_and_mask = nbio_hf_reg->ref_and_mask_cp0;
+ ref_and_mask = nbio_hf_reg->ref_and_mask_cp0 << ring->pipe;
reg_mem_engine = 1; /* pfp */
}
Re: [PATCH] drm/amdgpu: add vram reservation logic based on vram_usagebyfirmware_v2_2
Am 04.11.22 um 09:52 schrieb Tong Liu01:
Move TMR region from top of FB to 2MB for FFBM, so we need to reserve TMR region
firstly to make sure TMR can be allocated at 2MB
If I understand it correctly the two methods are mutual exclusive. So I
think you can just extend the existing function in the amdgpu TTM code
instead of adding a new one.
Signed-off-by: Tong Liu01
---
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 84 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 52
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 6 ++
drivers/gpu/drm/amd/include/atomfirmware.h| 56 +++--
4 files changed, 171 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index b81b77a9efa6..f577b1d151d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -106,34 +106,74 @@ int amdgpu_atomfirmware_allocate_fb_scratch(struct
amdgpu_device *adev)
struct atom_context *ctx = adev->mode_info.atom_context;
int index =
get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
vram_usagebyfirmware);
- struct vram_usagebyfirmware_v2_1 *firmware_usage;
- uint32_t start_addr, size;
+ struct vram_usagebyfirmware_v2_1 *firmware_usage_v2_1;
+ struct vram_usagebyfirmware_v2_2 *firmware_usage_v2_2;
+ uint32_t start_addr, size, fw_start_addr, fw_size, drv_addr, drv_size;
uint16_t data_offset;
+ uint8_t frev, crev;
int usage_bytes = 0;
- if (amdgpu_atom_parse_data_header(ctx, index, NULL, NULL, NULL, &data_offset)) {
- firmware_usage = (struct vram_usagebyfirmware_v2_1 *)(ctx->bios
+ data_offset);
- DRM_DEBUG("atom firmware requested %08x %dkb fw %dkb drv\n",
- le32_to_cpu(firmware_usage->start_address_in_kb),
- le16_to_cpu(firmware_usage->used_by_firmware_in_kb),
- le16_to_cpu(firmware_usage->used_by_driver_in_kb));
-
- start_addr = le32_to_cpu(firmware_usage->start_address_in_kb);
- size = le16_to_cpu(firmware_usage->used_by_firmware_in_kb);
-
- if ((uint32_t)(start_addr & ATOM_VRAM_OPERATION_FLAGS_MASK) ==
- (uint32_t)(ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATION
<<
- ATOM_VRAM_OPERATION_FLAGS_SHIFT)) {
- /* Firmware request VRAM reservation for SR-IOV */
- adev->mman.fw_vram_usage_start_offset = (start_addr &
- (~ATOM_VRAM_OPERATION_FLAGS_MASK)) << 10;
- adev->mman.fw_vram_usage_size = size << 10;
- /* Use the default scratch size */
+ if (amdgpu_atom_parse_data_header(ctx, index, NULL, &frev, &crev,
&data_offset)) {
+ if (frev == 2 && crev == 1) {
+ firmware_usage_v2_1 =
+ (struct vram_usagebyfirmware_v2_1 *)(ctx->bios
+ data_offset);
+ DRM_DEBUG("atom firmware v2_1 requested %08x %dkb fw %dkb
drv\n",
+ le32_to_cpu(firmware_usage_v2_1->start_address_in_kb),
+
le16_to_cpu(firmware_usage_v2_1->used_by_firmware_in_kb),
+
le16_to_cpu(firmware_usage_v2_1->used_by_driver_in_kb));
+
+ start_addr =
le32_to_cpu(firmware_usage_v2_1->start_address_in_kb);
+ size =
le16_to_cpu(firmware_usage_v2_1->used_by_firmware_in_kb);
+
+ if ((uint32_t)(start_addr &
ATOM_VRAM_OPERATION_FLAGS_MASK) ==
+
(uint32_t)(ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATION <<
+ ATOM_VRAM_OPERATION_FLAGS_SHIFT)) {
+ /* Firmware request VRAM reservation for SR-IOV
*/
+ adev->mman.fw_vram_usage_start_offset = (start_addr
&
+ (~ATOM_VRAM_OPERATION_FLAGS_MASK)) <<
10;
+ adev->mman.fw_vram_usage_size = size << 10;
+ /* Use the default scratch size */
+ usage_bytes = 0;
+ } else {
+ usage_bytes =
+
le16_to_cpu(firmware_usage_v2_1->used_by_driver_in_kb) << 10;
+ }
That should probably be put into separate functions.
Regards,
Christian.
+ } else if (frev >= 2 && crev >= 2) {
+ firmware_usage_v2_2 =
+ (struct vram_usagebyfirmware_v2_2 *)(ctx->bios
+ data_offset);
+ DRM_DEBUG("atom requested fw start at %08x %dkb and drv
start at %08x %dkb\n",
+
[PATCH] drm/amdgpu: add vram reservation logic based on vram_usagebyfirmware_v2_2
Move TMR region from top of FB to 2MB for FFBM, so we need to reserve TMR region
firstly to make sure TMR can be allocated at 2MB
Signed-off-by: Tong Liu01
---
.../gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 84 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 52
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 6 ++
drivers/gpu/drm/amd/include/atomfirmware.h| 56 +++--
4 files changed, 171 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index b81b77a9efa6..f577b1d151d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -106,34 +106,74 @@ int amdgpu_atomfirmware_allocate_fb_scratch(struct
amdgpu_device *adev)
struct atom_context *ctx = adev->mode_info.atom_context;
int index =
get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
vram_usagebyfirmware);
- struct vram_usagebyfirmware_v2_1 *firmware_usage;
- uint32_t start_addr, size;
+ struct vram_usagebyfirmware_v2_1 *firmware_usage_v2_1;
+ struct vram_usagebyfirmware_v2_2 *firmware_usage_v2_2;
+ uint32_t start_addr, size, fw_start_addr, fw_size, drv_addr, drv_size;
uint16_t data_offset;
+ uint8_t frev, crev;
int usage_bytes = 0;
- if (amdgpu_atom_parse_data_header(ctx, index, NULL, NULL, NULL,
&data_offset)) {
- firmware_usage = (struct vram_usagebyfirmware_v2_1 *)(ctx->bios
+ data_offset);
- DRM_DEBUG("atom firmware requested %08x %dkb fw %dkb drv\n",
- le32_to_cpu(firmware_usage->start_address_in_kb),
- le16_to_cpu(firmware_usage->used_by_firmware_in_kb),
- le16_to_cpu(firmware_usage->used_by_driver_in_kb));
-
- start_addr = le32_to_cpu(firmware_usage->start_address_in_kb);
- size = le16_to_cpu(firmware_usage->used_by_firmware_in_kb);
-
- if ((uint32_t)(start_addr & ATOM_VRAM_OPERATION_FLAGS_MASK) ==
- (uint32_t)(ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATION
<<
- ATOM_VRAM_OPERATION_FLAGS_SHIFT)) {
- /* Firmware request VRAM reservation for SR-IOV */
- adev->mman.fw_vram_usage_start_offset = (start_addr &
- (~ATOM_VRAM_OPERATION_FLAGS_MASK)) << 10;
- adev->mman.fw_vram_usage_size = size << 10;
- /* Use the default scratch size */
+ if (amdgpu_atom_parse_data_header(ctx, index, NULL, &frev, &crev,
&data_offset)) {
+ if (frev == 2 && crev == 1) {
+ firmware_usage_v2_1 =
+ (struct vram_usagebyfirmware_v2_1 *)(ctx->bios
+ data_offset);
+ DRM_DEBUG("atom firmware v2_1 requested %08x %dkb fw
%dkb drv\n",
+ le32_to_cpu(firmware_usage_v2_1->start_address_in_kb),
+
le16_to_cpu(firmware_usage_v2_1->used_by_firmware_in_kb),
+
le16_to_cpu(firmware_usage_v2_1->used_by_driver_in_kb));
+
+ start_addr =
le32_to_cpu(firmware_usage_v2_1->start_address_in_kb);
+ size =
le16_to_cpu(firmware_usage_v2_1->used_by_firmware_in_kb);
+
+ if ((uint32_t)(start_addr &
ATOM_VRAM_OPERATION_FLAGS_MASK) ==
+
(uint32_t)(ATOM_VRAM_BLOCK_SRIOV_MSG_SHARE_RESERVATION <<
+ ATOM_VRAM_OPERATION_FLAGS_SHIFT)) {
+ /* Firmware request VRAM reservation for SR-IOV
*/
+ adev->mman.fw_vram_usage_start_offset =
(start_addr &
+ (~ATOM_VRAM_OPERATION_FLAGS_MASK)) <<
10;
+ adev->mman.fw_vram_usage_size = size << 10;
+ /* Use the default scratch size */
+ usage_bytes = 0;
+ } else {
+ usage_bytes =
+
le16_to_cpu(firmware_usage_v2_1->used_by_driver_in_kb) << 10;
+ }
+ } else if (frev >= 2 && crev >= 2) {
+ firmware_usage_v2_2 =
+ (struct vram_usagebyfirmware_v2_2 *)(ctx->bios
+ data_offset);
+ DRM_DEBUG("atom requested fw start at %08x %dkb and drv
start at %08x %dkb\n",
+
le32_to_cpu(firmware_usage_v2_2->fw_region_start_address_in_kb),
+
le16_to_cpu(firmware_usage_v2_2->used_by_firmware_in_kb),
+
le32_to_cpu(firmware_usage_v2_2->driver_region0_start_address_in_kb),
+
le32_to_cpu(fir
Re: [PATCH] drm/amdgpu: workaround for TLB seq race
Working fine after 26hrs+ of testing, plus another 24hrs of the previous version of this patch. Sorry if there are multiple replies, I had to figure out how to properly reply to a mailing list, such that the reply is picked up by patchwork (1st time doing this). Tested-by: Stefan Springer
RE: [PATCH] drm/amdgpu: wait till Gfxoff allow signal completes during suspend
[AMD Official Use Only - General]
> -Original Message-
> From: amd-gfx On Behalf Of
> Harsh Jain
> Sent: Friday, November 4, 2022 12:17 PM
> To: Deucher, Alexander
> Cc: Jain, Harsh ; amd-gfx@lists.freedesktop.org
> Subject: [PATCH] drm/amdgpu: wait till Gfxoff allow signal completes during
> suspend
>
> change guarantees that gfxoff is allowed before moving further in
> s2idle sequence to add more reliablity about gfxoff in amdgpu IP's
> suspend flow
>
> Signed-off-by: Harsh Jain
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 222d3d7ea076..5b2afe348c23 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -581,11 +581,11 @@ void amdgpu_gfx_off_ctrl(struct amdgpu_device
> *adev, bool enable)
>
> if (adev->gfx.gfx_off_req_count == 0 &&
> !adev->gfx.gfx_off_state) {
> - /* If going to s2idle, no need to wait */
> - if (adev->in_s0ix)
> - delay = GFX_OFF_NO_DELAY;
> schedule_delayed_work(&adev-
> >gfx.gfx_off_delay_work,
> delay);
> + /* If going to s2idle, no need to wait */
> + if (adev->in_s0ix)
> + flush_delayed_work(&adev-
> >gfx.gfx_off_delay_work);
[Quan, Evan] This seems a little weird. Can you just call
amdgpu_device_delay_enable_gfx_off directly for s0ix case?
Evan
> }
> } else {
> if (adev->gfx.gfx_off_req_count == 0) {
> --
> 2.25.1
<>
Re: [PATCH] drm/amdgpu: workaround for TLB seq race
Am 03.11.22 um 22:18 schrieb Philip Yang:
On 2022-11-02 10:58, Christian König wrote:
It can happen that we query the sequence value before the callback
had a chance to run.
Work around that by grabbing the fence lock and releasing it again.
Should be replaced by hw handling soon.
kfd_flush_tlb is always called after waiting for map/unmap to GPU
fence signalled, that means the callback is already executed
And exactly that's incorrect.
Waiting for the fence to signal means that the callback has started
executing, but it doesn't mean that it is finished.
This can then result in one CPU racing with the callback handler and
because of this you see the wrong TLB seq.
Regards,
Christian.
and the sequence is increased if tlb flush is needed, so no such race
from KFD.
I am not sure but seems the race does exist for amdgpu to grab vm and
schedule job.
Acked-by: Philip Yang
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 9ecb7f663e19..e51a46c9582b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -485,6 +485,21 @@ void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm
*vm, struct seq_file *m);
*/
static inline uint64_t amdgpu_vm_tlb_seq(struct amdgpu_vm *vm)
{
+ unsigned long flags;
+ spinlock_t *lock;
+
+ /*
+ * Work around to stop racing between the fence signaling and
handling
+ * the cb. The lock is static after initially setting it up,
just make
+ * sure that the dma_fence structure isn't freed up.
+ */
+ rcu_read_lock();
+ lock = vm->last_tlb_flush->lock;
+ rcu_read_unlock();
+
+ spin_lock_irqsave(lock, flags);
+ spin_unlock_irqrestore(lock, flags);
+
return atomic64_read(&vm->tlb_seq);
}
