Re: [PATCH v3 1/4] drm/amd/display: Introduce FPU directory inside DC

Am 20.07.21 um 02:49 schrieb Rodrigo Siqueira:

The display core files rely on FPU, which requires to be compiled with special flags. Ideally, we don't want these FPU operations spread around the DC code; nevertheless, it happens in the current source. This commit introduces a new directory named fpu_operations that intends to centralize all files that require the FPU compilation flag. As part of this new component, this patch also moves one of the functions (dcn20_populate_dml_writeback_from_context) that require FPU access to a single shared file. Notice that this is the first part of the work, and it does not fix the FPU issue yet; we still need other patches for achieving the complete FPU isolation.

Changes since V2:

• Christian: Remove unnecessary wrapper.
• lkp: Add missing prototype.
• Only compile the FPU operations if the DCN option is enabled.

Change since V1:

• Update documentation and rebase.

Cc: Harry Wentland harry.wentland@amd.com Cc: Anson Jacob Anson.Jacob@amd.com Cc: Christian König christian.koenig@amd.com Cc: Hersen Wu hersenxs.wu@amd.com Cc: Aric Cyr aric.cyr@amd.com Reported-by: kernel test robot lkp@intel.com Signed-off-by: Rodrigo Siqueira Rodrigo.Siqueira@amd.com

---

drivers/gpu/drm/amd/display/dc/Makefile | 1 + .../drm/amd/display/dc/dcn20/dcn20_resource.c | 39 +-------- .../drm/amd/display/dc/dcn20/dcn20_resource.h | 2 - .../drm/amd/display/dc/dcn21/dcn21_resource.c | 2 + .../amd/display/dc/fpu_operations/Makefile | 58 +++++++++++++ .../drm/amd/display/dc/fpu_operations/dcn2x.c | 84 +++++++++++++++++++ .../drm/amd/display/dc/fpu_operations/dcn2x.h | 33 ++++++++ 7 files changed, 180 insertions(+), 39 deletions(-) create mode 100644 drivers/gpu/drm/amd/display/dc/fpu_operations/Makefile create mode 100644 drivers/gpu/drm/amd/display/dc/fpu_operations/dcn2x.c create mode 100644 drivers/gpu/drm/amd/display/dc/fpu_operations/dcn2x.h

diff --git a/drivers/gpu/drm/amd/display/dc/Makefile b/drivers/gpu/drm/amd/display/dc/Makefile index 943fcb164876..0de4baefa91e 100644 --- a/drivers/gpu/drm/amd/display/dc/Makefile +++ b/drivers/gpu/drm/amd/display/dc/Makefile @@ -35,6 +35,7 @@ DC_LIBS += dcn301 DC_LIBS += dcn302 DC_LIBS += dcn303 DC_LIBS += dcn31 +DC_LIBS += fpu_operations endif

DC_LIBS += dce120 diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c index 1b05a37b674d..f99b09643a52 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c @@ -35,6 +35,8 @@ #include "include/irq_service_interface.h" #include "dcn20/dcn20_resource.h"

+#include "fpu_operations/dcn2x.h"

• #include "dcn10/dcn10_hubp.h" #include "dcn10/dcn10_ipp.h" #include "dcn20_hubbub.h"

@@ -1974,43 +1976,6 @@ void dcn20_split_stream_for_mpc( ASSERT(primary_pipe->plane_state); }

-void dcn20_populate_dml_writeback_from_context(

• struct dc *dc, struct resource_context *res_ctx, display_e2e_pipe_params_st *pipes)
  

BTW: This is not good coding style.

Maybe use a different editor or re-read the coding style document.

-{

• int pipe_cnt, i;
• for (i = 0, pipe_cnt = 0; i < dc->res_pool->pipe_count; i++) {

• struct dc_writeback_info *wb_info = &res_ctx->pipe_ctx[i].stream->writeback_info[0];
  

• if (!res_ctx->pipe_ctx[i].stream)
  

• 	continue;
  

• /* Set writeback information */
  

• pipes[pipe_cnt].dout.wb_enable = (wb_info->wb_enabled == true) ? 1 : 0;
  

• pipes[pipe_cnt].dout.num_active_wb++;
  

• pipes[pipe_cnt].dout.wb.wb_src_height = wb_info->dwb_params.cnv_params.crop_height;
  

• pipes[pipe_cnt].dout.wb.wb_src_width = wb_info->dwb_params.cnv_params.crop_width;
  

• pipes[pipe_cnt].dout.wb.wb_dst_width = wb_info->dwb_params.dest_width;
  

• pipes[pipe_cnt].dout.wb.wb_dst_height = wb_info->dwb_params.dest_height;
  

• pipes[pipe_cnt].dout.wb.wb_htaps_luma = 1;
  

• pipes[pipe_cnt].dout.wb.wb_vtaps_luma = 1;
  

• pipes[pipe_cnt].dout.wb.wb_htaps_chroma = wb_info->dwb_params.scaler_taps.h_taps_c;
  

• pipes[pipe_cnt].dout.wb.wb_vtaps_chroma = wb_info->dwb_params.scaler_taps.v_taps_c;
  

• pipes[pipe_cnt].dout.wb.wb_hratio = 1.0;
  

• pipes[pipe_cnt].dout.wb.wb_vratio = 1.0;
  

• if (wb_info->dwb_params.out_format == dwb_scaler_mode_yuv420) {
  

• 	if (wb_info->dwb_params.output_depth == DWB_OUTPUT_PIXEL_DEPTH_8BPC)
  

• 		pipes[pipe_cnt].dout.wb.wb_pixel_format = dm_420_8;
  

• 	else
  

• 		pipes[pipe_cnt].dout.wb.wb_pixel_format = dm_420_10;
  

• } else
  

• 	pipes[pipe_cnt].dout.wb.wb_pixel_format = dm_444_32;
  

Ideally you would only have this chunk in the FPU specific directory and not the outer loop around it.

But it's at least a start, feel free to add an Reviewed-by: Christian König christian.koenig@amd.com to the patch and commit it.

Thanks, Christian.

• pipe_cnt++;
  

• }

-}

• int dcn20_populate_dml_pipes_from_context( struct dc *dc, struct dc_state *context,

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h index c8f3127bbcdf..6ec8ff45f0f7 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h @@ -58,8 +58,6 @@ struct pipe_ctx *dcn20_acquire_idle_pipe_for_layer( struct dc_state *state, const struct resource_pool *pool, struct dc_stream_state *stream); -void dcn20_populate_dml_writeback_from_context(

• struct dc *dc, struct resource_context *res_ctx, display_e2e_pipe_params_st *pipes);
  
  

  struct stream_encoder *dcn20_stream_encoder_create( enum engine_id eng_id,

diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c index f3d98e3ba624..85385144e2c5 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c @@ -35,6 +35,8 @@ #include "include/irq_service_interface.h" #include "dcn20/dcn20_resource.h"

+#include "fpu_operations/dcn2x.h"

• #include "clk_mgr.h" #include "dcn10/dcn10_hubp.h" #include "dcn10/dcn10_ipp.h"

diff --git a/drivers/gpu/drm/amd/display/dc/fpu_operations/Makefile b/drivers/gpu/drm/amd/display/dc/fpu_operations/Makefile new file mode 100644 index 000000000000..7f6f90c3f267 --- /dev/null +++ b/drivers/gpu/drm/amd/display/dc/fpu_operations/Makefile @@ -0,0 +1,58 @@ +# SPDX-License-Identifier: MIT +# +# Copyright 2021 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. +# +# +# Makefile for fpu operations component. +#

+FPU_OPERATIONS = dcn2x.o

+ifdef CONFIG_X86 +fpu_ccflags := -mhard-float -msse +endif

+ifdef CONFIG_PPC64 +fpu_ccflags := -mhard-float -maltivec +endif

+ifdef CONFIG_CC_IS_GCC +ifeq ($(call cc-ifversion, -lt, 0701, y), y) +IS_OLD_GCC = 1 +endif +endif

+ifdef CONFIG_X86 +ifdef IS_OLD_GCC +# Stack alignment mismatch, proceed with caution. +# GCC < 7.1 cannot compile code using `double` and -mpreferred-stack-boundary=3 +# (8B stack alignment). +fpu_ccflags := -mpreferred-stack-boundary=4 +else +fpu_ccflags := -msse2 +endif +endif

+CFLAGS_$(AMDDALPATH)/dc/fpu_operations/dcn2x.o += $(fpu_ccflags)

+AMD_DAL_FPU_OPERATIONS = $(addprefix $(AMDDALPATH)/dc/fpu_operations/,$(FPU_OPERATIONS))

+AMD_DISPLAY_FILES += $(AMD_DAL_FPU_OPERATIONS) diff --git a/drivers/gpu/drm/amd/display/dc/fpu_operations/dcn2x.c b/drivers/gpu/drm/amd/display/dc/fpu_operations/dcn2x.c new file mode 100644 index 000000000000..95a0b89302a9 --- /dev/null +++ b/drivers/gpu/drm/amd/display/dc/fpu_operations/dcn2x.c @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: MIT +/*

• • Copyright 2021 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 "resource.h"

+#include "fpu_operations/dcn2x.h"

+/**

• • DOC: DCN2x FPU manipulation Overview
• • The DCN architecture relies on FPU operations, which require special
• • compilation flags and the use of kernel_fpu_begin/end functions; ideally, we
• • want to avoid spreading FPU access across multiple files. With this idea in
• • mind, this file aims to centralize all DCN20 and DCN2.1 (DCN2x) functions
• • that require FPU access in a single place. Code in this file follows the
• • following code pattern:
• • 1. Functions that use FPU operations should be isolated in static functions.
• • 2. The FPU functions should have the noinline attribute to ensure anything
• • that deals with FP register is contained within this call.
• • 3. All function that needs to be accessed outside this file requires a
• • public interface that not uses any FPU reference.
• */

+void dcn20_populate_dml_writeback_from_context(struct dc *dc,

• 			       struct resource_context *res_ctx,
  

• 			       display_e2e_pipe_params_st *pipes)
  

+{

• int pipe_cnt, i;
• for (i = 0, pipe_cnt = 0; i < dc->res_pool->pipe_count; i++) {

• struct dc_writeback_info *wb_info = &res_ctx->pipe_ctx[i].stream->writeback_info[0];
  

• if (!res_ctx->pipe_ctx[i].stream)
  

• 	continue;
  

• /* Set writeback information */
  

• pipes[pipe_cnt].dout.wb_enable = (wb_info->wb_enabled == true) ? 1 : 0;
  

• pipes[pipe_cnt].dout.num_active_wb++;
  

• pipes[pipe_cnt].dout.wb.wb_src_height = wb_info->dwb_params.cnv_params.crop_height;
  

• pipes[pipe_cnt].dout.wb.wb_src_width = wb_info->dwb_params.cnv_params.crop_width;
  

• pipes[pipe_cnt].dout.wb.wb_dst_width = wb_info->dwb_params.dest_width;
  

• pipes[pipe_cnt].dout.wb.wb_dst_height = wb_info->dwb_params.dest_height;
  

• pipes[pipe_cnt].dout.wb.wb_htaps_luma = 1;
  

• pipes[pipe_cnt].dout.wb.wb_vtaps_luma = 1;
  

• pipes[pipe_cnt].dout.wb.wb_htaps_chroma = wb_info->dwb_params.scaler_taps.h_taps_c;
  

• pipes[pipe_cnt].dout.wb.wb_vtaps_chroma = wb_info->dwb_params.scaler_taps.v_taps_c;
  

• pipes[pipe_cnt].dout.wb.wb_hratio = 1.0;
  

• pipes[pipe_cnt].dout.wb.wb_vratio = 1.0;
  

• if (wb_info->dwb_params.out_format == dwb_scaler_mode_yuv420) {
  

• 	if (wb_info->dwb_params.output_depth == DWB_OUTPUT_PIXEL_DEPTH_8BPC)
  

• 		pipes[pipe_cnt].dout.wb.wb_pixel_format = dm_420_8;
  

• 	else
  

• 		pipes[pipe_cnt].dout.wb.wb_pixel_format = dm_420_10;
  

• } else {
  

• 	pipes[pipe_cnt].dout.wb.wb_pixel_format = dm_444_32;
  

• }
  

• pipe_cnt++;
  

• }

+} diff --git a/drivers/gpu/drm/amd/display/dc/fpu_operations/dcn2x.h b/drivers/gpu/drm/amd/display/dc/fpu_operations/dcn2x.h new file mode 100644 index 000000000000..c060f909164b --- /dev/null +++ b/drivers/gpu/drm/amd/display/dc/fpu_operations/dcn2x.h @@ -0,0 +1,33 @@ +/* SPDX-License-Identifier: MIT */ +/*

• • Copyright 2021 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 __DCN2X_H__ +#define __DCN2X_H__

+void dcn20_populate_dml_writeback_from_context(struct dc *dc,

• struct resource_context *res_ctx, display_e2e_pipe_params_st *pipes);

+#endif /* __DCN2X_H__ */