Skip to contentSkip to conversation

drm/amdgpu: change pp_dpm clk/mclk/pcie input format

PATCH In review welu 2 participants 3 messages Apr 19, 2018 – Apr 24, 2018 2 messages not in the archive

Subsystems (2)
DRM drivers Radeon and amdgpu DRM drivers
lore
Share & tools
lore

welu

author
#0 lore raw

PATCH 1/1 drm/amdgpu: change pp_dpm clk/mclk/pcie input format

1. support more than 8 values when setting get_pp_dpm_mclk/ sclk/pcie, the former design just parse command format like "echo xxxx >　pp_dpm_sclk" and current can parse "echo xx xxx xxxx > pp_dpm_sclk" whose operation is more user-friendly
and convinent and can offer more values;
2. be compatible with former design like "xx". Bug:KFD-385

Change-Id Id7d95ce45f4ee0564b18ebcfc16976f1a5c6bf72
Signed-off-by welu <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 95 ++++++++++++++++++++--------------
 1 file changed, 55 insertions(+), 40 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +55 -40
index 744f105..58f46f9 100644
@@ -466,23 +466,27 @@ static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
 	struct amdgpu_device *adev = ddev->dev_private;
 	int ret;
 	long level;
-	uint32_t i, mask = 0;
-	char sub_str[2];
+	uint32_t mask = 0;
+	char *sub_str = NULL;
+	char *tmp;
+	char buf_cpy[count];
+	const char delimiter[3] = {' ', '\n', '\0'};
 
-	for (i = 0; i < strlen(buf); i++) {
-		if (*(buf + i) == '\n')
-			continue;
-		sub_str[0] = *(buf + i);
-		sub_str[1] = '\0';
-		ret = kstrtol(sub_str, 0, &level);
+	memcpy(buf_cpy, buf, count+1);
+	tmp = buf_cpy;
+	while (tmp[0]) {
+		sub_str =  strsep(&tmp, delimiter);
+		if (strlen(sub_str)) {
+			ret = kstrtol(sub_str, 0, &level);
 
-		if (ret) {
-			count = -EINVAL;
-			goto fail;
-		}
-		mask |= 1 << level;
+			if (ret) {
+				count = -EINVAL;
+				goto fail;
+			}
+			mask |= 1 << level;
+		} else
+			break;
 	}
-
 	if (adev->powerplay.pp_funcs->force_clock_level)
 		amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask);
 

@@ -512,21 +516,26 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev,
 	struct amdgpu_device *adev = ddev->dev_private;
 	int ret;
 	long level;
-	uint32_t i, mask = 0;
-	char sub_str[2];
+	uint32_t mask = 0;
+	char *sub_str = NULL;
+	char *tmp;
+	char buf_cpy[count];
+	const char delimiter[3] = {' ', '\n', '\0'};
 
-	for (i = 0; i < strlen(buf); i++) {
-		if (*(buf + i) == '\n')
-			continue;
-		sub_str[0] = *(buf + i);
-		sub_str[1] = '\0';
-		ret = kstrtol(sub_str, 0, &level);
+	memcpy(buf_cpy, buf, count+1);
+	tmp = buf_cpy;
+	while (tmp[0]) {
+		sub_str =  strsep(&tmp, delimiter);
+		if (strlen(sub_str)) {
+			ret = kstrtol(sub_str, 0, &level);
 
-		if (ret) {
-			count = -EINVAL;
-			goto fail;
-		}
-		mask |= 1 << level;
+			if (ret) {
+				count = -EINVAL;
+				goto fail;
+			}
+			mask |= 1 << level;
+		} else
+			break;
 	}
 	if (adev->powerplay.pp_funcs->force_clock_level)
 		amdgpu_dpm_force_clock_level(adev, PP_MCLK, mask);

@@ -557,21 +566,27 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device *dev,
 	struct amdgpu_device *adev = ddev->dev_private;
 	int ret;
 	long level;
-	uint32_t i, mask = 0;
-	char sub_str[2];
+	uint32_t mask = 0;
+	char *sub_str = NULL;
+	char *tmp;
+	char buf_cpy[count];
+	const char delimiter[3] = {' ', '\n', '\0'};
 
-	for (i = 0; i < strlen(buf); i++) {
-		if (*(buf + i) == '\n')
-			continue;
-		sub_str[0] = *(buf + i);
-		sub_str[1] = '\0';
-		ret = kstrtol(sub_str, 0, &level);
+	memcpy(buf_cpy, buf, count+1);
+	tmp = buf_cpy;
 
-		if (ret) {
-			count = -EINVAL;
-			goto fail;
-		}
-		mask |= 1 << level;
+	while (tmp[0]) {
+		sub_str =  strsep(&tmp, delimiter);
+		if (strlen(sub_str)) {
+			ret = kstrtol(sub_str, 0, &level);
+
+			if (ret) {
+				count = -EINVAL;
+				goto fail;
+			}
+			mask |= 1 << level;
+		} else
+			break;
 	}
 	if (adev->powerplay.pp_funcs->force_clock_level)
 		amdgpu_dpm_force_clock_level(adev, PP_PCIE, mask);
Signature · 6 lines
2.7.4

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 1524171079-657-1-git-send-email-wei.lu2-5C7GfCeVMHo@public.gmane.org

Reviewed-by
#2 lore raw
Quoted 7 lines · welu

1. support more than 8 values when setting get_pp_dpm_mclk/ sclk/pcie, the former design just parse command format like "echo xxxx >　pp_dpm_sclk" and current can parse "echo xx xxx xxxx > pp_dpm_sclk" whose operation is more user-friendly
and convinent and can offer more values;
2. be compatible with former design like "xx". Bug:KFD-385

Please update the documentation labeled "DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_pcie", With that fixed, patch is:

Reviewed-by Alex Deucher <[email protected]>
Quoted 137 lines
Change-Id Id7d95ce45f4ee0564b18ebcfc16976f1a5c6bf72
Signed-off-by welu <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 95 ++++++++++++++++++++--------------
 1 file changed, 55 insertions(+), 40 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +55 -41
index 744f105..58f46f9 100644
@@ -466,23 +466,27 @@ static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
        struct amdgpu_device *adev = ddev->dev_private;
        int ret;
        long level;
-       uint32_t i, mask = 0;
-       char sub_str[2];
+       uint32_t mask = 0;
+       char *sub_str = NULL;
+       char *tmp;
+       char buf_cpy[count];
+       const char delimiter[3] = {' ', '\n', '\0'};
 
-       for (i = 0; i < strlen(buf); i++) {
-               if (*(buf + i) == '\n')
-                       continue;
-               sub_str[0] = *(buf + i);
-               sub_str[1] = '\0';
-               ret = kstrtol(sub_str, 0, &level);
+       memcpy(buf_cpy, buf, count+1);
+       tmp = buf_cpy;
+       while (tmp[0]) {
+               sub_str =  strsep(&tmp, delimiter);
+               if (strlen(sub_str)) {
+                       ret = kstrtol(sub_str, 0, &level);
 
-               if (ret) {
-                       count = -EINVAL;
-                       goto fail;
-               }
-               mask |= 1 << level;
+                       if (ret) {
+                               count = -EINVAL;
+                               goto fail;
+                       }
+                       mask |= 1 << level;
+               } else
+                       break;
        }
-
        if (adev->powerplay.pp_funcs->force_clock_level)
                amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask);
 

@@ -512,21 +516,26 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev,
        struct amdgpu_device *adev = ddev->dev_private;
        int ret;
        long level;
-       uint32_t i, mask = 0;
-       char sub_str[2];
+       uint32_t mask = 0;
+       char *sub_str = NULL;
+       char *tmp;
+       char buf_cpy[count];
+       const char delimiter[3] = {' ', '\n', '\0'};
 
-       for (i = 0; i < strlen(buf); i++) {
-               if (*(buf + i) == '\n')
-                       continue;
-               sub_str[0] = *(buf + i);
-               sub_str[1] = '\0';
-               ret = kstrtol(sub_str, 0, &level);
+       memcpy(buf_cpy, buf, count+1);
+       tmp = buf_cpy;
+       while (tmp[0]) {
+               sub_str =  strsep(&tmp, delimiter);
+               if (strlen(sub_str)) {
+                       ret = kstrtol(sub_str, 0, &level);
 
-               if (ret) {
-                       count = -EINVAL;
-                       goto fail;
-               }
-               mask |= 1 << level;
+                       if (ret) {
+                               count = -EINVAL;
+                               goto fail;
+                       }
+                       mask |= 1 << level;
+               } else
+                       break;
        }
        if (adev->powerplay.pp_funcs->force_clock_level)
                amdgpu_dpm_force_clock_level(adev, PP_MCLK, mask);

@@ -557,21 +566,27 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device *dev,
        struct amdgpu_device *adev = ddev->dev_private;
        int ret;
        long level;
-       uint32_t i, mask = 0;
-       char sub_str[2];
+       uint32_t mask = 0;
+       char *sub_str = NULL;
+       char *tmp;
+       char buf_cpy[count];
+       const char delimiter[3] = {' ', '\n', '\0'};
 
-       for (i = 0; i < strlen(buf); i++) {
-               if (*(buf + i) == '\n')
-                       continue;
-               sub_str[0] = *(buf + i);
-               sub_str[1] = '\0';
-               ret = kstrtol(sub_str, 0, &level);
+       memcpy(buf_cpy, buf, count+1);
+       tmp = buf_cpy;
 
-               if (ret) {
-                       count = -EINVAL;
-                       goto fail;
-               }
-               mask |= 1 << level;
+       while (tmp[0]) {
+               sub_str =  strsep(&tmp, delimiter);
+               if (strlen(sub_str)) {
+                       ret = kstrtol(sub_str, 0, &level);
+
+                       if (ret) {
+                               count = -EINVAL;
+                               goto fail;
+                       }
+                       mask |= 1 << level;
+               } else
+                       break;
        }
        if (adev->powerplay.pp_funcs->force_clock_level)
                amdgpu_dpm_force_clock_level(adev, PP_PCIE, mask);
--

2.7.4

Quoted 4 lines

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): CADnq5_NyeoyGQTqq3okvFpjNrRN2CSU+trFSTENT9cor2=aj7w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

author Reviewed-by Alex Deucher
#4 lore raw

hi Alex,

I have sent the patch with "Add: DOC " and last patch to [email protected] already.

please check.

Thanks!

Wei

________________________________
From: Alex Deucher <[email protected]>
Sent: 23 April 2018 16:06:16
To: Lu, Wei
Cc: amd-gfx list
Subject: Re: [PATCH] drm/amdgpu: change pp_dpm clk/mclk/pcie input format

Quoted 7 lines · welu

1. support more than 8 values when setting get_pp_dpm_mclk/ sclk/pcie, the former design just parse command format like "echo xxxx >　pp_dpm_sclk" and current can parse "echo xx xxx xxxx > pp_dpm_sclk" whose operation is more user-friendly
and convinent and can offer more values;
2. be compatible with former design like "xx". Bug:KFD-385

Please update the documentation labeled "DOC: pp_dpm_sclk pp_dpm_mclk pp_dpm_pcie", With that fixed, patch is:

Reviewed-by Alex Deucher <[email protected]>
Quoted 137 lines
Change-Id Id7d95ce45f4ee0564b18ebcfc16976f1a5c6bf72
Signed-off-by welu <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 95 ++++++++++++++++++++--------------
 1 file changed, 55 insertions(+), 40 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c +55 -41
index 744f105..58f46f9 100644
@@ -466,23 +466,27 @@ static ssize_t amdgpu_set_pp_dpm_sclk(struct device *dev,
        struct amdgpu_device *adev = ddev->dev_private;
        int ret;
        long level;
-       uint32_t i, mask = 0;
-       char sub_str[2];
+       uint32_t mask = 0;
+       char *sub_str = NULL;
+       char *tmp;
+       char buf_cpy[count];
+       const char delimiter[3] = {' ', '\n', '\0'};
 
-       for (i = 0; i < strlen(buf); i++) {
-               if (*(buf + i) == '\n')
-                       continue;
-               sub_str[0] = *(buf + i);
-               sub_str[1] = '\0';
-               ret = kstrtol(sub_str, 0, &level);
+       memcpy(buf_cpy, buf, count+1);
+       tmp = buf_cpy;
+       while (tmp[0]) {
+               sub_str =  strsep(&tmp, delimiter);
+               if (strlen(sub_str)) {
+                       ret = kstrtol(sub_str, 0, &level);
 
-               if (ret) {
-                       count = -EINVAL;
-                       goto fail;
-               }
-               mask |= 1 << level;
+                       if (ret) {
+                               count = -EINVAL;
+                               goto fail;
+                       }
+                       mask |= 1 << level;
+               } else
+                       break;
        }
-
        if (adev->powerplay.pp_funcs->force_clock_level)
                amdgpu_dpm_force_clock_level(adev, PP_SCLK, mask);
 

@@ -512,21 +516,26 @@ static ssize_t amdgpu_set_pp_dpm_mclk(struct device *dev,
        struct amdgpu_device *adev = ddev->dev_private;
        int ret;
        long level;
-       uint32_t i, mask = 0;
-       char sub_str[2];
+       uint32_t mask = 0;
+       char *sub_str = NULL;
+       char *tmp;
+       char buf_cpy[count];
+       const char delimiter[3] = {' ', '\n', '\0'};
 
-       for (i = 0; i < strlen(buf); i++) {
-               if (*(buf + i) == '\n')
-                       continue;
-               sub_str[0] = *(buf + i);
-               sub_str[1] = '\0';
-               ret = kstrtol(sub_str, 0, &level);
+       memcpy(buf_cpy, buf, count+1);
+       tmp = buf_cpy;
+       while (tmp[0]) {
+               sub_str =  strsep(&tmp, delimiter);
+               if (strlen(sub_str)) {
+                       ret = kstrtol(sub_str, 0, &level);
 
-               if (ret) {
-                       count = -EINVAL;
-                       goto fail;
-               }
-               mask |= 1 << level;
+                       if (ret) {
+                               count = -EINVAL;
+                               goto fail;
+                       }
+                       mask |= 1 << level;
+               } else
+                       break;
        }
        if (adev->powerplay.pp_funcs->force_clock_level)
                amdgpu_dpm_force_clock_level(adev, PP_MCLK, mask);

@@ -557,21 +566,27 @@ static ssize_t amdgpu_set_pp_dpm_pcie(struct device *dev,
        struct amdgpu_device *adev = ddev->dev_private;
        int ret;
        long level;
-       uint32_t i, mask = 0;
-       char sub_str[2];
+       uint32_t mask = 0;
+       char *sub_str = NULL;
+       char *tmp;
+       char buf_cpy[count];
+       const char delimiter[3] = {' ', '\n', '\0'};
 
-       for (i = 0; i < strlen(buf); i++) {
-               if (*(buf + i) == '\n')
-                       continue;
-               sub_str[0] = *(buf + i);
-               sub_str[1] = '\0';
-               ret = kstrtol(sub_str, 0, &level);
+       memcpy(buf_cpy, buf, count+1);
+       tmp = buf_cpy;
 
-               if (ret) {
-                       count = -EINVAL;
-                       goto fail;
-               }
-               mask |= 1 << level;
+       while (tmp[0]) {
+               sub_str =  strsep(&tmp, delimiter);
+               if (strlen(sub_str)) {
+                       ret = kstrtol(sub_str, 0, &level);
+
+                       if (ret) {
+                               count = -EINVAL;
+                               goto fail;
+                       }
+                       mask |= 1 << level;
+               } else
+                       break;
        }
        if (adev->powerplay.pp_funcs->force_clock_level)
                amdgpu_dpm_force_clock_level(adev, PP_PCIE, mask);
--

2.7.4

Quoted 4 lines

_______________________________________________
amd-gfx mailing list [email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx