2021-01-08 21:02:33

by Alex Deucher

[permalink] [raw]

Subject: Re: [PATCH 19/40] drm/amd/display/dc/bios/command_table: Remove unused variable

On Fri, Jan 8, 2021 at 3:15 PM Lee Jones <[email protected]> wrote:
>
> None of the surrounding code was removed just in case even a small
> fraction of it was functional.
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/command_table.c: In function ‘adjust_display_pll_v2’:
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/command_table.c:1459:35: warning: variable ‘params’ set but not used [-Wunused-but-set-variable]
>
> Cc: Harry Wentland <[email protected]>
> Cc: Leo Li <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: "Christian König" <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Qinglang Miao <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/gpu/drm/amd/display/dc/bios/command_table.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table.c b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> index 070459e3e4070..dd893a1176979 100644
> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table.c
> @@ -1456,20 +1456,14 @@ static enum bp_result adjust_display_pll_v2(
> struct bp_adjust_pixel_clock_parameters *bp_params)
> {

@Harry Wentland, this function looks like it's missing the call to
EXEC_BIOS_CMD_TABLE(). I just sent a patch to fix that up, although
I'm not sure if this function every gets used on any asics supported
by amdgpu, so maybe we can just drop it.

Alex

> enum bp_result result = BP_RESULT_FAILURE;
> - ADJUST_DISPLAY_PLL_PS_ALLOCATION params = { 0 };
>
> /* We need to convert from KHz units into 10KHz units and then convert
> * output pixel clock back 10KHz-->KHz */
> uint32_t pixel_clock_10KHz_in = bp_params->pixel_clock / 10;
>
> - params.usPixelClock = cpu_to_le16((uint16_t)(pixel_clock_10KHz_in));
> - params.ucTransmitterID =
> - bp->cmd_helper->encoder_id_to_atom(
> - dal_graphics_object_id_get_encoder_id(
> - bp_params->encoder_object_id));
> - params.ucEncodeMode =
> - (uint8_t)bp->cmd_helper->encoder_mode_bp_to_atom(
> - bp_params->signal_type, false);
> + bp->cmd_helper->encoder_id_to_atom(
> + dal_graphics_object_id_get_encoder_id(bp_params->encoder_object_id));
> + bp->cmd_helper->encoder_mode_bp_to_atom(bp_params->signal_type, false);
> return result;
> }
>
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

2021-01-08 21:09:17

by Alex Deucher

[permalink] [raw]

Subject: Re: [PATCH 21/40] drm/amd/display/dc/calcs/dce_calcs: Remove unused variables 'v_filter_init_mode' and 'sclk_lvl'

On Fri, Jan 8, 2021 at 3:15 PM Lee Jones <[email protected]> wrote:
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c: In function ‘calculate_bandwidth’:
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:109:18: warning: variable ‘v_filter_init_mode’ set but not used [-Wunused-but-set-variable]
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c: In function ‘bw_calcs’:
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3031:21: warning: variable ‘sclk_lvl’ set but not used [-Wunused-but-set-variable]
>
> Cc: Harry Wentland <[email protected]>
> Cc: Leo Li <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: "Christian König" <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>

@Harry Wentland
, @Leo (Sunpeng) Li I think this file is generated from a script or
something. Wanted to get your take on how to properly integrate a
change like this.

Alex

> ---
> drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
> index ef41b287cbe23..158d927c03e55 100644
> --- a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
> +++ b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
> @@ -106,7 +106,6 @@ static void calculate_bandwidth(
> bool lpt_enabled;
> enum bw_defines sclk_message;
> enum bw_defines yclk_message;
> - enum bw_defines v_filter_init_mode[maximum_number_of_surfaces];
> enum bw_defines tiling_mode[maximum_number_of_surfaces];
> enum bw_defines surface_type[maximum_number_of_surfaces];
> enum bw_defines voltage;
> @@ -792,12 +791,8 @@ static void calculate_bandwidth(
> data->v_filter_init[i] = bw_add(data->v_filter_init[i], bw_int_to_fixed(1));
> }
> if (data->stereo_mode[i] == bw_def_top_bottom) {
> - v_filter_init_mode[i] = bw_def_manual;
> data->v_filter_init[i] = bw_min2(data->v_filter_init[i], bw_int_to_fixed(4));
> }
> - else {
> - v_filter_init_mode[i] = bw_def_auto;
> - }
> if (data->stereo_mode[i] == bw_def_top_bottom) {
> data->num_lines_at_frame_start = bw_int_to_fixed(1);
> }
> @@ -3028,7 +3023,7 @@ bool bw_calcs(struct dc_context *ctx,
> calcs_output->all_displays_in_sync = false;
>
> if (data->number_of_displays != 0) {
> - uint8_t yclk_lvl, sclk_lvl;
> + uint8_t yclk_lvl;
> struct bw_fixed high_sclk = vbios->high_sclk;
> struct bw_fixed mid1_sclk = vbios->mid1_sclk;
> struct bw_fixed mid2_sclk = vbios->mid2_sclk;
> @@ -3049,7 +3044,6 @@ bool bw_calcs(struct dc_context *ctx,
> calculate_bandwidth(dceip, vbios, data);
>
> yclk_lvl = data->y_clk_level;
> - sclk_lvl = data->sclk_level;
>
> calcs_output->nbp_state_change_enable =
> data->nbp_state_change_enable;
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

2021-01-08 21:10:44

by Alex Deucher

[permalink] [raw]

Subject: Re: [PATCH 22/40] drm/amd/display/dc/bios/bios_parser2: Fix some formatting issues and missing parameter docs

On Fri, Jan 8, 2021 at 3:15 PM Lee Jones <[email protected]> wrote:
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/bios_parser2.c:501: warning: Function parameter or member 'dcb' not described in 'bios_parser_get_gpio_pin_info'
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/bios_parser2.c:501: warning: Function parameter or member 'gpio_id' not described in 'bios_parser_get_gpio_pin_info'
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/bios_parser2.c:501: warning: Function parameter or member 'info' not described in 'bios_parser_get_gpio_pin_info'
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/bios_parser2.c:815: warning: Function parameter or member 'dcb' not described in 'bios_parser_get_spread_spectrum_info'
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/bios_parser2.c:815: warning: Function parameter or member 'signal' not described in 'bios_parser_get_spread_spectrum_info'
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/bios_parser2.c:815: warning: Function parameter or member 'index' not described in 'bios_parser_get_spread_spectrum_info'
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/bios_parser2.c:815: warning: Function parameter or member 'ss_info' not described in 'bios_parser_get_spread_spectrum_info'
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/bios_parser2.c:1210: warning: Function parameter or member 'dcb' not described in 'bios_parser_set_scratch_critical_state'
> drivers/gpu/drm/amd/amdgpu/../display/dc/bios/bios_parser2.c:1210: warning: Function parameter or member 'state' not described in 'bios_parser_set_scratch_critical_state'
>
> Cc: Harry Wentland <[email protected]>
> Cc: Leo Li <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: "Christian König" <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>

Applied. Thanks!

Alex

> ---
> .../drm/amd/display/dc/bios/bios_parser2.c | 29 +++++++++----------
> 1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
> index 670c265838178..9f9fda3118d1f 100644
> --- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
> +++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
> @@ -485,10 +485,11 @@ static struct atom_hpd_int_record *get_hpd_record(
> * bios_parser_get_gpio_pin_info
> * Get GpioPin information of input gpio id
> *
> - * @param gpio_id, GPIO ID
> - * @param info, GpioPin information structure
> - * @return Bios parser result code
> - * @note
> + * @dcb: pointer to the DC BIOS
> + * @gpio_id: GPIO ID
> + * @info: GpioPin information structure
> + * return: Bios parser result code
> + * note:
> * to get the GPIO PIN INFO, we need:
> * 1. get the GPIO_ID from other object table, see GetHPDInfo()
> * 2. in DATA_TABLE.GPIO_Pin_LUT, search all records,
> @@ -801,11 +802,11 @@ static enum bp_result get_ss_info_v4_2(
> * ver 3.1,
> * there is only one entry for each signal /ss id. However, there is
> * no planning of supporting multiple spread Sprectum entry for EverGreen
> - * @param [in] this
> - * @param [in] signal, ASSignalType to be converted to info index
> - * @param [in] index, number of entries that match the converted info index
> - * @param [out] ss_info, sprectrum information structure,
> - * @return Bios parser result code
> + * @dcb: pointer to the DC BIOS
> + * @signal: ASSignalType to be converted to info index
> + * @index: number of entries that match the converted info index
> + * @ss_info: sprectrum information structure,
> + * return: Bios parser result code
> */
> static enum bp_result bios_parser_get_spread_spectrum_info(
> struct dc_bios *dcb,
> @@ -1196,13 +1197,11 @@ static bool bios_parser_is_accelerated_mode(
> }
>
> /**
> - * bios_parser_set_scratch_critical_state
> + * bios_parser_set_scratch_critical_state - update critical state bit
> + * in VBIOS scratch register
> *
> - * @brief
> - * update critical state bit in VBIOS scratch register
> - *
> - * @param
> - * bool - to set or reset state
> + * @dcb: pointer to the DC BIO
> + * @state: set or reset state
> */
> static void bios_parser_set_scratch_critical_state(
> struct dc_bios *dcb,
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

2021-01-08 21:11:50

by Alex Deucher

[permalink] [raw]

Subject: Re: [PATCH 23/40] drm/amd/display/dc/dce/dce_audio: Make function invoked by reference static

On Fri, Jan 8, 2021 at 3:15 PM Lee Jones <[email protected]> wrote:
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_audio.c:871:6: warning: no previous prototype for ‘dce60_aud_wall_dto_setup’ [-Wmissing-prototypes]
>
> Cc: Harry Wentland <[email protected]>
> Cc: Leo Li <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: "Christian König" <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Mauro Rossi <[email protected]>
> Cc: Charlene Liu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>

Applied. Thanks!

Alex

> ---
> drivers/gpu/drm/amd/display/dc/dce/dce_audio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
> index 2a2a0fdb92539..7866cf2a668fa 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
> @@ -868,7 +868,7 @@ void dce_aud_wall_dto_setup(
> }
>
> #if defined(CONFIG_DRM_AMD_DC_SI)
> -void dce60_aud_wall_dto_setup(
> +static void dce60_aud_wall_dto_setup(
> struct audio *audio,
> enum signal_type signal,
> const struct audio_crtc_info *crtc_info,
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

2021-01-08 21:16:02

by Alex Deucher

[permalink] [raw]

Applied. Thanks!

Alex

On Fri, Jan 8, 2021 at 3:15 PM Lee Jones <[email protected]> wrote:
>
> Fixes the following W=1 kernel build warning(s):
>
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c: In function ‘calculate_bandwidth’:
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:109:18: warning: variable ‘v_filter_init_mode’ set but not used [-Wunused-but-set-variable]
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c: In function ‘bw_calcs’:
> drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dce_calcs.c:3031:21: warning: variable ‘sclk_lvl’ set but not used [-Wunused-but-set-variable]
>
> Cc: Harry Wentland <[email protected]>
> Cc: Leo Li <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: "Christian König" <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
> index ef41b287cbe23..158d927c03e55 100644
> --- a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
> +++ b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c
> @@ -106,7 +106,6 @@ static void calculate_bandwidth(
> bool lpt_enabled;
> enum bw_defines sclk_message;
> enum bw_defines yclk_message;
> - enum bw_defines v_filter_init_mode[maximum_number_of_surfaces];
> enum bw_defines tiling_mode[maximum_number_of_surfaces];
> enum bw_defines surface_type[maximum_number_of_surfaces];
> enum bw_defines voltage;
> @@ -792,12 +791,8 @@ static void calculate_bandwidth(
> data->v_filter_init[i] = bw_add(data->v_filter_init[i], bw_int_to_fixed(1));
> }
> if (data->stereo_mode[i] == bw_def_top_bottom) {
> - v_filter_init_mode[i] = bw_def_manual;
> data->v_filter_init[i] = bw_min2(data->v_filter_init[i], bw_int_to_fixed(4));
> }
> - else {
> - v_filter_init_mode[i] = bw_def_auto;
> - }
> if (data->stereo_mode[i] == bw_def_top_bottom) {
> data->num_lines_at_frame_start = bw_int_to_fixed(1);
> }
> @@ -3028,7 +3023,7 @@ bool bw_calcs(struct dc_context *ctx,
> calcs_output->all_displays_in_sync = false;
>
> if (data->number_of_displays != 0) {
> - uint8_t yclk_lvl, sclk_lvl;
> + uint8_t yclk_lvl;
> struct bw_fixed high_sclk = vbios->high_sclk;
> struct bw_fixed mid1_sclk = vbios->mid1_sclk;
> struct bw_fixed mid2_sclk = vbios->mid2_sclk;
> @@ -3049,7 +3044,6 @@ bool bw_calcs(struct dc_context *ctx,
> calculate_bandwidth(dceip, vbios, data);
>
> yclk_lvl = data->y_clk_level;
> - sclk_lvl = data->sclk_level;
>
> calcs_output->nbp_state_change_enable =
> data->nbp_state_change_enable;
> --
> 2.25.1
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel