[Please CC me on all replies, I'm not subscribed to the list.] Harry Wentland wrote on 08.12.2016 02:26: > Some of those are potential bugs
Are they related? If not: maybe split the changes up? > Change-Id: I53b2c663e18b57013e1b891fc2ecf1fb2d7d3a08 > Signed-off-by: Harry Wentland <[email protected]> > Reviewed-by: Tony Cheng <[email protected]> It'd be nice to see these reviews on list... (applies to all patches here) > Acked-by: Harry Wentland <[email protected]> > --- > .../gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c | 2 +- > drivers/gpu/drm/amd/display/dc/core/dc_resource.c | 5 +-- > .../gpu/drm/amd/display/dc/dce/dce_clock_source.c | 5 --- > drivers/gpu/drm/amd/display/dc/dce/dce_clocks.c | 43 > +--------------------- > drivers/gpu/drm/amd/display/dc/dce/dce_transform.c | 2 - > .../drm/amd/display/dc/dce110/dce110_transform_v.c | 3 +- > 6 files changed, 6 insertions(+), 54 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c > b/drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c > index 0b2bb3992f1a..3b0710ef4716 100644 > --- a/drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c > +++ b/drivers/gpu/drm/amd/display/dc/calcs/bandwidth_calcs.c > @@ -1709,7 +1709,7 @@ static void calculate_bandwidth( > else { > data->blackout_recovery_time = > bw_max2(data->blackout_recovery_time, bw_add(bw_mul(bw_int_to_fixed(2), > vbios->mcifwrmc_urgent_latency), > data->mcifwr_burst_time[data->y_clk_level][data->sclk_level])); > if (bw_ltn(data->adjusted_data_buffer_size[k], > bw_mul(bw_div(bw_mul(data->display_bandwidth[k], > data->useful_bytes_per_request[k]), data->bytes_per_request[k]), > (bw_add(vbios->blackout_duration, bw_add(bw_mul(bw_int_to_fixed(2), > vbios->mcifwrmc_urgent_latency), > data->mcifwr_burst_time[data->y_clk_level][data->sclk_level])))))) { > - data->blackout_recovery_time = > bw_max2(data->blackout_recovery_time, > bw_div((bw_add(bw_mul(bw_div(bw_mul(data->display_bandwidth[k], > data->useful_bytes_per_request[k]), data->bytes_per_request[k]), > vbios->blackout_duration), > bw_sub(bw_div(bw_mul(bw_mul(bw_mul((bw_add(bw_add(bw_mul(bw_int_to_fixed(2), > vbios->mcifwrmc_urgent_latency), data->dmif_burst_time[i][j]), > data->mcifwr_burst_time[data->y_clk_level][data->sclk_level])), > data->dispclk), bw_int_to_fixed(data->bytes_per_pixel[k])), > data->lines_interleaved_in_mem_access[k]), data->latency_hiding_lines[k]), > data->adjusted_data_buffer_size[k]))), > (bw_sub(bw_div(bw_mul(bw_mul(data->dispclk, > bw_int_to_fixed(data->bytes_per_pixel[k])), > data->lines_interleaved_in_mem_access[k]), data->latency_hiding_lines[k]), > bw_div(bw_mul(data->display_bandwidth[k], data->useful_bytes_per_request[k]), > data->bytes_per_request[k]))))); > + data->blackout_recovery_time = > bw_max2(data->blackout_recovery_time, > bw_div((bw_add(bw_mul(bw_div(bw_mul(data->display_bandwidth[k], > data->useful_bytes_per_request[k]), data->bytes_per_request[k]), > vbios->blackout_duration), > bw_sub(bw_div(bw_mul(bw_mul(bw_mul((bw_add(bw_add(bw_mul(bw_int_to_fixed(2), > vbios->mcifwrmc_urgent_latency), > data->dmif_burst_time[data->y_clk_level][data->sclk_level]), > data->mcifwr_burst_time[data->y_clk_level][data->sclk_level])), > data->dispclk), bw_int_to_fixed(data->bytes_per_pixel[k])), > data->lines_interleaved_in_mem_access[k]), data->latency_hiding_lines[k]), > data->adjusted_data_buffer_size[k]))), > (bw_sub(bw_div(bw_mul(bw_mul(data->dispclk, > bw_int_to_fixed(data->bytes_per_pixel[k])), > data->lines_interleaved_in_mem_access[k]), data->latency_hiding_lines[k]), > bw_div(bw_mul(data->display_bandwidth[k], data->useful_bytes_per_request[k]), > data->bytes_per_request[k]))))); Uhg, can this be wrapped/split up somehow? Seeing the changes is nigh impossible and I suspect it might even be impossible for you to check internally. A nice comment above giving the formula and maybe some reasoning would also be nice, that way somebody from the outside has at least a chance to check if the code does what it's supposed to do. Cheers, Kai > } > } > } > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c > b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c > index bd53d27e5414..f552b0468186 100644 > --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c > @@ -1834,11 +1834,10 @@ void resource_build_info_frame(struct pipe_ctx > *pipe_ctx) > set_vendor_info_packet( > pipe_ctx->stream, &info_frame.vendor_info_packet); > set_spd_info_packet(pipe_ctx->stream, &info_frame.spd_packet); > - } > - > - else if (dc_is_dp_signal(signal)) > + } else if (dc_is_dp_signal(signal)) { > set_vsc_info_packet(pipe_ctx->stream, &info_frame.vsc_packet); > set_spd_info_packet(pipe_ctx->stream, &info_frame.spd_packet); > + } > > translate_info_frame(&info_frame, > &pipe_ctx->encoder_info_frame); > diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c > b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c > index 80ac5d9efa71..3d1c32122d69 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c > +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c > @@ -465,7 +465,6 @@ static uint32_t dce110_get_pix_clk_dividers_helper ( > struct pll_settings *pll_settings, > struct pixel_clk_params *pix_clk_params) > { > - uint32_t addr = 0; > uint32_t value = 0; > uint32_t field = 0; > uint32_t pll_calc_error = MAX_PLL_CALC_ERROR; > @@ -731,8 +730,6 @@ static void dce110_program_pixel_clk_resync( > enum signal_type signal_type, > enum dc_color_depth colordepth) > { > - uint32_t value = 0; > - > REG_UPDATE(RESYNC_CNTL, > DCCG_DEEP_COLOR_CNTL1, 0); > /* > @@ -772,8 +769,6 @@ static void dce112_program_pixel_clk_resync( > enum dc_color_depth colordepth, > bool enable_ycbcr420) > { > - uint32_t value = 0; > - > REG_UPDATE(PIXCLK_RESYNC_CNTL, > PHYPLLA_DCCG_DEEP_COLOR_CNTL, 0); > /* > 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 c2bd8dc7b4ad..262612061c68 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clocks.c > +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clocks.c > @@ -148,29 +148,6 @@ static int dce_divider_range_calc_divider( > > } > > -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, > @@ -189,25 +166,7 @@ static int dce_divider_range_get_divider( > 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) > +static int dce_clocks_get_dp_ref_freq(struct display_clock *clk) > { > struct dce_disp_clk *clk_dce = TO_DCE_CLOCKS(clk); > int dprefclk_wdivider; > diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c > b/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c > index f47b6617f662..bbf4d97cb980 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c > +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_transform.c > @@ -83,8 +83,6 @@ static bool setup_scaling_configuration( > struct dce_transform *xfm_dce, > const struct scaler_data *data) > { > - struct dc_context *ctx = xfm_dce->base.ctx; > - > if (data->taps.h_taps + data->taps.v_taps <= 2) { > /* Set bypass */ > REG_UPDATE_2(SCL_MODE, SCL_MODE, 0, SCL_PSCL_EN, 0); > diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_transform_v.c > b/drivers/gpu/drm/amd/display/dc/dce110/dce110_transform_v.c > index 7d8cf7a58f46..feb5f3c29804 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_transform_v.c > +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_transform_v.c > @@ -621,7 +621,8 @@ static void dce110_xfmv_set_pixel_storage_depth( > const struct bit_depth_reduction_params *bit_depth_params) > { > struct dce_transform *xfm_dce = TO_DCE_TRANSFORM(xfm); > - int pixel_depth, expan_mode; > + int pixel_depth = 0; > + int expan_mode = 0; > uint32_t reg_data = 0; > > switch (depth) { > _______________________________________________ amd-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/amd-gfx
