[PATCH 1/2] drm/amd/display: fix array lenth error.

[PATCH 1/2] drm/amd/display: fix array lenth error.

[Rex Zhu]

Change-Id: I09011c5e6d5493db7e3d9a7ff7ab8c871a8db862
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
index 5af27aa..50576c6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
@@ -358,7 +358,7 @@ bool dm_pp_get_clock_levels_by_type(
 				 * non-boosted one. */
 				DRM_INFO("DM_PPLIB: reducing engine clock level from %d to %d\n",
 						dc_clks->num_levels, i + 1);
-				dc_clks->num_levels = i;
+				dc_clks->num_levels = i + 1;
 				break;
 			}
 		}
@@ -367,7 +367,7 @@ bool dm_pp_get_clock_levels_by_type(
 			if (dc_clks->clocks_in_khz[i] > validation_clks.memory_max_clock) {
 				DRM_INFO("DM_PPLIB: reducing memory clock level from %d to %d\n",
 						dc_clks->num_levels, i + 1);
-				dc_clks->num_levels = i;
+				dc_clks->num_levels = i + 1;
 				break;
 			}
 		}
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/2] drm/amd/powerplay: refine code to avoid potential bug that the memory not cleared.

[Rex Zhu]

Change-Id: If286d163cbabd8e9921a9d3cfcb71bb2b99aaceb
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 0a6c833..18f8ee7 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -4397,16 +4397,14 @@ static int smu7_get_sclks(struct pp_hwmgr *hwmgr, struct amd_pp_clocks *clocks)
 		if (table_info == NULL || table_info->vdd_dep_on_sclk == NULL)
 			return -EINVAL;
 		dep_sclk_table = table_info->vdd_dep_on_sclk;
-		for (i = 0; i < dep_sclk_table->count; i++) {
+		for (i = 0; i < dep_sclk_table->count; i++)
 			clocks->clock[i] = dep_sclk_table->entries[i].clk;
-			clocks->count++;
-		}
+		clocks->count = dep_sclk_table->count;
 	} else if (hwmgr->pp_table_version == PP_TABLE_V0) {
 		sclk_table = hwmgr->dyn_state.vddc_dependency_on_sclk;
-		for (i = 0; i < sclk_table->count; i++) {
+		for (i = 0; i < sclk_table->count; i++)
 			clocks->clock[i] = sclk_table->entries[i].clk;
-			clocks->count++;
-		}
+		clocks->count = sclk_table->count;
 	}
 
 	return 0;
@@ -4440,14 +4438,13 @@ static int smu7_get_mclks(struct pp_hwmgr *hwmgr, struct amd_pp_clocks *clocks)
 			clocks->clock[i] = dep_mclk_table->entries[i].clk;
 			clocks->latency[i] = smu7_get_mem_latency(hwmgr,
 						dep_mclk_table->entries[i].clk);
-			clocks->count++;
 		}
+		clocks->count = dep_mclk_table->count;
 	} else if (hwmgr->pp_table_version == PP_TABLE_V0) {
 		mclk_table = hwmgr->dyn_state.vddc_dependency_on_mclk;
-		for (i = 0; i < mclk_table->count; i++) {
+		for (i = 0; i < mclk_table->count; i++)
 			clocks->clock[i] = mclk_table->entries[i].clk;
-			clocks->count++;
-		}
+		clocks->count = mclk_table->count
 	}
 	return 0;
 }
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/2] drm/amd/powerplay: refine code to avoid potential bug that the memory not cleared.

[Zhu, Rex]

+		clocks->count = mclk_table->count

Missing semicolon. Will fix in V2.

Best Regards
Rex

-----Original Message-----
From: Rex Zhu [mailto:Rex.Zhu@amd.com] 
Sent: Monday, February 06, 2017 1:09 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhu, Rex
Subject: [PATCH 2/2] drm/amd/powerplay: refine code to avoid potential bug that the memory not cleared.

Change-Id: If286d163cbabd8e9921a9d3cfcb71bb2b99aaceb
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
index 0a6c833..18f8ee7 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
@@ -4397,16 +4397,14 @@ static int smu7_get_sclks(struct pp_hwmgr *hwmgr, struct amd_pp_clocks *clocks)
 		if (table_info == NULL || table_info->vdd_dep_on_sclk == NULL)
 			return -EINVAL;
 		dep_sclk_table = table_info->vdd_dep_on_sclk;
-		for (i = 0; i < dep_sclk_table->count; i++) {
+		for (i = 0; i < dep_sclk_table->count; i++)
 			clocks->clock[i] = dep_sclk_table->entries[i].clk;
-			clocks->count++;
-		}
+		clocks->count = dep_sclk_table->count;
 	} else if (hwmgr->pp_table_version == PP_TABLE_V0) {
 		sclk_table = hwmgr->dyn_state.vddc_dependency_on_sclk;
-		for (i = 0; i < sclk_table->count; i++) {
+		for (i = 0; i < sclk_table->count; i++)
 			clocks->clock[i] = sclk_table->entries[i].clk;
-			clocks->count++;
-		}
+		clocks->count = sclk_table->count;
 	}
 
 	return 0;
@@ -4440,14 +4438,13 @@ static int smu7_get_mclks(struct pp_hwmgr *hwmgr, struct amd_pp_clocks *clocks)
 			clocks->clock[i] = dep_mclk_table->entries[i].clk;
 			clocks->latency[i] = smu7_get_mem_latency(hwmgr,
 						dep_mclk_table->entries[i].clk);
-			clocks->count++;
 		}
+		clocks->count = dep_mclk_table->count;
 	} else if (hwmgr->pp_table_version == PP_TABLE_V0) {
 		mclk_table = hwmgr->dyn_state.vddc_dependency_on_mclk;
-		for (i = 0; i < mclk_table->count; i++) {
+		for (i = 0; i < mclk_table->count; i++)
 			clocks->clock[i] = mclk_table->entries[i].clk;
-			clocks->count++;
-		}
+		clocks->count = mclk_table->count
 	}
 	return 0;
 }
-- 
1.9.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amd/display: fix array lenth error.

[Harry Wentland]

On 2017-02-06 12:08 AM, Rex Zhu wrote:
> Change-Id: I09011c5e6d5493db7e3d9a7ff7ab8c871a8db862
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
> index 5af27aa..50576c6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
> @@ -358,7 +358,7 @@ bool dm_pp_get_clock_levels_by_type(
>  				 * non-boosted one. */
>  				DRM_INFO("DM_PPLIB: reducing engine clock level from %d to %d\n",
>  						dc_clks->num_levels, i + 1);
> -				dc_clks->num_levels = i;
> +				dc_clks->num_levels = i + 1;

It seems to me the DRM_INFO print is wrong here, not the actual
assignment. We're setting num_levels to the current index if the clocks
for that index are higher than the max validation clocks, hence this
index now should become num_levels.

>  				break;
>  			}
>  		}
> @@ -367,7 +367,7 @@ bool dm_pp_get_clock_levels_by_type(
>  			if (dc_clks->clocks_in_khz[i] > validation_clks.memory_max_clock) {
>  				DRM_INFO("DM_PPLIB: reducing memory clock level from %d to %d\n",
>  						dc_clks->num_levels, i + 1);
> -				dc_clks->num_levels = i;
> +				dc_clks->num_levels = i + 1;
>  				break;
>  			}
>  		}
> 

Same comment as above.

Harry
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amd/display: fix array lenth error.

[Zhu, Rex]

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

seems reasonable.


but i think need to check num_levels  can't be 0. in some case, there is only one level of mclk, and higher than the max validation clocks.. and will lead kernel panic.


Best Regards

Rex

________________________________
From: Wentland, Harry
Sent: Monday, February 6, 2017 10:54:24 PM
To: Zhu, Rex; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Subject: Re: [PATCH 1/2] drm/amd/display: fix array lenth error.

On 2017-02-06 12:08 AM, Rex Zhu wrote:
> Change-Id: I09011c5e6d5493db7e3d9a7ff7ab8c871a8db862
> Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
> index 5af27aa..50576c6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
> @@ -358,7 +358,7 @@ bool dm_pp_get_clock_levels_by_type(
>                                 * non-boosted one. */
>                                DRM_INFO("DM_PPLIB: reducing engine clock level from %d to %d\n",
>                                                dc_clks->num_levels, i + 1);
> -                             dc_clks->num_levels = i;
> +                             dc_clks->num_levels = i + 1;

It seems to me the DRM_INFO print is wrong here, not the actual
assignment. We're setting num_levels to the current index if the clocks
for that index are higher than the max validation clocks, hence this
index now should become num_levels.

>                                break;
>                        }
>                }
> @@ -367,7 +367,7 @@ bool dm_pp_get_clock_levels_by_type(
>                        if (dc_clks->clocks_in_khz[i] > validation_clks.memory_max_clock) {
>                                DRM_INFO("DM_PPLIB: reducing memory clock level from %d to %d\n",
>                                                dc_clks->num_levels, i + 1);
> -                             dc_clks->num_levels = i;
> +                             dc_clks->num_levels = i + 1;
>                                break;
>                        }
>                }
>

Same comment as above.

Harry

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

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

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amd/display: fix array lenth error.

[Harry Wentland]

True, we should probably always keep one level even if it's larger than
max_clock, but output a big fat warning message when that happens.

Harry

On 2017-02-06 10:36 AM, Zhu, Rex wrote:
> seems reasonable.
> 
> 
> but i think need to check num_levels  can't be 0. in some case, there is
> only one level of mclk, and higher than the max validation clocks.. and
> will lead kernel panic.
> 
> 
> Best Regards
> 
> Rex
> 
> ------------------------------------------------------------------------
> *From:* Wentland, Harry
> *Sent:* Monday, February 6, 2017 10:54:24 PM
> *To:* Zhu, Rex; amd-gfx@lists.freedesktop.org
> *Subject:* Re: [PATCH 1/2] drm/amd/display: fix array lenth error.
>  
> On 2017-02-06 12:08 AM, Rex Zhu wrote:
>> Change-Id: I09011c5e6d5493db7e3d9a7ff7ab8c871a8db862
>> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
>> ---
>>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
>> index 5af27aa..50576c6 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_services.c
>> @@ -358,7 +358,7 @@ bool dm_pp_get_clock_levels_by_type(
>>                                 * non-boosted one. */
>>                                DRM_INFO("DM_PPLIB: reducing engine clock level from %d to %d\n",
>>                                                dc_clks->num_levels, i + 1);
>> -                             dc_clks->num_levels = i;
>> +                             dc_clks->num_levels = i + 1;
> 
> It seems to me the DRM_INFO print is wrong here, not the actual
> assignment. We're setting num_levels to the current index if the clocks
> for that index are higher than the max validation clocks, hence this
> index now should become num_levels.
> 
>>                                break;
>>                        }
>>                }
>> @@ -367,7 +367,7 @@ bool dm_pp_get_clock_levels_by_type(
>>                        if (dc_clks->clocks_in_khz[i] > validation_clks.memory_max_clock) {
>>                                DRM_INFO("DM_PPLIB: reducing memory clock level from %d to %d\n",
>>                                                dc_clks->num_levels, i + 1);
>> -                             dc_clks->num_levels = i;
>> +                             dc_clks->num_levels = i + 1;
>>                                break;
>>                        }
>>                }
>> 
> 
> Same comment as above.
> 
> Harry
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx