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 at 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 at 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
