[PATCH 0/6] Initial sclk OD support for amdgpu

[PATCH 0/6] Initial sclk OD support for amdgpu

[Alex Deucher]

This adds initial OverDrive (OD) support for the gfx engine
clock (sclk).  It's enabled by selecting a percentage (0-20)
and writing it to a new sysfs file.  It's currently available
on Tonga, Fiji, and Polaris.

Eric Huang (6):
  drm/amd/powerplay: fix a bug on updating sclk for Fiji
  drm/amd/powerplay: fix a bug on updating sclk for Tonga
  drm/amdgpu: add powerplay sclk OD support through sysfs
  drm/amd/powerplay: add sclk OD support on Fiji
  drm/amd/powerplay: add sclk OD support on Tonga
  drm/amd/powerplay: add sclk OD support on Polaris10

 drivers/gpu/drm/amd/amdgpu/amdgpu.h                |  6 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c             | 49 ++++++++++++++++++++++
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c      | 44 +++++++++++++++++++
 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c   | 45 +++++++++++++++++++-
 .../gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c  | 44 +++++++++++++++++++
 drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c  | 46 +++++++++++++++++++-
 drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h  |  2 +
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h          |  2 +
 8 files changed, 236 insertions(+), 2 deletions(-)

-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/6] drm/amd/powerplay: fix a bug on updating sclk for Fiji

[Alex Deucher]

From: Eric Huang <JinHuiEric.Huang@amd.com>

Update sclk smc table rather than mclk smc table for sclk updates.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
index 382223b..6f1bad4 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
@@ -4314,7 +4314,7 @@ static int fiji_populate_and_upload_sclk_mclk_dpm_levels(
 
 	if (data->need_update_smu7_dpm_table &
 			(DPMTABLE_OD_UPDATE_SCLK + DPMTABLE_UPDATE_SCLK)) {
-		result = fiji_populate_all_memory_levels(hwmgr);
+		result = fiji_populate_all_graphic_levels(hwmgr);
 		PP_ASSERT_WITH_CODE((0 == result),
 				"Failed to populate SCLK during PopulateNewDPMClocksStates Function!",
 				return result);
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/6] drm/amd/powerplay: fix a bug on updating sclk for Tonga

[Alex Deucher]

From: Eric Huang <JinHuiEric.Huang@amd.com>

Update sclk smc table rather than mclk smc table for sclk updates.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
index 1faad92..cb28335 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
@@ -5445,7 +5445,7 @@ static int tonga_populate_and_upload_sclk_mclk_dpm_levels(struct pp_hwmgr *hwmgr
 	}
 
 	if (data->need_update_smu7_dpm_table & (DPMTABLE_OD_UPDATE_SCLK + DPMTABLE_UPDATE_SCLK)) {
-		result = tonga_populate_all_memory_levels(hwmgr);
+		result = tonga_populate_all_graphic_levels(hwmgr);
 		PP_ASSERT_WITH_CODE((0 == result),
 			"Failed to populate SCLK during PopulateNewDPMClocksStates Function!",
 			return result);
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/6] drm/amdgpu: add powerplay sclk OD support through sysfs

[Alex Deucher]

From: Eric Huang <JinHuiEric.Huang@amd.com>

Add a new sysfs entry pp_sclk_od to support sclk overdrive(OD) overclocking,
the entry is read/write, the value of input/output is an integer which is the
over percentage of the highest sclk.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h               |  6 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c            | 49 +++++++++++++++++++++++
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c     | 44 ++++++++++++++++++++
 drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h |  2 +
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h         |  2 +
 5 files changed, 103 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 992f00b..367dbc4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -2335,6 +2335,12 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
 #define amdgpu_dpm_force_clock_level(adev, type, level) \
 		(adev)->powerplay.pp_funcs->force_clock_level((adev)->powerplay.pp_handle, type, level)
 
+#define amdgpu_dpm_get_sclk_od(adev) \
+	(adev)->powerplay.pp_funcs->get_sclk_od((adev)->powerplay.pp_handle)
+
+#define amdgpu_dpm_set_sclk_od(adev, value) \
+	(adev)->powerplay.pp_funcs->set_sclk_od((adev)->powerplay.pp_handle, value)
+
 #define amdgpu_dpm_dispatch_task(adev, event_id, input, output)		\
 	(adev)->powerplay.pp_funcs->dispatch_tasks((adev)->powerplay.pp_handle, (event_id), (input), (output))
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
index 589b36e..f7ecaf4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
@@ -471,6 +471,46 @@ fail:
 	return count;
 }
 
+static ssize_t amdgpu_get_pp_sclk_od(struct device *dev,
+		struct device_attribute *attr,
+		char *buf)
+{
+	struct drm_device *ddev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = ddev->dev_private;
+	uint32_t value = 0;
+
+	if (adev->pp_enabled)
+		value = amdgpu_dpm_get_sclk_od(adev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", value);;
+}
+
+static ssize_t amdgpu_set_pp_sclk_od(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf,
+		size_t count)
+{
+	struct drm_device *ddev = dev_get_drvdata(dev);
+	struct amdgpu_device *adev = ddev->dev_private;
+	int ret;
+	long int value;
+
+	ret = kstrtol(buf, 0, &value);
+
+	if (ret) {
+		count = -EINVAL;
+		goto fail;
+	}
+
+	if (adev->pp_enabled)
+		amdgpu_dpm_set_sclk_od(adev, (uint32_t)value);
+
+	amdgpu_dpm_dispatch_task(adev, AMD_PP_EVENT_READJUST_POWER_STATE, NULL, NULL);
+
+fail:
+	return count;
+}
+
 static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR, amdgpu_get_dpm_state, amdgpu_set_dpm_state);
 static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
 		   amdgpu_get_dpm_forced_performance_level,
@@ -492,6 +532,9 @@ static DEVICE_ATTR(pp_dpm_mclk, S_IRUGO | S_IWUSR,
 static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR,
 		amdgpu_get_pp_dpm_pcie,
 		amdgpu_set_pp_dpm_pcie);
+static DEVICE_ATTR(pp_sclk_od, S_IRUGO | S_IWUSR,
+		amdgpu_get_pp_sclk_od,
+		amdgpu_set_pp_sclk_od);
 
 static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
 				      struct device_attribute *attr,
@@ -1125,6 +1168,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
 			DRM_ERROR("failed to create device file pp_dpm_pcie\n");
 			return ret;
 		}
+		ret = device_create_file(adev->dev, &dev_attr_pp_sclk_od);
+		if (ret) {
+			DRM_ERROR("failed to create device file pp_sclk_od\n");
+			return ret;
+		}
 	}
 	ret = amdgpu_debugfs_pm_init(adev);
 	if (ret) {
@@ -1151,6 +1199,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
 		device_remove_file(adev->dev, &dev_attr_pp_dpm_sclk);
 		device_remove_file(adev->dev, &dev_attr_pp_dpm_mclk);
 		device_remove_file(adev->dev, &dev_attr_pp_dpm_pcie);
+		device_remove_file(adev->dev, &dev_attr_pp_sclk_od);
 	}
 }
 
diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
index 8e345bf..e0f2440 100644
--- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
@@ -530,6 +530,10 @@ int pp_dpm_dispatch_tasks(void *handle, enum amd_pp_event event_id, void *input,
 	case AMD_PP_EVENT_COMPLETE_INIT:
 		ret = pem_handle_event(pp_handle->eventmgr, event_id, &data);
 		break;
+	case AMD_PP_EVENT_READJUST_POWER_STATE:
+		pp_handle->hwmgr->current_ps = pp_handle->hwmgr->boot_ps;
+		ret = pem_handle_event(pp_handle->eventmgr, event_id, &data);
+		break;
 	default:
 		break;
 	}
@@ -800,6 +804,44 @@ static int pp_dpm_print_clock_levels(void *handle,
 	return hwmgr->hwmgr_func->print_clock_levels(hwmgr, type, buf);
 }
 
+static int pp_dpm_get_sclk_od(void *handle)
+{
+	struct pp_hwmgr *hwmgr;
+
+	if (!handle)
+		return -EINVAL;
+
+	hwmgr = ((struct pp_instance *)handle)->hwmgr;
+
+	PP_CHECK_HW(hwmgr);
+
+	if (hwmgr->hwmgr_func->get_sclk_od == NULL) {
+		printk(KERN_INFO "%s was not implemented.\n", __func__);
+		return 0;
+	}
+
+	return hwmgr->hwmgr_func->get_sclk_od(hwmgr);
+}
+
+static int pp_dpm_set_sclk_od(void *handle, uint32_t value)
+{
+	struct pp_hwmgr *hwmgr;
+
+	if (!handle)
+		return -EINVAL;
+
+	hwmgr = ((struct pp_instance *)handle)->hwmgr;
+
+	PP_CHECK_HW(hwmgr);
+
+	if (hwmgr->hwmgr_func->set_sclk_od == NULL) {
+		printk(KERN_INFO "%s was not implemented.\n", __func__);
+		return 0;
+	}
+
+	return hwmgr->hwmgr_func->set_sclk_od(hwmgr, value);
+}
+
 const struct amd_powerplay_funcs pp_dpm_funcs = {
 	.get_temperature = pp_dpm_get_temperature,
 	.load_firmware = pp_dpm_load_fw,
@@ -822,6 +864,8 @@ const struct amd_powerplay_funcs pp_dpm_funcs = {
 	.set_pp_table = pp_dpm_set_pp_table,
 	.force_clock_level = pp_dpm_force_clock_level,
 	.print_clock_levels = pp_dpm_print_clock_levels,
+	.get_sclk_od = pp_dpm_get_sclk_od,
+	.set_sclk_od = pp_dpm_set_sclk_od,
 };
 
 static int amd_pp_instance_init(struct amd_pp_init *pp_init,
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
index 50b367d..154d406 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
@@ -342,6 +342,8 @@ struct amd_powerplay_funcs {
 	int (*set_pp_table)(void *handle, const char *buf, size_t size);
 	int (*force_clock_level)(void *handle, enum pp_clock_type type, uint32_t mask);
 	int (*print_clock_levels)(void *handle, enum pp_clock_type type, char *buf);
+	int (*get_sclk_od)(void *handle);
+	int (*set_sclk_od)(void *handle, uint32_t value);
 };
 
 struct amd_powerplay {
diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
index 28f5714..37ebfa2 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
@@ -338,6 +338,8 @@ struct pp_hwmgr_func {
 	int (*force_clock_level)(struct pp_hwmgr *hwmgr, enum pp_clock_type type, uint32_t mask);
 	int (*print_clock_levels)(struct pp_hwmgr *hwmgr, enum pp_clock_type type, char *buf);
 	int (*enable_per_cu_power_gating)(struct pp_hwmgr *hwmgr, bool enable);
+	int (*get_sclk_od)(struct pp_hwmgr *hwmgr);
+	int (*set_sclk_od)(struct pp_hwmgr *hwmgr, uint32_t value);
 };
 
 struct pp_table_func {
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/6] drm/amd/powerplay: add sclk OD support on Fiji

[Alex Deucher]

From: Eric Huang <JinHuiEric.Huang@amd.com>

This implements sclk overdrive(OD) overclocking support for Fiji,
and the maximum overdrive percentage is 20.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 43 ++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
index 6f1bad4..bf7bf5f 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
@@ -5274,6 +5274,47 @@ bool fiji_check_smc_update_required_for_display_configuration(struct pp_hwmgr *h
 	return is_update_required;
 }
 
+static int fiji_get_sclk_od(struct pp_hwmgr *hwmgr)
+{
+	struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
+	struct fiji_single_dpm_table *sclk_table = &(data->dpm_table.sclk_table);
+	struct fiji_single_dpm_table *golden_sclk_table =
+			&(data->golden_dpm_table.sclk_table);
+	int value;
+
+	value = (sclk_table->dpm_levels[sclk_table->count - 1].value -
+			golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value) *
+			100 /
+			golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
+
+	return value;
+}
+
+static int fiji_set_sclk_od(struct pp_hwmgr *hwmgr, uint32_t value)
+{
+	struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
+	struct fiji_single_dpm_table *golden_sclk_table =
+			&(data->golden_dpm_table.sclk_table);
+	struct pp_power_state  *ps;
+	struct fiji_power_state  *fiji_ps;
+
+	if (value > 20)
+		value = 20;
+
+	ps = hwmgr->request_ps;
+
+	if (ps == NULL)
+		return -EINVAL;
+
+	fiji_ps = cast_phw_fiji_power_state(&ps->hardware);
+
+	fiji_ps->performance_levels[fiji_ps->performance_level_count - 1].engine_clock =
+			golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value *
+			value / 100 +
+			golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
+
+	return 0;
+}
 
 static const struct pp_hwmgr_func fiji_hwmgr_funcs = {
 	.backend_init = &fiji_hwmgr_backend_init,
@@ -5316,6 +5357,8 @@ static const struct pp_hwmgr_func fiji_hwmgr_funcs = {
 	.set_pp_table = fiji_set_pp_table,
 	.force_clock_level = fiji_force_clock_level,
 	.print_clock_levels = fiji_print_clock_levels,
+	.get_sclk_od = fiji_get_sclk_od,
+	.set_sclk_od = fiji_set_sclk_od,
 };
 
 int fiji_hwmgr_init(struct pp_hwmgr *hwmgr)
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/6] drm/amd/powerplay: add sclk OD support on Tonga

[Alex Deucher]

From: Eric Huang <JinHuiEric.Huang@amd.com>

This implements sclk overdrive(OD) overclocking support for Tonga,
and the maximum overdrive percentage is 20.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 44 +++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
index cb28335..cf1fca4 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
@@ -6194,6 +6194,48 @@ static int tonga_print_clock_levels(struct pp_hwmgr *hwmgr,
 	return size;
 }
 
+static int tonga_get_sclk_od(struct pp_hwmgr *hwmgr)
+{
+	struct tonga_hwmgr *data = (struct tonga_hwmgr *)(hwmgr->backend);
+	struct tonga_single_dpm_table *sclk_table = &(data->dpm_table.sclk_table);
+	struct tonga_single_dpm_table *golden_sclk_table =
+			&(data->golden_dpm_table.sclk_table);
+	int value;
+
+	value = (sclk_table->dpm_levels[sclk_table->count - 1].value -
+			golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value) *
+			100 /
+			golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
+
+	return value;
+}
+
+static int tonga_set_sclk_od(struct pp_hwmgr *hwmgr, uint32_t value)
+{
+	struct tonga_hwmgr *data = (struct tonga_hwmgr *)(hwmgr->backend);
+	struct tonga_single_dpm_table *golden_sclk_table =
+			&(data->golden_dpm_table.sclk_table);
+	struct pp_power_state  *ps;
+	struct tonga_power_state  *tonga_ps;
+
+	if (value > 20)
+		value = 20;
+
+	ps = hwmgr->request_ps;
+
+	if (ps == NULL)
+		return -EINVAL;
+
+	tonga_ps = cast_phw_tonga_power_state(&ps->hardware);
+
+	tonga_ps->performance_levels[tonga_ps->performance_level_count - 1].engine_clock =
+			golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value *
+			value / 100 +
+			golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
+
+	return 0;
+}
+
 static const struct pp_hwmgr_func tonga_hwmgr_funcs = {
 	.backend_init = &tonga_hwmgr_backend_init,
 	.backend_fini = &tonga_hwmgr_backend_fini,
@@ -6236,6 +6278,8 @@ static const struct pp_hwmgr_func tonga_hwmgr_funcs = {
 	.set_pp_table = tonga_set_pp_table,
 	.force_clock_level = tonga_force_clock_level,
 	.print_clock_levels = tonga_print_clock_levels,
+	.get_sclk_od = tonga_get_sclk_od,
+	.set_sclk_od = tonga_set_sclk_od,
 };
 
 int tonga_hwmgr_init(struct pp_hwmgr *hwmgr)
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/6] drm/amd/powerplay: add sclk OD support on Polaris10

[Alex Deucher]

From: Eric Huang <JinHuiEric.Huang@amd.com>

This implements sclk overdrive(OD) overclocking support for Polaris10,
and the maximum overdrive percentage is 20.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 .../gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c  | 44 ++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
index 4cef7a5..05cb9a0 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
@@ -4899,6 +4899,48 @@ static int polaris10_get_fan_control_mode(struct pp_hwmgr *hwmgr)
 				CG_FDO_CTRL2, FDO_PWM_MODE);
 }
 
+static int polaris10_get_sclk_od(struct pp_hwmgr *hwmgr)
+{
+	struct polaris10_hwmgr *data = (struct polaris10_hwmgr *)(hwmgr->backend);
+	struct polaris10_single_dpm_table *sclk_table = &(data->dpm_table.sclk_table);
+	struct polaris10_single_dpm_table *golden_sclk_table =
+			&(data->golden_dpm_table.sclk_table);
+	int value;
+
+	value = (sclk_table->dpm_levels[sclk_table->count - 1].value -
+			golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value) *
+			100 /
+			golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
+
+	return value;
+}
+
+static int polaris10_set_sclk_od(struct pp_hwmgr *hwmgr, uint32_t value)
+{
+	struct polaris10_hwmgr *data = (struct polaris10_hwmgr *)(hwmgr->backend);
+	struct polaris10_single_dpm_table *golden_sclk_table =
+			&(data->golden_dpm_table.sclk_table);
+	struct pp_power_state  *ps;
+	struct polaris10_power_state  *polaris10_ps;
+
+	if (value > 20)
+		value = 20;
+
+	ps = hwmgr->request_ps;
+
+	if (ps == NULL)
+		return -EINVAL;
+
+	polaris10_ps = cast_phw_polaris10_power_state(&ps->hardware);
+
+	polaris10_ps->performance_levels[polaris10_ps->performance_level_count - 1].engine_clock =
+			golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value *
+			value / 100 +
+			golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
+
+	return 0;
+}
+
 static const struct pp_hwmgr_func polaris10_hwmgr_funcs = {
 	.backend_init = &polaris10_hwmgr_backend_init,
 	.backend_fini = &polaris10_hwmgr_backend_fini,
@@ -4942,6 +4984,8 @@ static const struct pp_hwmgr_func polaris10_hwmgr_funcs = {
 	.force_clock_level = polaris10_force_clock_level,
 	.print_clock_levels = polaris10_print_clock_levels,
 	.enable_per_cu_power_gating = polaris10_phm_enable_per_cu_power_gating,
+	.get_sclk_od = polaris10_get_sclk_od,
+	.set_sclk_od = polaris10_set_sclk_od,
 };
 
 int polaris10_hwmgr_init(struct pp_hwmgr *hwmgr)
-- 
2.5.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/6] Initial sclk OD support for amdgpu

[Mike Lothian]

[-- Attachment #1.1: Type: text/plain, Size: 1525 bytes --]

Sounds fancy but what does it do?

On Fri, 13 May 2016 at 19:49 Alex Deucher <alexdeucher@gmail.com> wrote:

> This adds initial OverDrive (OD) support for the gfx engine
> clock (sclk).  It's enabled by selecting a percentage (0-20)
> and writing it to a new sysfs file.  It's currently available
> on Tonga, Fiji, and Polaris.
>
> Eric Huang (6):
>   drm/amd/powerplay: fix a bug on updating sclk for Fiji
>   drm/amd/powerplay: fix a bug on updating sclk for Tonga
>   drm/amdgpu: add powerplay sclk OD support through sysfs
>   drm/amd/powerplay: add sclk OD support on Fiji
>   drm/amd/powerplay: add sclk OD support on Tonga
>   drm/amd/powerplay: add sclk OD support on Polaris10
>
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h                |  6 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c             | 49
> ++++++++++++++++++++++
>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c      | 44
> +++++++++++++++++++
>  drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c   | 45
> +++++++++++++++++++-
>  .../gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c  | 44
> +++++++++++++++++++
>  drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c  | 46
> +++++++++++++++++++-
>  drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h  |  2 +
>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h          |  2 +
>  8 files changed, 236 insertions(+), 2 deletions(-)
>
> --
> 2.5.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[-- Attachment #1.2: Type: text/html, Size: 2079 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/6] Initial sclk OD support for amdgpu

[Alex Deucher]

On Fri, May 13, 2016 at 2:54 PM, Mike Lothian <mike@fireburn.co.uk> wrote:
> Sounds fancy but what does it do?

Whoops, meant to define OD in the cover letter, the patches have the
details.  OD = Overclocking.

Alex

>
> On Fri, 13 May 2016 at 19:49 Alex Deucher <alexdeucher@gmail.com> wrote:
>>
>> This adds initial OverDrive (OD) support for the gfx engine
>> clock (sclk).  It's enabled by selecting a percentage (0-20)
>> and writing it to a new sysfs file.  It's currently available
>> on Tonga, Fiji, and Polaris.
>>
>> Eric Huang (6):
>>   drm/amd/powerplay: fix a bug on updating sclk for Fiji
>>   drm/amd/powerplay: fix a bug on updating sclk for Tonga
>>   drm/amdgpu: add powerplay sclk OD support through sysfs
>>   drm/amd/powerplay: add sclk OD support on Fiji
>>   drm/amd/powerplay: add sclk OD support on Tonga
>>   drm/amd/powerplay: add sclk OD support on Polaris10
>>
>>  drivers/gpu/drm/amd/amdgpu/amdgpu.h                |  6 +++
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c             | 49
>> ++++++++++++++++++++++
>>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c      | 44
>> +++++++++++++++++++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c   | 45
>> +++++++++++++++++++-
>>  .../gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c  | 44
>> +++++++++++++++++++
>>  drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c  | 46
>> +++++++++++++++++++-
>>  drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h  |  2 +
>>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h          |  2 +
>>  8 files changed, 236 insertions(+), 2 deletions(-)
>>
>> --
>> 2.5.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/6] Initial sclk OD support for amdgpu

[Mike Lothian]

[-- Attachment #1.1: Type: text/plain, Size: 2234 bytes --]

Hi

I gave this a spin but I just get:

[ 1073.096585] Trying to freeze SCLK DPM when DPM is disabled
[ 1073.097667] Trying to Unfreeze SCLK DPM when DPM is disabled
[ 1073.100118] Trying to freeze SCLK DPM when DPM is disabled
[ 1073.101618] Trying to Unfreeze SCLK DPM when DPM is disabled

DPM is enabled

Cheers

Mike

On Fri, 13 May 2016 at 19:56 Alex Deucher <alexdeucher@gmail.com> wrote:

> On Fri, May 13, 2016 at 2:54 PM, Mike Lothian <mike@fireburn.co.uk> wrote:
> > Sounds fancy but what does it do?
>
> Whoops, meant to define OD in the cover letter, the patches have the
> details.  OD = Overclocking.
>
> Alex
>
> >
> > On Fri, 13 May 2016 at 19:49 Alex Deucher <alexdeucher@gmail.com> wrote:
> >>
> >> This adds initial OverDrive (OD) support for the gfx engine
> >> clock (sclk).  It's enabled by selecting a percentage (0-20)
> >> and writing it to a new sysfs file.  It's currently available
> >> on Tonga, Fiji, and Polaris.
> >>
> >> Eric Huang (6):
> >>   drm/amd/powerplay: fix a bug on updating sclk for Fiji
> >>   drm/amd/powerplay: fix a bug on updating sclk for Tonga
> >>   drm/amdgpu: add powerplay sclk OD support through sysfs
> >>   drm/amd/powerplay: add sclk OD support on Fiji
> >>   drm/amd/powerplay: add sclk OD support on Tonga
> >>   drm/amd/powerplay: add sclk OD support on Polaris10
> >>
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu.h                |  6 +++
> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c             | 49
> >> ++++++++++++++++++++++
> >>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c      | 44
> >> +++++++++++++++++++
> >>  drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c   | 45
> >> +++++++++++++++++++-
> >>  .../gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c  | 44
> >> +++++++++++++++++++
> >>  drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c  | 46
> >> +++++++++++++++++++-
> >>  drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h  |  2 +
> >>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h          |  2 +
> >>  8 files changed, 236 insertions(+), 2 deletions(-)
> >>
> >> --
> >> 2.5.5
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[-- Attachment #1.2: Type: text/html, Size: 3889 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/6] Initial sclk OD support for amdgpu

[Alex Deucher]

On Fri, May 13, 2016 at 3:45 PM, Mike Lothian <mike@fireburn.co.uk> wrote:
> Hi
>
> I gave this a spin but I just get:
>
> [ 1073.096585] Trying to freeze SCLK DPM when DPM is disabled
> [ 1073.097667] Trying to Unfreeze SCLK DPM when DPM is disabled
> [ 1073.100118] Trying to freeze SCLK DPM when DPM is disabled
> [ 1073.101618] Trying to Unfreeze SCLK DPM when DPM is disabled
>
> DPM is enabled

Fixed in patch 3.5.

Alex

>
> Cheers
>
> Mike
>
> On Fri, 13 May 2016 at 19:56 Alex Deucher <alexdeucher@gmail.com> wrote:
>>
>> On Fri, May 13, 2016 at 2:54 PM, Mike Lothian <mike@fireburn.co.uk> wrote:
>> > Sounds fancy but what does it do?
>>
>> Whoops, meant to define OD in the cover letter, the patches have the
>> details.  OD = Overclocking.
>>
>> Alex
>>
>> >
>> > On Fri, 13 May 2016 at 19:49 Alex Deucher <alexdeucher@gmail.com> wrote:
>> >>
>> >> This adds initial OverDrive (OD) support for the gfx engine
>> >> clock (sclk).  It's enabled by selecting a percentage (0-20)
>> >> and writing it to a new sysfs file.  It's currently available
>> >> on Tonga, Fiji, and Polaris.
>> >>
>> >> Eric Huang (6):
>> >>   drm/amd/powerplay: fix a bug on updating sclk for Fiji
>> >>   drm/amd/powerplay: fix a bug on updating sclk for Tonga
>> >>   drm/amdgpu: add powerplay sclk OD support through sysfs
>> >>   drm/amd/powerplay: add sclk OD support on Fiji
>> >>   drm/amd/powerplay: add sclk OD support on Tonga
>> >>   drm/amd/powerplay: add sclk OD support on Polaris10
>> >>
>> >>  drivers/gpu/drm/amd/amdgpu/amdgpu.h                |  6 +++
>> >>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c             | 49
>> >> ++++++++++++++++++++++
>> >>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c      | 44
>> >> +++++++++++++++++++
>> >>  drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c   | 45
>> >> +++++++++++++++++++-
>> >>  .../gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c  | 44
>> >> +++++++++++++++++++
>> >>  drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c  | 46
>> >> +++++++++++++++++++-
>> >>  drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h  |  2 +
>> >>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h          |  2 +
>> >>  8 files changed, 236 insertions(+), 2 deletions(-)
>> >>
>> >> --
>> >> 2.5.5
>> >>
>> >> _______________________________________________
>> >> dri-devel mailing list
>> >> dri-devel@lists.freedesktop.org
>> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/6] drm/amdgpu: add powerplay sclk OD support through sysfs

[Nils Wallménius]

[-- Attachment #1.1: Type: text/plain, Size: 9095 bytes --]

Hi Eric,

A little nitpick below.

Regards
Nils

On Fri, May 13, 2016 at 8:48 PM, Alex Deucher <alexdeucher@gmail.com> wrote:

> From: Eric Huang <JinHuiEric.Huang@amd.com>
>
> Add a new sysfs entry pp_sclk_od to support sclk overdrive(OD)
> overclocking,
> the entry is read/write, the value of input/output is an integer which is
> the
> over percentage of the highest sclk.
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h               |  6 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c            | 49
> +++++++++++++++++++++++
>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c     | 44
> ++++++++++++++++++++
>  drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h |  2 +
>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h         |  2 +
>  5 files changed, 103 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 992f00b..367dbc4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -2335,6 +2335,12 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
>  #define amdgpu_dpm_force_clock_level(adev, type, level) \
>
> (adev)->powerplay.pp_funcs->force_clock_level((adev)->powerplay.pp_handle,
> type, level)
>
> +#define amdgpu_dpm_get_sclk_od(adev) \
> +
>  (adev)->powerplay.pp_funcs->get_sclk_od((adev)->powerplay.pp_handle)
> +
> +#define amdgpu_dpm_set_sclk_od(adev, value) \
> +
>  (adev)->powerplay.pp_funcs->set_sclk_od((adev)->powerplay.pp_handle, value)
> +
>  #define amdgpu_dpm_dispatch_task(adev, event_id, input, output)
>       \
>
> (adev)->powerplay.pp_funcs->dispatch_tasks((adev)->powerplay.pp_handle,
> (event_id), (input), (output))
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 589b36e..f7ecaf4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -471,6 +471,46 @@ fail:
>         return count;
>  }
>
> +static ssize_t amdgpu_get_pp_sclk_od(struct device *dev,
> +               struct device_attribute *attr,
> +               char *buf)
> +{
> +       struct drm_device *ddev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = ddev->dev_private;
> +       uint32_t value = 0;
> +
> +       if (adev->pp_enabled)
> +               value = amdgpu_dpm_get_sclk_od(adev);
> +
> +       return snprintf(buf, PAGE_SIZE, "%d\n", value);;
>

Double semicolon here ^


> +}
> +
> +static ssize_t amdgpu_set_pp_sclk_od(struct device *dev,
> +               struct device_attribute *attr,
> +               const char *buf,
> +               size_t count)
> +{
> +       struct drm_device *ddev = dev_get_drvdata(dev);
> +       struct amdgpu_device *adev = ddev->dev_private;
> +       int ret;
> +       long int value;
> +
> +       ret = kstrtol(buf, 0, &value);
> +
> +       if (ret) {
> +               count = -EINVAL;
> +               goto fail;
> +       }
> +
> +       if (adev->pp_enabled)
> +               amdgpu_dpm_set_sclk_od(adev, (uint32_t)value);
> +
> +       amdgpu_dpm_dispatch_task(adev, AMD_PP_EVENT_READJUST_POWER_STATE,
> NULL, NULL);
> +
> +fail:
> +       return count;
> +}
> +
>  static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR,
> amdgpu_get_dpm_state, amdgpu_set_dpm_state);
>  static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
>                    amdgpu_get_dpm_forced_performance_level,
> @@ -492,6 +532,9 @@ static DEVICE_ATTR(pp_dpm_mclk, S_IRUGO | S_IWUSR,
>  static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR,
>                 amdgpu_get_pp_dpm_pcie,
>                 amdgpu_set_pp_dpm_pcie);
> +static DEVICE_ATTR(pp_sclk_od, S_IRUGO | S_IWUSR,
> +               amdgpu_get_pp_sclk_od,
> +               amdgpu_set_pp_sclk_od);
>
>  static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
>                                       struct device_attribute *attr,
> @@ -1125,6 +1168,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
>                         DRM_ERROR("failed to create device file
> pp_dpm_pcie\n");
>                         return ret;
>                 }
> +               ret = device_create_file(adev->dev, &dev_attr_pp_sclk_od);
> +               if (ret) {
> +                       DRM_ERROR("failed to create device file
> pp_sclk_od\n");
> +                       return ret;
> +               }
>         }
>         ret = amdgpu_debugfs_pm_init(adev);
>         if (ret) {
> @@ -1151,6 +1199,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
>                 device_remove_file(adev->dev, &dev_attr_pp_dpm_sclk);
>                 device_remove_file(adev->dev, &dev_attr_pp_dpm_mclk);
>                 device_remove_file(adev->dev, &dev_attr_pp_dpm_pcie);
> +               device_remove_file(adev->dev, &dev_attr_pp_sclk_od);
>         }
>  }
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> index 8e345bf..e0f2440 100644
> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> @@ -530,6 +530,10 @@ int pp_dpm_dispatch_tasks(void *handle, enum
> amd_pp_event event_id, void *input,
>         case AMD_PP_EVENT_COMPLETE_INIT:
>                 ret = pem_handle_event(pp_handle->eventmgr, event_id,
> &data);
>                 break;
> +       case AMD_PP_EVENT_READJUST_POWER_STATE:
> +               pp_handle->hwmgr->current_ps = pp_handle->hwmgr->boot_ps;
> +               ret = pem_handle_event(pp_handle->eventmgr, event_id,
> &data);
> +               break;
>         default:
>                 break;
>         }
> @@ -800,6 +804,44 @@ static int pp_dpm_print_clock_levels(void *handle,
>         return hwmgr->hwmgr_func->print_clock_levels(hwmgr, type, buf);
>  }
>
> +static int pp_dpm_get_sclk_od(void *handle)
> +{
> +       struct pp_hwmgr *hwmgr;
> +
> +       if (!handle)
> +               return -EINVAL;
> +
> +       hwmgr = ((struct pp_instance *)handle)->hwmgr;
> +
> +       PP_CHECK_HW(hwmgr);
> +
> +       if (hwmgr->hwmgr_func->get_sclk_od == NULL) {
> +               printk(KERN_INFO "%s was not implemented.\n", __func__);
> +               return 0;
> +       }
> +
> +       return hwmgr->hwmgr_func->get_sclk_od(hwmgr);
> +}
> +
> +static int pp_dpm_set_sclk_od(void *handle, uint32_t value)
> +{
> +       struct pp_hwmgr *hwmgr;
> +
> +       if (!handle)
> +               return -EINVAL;
> +
> +       hwmgr = ((struct pp_instance *)handle)->hwmgr;
> +
> +       PP_CHECK_HW(hwmgr);
> +
> +       if (hwmgr->hwmgr_func->set_sclk_od == NULL) {
> +               printk(KERN_INFO "%s was not implemented.\n", __func__);
> +               return 0;
> +       }
> +
> +       return hwmgr->hwmgr_func->set_sclk_od(hwmgr, value);
> +}
> +
>  const struct amd_powerplay_funcs pp_dpm_funcs = {
>         .get_temperature = pp_dpm_get_temperature,
>         .load_firmware = pp_dpm_load_fw,
> @@ -822,6 +864,8 @@ const struct amd_powerplay_funcs pp_dpm_funcs = {
>         .set_pp_table = pp_dpm_set_pp_table,
>         .force_clock_level = pp_dpm_force_clock_level,
>         .print_clock_levels = pp_dpm_print_clock_levels,
> +       .get_sclk_od = pp_dpm_get_sclk_od,
> +       .set_sclk_od = pp_dpm_set_sclk_od,
>  };
>
>  static int amd_pp_instance_init(struct amd_pp_init *pp_init,
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> index 50b367d..154d406 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
> @@ -342,6 +342,8 @@ struct amd_powerplay_funcs {
>         int (*set_pp_table)(void *handle, const char *buf, size_t size);
>         int (*force_clock_level)(void *handle, enum pp_clock_type type,
> uint32_t mask);
>         int (*print_clock_levels)(void *handle, enum pp_clock_type type,
> char *buf);
> +       int (*get_sclk_od)(void *handle);
> +       int (*set_sclk_od)(void *handle, uint32_t value);
>  };
>
>  struct amd_powerplay {
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> index 28f5714..37ebfa2 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> @@ -338,6 +338,8 @@ struct pp_hwmgr_func {
>         int (*force_clock_level)(struct pp_hwmgr *hwmgr, enum
> pp_clock_type type, uint32_t mask);
>         int (*print_clock_levels)(struct pp_hwmgr *hwmgr, enum
> pp_clock_type type, char *buf);
>         int (*enable_per_cu_power_gating)(struct pp_hwmgr *hwmgr, bool
> enable);
> +       int (*get_sclk_od)(struct pp_hwmgr *hwmgr);
> +       int (*set_sclk_od)(struct pp_hwmgr *hwmgr, uint32_t value);
>  };
>
>  struct pp_table_func {
> --
> 2.5.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[-- Attachment #1.2: Type: text/html, Size: 11513 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/6] drm/amd/powerplay: add sclk OD support on Fiji

[Emil Velikov]

Hi all,

On 13 May 2016 at 19:48, Alex Deucher <alexdeucher@gmail.com> wrote:
> From: Eric Huang <JinHuiEric.Huang@amd.com>
>
> This implements sclk overdrive(OD) overclocking support for Fiji,
> and the maximum overdrive percentage is 20.
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 43 ++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
> index 6f1bad4..bf7bf5f 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
> @@ -5274,6 +5274,47 @@ bool fiji_check_smc_update_required_for_display_configuration(struct pp_hwmgr *h
>         return is_update_required;
>  }
>
> +static int fiji_get_sclk_od(struct pp_hwmgr *hwmgr)
> +{
> +       struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
> +       struct fiji_single_dpm_table *sclk_table = &(data->dpm_table.sclk_table);
> +       struct fiji_single_dpm_table *golden_sclk_table =
> +                       &(data->golden_dpm_table.sclk_table);
> +       int value;
> +
> +       value = (sclk_table->dpm_levels[sclk_table->count - 1].value -
> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value) *
> +                       100 /
> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
> +
> +       return value;
> +}
> +
> +static int fiji_set_sclk_od(struct pp_hwmgr *hwmgr, uint32_t value)
> +{
> +       struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
> +       struct fiji_single_dpm_table *golden_sclk_table =
> +                       &(data->golden_dpm_table.sclk_table);
> +       struct pp_power_state  *ps;
> +       struct fiji_power_state  *fiji_ps;
> +
> +       if (value > 20)
> +               value = 20;
> +
> +       ps = hwmgr->request_ps;
> +
> +       if (ps == NULL)
> +               return -EINVAL;
> +
> +       fiji_ps = cast_phw_fiji_power_state(&ps->hardware);
> +
> +       fiji_ps->performance_levels[fiji_ps->performance_level_count - 1].engine_clock =
> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value *
> +                       value / 100 +
> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
> +
> +       return 0;
> +}
>
Is it me or this patch 5/6 and 6/6 are identical ? Apart from the
structs of course which... seems to be identical as well despite that
they are copied across different headers and given different names.
Imho one should try to hold them alongside this (and other related)
code as opposed to duplicating even more code ?

Sadly, I won't be able to do any of it, plus I'm not a amdgpu
contributor so "my 2c" as they say.

Regards,
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/6] drm/amd/powerplay: add sclk OD support on Fiji

[Alex Deucher]

On Sat, May 14, 2016 at 10:03 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> Hi all,
>
> On 13 May 2016 at 19:48, Alex Deucher <alexdeucher@gmail.com> wrote:
>> From: Eric Huang <JinHuiEric.Huang@amd.com>
>>
>> This implements sclk overdrive(OD) overclocking support for Fiji,
>> and the maximum overdrive percentage is 20.
>>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>> Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>  drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 43 ++++++++++++++++++++++++
>>  1 file changed, 43 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
>> index 6f1bad4..bf7bf5f 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
>> @@ -5274,6 +5274,47 @@ bool fiji_check_smc_update_required_for_display_configuration(struct pp_hwmgr *h
>>         return is_update_required;
>>  }
>>
>> +static int fiji_get_sclk_od(struct pp_hwmgr *hwmgr)
>> +{
>> +       struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
>> +       struct fiji_single_dpm_table *sclk_table = &(data->dpm_table.sclk_table);
>> +       struct fiji_single_dpm_table *golden_sclk_table =
>> +                       &(data->golden_dpm_table.sclk_table);
>> +       int value;
>> +
>> +       value = (sclk_table->dpm_levels[sclk_table->count - 1].value -
>> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value) *
>> +                       100 /
>> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
>> +
>> +       return value;
>> +}
>> +
>> +static int fiji_set_sclk_od(struct pp_hwmgr *hwmgr, uint32_t value)
>> +{
>> +       struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
>> +       struct fiji_single_dpm_table *golden_sclk_table =
>> +                       &(data->golden_dpm_table.sclk_table);
>> +       struct pp_power_state  *ps;
>> +       struct fiji_power_state  *fiji_ps;
>> +
>> +       if (value > 20)
>> +               value = 20;
>> +
>> +       ps = hwmgr->request_ps;
>> +
>> +       if (ps == NULL)
>> +               return -EINVAL;
>> +
>> +       fiji_ps = cast_phw_fiji_power_state(&ps->hardware);
>> +
>> +       fiji_ps->performance_levels[fiji_ps->performance_level_count - 1].engine_clock =
>> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value *
>> +                       value / 100 +
>> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
>> +
>> +       return 0;
>> +}
>>
> Is it me or this patch 5/6 and 6/6 are identical ? Apart from the
> structs of course which... seems to be identical as well despite that
> they are copied across different headers and given different names.
> Imho one should try to hold them alongside this (and other related)
> code as opposed to duplicating even more code ?
>

If someone was motived.  The hw is similar, but not quite the same.
Generally new asics start with a copy of the old structs and then we
add/remove stuff to handle the difference in feature sets.
Unfortunately, power management is a bit less well defined compared to
other IP blocks since it has fingers into all of the other blocks.
With amdgpu, we've generally tried to keep IP blocks versions distinct
even if they are pretty similar and results in slightly more code to
avoid regressions on different asics due to differences in hw behavior
and changes in register offsets, etc.

Alex

> Sadly, I won't be able to do any of it, plus I'm not a amdgpu
> contributor so "my 2c" as they say.
>
> Regards,
> Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/6] drm/amdgpu: add powerplay sclk OD support through sysfs

[Eric Huang]

Thanks for your catch, Nils.

On 16-05-14 02:27 AM, Nils Wallménius wrote:
> Hi Eric,
>
> A little nitpick below.
>
> Regards
> Nils
>
> On Fri, May 13, 2016 at 8:48 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
>
>> From: Eric Huang <JinHuiEric.Huang@amd.com>
>>
>> Add a new sysfs entry pp_sclk_od to support sclk overdrive(OD)
>> overclocking,
>> the entry is read/write, the value of input/output is an integer which is
>> the
>> over percentage of the highest sclk.
>>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>> Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h               |  6 +++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c            | 49
>> +++++++++++++++++++++++
>>   drivers/gpu/drm/amd/powerplay/amd_powerplay.c     | 44
>> ++++++++++++++++++++
>>   drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h |  2 +
>>   drivers/gpu/drm/amd/powerplay/inc/hwmgr.h         |  2 +
>>   5 files changed, 103 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 992f00b..367dbc4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -2335,6 +2335,12 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
>>   #define amdgpu_dpm_force_clock_level(adev, type, level) \
>>
>> (adev)->powerplay.pp_funcs->force_clock_level((adev)->powerplay.pp_handle,
>> type, level)
>>
>> +#define amdgpu_dpm_get_sclk_od(adev) \
>> +
>>   (adev)->powerplay.pp_funcs->get_sclk_od((adev)->powerplay.pp_handle)
>> +
>> +#define amdgpu_dpm_set_sclk_od(adev, value) \
>> +
>>   (adev)->powerplay.pp_funcs->set_sclk_od((adev)->powerplay.pp_handle, value)
>> +
>>   #define amdgpu_dpm_dispatch_task(adev, event_id, input, output)
>>        \
>>
>> (adev)->powerplay.pp_funcs->dispatch_tasks((adev)->powerplay.pp_handle,
>> (event_id), (input), (output))
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> index 589b36e..f7ecaf4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
>> @@ -471,6 +471,46 @@ fail:
>>          return count;
>>   }
>>
>> +static ssize_t amdgpu_get_pp_sclk_od(struct device *dev,
>> +               struct device_attribute *attr,
>> +               char *buf)
>> +{
>> +       struct drm_device *ddev = dev_get_drvdata(dev);
>> +       struct amdgpu_device *adev = ddev->dev_private;
>> +       uint32_t value = 0;
>> +
>> +       if (adev->pp_enabled)
>> +               value = amdgpu_dpm_get_sclk_od(adev);
>> +
>> +       return snprintf(buf, PAGE_SIZE, "%d\n", value);;
>>
> Double semicolon here ^
>
>
>> +}
>> +
>> +static ssize_t amdgpu_set_pp_sclk_od(struct device *dev,
>> +               struct device_attribute *attr,
>> +               const char *buf,
>> +               size_t count)
>> +{
>> +       struct drm_device *ddev = dev_get_drvdata(dev);
>> +       struct amdgpu_device *adev = ddev->dev_private;
>> +       int ret;
>> +       long int value;
>> +
>> +       ret = kstrtol(buf, 0, &value);
>> +
>> +       if (ret) {
>> +               count = -EINVAL;
>> +               goto fail;
>> +       }
>> +
>> +       if (adev->pp_enabled)
>> +               amdgpu_dpm_set_sclk_od(adev, (uint32_t)value);
>> +
>> +       amdgpu_dpm_dispatch_task(adev, AMD_PP_EVENT_READJUST_POWER_STATE,
>> NULL, NULL);
>> +
>> +fail:
>> +       return count;
>> +}
>> +
>>   static DEVICE_ATTR(power_dpm_state, S_IRUGO | S_IWUSR,
>> amdgpu_get_dpm_state, amdgpu_set_dpm_state);
>>   static DEVICE_ATTR(power_dpm_force_performance_level, S_IRUGO | S_IWUSR,
>>                     amdgpu_get_dpm_forced_performance_level,
>> @@ -492,6 +532,9 @@ static DEVICE_ATTR(pp_dpm_mclk, S_IRUGO | S_IWUSR,
>>   static DEVICE_ATTR(pp_dpm_pcie, S_IRUGO | S_IWUSR,
>>                  amdgpu_get_pp_dpm_pcie,
>>                  amdgpu_set_pp_dpm_pcie);
>> +static DEVICE_ATTR(pp_sclk_od, S_IRUGO | S_IWUSR,
>> +               amdgpu_get_pp_sclk_od,
>> +               amdgpu_set_pp_sclk_od);
>>
>>   static ssize_t amdgpu_hwmon_show_temp(struct device *dev,
>>                                        struct device_attribute *attr,
>> @@ -1125,6 +1168,11 @@ int amdgpu_pm_sysfs_init(struct amdgpu_device *adev)
>>                          DRM_ERROR("failed to create device file
>> pp_dpm_pcie\n");
>>                          return ret;
>>                  }
>> +               ret = device_create_file(adev->dev, &dev_attr_pp_sclk_od);
>> +               if (ret) {
>> +                       DRM_ERROR("failed to create device file
>> pp_sclk_od\n");
>> +                       return ret;
>> +               }
>>          }
>>          ret = amdgpu_debugfs_pm_init(adev);
>>          if (ret) {
>> @@ -1151,6 +1199,7 @@ void amdgpu_pm_sysfs_fini(struct amdgpu_device *adev)
>>                  device_remove_file(adev->dev, &dev_attr_pp_dpm_sclk);
>>                  device_remove_file(adev->dev, &dev_attr_pp_dpm_mclk);
>>                  device_remove_file(adev->dev, &dev_attr_pp_dpm_pcie);
>> +               device_remove_file(adev->dev, &dev_attr_pp_sclk_od);
>>          }
>>   }
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
>> b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
>> index 8e345bf..e0f2440 100644
>> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
>> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
>> @@ -530,6 +530,10 @@ int pp_dpm_dispatch_tasks(void *handle, enum
>> amd_pp_event event_id, void *input,
>>          case AMD_PP_EVENT_COMPLETE_INIT:
>>                  ret = pem_handle_event(pp_handle->eventmgr, event_id,
>> &data);
>>                  break;
>> +       case AMD_PP_EVENT_READJUST_POWER_STATE:
>> +               pp_handle->hwmgr->current_ps = pp_handle->hwmgr->boot_ps;
>> +               ret = pem_handle_event(pp_handle->eventmgr, event_id,
>> &data);
>> +               break;
>>          default:
>>                  break;
>>          }
>> @@ -800,6 +804,44 @@ static int pp_dpm_print_clock_levels(void *handle,
>>          return hwmgr->hwmgr_func->print_clock_levels(hwmgr, type, buf);
>>   }
>>
>> +static int pp_dpm_get_sclk_od(void *handle)
>> +{
>> +       struct pp_hwmgr *hwmgr;
>> +
>> +       if (!handle)
>> +               return -EINVAL;
>> +
>> +       hwmgr = ((struct pp_instance *)handle)->hwmgr;
>> +
>> +       PP_CHECK_HW(hwmgr);
>> +
>> +       if (hwmgr->hwmgr_func->get_sclk_od == NULL) {
>> +               printk(KERN_INFO "%s was not implemented.\n", __func__);
>> +               return 0;
>> +       }
>> +
>> +       return hwmgr->hwmgr_func->get_sclk_od(hwmgr);
>> +}
>> +
>> +static int pp_dpm_set_sclk_od(void *handle, uint32_t value)
>> +{
>> +       struct pp_hwmgr *hwmgr;
>> +
>> +       if (!handle)
>> +               return -EINVAL;
>> +
>> +       hwmgr = ((struct pp_instance *)handle)->hwmgr;
>> +
>> +       PP_CHECK_HW(hwmgr);
>> +
>> +       if (hwmgr->hwmgr_func->set_sclk_od == NULL) {
>> +               printk(KERN_INFO "%s was not implemented.\n", __func__);
>> +               return 0;
>> +       }
>> +
>> +       return hwmgr->hwmgr_func->set_sclk_od(hwmgr, value);
>> +}
>> +
>>   const struct amd_powerplay_funcs pp_dpm_funcs = {
>>          .get_temperature = pp_dpm_get_temperature,
>>          .load_firmware = pp_dpm_load_fw,
>> @@ -822,6 +864,8 @@ const struct amd_powerplay_funcs pp_dpm_funcs = {
>>          .set_pp_table = pp_dpm_set_pp_table,
>>          .force_clock_level = pp_dpm_force_clock_level,
>>          .print_clock_levels = pp_dpm_print_clock_levels,
>> +       .get_sclk_od = pp_dpm_get_sclk_od,
>> +       .set_sclk_od = pp_dpm_set_sclk_od,
>>   };
>>
>>   static int amd_pp_instance_init(struct amd_pp_init *pp_init,
>> diff --git a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
>> b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
>> index 50b367d..154d406 100644
>> --- a/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
>> +++ b/drivers/gpu/drm/amd/powerplay/inc/amd_powerplay.h
>> @@ -342,6 +342,8 @@ struct amd_powerplay_funcs {
>>          int (*set_pp_table)(void *handle, const char *buf, size_t size);
>>          int (*force_clock_level)(void *handle, enum pp_clock_type type,
>> uint32_t mask);
>>          int (*print_clock_levels)(void *handle, enum pp_clock_type type,
>> char *buf);
>> +       int (*get_sclk_od)(void *handle);
>> +       int (*set_sclk_od)(void *handle, uint32_t value);
>>   };
>>
>>   struct amd_powerplay {
>> diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
>> b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
>> index 28f5714..37ebfa2 100644
>> --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
>> +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
>> @@ -338,6 +338,8 @@ struct pp_hwmgr_func {
>>          int (*force_clock_level)(struct pp_hwmgr *hwmgr, enum
>> pp_clock_type type, uint32_t mask);
>>          int (*print_clock_levels)(struct pp_hwmgr *hwmgr, enum
>> pp_clock_type type, char *buf);
>>          int (*enable_per_cu_power_gating)(struct pp_hwmgr *hwmgr, bool
>> enable);
>> +       int (*get_sclk_od)(struct pp_hwmgr *hwmgr);
>> +       int (*set_sclk_od)(struct pp_hwmgr *hwmgr, uint32_t value);
>>   };
>>
>>   struct pp_table_func {
>> --
>> 2.5.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/6] drm/amd/powerplay: add sclk OD support on Fiji

[Emil Velikov]

On 16 May 2016 at 15:42, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Sat, May 14, 2016 at 10:03 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> Hi all,
>>
>> On 13 May 2016 at 19:48, Alex Deucher <alexdeucher@gmail.com> wrote:
>>> From: Eric Huang <JinHuiEric.Huang@amd.com>
>>>
>>> This implements sclk overdrive(OD) overclocking support for Fiji,
>>> and the maximum overdrive percentage is 20.
>>>
>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>> Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 43 ++++++++++++++++++++++++
>>>  1 file changed, 43 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
>>> index 6f1bad4..bf7bf5f 100644
>>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
>>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
>>> @@ -5274,6 +5274,47 @@ bool fiji_check_smc_update_required_for_display_configuration(struct pp_hwmgr *h
>>>         return is_update_required;
>>>  }
>>>
>>> +static int fiji_get_sclk_od(struct pp_hwmgr *hwmgr)
>>> +{
>>> +       struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
>>> +       struct fiji_single_dpm_table *sclk_table = &(data->dpm_table.sclk_table);
>>> +       struct fiji_single_dpm_table *golden_sclk_table =
>>> +                       &(data->golden_dpm_table.sclk_table);
>>> +       int value;
>>> +
>>> +       value = (sclk_table->dpm_levels[sclk_table->count - 1].value -
>>> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value) *
>>> +                       100 /
>>> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
>>> +
>>> +       return value;
>>> +}
>>> +
>>> +static int fiji_set_sclk_od(struct pp_hwmgr *hwmgr, uint32_t value)
>>> +{
>>> +       struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
>>> +       struct fiji_single_dpm_table *golden_sclk_table =
>>> +                       &(data->golden_dpm_table.sclk_table);
>>> +       struct pp_power_state  *ps;
>>> +       struct fiji_power_state  *fiji_ps;
>>> +
>>> +       if (value > 20)
>>> +               value = 20;
>>> +
>>> +       ps = hwmgr->request_ps;
>>> +
>>> +       if (ps == NULL)
>>> +               return -EINVAL;
>>> +
>>> +       fiji_ps = cast_phw_fiji_power_state(&ps->hardware);
>>> +
>>> +       fiji_ps->performance_levels[fiji_ps->performance_level_count - 1].engine_clock =
>>> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value *
>>> +                       value / 100 +
>>> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
>>> +
>>> +       return 0;
>>> +}
>>>
>> Is it me or this patch 5/6 and 6/6 are identical ? Apart from the
>> structs of course which... seems to be identical as well despite that
>> they are copied across different headers and given different names.
>> Imho one should try to hold them alongside this (and other related)
>> code as opposed to duplicating even more code ?
>>
>
> If someone was motived.
I guess the people working/maintaining it should be motivated. Less
code, easier to maintain, debug, smaller binary and a bunch other
arguments that you know far better than me.

> The hw is similar, but not quite the same.
> Generally new asics start with a copy of the old structs and then we
> add/remove stuff to handle the difference in feature sets.
> Unfortunately, power management is a bit less well defined compared to
> other IP blocks since it has fingers into all of the other blocks.
> With amdgpu, we've generally tried to keep IP blocks versions distinct
> even if they are pretty similar and results in slightly more code to
> avoid regressions on different asics due to differences in hw behavior
> and changes in register offsets, etc.
>
So that's the root here - one copies the code and then makes changes if any.

If people are using the "let's copy it all just in case for the
future" approach this sounds like over-engineering (according to some
people). Or perhaps it serves as a nice way to boost the stats -
kernel is not XX lines of code :-P

Fwiw I would strongly encourage that you and/or members of the team
take a look at nouveau's structure [1]. It has handled duplication
across generations, abstraction layers, using the driver as an user
space application, etc., in a very elegant manner, imho. I believe
others have mentioned/given it as an example, as well ;-)


Thanks
Emil

[1] https://cgit.freedesktop.org/~darktama/nouveau/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/6] drm/amd/powerplay: add sclk OD support on Fiji

[Alex Deucher]

On Mon, May 16, 2016 at 6:04 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 16 May 2016 at 15:42, Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Sat, May 14, 2016 at 10:03 AM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>>> Hi all,
>>>
>>> On 13 May 2016 at 19:48, Alex Deucher <alexdeucher@gmail.com> wrote:
>>>> From: Eric Huang <JinHuiEric.Huang@amd.com>
>>>>
>>>> This implements sclk overdrive(OD) overclocking support for Fiji,
>>>> and the maximum overdrive percentage is 20.
>>>>
>>>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>>>> Signed-off-by: Eric Huang <JinHuiEric.Huang@amd.com>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>>> ---
>>>>  drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 43 ++++++++++++++++++++++++
>>>>  1 file changed, 43 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
>>>> index 6f1bad4..bf7bf5f 100644
>>>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
>>>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
>>>> @@ -5274,6 +5274,47 @@ bool fiji_check_smc_update_required_for_display_configuration(struct pp_hwmgr *h
>>>>         return is_update_required;
>>>>  }
>>>>
>>>> +static int fiji_get_sclk_od(struct pp_hwmgr *hwmgr)
>>>> +{
>>>> +       struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
>>>> +       struct fiji_single_dpm_table *sclk_table = &(data->dpm_table.sclk_table);
>>>> +       struct fiji_single_dpm_table *golden_sclk_table =
>>>> +                       &(data->golden_dpm_table.sclk_table);
>>>> +       int value;
>>>> +
>>>> +       value = (sclk_table->dpm_levels[sclk_table->count - 1].value -
>>>> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value) *
>>>> +                       100 /
>>>> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
>>>> +
>>>> +       return value;
>>>> +}
>>>> +
>>>> +static int fiji_set_sclk_od(struct pp_hwmgr *hwmgr, uint32_t value)
>>>> +{
>>>> +       struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
>>>> +       struct fiji_single_dpm_table *golden_sclk_table =
>>>> +                       &(data->golden_dpm_table.sclk_table);
>>>> +       struct pp_power_state  *ps;
>>>> +       struct fiji_power_state  *fiji_ps;
>>>> +
>>>> +       if (value > 20)
>>>> +               value = 20;
>>>> +
>>>> +       ps = hwmgr->request_ps;
>>>> +
>>>> +       if (ps == NULL)
>>>> +               return -EINVAL;
>>>> +
>>>> +       fiji_ps = cast_phw_fiji_power_state(&ps->hardware);
>>>> +
>>>> +       fiji_ps->performance_levels[fiji_ps->performance_level_count - 1].engine_clock =
>>>> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value *
>>>> +                       value / 100 +
>>>> +                       golden_sclk_table->dpm_levels[golden_sclk_table->count - 1].value;
>>>> +
>>>> +       return 0;
>>>> +}
>>>>
>>> Is it me or this patch 5/6 and 6/6 are identical ? Apart from the
>>> structs of course which... seems to be identical as well despite that
>>> they are copied across different headers and given different names.
>>> Imho one should try to hold them alongside this (and other related)
>>> code as opposed to duplicating even more code ?
>>>
>>
>> If someone was motived.
> I guess the people working/maintaining it should be motivated. Less
> code, easier to maintain, debug, smaller binary and a bunch other
> arguments that you know far better than me.

It cuts both ways.  Sharing a lot of code also hides bugs and makes
things harder to follow and debug.  You end up with lot of chips
specific if/else cases and you end up adding lot of function pointers
and forms of abstraction to split things up that make the code harder
to follow.  It's also really annoying when a handful of registers
change offsets so the code looks right but you aren't actually writing
to the registers you think you are.  It's a fine line.  We try and
share as much as makes sense, but still try and make the code easy to
follow and relatively compartmentalized.

>
>> The hw is similar, but not quite the same.
>> Generally new asics start with a copy of the old structs and then we
>> add/remove stuff to handle the difference in feature sets.
>> Unfortunately, power management is a bit less well defined compared to
>> other IP blocks since it has fingers into all of the other blocks.
>> With amdgpu, we've generally tried to keep IP blocks versions distinct
>> even if they are pretty similar and results in slightly more code to
>> avoid regressions on different asics due to differences in hw behavior
>> and changes in register offsets, etc.
>>
> So that's the root here - one copies the code and then makes changes if any.
>
> If people are using the "let's copy it all just in case for the
> future" approach this sounds like over-engineering (according to some
> people). Or perhaps it serves as a nice way to boost the stats -
> kernel is not XX lines of code :-P
>
> Fwiw I would strongly encourage that you and/or members of the team
> take a look at nouveau's structure [1]. It has handled duplication
> across generations, abstraction layers, using the driver as an user
> space application, etc., in a very elegant manner, imho. I believe
> others have mentioned/given it as an example, as well ;-)

I haven't looked too closely at the nouveau code, but I think our
structure is actually pretty similar.  Each asic is a collection of
versioned hw blocks.  We have a separate component for each hw block
(e.g., uvd5.x, uvd6.x, gfx7.x, gfx8.x, etc.) and each asic consists of
a table of hw blocks that correspond to the different block version
that make up the asic.  The hw block version specific code provides
callbacks to the asic independent core to provide the functionality
fullfilled by that hw block.

Alex


>
>
> Thanks
> Emil
>
> [1] https://cgit.freedesktop.org/~darktama/nouveau/
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel