Hey Harry, I can't say much about the overall changes (DAL/DC is a large code base and I've no deep understanding of it), but I noticed a couple of things regarding readability, see below. Harry Wentland wrote on 08.12.2016 02:26: > From: Dmytro Laktyushkin <Dmytro.Laktyushkin at amd.com> > > Change-Id: I2eaf2a49eb3ea492fc1960ef8018462f1a83a472 > Signed-off-by: Dmytro Laktyushkin <Dmytro.Laktyushkin at amd.com> > Reviewed-by: Dmytro Laktyushkin <Dmytro.Laktyushkin at amd.com> > Acked-by: Harry Wentland <Harry.Wentland at amd.com> > --- > drivers/gpu/drm/amd/display/dc/Makefile | 7 +- > drivers/gpu/drm/amd/display/dc/dce/dce_clocks.c | 181 ++++++++++++-- > drivers/gpu/drm/amd/display/dc/dce/dce_clocks.h | 36 ++- > drivers/gpu/drm/amd/display/dc/gpu/Makefile | 21 -- > .../display/dc/gpu/dce112/display_clock_dce112.c | 277 --------------------- > .../display/dc/gpu/dce112/display_clock_dce112.h | 79 ------ > drivers/gpu/drm/amd/display/dc/gpu/divider_range.c | 127 ---------- > drivers/gpu/drm/amd/display/dc/gpu/divider_range.h | 62 ----- > drivers/gpu/drm/amd/display/dc/inc/core_types.h | 2 +- > .../inc/hw/display_clock.h} | 27 +- > 10 files changed, 206 insertions(+), 613 deletions(-) > delete mode 100644 drivers/gpu/drm/amd/display/dc/gpu/Makefile > delete mode 100644 drivers/gpu/drm/amd/display/dc/gpu/dce112/display_clock_dce112.c > delete mode 100644 drivers/gpu/drm/amd/display/dc/gpu/dce112/display_clock_dce112.h > delete mode 100644 drivers/gpu/drm/amd/display/dc/gpu/divider_range.c > delete mode 100644 drivers/gpu/drm/amd/display/dc/gpu/divider_range.h > rename drivers/gpu/drm/amd/display/{include/display_clock_interface.h => dc/inc/hw/display_clock.h} (75%) > > diff --git a/drivers/gpu/drm/amd/display/dc/Makefile b/drivers/gpu/drm/amd/display/dc/Makefile > index 5fac034093e9..26e2b50e4954 100644 > --- a/drivers/gpu/drm/amd/display/dc/Makefile > +++ b/drivers/gpu/drm/amd/display/dc/Makefile > @@ -2,8 +2,7 @@ > # Makefile for Display Core (dc) component. > # > > -DC_LIBS = basics bios calcs dce \ > -gpio gpu i2caux irq virtual > +DC_LIBS = basics bios calcs dce gpio i2caux irq virtual > > DC_LIBS += dce112 > DC_LIBS += dce110 > @@ -14,8 +13,8 @@ AMD_DC = $(addsuffix /Makefile, $(addprefix $(FULL_AMD_DISPLAY_PATH)/dc/,$(DC_LI > > include $(AMD_DC) > > -DISPLAY_CORE = dc.o dc_link.o dc_resource.o dc_hw_sequencer.o dc_target.o dc_sink.o dc_stream.o \ > -dc_surface.o dc_link_hwss.o dc_link_dp.o dc_link_ddc.o dc_debug.o > +DISPLAY_CORE = dc.o dc_link.o dc_resource.o dc_hw_sequencer.o dc_target.o dc_sink.o \ > +dc_surface.o dc_link_hwss.o dc_link_dp.o dc_link_ddc.o dc_debug.o dc_stream.o > > AMD_DISPLAY_CORE = $(addprefix $(AMDDALPATH)/dc/core/,$(DISPLAY_CORE)) > > diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clocks.c b/drivers/gpu/drm/amd/display/dc/dce/dce_clocks.c > index 846754c16831..c2bd8dc7b4ad 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clocks.c > +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clocks.c > @@ -30,6 +30,8 @@ > #include "bios_parser_interface.h" > #include "dc.h" > > +#define TO_DCE_CLOCKS(clocks)\ > + container_of(clocks, struct dce_disp_clk, base) > > #define REG(reg) \ > (clk_dce->regs->reg) > @@ -41,8 +43,45 @@ > #define CTX \ > clk_dce->base.ctx > > +/* Max clock values for each state indexed by "enum clocks_state": */ > +static struct state_dependent_clocks dce80_max_clks_by_state[] = { > +/* ClocksStateInvalid - should not be used */ > +{ .display_clk_khz = 0, .pixel_clk_khz = 0 }, > +/* ClocksStateUltraLow - not expected to be used for DCE 8.0 */ > +{ .display_clk_khz = 0, .pixel_clk_khz = 0 }, > +/* ClocksStateLow */ > +{ .display_clk_khz = 352000, .pixel_clk_khz = 330000}, > +/* ClocksStateNominal */ > +{ .display_clk_khz = 600000, .pixel_clk_khz = 400000 }, > +/* ClocksStatePerformance */ > +{ .display_clk_khz = 600000, .pixel_clk_khz = 400000 } }; > + > +static struct state_dependent_clocks dce110_max_clks_by_state[] = { > +/*ClocksStateInvalid - should not be used*/ > +{ .display_clk_khz = 0, .pixel_clk_khz = 0 }, > +/*ClocksStateUltraLow - currently by HW design team not supposed to be used*/ > +{ .display_clk_khz = 352000, .pixel_clk_khz = 330000 }, > +/*ClocksStateLow*/ > +{ .display_clk_khz = 352000, .pixel_clk_khz = 330000 }, > +/*ClocksStateNominal*/ > +{ .display_clk_khz = 467000, .pixel_clk_khz = 400000 }, > +/*ClocksStatePerformance*/ > +{ .display_clk_khz = 643000, .pixel_clk_khz = 400000 } }; > + > +static struct state_dependent_clocks dce112_max_clks_by_state[] = { > +/*ClocksStateInvalid - should not be used*/ > +{ .display_clk_khz = 0, .pixel_clk_khz = 0 }, > +/*ClocksStateUltraLow - currently by HW design team not supposed to be used*/ > +{ .display_clk_khz = 389189, .pixel_clk_khz = 346672 }, > +/*ClocksStateLow*/ > +{ .display_clk_khz = 459000, .pixel_clk_khz = 400000 }, > +/*ClocksStateNominal*/ > +{ .display_clk_khz = 667000, .pixel_clk_khz = 600000 }, > +/*ClocksStatePerformance*/ > +{ .display_clk_khz = 1132000, .pixel_clk_khz = 600000 } }; Can these be aligned like a table? For example like: static struct state_dependent_clocks dce112_max_clks_by_state[] = { { .display_clk_khz = 0, .pixel_clk_khz = 0 }, { .display_clk_khz = 389189, .pixel_clk_khz = 346672 }, { .display_clk_khz = 459000, .pixel_clk_khz = 400000 }, { .display_clk_khz = 667000, .pixel_clk_khz = 600000 }, { .display_clk_khz = 1132000, .pixel_clk_khz = 600000 }, }; Bonus points, if you make the alignment match across all of these structs. ;-) > /* Starting point for each divider range.*/ > -enum divider_range_start { > +enum dce_divider_range_start { > DIVIDER_RANGE_01_START = 200, /* 2.00*/ > DIVIDER_RANGE_02_START = 1600, /* 16.00*/ > DIVIDER_RANGE_03_START = 3200, /* 32.00*/ > @@ -51,7 +90,7 @@ enum divider_range_start { > > /* Ranges for divider identifiers (Divider ID or DID) > mmDENTIST_DISPCLK_CNTL.DENTIST_DISPCLK_WDIVIDER*/ > -enum divider_id_register_setting { > +enum dce_divider_id_register_setting { > DIVIDER_RANGE_01_BASE_DIVIDER_ID = 0X08, > DIVIDER_RANGE_02_BASE_DIVIDER_ID = 0X40, > DIVIDER_RANGE_03_BASE_DIVIDER_ID = 0X60, > @@ -61,12 +100,112 @@ enum divider_id_register_setting { > /* Step size between each divider within a range. > Incrementing the DENTIST_DISPCLK_WDIVIDER by one > will increment the divider by this much.*/ > -enum divider_range_step_size { > +enum dce_divider_range_step_size { > DIVIDER_RANGE_01_STEP_SIZE = 25, /* 0.25*/ > DIVIDER_RANGE_02_STEP_SIZE = 50, /* 0.50*/ > DIVIDER_RANGE_03_STEP_SIZE = 100 /* 1.00 */ > }; > > +static bool dce_divider_range_construct( > + struct dce_divider_range *div_range, > + int range_start, > + int range_step, > + int did_min, > + int did_max) > +{ Is it guaranteed, that div_range is not a null pointer? This is a common question and applies to the functions below as well. I expected an if(!div_range) return false; > + div_range->div_range_start = range_start; > + div_range->div_range_step = range_step; > + div_range->did_min = did_min; > + div_range->did_max = did_max; > + > + if (div_range->div_range_step == 0) { > + div_range->div_range_step = 1; > + /*div_range_step cannot be zero*/ > + BREAK_TO_DEBUGGER(); > + } > + /* Calculate this based on the other inputs.*/ > + /* See DividerRange.h for explanation of */ > + /* the relationship between divider id (DID) and a divider.*/ > + /* Number of Divider IDs = (Maximum Divider ID - Minimum Divider ID)*/ > + /* Maximum divider identified in this range = > + * (Number of Divider IDs)*Step size between dividers > + * + The start of this range.*/ I'd be nice if you used only one opening and one closing statement for the comment. You can always add a asterisk to the front: /* * Multi-line * comment here */ It really took me a while to parse the comment â?? especially since there are sometimes spaces between the closing statement and sometimes not. > + div_range->div_range_end = (did_max - did_min) * range_step > + + range_start; > + return true; > +} > + > +static int dce_divider_range_calc_divider( > + struct dce_divider_range *div_range, > + int did) > +{ > + /* Is this DID within our range?*/ > + if ((did < div_range->did_min) || (did >= div_range->did_max)) > + return INVALID_DIVIDER; > + > + return ((did - div_range->did_min) * div_range->div_range_step) > + + div_range->div_range_start; > + > +} > + > +static int dce_divider_range_calc_did( > + struct dce_divider_range *div_range, > + int div) > +{ > + int did; > + /* Check before dividing.*/ > + if (div_range->div_range_step == 0) { > + div_range->div_range_step = 1; > + /*div_range_step cannot be zero*/ > + BREAK_TO_DEBUGGER(); > + } > + /* Is this divider within our range?*/ > + if ((div < div_range->div_range_start) > + || (div >= div_range->div_range_end)) > + return INVALID_DID; > +/* did = (divider - range_start + (range_step-1)) / range_step) + did_min*/ > + did = div - div_range->div_range_start; > + did += div_range->div_range_step - 1; > + did /= div_range->div_range_step; > + did += div_range->did_min; > + return did; > +} > + > +static int dce_divider_range_get_divider( > + struct dce_divider_range *div_range, > + int ranges_num, > + int did) > +{ > + int div = INVALID_DIVIDER; > + int i; > + > + for (i = 0; i < ranges_num; i++) { > + /* Calculate divider with given divider ID*/ > + div = dce_divider_range_calc_divider(&div_range[i], did); > + /* Found a valid return divider*/ > + if (div != INVALID_DIVIDER) > + break; > + } > + return div; > +} > + > +static int dce_divider_range_get_did( > + struct dce_divider_range *div_range, > + int ranges_num, > + int divider) > +{ > + int did = INVALID_DID; > + int i; > + > + for (i = 0; i < ranges_num; i++) { > + /* CalcDid returns InvalidDid if a divider ID isn't found*/ > + did = dce_divider_range_calc_did(&div_range[i], divider); > + /* Found a valid return did*/ > + if (did != INVALID_DID) > + break; > + } > + return did; > +} > > static uint32_t dce_clocks_get_dp_ref_freq(struct display_clock *clk) > { > @@ -85,7 +224,7 @@ static uint32_t dce_clocks_get_dp_ref_freq(struct display_clock *clk) > REG_GET(DENTIST_DISPCLK_CNTL, DENTIST_DPREFCLK_WDIVIDER, &dprefclk_wdivider); > > /* Convert DENTIST_DPREFCLK_WDIVIDERto actual divider*/ > - target_div = dal_divider_range_get_divider( > + target_div = dce_divider_range_get_divider( > clk_dce->divider_ranges, > DIVIDER_RANGE_MAX, > dprefclk_wdivider); > @@ -183,7 +322,7 @@ static bool dce_clock_set_min_clocks_state( > > static void dce_set_clock( > struct display_clock *clk, > - uint32_t requested_clk_khz) > + int requested_clk_khz) > { > struct dce_disp_clk *clk_dce = TO_DCE_CLOCKS(clk); > struct bp_pixel_clock_parameters pxl_clk_params = { 0 }; > @@ -245,7 +384,7 @@ static void dce_psr_wait_loop( > > static void dce_psr_set_clock( > struct display_clock *clk, > - uint32_t requested_clk_khz) > + int requested_clk_khz) > { > struct dce_disp_clk *clk_dce = TO_DCE_CLOCKS(clk); > > @@ -253,9 +392,9 @@ static void dce_psr_set_clock( > dce_psr_wait_loop(clk_dce, requested_clk_khz); > } > > -static void polaris_set_clock( > +static void dce112_set_clock( > struct display_clock *clk, > - uint32_t requested_clk_khz) > + int requested_clk_khz) > { > struct dce_disp_clk *clk_dce = TO_DCE_CLOCKS(clk); > struct bp_set_dce_clock_parameters dce_clk_params; > @@ -267,7 +406,7 @@ static void polaris_set_clock( > /* Make sure requested clock isn't lower than minimum threshold*/ > if (requested_clk_khz > 0) > requested_clk_khz = dm_max(requested_clk_khz, > - clk_dce->dentist_vco_freq_khz / 64); > + clk_dce->dentist_vco_freq_khz / 62); Can you document why 62? Maybe a speaking #define would be best? Cheers, Kai > dce_clk_params.target_clock_frequency = requested_clk_khz; > dce_clk_params.pll_id = CLOCK_SOURCE_ID_DFS; > @@ -284,7 +423,9 @@ static void polaris_set_clock( > /*VBIOS will determine DPREFCLK frequency, so we don't set it*/ > dce_clk_params.target_clock_frequency = 0; > dce_clk_params.clock_type = DCECLOCK_TYPE_DPREFCLK; > - dce_clk_params.flags.USE_GENLOCK_AS_SOURCE_FOR_DPREFCLK = 0; > + dce_clk_params.flags.USE_GENLOCK_AS_SOURCE_FOR_DPREFCLK = > + (dce_clk_params.pll_id == > + CLOCK_SOURCE_COMBO_DISPLAY_PLL0); > > bp->funcs->set_dce_clock(bp, &dce_clk_params); > } > @@ -386,7 +527,7 @@ static const struct display_clock_funcs dce112_funcs = { > .get_dp_ref_clk_frequency = dce_clocks_get_dp_ref_freq, > .get_required_clocks_state = dce_get_required_clocks_state, > .set_min_clocks_state = dce_clock_set_min_clocks_state, > - .set_clock = polaris_set_clock > + .set_clock = dce112_set_clock > }; > > static const struct display_clock_funcs dce110_funcs = { > @@ -429,19 +570,19 @@ static void dce_disp_clk_construct( > dce_clock_read_integrated_info(clk_dce); > dce_clock_read_ss_info(clk_dce); > > - dal_divider_range_construct( > + dce_divider_range_construct( > &clk_dce->divider_ranges[DIVIDER_RANGE_01], > DIVIDER_RANGE_01_START, > DIVIDER_RANGE_01_STEP_SIZE, > DIVIDER_RANGE_01_BASE_DIVIDER_ID, > DIVIDER_RANGE_02_BASE_DIVIDER_ID); > - dal_divider_range_construct( > + dce_divider_range_construct( > &clk_dce->divider_ranges[DIVIDER_RANGE_02], > DIVIDER_RANGE_02_START, > DIVIDER_RANGE_02_STEP_SIZE, > DIVIDER_RANGE_02_BASE_DIVIDER_ID, > DIVIDER_RANGE_03_BASE_DIVIDER_ID); > - dal_divider_range_construct( > + dce_divider_range_construct( > &clk_dce->divider_ranges[DIVIDER_RANGE_03], > DIVIDER_RANGE_03_START, > DIVIDER_RANGE_03_STEP_SIZE, > @@ -462,6 +603,10 @@ struct display_clock *dce_disp_clk_create( > return NULL; > } > > + memcpy(clk_dce->max_clks_by_state, > + dce80_max_clks_by_state, > + sizeof(dce80_max_clks_by_state)); > + > dce_disp_clk_construct( > clk_dce, ctx, regs, clk_shift, clk_mask); > > @@ -481,6 +626,10 @@ struct display_clock *dce110_disp_clk_create( > return NULL; > } > > + memcpy(clk_dce->max_clks_by_state, > + dce110_max_clks_by_state, > + sizeof(dce110_max_clks_by_state)); > + > dce_disp_clk_construct( > clk_dce, ctx, regs, clk_shift, clk_mask); > > @@ -502,6 +651,10 @@ struct display_clock *dce112_disp_clk_create( > return NULL; > } > > + memcpy(clk_dce->max_clks_by_state, > + dce112_max_clks_by_state, > + sizeof(dce112_max_clks_by_state)); > + > dce_disp_clk_construct( > clk_dce, ctx, regs, clk_shift, clk_mask); > > diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clocks.h b/drivers/gpu/drm/amd/display/dc/dce/dce_clocks.h > index e3b23749f0a2..4ad6fe4c2841 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clocks.h > +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clocks.h > @@ -27,12 +27,7 @@ > #ifndef _DCE_CLOCKS_H_ > #define _DCE_CLOCKS_H_ > > -#include "display_clock_interface.h" > -#include "../gpu/divider_range.h" > - > - > -#define TO_DCE_CLOCKS(clocks)\ > - container_of(clocks, struct dce_disp_clk, base) > +#include "display_clock.h" > > #define CLK_COMMON_REG_LIST_DCE_BASE() \ > .DPREFCLK_CNTL = mmDPREFCLK_CNTL, \ > @@ -74,13 +69,30 @@ struct dce_disp_clk_registers { > }; > > /* Array identifiers and count for the divider ranges.*/ > -enum divider_range_count { > +enum dce_divider_range_count { > DIVIDER_RANGE_01 = 0, > DIVIDER_RANGE_02, > DIVIDER_RANGE_03, > DIVIDER_RANGE_MAX /* == 3*/ > }; > > +enum dce_divider_error_types { > + INVALID_DID = 0, > + INVALID_DIVIDER = 1 > +}; > + > +struct dce_divider_range { > + int div_range_start; > + /* The end of this range of dividers.*/ > + int div_range_end; > + /* The distance between each divider in this range.*/ > + int div_range_step; > + /* The divider id for the lowest divider.*/ > + int did_min; > + /* The divider id for the highest divider.*/ > + int did_max; > +}; > + > struct dce_disp_clk { > struct display_clock base; > const struct dce_disp_clk_registers *regs; > @@ -88,23 +100,23 @@ struct dce_disp_clk { > const struct dce_disp_clk_mask *clk_mask; > > struct state_dependent_clocks max_clks_by_state[DM_PP_CLOCKS_MAX_STATES]; > - struct divider_range divider_ranges[DIVIDER_RANGE_MAX]; > + struct dce_divider_range divider_ranges[DIVIDER_RANGE_MAX]; > > bool use_max_disp_clk; > - uint32_t dentist_vco_freq_khz; > + int dentist_vco_freq_khz; > > /* Cache the status of DFS-bypass feature*/ > bool dfs_bypass_enabled; > /* Cache the display clock returned by VBIOS if DFS-bypass is enabled. > * This is basically "Crystal Frequency In KHz" (XTALIN) frequency */ > - uint32_t dfs_bypass_disp_clk; > + int dfs_bypass_disp_clk; > > /* Flag for Enabled SS on GPU PLL */ > bool ss_on_gpu_pll; > /* GPU PLL SS percentage (if down-spread enabled) */ > - uint32_t gpu_pll_ss_percentage; > + int gpu_pll_ss_percentage; > /* GPU PLL SS percentage Divider (100 or 1000) */ > - uint32_t gpu_pll_ss_divider; > + int gpu_pll_ss_divider; > > > }; > diff --git a/drivers/gpu/drm/amd/display/dc/gpu/Makefile b/drivers/gpu/drm/amd/display/dc/gpu/Makefile > deleted file mode 100644 > index 6ab4078405f2..000000000000 > --- a/drivers/gpu/drm/amd/display/dc/gpu/Makefile > +++ /dev/null > @@ -1,21 +0,0 @@ > -# > -# Makefile for the 'gpu' sub-component of DAL. > -# It provides the control and status of HW adapter resources, > -# that are global for the ASIC and sharable between pipes. > - > -GPU = divider_range.o > - > -AMD_DAL_GPU = $(addprefix $(AMDDALPATH)/dc/gpu/,$(GPU)) > - > -AMD_DISPLAY_FILES += $(AMD_DAL_GPU) > - > -############################################################################### > -# DCE 110 family > -############################################################################### > - > -GPU_DCE112 = display_clock_dce112.o > - > -AMD_DAL_GPU_DCE112 = $(addprefix $(AMDDALPATH)/dc/gpu/dce112/,$(GPU_DCE112)) > - > -AMD_DISPLAY_FILES += $(AMD_DAL_GPU_DCE110) $(AMD_DAL_GPU_DCE112) > - > diff --git a/drivers/gpu/drm/amd/display/dc/gpu/dce112/display_clock_dce112.c b/drivers/gpu/drm/amd/display/dc/gpu/dce112/display_clock_dce112.c > deleted file mode 100644 > index e0d67fb6c633..000000000000 > --- a/drivers/gpu/drm/amd/display/dc/gpu/dce112/display_clock_dce112.c > +++ /dev/null > @@ -1,277 +0,0 @@ > -/* > - * Copyright 2012-15 Advanced Micro Devices, Inc. > - * > - * Permission is hereby granted, free of charge, to any person obtaining a > - * copy of this software and associated documentation files (the "Software"), > - * to deal in the Software without restriction, including without limitation > - * the rights to use, copy, modify, merge, publish, distribute, sublicense, > - * and/or sell copies of the Software, and to permit persons to whom the > - * Software is furnished to do so, subject to the following conditions: > - * > - * The above copyright notice and this permission notice shall be included in > - * all copies or substantial portions of the Software. > - * > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR > - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > - * OTHER DEALINGS IN THE SOFTWARE. > - * > - * Authors: AMD > - * > - */ > - > -#include "dm_services.h" > - > -#include "dce/dce_11_2_d.h" > -#include "dce/dce_11_2_sh_mask.h" > - > -#include "include/bios_parser_interface.h" > -#include "include/fixed32_32.h" > -#include "include/logger_interface.h" > - > -#include "../divider_range.h" > - > -#include "display_clock_dce112.h" > - > -#define FROM_DISPLAY_CLOCK(base) \ > - container_of(base, struct display_clock_dce112, disp_clk_base) > - > -static struct state_dependent_clocks max_clks_by_state[] = { > -/*ClocksStateInvalid - should not be used*/ > -{ .display_clk_khz = 0, .pixel_clk_khz = 0 }, > -/*ClocksStateUltraLow - currently by HW design team not supposed to be used*/ > -{ .display_clk_khz = 389189, .pixel_clk_khz = 346672 }, > -/*ClocksStateLow*/ > -{ .display_clk_khz = 459000, .pixel_clk_khz = 400000 }, > -/*ClocksStateNominal*/ > -{ .display_clk_khz = 667000, .pixel_clk_khz = 600000 }, > -/*ClocksStatePerformance*/ > -{ .display_clk_khz = 1132000, .pixel_clk_khz = 600000 } }; > - > -/* Ranges for divider identifiers (Divider ID or DID) > - mmDENTIST_DISPCLK_CNTL.DENTIST_DISPCLK_WDIVIDER*/ > -enum divider_id_register_setting { > - DIVIDER_RANGE_01_BASE_DIVIDER_ID = 0X08, > - DIVIDER_RANGE_02_BASE_DIVIDER_ID = 0X40, > - DIVIDER_RANGE_03_BASE_DIVIDER_ID = 0X60, > - DIVIDER_RANGE_MAX_DIVIDER_ID = 0X80 > -}; > - > -/* Step size between each divider within a range. > - Incrementing the DENTIST_DISPCLK_WDIVIDER by one > - will increment the divider by this much.*/ > -enum divider_range_step_size { > - DIVIDER_RANGE_01_STEP_SIZE = 25, /* 0.25*/ > - DIVIDER_RANGE_02_STEP_SIZE = 50, /* 0.50*/ > - DIVIDER_RANGE_03_STEP_SIZE = 100 /* 1.00 */ > -}; > - > -static struct divider_range divider_ranges[DIVIDER_RANGE_MAX]; > - > - > -void dispclk_dce112_destroy(struct display_clock **base) > -{ > - struct display_clock_dce112 *dc112; > - > - dc112 = DCLCK112_FROM_BASE(*base); > - > - dm_free(dc112); > - > - *base = NULL; > -} > - > -static bool display_clock_integrated_info_construct( > - struct display_clock_dce112 *disp_clk) > -{ > - struct integrated_info info; > - uint32_t i; > - struct display_clock *base = &disp_clk->disp_clk_base; > - > - memset(&info, 0, sizeof(struct integrated_info)); > - > - disp_clk->dentist_vco_freq_khz = info.dentist_vco_freq; > - if (disp_clk->dentist_vco_freq_khz == 0) > - disp_clk->dentist_vco_freq_khz = 3600000; > - > - base->min_display_clk_threshold_khz = > - disp_clk->dentist_vco_freq_khz / 64; > - > - /*update the maximum display clock for each power state*/ > - for (i = 0; i < NUMBER_OF_DISP_CLK_VOLTAGE; ++i) { > - enum dm_pp_clocks_state clk_state = DM_PP_CLOCKS_STATE_INVALID; > - > - switch (i) { > - case 0: > - clk_state = DM_PP_CLOCKS_STATE_ULTRA_LOW; > - break; > - > - case 1: > - clk_state = DM_PP_CLOCKS_STATE_LOW; > - break; > - > - case 2: > - clk_state = DM_PP_CLOCKS_STATE_NOMINAL; > - break; > - > - case 3: > - clk_state = DM_PP_CLOCKS_STATE_PERFORMANCE; > - break; > - > - default: > - clk_state = DM_PP_CLOCKS_STATE_INVALID; > - break; > - } > - > - /*Do not allow bad VBIOS/SBIOS to override with invalid values, > - * check for > 100MHz*/ > - if (info.disp_clk_voltage[i].max_supported_clk >= 100000) { > - (disp_clk->max_clks_by_state + clk_state)-> > - display_clk_khz = > - info.disp_clk_voltage[i].max_supported_clk; > - } > - } > - > - return true; > -} > - > -void dce112_set_clock( > - struct display_clock *base, > - uint32_t requested_clk_khz) > -{ > - struct bp_set_dce_clock_parameters dce_clk_params; > - struct dc_bios *bp = base->ctx->dc_bios; > - > - /* Prepare to program display clock*/ > - memset(&dce_clk_params, 0, sizeof(dce_clk_params)); > - > - /* Make sure requested clock isn't lower than minimum threshold*/ > - if (requested_clk_khz > 0) > - requested_clk_khz = dm_max(requested_clk_khz, > - base->min_display_clk_threshold_khz); > - > - dce_clk_params.target_clock_frequency = requested_clk_khz; > - dce_clk_params.pll_id = CLOCK_SOURCE_ID_DFS; > - dce_clk_params.clock_type = DCECLOCK_TYPE_DISPLAY_CLOCK; > - > - bp->funcs->set_dce_clock(bp, &dce_clk_params); > - > - /* from power down, we need mark the clock state as ClocksStateNominal > - * from HWReset, so when resume we will call pplib voltage regulator.*/ > - if (requested_clk_khz == 0) > - base->cur_min_clks_state = DM_PP_CLOCKS_STATE_NOMINAL; > - > - /*Program DP ref Clock*/ > - /*VBIOS will determine DPREFCLK frequency, so we don't set it*/ > - dce_clk_params.target_clock_frequency = 0; > - dce_clk_params.clock_type = DCECLOCK_TYPE_DPREFCLK; > - dce_clk_params.flags.USE_GENLOCK_AS_SOURCE_FOR_DPREFCLK = > - (dce_clk_params.pll_id == > - CLOCK_SOURCE_COMBO_DISPLAY_PLL0); > - > - bp->funcs->set_dce_clock(bp, &dce_clk_params); > -} > - > -static const struct display_clock_funcs funcs = { > - .destroy = dispclk_dce112_destroy, > - .set_clock = dce112_set_clock, > -}; > - > -bool dal_display_clock_dce112_construct( > - struct display_clock_dce112 *dc112, > - struct dc_context *ctx) > -{ > - struct display_clock *dc_base = &dc112->disp_clk_base; > - > - dc_base->ctx = ctx; > - dc_base->min_display_clk_threshold_khz = 0; > - > - dc_base->cur_min_clks_state = DM_PP_CLOCKS_STATE_INVALID; > - > - dc_base->funcs = &funcs; > - > - dc112->dfs_bypass_disp_clk = 0; > - > - if (!display_clock_integrated_info_construct(dc112)) > - dm_logger_write(dc_base->ctx->logger, LOG_WARNING, > - "Cannot obtain VBIOS integrated info\n"); > - > - dc112->gpu_pll_ss_percentage = 0; > - dc112->gpu_pll_ss_divider = 1000; > - dc112->ss_on_gpu_pll = false; > - > -/* Initially set max clocks state to nominal. This should be updated by > - * via a pplib call to DAL IRI eventually calling a > - * DisplayEngineClock_dce112::StoreMaxClocksState(). This call will come in > - * on PPLIB init. This is from DCE5x. in case HW wants to use mixed method.*/ > - dc_base->max_clks_state = DM_PP_CLOCKS_STATE_NOMINAL; > - > - dc112->disp_clk_base.min_display_clk_threshold_khz = > - (dc112->dentist_vco_freq_khz / 62); > - > - dal_divider_range_construct( > - ÷r_ranges[DIVIDER_RANGE_01], > - DIVIDER_RANGE_01_START, > - DIVIDER_RANGE_01_STEP_SIZE, > - DIVIDER_RANGE_01_BASE_DIVIDER_ID, > - DIVIDER_RANGE_02_BASE_DIVIDER_ID); > - dal_divider_range_construct( > - ÷r_ranges[DIVIDER_RANGE_02], > - DIVIDER_RANGE_02_START, > - DIVIDER_RANGE_02_STEP_SIZE, > - DIVIDER_RANGE_02_BASE_DIVIDER_ID, > - DIVIDER_RANGE_03_BASE_DIVIDER_ID); > - dal_divider_range_construct( > - ÷r_ranges[DIVIDER_RANGE_03], > - DIVIDER_RANGE_03_START, > - DIVIDER_RANGE_03_STEP_SIZE, > - DIVIDER_RANGE_03_BASE_DIVIDER_ID, > - DIVIDER_RANGE_MAX_DIVIDER_ID); > - > - { > - uint32_t ss_info_num = > - ctx->dc_bios->funcs-> > - get_ss_entry_number(ctx->dc_bios, AS_SIGNAL_TYPE_GPU_PLL); > - > - if (ss_info_num) { > - struct spread_spectrum_info info; > - bool result; > - > - memset(&info, 0, sizeof(info)); > - > - result = > - (BP_RESULT_OK == ctx->dc_bios->funcs-> > - get_spread_spectrum_info(ctx->dc_bios, > - AS_SIGNAL_TYPE_GPU_PLL, 0, &info)) ? true : false; > - > - > - /* Based on VBIOS, VBIOS will keep entry for GPU PLL SS > - * even if SS not enabled and in that case > - * SSInfo.spreadSpectrumPercentage !=0 would be sign > - * that SS is enabled > - */ > - if (result && info.spread_spectrum_percentage != 0) { > - dc112->ss_on_gpu_pll = true; > - dc112->gpu_pll_ss_divider = > - info.spread_percentage_divider; > - > - if (info.type.CENTER_MODE == 0) { > - /* Currently for DP Reference clock we > - * need only SS percentage for > - * downspread */ > - dc112->gpu_pll_ss_percentage = > - info.spread_spectrum_percentage; > - } > - } > - > - } > - } > - > - dc112->use_max_disp_clk = true; > - dc112->max_clks_by_state = max_clks_by_state; > - > - return true; > -} > - > diff --git a/drivers/gpu/drm/amd/display/dc/gpu/dce112/display_clock_dce112.h b/drivers/gpu/drm/amd/display/dc/gpu/dce112/display_clock_dce112.h > deleted file mode 100644 > index 0743c514a60f..000000000000 > --- a/drivers/gpu/drm/amd/display/dc/gpu/dce112/display_clock_dce112.h > +++ /dev/null > @@ -1,79 +0,0 @@ > -/* > - * Copyright 2012-15 Advanced Micro Devices, Inc. > - * > - * Permission is hereby granted, free of charge, to any person obtaining a > - * copy of this software and associated documentation files (the "Software"), > - * to deal in the Software without restriction, including without limitation > - * the rights to use, copy, modify, merge, publish, distribute, sublicense, > - * and/or sell copies of the Software, and to permit persons to whom the > - * Software is furnished to do so, subject to the following conditions: > - * > - * The above copyright notice and this permission notice shall be included in > - * all copies or substantial portions of the Software. > - * > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR > - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > - * OTHER DEALINGS IN THE SOFTWARE. > - * > - * Authors: AMD > - * > - */ > -#ifndef __DAL_DISPLAY_CLOCK_DCE112_H__ > -#define __DAL_DISPLAY_CLOCK_DCE112_H__ > - > -#include "display_clock_interface.h" > - > -struct display_clock_dce112 { > - struct display_clock disp_clk_base; > - bool use_max_disp_clk; > - uint32_t dentist_vco_freq_khz; > - /* Cache the status of DFS-bypass feature*/ > - bool dfs_bypass_enabled; > - /* GPU PLL SS percentage (if down-spread enabled) */ > - uint32_t gpu_pll_ss_percentage; > - /* GPU PLL SS percentage Divider (100 or 1000) */ > - uint32_t gpu_pll_ss_divider; > - /* Flag for Enabled SS on GPU PLL */ > - bool ss_on_gpu_pll; > - /* Cache the display clock returned by VBIOS if DFS-bypass is enabled. > - * This is basically "Crystal Frequency In KHz" (XTALIN) frequency */ > - uint32_t dfs_bypass_disp_clk; > - struct state_dependent_clocks *max_clks_by_state; > - > -}; > - > -#define DCLCK112_FROM_BASE(dc_base) \ > - container_of(dc_base, struct display_clock_dce112, disp_clk_base) > - > -/* Array identifiers and count for the divider ranges.*/ > -enum divider_range_count { > - DIVIDER_RANGE_01 = 0, > - DIVIDER_RANGE_02, > - DIVIDER_RANGE_03, > - DIVIDER_RANGE_MAX /* == 3*/ > -}; > - > -/* Starting point for each divider range.*/ > -enum divider_range_start { > - DIVIDER_RANGE_01_START = 200, /* 2.00*/ > - DIVIDER_RANGE_02_START = 1600, /* 16.00*/ > - DIVIDER_RANGE_03_START = 3200, /* 32.00*/ > - DIVIDER_RANGE_SCALE_FACTOR = 100 /* Results are scaled up by 100.*/ > -}; > - > -bool dal_display_clock_dce112_construct( > - struct display_clock_dce112 *dc112, > - struct dc_context *ctx); > - > -void dispclk_dce112_destroy(struct display_clock **base); > - > -void dce112_set_clock( > - struct display_clock *base, > - uint32_t requested_clk_khz); > - > - > -#endif /* __DAL_DISPLAY_CLOCK_DCE112_H__ */ > diff --git a/drivers/gpu/drm/amd/display/dc/gpu/divider_range.c b/drivers/gpu/drm/amd/display/dc/gpu/divider_range.c > deleted file mode 100644 > index 59d44004411b..000000000000 > --- a/drivers/gpu/drm/amd/display/dc/gpu/divider_range.c > +++ /dev/null > @@ -1,127 +0,0 @@ > -/* > - * Copyright 2012-15 Advanced Micro Devices, Inc. > - * > - * Permission is hereby granted, free of charge, to any person obtaining a > - * copy of this software and associated documentation files (the "Software"), > - * to deal in the Software without restriction, including without limitation > - * the rights to use, copy, modify, merge, publish, distribute, sublicense, > - * and/or sell copies of the Software, and to permit persons to whom the > - * Software is furnished to do so, subject to the following conditions: > - * > - * The above copyright notice and this permission notice shall be included in > - * all copies or substantial portions of the Software. > - * > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR > - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > - * OTHER DEALINGS IN THE SOFTWARE. > - * > - * Authors: AMD > - * > - */ > -#include "dm_services.h" > -#include "divider_range.h" > - > -bool dal_divider_range_construct( > - struct divider_range *div_range, > - uint32_t range_start, > - uint32_t range_step, > - uint32_t did_min, > - uint32_t did_max) > -{ > - div_range->div_range_start = range_start; > - div_range->div_range_step = range_step; > - div_range->did_min = did_min; > - div_range->did_max = did_max; > - > - if (div_range->div_range_step == 0) { > - div_range->div_range_step = 1; > - /*div_range_step cannot be zero*/ > - BREAK_TO_DEBUGGER(); > - } > - /* Calculate this based on the other inputs.*/ > - /* See DividerRange.h for explanation of */ > - /* the relationship between divider id (DID) and a divider.*/ > - /* Number of Divider IDs = (Maximum Divider ID - Minimum Divider ID)*/ > - /* Maximum divider identified in this range = > - * (Number of Divider IDs)*Step size between dividers > - * + The start of this range.*/ > - div_range->div_range_end = (did_max - did_min) * range_step > - + range_start; > - return true; > -} > - > -static uint32_t dal_divider_range_calc_divider( > - struct divider_range *div_range, > - uint32_t did) > -{ > - /* Is this DID within our range?*/ > - if ((did < div_range->did_min) || (did >= div_range->did_max)) > - return INVALID_DIVIDER; > - > - return ((did - div_range->did_min) * div_range->div_range_step) > - + div_range->div_range_start; > - > -} > - > -static uint32_t dal_divider_range_calc_did( > - struct divider_range *div_range, > - uint32_t div) > -{ > - uint32_t did; > - /* Check before dividing.*/ > - if (div_range->div_range_step == 0) { > - div_range->div_range_step = 1; > - /*div_range_step cannot be zero*/ > - BREAK_TO_DEBUGGER(); > - } > - /* Is this divider within our range?*/ > - if ((div < div_range->div_range_start) > - || (div >= div_range->div_range_end)) > - return INVALID_DID; > -/* did = (divider - range_start + (range_step-1)) / range_step) + did_min*/ > - did = div - div_range->div_range_start; > - did += div_range->div_range_step - 1; > - did /= div_range->div_range_step; > - did += div_range->did_min; > - return did; > -} > - > -uint32_t dal_divider_range_get_divider( > - struct divider_range *div_range, > - uint32_t ranges_num, > - uint32_t did) > -{ > - uint32_t div = INVALID_DIVIDER; > - uint32_t i; > - > - for (i = 0; i < ranges_num; i++) { > - /* Calculate divider with given divider ID*/ > - div = dal_divider_range_calc_divider(&div_range[i], did); > - /* Found a valid return divider*/ > - if (div != INVALID_DIVIDER) > - break; > - } > - return div; > -} > -uint32_t dal_divider_range_get_did( > - struct divider_range *div_range, > - uint32_t ranges_num, > - uint32_t divider) > -{ > - uint32_t did = INVALID_DID; > - uint32_t i; > - > - for (i = 0; i < ranges_num; i++) { > - /* CalcDid returns InvalidDid if a divider ID isn't found*/ > - did = dal_divider_range_calc_did(&div_range[i], divider); > - /* Found a valid return did*/ > - if (did != INVALID_DID) > - break; > - } > - return did; > -} > - > diff --git a/drivers/gpu/drm/amd/display/dc/gpu/divider_range.h b/drivers/gpu/drm/amd/display/dc/gpu/divider_range.h > deleted file mode 100644 > index e53522f652cc..000000000000 > --- a/drivers/gpu/drm/amd/display/dc/gpu/divider_range.h > +++ /dev/null > @@ -1,62 +0,0 @@ > -/* > - * Copyright 2012-15 Advanced Micro Devices, Inc. > - * > - * Permission is hereby granted, free of charge, to any person obtaining a > - * copy of this software and associated documentation files (the "Software"), > - * to deal in the Software without restriction, including without limitation > - * the rights to use, copy, modify, merge, publish, distribute, sublicense, > - * and/or sell copies of the Software, and to permit persons to whom the > - * Software is furnished to do so, subject to the following conditions: > - * > - * The above copyright notice and this permission notice shall be included in > - * all copies or substantial portions of the Software. > - * > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR > - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > - * OTHER DEALINGS IN THE SOFTWARE. > - * > - * Authors: AMD > - * > - */ > - > -#ifndef __DAL_DIVIDER_RANGE_H__ > -#define __DAL_DIVIDER_RANGE_H__ > - > -enum divider_error_types { > - INVALID_DID = 0, > - INVALID_DIVIDER = 1 > -}; > - > -struct divider_range { > - uint32_t div_range_start; > - /* The end of this range of dividers.*/ > - uint32_t div_range_end; > - /* The distance between each divider in this range.*/ > - uint32_t div_range_step; > - /* The divider id for the lowest divider.*/ > - uint32_t did_min; > - /* The divider id for the highest divider.*/ > - uint32_t did_max; > -}; > - > -bool dal_divider_range_construct( > - struct divider_range *div_range, > - uint32_t range_start, > - uint32_t range_step, > - uint32_t did_min, > - uint32_t did_max); > - > -uint32_t dal_divider_range_get_divider( > - struct divider_range *div_range, > - uint32_t ranges_num, > - uint32_t did); > -uint32_t dal_divider_range_get_did( > - struct divider_range *div_range, > - uint32_t ranges_num, > - uint32_t divider); > - > -#endif /* __DAL_DIVIDER_RANGE_H__ */ > diff --git a/drivers/gpu/drm/amd/display/dc/inc/core_types.h b/drivers/gpu/drm/amd/display/dc/inc/core_types.h > index f2eb8945d5c4..0418e3e02c7a 100644 > --- a/drivers/gpu/drm/amd/display/dc/inc/core_types.h > +++ b/drivers/gpu/drm/amd/display/dc/inc/core_types.h > @@ -188,7 +188,7 @@ void core_link_enable_stream(struct pipe_ctx *pipe_ctx); > void core_link_disable_stream(struct pipe_ctx *pipe_ctx); > > /********** DAL Core*********************/ > -#include "display_clock_interface.h" > +#include "display_clock.h" > #include "transform.h" > > struct resource_pool; > diff --git a/drivers/gpu/drm/amd/display/include/display_clock_interface.h b/drivers/gpu/drm/amd/display/dc/inc/hw/display_clock.h > similarity index 75% > rename from drivers/gpu/drm/amd/display/include/display_clock_interface.h > rename to drivers/gpu/drm/amd/display/dc/inc/hw/display_clock.h > index 53a01381c638..e163f5818dd9 100644 > --- a/drivers/gpu/drm/amd/display/include/display_clock_interface.h > +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/display_clock.h > @@ -1,5 +1,5 @@ > /* > - * Copyright 2012-15 Advanced Micro Devices, Inc. > + * Copyright 2012-16 Advanced Micro Devices, Inc. > * > * Permission is hereby granted, free of charge, to any person obtaining a > * copy of this software and associated documentation files (the "Software"), > @@ -23,44 +23,39 @@ > * > */ > > -#ifndef __DISPLAY_CLOCK_INTERFACE_H__ > -#define __DISPLAY_CLOCK_INTERFACE_H__ > +#ifndef __DISPLAY_CLOCK_H__ > +#define __DISPLAY_CLOCK_H__ > > #include "dm_services_types.h" > -#include "hw_sequencer_types.h" > -#include "grph_object_defs.h" > -#include "signal_types.h" > > /* Structure containing all state-dependent clocks > * (dependent on "enum clocks_state") */ > struct state_dependent_clocks { > - uint32_t display_clk_khz; > - uint32_t pixel_clk_khz; > + int display_clk_khz; > + int pixel_clk_khz; > }; > > struct display_clock { > struct dc_context *ctx; > const struct display_clock_funcs *funcs; > > - int min_display_clk_threshold_khz; > enum dm_pp_clocks_state max_clks_state; > enum dm_pp_clocks_state cur_min_clks_state; > }; > > struct display_clock_funcs { > - void (*destroy)(struct display_clock **to_destroy); > void (*set_clock)(struct display_clock *disp_clk, > - uint32_t requested_clock_khz); > + int requested_clock_khz); > + > enum dm_pp_clocks_state (*get_required_clocks_state)( > struct display_clock *disp_clk, > struct state_dependent_clocks *req_clocks); > + > bool (*set_min_clocks_state)(struct display_clock *disp_clk, > enum dm_pp_clocks_state dm_pp_clocks_state); > - uint32_t (*get_dp_ref_clk_frequency)(struct display_clock *disp_clk); > > -}; > - > -void dal_display_clock_destroy(struct display_clock **to_destroy); > + int (*get_dp_ref_clk_frequency)(struct display_clock *disp_clk); > > +}; > > -#endif /* __DISPLAY_CLOCK_INTERFACE_H__ */ > +#endif /* __DISPLAY_CLOCK_H__ */ >