RE: [PATCH] drm/amd/powerplay: grant Arcturus softmin/max setting on latest PM firmware
[AMD Official Use Only - Internal Distribution Only]
Reviewed-by: Kenneth Feng
-Original Message-
From: Quan, Evan
Sent: Wednesday, August 5, 2020 11:24 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Feng, Kenneth
; Quan, Evan
Subject: [PATCH] drm/amd/powerplay: grant Arcturus softmin/max setting on
latest PM firmware
For Arcturus, the softmin/max settings from driver are permitted on the
latest(54.26 later) SMU firmware. Thus enabling them in driver.
Change-Id: Iff9ac326610075aa7f61cb89c64d2c4128678755
Signed-off-by: Evan Quan
---
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index a2ba6633fc21..73de3c013834 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -942,9 +942,10 @@ static int arcturus_force_clk_levels(struct smu_context
*smu,
return ret;
}
- if (smu_version >= 0x361200) {
+ if ((smu_version >= 0x361200) &&
+ (smu_version <= 0x361a00)) {
dev_err(smu->adev->dev, "Forcing clock level is not supported
with "
- "54.18 and onwards SMU firmwares\n");
+ "54.18 - 54.26(included) SMU firmwares\n");
return -EOPNOTSUPP;
}
@@ -1437,9 +1438,10 @@ static int arcturus_set_performance_level(struct
smu_context *smu,
case AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK:
case AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK:
case AMD_DPM_FORCED_LEVEL_PROFILE_PEAK:
- if (smu_version >= 0x361200) {
+ if ((smu_version >= 0x361200) &&
+ (smu_version <= 0x361a00)) {
dev_err(smu->adev->dev, "Forcing clock level is not
supported with "
- "54.18 and onwards SMU firmwares\n");
+ "54.18 - 54.26(included) SMU firmwares\n");
return -EOPNOTSUPP;
}
break;
--
2.28.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/powerplay: grant Arcturus softmin/max setting on latest PM firmware
For Arcturus, the softmin/max settings from driver are permitted on the
latest(54.26 later) SMU firmware. Thus enabling them in driver.
Change-Id: Iff9ac326610075aa7f61cb89c64d2c4128678755
Signed-off-by: Evan Quan
---
drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index a2ba6633fc21..73de3c013834 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -942,9 +942,10 @@ static int arcturus_force_clk_levels(struct smu_context
*smu,
return ret;
}
- if (smu_version >= 0x361200) {
+ if ((smu_version >= 0x361200) &&
+ (smu_version <= 0x361a00)) {
dev_err(smu->adev->dev, "Forcing clock level is not supported
with "
- "54.18 and onwards SMU firmwares\n");
+ "54.18 - 54.26(included) SMU firmwares\n");
return -EOPNOTSUPP;
}
@@ -1437,9 +1438,10 @@ static int arcturus_set_performance_level(struct
smu_context *smu,
case AMD_DPM_FORCED_LEVEL_PROFILE_MIN_SCLK:
case AMD_DPM_FORCED_LEVEL_PROFILE_MIN_MCLK:
case AMD_DPM_FORCED_LEVEL_PROFILE_PEAK:
- if (smu_version >= 0x361200) {
+ if ((smu_version >= 0x361200) &&
+ (smu_version <= 0x361a00)) {
dev_err(smu->adev->dev, "Forcing clock level is not
supported with "
- "54.18 and onwards SMU firmwares\n");
+ "54.18 - 54.26(included) SMU firmwares\n");
return -EOPNOTSUPP;
}
break;
--
2.28.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 17/17] drm/amd/powerplay: add control method to bypass metrics cache on Vega12
[AMD Official Use Only - Internal Distribution Only]
The cache is useful for the case like sysfs "amdgpu_pm_info". Which inquires
many metrics data in a very short period.
Without the cache, there will be multiple table transfers triggered(unnecessary
as the PMFW sample interval is 1ms).
The unreasonable setting in our driver is the cache interval. For this special
ASIC(vega12), 0.5S is used which is too big I think.
It should not be bigger than the PMFW sample internal setting(1ms). Otherwise
we may get outdated data.
BR
Evan
-Original Message-
From: Alex Deucher
Sent: Wednesday, August 5, 2020 4:42 AM
To: Quan, Evan
Cc: amd-gfx list ; Deucher, Alexander
; Kuehling, Felix ;
Kasiviswanathan, Harish ; Das, Nirmoy
Subject: Re: [PATCH 17/17] drm/amd/powerplay: add control method to bypass
metrics cache on Vega12
Do we want the metrics cache at all? I can see arguments both ways.
Patches 12-17 are:
Reviewed-by: Alex Deucher
On Thu, Jul 30, 2020 at 10:45 PM Evan Quan wrote:
>
> As for the gpu metric export, metrics cache makes no sense. It's up to
> user to decide how often the metrics should be retrieved.
>
> Change-Id: Ic2a27ebc90f0a7cf581d0697c121b6d7df030f3b
> Signed-off-by: Evan Quan
> ---
> .../drm/amd/powerplay/hwmgr/vega12_hwmgr.c| 29 ---
> 1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> index 40bb0c2e4e8c..c70c30175801 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> @@ -1262,22 +1262,29 @@ static uint32_t vega12_dpm_get_mclk(struct pp_hwmgr
> *hwmgr, bool low)
> return (mem_clk * 100);
> }
>
> -static int vega12_get_metrics_table(struct pp_hwmgr *hwmgr,
> SmuMetrics_t *metrics_table)
> +static int vega12_get_metrics_table(struct pp_hwmgr *hwmgr,
> + SmuMetrics_t *metrics_table,
> + bool bypass_cache)
> {
> struct vega12_hwmgr *data =
> (struct vega12_hwmgr *)(hwmgr->backend);
> int ret = 0;
>
> - if (!data->metrics_time || time_after(jiffies, data->metrics_time +
> HZ / 2)) {
> - ret = smum_smc_table_manager(hwmgr, (uint8_t *)metrics_table,
> - TABLE_SMU_METRICS, true);
> + if (bypass_cache ||
> + !data->metrics_time ||
> + time_after(jiffies, data->metrics_time + HZ / 2)) {
> + ret = smum_smc_table_manager(hwmgr,
> +(uint8_t
> *)(>metrics_table),
> +TABLE_SMU_METRICS,
> +true);
> if (ret) {
> pr_info("Failed to export SMU metrics table!\n");
> return ret;
> }
> - memcpy(>metrics_table, metrics_table,
> sizeof(SmuMetrics_t));
> data->metrics_time = jiffies;
> - } else
> + }
> +
> + if (metrics_table)
> memcpy(metrics_table, >metrics_table,
> sizeof(SmuMetrics_t));
>
> return ret;
> @@ -1288,7 +1295,7 @@ static int vega12_get_gpu_power(struct pp_hwmgr *hwmgr,
> uint32_t *query)
> SmuMetrics_t metrics_table;
> int ret = 0;
>
> - ret = vega12_get_metrics_table(hwmgr, _table);
> + ret = vega12_get_metrics_table(hwmgr, _table, false);
> if (ret)
> return ret;
>
> @@ -1339,7 +1346,7 @@ static int vega12_get_current_activity_percent(
> SmuMetrics_t metrics_table;
> int ret = 0;
>
> - ret = vega12_get_metrics_table(hwmgr, _table);
> + ret = vega12_get_metrics_table(hwmgr, _table, false);
> if (ret)
> return ret;
>
> @@ -1387,7 +1394,7 @@ static int vega12_read_sensor(struct pp_hwmgr *hwmgr,
> int idx,
> *size = 4;
> break;
> case AMDGPU_PP_SENSOR_HOTSPOT_TEMP:
> - ret = vega12_get_metrics_table(hwmgr, _table);
> + ret = vega12_get_metrics_table(hwmgr, _table,
> + false);
> if (ret)
> return ret;
>
> @@ -1396,7 +1403,7 @@ static int vega12_read_sensor(struct pp_hwmgr *hwmgr,
> int idx,
> *size = 4;
> break;
> case AMDGPU_PP_SENSOR_MEM_TEMP:
> - ret = vega12_get_metrics_table(hwmgr, _table);
> + ret = vega12_get_metrics_table(hwmgr, _table,
> + false);
> if (ret)
> return ret;
>
> @@ -2752,7 +2759,7 @@ static ssize_t vega12_get_gpu_metrics(struct pp_hwmgr
> *hwmgr,
> uint32_t fan_speed_rpm;
> int ret;
>
> - ret = vega12_get_metrics_table(hwmgr, );
> + ret = vega12_get_metrics_table(hwmgr, , true);
> if (ret)
>
Re: Enabling AMDGPU by default for SI & CIK
[AMD Official Use Only - Internal Distribution Only] At the risk of asking a dumb question, does amdgpu default to using DC on SI and CI ? I'm asking because a lot of people seem to be using amdgpu successfully with analog outputs today on SI/CI... suggests that they are not using DC ? If so then would enabling HDMI/DP audio support without DC be sufficient to flip the switch assuming we felt that other risks were manageable ? Thanks, John From: amd-gfx on behalf of Alex Deucher Sent: August 4, 2020 1:35 PM To: Michel Dänzer Cc: Deucher, Alexander ; Koenig, Christian ; amd-gfx mailing list ; Bas Nieuwenhuizen Subject: Re: Enabling AMDGPU by default for SI & CIK On Tue, Aug 4, 2020 at 4:38 AM Michel Dänzer wrote: > > On 2020-08-03 1:45 a.m., Bas Nieuwenhuizen wrote: > > Hi all, > > > > Now that we have recently made some progress on getting feature parity > > with the Radeon driver for SI, I'm wondering what it would take to > > make AMDGPU the default driver for these generations. > > > > As far as I understand AMDGPU has had these features for CIK for a > > while already but it is still not the default driver. What would it > > take to make it the default? What is missing and/or broken? > > The main blockers I'm aware of for CIK are: > > 1) Lack of analogue connector support with DC > 2) Lack of HDMI/DP audio support without DC > > > 1) may apply to SI as well. Also, IIRC, there are suspend and resume problems with some CIK parts using amdgpu. Alex ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cjohn.bridgman%40amd.com%7C26df81f9a6df4e2f9fbb08d8389cda79%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637321593620378481sdata=Ep%2BYRRT1dAcE8zSDIaZiXuVMb9gBVUnLnbtP1%2Be7Pkc%3Dreserved=0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v3] drm/amdkfd: option to disable system mem limit
Am 2020-08-04 um 8:43 p.m. schrieb Philip Yang:
> If multiple process share system memory through /dev/shm, KFD allocate
> memory should not fail if it reaches the system memory limit because
> one copy of physical system memory are shared by multiple process.
>
> Add module parameter no_system_mem_limit to provide user option to
> disable system memory limit check at runtime using sysfs or during
> driver module init using kernel boot argument. By default the system
> memory limit is on.
>
> Print out debug message to warn user if KFD allocate memory failed
> because system memory reaches limit.
>
> Signed-off-by: Philip Yang
Reviewed-by: Felix Kuehling
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 4e8622854e61..0459e53f5917 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -188,9 +188,11 @@ extern int amdgpu_force_asic_type;
> #ifdef CONFIG_HSA_AMD
> extern int sched_policy;
> extern bool debug_evictions;
> +extern bool no_system_mem_limit;
> #else
> static const int sched_policy = KFD_SCHED_POLICY_HWS;
> static const bool debug_evictions; /* = false */
> +static const bool no_system_mem_limit;
> #endif
>
> extern int amdgpu_tmz;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 8703aa1fe4a5..0d75726bd228 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -148,8 +148,12 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
> amdgpu_device *adev,
>
> spin_lock(_mem_limit.mem_limit_lock);
>
> + if (kfd_mem_limit.system_mem_used + system_mem_needed >
> + kfd_mem_limit.max_system_mem_limit)
> + pr_debug("Set no_system_mem_limit=1 if using shared memory\n");
> +
> if ((kfd_mem_limit.system_mem_used + system_mem_needed >
> - kfd_mem_limit.max_system_mem_limit) ||
> + kfd_mem_limit.max_system_mem_limit && !no_system_mem_limit) ||
> (kfd_mem_limit.ttm_mem_used + ttm_mem_needed >
>kfd_mem_limit.max_ttm_mem_limit) ||
> (adev->kfd.vram_used + vram_needed >
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index a252450734f6..d3bd7a7da174 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -717,6 +717,15 @@ MODULE_PARM_DESC(queue_preemption_timeout_ms, "queue
> preemption timeout in ms (1
> bool debug_evictions;
> module_param(debug_evictions, bool, 0644);
> MODULE_PARM_DESC(debug_evictions, "enable eviction debug messages (false =
> default)");
> +
> +/**
> + * DOC: no_system_mem_limit(bool)
> + * Disable system memory limit, to support multiple process shared memory
> + */
> +bool no_system_mem_limit;
> +module_param(no_system_mem_limit, bool, 0644);
> +MODULE_PARM_DESC(no_system_mem_limit, "disable system memory limit (false =
> default)");
> +
> #endif
>
> /**
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v3] drm/amdkfd: option to disable system mem limit
If multiple process share system memory through /dev/shm, KFD allocate
memory should not fail if it reaches the system memory limit because
one copy of physical system memory are shared by multiple process.
Add module parameter no_system_mem_limit to provide user option to
disable system memory limit check at runtime using sysfs or during
driver module init using kernel boot argument. By default the system
memory limit is on.
Print out debug message to warn user if KFD allocate memory failed
because system memory reaches limit.
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 4e8622854e61..0459e53f5917 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -188,9 +188,11 @@ extern int amdgpu_force_asic_type;
#ifdef CONFIG_HSA_AMD
extern int sched_policy;
extern bool debug_evictions;
+extern bool no_system_mem_limit;
#else
static const int sched_policy = KFD_SCHED_POLICY_HWS;
static const bool debug_evictions; /* = false */
+static const bool no_system_mem_limit;
#endif
extern int amdgpu_tmz;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 8703aa1fe4a5..0d75726bd228 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -148,8 +148,12 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
amdgpu_device *adev,
spin_lock(_mem_limit.mem_limit_lock);
+ if (kfd_mem_limit.system_mem_used + system_mem_needed >
+ kfd_mem_limit.max_system_mem_limit)
+ pr_debug("Set no_system_mem_limit=1 if using shared memory\n");
+
if ((kfd_mem_limit.system_mem_used + system_mem_needed >
-kfd_mem_limit.max_system_mem_limit) ||
+kfd_mem_limit.max_system_mem_limit && !no_system_mem_limit) ||
(kfd_mem_limit.ttm_mem_used + ttm_mem_needed >
kfd_mem_limit.max_ttm_mem_limit) ||
(adev->kfd.vram_used + vram_needed >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index a252450734f6..d3bd7a7da174 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -717,6 +717,15 @@ MODULE_PARM_DESC(queue_preemption_timeout_ms, "queue
preemption timeout in ms (1
bool debug_evictions;
module_param(debug_evictions, bool, 0644);
MODULE_PARM_DESC(debug_evictions, "enable eviction debug messages (false =
default)");
+
+/**
+ * DOC: no_system_mem_limit(bool)
+ * Disable system memory limit, to support multiple process shared memory
+ */
+bool no_system_mem_limit;
+module_param(no_system_mem_limit, bool, 0644);
+MODULE_PARM_DESC(no_system_mem_limit, "disable system memory limit (false =
default)");
+
#endif
/**
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: Fix wrong return value in dm_update_plane_state()
On Mon, Aug 3, 2020 at 4:21 AM Tianjia Zhang
wrote:
>
> On an error exit path, a negative error code should be returned
> instead of a positive return value.
>
> Fixes: 9e869063b0021 ("drm/amd/display: Move iteration out of
> dm_update_planes")
> Cc: Leo Li
> Signed-off-by: Tianjia Zhang
Applied. Thanks!
Alex
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +--
> 1 file changed, 1 insertion(+), 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 710edc70e37e..5810416e2ddc 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8182,8 +8182,7 @@ static int dm_update_plane_state(struct dc *dc,
> dm_old_plane_state->dc_state,
> dm_state->context)) {
>
> - ret = EINVAL;
> - return ret;
> + return -EINVAL;
> }
>
>
> --
> 2.26.2
>
> ___
> 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
[PATCH 6/8] drm/amd/display: Set DC options from modifiers.
This sets the DC tiling options from the modifier, if modifiers
are used for the FB. This patch by itself does not expose the
support yet though.
There is not much validation yet to limit the scope of this
patch, but the current validation is at the same level as
the BO metadata path.
Signed-off-by: Bas Nieuwenhuizen
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 109 +-
1 file changed, 103 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 6ef7f2f8acab..ac913b8f10ef 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3754,6 +3754,93 @@ fill_gfx9_plane_attributes_from_flags(struct
amdgpu_device *adev,
return 0;
}
+static bool
+modifier_has_dcc(uint64_t modifier)
+{
+ return IS_AMD_FMT_MOD(modifier) && AMD_FMT_MOD_GET(DCC, modifier);
+}
+
+static unsigned
+modifier_gfx9_swizzle_mode(uint64_t modifier)
+{
+ if (modifier == DRM_FORMAT_MOD_LINEAR)
+ return 0;
+
+ return AMD_FMT_MOD_GET(TILE, modifier);
+}
+
+static void
+fill_gfx9_tiling_info_from_modifier(const struct amdgpu_device *adev,
+ union dc_tiling_info *tiling_info,
+ uint64_t modifier)
+{
+ unsigned int mod_bank_xor_bits = AMD_FMT_MOD_GET(BANK_XOR_BITS,
modifier);
+ unsigned int mod_pipe_xor_bits = AMD_FMT_MOD_GET(PIPE_XOR_BITS,
modifier);
+ unsigned int pkrs_log2 = AMD_FMT_MOD_GET(PACKERS, modifier);
+ unsigned int pipes_log2 = min(4u, mod_pipe_xor_bits);
+
+ fill_gfx9_tiling_info_from_device(adev, tiling_info);
+
+ if (!IS_AMD_FMT_MOD(modifier))
+ return;
+
+ tiling_info->gfx9.num_pipes = 1u << pipes_log2;
+ tiling_info->gfx9.num_shader_engines = 1u << (mod_pipe_xor_bits -
pipes_log2);
+
+ if (adev->family >= AMDGPU_FAMILY_NV) {
+ tiling_info->gfx9.num_pkrs = 1u << pkrs_log2;
+ } else {
+ tiling_info->gfx9.num_banks = 1u << mod_bank_xor_bits;
+
+ /* for DCC we know it isn't rb aligned, so rb_per_se doesn't
matter. */
+ }
+}
+
+static void
+block_alignment(unsigned int blocksize_log2, unsigned int *width, unsigned int
*height)
+{
+ unsigned int height_log2 = blocksize_log2 / 2;
+ unsigned int width_log2 = blocksize_log2 - height_log2;
+
+ *width = 1u << width_log2;
+ *height = 1u << height_log2;
+}
+
+static int
+fill_gfx9_plane_attributes_from_modifiers(struct amdgpu_device *adev,
+ const struct amdgpu_framebuffer *afb,
+ const enum surface_pixel_format format,
+ const enum dc_rotation_angle rotation,
+ const struct plane_size *plane_size,
+ union dc_tiling_info *tiling_info,
+ struct dc_plane_dcc_param *dcc,
+ struct dc_plane_address *address,
+ const bool force_disable_dcc)
+{
+ const uint64_t modifier = afb->base.modifier;
+ int ret;
+
+ fill_gfx9_tiling_info_from_modifier(adev, tiling_info, modifier);
+ tiling_info->gfx9.swizzle = modifier_gfx9_swizzle_mode(modifier);
+
+ if (modifier_has_dcc(modifier) && !force_disable_dcc) {
+ uint64_t dcc_address = afb->address + afb->base.offsets[1];
+
+ dcc->enable = 1;
+ dcc->meta_pitch = afb->base.pitches[1];
+ dcc->independent_64b_blks =
AMD_FMT_MOD_GET(DCC_INDEPENDENT_64B, modifier);
+
+ address->grph.meta_addr.low_part = lower_32_bits(dcc_address);
+ address->grph.meta_addr.high_part = upper_32_bits(dcc_address);
+ }
+
+ ret = validate_dcc(adev, format, rotation, tiling_info, dcc, address,
plane_size);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
static int
fill_plane_buffer_attributes(struct amdgpu_device *adev,
const struct amdgpu_framebuffer *afb,
@@ -3823,12 +3910,22 @@ fill_plane_buffer_attributes(struct amdgpu_device *adev,
if (adev->family >= AMDGPU_FAMILY_AI) {
- ret = fill_gfx9_plane_attributes_from_flags(adev, afb, format,
rotation,
- plane_size,
tiling_info, dcc,
- address,
tiling_flags,
- force_disable_dcc);
- if (ret)
- return ret;
+ if (afb->base.flags & DRM_MODE_FB_MODIFIERS) {
+ ret = fill_gfx9_plane_attributes_from_modifiers(adev,
afb, format,
+
[PATCH 0/8] amd/display: Add GFX9+ modifier support.
This adds modifier support to radeonsi. It has been tested on - VEGA10, RAVEN, NAVI14 - weston, sway, X with xf86-video-amdgpu (i.e. legacy path still works) and includes some basic testing of the layout code. The main goal is to keep it somewhat simple and regression free, so on the display side this series only exposes what the current GPU can render to. While we could expose more I think that is more suitable for follow-up work as the benefit would be minimal and there are some more design discussion there to discuss that are orthogonal from the initial implementation. Similarly this series only exposes 32-bpp displayable DCC in the cases that radeonsi would use it and any extra capabilities here should be future work. I believe these are by far the most complicated modifiers we've seen up till now, mostly related to - GPU identification for cases where it matters wrt tiling. - Every generation having tiling layout changes - Compression settings. I believe the complexity is necessary as every layout should be different and all layouts should be the best in some situation (though not all combinations of GPU parameters will actually have an existing GPU). That said, on the render side the number of modifiers actually listed for a given GPU is ~10, and in the current implementation that is the same for the display side. (we could expose more actually differing layouts on the display side for cross-GPU compatibility, but I consider that out of scope for this initial work). This series can be found on https://github.com/BNieuwenhuizen/linux/tree/modifiers An userspace implementation in radeonsi can be found on https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6176 Bas Nieuwenhuizen (8): drm/amd/display: Do not silently accept DCC for multiplane formats. drm/amd: Init modifier field of helper fb. drm/amd/display: Honor the offset for plane 0. drm/fourcc: Add AMD DRM modifiers. drm/amd/display: Refactor surface tiling setup. drm/amd/display: Set DC options from modifiers. drm/amd/display: Add formats for DCC with 2/3 planes. drm/amd/display: Expose modifiers. drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c| 2 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 758 +++--- include/uapi/drm/drm_fourcc.h | 115 +++ 3 files changed, 775 insertions(+), 100 deletions(-) -- 2.28.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 8/8] drm/amd/display: Expose modifiers.
This expose modifier support on GFX9+.
Only modifiers that can be rendered on the current GPU are
added. This is to reduce the number of modifiers exposed.
The HW could expose more, but the best mechanism to decide
what to expose without an explosion in modifiers is still
to be decided, and in the meantime this should not regress
things from pre-modifiers and does not risk regressions as
we make up our mind in the future.
Signed-off-by: Bas Nieuwenhuizen
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 343 +-
1 file changed, 342 insertions(+), 1 deletion(-)
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 c38257081868..6594cbe625f9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3891,6 +3891,340 @@ fill_gfx9_tiling_info_from_modifier(const struct
amdgpu_device *adev,
}
}
+enum dm_micro_swizzle {
+ MICRO_SWIZZLE_Z = 0,
+ MICRO_SWIZZLE_S = 1,
+ MICRO_SWIZZLE_D = 2,
+ MICRO_SWIZZLE_R = 3
+};
+
+static bool dm_plane_format_mod_supported(struct drm_plane *plane,
+ uint32_t format,
+ uint64_t modifier)
+{
+ struct amdgpu_device *adev = plane->dev->dev_private;
+ const struct drm_format_info *info = drm_format_info(format);
+
+ enum dm_micro_swizzle microtile = modifier_gfx9_swizzle_mode(modifier)
& 3;
+
+ if (!info)
+ return false;
+
+ /*
+* We always have to allow this modifier, because core DRM still
+* checks LINEAR support if userspace does not provide modifers.
+*/
+ if (modifier == DRM_FORMAT_MOD_LINEAR)
+ return true;
+
+ /*
+* The arbitrary tiling support for multiplane formats has not been
hooked
+* up.
+*/
+ if (info->num_planes > 1)
+ return false;
+
+ /*
+* For D swizzle the canonical modifier depends on the bpp, so check
+* it here.
+*/
+ if (AMD_FMT_MOD_GET(TILE_VERSION, modifier) ==
AMD_FMT_MOD_TILE_VER_GFX9 &&
+ adev->family >= AMDGPU_FAMILY_NV) {
+ if (microtile == MICRO_SWIZZLE_D && info->cpp[0] == 4)
+ return false;
+ }
+
+ if (adev->family >= AMDGPU_FAMILY_RV && microtile == MICRO_SWIZZLE_D &&
+ info->cpp[0] < 8)
+ return false;
+
+ if (modifier_has_dcc(modifier)) {
+ /* Per radeonsi comments 16/64 bpp are more complicated. */
+ if (info->cpp[0] != 4)
+ return false;
+ }
+
+ return true;
+}
+
+static void
+add_modifier(uint64_t **mods, uint64_t *size, uint64_t *cap, uint64_t mod)
+{
+ if (!*mods)
+ return;
+
+ if (*cap - *size < 1) {
+ uint64_t new_cap = *cap * 2;
+ uint64_t *new_mods = kmalloc(new_cap * sizeof(uint64_t),
GFP_KERNEL);
+
+ if (!new_mods) {
+ kfree(*mods);
+ *mods = NULL;
+ return;
+ }
+
+ memcpy(new_mods, *mods, sizeof(uint64_t) * *size);
+ kfree(*mods);
+ *mods = new_mods;
+ *cap = new_cap;
+ }
+
+ (*mods)[*size] = mod;
+ *size += 1;
+}
+
+static void
+add_gfx9_modifiers(const struct amdgpu_device *adev,
+ uint64_t **mods, uint64_t *size, uint64_t *capacity)
+{
+ int pipes = ilog2(adev->gfx.config.gb_addr_config_fields.num_pipes);
+ int pipe_xor_bits = min(8, pipes +
+
ilog2(adev->gfx.config.gb_addr_config_fields.num_se));
+ int bank_xor_bits = min(8 - pipe_xor_bits,
+
ilog2(adev->gfx.config.gb_addr_config_fields.num_banks));
+ int rb = ilog2(adev->gfx.config.gb_addr_config_fields.num_se) +
+ilog2(adev->gfx.config.gb_addr_config_fields.num_rb_per_se);
+
+
+ if (adev->family == AMDGPU_FAMILY_RV) {
+ /*
+* No _D DCC swizzles yet because we only allow 32bpp, which
+* doesn't support _D on DCN
+*/
+
+ /*
+* Always enable constant encoding, because the only unit that
+* didn't support it was CB. But on texture/display we can
+* always interpret it.
+*/
+ add_modifier(mods, size, capacity, AMD_FMT_MOD |
+ AMD_FMT_MOD_SET(TILE,
AMD_FMT_MOD_TILE_GFX9_64K_S_X) |
+ AMD_FMT_MOD_SET(TILE_VERSION,
AMD_FMT_MOD_TILE_VER_GFX9) |
+ AMD_FMT_MOD_SET(PIPE_XOR_BITS, pipe_xor_bits) |
+ AMD_FMT_MOD_SET(BANK_XOR_BITS, bank_xor_bits) |
+ AMD_FMT_MOD_SET(DCC, 1) |
+
[PATCH 5/8] drm/amd/display: Refactor surface tiling setup.
Prepare for inserting modifiers based configuration, while sharing
a bunch of DCC validation & initializing the device-based configuration.
Signed-off-by: Bas Nieuwenhuizen
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 209 ++
1 file changed, 116 insertions(+), 93 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 abc70fbe176d..6ef7f2f8acab 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3601,46 +3601,83 @@ static int get_fb_info(const struct amdgpu_framebuffer
*amdgpu_fb,
return r;
}
-static inline uint64_t get_dcc_address(uint64_t address, uint64_t tiling_flags)
+static void
+fill_gfx8_tiling_info_from_flags(union dc_tiling_info *tiling_info,
+uint64_t tiling_flags)
{
- uint32_t offset = AMDGPU_TILING_GET(tiling_flags, DCC_OFFSET_256B);
+ /* Fill GFX8 params */
+ if (AMDGPU_TILING_GET(tiling_flags, ARRAY_MODE) ==
DC_ARRAY_2D_TILED_THIN1) {
+ unsigned int bankw, bankh, mtaspect, tile_split, num_banks;
+
+ bankw = AMDGPU_TILING_GET(tiling_flags, BANK_WIDTH);
+ bankh = AMDGPU_TILING_GET(tiling_flags, BANK_HEIGHT);
+ mtaspect = AMDGPU_TILING_GET(tiling_flags, MACRO_TILE_ASPECT);
+ tile_split = AMDGPU_TILING_GET(tiling_flags, TILE_SPLIT);
+ num_banks = AMDGPU_TILING_GET(tiling_flags, NUM_BANKS);
- return offset ? (address + offset * 256) : 0;
+ /* XXX fix me for VI */
+ tiling_info->gfx8.num_banks = num_banks;
+ tiling_info->gfx8.array_mode =
+ DC_ARRAY_2D_TILED_THIN1;
+ tiling_info->gfx8.tile_split = tile_split;
+ tiling_info->gfx8.bank_width = bankw;
+ tiling_info->gfx8.bank_height = bankh;
+ tiling_info->gfx8.tile_aspect = mtaspect;
+ tiling_info->gfx8.tile_mode =
+ DC_ADDR_SURF_MICRO_TILING_DISPLAY;
+ } else if (AMDGPU_TILING_GET(tiling_flags, ARRAY_MODE)
+ == DC_ARRAY_1D_TILED_THIN1) {
+ tiling_info->gfx8.array_mode = DC_ARRAY_1D_TILED_THIN1;
+ }
+
+ tiling_info->gfx8.pipe_config =
+ AMDGPU_TILING_GET(tiling_flags, PIPE_CONFIG);
+}
+
+static void
+fill_gfx9_tiling_info_from_device(const struct amdgpu_device *adev,
+ union dc_tiling_info *tiling_info)
+{
+ tiling_info->gfx9.num_pipes =
+ adev->gfx.config.gb_addr_config_fields.num_pipes;
+ tiling_info->gfx9.num_banks =
+ adev->gfx.config.gb_addr_config_fields.num_banks;
+ tiling_info->gfx9.pipe_interleave =
+ adev->gfx.config.gb_addr_config_fields.pipe_interleave_size;
+ tiling_info->gfx9.num_shader_engines =
+ adev->gfx.config.gb_addr_config_fields.num_se;
+ tiling_info->gfx9.max_compressed_frags =
+ adev->gfx.config.gb_addr_config_fields.max_compress_frags;
+ tiling_info->gfx9.num_rb_per_se =
+ adev->gfx.config.gb_addr_config_fields.num_rb_per_se;
+ tiling_info->gfx9.shaderEnable = 1;
+#ifdef CONFIG_DRM_AMD_DC_DCN3_0
+ if (adev->asic_type == CHIP_SIENNA_CICHLID)
+ tiling_info->gfx9.num_pkrs =
adev->gfx.config.gb_addr_config_fields.num_pkrs;
+#endif
}
static int
-fill_plane_dcc_attributes(struct amdgpu_device *adev,
- const struct amdgpu_framebuffer *afb,
- const enum surface_pixel_format format,
- const enum dc_rotation_angle rotation,
- const struct plane_size *plane_size,
- const union dc_tiling_info *tiling_info,
- const uint64_t info,
- struct dc_plane_dcc_param *dcc,
- struct dc_plane_address *address,
- bool force_disable_dcc)
+validate_dcc(struct amdgpu_device *adev,
+const enum surface_pixel_format format,
+const enum dc_rotation_angle rotation,
+const union dc_tiling_info *tiling_info,
+const struct dc_plane_dcc_param *dcc,
+const struct dc_plane_address *address,
+const struct plane_size *plane_size)
{
struct dc *dc = adev->dm.dc;
struct dc_dcc_surface_param input;
struct dc_surface_dcc_cap output;
- uint64_t plane_address = afb->address + afb->base.offsets[0];
- uint32_t offset = AMDGPU_TILING_GET(info, DCC_OFFSET_256B);
- uint32_t i64b = AMDGPU_TILING_GET(info, DCC_INDEPENDENT_64B) != 0;
- uint64_t dcc_address;
memset(, 0, sizeof(input));
memset(, 0, sizeof(output));
- if (force_disable_dcc)
- return 0;
-
-
[PATCH 1/8] drm/amd/display: Do not silently accept DCC for multiplane formats.
Silently accepting it could result in corruption. Signed-off-by: Bas Nieuwenhuizen --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f348693217d8..005331c772b7 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -3637,7 +3637,7 @@ fill_plane_dcc_attributes(struct amdgpu_device *adev, return 0; if (format >= SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) - return 0; + return -EINVAL; if (!dc->cap_funcs.get_dcc_compression_cap) return -EINVAL; -- 2.28.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 4/8] drm/fourcc: Add AMD DRM modifiers.
This adds modifiers for GFX9+ AMD GPUs.
As the modifiers need a lot of parameters I split things out in
getters and setters.
- Advantage: simplifies the code a lot
- Disadvantage: Makes it harder to check that you're setting all
the required fields.
The tiling modes seem to change every generatio, but the structure
of what each tiling mode is good for stays really similar. As such
the core of the modifier is
- the tiling mode
- a version. Not explicitly a GPU generation, but splitting out
a new set of tiling equations.
Sometimes one or two tiling modes stay the same and for those we
specify a canonical version.
Then we have a bunch of parameters on how the compression works.
Different HW units have different requirements for these and we
actually have some conflicts here.
e.g. the render backends need a specific alignment but the display
unit only works with unaligned compression surfaces. To work around
that we have a DCC_RETILE option where both an aligned and unaligned
compression surface are allocated and a writer has to sync the
aligned surface to the unaligned surface on handoff.
Finally there are some GPU parameters that participate in the tiling
equations. These are constant for each GPU on the rendering/texturing
side. The display unit is very flexible however and supports all
of them :|
Some estimates:
- Single GPU, render+texture: ~10 modifiers
- All possible configs in a gen, display: ~1000 modifiers
- Configs of actually existing GPUs in a gen: ~100 modifiers
For formats with a single plane everything gets put in a separate
DRM plane. However, this doesn't fit for some YUV formats, so if
the format has >1 plane, we let the driver pack the surfaces into
1 DRM plane per format plane.
This way we avoid X11 rendering onto the frontbuffer with DCC, but
still fit into 4 DRM planes.
Signed-off-by: Bas Nieuwenhuizen
---
include/uapi/drm/drm_fourcc.h | 115 ++
1 file changed, 115 insertions(+)
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 8bc0b31597d8..2f8d2b717285 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -804,6 +804,121 @@ extern "C" {
*/
#define DRM_FORMAT_MOD_ALLWINNER_TILED fourcc_mod_code(ALLWINNER, 1)
+/*
+ * AMD modifiers
+ *
+ * Memory layout:
+ *
+ * without DCC:
+ * - main surface
+ *
+ * with DCC & without DCC_RETILE:
+ * - main surface in plane 0
+ * - DCC surface in plane 1 (RB-aligned, pipe-aligned if DCC_PIPE_ALIGN is
set)
+ *
+ * with DCC & DCC_RETILE:
+ * - main surface in plane 0
+ * - displayable DCC surface in plane 1 (not RB-aligned & not pipe-aligned)
+ * - pipe-aligned DCC surface in plane 2 (RB-aligned & pipe-aligned)
+ *
+ * For multi-plane formats the above surfaces get merged into one plane for
+ * each for format plane, based on the required alignment only.
+ */
+#define AMD_FMT_MOD fourcc_mod_code(AMD, 0)
+
+#define IS_AMD_FMT_MOD(val) (((val) >> 56) == DRM_FORMAT_MOD_VENDOR_AMD)
+
+/* Reserve 0 for GFX8 and older */
+#define AMD_FMT_MOD_TILE_VER_GFX9 1
+#define AMD_FMT_MOD_TILE_VER_GFX10 2
+#define AMD_FMT_MOD_TILE_VER_GFX10_RBPLUS 3
+
+/*
+ * 64K_S is the same for GFX9/GFX10/GFX10_RBPLUS and hence has GFX9 as
canonical
+ * version.
+ */
+#define AMD_FMT_MOD_TILE_GFX9_64K_S 9
+
+/*
+ * 64K_D for non-32 bpp is the same for GFX9/GFX10/GFX10_RBPLUS and hence has
+ * GFX9 as canonical version.
+ */
+#define AMD_FMT_MOD_TILE_GFX9_64K_D 10
+#define AMD_FMT_MOD_TILE_GFX9_64K_S_X 25
+#define AMD_FMT_MOD_TILE_GFX9_64K_D_X 26
+#define AMD_FMT_MOD_TILE_GFX9_64K_R_X 27
+
+#define AMD_FMT_MOD_DCC_BLOCK_64B 0
+#define AMD_FMT_MOD_DCC_BLOCK_128B 1
+#define AMD_FMT_MOD_DCC_BLOCK_256B 2
+
+#define AMD_FMT_MOD_TILE_VERSION_SHIFT 0
+#define AMD_FMT_MOD_TILE_VERSION_MASK 0xFF
+#define AMD_FMT_MOD_TILE_SHIFT 8
+#define AMD_FMT_MOD_TILE_MASK 0x1F
+
+/* Whether DCC compression is enabled. */
+#define AMD_FMT_MOD_DCC_SHIFT 13
+#define AMD_FMT_MOD_DCC_MASK 0x1
+
+/*
+ * Whether to include two DCC surfaces, one which is rb & pipe aligned, and
+ * one which is not-aligned.
+ */
+#define AMD_FMT_MOD_DCC_RETILE_SHIFT 14
+#define AMD_FMT_MOD_DCC_RETILE_MASK 0x1
+
+/* Only set if DCC_RETILE = false */
+#define AMD_FMT_MOD_DCC_PIPE_ALIGN_SHIFT 15
+#define AMD_FMT_MOD_DCC_PIPE_ALIGN_MASK 0x1
+
+#define AMD_FMT_MOD_DCC_INDEPENDENT_64B_SHIFT 16
+#define AMD_FMT_MOD_DCC_INDEPENDENT_64B_MASK 0x1
+#define AMD_FMT_MOD_DCC_INDEPENDENT_128B_SHIFT 17
+#define AMD_FMT_MOD_DCC_INDEPENDENT_128B_MASK 0x1
+#define AMD_FMT_MOD_DCC_MAX_COMPRESSED_BLOCK_SHIFT 18
+#define AMD_FMT_MOD_DCC_MAX_COMPRESSED_BLOCK_MASK 0x1
+
+/*
+ * DCC supports embedding some clear colors directly in the DCC surface.
+ * However, on older GPUs the rendering HW ignores the embedded clear color
+ * and prefers the driver provided color. This necessitates doing a fastclear
+ * eliminate operation before a process transfers control.
+ *
+ * If this bit is set that means the
[PATCH 2/8] drm/amd: Init modifier field of helper fb.
Otherwise the field ends up being used uninitialized when
enabling modifiers, failing validation with high likelihood.
Signed-off-by: Bas Nieuwenhuizen
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 25ddb482466a..c2d6952d0a7d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -201,7 +201,7 @@ static int amdgpufb_create(struct drm_fb_helper *helper,
struct amdgpu_device *adev = rfbdev->adev;
struct fb_info *info;
struct drm_framebuffer *fb = NULL;
- struct drm_mode_fb_cmd2 mode_cmd;
+ struct drm_mode_fb_cmd2 mode_cmd = {0};
struct drm_gem_object *gobj = NULL;
struct amdgpu_bo *abo = NULL;
int ret;
--
2.28.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 3/8] drm/amd/display: Honor the offset for plane 0.
With modifiers I'd like to support non-dedicated buffers for
images.
Signed-off-by: Bas Nieuwenhuizen
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 +-
1 file changed, 9 insertions(+), 5 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 005331c772b7..abc70fbe176d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3623,6 +3623,7 @@ fill_plane_dcc_attributes(struct amdgpu_device *adev,
struct dc *dc = adev->dm.dc;
struct dc_dcc_surface_param input;
struct dc_surface_dcc_cap output;
+ uint64_t plane_address = afb->address + afb->base.offsets[0];
uint32_t offset = AMDGPU_TILING_GET(info, DCC_OFFSET_256B);
uint32_t i64b = AMDGPU_TILING_GET(info, DCC_INDEPENDENT_64B) != 0;
uint64_t dcc_address;
@@ -3666,7 +3667,7 @@ fill_plane_dcc_attributes(struct amdgpu_device *adev,
AMDGPU_TILING_GET(info, DCC_PITCH_MAX) + 1;
dcc->independent_64b_blks = i64b;
- dcc_address = get_dcc_address(afb->address, info);
+ dcc_address = get_dcc_address(plane_address, info);
address->grph.meta_addr.low_part = lower_32_bits(dcc_address);
address->grph.meta_addr.high_part = upper_32_bits(dcc_address);
@@ -3697,6 +3698,8 @@ fill_plane_buffer_attributes(struct amdgpu_device *adev,
address->tmz_surface = tmz_surface;
if (format < SURFACE_PIXEL_FORMAT_VIDEO_BEGIN) {
+ uint64_t addr = afb->address + fb->offsets[0];
+
plane_size->surface_size.x = 0;
plane_size->surface_size.y = 0;
plane_size->surface_size.width = fb->width;
@@ -3705,9 +3708,10 @@ fill_plane_buffer_attributes(struct amdgpu_device *adev,
fb->pitches[0] / fb->format->cpp[0];
address->type = PLN_ADDR_TYPE_GRAPHICS;
- address->grph.addr.low_part = lower_32_bits(afb->address);
- address->grph.addr.high_part = upper_32_bits(afb->address);
+ address->grph.addr.low_part = lower_32_bits(addr);
+ address->grph.addr.high_part = upper_32_bits(addr);
} else if (format < SURFACE_PIXEL_FORMAT_INVALID) {
+ uint64_t luma_addr = afb->address + fb->offsets[0];
uint64_t chroma_addr = afb->address + fb->offsets[1];
plane_size->surface_size.x = 0;
@@ -3728,9 +3732,9 @@ fill_plane_buffer_attributes(struct amdgpu_device *adev,
address->type = PLN_ADDR_TYPE_VIDEO_PROGRESSIVE;
address->video_progressive.luma_addr.low_part =
- lower_32_bits(afb->address);
+ lower_32_bits(luma_addr);
address->video_progressive.luma_addr.high_part =
- upper_32_bits(afb->address);
+ upper_32_bits(luma_addr);
address->video_progressive.chroma_addr.low_part =
lower_32_bits(chroma_addr);
address->video_progressive.chroma_addr.high_part =
--
2.28.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 7/8] drm/amd/display: Add formats for DCC with 2/3 planes.
For DCC we will use 2/3 planes to avoid X rendering to the frontbuffer
with DCC compressed images. To make this work with the core KMS
validation we need to add extra formats with the extra planes.
However, due to flexibility we set bpp = 0 for the extra planes and
do the validation ourselves.
Signed-off-by: Bas Nieuwenhuizen
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 95 +++
1 file changed, 95 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 ac913b8f10ef..c38257081868 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -170,6 +170,8 @@ static bool amdgpu_dm_psr_enable(struct dc_stream_state
*stream);
static bool amdgpu_dm_link_setup_psr(struct dc_stream_state *stream);
static bool amdgpu_dm_psr_disable(struct dc_stream_state *stream);
+static const struct drm_format_info *
+amd_get_format_info(const struct drm_mode_fb_cmd2 *cmd);
/*
* dm_vblank_get_counter
@@ -2007,6 +2009,7 @@ const struct amdgpu_ip_block_version dm_ip_block =
static const struct drm_mode_config_funcs amdgpu_dm_mode_funcs = {
.fb_create = amdgpu_display_user_framebuffer_create,
+ .get_format_info = amd_get_format_info,
.output_poll_changed = drm_fb_helper_output_poll_changed,
.atomic_check = amdgpu_dm_atomic_check,
.atomic_commit = amdgpu_dm_atomic_commit,
@@ -3769,6 +3772,98 @@ modifier_gfx9_swizzle_mode(uint64_t modifier)
return AMD_FMT_MOD_GET(TILE, modifier);
}
+static const struct drm_format_info dcc_formats[] = {
+ { .format = DRM_FORMAT_XRGB, .depth = 24, .num_planes = 2,
+ .cpp = { 4, 0, }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub =
1, .vsub = 1, },
+{ .format = DRM_FORMAT_XBGR, .depth = 24, .num_planes = 2,
+ .cpp = { 4, 0, }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub =
1, .vsub = 1, },
+ { .format = DRM_FORMAT_ARGB, .depth = 32, .num_planes = 2,
+ .cpp = { 4, 0, }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub =
1, .vsub = 1,
+ .has_alpha = true, },
+ { .format = DRM_FORMAT_ABGR, .depth = 32, .num_planes = 2,
+ .cpp = { 4, 0, }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub =
1, .vsub = 1,
+ .has_alpha = true, },
+ { .format = DRM_FORMAT_BGRA, .depth = 32, .num_planes = 2,
+ .cpp = { 4, 0, }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub =
1, .vsub = 1,
+ .has_alpha = true, },
+ { .format = DRM_FORMAT_XRGB2101010, .depth = 30, .num_planes = 2,
+ .cpp = { 4, 0, }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub =
1, .vsub = 1, },
+ { .format = DRM_FORMAT_XBGR2101010, .depth = 30, .num_planes = 2,
+ .cpp = { 4, 0, }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub =
1, .vsub = 1, },
+ { .format = DRM_FORMAT_ARGB2101010, .depth = 30, .num_planes = 2,
+ .cpp = { 4, 0, }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub =
1, .vsub = 1,
+ .has_alpha = true, },
+ { .format = DRM_FORMAT_ABGR2101010, .depth = 30, .num_planes = 2,
+ .cpp = { 4, 0, }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub =
1, .vsub = 1,
+ .has_alpha = true, },
+ { .format = DRM_FORMAT_RGB565, .depth = 16, .num_planes = 2,
+ .cpp = { 2, 0, }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub =
1, .vsub = 1, },
+};
+
+static const struct drm_format_info dcc_retile_formats[] = {
+ { .format = DRM_FORMAT_XRGB, .depth = 24, .num_planes = 3,
+ .cpp = { 4, 0, 0 }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub
= 1, .vsub = 1, },
+{ .format = DRM_FORMAT_XBGR, .depth = 24, .num_planes = 3,
+ .cpp = { 4, 0, 0 }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub
= 1, .vsub = 1, },
+ { .format = DRM_FORMAT_ARGB, .depth = 32, .num_planes = 3,
+ .cpp = { 4, 0, 0 }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub
= 1, .vsub = 1,
+ .has_alpha = true, },
+ { .format = DRM_FORMAT_ABGR, .depth = 32, .num_planes = 3,
+ .cpp = { 4, 0, 0 }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub
= 1, .vsub = 1,
+ .has_alpha = true, },
+ { .format = DRM_FORMAT_BGRA, .depth = 32, .num_planes = 3,
+ .cpp = { 4, 0, 0 }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub
= 1, .vsub = 1,
+ .has_alpha = true, },
+ { .format = DRM_FORMAT_XRGB2101010, .depth = 30, .num_planes = 3,
+ .cpp = { 4, 0, 0 }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub
= 1, .vsub = 1, },
+ { .format = DRM_FORMAT_XBGR2101010, .depth = 30, .num_planes = 3,
+ .cpp = { 4, 0, 0 }, .block_w = {1, 1, 1}, .block_h = {1, 1, 1}, .hsub
= 1, .vsub = 1, },
+ { .format = DRM_FORMAT_ARGB2101010, .depth = 30, .num_planes = 3,
+ .cpp = { 4, 0, 0 }, .block_w = {1, 1, 1},
Re: [PATCH v2] drm/amdkfd: option to disable system mem limit
Am 2020-08-04 um 4:42 p.m. schrieb Philip Yang:
> If multiple process share system memory through /dev/shm, KFD allocate
> memory should not fail if it reaches the system memory limit because
> one copy of physical system memory are shared by multiple process.
>
> Add module parameter no_system_mem_limit to provide user option to
> disable system memory limit check at runtime using sysfs or during
> driver module init using kernel boot argument. By default the system
> memory limit is on.
>
> Print out debug message to warn user if KFD allocate memory failed
> because system memory reaches limit.
>
> v2: support change setting at runtime using sysfs
>
> Signed-off-by: Philip Yang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 12 +++-
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
> 3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 4e8622854e61..0459e53f5917 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -188,9 +188,11 @@ extern int amdgpu_force_asic_type;
> #ifdef CONFIG_HSA_AMD
> extern int sched_policy;
> extern bool debug_evictions;
> +extern bool no_system_mem_limit;
> #else
> static const int sched_policy = KFD_SCHED_POLICY_HWS;
> static const bool debug_evictions; /* = false */
> +static const bool no_system_mem_limit;
> #endif
>
> extern int amdgpu_tmz;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 8703aa1fe4a5..be29b1e8606d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -136,7 +136,10 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
> amdgpu_device *adev,
> ttm_mem_needed = acc_size + size;
> } else if (domain == AMDGPU_GEM_DOMAIN_CPU && !sg) {
> /* Userptr */
> - system_mem_needed = acc_size + size;
> + if (no_system_mem_limit)
> + system_mem_needed = 0;
> + else
> + system_mem_needed = acc_size + size;
This will give incorrect results if the user changes the setting at
runtime. It would be better to do all the accounting correctly but let
the module parameter only affect the condition that checks whether the
reservation is OK.
E.g.
if ((kfd_mem_limit.system_mem_used + system_mem_needed >
kfd_mem_limit.max_system_mem_limit && !no_system_mem_limit) ||
/* ^- */
(kfd_mem_limit.ttm_mem_used + ttm_mem_needed >
kfd_mem_limit.max_ttm_mem_limit) ||
(adev->kfd.vram_used + vram_needed >
adev->gmc.real_vram_size - reserved_for_pt)) {
ret = -ENOMEM;
} ...
> ttm_mem_needed = acc_size;
> } else {
> /* VRAM and SG */
> @@ -148,6 +151,10 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
> amdgpu_device *adev,
>
> spin_lock(_mem_limit.mem_limit_lock);
>
> + if (kfd_mem_limit.system_mem_used + system_mem_needed >
> + kfd_mem_limit.max_system_mem_limit)
> + pr_debug("Set no_system_mem_limit=1 if using shared memory\n");
> +
> if ((kfd_mem_limit.system_mem_used + system_mem_needed >
>kfd_mem_limit.max_system_mem_limit) ||
> (kfd_mem_limit.ttm_mem_used + ttm_mem_needed >
> @@ -204,6 +211,9 @@ void amdgpu_amdkfd_unreserve_memory_limit(struct
> amdgpu_bo *bo)
> bool sg = (bo->preferred_domains == AMDGPU_GEM_DOMAIN_CPU);
>
> if (bo->flags & AMDGPU_AMDKFD_USERPTR_BO) {
> + if (no_system_mem_limit)
> + return;
> +
Same as above. This results in incorrect accounting when the user
changes the setting at runtime.
> domain = AMDGPU_GEM_DOMAIN_CPU;
> sg = false;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index a252450734f6..d3bd7a7da174 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -717,6 +717,15 @@ MODULE_PARM_DESC(queue_preemption_timeout_ms, "queue
> preemption timeout in ms (1
> bool debug_evictions;
> module_param(debug_evictions, bool, 0644);
> MODULE_PARM_DESC(debug_evictions, "enable eviction debug messages (false =
> default)");
> +
> +/**
> + * DOC: no_system_mem_limit(bool)
> + * Disable system memory limit, to support multiple process shared memory
> + */
> +bool no_system_mem_limit;
> +module_param(no_system_mem_limit, bool, 0644);
> +MODULE_PARM_DESC(no_system_mem_limit, "disable system memory limit (false =
> default)");
> +
> #endif
>
> /**
___
amd-gfx mailing list
Re: [PATCH 0/3] drm/amd/display: Constify static resource_funcs
Applied the series. Thanks! Alex On Tue, Aug 4, 2020 at 4:08 PM Rikard Falkeborn wrote: > > Constify a couple of instances of resource_funcs that are never > modified to allow the compiler to put it in read-only memory. > > The other drivers in drivers/gpu/drm/amd/display/dc already have > these as const. > > Rikard Falkeborn (3): > drm/amd/display: Constify dcn20_res_pool_funcs > drm/amd/display: Constify dcn21_res_pool_funcs > drm/amd/display: Constify dcn30_res_pool_funcs > > drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 2 +- > drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c | 2 +- > drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > -- > 2.28.0 > > ___ > 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 v2] drm/amdkfd: option to disable system mem limit
If multiple process share system memory through /dev/shm, KFD allocate
memory should not fail if it reaches the system memory limit because
one copy of physical system memory are shared by multiple process.
Add module parameter no_system_mem_limit to provide user option to
disable system memory limit check at runtime using sysfs or during
driver module init using kernel boot argument. By default the system
memory limit is on.
Print out debug message to warn user if KFD allocate memory failed
because system memory reaches limit.
v2: support change setting at runtime using sysfs
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 12 +++-
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 4e8622854e61..0459e53f5917 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -188,9 +188,11 @@ extern int amdgpu_force_asic_type;
#ifdef CONFIG_HSA_AMD
extern int sched_policy;
extern bool debug_evictions;
+extern bool no_system_mem_limit;
#else
static const int sched_policy = KFD_SCHED_POLICY_HWS;
static const bool debug_evictions; /* = false */
+static const bool no_system_mem_limit;
#endif
extern int amdgpu_tmz;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 8703aa1fe4a5..be29b1e8606d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -136,7 +136,10 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
amdgpu_device *adev,
ttm_mem_needed = acc_size + size;
} else if (domain == AMDGPU_GEM_DOMAIN_CPU && !sg) {
/* Userptr */
- system_mem_needed = acc_size + size;
+ if (no_system_mem_limit)
+ system_mem_needed = 0;
+ else
+ system_mem_needed = acc_size + size;
ttm_mem_needed = acc_size;
} else {
/* VRAM and SG */
@@ -148,6 +151,10 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
amdgpu_device *adev,
spin_lock(_mem_limit.mem_limit_lock);
+ if (kfd_mem_limit.system_mem_used + system_mem_needed >
+ kfd_mem_limit.max_system_mem_limit)
+ pr_debug("Set no_system_mem_limit=1 if using shared memory\n");
+
if ((kfd_mem_limit.system_mem_used + system_mem_needed >
kfd_mem_limit.max_system_mem_limit) ||
(kfd_mem_limit.ttm_mem_used + ttm_mem_needed >
@@ -204,6 +211,9 @@ void amdgpu_amdkfd_unreserve_memory_limit(struct amdgpu_bo
*bo)
bool sg = (bo->preferred_domains == AMDGPU_GEM_DOMAIN_CPU);
if (bo->flags & AMDGPU_AMDKFD_USERPTR_BO) {
+ if (no_system_mem_limit)
+ return;
+
domain = AMDGPU_GEM_DOMAIN_CPU;
sg = false;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index a252450734f6..d3bd7a7da174 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -717,6 +717,15 @@ MODULE_PARM_DESC(queue_preemption_timeout_ms, "queue
preemption timeout in ms (1
bool debug_evictions;
module_param(debug_evictions, bool, 0644);
MODULE_PARM_DESC(debug_evictions, "enable eviction debug messages (false =
default)");
+
+/**
+ * DOC: no_system_mem_limit(bool)
+ * Disable system memory limit, to support multiple process shared memory
+ */
+bool no_system_mem_limit;
+module_param(no_system_mem_limit, bool, 0644);
+MODULE_PARM_DESC(no_system_mem_limit, "disable system memory limit (false =
default)");
+
#endif
/**
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 17/17] drm/amd/powerplay: add control method to bypass metrics cache on Vega12
Do we want the metrics cache at all? I can see arguments both ways.
Patches 12-17 are:
Reviewed-by: Alex Deucher
On Thu, Jul 30, 2020 at 10:45 PM Evan Quan wrote:
>
> As for the gpu metric export, metrics cache makes no sense. It's up to
> user to decide how often the metrics should be retrieved.
>
> Change-Id: Ic2a27ebc90f0a7cf581d0697c121b6d7df030f3b
> Signed-off-by: Evan Quan
> ---
> .../drm/amd/powerplay/hwmgr/vega12_hwmgr.c| 29 ---
> 1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> index 40bb0c2e4e8c..c70c30175801 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> @@ -1262,22 +1262,29 @@ static uint32_t vega12_dpm_get_mclk(struct pp_hwmgr
> *hwmgr, bool low)
> return (mem_clk * 100);
> }
>
> -static int vega12_get_metrics_table(struct pp_hwmgr *hwmgr, SmuMetrics_t
> *metrics_table)
> +static int vega12_get_metrics_table(struct pp_hwmgr *hwmgr,
> + SmuMetrics_t *metrics_table,
> + bool bypass_cache)
> {
> struct vega12_hwmgr *data =
> (struct vega12_hwmgr *)(hwmgr->backend);
> int ret = 0;
>
> - if (!data->metrics_time || time_after(jiffies, data->metrics_time +
> HZ / 2)) {
> - ret = smum_smc_table_manager(hwmgr, (uint8_t *)metrics_table,
> - TABLE_SMU_METRICS, true);
> + if (bypass_cache ||
> + !data->metrics_time ||
> + time_after(jiffies, data->metrics_time + HZ / 2)) {
> + ret = smum_smc_table_manager(hwmgr,
> +(uint8_t
> *)(>metrics_table),
> +TABLE_SMU_METRICS,
> +true);
> if (ret) {
> pr_info("Failed to export SMU metrics table!\n");
> return ret;
> }
> - memcpy(>metrics_table, metrics_table,
> sizeof(SmuMetrics_t));
> data->metrics_time = jiffies;
> - } else
> + }
> +
> + if (metrics_table)
> memcpy(metrics_table, >metrics_table,
> sizeof(SmuMetrics_t));
>
> return ret;
> @@ -1288,7 +1295,7 @@ static int vega12_get_gpu_power(struct pp_hwmgr *hwmgr,
> uint32_t *query)
> SmuMetrics_t metrics_table;
> int ret = 0;
>
> - ret = vega12_get_metrics_table(hwmgr, _table);
> + ret = vega12_get_metrics_table(hwmgr, _table, false);
> if (ret)
> return ret;
>
> @@ -1339,7 +1346,7 @@ static int vega12_get_current_activity_percent(
> SmuMetrics_t metrics_table;
> int ret = 0;
>
> - ret = vega12_get_metrics_table(hwmgr, _table);
> + ret = vega12_get_metrics_table(hwmgr, _table, false);
> if (ret)
> return ret;
>
> @@ -1387,7 +1394,7 @@ static int vega12_read_sensor(struct pp_hwmgr *hwmgr,
> int idx,
> *size = 4;
> break;
> case AMDGPU_PP_SENSOR_HOTSPOT_TEMP:
> - ret = vega12_get_metrics_table(hwmgr, _table);
> + ret = vega12_get_metrics_table(hwmgr, _table, false);
> if (ret)
> return ret;
>
> @@ -1396,7 +1403,7 @@ static int vega12_read_sensor(struct pp_hwmgr *hwmgr,
> int idx,
> *size = 4;
> break;
> case AMDGPU_PP_SENSOR_MEM_TEMP:
> - ret = vega12_get_metrics_table(hwmgr, _table);
> + ret = vega12_get_metrics_table(hwmgr, _table, false);
> if (ret)
> return ret;
>
> @@ -2752,7 +2759,7 @@ static ssize_t vega12_get_gpu_metrics(struct pp_hwmgr
> *hwmgr,
> uint32_t fan_speed_rpm;
> int ret;
>
> - ret = vega12_get_metrics_table(hwmgr, );
> + ret = vega12_get_metrics_table(hwmgr, , true);
> if (ret)
> return ret;
>
> --
> 2.28.0
>
> ___
> 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 11/17] drm/amd/powerplay: add Vega12 support for gpu metrics export
Patches 9-11 are:
Reviewed-by: Alex Deucher
On Thu, Jul 30, 2020 at 10:44 PM Evan Quan wrote:
>
> Add Vega12 gpu metrics export interface.
>
> Change-Id: I2c910f523049f0f90eecb8d74cb73ebb39a22bd9
> Signed-off-by: Evan Quan
> ---
> .../drm/amd/powerplay/hwmgr/vega12_hwmgr.c| 111 ++
> .../drm/amd/powerplay/hwmgr/vega12_hwmgr.h| 1 +
> 2 files changed, 112 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> index a678a67f1c0d..40bb0c2e4e8c 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega12_hwmgr.c
> @@ -47,6 +47,13 @@
> #include "pp_thermal.h"
> #include "vega12_baco.h"
>
> +#define smnPCIE_LC_SPEED_CNTL 0x11140290
> +#define smnPCIE_LC_LINK_WIDTH_CNTL 0x11140288
> +
> +#define LINK_WIDTH_MAX 6
> +#define LINK_SPEED_MAX 3
> +static int link_width[] = {0, 1, 2, 4, 8, 12, 16};
> +static int link_speed[] = {25, 50, 80, 160};
>
> static int vega12_force_clock_level(struct pp_hwmgr *hwmgr,
> enum pp_clock_type type, uint32_t mask);
> @@ -2095,6 +2102,46 @@ static int vega12_set_ppfeature_status(struct pp_hwmgr
> *hwmgr, uint64_t new_ppfe
> return 0;
> }
>
> +static int vega12_get_current_pcie_link_width_level(struct pp_hwmgr *hwmgr)
> +{
> + struct amdgpu_device *adev = hwmgr->adev;
> +
> + return (RREG32_PCIE(smnPCIE_LC_LINK_WIDTH_CNTL) &
> + PCIE_LC_LINK_WIDTH_CNTL__LC_LINK_WIDTH_RD_MASK)
> + >> PCIE_LC_LINK_WIDTH_CNTL__LC_LINK_WIDTH_RD__SHIFT;
> +}
> +
> +static int vega12_get_current_pcie_link_width(struct pp_hwmgr *hwmgr)
> +{
> + uint32_t width_level;
> +
> + width_level = vega12_get_current_pcie_link_width_level(hwmgr);
> + if (width_level > LINK_WIDTH_MAX)
> + width_level = 0;
> +
> + return link_width[width_level];
> +}
> +
> +static int vega12_get_current_pcie_link_speed_level(struct pp_hwmgr *hwmgr)
> +{
> + struct amdgpu_device *adev = hwmgr->adev;
> +
> + return (RREG32_PCIE(smnPCIE_LC_SPEED_CNTL) &
> + PSWUSP0_PCIE_LC_SPEED_CNTL__LC_CURRENT_DATA_RATE_MASK)
> + >> PSWUSP0_PCIE_LC_SPEED_CNTL__LC_CURRENT_DATA_RATE__SHIFT;
> +}
> +
> +static int vega12_get_current_pcie_link_speed(struct pp_hwmgr *hwmgr)
> +{
> + uint32_t speed_level;
> +
> + speed_level = vega12_get_current_pcie_link_speed_level(hwmgr);
> + if (speed_level > LINK_SPEED_MAX)
> + speed_level = 0;
> +
> + return link_speed[speed_level];
> +}
> +
> static int vega12_print_clock_levels(struct pp_hwmgr *hwmgr,
> enum pp_clock_type type, char *buf)
> {
> @@ -2682,6 +2729,69 @@ static int vega12_set_mp1_state(struct pp_hwmgr *hwmgr,
> return 0;
> }
>
> +static void vega12_init_gpu_metrics_v1_0(struct gpu_metrics_v1_0
> *gpu_metrics)
> +{
> + memset(gpu_metrics, 0xFF, sizeof(struct gpu_metrics_v1_0));
> +
> + gpu_metrics->common_header.structure_size =
> + sizeof(struct gpu_metrics_v1_0);
> + gpu_metrics->common_header.format_revision = 1;
> + gpu_metrics->common_header.content_revision = 0;
> +
> + gpu_metrics->system_clock_counter = ktime_get_boottime_ns();
> +}
> +
> +static ssize_t vega12_get_gpu_metrics(struct pp_hwmgr *hwmgr,
> + void **table)
> +{
> + struct vega12_hwmgr *data =
> + (struct vega12_hwmgr *)(hwmgr->backend);
> + struct gpu_metrics_v1_0 *gpu_metrics =
> + >gpu_metrics_table;
> + SmuMetrics_t metrics;
> + uint32_t fan_speed_rpm;
> + int ret;
> +
> + ret = vega12_get_metrics_table(hwmgr, );
> + if (ret)
> + return ret;
> +
> + vega12_init_gpu_metrics_v1_0(gpu_metrics);
> +
> + gpu_metrics->temperature_edge = metrics.TemperatureEdge;
> + gpu_metrics->temperature_hotspot = metrics.TemperatureHotspot;
> + gpu_metrics->temperature_mem = metrics.TemperatureHBM;
> + gpu_metrics->temperature_vrgfx = metrics.TemperatureVrGfx;
> + gpu_metrics->temperature_vrmem = metrics.TemperatureVrMem;
> +
> + gpu_metrics->average_gfx_activity = metrics.AverageGfxActivity;
> + gpu_metrics->average_umc_activity = metrics.AverageUclkActivity;
> +
> + gpu_metrics->average_gfxclk_frequency =
> metrics.AverageGfxclkFrequency;
> + gpu_metrics->average_socclk_frequency =
> metrics.AverageSocclkFrequency;
> + gpu_metrics->average_uclk_frequency = metrics.AverageUclkFrequency;
> +
> + gpu_metrics->current_gfxclk = metrics.CurrClock[PPCLK_GFXCLK];
> + gpu_metrics->current_socclk = metrics.CurrClock[PPCLK_SOCCLK];
> + gpu_metrics->current_uclk = metrics.CurrClock[PPCLK_UCLK];
> + gpu_metrics->current_vclk0 =
Re: [PATCH] drm/amd/display: Indent an if statement
Applied. Thanks! Alex On Mon, Aug 3, 2020 at 10:35 AM Dan Carpenter wrote: > > The if statement wasn't indented so it's confusing. > > Signed-off-by: Dan Carpenter > --- > drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c > b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c > index ca26714c800e..c6b737dd8425 100644 > --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c > @@ -71,7 +71,7 @@ enum dce_version resource_parse_asic_id(struct hw_asic_id > asic_id) > if (ASIC_REV_IS_TAHITI_P(asic_id.hw_internal_rev) || > ASIC_REV_IS_PITCAIRN_PM(asic_id.hw_internal_rev) || > ASIC_REV_IS_CAPEVERDE_M(asic_id.hw_internal_rev)) > - dc_version = DCE_VERSION_6_0; > + dc_version = DCE_VERSION_6_0; > else if (ASIC_REV_IS_OLAND_M(asic_id.hw_internal_rev)) > dc_version = DCE_VERSION_6_4; > else > -- > 2.27.0 > > ___ > 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 v2] drm/amd/display: use correct scale for actual_brightness
Documentation for sysfs backlight level interface requires that
values in both 'brightness' and 'actual_brightness' files are
interpreted to be in range from 0 to the value given in the
'max_brightness' file.
With amdgpu, max_brightness gives 255, and values written by the user
into 'brightness' are internally rescaled to a wider range. However,
reading from 'actual_brightness' gives the raw register value without
inverse rescaling. This causes issues for various userspace tools such
as PowerTop and systemd that expect the value to be in the correct
range.
Introduce a helper to retrieve internal backlight range. Use it to
reimplement 'convert_brightness' as 'convert_brightness_from_user' and
introduce 'convert_brightness_to_user'.
Bug: https://bugzilla.kernel.org/show_bug.cgi?id=203905
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1242
Cc: Alex Deucher
Cc: Nicholas Kazlauskas
Signed-off-by: Alexander Monakov
---
v2: split convert_brightness to &_from_user and &_to_user (Nicholas)
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 81 +--
1 file changed, 40 insertions(+), 41 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 710edc70e37e..b60a763f3f95 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2881,51 +2881,50 @@ static int set_backlight_via_aux(struct dc_link *link,
uint32_t brightness)
return rc ? 0 : 1;
}
-static u32 convert_brightness(const struct amdgpu_dm_backlight_caps *caps,
- const uint32_t user_brightness)
+static int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps,
+ unsigned *min, unsigned *max)
{
- u32 min, max, conversion_pace;
- u32 brightness = user_brightness;
-
if (!caps)
- goto out;
+ return 0;
- if (!caps->aux_support) {
- max = caps->max_input_signal;
- min = caps->min_input_signal;
- /*
-* The brightness input is in the range 0-255
-* It needs to be rescaled to be between the
-* requested min and max input signal
-* It also needs to be scaled up by 0x101 to
-* match the DC interface which has a range of
-* 0 to 0x
-*/
- conversion_pace = 0x101;
- brightness =
- user_brightness
- * conversion_pace
- * (max - min)
- / AMDGPU_MAX_BL_LEVEL
- + min * conversion_pace;
+ if (caps->aux_support) {
+ // Firmware limits are in nits, DC API wants millinits.
+ *max = 1000 * caps->aux_max_input_signal;
+ *min = 1000 * caps->aux_min_input_signal;
} else {
- /* TODO
-* We are doing a linear interpolation here, which is OK but
-* does not provide the optimal result. We probably want
-* something close to the Perceptual Quantizer (PQ) curve.
-*/
- max = caps->aux_max_input_signal;
- min = caps->aux_min_input_signal;
-
- brightness = (AMDGPU_MAX_BL_LEVEL - user_brightness) * min
- + user_brightness * max;
- // Multiple the value by 1000 since we use millinits
- brightness *= 1000;
- brightness = DIV_ROUND_CLOSEST(brightness, AMDGPU_MAX_BL_LEVEL);
+ // Firmware limits are 8-bit, PWM control is 16-bit.
+ *max = 0x101 * caps->max_input_signal;
+ *min = 0x101 * caps->min_input_signal;
}
+ return 1;
+}
-out:
- return brightness;
+static u32 convert_brightness_from_user(const struct amdgpu_dm_backlight_caps
*caps,
+ uint32_t brightness)
+{
+ unsigned min, max;
+
+ if (!get_brightness_range(caps, , ))
+ return brightness;
+
+ // Rescale 0..255 to min..max
+ return min + DIV_ROUND_CLOSEST((max - min) * brightness,
+ AMDGPU_MAX_BL_LEVEL);
+}
+
+static u32 convert_brightness_to_user(const struct amdgpu_dm_backlight_caps
*caps,
+ uint32_t brightness)
+{
+ unsigned min, max;
+
+ if (!get_brightness_range(caps, , ))
+ return brightness;
+
+ if (brightness < min)
+ return 0;
+ // Rescale min..max to 0..255
+ return DIV_ROUND_CLOSEST(AMDGPU_MAX_BL_LEVEL * (brightness - min),
+max - min);
}
static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
@@ -2941,7 +2940,7 @@ static int amdgpu_dm_backlight_update_status(struct
backlight_device
[PATCH 1/3] drm/amd/display: Constify dcn20_res_pool_funcs
The only usage of dcn20_res_pool_funcs is to assign its address to a
const pointer. Make it const to allow the compiler to put it in
read-only memory.
Signed-off-by: Rikard Falkeborn
---
drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index 2a5e7175926a..d7ce984a2ce3 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -3320,7 +3320,7 @@ enum dc_status dcn20_patch_unknown_plane_state(struct
dc_plane_state *plane_stat
return DC_OK;
}
-static struct resource_funcs dcn20_res_pool_funcs = {
+static const struct resource_funcs dcn20_res_pool_funcs = {
.destroy = dcn20_destroy_resource_pool,
.link_enc_create = dcn20_link_encoder_create,
.panel_cntl_create = dcn20_panel_cntl_create,
--
2.28.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 0/3] drm/amd/display: Constify static resource_funcs
Constify a couple of instances of resource_funcs that are never modified to allow the compiler to put it in read-only memory. The other drivers in drivers/gpu/drm/amd/display/dc already have these as const. Rikard Falkeborn (3): drm/amd/display: Constify dcn20_res_pool_funcs drm/amd/display: Constify dcn21_res_pool_funcs drm/amd/display: Constify dcn30_res_pool_funcs drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 2 +- drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c | 2 +- drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) -- 2.28.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 3/3] drm/amd/display: Constify dcn30_res_pool_funcs
The only usage of dcn30_res_pool_funcs is to assign its address to a
const pointer. Make it const to allow the compiler to put it in
read-only memory.
Signed-off-by: Rikard Falkeborn
---
drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 653a571e366d..d474a6188445 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c
@@ -2412,7 +2412,7 @@ static void dcn30_update_bw_bounding_box(struct dc *dc,
struct clk_bw_params *bw
dml_init_instance(>current_state->bw_ctx.dml, _0_soc,
_0_ip, DML_PROJECT_DCN30);
}
-static struct resource_funcs dcn30_res_pool_funcs = {
+static const struct resource_funcs dcn30_res_pool_funcs = {
.destroy = dcn30_destroy_resource_pool,
.link_enc_create = dcn30_link_encoder_create,
.panel_cntl_create = dcn30_panel_cntl_create,
--
2.28.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH 2/3] drm/amd/display: Constify dcn21_res_pool_funcs
The only usage of dcn21_res_pool_funcs is to assign its address to a
const pointer. Make it const to allow the compiler to put it in
read-only memory.
Signed-off-by: Rikard Falkeborn
---
drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
index 88d41a385add..a828696668bf 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c
@@ -1754,7 +1754,7 @@ enum dc_status dcn21_patch_unknown_plane_state(struct
dc_plane_state *plane_stat
return result;
}
-static struct resource_funcs dcn21_res_pool_funcs = {
+static const struct resource_funcs dcn21_res_pool_funcs = {
.destroy = dcn21_destroy_resource_pool,
.link_enc_create = dcn21_link_encoder_create,
.panel_cntl_create = dcn21_panel_cntl_create,
--
2.28.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Amdgpu kernel oops and freezing graphics
On Tue, Jul 21, 2020 at 2:21 PM Harvey wrote: > > Alex, > > tnak you so much - you're my hero! > > Am 21.07.20 um 18:17 schrieb Alex Deucher: > > On Mon, Jul 20, 2020 at 4:22 AM Harvey wrote: > >> > >> Hello, > >> > >> this is my first post to this list so please be patient with me ;) > >> > >> The facts: > >> > >> it is now one week that I own a new laptop, a MSI Bravo 17 A4DDR/MS-17FK > >> with Ryzen 7 4800U and hybrid graphics on a Radeon RX 5500M. I installed > >> my beloved Archlinux but I can't start any graphics withpout kernel oops > >> on it beside the normal console, even calling 'lspci' on the console is > >> provoking errors. > >> > >> I am using linux kernel 5.7.9 and linux-firmware 20200619.e96c121 > >> > >> (FWIW: I even tried with a self-cmpiled kernel 5.8-rc5 and > >> linux-firmware directly from the git repository - no changes) > >> > >> The following is only part of the information I can provide but I didn't > >> want to make this mail bigger than it already is. > > > > Does appending amdgpu.runpm=0 on the kernel command line in grub help? > > Yes it does. Woohoo! The system is not freezing anymore! Can I provide > any further information to get this sorted? > > I will be happy to help investigating and testing if needed. Does appending pci=noats on the kernel command line in grub also fix the issue? Alex ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/2] drm/amd/powerplay: put VCN/JPEG into PG ungate state before dpm table setup
On Mon, Aug 3, 2020 at 12:47 AM Evan Quan wrote:
>
> As VCN related dpm table setup needs VCN be in PG ungate state. Same logics
> applies to JPEG.
>
> Change-Id: I94335efc4e0424cfe0991e984c938998fd8f1287
> Signed-off-by: Evan Quan
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 38 +-
> 1 file changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 85b04c48bd09..1349d05c5f3d 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -134,7 +134,8 @@ int smu_get_dpm_freq_range(struct smu_context *smu,
> }
>
> static int smu_dpm_set_vcn_enable(struct smu_context *smu,
> - bool enable)
> + bool enable,
> + int *previous_pg_state)
> {
> struct smu_power_context *smu_power = >smu_power;
> struct smu_power_gate *power_gate = _power->power_gate;
> @@ -148,6 +149,9 @@ static int smu_dpm_set_vcn_enable(struct smu_context *smu,
> if (atomic_read(_gate->vcn_gated) ^ enable)
> goto out;
>
> + if (previous_pg_state)
> + *previous_pg_state = atomic_read(_gate->vcn_gated);
> +
> ret = smu->ppt_funcs->dpm_set_vcn_enable(smu, enable);
> if (!ret)
> atomic_set(_gate->vcn_gated, !enable);
> @@ -159,7 +163,8 @@ static int smu_dpm_set_vcn_enable(struct smu_context *smu,
> }
>
> static int smu_dpm_set_jpeg_enable(struct smu_context *smu,
> - bool enable)
> + bool enable,
> + int *previous_pg_state)
> {
> struct smu_power_context *smu_power = >smu_power;
> struct smu_power_gate *power_gate = _power->power_gate;
> @@ -173,6 +178,9 @@ static int smu_dpm_set_jpeg_enable(struct smu_context
> *smu,
> if (atomic_read(_gate->jpeg_gated) ^ enable)
> goto out;
>
> + if (previous_pg_state)
> + *previous_pg_state = atomic_read(_gate->jpeg_gated);
> +
> ret = smu->ppt_funcs->dpm_set_jpeg_enable(smu, enable);
> if (!ret)
> atomic_set(_gate->jpeg_gated, !enable);
> @@ -212,7 +220,7 @@ int smu_dpm_set_power_gate(struct smu_context *smu,
> uint32_t block_type,
> */
> case AMD_IP_BLOCK_TYPE_UVD:
> case AMD_IP_BLOCK_TYPE_VCN:
> - ret = smu_dpm_set_vcn_enable(smu, !gate);
> + ret = smu_dpm_set_vcn_enable(smu, !gate, NULL);
> if (ret)
> dev_err(smu->adev->dev, "Failed to power %s VCN!\n",
> gate ? "gate" : "ungate");
> @@ -230,7 +238,7 @@ int smu_dpm_set_power_gate(struct smu_context *smu,
> uint32_t block_type,
> gate ? "gate" : "ungate");
> break;
> case AMD_IP_BLOCK_TYPE_JPEG:
> - ret = smu_dpm_set_jpeg_enable(smu, !gate);
> + ret = smu_dpm_set_jpeg_enable(smu, !gate, NULL);
> if (ret)
> dev_err(smu->adev->dev, "Failed to power %s JPEG!\n",
> gate ? "gate" : "ungate");
> @@ -407,6 +415,7 @@ static int smu_late_init(void *handle)
> {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> struct smu_context *smu = >smu;
> + int vcn_gate, jpeg_gate;
> int ret = 0;
>
> if (!smu->pm_enabled)
> @@ -418,6 +427,14 @@ static int smu_late_init(void *handle)
> return ret;
> }
>
> + /*
> +* 1. Power up VCN/JPEG as the succeeding smu_set_default_dpm_table()
> +*needs VCN/JPEG up.
> +* 2. Save original gate states and then we can restore back
> afterwards.
> +*/
> + smu_dpm_set_vcn_enable(smu, true, _gate);
> + smu_dpm_set_jpeg_enable(smu, true, _gate);
> +
> /*
> * Set initialized values (get from vbios) to dpm tables context such
> as
> * gfxclk, memclk, dcefclk, and etc. And enable the DPM feature for
> each
> @@ -429,6 +446,11 @@ static int smu_late_init(void *handle)
> return ret;
> }
>
> + /* Restore back to original VCN/JPEG power gate states */
> + smu_dpm_set_vcn_enable(smu, !vcn_gate, NULL);
> + smu_dpm_set_jpeg_enable(smu, !vcn_gate, NULL);
Copy paste typo. This should be !jpeg_gate. With that fixed, the series is:
Reviewed-by: Alex Deucher
Alex
> +
> +
> ret = smu_populate_umd_state_clk(smu);
> if (ret) {
> dev_err(adev->dev, "Failed to populate UMD state clocks!\n");
> @@ -991,8 +1013,8 @@ static int smu_hw_init(void *handle)
>
> if (smu->is_apu) {
> smu_powergate_sdma(>smu, false);
> - smu_dpm_set_vcn_enable(smu, true);
> -
NAK: [PATCH][next] drm/amd/display: fix spelling mistake "Usupported" -> "Unsupported"
On 04/08/2020 18:34, Colin King wrote:
> From: Colin Ian King
>
> There are spelling mistakes in two DRM_ERROR error messages. Fix them.
>
> Signed-off-by: Colin Ian King
> ---
> 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 27579443cdc5..fe5f6350e288 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1549,7 +1549,7 @@ static int amdgpu_dm_initialize_drm_device(struct
> amdgpu_device *adev)
> break;
> #endif
> default:
> - DRM_ERROR("Usupported ASIC type: 0x%X\n", adev->asic_type);
> + DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
> goto fail;
> }
>
> @@ -1743,7 +1743,7 @@ static int dm_early_init(void *handle)
> break;
> #endif
> default:
> - DRM_ERROR("Usupported ASIC type: 0x%X\n", adev->asic_type);
> + DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
> return -EINVAL;
> }
>
>
Nack. I was on the wrong repo.
Colin
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH][next] drm/amd/display: fix spelling mistake "Usupported" -> "Unsupported"
From: Colin Ian King
There are spelling mistakes in two DRM_ERROR error messages. Fix them.
Signed-off-by: Colin Ian King
---
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 27579443cdc5..fe5f6350e288 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1549,7 +1549,7 @@ static int amdgpu_dm_initialize_drm_device(struct
amdgpu_device *adev)
break;
#endif
default:
- DRM_ERROR("Usupported ASIC type: 0x%X\n", adev->asic_type);
+ DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
goto fail;
}
@@ -1743,7 +1743,7 @@ static int dm_early_init(void *handle)
break;
#endif
default:
- DRM_ERROR("Usupported ASIC type: 0x%X\n", adev->asic_type);
+ DRM_ERROR("Unsupported ASIC type: 0x%X\n", adev->asic_type);
return -EINVAL;
}
--
2.27.0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: use correct scale for actual_brightness
On Tue, 4 Aug 2020, Kazlauskas, Nicholas wrote:
> This is a cleaner change the other proposed patch since it doesn't need to
Can you give a URL to the other patch please?
> modify the exist conversion functions but I'd be worried about broken
> userspace relying on 0-255 as the only acceptable range.
Not sure what you mean by this. Userspace simply reads the maximum value from
max_brightness sysfs file. On other gpu/firmware combinations it can be 7 or 9
for example, it just happens to be 255 with modern amdgpu. Minimum value is
always zero.
Value seen in max_brightness remains 255 with this patch, so as far as userspace
is concerned nothing is changed apart from value given by actual_brightness
file.
Alexander
>
> Not an expert on existing implementations to point out a specific one though.
>
> Regards,
> Nicholas Kazlauskas
>
> On 2020-08-03 4:02 p.m., Alexander Monakov wrote:
> > Documentation for sysfs backlight level interface requires that
> > values in both 'brightness' and 'actual_brightness' files are
> > interpreted to be in range from 0 to the value given in the
> > 'max_brightness' file.
> >
> > With amdgpu, max_brightness gives 255, and values written by the user
> > into 'brightness' are internally rescaled to a wider range. However,
> > reading from 'actual_brightness' gives the raw register value without
> > inverse rescaling. This causes issues for various userspace tools such
> > as PowerTop and systemd that expect the value to be in the correct
> > range.
> >
> > Introduce a helper to retrieve internal backlight range. Extend the
> > existing 'convert_brightness' function to handle conversion in both
> > directions.
> >
> > Bug: https://bugzilla.kernel.org/show_bug.cgi?id=203905
> > Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1242
> > Cc: Alex Deucher
> > Signed-off-by: Alexander Monakov
> > ---
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 73 ---
> > 1 file changed, 32 insertions(+), 41 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 710edc70e37e..03e21e7b7917 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -2881,51 +2881,42 @@ static int set_backlight_via_aux(struct dc_link
> > *link, uint32_t brightness)
> > return rc ? 0 : 1;
> > }
> > -static u32 convert_brightness(const struct amdgpu_dm_backlight_caps
> > *caps,
> > - const uint32_t user_brightness)
> > +static int get_brightness_range(const struct amdgpu_dm_backlight_caps
> > *caps,
> > + unsigned *min, unsigned *max)
> > {
> > - u32 min, max, conversion_pace;
> > - u32 brightness = user_brightness;
> > -
> > if (!caps)
> > - goto out;
> > + return 0;
> > - if (!caps->aux_support) {
> > - max = caps->max_input_signal;
> > - min = caps->min_input_signal;
> > - /*
> > -* The brightness input is in the range 0-255
> > -* It needs to be rescaled to be between the
> > -* requested min and max input signal
> > -* It also needs to be scaled up by 0x101 to
> > -* match the DC interface which has a range of
> > -* 0 to 0x
> > -*/
> > - conversion_pace = 0x101;
> > - brightness =
> > - user_brightness
> > - * conversion_pace
> > - * (max - min)
> > - / AMDGPU_MAX_BL_LEVEL
> > - + min * conversion_pace;
> > + if (caps->aux_support) {
> > + // Firmware limits are in nits, DC API wants millinits.
> > + *max = 1000 * caps->aux_max_input_signal;
> > + *min = 1000 * caps->aux_min_input_signal;
> > } else {
> > - /* TODO
> > -* We are doing a linear interpolation here, which is OK but
> > -* does not provide the optimal result. We probably want
> > -* something close to the Perceptual Quantizer (PQ) curve.
> > -*/
> > - max = caps->aux_max_input_signal;
> > - min = caps->aux_min_input_signal;
> > -
> > - brightness = (AMDGPU_MAX_BL_LEVEL - user_brightness) * min
> > - + user_brightness * max;
> > - // Multiple the value by 1000 since we use millinits
> > - brightness *= 1000;
> > - brightness = DIV_ROUND_CLOSEST(brightness,
> > AMDGPU_MAX_BL_LEVEL);
> > + // Firmware limits are 8-bit, PWM control is 16-bit.
> > + *max = 0x101 * caps->max_input_signal;
> > + *min = 0x101 * caps->min_input_signal;
> > }
> > + return 1;
> > +}
> > -out:
> > - return brightness;
> > +static u32 convert_brightness(const struct amdgpu_dm_backlight_caps *caps,
> > + const uint32_t
Re: Enabling AMDGPU by default for SI & CIK
On Tue, Aug 4, 2020 at 4:38 AM Michel Dänzer wrote: > > On 2020-08-03 1:45 a.m., Bas Nieuwenhuizen wrote: > > Hi all, > > > > Now that we have recently made some progress on getting feature parity > > with the Radeon driver for SI, I'm wondering what it would take to > > make AMDGPU the default driver for these generations. > > > > As far as I understand AMDGPU has had these features for CIK for a > > while already but it is still not the default driver. What would it > > take to make it the default? What is missing and/or broken? > > The main blockers I'm aware of for CIK are: > > 1) Lack of analogue connector support with DC > 2) Lack of HDMI/DP audio support without DC > > > 1) may apply to SI as well. Also, IIRC, there are suspend and resume problems with some CIK parts using amdgpu. Alex ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu/smu: rework i2c adpater registration
Reviewed-By: Kevin Wang
Best Regards
Kevin
> 在 2020年8月5日，上午12:57，Alex Deucher 写道：
>
> Ping?
>
>
>> On Thu, Jul 30, 2020 at 3:24 PM Alex Deucher wrote:
>>
>> The i2c init/fini functions just register the i2c adapter.
>> There is no need to call them during hw init/fini. They only
>> need to be called once per driver init/fini. The previous
>> behavior broke runtime pm because we unregistered the i2c
>> adapter during suspend.
>>
>> Signed-off-by: Alex Deucher
>> ---
>> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 12 ++--
>> drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 14 --
>> drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 14 --
>> drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c | 14 --
>> 4 files changed, 6 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
>> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
>> index 55463e7a11e2..d03b4852ed5f 100644
>> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
>> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
>> @@ -579,6 +579,10 @@ static int smu_smc_table_sw_init(struct smu_context
>> *smu)
>>if (ret)
>>return ret;
>>
>> + ret = smu_i2c_init(smu, >adev->pm.smu_i2c);
>> + if (ret)
>> + return ret;
>> +
>>return 0;
>> }
>>
>> @@ -586,6 +590,8 @@ static int smu_smc_table_sw_fini(struct smu_context *smu)
>> {
>>int ret;
>>
>> + smu_i2c_fini(smu, >adev->pm.smu_i2c);
>> +
>>ret = smu_free_memory_pool(smu);
>>if (ret)
>>return ret;
>> @@ -845,10 +851,6 @@ static int smu_smc_hw_setup(struct smu_context *smu)
>>return ret;
>>}
>>
>> - ret = smu_i2c_init(smu, >pm.smu_i2c);
>> - if (ret)
>> - return ret;
>> -
>>ret = smu_disable_umc_cdr_12gbps_workaround(smu);
>>if (ret) {
>>dev_err(adev->dev, "Workaround failed to disable UMC CDR
>> feature on 12Gbps SKU!\n");
>> @@ -1047,8 +1049,6 @@ static int smu_smc_hw_cleanup(struct smu_context *smu)
>>struct amdgpu_device *adev = smu->adev;
>>int ret = 0;
>>
>> - smu_i2c_fini(smu, >pm.smu_i2c);
>> -
>>cancel_work_sync(>throttling_logging_work);
>>
>>ret = smu_disable_thermal_alert(smu);
>> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
>> b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
>> index f13979687b9e..0147a5b9b06d 100644
>> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
>> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
>> @@ -2080,22 +2080,11 @@ static const struct i2c_algorithm arcturus_i2c_algo
>> = {
>>.functionality = arcturus_i2c_func,
>> };
>>
>> -static bool arcturus_i2c_adapter_is_added(struct i2c_adapter *control)
>> -{
>> - struct amdgpu_device *adev = to_amdgpu_device(control);
>> -
>> - return control->dev.parent == >pdev->dev;
>> -}
>> -
>> static int arcturus_i2c_control_init(struct smu_context *smu, struct
>> i2c_adapter *control)
>> {
>>struct amdgpu_device *adev = to_amdgpu_device(control);
>>int res;
>>
>> - /* smu_i2c_eeprom_init may be called twice in sriov */
>> - if (arcturus_i2c_adapter_is_added(control))
>> - return 0;
>> -
>>control->owner = THIS_MODULE;
>>control->class = I2C_CLASS_SPD;
>>control->dev.parent = >pdev->dev;
>> @@ -2111,9 +2100,6 @@ static int arcturus_i2c_control_init(struct
>> smu_context *smu, struct i2c_adapter
>>
>> static void arcturus_i2c_control_fini(struct smu_context *smu, struct
>> i2c_adapter *control)
>> {
>> - if (!arcturus_i2c_adapter_is_added(control))
>> - return;
>> -
>>i2c_del_adapter(control);
>> }
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
>> b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
>> index 6aaf483858a0..c33bdc6747f2 100644
>> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
>> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
>> @@ -2457,22 +2457,11 @@ static const struct i2c_algorithm navi10_i2c_algo = {
>>.functionality = navi10_i2c_func,
>> };
>>
>> -static bool navi10_i2c_adapter_is_added(struct i2c_adapter *control)
>> -{
>> - struct amdgpu_device *adev = to_amdgpu_device(control);
>> -
>> - return control->dev.parent == >pdev->dev;
>> -}
>> -
>> static int navi10_i2c_control_init(struct smu_context *smu, struct
>> i2c_adapter *control)
>> {
>>struct amdgpu_device *adev = to_amdgpu_device(control);
>>int res;
>>
>> - /* smu_i2c_eeprom_init may be called twice in sriov */
>> - if (navi10_i2c_adapter_is_added(control))
>> - return 0;
>> -
>>control->owner = THIS_MODULE;
>>control->class = I2C_CLASS_SPD;
>>control->dev.parent = >pdev->dev;
>> @@ -2488,9 +2477,6 @@ static int navi10_i2c_control_init(struct smu_context
>> *smu, struct i2c_adapter *
>>
>> static
Re: [PATCH] drm/amdkfd: option to disable system mem limit
Am 2020-08-04 um 1:29 p.m. schrieb Alex Deucher:
> On Tue, Aug 4, 2020 at 1:27 PM Felix Kuehling wrote:
>> Am 2020-07-27 um 9:24 a.m. schrieb Philip Yang:
>>> If multiple process share system memory through /dev/shm, KFD allocate
>>> memory should not fail if it reachs the system memory limit because
>>> one copy of physical system memory are shared by multiple process.
>>>
>>> Add module parameter to provide user option to disable system memory
>>> limit check, to run multiple process share memory application. By
>>> default the system memory limit is on.
>>>
>>> Print out debug message to warn user if KFD allocate memory failed
>>> because of system memory limit.
>>>
>>> Signed-off-by: Philip Yang
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 9 -
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
>>> 3 files changed, 19 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index e97c088d03b3..3c0d5ecfe0d5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -187,9 +187,11 @@ extern int amdgpu_force_asic_type;
>>> #ifdef CONFIG_HSA_AMD
>>> extern int sched_policy;
>>> extern bool debug_evictions;
>>> +extern bool no_system_mem_limit;
>>> #else
>>> static const int sched_policy = KFD_SCHED_POLICY_HWS;
>>> static const bool debug_evictions; /* = false */
>>> +static const bool no_system_mem_limit;
>>> #endif
>>>
>>> extern int amdgpu_tmz;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> index 8703aa1fe4a5..502e8204c012 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> @@ -99,7 +99,10 @@ void amdgpu_amdkfd_gpuvm_init_mem_limits(void)
>>> mem *= si.mem_unit;
>>>
>>> spin_lock_init(_mem_limit.mem_limit_lock);
>>> - kfd_mem_limit.max_system_mem_limit = mem - (mem >> 4);
>>> + if (no_system_mem_limit)
>>> + kfd_mem_limit.max_system_mem_limit = U64_MAX;
>>> + else
>>> + kfd_mem_limit.max_system_mem_limit = mem - (mem >> 4);
>>> kfd_mem_limit.max_ttm_mem_limit = (mem >> 1) - (mem >> 3);
>>> pr_debug("Kernel memory limit %lluM, TTM limit %lluM\n",
>>> (kfd_mem_limit.max_system_mem_limit >> 20),
>>> @@ -148,6 +151,10 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
>>> amdgpu_device *adev,
>>>
>>> spin_lock(_mem_limit.mem_limit_lock);
>>>
>>> + if (kfd_mem_limit.system_mem_used + system_mem_needed >
>>> + kfd_mem_limit.max_system_mem_limit)
>>> + pr_debug("Set no_system_mem_limit if using shared memory\n");
>>> +
>>> if ((kfd_mem_limit.system_mem_used + system_mem_needed >
>>>kfd_mem_limit.max_system_mem_limit) ||
>>> (kfd_mem_limit.ttm_mem_used + ttm_mem_needed >
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index 6291f5f0d223..e9acd0a9f327 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -715,6 +715,15 @@ MODULE_PARM_DESC(queue_preemption_timeout_ms, "queue
>>> preemption timeout in ms (1
>>> bool debug_evictions;
>>> module_param(debug_evictions, bool, 0644);
>>> MODULE_PARM_DESC(debug_evictions, "enable eviction debug messages (false =
>>> default)");
>>> +
>>> +/**
>>> + * DOC: no_system_mem_limit(bool)
>>> + * Disable system memory limit, to support multiple process shared memory
>>> + */
>>> +bool no_system_mem_limit;
>>> +module_param(no_system_mem_limit, bool, 0644);
>> The permissions suggest that this parameter is writable at runtime using
>> sysfs. However, the parameter is only read once during module init. So
>> any runtime changes to this parameter will not take effect.
>>
>> You can fix this in two ways:
>>
>> 1. Make the parameter read only
>> 2. Change the implementation of amdgpu_amdkfd_reserve_mem_limit to
>> check the parameter every time and only apply the system memory
>> limit check if necessary
>>
>> I think the second option is preferable, because it allows user to
>> experiment with this without rebooting.
> Agreed. If we go with that approach, maybe just drop the module
> parameter altogether and just let the user set it manually per device
> at runtime.
The KFD system memory limit is global. There is no useful way to apply
this limit per device.
Regards,
Felix
>
> Alex
>
>> Regards,
>> Felix
>>
>>
>>> +MODULE_PARM_DESC(no_system_mem_limit, "disable system memory limit (false
>>> = default)");
>>> +
>>> #endif
>>>
>>> /**
>> ___
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdkfd: option to disable system mem limit
On Tue, Aug 4, 2020 at 1:27 PM Felix Kuehling wrote:
>
> Am 2020-07-27 um 9:24 a.m. schrieb Philip Yang:
> > If multiple process share system memory through /dev/shm, KFD allocate
> > memory should not fail if it reachs the system memory limit because
> > one copy of physical system memory are shared by multiple process.
> >
> > Add module parameter to provide user option to disable system memory
> > limit check, to run multiple process share memory application. By
> > default the system memory limit is on.
> >
> > Print out debug message to warn user if KFD allocate memory failed
> > because of system memory limit.
> >
> > Signed-off-by: Philip Yang
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 9 -
> > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
> > 3 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index e97c088d03b3..3c0d5ecfe0d5 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -187,9 +187,11 @@ extern int amdgpu_force_asic_type;
> > #ifdef CONFIG_HSA_AMD
> > extern int sched_policy;
> > extern bool debug_evictions;
> > +extern bool no_system_mem_limit;
> > #else
> > static const int sched_policy = KFD_SCHED_POLICY_HWS;
> > static const bool debug_evictions; /* = false */
> > +static const bool no_system_mem_limit;
> > #endif
> >
> > extern int amdgpu_tmz;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > index 8703aa1fe4a5..502e8204c012 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > @@ -99,7 +99,10 @@ void amdgpu_amdkfd_gpuvm_init_mem_limits(void)
> > mem *= si.mem_unit;
> >
> > spin_lock_init(_mem_limit.mem_limit_lock);
> > - kfd_mem_limit.max_system_mem_limit = mem - (mem >> 4);
> > + if (no_system_mem_limit)
> > + kfd_mem_limit.max_system_mem_limit = U64_MAX;
> > + else
> > + kfd_mem_limit.max_system_mem_limit = mem - (mem >> 4);
> > kfd_mem_limit.max_ttm_mem_limit = (mem >> 1) - (mem >> 3);
> > pr_debug("Kernel memory limit %lluM, TTM limit %lluM\n",
> > (kfd_mem_limit.max_system_mem_limit >> 20),
> > @@ -148,6 +151,10 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
> > amdgpu_device *adev,
> >
> > spin_lock(_mem_limit.mem_limit_lock);
> >
> > + if (kfd_mem_limit.system_mem_used + system_mem_needed >
> > + kfd_mem_limit.max_system_mem_limit)
> > + pr_debug("Set no_system_mem_limit if using shared memory\n");
> > +
> > if ((kfd_mem_limit.system_mem_used + system_mem_needed >
> >kfd_mem_limit.max_system_mem_limit) ||
> > (kfd_mem_limit.ttm_mem_used + ttm_mem_needed >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 6291f5f0d223..e9acd0a9f327 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -715,6 +715,15 @@ MODULE_PARM_DESC(queue_preemption_timeout_ms, "queue
> > preemption timeout in ms (1
> > bool debug_evictions;
> > module_param(debug_evictions, bool, 0644);
> > MODULE_PARM_DESC(debug_evictions, "enable eviction debug messages (false =
> > default)");
> > +
> > +/**
> > + * DOC: no_system_mem_limit(bool)
> > + * Disable system memory limit, to support multiple process shared memory
> > + */
> > +bool no_system_mem_limit;
> > +module_param(no_system_mem_limit, bool, 0644);
>
> The permissions suggest that this parameter is writable at runtime using
> sysfs. However, the parameter is only read once during module init. So
> any runtime changes to this parameter will not take effect.
>
> You can fix this in two ways:
>
> 1. Make the parameter read only
> 2. Change the implementation of amdgpu_amdkfd_reserve_mem_limit to
> check the parameter every time and only apply the system memory
> limit check if necessary
>
> I think the second option is preferable, because it allows user to
> experiment with this without rebooting.
Agreed. If we go with that approach, maybe just drop the module
parameter altogether and just let the user set it manually per device
at runtime.
Alex
>
> Regards,
> Felix
>
>
> > +MODULE_PARM_DESC(no_system_mem_limit, "disable system memory limit (false
> > = default)");
> > +
> > #endif
> >
> > /**
> ___
> 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/amdkfd: option to disable system mem limit
Am 2020-07-27 um 9:24 a.m. schrieb Philip Yang:
> If multiple process share system memory through /dev/shm, KFD allocate
> memory should not fail if it reachs the system memory limit because
> one copy of physical system memory are shared by multiple process.
>
> Add module parameter to provide user option to disable system memory
> limit check, to run multiple process share memory application. By
> default the system memory limit is on.
>
> Print out debug message to warn user if KFD allocate memory failed
> because of system memory limit.
>
> Signed-off-by: Philip Yang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 9 -
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
> 3 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e97c088d03b3..3c0d5ecfe0d5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -187,9 +187,11 @@ extern int amdgpu_force_asic_type;
> #ifdef CONFIG_HSA_AMD
> extern int sched_policy;
> extern bool debug_evictions;
> +extern bool no_system_mem_limit;
> #else
> static const int sched_policy = KFD_SCHED_POLICY_HWS;
> static const bool debug_evictions; /* = false */
> +static const bool no_system_mem_limit;
> #endif
>
> extern int amdgpu_tmz;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 8703aa1fe4a5..502e8204c012 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -99,7 +99,10 @@ void amdgpu_amdkfd_gpuvm_init_mem_limits(void)
> mem *= si.mem_unit;
>
> spin_lock_init(_mem_limit.mem_limit_lock);
> - kfd_mem_limit.max_system_mem_limit = mem - (mem >> 4);
> + if (no_system_mem_limit)
> + kfd_mem_limit.max_system_mem_limit = U64_MAX;
> + else
> + kfd_mem_limit.max_system_mem_limit = mem - (mem >> 4);
> kfd_mem_limit.max_ttm_mem_limit = (mem >> 1) - (mem >> 3);
> pr_debug("Kernel memory limit %lluM, TTM limit %lluM\n",
> (kfd_mem_limit.max_system_mem_limit >> 20),
> @@ -148,6 +151,10 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
> amdgpu_device *adev,
>
> spin_lock(_mem_limit.mem_limit_lock);
>
> + if (kfd_mem_limit.system_mem_used + system_mem_needed >
> + kfd_mem_limit.max_system_mem_limit)
> + pr_debug("Set no_system_mem_limit if using shared memory\n");
> +
> if ((kfd_mem_limit.system_mem_used + system_mem_needed >
>kfd_mem_limit.max_system_mem_limit) ||
> (kfd_mem_limit.ttm_mem_used + ttm_mem_needed >
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 6291f5f0d223..e9acd0a9f327 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -715,6 +715,15 @@ MODULE_PARM_DESC(queue_preemption_timeout_ms, "queue
> preemption timeout in ms (1
> bool debug_evictions;
> module_param(debug_evictions, bool, 0644);
> MODULE_PARM_DESC(debug_evictions, "enable eviction debug messages (false =
> default)");
> +
> +/**
> + * DOC: no_system_mem_limit(bool)
> + * Disable system memory limit, to support multiple process shared memory
> + */
> +bool no_system_mem_limit;
> +module_param(no_system_mem_limit, bool, 0644);
The permissions suggest that this parameter is writable at runtime using
sysfs. However, the parameter is only read once during module init. So
any runtime changes to this parameter will not take effect.
You can fix this in two ways:
1. Make the parameter read only
2. Change the implementation of amdgpu_amdkfd_reserve_mem_limit to
check the parameter every time and only apply the system memory
limit check if necessary
I think the second option is preferable, because it allows user to
experiment with this without rebooting.
Regards,
Felix
> +MODULE_PARM_DESC(no_system_mem_limit, "disable system memory limit (false =
> default)");
> +
> #endif
>
> /**
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu/smu: rework i2c adpater registration
Ping?
On Thu, Jul 30, 2020 at 3:24 PM Alex Deucher wrote:
>
> The i2c init/fini functions just register the i2c adapter.
> There is no need to call them during hw init/fini. They only
> need to be called once per driver init/fini. The previous
> behavior broke runtime pm because we unregistered the i2c
> adapter during suspend.
>
> Signed-off-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 12 ++--
> drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 14 --
> drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 14 --
> drivers/gpu/drm/amd/powerplay/sienna_cichlid_ppt.c | 14 --
> 4 files changed, 6 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 55463e7a11e2..d03b4852ed5f 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -579,6 +579,10 @@ static int smu_smc_table_sw_init(struct smu_context *smu)
> if (ret)
> return ret;
>
> + ret = smu_i2c_init(smu, >adev->pm.smu_i2c);
> + if (ret)
> + return ret;
> +
> return 0;
> }
>
> @@ -586,6 +590,8 @@ static int smu_smc_table_sw_fini(struct smu_context *smu)
> {
> int ret;
>
> + smu_i2c_fini(smu, >adev->pm.smu_i2c);
> +
> ret = smu_free_memory_pool(smu);
> if (ret)
> return ret;
> @@ -845,10 +851,6 @@ static int smu_smc_hw_setup(struct smu_context *smu)
> return ret;
> }
>
> - ret = smu_i2c_init(smu, >pm.smu_i2c);
> - if (ret)
> - return ret;
> -
> ret = smu_disable_umc_cdr_12gbps_workaround(smu);
> if (ret) {
> dev_err(adev->dev, "Workaround failed to disable UMC CDR
> feature on 12Gbps SKU!\n");
> @@ -1047,8 +1049,6 @@ static int smu_smc_hw_cleanup(struct smu_context *smu)
> struct amdgpu_device *adev = smu->adev;
> int ret = 0;
>
> - smu_i2c_fini(smu, >pm.smu_i2c);
> -
> cancel_work_sync(>throttling_logging_work);
>
> ret = smu_disable_thermal_alert(smu);
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index f13979687b9e..0147a5b9b06d 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -2080,22 +2080,11 @@ static const struct i2c_algorithm arcturus_i2c_algo =
> {
> .functionality = arcturus_i2c_func,
> };
>
> -static bool arcturus_i2c_adapter_is_added(struct i2c_adapter *control)
> -{
> - struct amdgpu_device *adev = to_amdgpu_device(control);
> -
> - return control->dev.parent == >pdev->dev;
> -}
> -
> static int arcturus_i2c_control_init(struct smu_context *smu, struct
> i2c_adapter *control)
> {
> struct amdgpu_device *adev = to_amdgpu_device(control);
> int res;
>
> - /* smu_i2c_eeprom_init may be called twice in sriov */
> - if (arcturus_i2c_adapter_is_added(control))
> - return 0;
> -
> control->owner = THIS_MODULE;
> control->class = I2C_CLASS_SPD;
> control->dev.parent = >pdev->dev;
> @@ -2111,9 +2100,6 @@ static int arcturus_i2c_control_init(struct smu_context
> *smu, struct i2c_adapter
>
> static void arcturus_i2c_control_fini(struct smu_context *smu, struct
> i2c_adapter *control)
> {
> - if (!arcturus_i2c_adapter_is_added(control))
> - return;
> -
> i2c_del_adapter(control);
> }
>
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index 6aaf483858a0..c33bdc6747f2 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -2457,22 +2457,11 @@ static const struct i2c_algorithm navi10_i2c_algo = {
> .functionality = navi10_i2c_func,
> };
>
> -static bool navi10_i2c_adapter_is_added(struct i2c_adapter *control)
> -{
> - struct amdgpu_device *adev = to_amdgpu_device(control);
> -
> - return control->dev.parent == >pdev->dev;
> -}
> -
> static int navi10_i2c_control_init(struct smu_context *smu, struct
> i2c_adapter *control)
> {
> struct amdgpu_device *adev = to_amdgpu_device(control);
> int res;
>
> - /* smu_i2c_eeprom_init may be called twice in sriov */
> - if (navi10_i2c_adapter_is_added(control))
> - return 0;
> -
> control->owner = THIS_MODULE;
> control->class = I2C_CLASS_SPD;
> control->dev.parent = >pdev->dev;
> @@ -2488,9 +2477,6 @@ static int navi10_i2c_control_init(struct smu_context
> *smu, struct i2c_adapter *
>
> static void navi10_i2c_control_fini(struct smu_context *smu, struct
> i2c_adapter *control)
> {
> - if (!navi10_i2c_adapter_is_added(control))
> - return;
> -
> i2c_del_adapter(control);
>
Re: [PATCH] drm/amdkfd: option to disable system mem limit
Ping.
On 2020-07-27 9:24 a.m., Philip Yang wrote:
If multiple process share system memory through /dev/shm, KFD allocate
memory should not fail if it reachs the system memory limit because
one copy of physical system memory are shared by multiple process.
Add module parameter to provide user option to disable system memory
limit check, to run multiple process share memory application. By
default the system memory limit is on.
Print out debug message to warn user if KFD allocate memory failed
because of system memory limit.
Signed-off-by: Philip Yang
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 9 -
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 9 +
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e97c088d03b3..3c0d5ecfe0d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -187,9 +187,11 @@ extern int amdgpu_force_asic_type;
#ifdef CONFIG_HSA_AMD
extern int sched_policy;
extern bool debug_evictions;
+extern bool no_system_mem_limit;
#else
static const int sched_policy = KFD_SCHED_POLICY_HWS;
static const bool debug_evictions; /* = false */
+static const bool no_system_mem_limit;
#endif
extern int amdgpu_tmz;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 8703aa1fe4a5..502e8204c012 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -99,7 +99,10 @@ void amdgpu_amdkfd_gpuvm_init_mem_limits(void)
mem *= si.mem_unit;
spin_lock_init(_mem_limit.mem_limit_lock);
- kfd_mem_limit.max_system_mem_limit = mem - (mem >> 4);
+ if (no_system_mem_limit)
+ kfd_mem_limit.max_system_mem_limit = U64_MAX;
+ else
+ kfd_mem_limit.max_system_mem_limit = mem - (mem >> 4);
kfd_mem_limit.max_ttm_mem_limit = (mem >> 1) - (mem >> 3);
pr_debug("Kernel memory limit %lluM, TTM limit %lluM\n",
(kfd_mem_limit.max_system_mem_limit >> 20),
@@ -148,6 +151,10 @@ static int amdgpu_amdkfd_reserve_mem_limit(struct
amdgpu_device *adev,
spin_lock(_mem_limit.mem_limit_lock);
+ if (kfd_mem_limit.system_mem_used + system_mem_needed >
+ kfd_mem_limit.max_system_mem_limit)
+ pr_debug("Set no_system_mem_limit if using shared memory\n");
+
if ((kfd_mem_limit.system_mem_used + system_mem_needed >
kfd_mem_limit.max_system_mem_limit) ||
(kfd_mem_limit.ttm_mem_used + ttm_mem_needed >
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 6291f5f0d223..e9acd0a9f327 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -715,6 +715,15 @@ MODULE_PARM_DESC(queue_preemption_timeout_ms, "queue
preemption timeout in ms (1
bool debug_evictions;
module_param(debug_evictions, bool, 0644);
MODULE_PARM_DESC(debug_evictions, "enable eviction debug messages (false =
default)");
+
+/**
+ * DOC: no_system_mem_limit(bool)
+ * Disable system memory limit, to support multiple process shared memory
+ */
+bool no_system_mem_limit;
+module_param(no_system_mem_limit, bool, 0644);
+MODULE_PARM_DESC(no_system_mem_limit, "disable system memory limit (false =
default)");
+
#endif
/**
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [RFC PATCH v1 0/22] backlight: add init macros and accessors
On Sun, Aug 02, 2020 at 01:06:14PM +0200, Sam Ravnborg wrote: > The backlight drivers uses several different patterns when registering > a backlight: > > - Register backlight and assign properties later > - Define a local backlight_properties variable and use memset > - Define a const backlight_properties and assign relevant properties > > On top of this there was differences in what members was assigned in > backlight_properties. > > To align how backlight drivers are initialized introduce following helper > macros: > - DECLARE_BACKLIGHT_INIT_FIRMWARE() > - DECLARE_BACKLIGHT_INIT_PLATFORM() > - DECLARE_BACKLIGHT_INIT_RAW() > > The macros are introduced in patch 2. > > The backlight drivers used direct access to backlight_properties. > Encapsulate these in get/set access operations resulting in following > benefits: > - The drivers no longer need to be concerned about the confusing power states, > as there is now only a set_power_on() and set_power_off() operation. > - The access methods can be called with a NULL pointer so logic around the > access can be made simpler. > - The code is in most cases more readable with the access operations. > - When everyone uses the access methods refactroring in the backlight core is > simpler. > > The get/set operations are introduced in patch 3. > > The first patch trims backlight_update_status() so it can be called with a > NULL > backlight_device. Then the called do not need to add this check just to avoid > a NULL reference. > > The fourth patch introduce the new macros and get/set operations for the > gpio backlight driver, as an example. > > The remaining patches updates most backlight users in drivers/gpu/drm/* > With this patch set applied then: > - Almost all references to FB_BLANK* are gone from drm/* > - All panel drivers uses devm_ variant for registering backlight > - Almost all direct references to backlight properties are gone > > The drm/* patches are used as examples how drivers can benefit from the > new macros and get/set operations. > > Individual patches are only sent to the people listed in the patch + a few > more. > Please check https://lore.kernel.org/dri-devel/ for the full series. > > Feedback welcome! Since this needs backlight patches queued up outside of drm there's two options: - merge the backlight stuff through drm-misc (imo simplest, we have all the fbdev stuff in there too by now) - shared topic branch merged in drm-misc and optionally backlight tree Otherwise this is going to be a pain to merge. -Daniel > > Sam > > Cc: Alex Deucher > Cc: amd-gfx@lists.freedesktop.org > Cc: Andrzej Hajda > Cc: Christian König > Cc: Chris Wilson > Cc: Daniel Thompson > Cc: Ezequiel Garcia > Cc: Hans de Goede > Cc: Hoegeun Kwon > Cc: Inki Dae > Cc: Jani Nikula > Cc: Jernej Skrabec > Cc: Jingoo Han > Cc: Jonas Karlman > Cc: Joonas Lahtinen > Cc: Jyri Sarha > Cc: Kieran Bingham > Cc: Konrad Dybcio > Cc: Laurent Pinchart > Cc: Lee Jones > Cc: Linus Walleij > Cc: linux-renesas-...@vger.kernel.org > Cc: Maarten Lankhorst > Cc: Manasi Navare > Cc: Neil Armstrong > Cc: Patrik Jakobsson > Cc: Paweł Chmiel > Cc: Philippe CORNU > Cc: Rob Clark > Cc: Robert Chiras > Cc: Rodrigo Vivi > Cc: Sam Ravnborg > Cc: Sebastian Reichel > Cc: Thierry Reding > Cc: Tomi Valkeinen > Cc: "Ville Syrjälä" > Cc: Vinay Simha BN > Cc: Wambui Karuga > Cc: Zheng Bin > > Sam Ravnborg (22): > backlight: Silently fail backlight_update_status() if no device > backlight: Add DECLARE_* macro for device registration > backlight: Add get/set operations for brightness/power properties > backlight: gpio: Use DECLARE_BACKLIGHT_INIT_RAW and get/setters > drm/gma500: Backlight support > drm/panel: asus-z00t-tm5p5-n35596: Backlight update > drm/panel: jdi-lt070me05000: Backlight update > drm/panel: novatek-nt35510: Backlight update > drm/panel: orisetech-otm8009a: Backlight update > drm/panel: raydium-rm67191: Backlight update > drm/panel: samsung-s6e63m0: Backlight update > drm/panel: samsung-s6e63j0x03: Backlight update > drm/panel: samsung-s6e3ha2: Backlight update > drm/panel: sony-acx424akp: Backlight update > drm/panel: sony-acx565akm: Backlight update > drm/bridge: parade-ps8622: Backlight update > drm/tilcdc: Backlight update > drm/radeon: Backlight update > drm/amdgpu/atom: Backlight update > drm/i915: Backlight update > drm/omap: display: Backlight update > drm/shmobile: Backlight update > > drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 15 ++- > drivers/gpu/drm/bridge/parade-ps8622.c | 43 > drivers/gpu/drm/gma500/backlight.c | 35 ++ > drivers/gpu/drm/gma500/cdv_device.c| 29 +++-- > drivers/gpu/drm/gma500/mdfld_device.c | 9 +- > drivers/gpu/drm/gma500/oaktrail_device.c | 10 +- >
Re: [PATCH] drm/amd/display: use correct scale for actual_brightness
On 2020-08-04 12:28 p.m., Alexander Monakov wrote:
On Tue, 4 Aug 2020, Kazlauskas, Nicholas wrote:
This is a cleaner change the other proposed patch since it doesn't need to
Can you give a URL to the other patch please?
Sorry, replied to the wrong email by accident here.
The other change was modifying the max_brightness range and rescaling
internal min/max defaults.
I don't think it was sent out to the list yet.
Regards,
Nicholas Kazlauskas
modify the exist conversion functions but I'd be worried about broken
userspace relying on 0-255 as the only acceptable range.
Not sure what you mean by this. Userspace simply reads the maximum value from
max_brightness sysfs file. On other gpu/firmware combinations it can be 7 or 9
for example, it just happens to be 255 with modern amdgpu. Minimum value is
always zero.
Value seen in max_brightness remains 255 with this patch, so as far as userspace
is concerned nothing is changed apart from value given by actual_brightness
file.
Alexander
Not an expert on existing implementations to point out a specific one though.
Regards,
Nicholas Kazlauskas
On 2020-08-03 4:02 p.m., Alexander Monakov wrote:
Documentation for sysfs backlight level interface requires that
values in both 'brightness' and 'actual_brightness' files are
interpreted to be in range from 0 to the value given in the
'max_brightness' file.
With amdgpu, max_brightness gives 255, and values written by the user
into 'brightness' are internally rescaled to a wider range. However,
reading from 'actual_brightness' gives the raw register value without
inverse rescaling. This causes issues for various userspace tools such
as PowerTop and systemd that expect the value to be in the correct
range.
Introduce a helper to retrieve internal backlight range. Extend the
existing 'convert_brightness' function to handle conversion in both
directions.
Bug: https://bugzilla.kernel.org/show_bug.cgi?id=203905
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1242
Cc: Alex Deucher
Signed-off-by: Alexander Monakov
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 73 ---
1 file changed, 32 insertions(+), 41 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 710edc70e37e..03e21e7b7917 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2881,51 +2881,42 @@ static int set_backlight_via_aux(struct dc_link
*link, uint32_t brightness)
return rc ? 0 : 1;
}
-static u32 convert_brightness(const struct amdgpu_dm_backlight_caps
*caps,
- const uint32_t user_brightness)
+static int get_brightness_range(const struct amdgpu_dm_backlight_caps
*caps,
+ unsigned *min, unsigned *max)
{
- u32 min, max, conversion_pace;
- u32 brightness = user_brightness;
-
if (!caps)
- goto out;
+ return 0;
-if (!caps->aux_support) {
- max = caps->max_input_signal;
- min = caps->min_input_signal;
- /*
-* The brightness input is in the range 0-255
-* It needs to be rescaled to be between the
-* requested min and max input signal
-* It also needs to be scaled up by 0x101 to
-* match the DC interface which has a range of
-* 0 to 0x
-*/
- conversion_pace = 0x101;
- brightness =
- user_brightness
- * conversion_pace
- * (max - min)
- / AMDGPU_MAX_BL_LEVEL
- + min * conversion_pace;
+ if (caps->aux_support) {
+ // Firmware limits are in nits, DC API wants millinits.
+ *max = 1000 * caps->aux_max_input_signal;
+ *min = 1000 * caps->aux_min_input_signal;
} else {
- /* TODO
-* We are doing a linear interpolation here, which is OK but
-* does not provide the optimal result. We probably want
-* something close to the Perceptual Quantizer (PQ) curve.
-*/
- max = caps->aux_max_input_signal;
- min = caps->aux_min_input_signal;
-
- brightness = (AMDGPU_MAX_BL_LEVEL - user_brightness) * min
- + user_brightness * max;
- // Multiple the value by 1000 since we use millinits
- brightness *= 1000;
- brightness = DIV_ROUND_CLOSEST(brightness,
AMDGPU_MAX_BL_LEVEL);
+ // Firmware limits are 8-bit, PWM control is 16-bit.
+ *max = 0x101 * caps->max_input_signal;
+ *min = 0x101 * caps->min_input_signal;
}
+ return 1;
+}
-out:
- return brightness;
+static u32
Re: [PATCH] drm/amd/display: use correct scale for actual_brightness
On 2020-08-03 4:02 p.m., Alexander Monakov wrote:
Documentation for sysfs backlight level interface requires that
values in both 'brightness' and 'actual_brightness' files are
interpreted to be in range from 0 to the value given in the
'max_brightness' file.
With amdgpu, max_brightness gives 255, and values written by the user
into 'brightness' are internally rescaled to a wider range. However,
reading from 'actual_brightness' gives the raw register value without
inverse rescaling. This causes issues for various userspace tools such
as PowerTop and systemd that expect the value to be in the correct
range.
Introduce a helper to retrieve internal backlight range. Extend the
existing 'convert_brightness' function to handle conversion in both
directions.
Bug: https://bugzilla.kernel.org/show_bug.cgi?id=203905
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1242
Cc: Alex Deucher
Signed-off-by: Alexander Monakov
Overall approach seems reasonable, nice catch.
I suggest to add convert_to_user_brightness() instead of making
from_user a flag and extending the current functionality though. It
makes it more clear from the call site what's happening.
Regards,
Nicholas Kazlauskas
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 73 ---
1 file changed, 32 insertions(+), 41 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 710edc70e37e..03e21e7b7917 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2881,51 +2881,42 @@ static int set_backlight_via_aux(struct dc_link *link,
uint32_t brightness)
return rc ? 0 : 1;
}
-static u32 convert_brightness(const struct amdgpu_dm_backlight_caps *caps,
- const uint32_t user_brightness)
+static int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps,
+ unsigned *min, unsigned *max)
{
- u32 min, max, conversion_pace;
- u32 brightness = user_brightness;
-
if (!caps)
- goto out;
+ return 0;
- if (!caps->aux_support) {
- max = caps->max_input_signal;
- min = caps->min_input_signal;
- /*
-* The brightness input is in the range 0-255
-* It needs to be rescaled to be between the
-* requested min and max input signal
-* It also needs to be scaled up by 0x101 to
-* match the DC interface which has a range of
-* 0 to 0x
-*/
- conversion_pace = 0x101;
- brightness =
- user_brightness
- * conversion_pace
- * (max - min)
- / AMDGPU_MAX_BL_LEVEL
- + min * conversion_pace;
+ if (caps->aux_support) {
+ // Firmware limits are in nits, DC API wants millinits.
+ *max = 1000 * caps->aux_max_input_signal;
+ *min = 1000 * caps->aux_min_input_signal;
} else {
- /* TODO
-* We are doing a linear interpolation here, which is OK but
-* does not provide the optimal result. We probably want
-* something close to the Perceptual Quantizer (PQ) curve.
-*/
- max = caps->aux_max_input_signal;
- min = caps->aux_min_input_signal;
-
- brightness = (AMDGPU_MAX_BL_LEVEL - user_brightness) * min
- + user_brightness * max;
- // Multiple the value by 1000 since we use millinits
- brightness *= 1000;
- brightness = DIV_ROUND_CLOSEST(brightness, AMDGPU_MAX_BL_LEVEL);
+ // Firmware limits are 8-bit, PWM control is 16-bit.
+ *max = 0x101 * caps->max_input_signal;
+ *min = 0x101 * caps->min_input_signal;
}
+ return 1;
+}
-out:
- return brightness;
+static u32 convert_brightness(const struct amdgpu_dm_backlight_caps *caps,
+ const uint32_t brightness, int from_user)
+{
+ unsigned min, max;
+
+ if (!get_brightness_range(caps, , ))
+ return brightness;
+
+ if (from_user)
+ // Rescale 0..255 to min..max
+ return min + DIV_ROUND_CLOSEST((max - min) * brightness,
+ AMDGPU_MAX_BL_LEVEL);
+
+ if (brightness < min)
+ return 0;
+ // Rescale min..max to 0..255
+ return DIV_ROUND_CLOSEST(AMDGPU_MAX_BL_LEVEL * (brightness - min),
+max - min);
}
static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
@@ -2941,7 +2932,7 @@ static int amdgpu_dm_backlight_update_status(struct
backlight_device *bd)
Re: [PATCH] drm/amd/display: use correct scale for actual_brightness
This is a cleaner change the other proposed patch since it doesn't need
to modify the exist conversion functions but I'd be worried about broken
userspace relying on 0-255 as the only acceptable range.
Not an expert on existing implementations to point out a specific one
though.
Regards,
Nicholas Kazlauskas
On 2020-08-03 4:02 p.m., Alexander Monakov wrote:
Documentation for sysfs backlight level interface requires that
values in both 'brightness' and 'actual_brightness' files are
interpreted to be in range from 0 to the value given in the
'max_brightness' file.
With amdgpu, max_brightness gives 255, and values written by the user
into 'brightness' are internally rescaled to a wider range. However,
reading from 'actual_brightness' gives the raw register value without
inverse rescaling. This causes issues for various userspace tools such
as PowerTop and systemd that expect the value to be in the correct
range.
Introduce a helper to retrieve internal backlight range. Extend the
existing 'convert_brightness' function to handle conversion in both
directions.
Bug: https://bugzilla.kernel.org/show_bug.cgi?id=203905
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1242
Cc: Alex Deucher
Signed-off-by: Alexander Monakov
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 73 ---
1 file changed, 32 insertions(+), 41 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 710edc70e37e..03e21e7b7917 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2881,51 +2881,42 @@ static int set_backlight_via_aux(struct dc_link *link,
uint32_t brightness)
return rc ? 0 : 1;
}
-static u32 convert_brightness(const struct amdgpu_dm_backlight_caps *caps,
- const uint32_t user_brightness)
+static int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps,
+ unsigned *min, unsigned *max)
{
- u32 min, max, conversion_pace;
- u32 brightness = user_brightness;
-
if (!caps)
- goto out;
+ return 0;
- if (!caps->aux_support) {
- max = caps->max_input_signal;
- min = caps->min_input_signal;
- /*
-* The brightness input is in the range 0-255
-* It needs to be rescaled to be between the
-* requested min and max input signal
-* It also needs to be scaled up by 0x101 to
-* match the DC interface which has a range of
-* 0 to 0x
-*/
- conversion_pace = 0x101;
- brightness =
- user_brightness
- * conversion_pace
- * (max - min)
- / AMDGPU_MAX_BL_LEVEL
- + min * conversion_pace;
+ if (caps->aux_support) {
+ // Firmware limits are in nits, DC API wants millinits.
+ *max = 1000 * caps->aux_max_input_signal;
+ *min = 1000 * caps->aux_min_input_signal;
} else {
- /* TODO
-* We are doing a linear interpolation here, which is OK but
-* does not provide the optimal result. We probably want
-* something close to the Perceptual Quantizer (PQ) curve.
-*/
- max = caps->aux_max_input_signal;
- min = caps->aux_min_input_signal;
-
- brightness = (AMDGPU_MAX_BL_LEVEL - user_brightness) * min
- + user_brightness * max;
- // Multiple the value by 1000 since we use millinits
- brightness *= 1000;
- brightness = DIV_ROUND_CLOSEST(brightness, AMDGPU_MAX_BL_LEVEL);
+ // Firmware limits are 8-bit, PWM control is 16-bit.
+ *max = 0x101 * caps->max_input_signal;
+ *min = 0x101 * caps->min_input_signal;
}
+ return 1;
+}
-out:
- return brightness;
+static u32 convert_brightness(const struct amdgpu_dm_backlight_caps *caps,
+ const uint32_t brightness, int from_user)
+{
+ unsigned min, max;
+
+ if (!get_brightness_range(caps, , ))
+ return brightness;
+
+ if (from_user)
+ // Rescale 0..255 to min..max
+ return min + DIV_ROUND_CLOSEST((max - min) * brightness,
+ AMDGPU_MAX_BL_LEVEL);
+
+ if (brightness < min)
+ return 0;
+ // Rescale min..max to 0..255
+ return DIV_ROUND_CLOSEST(AMDGPU_MAX_BL_LEVEL * (brightness - min),
+max - min);
}
static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
@@ -2941,7 +2932,7 @@ static int
RE: [PATCH] drm/amd/dc: Read from DP_SINK_COUNT_ESI for DPDC r1.2 or higher
[AMD Official Use Only - Internal Distribution Only] -Original Message- From: Pillai, Aurabindo Sent: Tuesday, August 4, 2020 9:56 AM To: Lin, Wayne ; amd-gfx@lists.freedesktop.org Cc: Wu, Hersen ; Kazlauskas, Nicholas ; Siqueira, Rodrigo ; Zuo, Jerry Subject: Re: [PATCH] drm/amd/dc: Read from DP_SINK_COUNT_ESI for DPDC r1.2 or higher On Tue, 2020-08-04 at 11:42 +0800, Wayne Lin wrote: > [Why] > According to DP spec, DPRX with DPCD r1.2 or higher shall have the > same Link/Sink Device Status field registers at DPCD Addresses 00200h > through 00205h to the corresponding DPRX Event Status Indicator > registers at DPCD Addresses 02002h through 0200Fh. We now only read > from 02002h when DPCD revision number is r1.4 or higher while handling > short HPD. Need to correct that. > > [How] > Set to read from 02002h when DPCD is r1.2 or higher > > Signed-off-by: Wayne Lin < > wayne@amd.com > > > --- > drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > index 2bfa4e35c2cf..9fb1543b4c73 100644 > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > @@ -1834,9 +1834,9 @@ static enum dc_status read_hpd_rx_irq_data( >* fail, so we now explicitly read 6 bytes which is >* the req from the above mentioned test cases. >* > - * For DP 1.4 we need to read those from 2002h range. > + * For DPCD r1.2 or higher, we need to read those from 2002h > range. >*/ > - if (link->dpcd_caps.dpcd_rev.raw < DPCD_REV_14) > + if (link->dpcd_caps.dpcd_rev.raw < DPCD_REV_12) > retval = core_link_read_dpcd( > link, > DP_SINK_COUNT, Reviewed-by: Aurabindo Pillai ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/dc: Read from DP_SINK_COUNT_ESI for DPDC r1.2 or higher
On Tue, 2020-08-04 at 11:42 +0800, Wayne Lin wrote: > [Why] > According to DP spec, DPRX with DPCD r1.2 or higher shall have the > same Link/Sink Device Status field registers at DPCD Addresses 00200h > through 00205h to the corresponding DPRX Event Status Indicator > registers at DPCD Addresses 02002h through 0200Fh. We now only read > from > 02002h when DPCD revision number is r1.4 or higher while handling > short > HPD. Need to correct that. > > [How] > Set to read from 02002h when DPCD is r1.2 or higher > > Signed-off-by: Wayne Lin < > wayne@amd.com > > > --- > drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > index 2bfa4e35c2cf..9fb1543b4c73 100644 > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > @@ -1834,9 +1834,9 @@ static enum dc_status read_hpd_rx_irq_data( >* fail, so we now explicitly read 6 bytes which is >* the req from the above mentioned test cases. >* > - * For DP 1.4 we need to read those from 2002h range. > + * For DPCD r1.2 or higher, we need to read those from 2002h > range. >*/ > - if (link->dpcd_caps.dpcd_rev.raw < DPCD_REV_14) > + if (link->dpcd_caps.dpcd_rev.raw < DPCD_REV_12) > retval = core_link_read_dpcd( > link, > DP_SINK_COUNT, Reviewed-by: Aurabindo Pillai ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Enabling AMDGPU by default for SI & CIK
Am 04.08.20 um 10:37 schrieb Michel Dänzer: On 2020-08-03 1:45 a.m., Bas Nieuwenhuizen wrote: Hi all, Now that we have recently made some progress on getting feature parity with the Radeon driver for SI, I'm wondering what it would take to make AMDGPU the default driver for these generations. As far as I understand AMDGPU has had these features for CIK for a while already but it is still not the default driver. What would it take to make it the default? What is missing and/or broken? The main blockers I'm aware of for CIK are: 1) Lack of analogue connector support with DC 2) Lack of HDMI/DP audio support without DC 1) may apply to SI as well. Yes, analog connector support is the biggest problem. HDMI/DP audio is just lacking a few register programming as far as I know, but analog support needs to be somehow added to the DC design. Regards, Christian. ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Enabling AMDGPU by default for SI & CIK
On 2020-08-03 1:45 a.m., Bas Nieuwenhuizen wrote: > Hi all, > > Now that we have recently made some progress on getting feature parity > with the Radeon driver for SI, I'm wondering what it would take to > make AMDGPU the default driver for these generations. > > As far as I understand AMDGPU has had these features for CIK for a > while already but it is still not the default driver. What would it > take to make it the default? What is missing and/or broken? The main blockers I'm aware of for CIK are: 1) Lack of analogue connector support with DC 2) Lack of HDMI/DP audio support without DC 1) may apply to SI as well. -- Earthling Michel Dänzer | https://redhat.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: [PATCH 1/2] drm/amdgpu: fix reload KMD hang on KIQ
[AMD Official Use Only - Internal Distribution Only]
>-Original Message-
>From: amd-gfx On Behalf Of Liu,
>Monk
>Sent: Tuesday, August 4, 2020 2:31 PM
>To: amd-gfx@lists.freedesktop.org
>Subject: RE: [PATCH 1/2] drm/amdgpu: fix reload KMD hang on KIQ
>
>[AMD Official Use Only - Internal Distribution Only]
>
>[AMD Official Use Only - Internal Distribution Only]
>
>Ping ... this is a severe bug fix
>
>_
>Monk Liu|GPU Virtualization Team |AMD
>
>
>-Original Message-
>From: amd-gfx On Behalf Of Liu,
>Monk
>Sent: Monday, August 3, 2020 9:55 AM
>To: Kuehling, Felix ; amd-gfx@lists.freedesktop.org
>Subject: RE: [PATCH 1/2] drm/amdgpu: fix reload KMD hang on KIQ
>
>[AMD Official Use Only - Internal Distribution Only]
>
>[AMD Official Use Only - Internal Distribution Only]
>
>>>In gfx_v10_0_sw_fini the KIQ ring gets freed. Wouldn't that be the
>>>right place to stop the KIQ
>
>KIQ (CPC) will never being stopped (the DISABLE on CPC is skipped for SRIOV )
>for SRIOV in SW_FINI because SRIOV relies on KIQ to do world switch
>
>But this is really a weird bug because even with the same approach it doesn't
>make KIQ (CP) hang on GFX9, only GFX10 need this patch
>
>By now I do not have other good explanation or better fix than this one
>
>_
>Monk Liu|GPU Virtualization Team |AMD
>
>
>-Original Message-
>From: Kuehling, Felix
>Sent: Friday, July 31, 2020 9:57 PM
>To: Liu, Monk ; amd-gfx@lists.freedesktop.org
>Subject: Re: [PATCH 1/2] drm/amdgpu: fix reload KMD hang on KIQ
>
>In gfx_v10_0_sw_fini the KIQ ring gets freed. Wouldn't that be the right place
>to stop the KIQ? Otherwise KIQ will hang as soon as someone allocates the
>memory that was previously used for the KIQ ring buffer and overwrites it with
>something that's not a valid PM4 packet.
>
>Regards,
> Felix
>
>Am 2020-07-31 um 3:51 a.m. schrieb Monk Liu:
>> KIQ will hang if we try below steps:
>> modprobe amdgpu
>> rmmod amdgpu
>> modprobe amdgpu sched_hw_submission=4
>>
>> the cause is that due to KIQ is always living there even after we
>> unload KMD thus when doing the realod of KMD KIQ will crash upon its
>> register programed with different values with the previous
>> configuration (the config like HQD addr, ring size, is easily changed
>> if we alter the sched_hw_submission)
>>
>> the fix is we must inactive KIQ first before touching any of its
>> registgers
>>
>> Signed-off-by: Monk Liu
>> ---
>> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> index db9f1e8..f571e25 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> @@ -6433,6 +6433,9 @@ static int gfx_v10_0_kiq_init_register(struct
>> amdgpu_ring *ring) struct v10_compute_mqd *mqd = ring->mqd_ptr; int
>> j;
>>
>> +/* activate the queue */
>> +WREG32_SOC15(GC, 0, mmCP_HQD_ACTIVE, 0);
>> +
Could we move follow to here?
if (RREG32_SOC15(GC, 0, mmCP_HQD_ACTIVE) & 1) {
WREG32_SOC15(GC, 0, mmCP_HQD_DEQUEUE_REQUEST, 1);
for (j = 0; j < adev->usec_timeout; j++) {
if (!(RREG32_SOC15(GC, 0, mmCP_HQD_ACTIVE) & 1))
break;
udelay(1);
}
>> /* disable wptr polling */
>> WREG32_FIELD15(GC, 0, CP_PQ_WPTR_POLL_CNTL, EN, 0);
>>
>___
>amd-gfx mailing list
>amd-gfx@lists.freedesktop.org
>https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.fre
>edesktop.org%2Fmailman%2Flistinfo%2Famd-
>gfxdata=02%7C01%7CEmily.Deng%40amd.com%7C1236f42617d246b20
>bc108d8384007e4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7
>C637321194957236933sdata=0%2BzHvJ1n4TZOYss4v1pR6i8bxq46JE73
>UIi%2B49x9joU%3Dreserved=0
>___
>amd-gfx mailing list
>amd-gfx@lists.freedesktop.org
>https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.fre
>edesktop.org%2Fmailman%2Flistinfo%2Famd-
>gfxdata=02%7C01%7CEmily.Deng%40amd.com%7C1236f42617d246b20
>bc108d8384007e4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7
>C637321194957236933sdata=0%2BzHvJ1n4TZOYss4v1pR6i8bxq46JE73
>UIi%2B49x9joU%3Dreserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v6 12/12] x86/traps: Fix up invalid PASID
> On Aug 3, 2020, at 10:34 AM, Dave Hansen wrote: > > On 8/3/20 10:16 AM, Andy Lutomirski wrote: >> - TILE: genuinely per-thread, but it's expensive so it's >> lazy-loadable. But the lazy-load mechanism reuses #NM, and it's not >> fully disambiguated from the other use of #NM. So it sort of works, >> but it's gross. > > For those playing along at home, there's a new whitepaper out from Intel > about some new CPU features which are going to be fun: > >> https://software.intel.com/content/dam/develop/public/us/en/documents/architecture-instruction-set-extensions-programming-reference.pdf > > Which part were you worried about? I thought it was fully disambuguated > from this: > >> When XFD causes an instruction to generate #NM, the processor loads >> the IA32_XFD_ERR MSR to identify the disabled state component(s). >> Specifically, the MSR is loaded with the logical AND of the IA32_XFD >> MSR and the bitmap corresponding to the state components required by >> the faulting instruction. >> >> Device-not-available exceptions that are not due to XFD — those >> resulting from setting CR0.TS to 1 — do not modify the IA32_XFD_ERR >> MSR. > > So if you always make sure to *clear* IA32_XFD_ERR after handing and XFD > exception, any #NM's with a clear IA32_XFD_ERR are from "legacy" > CR0.TS=1. Any bits set in IA32_XFD_ERR mean a new-style XFD exception. > > Am I missing something? I don’t think you’re missing anything, but this mechanism seems to be solidly in the category of “just barely works”, kind of like #DB and DR6, which also just barely works. And this PASID vs #GP mess is just sad. We’re trying to engineer around an issue that has no need to exist in the first place. For some reason we have two lazy-loading-fault mechanisms showing up in x86 in rapid succession, one of them is maybe 3/4-baked, and the other is totally separate and maybe 1/4 baked. If ENQCMD instead raise #NM, this would be trivial. (And it even makes sense — the error is, literally, “an instruction tried to use something in XSTATE that isn’t initialized”.). Or if someone had noticed that, kind of like PKRU, PASID didn’t really belong in XSTATE, we wouldn’t have this mess. Yes, obviously Linux can get all this stuff to work, but maybe Intel could aspire to design features that are straightforward to use well instead of merely possible to use well? ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amd/display: use correct scale for actual_brightness
Documentation for sysfs backlight level interface requires that
values in both 'brightness' and 'actual_brightness' files are
interpreted to be in range from 0 to the value given in the
'max_brightness' file.
With amdgpu, max_brightness gives 255, and values written by the user
into 'brightness' are internally rescaled to a wider range. However,
reading from 'actual_brightness' gives the raw register value without
inverse rescaling. This causes issues for various userspace tools such
as PowerTop and systemd that expect the value to be in the correct
range.
Introduce a helper to retrieve internal backlight range. Extend the
existing 'convert_brightness' function to handle conversion in both
directions.
Bug: https://bugzilla.kernel.org/show_bug.cgi?id=203905
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1242
Cc: Alex Deucher
Signed-off-by: Alexander Monakov
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 73 ---
1 file changed, 32 insertions(+), 41 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 710edc70e37e..03e21e7b7917 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2881,51 +2881,42 @@ static int set_backlight_via_aux(struct dc_link *link,
uint32_t brightness)
return rc ? 0 : 1;
}
-static u32 convert_brightness(const struct amdgpu_dm_backlight_caps *caps,
- const uint32_t user_brightness)
+static int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps,
+ unsigned *min, unsigned *max)
{
- u32 min, max, conversion_pace;
- u32 brightness = user_brightness;
-
if (!caps)
- goto out;
+ return 0;
- if (!caps->aux_support) {
- max = caps->max_input_signal;
- min = caps->min_input_signal;
- /*
-* The brightness input is in the range 0-255
-* It needs to be rescaled to be between the
-* requested min and max input signal
-* It also needs to be scaled up by 0x101 to
-* match the DC interface which has a range of
-* 0 to 0x
-*/
- conversion_pace = 0x101;
- brightness =
- user_brightness
- * conversion_pace
- * (max - min)
- / AMDGPU_MAX_BL_LEVEL
- + min * conversion_pace;
+ if (caps->aux_support) {
+ // Firmware limits are in nits, DC API wants millinits.
+ *max = 1000 * caps->aux_max_input_signal;
+ *min = 1000 * caps->aux_min_input_signal;
} else {
- /* TODO
-* We are doing a linear interpolation here, which is OK but
-* does not provide the optimal result. We probably want
-* something close to the Perceptual Quantizer (PQ) curve.
-*/
- max = caps->aux_max_input_signal;
- min = caps->aux_min_input_signal;
-
- brightness = (AMDGPU_MAX_BL_LEVEL - user_brightness) * min
- + user_brightness * max;
- // Multiple the value by 1000 since we use millinits
- brightness *= 1000;
- brightness = DIV_ROUND_CLOSEST(brightness, AMDGPU_MAX_BL_LEVEL);
+ // Firmware limits are 8-bit, PWM control is 16-bit.
+ *max = 0x101 * caps->max_input_signal;
+ *min = 0x101 * caps->min_input_signal;
}
+ return 1;
+}
-out:
- return brightness;
+static u32 convert_brightness(const struct amdgpu_dm_backlight_caps *caps,
+ const uint32_t brightness, int from_user)
+{
+ unsigned min, max;
+
+ if (!get_brightness_range(caps, , ))
+ return brightness;
+
+ if (from_user)
+ // Rescale 0..255 to min..max
+ return min + DIV_ROUND_CLOSEST((max - min) * brightness,
+ AMDGPU_MAX_BL_LEVEL);
+
+ if (brightness < min)
+ return 0;
+ // Rescale min..max to 0..255
+ return DIV_ROUND_CLOSEST(AMDGPU_MAX_BL_LEVEL * (brightness - min),
+max - min);
}
static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
@@ -2941,7 +2932,7 @@ static int amdgpu_dm_backlight_update_status(struct
backlight_device *bd)
link = (struct dc_link *)dm->backlight_link;
- brightness = convert_brightness(, bd->props.brightness);
+ brightness = convert_brightness(, bd->props.brightness, 1);
// Change brightness based on AUX property
if (caps.aux_support)
return set_backlight_via_aux(link,
RE: [PATCH 1/2] drm/amdgpu: fix reload KMD hang on KIQ
[AMD Official Use Only - Internal Distribution Only] Ping ... this is a severe bug fix _ Monk Liu|GPU Virtualization Team |AMD -Original Message- From: amd-gfx On Behalf Of Liu, Monk Sent: Monday, August 3, 2020 9:55 AM To: Kuehling, Felix ; amd-gfx@lists.freedesktop.org Subject: RE: [PATCH 1/2] drm/amdgpu: fix reload KMD hang on KIQ [AMD Official Use Only - Internal Distribution Only] [AMD Official Use Only - Internal Distribution Only] >>In gfx_v10_0_sw_fini the KIQ ring gets freed. Wouldn't that be the >>right place to stop the KIQ KIQ (CPC) will never being stopped (the DISABLE on CPC is skipped for SRIOV ) for SRIOV in SW_FINI because SRIOV relies on KIQ to do world switch But this is really a weird bug because even with the same approach it doesn't make KIQ (CP) hang on GFX9, only GFX10 need this patch By now I do not have other good explanation or better fix than this one _ Monk Liu|GPU Virtualization Team |AMD -Original Message- From: Kuehling, Felix Sent: Friday, July 31, 2020 9:57 PM To: Liu, Monk ; amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 1/2] drm/amdgpu: fix reload KMD hang on KIQ In gfx_v10_0_sw_fini the KIQ ring gets freed. Wouldn't that be the right place to stop the KIQ? Otherwise KIQ will hang as soon as someone allocates the memory that was previously used for the KIQ ring buffer and overwrites it with something that's not a valid PM4 packet. Regards, Felix Am 2020-07-31 um 3:51 a.m. schrieb Monk Liu: > KIQ will hang if we try below steps: > modprobe amdgpu > rmmod amdgpu > modprobe amdgpu sched_hw_submission=4 > > the cause is that due to KIQ is always living there even after we > unload KMD thus when doing the realod of KMD KIQ will crash upon its > register programed with different values with the previous > configuration (the config like HQD addr, ring size, is easily changed > if we alter the sched_hw_submission) > > the fix is we must inactive KIQ first before touching any of its > registgers > > Signed-off-by: Monk Liu > --- > drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c > b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c > index db9f1e8..f571e25 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c > @@ -6433,6 +6433,9 @@ static int gfx_v10_0_kiq_init_register(struct > amdgpu_ring *ring) struct v10_compute_mqd *mqd = ring->mqd_ptr; int > j; > > +/* activate the queue */ > +WREG32_SOC15(GC, 0, mmCP_HQD_ACTIVE, 0); > + > /* disable wptr polling */ > WREG32_FIELD15(GC, 0, CP_PQ_WPTR_POLL_CNTL, EN, 0); > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cmonk.liu%40amd.com%7C4837e2d566b44af845f608d837503a3b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637320165018899834sdata=TED%2BkhlYyAIyTmLJAZBBBHHnE6PRg4fpUsZhD9ke%2BPU%3Dreserved=0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
