On 2018-05-27 09:46 PM, Quan, Evan wrote: >> + /* translate 10kHz to kHz */ >> + clk_level_info->data[i].clocks_in_khz = >> pp_clks->data[i].clocks_in_khz; >> + clk_level_info->data[i].latency_in_us = >> pp_clks->data[i].clocks_in_khz; > > This seems a typo(latency_in_us = clocks_in_khz ?). >
Definitely a typo. Thanks for spotting this. > And the comment does not meet the code. If it translates 10kHz to kHz, why > there is no "*10"? >> + } > I'll send a patch with fixes for both. No need for the comment here, just left-over from copy-paste. Harry > > Regards, > > Evan > > ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ > *From:* amd-gfx <[email protected]> on behalf of Alex > Deucher <[email protected]> > *Sent:* Thursday, May 24, 2018 12:35:52 PM > *To:* Wentland, Harry > *Cc:* Cheng, Tony; Lipski, Mikita; Zhu, Rex; Wu, Hersen; amd-gfx list > *Subject:* Re: [PATCH] drm/amd/display: Implement > dm_pp_get_clock_levels_by_type_with_latency > > On Wed, May 23, 2018 at 1:51 PM, Harry Wentland <[email protected]> > wrote: >> This is required so we use the correct minimum clocks for Vega. Without >> this pplib will never be able to enter the lowest clock states. >> >> Signed-off-by: Harry Wentland <[email protected]> >> --- >> .../display/amdgpu_dm/amdgpu_dm_services.c | 46 ++++++++++++++++++- >> 1 file changed, 44 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 0229c7edb8ad..30ddd329104d 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 >> @@ -234,6 +234,34 @@ static void pp_to_dc_clock_levels( >> } >> } >> >> +static void pp_to_dc_clock_levels_with_latency( >> + const struct pp_clock_levels_with_latency *pp_clks, >> + struct dm_pp_clock_levels_with_latency *clk_level_info, >> + enum dm_pp_clock_type dc_clk_type) >> +{ >> + uint32_t i; >> + >> + if (pp_clks->num_levels > DM_PP_MAX_CLOCK_LEVELS) { >> + DRM_INFO("DM_PPLIB: Warning: %s clock: number of levels %d >> exceeds maximum of %d!\n", >> + DC_DECODE_PP_CLOCK_TYPE(dc_clk_type), >> + pp_clks->num_levels, >> + DM_PP_MAX_CLOCK_LEVELS); >> + >> + clk_level_info->num_levels = DM_PP_MAX_CLOCK_LEVELS; >> + } else >> + clk_level_info->num_levels = pp_clks->num_levels; >> + >> + DRM_INFO("DM_PPLIB: values for %s clock\n", >> + DC_DECODE_PP_CLOCK_TYPE(dc_clk_type)); >> + >> + for (i = 0; i < clk_level_info->num_levels; i++) { >> + DRM_INFO("DM_PPLIB:\t %d\n", pp_clks->data[i].clocks_in_khz); > > leftover debugging output? Maybe make that DRM_DEBUG? With that addressed: > Acked-by: Alex Deucher <[email protected]> > > Alex > >> + /* translate 10kHz to kHz */ >> + clk_level_info->data[i].clocks_in_khz = >> pp_clks->data[i].clocks_in_khz; >> + clk_level_info->data[i].latency_in_us = >> pp_clks->data[i].clocks_in_khz; >> + } >> +} >> + >> bool dm_pp_get_clock_levels_by_type( >> const struct dc_context *ctx, >> enum dm_pp_clock_type clk_type, >> @@ -311,8 +339,22 @@ bool dm_pp_get_clock_levels_by_type_with_latency( >> enum dm_pp_clock_type clk_type, >> struct dm_pp_clock_levels_with_latency *clk_level_info) >> { >> - /* TODO: to be implemented */ >> - return false; >> + struct amdgpu_device *adev = ctx->driver_context; >> + void *pp_handle = adev->powerplay.pp_handle; >> + struct pp_clock_levels_with_latency pp_clks = { 0 }; >> + const struct amd_pm_funcs *pp_funcs = adev->powerplay.pp_funcs; >> + >> + if (!pp_funcs->get_clock_by_type_with_latency) >> + return false; >> + >> + if (pp_funcs->get_clock_by_type_with_latency(pp_handle, >> + >> dc_to_pp_clock_type(clk_type), >> + &pp_clks)) >> + return false; >> + >> + pp_to_dc_clock_levels_with_latency(&pp_clks, clk_level_info, >> clk_type); >> + >> + return true; >> } >> >> bool dm_pp_get_clock_levels_by_type_with_voltage( >> -- >> 2.17.0 >> >> _______________________________________________ >> 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 _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
