Re: [PATCH] drm/amd/display: fix incompatible structure layouts
On Fri, Feb 2, 2018 at 4:39 PM, Harry Wentland wrote: > On 2018-02-02 07:31 AM, Arnd Bergmann wrote: >> Building the amd display driver with link-time optimizations revealed a bug > > Curious how I'd go about building with link-time optimizations. I got the idea from last week's LWN article on the topic, see https://lwn.net/Articles/744507/. I needed the latest gcc version to avoid some compiler bugs, and a few dozen kernel patches on top to get a clean build in random configurations (posted them all today). Most normal configurations probably work out of the box, but I have not actually tried running any ;-) Arnd ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: fix incompatible structure layouts
On 2018-02-02 11:02 AM, Arnd Bergmann wrote: > On Fri, Feb 2, 2018 at 4:39 PM, Harry Wentland wrote: >> On 2018-02-02 07:31 AM, Arnd Bergmann wrote: >>> Building the amd display driver with link-time optimizations revealed a bug >> >> Curious how I'd go about building with link-time optimizations. > > I got the idea from last week's LWN article on the topic, see > https://lwn.net/Articles/744507/. I needed the latest gcc version to > avoid some compiler bugs, and a few dozen kernel patches on top > to get a clean build in random configurations (posted them all today). > > Most normal configurations probably work out of the box, but I have > not actually tried running any ;-) > Thanks. Learn something new every day. I like this bit from the article: > So it is basically just like if it concatenated all source files into a > single one and made everything static. Probably not a bad idea to think that way when doing kernel development. :) Harry >Arnd > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: fix incompatible structure layouts
On 2018-02-02 07:31 AM, Arnd Bergmann wrote: > Building the amd display driver with link-time optimizations revealed a bug Curious how I'd go about building with link-time optimizations. > that caused dal_cmd_tbl_helper_dce80_get_table() and > dal_cmd_tbl_helper_dce110_get_table() get called with an incompatible > return type between the two callers in command_table_helper.c and > command_table_helper2.c: > > drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce80/command_table_helper_dce80.h:31: > error: type of 'dal_cmd_tbl_helper_dce80_get_table' does not match original > declaration [-Werror=lto-type-mismatch] > const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table(void); > > drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce80/command_table_helper_dce80.c:351: > note: 'dal_cmd_tbl_helper_dce80_get_table' was previously declared here > const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table(void) > > drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce110/command_table_helper_dce110.h:32: > error: type of 'dal_cmd_tbl_helper_dce110_get_table' does not match original > declaration [-Werror=lto-type-mismatch] > const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table(void); > > drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce110/command_table_helper_dce110.c:361: > note: 'dal_cmd_tbl_helper_dce110_get_table' was previously declared here > const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table(void) > > The two versions of the structure are obviously derived from the same > one, but have diverged over time, before they got added to the kernel. > > This moves the structure to a new shared header file and uses the superset > of the members, to ensure the interfaces are all compatible. > > Fixes: ae79c310b1a6 ("drm/amd/display: Add DCE12 bios parser support") > Signed-off-by: Arnd Bergmann Thanks for the fix. Reviewed-by: Harry Wentland Harry > --- > .../drm/amd/display/dc/bios/command_table_helper.h | 33 +-- > .../amd/display/dc/bios/command_table_helper2.h| 30 +- > .../display/dc/bios/command_table_helper_struct.h | 66 > ++ > 3 files changed, 68 insertions(+), 61 deletions(-) > create mode 100644 > drivers/gpu/drm/amd/display/dc/bios/command_table_helper_struct.h > > diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h > b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h > index 1fab634b66be..4c3789df253d 100644 > --- a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h > +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h > @@ -29,38 +29,7 @@ > #include "dce80/command_table_helper_dce80.h" > #include "dce110/command_table_helper_dce110.h" > #include "dce112/command_table_helper_dce112.h" > - > -struct command_table_helper { > - bool (*controller_id_to_atom)(enum controller_id id, uint8_t *atom_id); > - uint8_t (*encoder_action_to_atom)( > - enum bp_encoder_control_action action); > - uint32_t (*encoder_mode_bp_to_atom)(enum signal_type s, > - bool enable_dp_audio); > - bool (*engine_bp_to_atom)(enum engine_id engine_id, > - uint32_t *atom_engine_id); > - void (*assign_control_parameter)( > - const struct command_table_helper *h, > - struct bp_encoder_control *control, > - DIG_ENCODER_CONTROL_PARAMETERS_V2 *ctrl_param); > - bool (*clock_source_id_to_atom)(enum clock_source_id id, > - uint32_t *atom_pll_id); > - bool (*clock_source_id_to_ref_clk_src)( > - enum clock_source_id id, > - uint32_t *ref_clk_src_id); > - uint8_t (*transmitter_bp_to_atom)(enum transmitter t); > - uint8_t (*encoder_id_to_atom)(enum encoder_id id); > - uint8_t (*clock_source_id_to_atom_phy_clk_src_id)( > - enum clock_source_id id); > - uint8_t (*signal_type_to_atom_dig_mode)(enum signal_type s); > - uint8_t (*hpd_sel_to_atom)(enum hpd_source_id id); > - uint8_t (*dig_encoder_sel_to_atom)(enum engine_id engine_id); > - uint8_t (*phy_id_to_atom)(enum transmitter t); > - uint8_t (*disp_power_gating_action_to_atom)( > - enum bp_pipe_control_action action); > - bool (*dc_clock_type_to_atom)(enum bp_dce_clock_type id, > - uint32_t *atom_clock_type); > -uint8_t (*transmitter_color_depth_to_atom)(enum transmitter_color_depth > id); > -}; > +#include "command_table_helper_struct.h" > > bool dal_bios_parser_init_cmd_tbl_helper(const struct command_table_helper > **h, > enum dce_version dce); > diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h > b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h > index 9f587c91d843..785fcb20a1b9 100644 > --- a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h > +++ b/drivers/
[PATCH] drm/amd/display: fix incompatible structure layouts
Building the amd display driver with link-time optimizations revealed a bug that caused dal_cmd_tbl_helper_dce80_get_table() and dal_cmd_tbl_helper_dce110_get_table() get called with an incompatible return type between the two callers in command_table_helper.c and command_table_helper2.c: drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce80/command_table_helper_dce80.h:31: error: type of 'dal_cmd_tbl_helper_dce80_get_table' does not match original declaration [-Werror=lto-type-mismatch] const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table(void); drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce80/command_table_helper_dce80.c:351: note: 'dal_cmd_tbl_helper_dce80_get_table' was previously declared here const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table(void) drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce110/command_table_helper_dce110.h:32: error: type of 'dal_cmd_tbl_helper_dce110_get_table' does not match original declaration [-Werror=lto-type-mismatch] const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table(void); drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce110/command_table_helper_dce110.c:361: note: 'dal_cmd_tbl_helper_dce110_get_table' was previously declared here const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table(void) The two versions of the structure are obviously derived from the same one, but have diverged over time, before they got added to the kernel. This moves the structure to a new shared header file and uses the superset of the members, to ensure the interfaces are all compatible. Fixes: ae79c310b1a6 ("drm/amd/display: Add DCE12 bios parser support") Signed-off-by: Arnd Bergmann --- .../drm/amd/display/dc/bios/command_table_helper.h | 33 +-- .../amd/display/dc/bios/command_table_helper2.h| 30 +- .../display/dc/bios/command_table_helper_struct.h | 66 ++ 3 files changed, 68 insertions(+), 61 deletions(-) create mode 100644 drivers/gpu/drm/amd/display/dc/bios/command_table_helper_struct.h diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h index 1fab634b66be..4c3789df253d 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper.h @@ -29,38 +29,7 @@ #include "dce80/command_table_helper_dce80.h" #include "dce110/command_table_helper_dce110.h" #include "dce112/command_table_helper_dce112.h" - -struct command_table_helper { - bool (*controller_id_to_atom)(enum controller_id id, uint8_t *atom_id); - uint8_t (*encoder_action_to_atom)( - enum bp_encoder_control_action action); - uint32_t (*encoder_mode_bp_to_atom)(enum signal_type s, - bool enable_dp_audio); - bool (*engine_bp_to_atom)(enum engine_id engine_id, - uint32_t *atom_engine_id); - void (*assign_control_parameter)( - const struct command_table_helper *h, - struct bp_encoder_control *control, - DIG_ENCODER_CONTROL_PARAMETERS_V2 *ctrl_param); - bool (*clock_source_id_to_atom)(enum clock_source_id id, - uint32_t *atom_pll_id); - bool (*clock_source_id_to_ref_clk_src)( - enum clock_source_id id, - uint32_t *ref_clk_src_id); - uint8_t (*transmitter_bp_to_atom)(enum transmitter t); - uint8_t (*encoder_id_to_atom)(enum encoder_id id); - uint8_t (*clock_source_id_to_atom_phy_clk_src_id)( - enum clock_source_id id); - uint8_t (*signal_type_to_atom_dig_mode)(enum signal_type s); - uint8_t (*hpd_sel_to_atom)(enum hpd_source_id id); - uint8_t (*dig_encoder_sel_to_atom)(enum engine_id engine_id); - uint8_t (*phy_id_to_atom)(enum transmitter t); - uint8_t (*disp_power_gating_action_to_atom)( - enum bp_pipe_control_action action); - bool (*dc_clock_type_to_atom)(enum bp_dce_clock_type id, - uint32_t *atom_clock_type); -uint8_t (*transmitter_color_depth_to_atom)(enum transmitter_color_depth id); -}; +#include "command_table_helper_struct.h" bool dal_bios_parser_init_cmd_tbl_helper(const struct command_table_helper **h, enum dce_version dce); diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h index 9f587c91d843..785fcb20a1b9 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.h @@ -29,35 +29,7 @@ #include "dce80/command_table_helper_dce80.h" #include "dce110/command_table_helper_dce110.h" #include "dce112/command_table_helper2_dce112.h" - -struct command_table_helper { - bool (*controller_id_to_atom)(enum control