[PATCH v3 1/4] drm/amd/display: Introduce FPU directory inside DC
Christian König
christian.koenig at amd.com
Tue Jul 20 08:09:31 UTC 2021
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 at amd.com>
> Cc: Anson Jacob <Anson.Jacob at amd.com>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Hersen Wu <hersenxs.wu at amd.com>
> Cc: Aric Cyr <aric.cyr at amd.com>
> Reported-by: kernel test robot <lkp at intel.com>
> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira at 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 at 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__ */
More information about the amd-gfx
mailing list