<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> </head> <body> <div>Hi Isabelle,</div> <div> </div> <div>Using the dml makefile for everything sounds better to me. </div> <div>Could you send the v3 version using your way to me?</div> <div> </div> <div>Regards,</div> <div>Jasdeep</div> <div> </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Isabella Basso <isabbasso@riseup.net> Sent: January 3, 2022 2:52 PM To: Dhillon, Jasdeep <Jasdeep.Dhillon@amd.com> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; daniel@ffwll.ch <daniel@ffwll.ch>; Wentland, Harry <Harry.Wentland@amd.com>; Siqueira, Rodrigo <Rodrigo.Siqueira@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Zhuo, Qingqing (Lillian) <Qingqing.Zhuo@amd.com>; mwen@igalia.com <mwen@igalia.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; ~lkcamp/patches@lists.sr.ht <~lkcamp/patches@lists.sr.ht> Subject: Re: [PATCH v2] drm/amd/display: move calcs folder into DML <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">Hello, Jasdeep, On 2021-12-23 17:44, Dhillon, Jasdeep wrote: > [AMD Official Use Only] > > Hi Isabbasso, > > The patch fails to compile when there is No DCN because the calc > object files fail to generate since dml depends on the > CONFIG_DRM_AMD_DC_DCN being enabled (Makefile inside dc folder): > > ifdef CONFIG_DRM_AMD_DC_DCN > DC_LIBS += dcn20 > DC_LIBS += dsc > DC_LIBS += dcn10 dml > DC_LIBS += dcn21 > DC_LIBS += dcn30 > DC_LIBS += dcn301 > DC_LIBS += dcn302 > DC_LIBS += dcn303 endif > > A few changes need to be made to the patch, which are: > > -The Makefile in dc needs the line: DC_LIBS+= dml/calcs > -the Makefile in the calcs folder that the patch deletes can be > placed inside of dc/dml/calcs instead of adding it to the Makefiles in > dc/dml > Siqueira had warned me of this issue just before you emailed me, so I'd already prepared a v3 with no calcs Makefile but actually using the dml one for everything, as it's pretty much set up for working with or without the DRM_AMD_DC_DCN config turned on. Would this be alright, or would you rather have it as you suggested? >From what I tested locally both work pretty much the same. I think my solution is a little harder to test but ends up being a little more compact, which might translate into being more maintainable (maybe?). Please let me know what you think :). Cheers, -- Isabella Basso > Could you revise your patch based on these changes. > > Regards, > Jasdeep > > ------------------------- > > From: isabbasso@riseup.net <isabbasso@riseup.net> > Sent: December 20, 2021 6:23 PM > To: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian > <Christian.Koenig@amd.com>; daniel@ffwll.ch <daniel@ffwll.ch>; > Wentland, Harry <Harry.Wentland@amd.com>; Siqueira, Rodrigo > <Rodrigo.Siqueira@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>; > Pan, Xinhui <Xinhui.Pan@amd.com>; Zhuo, Qingqing (Lillian) > <Qingqing.Zhuo@amd.com>; Dhillon, Jasdeep <Jasdeep.Dhillon@amd.com>; > mwen@igalia.com <mwen@igalia.com> > Cc: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; > ~lkcamp/patches@lists.sr.ht <~lkcamp/patches@lists.sr.ht> > Subject: Re: [PATCH v2] drm/amd/display: move calcs folder into DML > > On 2021-12-20 20:20, Isabella Basso wrote: >> The calcs folder has FPU code on it, which should be isolated inside > the >> DML folder as per > <a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F93042%2F&data=04%7C01%7CJasdeep.Dhillon%40amd.com%7C8a3d537d1fb64bac26c308d9cef28d37%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637768363894413584%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=rpKQjmK4atQn0U3RMCmkMTe9Yyj0dq4YDBhQeA48lyM%3D&reserved=0"> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F93042%2F&data=04%7C01%7CJasdeep.Dhillon%40amd.com%7C8a3d537d1fb64bac26c308d9cef28d37%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637768363894413584%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=rpKQjmK4atQn0U3RMCmkMTe9Yyj0dq4YDBhQeA48lyM%3D&reserved=0</a> > [1]. >> >> This commit aims single-handedly to correct the location of such FPU >> code and does not refactor any functions. >> >> Signed-off-by: Isabella Basso <isabbasso@riseup.net> >> --- >> drivers/gpu/drm/amd/display/dc/Makefile | 2 +- >> drivers/gpu/drm/amd/display/dc/calcs/Makefile | 68 > ------------------- >> drivers/gpu/drm/amd/display/dc/dml/Makefile | 13 +++- >> .../amd/display/dc/{ => dml}/calcs/bw_fixed.c | 0 >> .../display/dc/{ => dml}/calcs/calcs_logger.h | 0 >> .../display/dc/{ => dml}/calcs/custom_float.c | 0 >> .../display/dc/{ => dml}/calcs/dce_calcs.c | 0 >> .../dc/{ => dml}/calcs/dcn_calc_auto.c | 0 >> .../dc/{ => dml}/calcs/dcn_calc_auto.h | 0 >> .../dc/{ => dml}/calcs/dcn_calc_math.c | 0 >> .../display/dc/{ => dml}/calcs/dcn_calcs.c | 0 >> 11 files changed, 13 insertions(+), 70 deletions(-) >> delete mode 100644 drivers/gpu/drm/amd/display/dc/calcs/Makefile >> rename drivers/gpu/drm/amd/display/dc/{ => dml}/calcs/bw_fixed.c > (100%) >> rename drivers/gpu/drm/amd/display/dc/{ => > dml}/calcs/calcs_logger.h (100%) >> rename drivers/gpu/drm/amd/display/dc/{ => > dml}/calcs/custom_float.c (100%) >> rename drivers/gpu/drm/amd/display/dc/{ => dml}/calcs/dce_calcs.c > (100%) >> rename drivers/gpu/drm/amd/display/dc/{ => > dml}/calcs/dcn_calc_auto.c (100%) >> rename drivers/gpu/drm/amd/display/dc/{ => > dml}/calcs/dcn_calc_auto.h (100%) >> rename drivers/gpu/drm/amd/display/dc/{ => > dml}/calcs/dcn_calc_math.c (100%) >> rename drivers/gpu/drm/amd/display/dc/{ => dml}/calcs/dcn_calcs.c > (100%) >> >> diff --git a/drivers/gpu/drm/amd/display/dc/Makefile >> b/drivers/gpu/drm/amd/display/dc/Makefile >> index b1f0d6260226..1872adc96a00 100644 >> --- a/drivers/gpu/drm/amd/display/dc/Makefile >> +++ b/drivers/gpu/drm/amd/display/dc/Makefile >> @@ -23,7 +23,7 @@ >> # Makefile for Display Core (dc) component. >> # >> >> -DC_LIBS = basics bios calcs clk_mgr dce gpio irq virtual >> +DC_LIBS = basics bios clk_mgr dce gpio irq virtual >> >> ifdef CONFIG_DRM_AMD_DC_DCN >> DC_LIBS += dcn20 >> diff --git a/drivers/gpu/drm/amd/display/dc/calcs/Makefile >> b/drivers/gpu/drm/amd/display/dc/calcs/Makefile >> deleted file mode 100644 >> index f3c00f479e1c..000000000000 >> --- a/drivers/gpu/drm/amd/display/dc/calcs/Makefile >> +++ /dev/null >> @@ -1,68 +0,0 @@ >> -# >> -# Copyright 2017 Advanced Micro Devices, Inc. >> -# Copyright 2019 Raptor Engineering, LLC >> -# >> -# 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 the 'calcs' sub-component of DAL. >> -# It calculates Bandwidth and Watermarks values for HW programming >> -# >> - >> -ifdef CONFIG_X86 >> -calcs_ccflags := -mhard-float -msse >> -endif >> - >> -ifdef CONFIG_PPC64 >> -calcs_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). >> -calcs_ccflags += -mpreferred-stack-boundary=4 >> -else >> -calcs_ccflags += -msse2 >> -endif >> -endif >> - >> -CFLAGS_$(AMDDALPATH)/dc/calcs/dcn_calcs.o := $(calcs_ccflags) >> -CFLAGS_$(AMDDALPATH)/dc/calcs/dcn_calc_auto.o := $(calcs_ccflags) >> -CFLAGS_$(AMDDALPATH)/dc/calcs/dcn_calc_math.o := $(calcs_ccflags) >> -Wno-tautological-compare >> -CFLAGS_REMOVE_$(AMDDALPATH)/dc/calcs/dcn_calcs.o := > $(calcs_rcflags) >> -CFLAGS_REMOVE_$(AMDDALPATH)/dc/calcs/dcn_calc_auto.o := > $(calcs_rcflags) >> -CFLAGS_REMOVE_$(AMDDALPATH)/dc/calcs/dcn_calc_math.o := > $(calcs_rcflags) >> - >> -BW_CALCS = dce_calcs.o bw_fixed.o custom_float.o >> - >> -ifdef CONFIG_DRM_AMD_DC_DCN >> -BW_CALCS += dcn_calcs.o dcn_calc_math.o dcn_calc_auto.o >> -endif >> - >> -AMD_DAL_BW_CALCS = $(addprefix $(AMDDALPATH)/dc/calcs/,$(BW_CALCS)) >> - >> -AMD_DISPLAY_FILES += $(AMD_DAL_BW_CALCS) >> diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile >> b/drivers/gpu/drm/amd/display/dc/dml/Makefile >> index eee6672bd32d..82f26da6778e 100644 >> --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile >> +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile >> @@ -73,6 +73,11 @@ >> CFLAGS_$(AMDDALPATH)/dc/dml/dcn31/display_rq_dlg_calc_31.o := >> $(dml_ccflags) >> CFLAGS_$(AMDDALPATH)/dc/dml/dcn301/dcn301_fpu.o := $(dml_ccflags) >> CFLAGS_$(AMDDALPATH)/dc/dml/dsc/rc_calc_fpu.o := $(dml_ccflags) >> CFLAGS_$(AMDDALPATH)/dc/dml/display_mode_lib.o := $(dml_ccflags) >> + >> +CFLAGS_$(AMDDALPATH)/dc/dml/calcs/dcn_calcs.o := $(dml_ccflags) >> +CFLAGS_$(AMDDALPATH)/dc/dml/calcs/dcn_calc_auto.o := $(dml_ccflags) >> +CFLAGS_$(AMDDALPATH)/dc/dml/calcs/dcn_calc_math.o := $(dml_ccflags) >> -Wno-tautological-compare >> + >> CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml/display_mode_vba.o := > $(dml_rcflags) >> CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml/dcn2x/dcn2x.o := $(dml_rcflags) >> CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml/dcn20/display_mode_vba_20.o := >> $(dml_rcflags) >> @@ -94,7 +99,11 @@ >> CFLAGS_$(AMDDALPATH)/dc/dml/display_rq_dlg_helpers.o := > $(dml_ccflags) >> CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml/dml1_display_rq_dlg_calc.o := > $(dml_rcflags) >> CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml/display_rq_dlg_helpers.o := > $(dml_rcflags) >> >> -DML = display_mode_lib.o display_rq_dlg_helpers.o > dml1_display_rq_dlg_calc.o \ >> +CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml/calcs/dcn_calcs.o := > $(dml_rcflags) >> +CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml/calcs/dcn_calc_auto.o := > $(dml_rcflags) >> +CFLAGS_REMOVE_$(AMDDALPATH)/dc/dml/calcs/dcn_calc_math.o := > $(dml_rcflags) >> + >> +DML = display_mode_lib.o display_rq_dlg_helpers.o > dml1_display_rq_dlg_calc.o >> >> ifdef CONFIG_DRM_AMD_DC_DCN >> DML += dcn20/dcn20_fpu.o >> @@ -105,6 +114,8 @@ DML += dcn30/display_mode_vba_30.o >> dcn30/display_rq_dlg_calc_30.o >> DML += dcn31/display_mode_vba_31.o dcn31/display_rq_dlg_calc_31.o >> DML += dcn301/dcn301_fpu.o >> DML += dsc/rc_calc_fpu.o >> +DML += calcs/dce_calcs.o calcs/bw_fixed.o calcs/custom_float.o >> +DML += calcs/dcn_calcs.o calcs/dcn_calc_math.o > calcs/dcn_calc_auto.o >> endif >> >> AMD_DAL_DML = $(addprefix $(AMDDALPATH)/dc/dml/,$(DML)) >> diff --git a/drivers/gpu/drm/amd/display/dc/calcs/bw_fixed.c >> b/drivers/gpu/drm/amd/display/dc/dml/calcs/bw_fixed.c >> similarity index 100% >> rename from drivers/gpu/drm/amd/display/dc/calcs/bw_fixed.c >> rename to drivers/gpu/drm/amd/display/dc/dml/calcs/bw_fixed.c >> diff --git a/drivers/gpu/drm/amd/display/dc/calcs/calcs_logger.h >> b/drivers/gpu/drm/amd/display/dc/dml/calcs/calcs_logger.h >> similarity index 100% >> rename from drivers/gpu/drm/amd/display/dc/calcs/calcs_logger.h >> rename to drivers/gpu/drm/amd/display/dc/dml/calcs/calcs_logger.h >> diff --git a/drivers/gpu/drm/amd/display/dc/calcs/custom_float.c >> b/drivers/gpu/drm/amd/display/dc/dml/calcs/custom_float.c >> similarity index 100% >> rename from drivers/gpu/drm/amd/display/dc/calcs/custom_float.c >> rename to drivers/gpu/drm/amd/display/dc/dml/calcs/custom_float.c >> diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c >> b/drivers/gpu/drm/amd/display/dc/dml/calcs/dce_calcs.c >> similarity index 100% >> rename from drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c >> rename to drivers/gpu/drm/amd/display/dc/dml/calcs/dce_calcs.c >> diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_auto.c >> b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calc_auto.c >> similarity index 100% >> rename from drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_auto.c >> rename to drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calc_auto.c >> diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_auto.h >> b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calc_auto.h >> similarity index 100% >> rename from drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_auto.h >> rename to drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calc_auto.h >> diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c >> b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calc_math.c >> similarity index 100% >> rename from drivers/gpu/drm/amd/display/dc/calcs/dcn_calc_math.c >> rename to drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calc_math.c >> diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c >> b/drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c >> similarity index 100% >> rename from drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c >> rename to drivers/gpu/drm/amd/display/dc/dml/calcs/dcn_calcs.c > > Hi all, > > This isn't a v2, but actually a v1, I apologize for the mistake. > > -- > Isabella Basso > > > Links: > ------ > [1] <a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F93042%2F&data=04%7C01%7CJasdeep.Dhillon%40amd.com%7C8a3d537d1fb64bac26c308d9cef28d37%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637768363894413584%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=rpKQjmK4atQn0U3RMCmkMTe9Yyj0dq4YDBhQeA48lyM%3D&reserved=0"> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F93042%2F&data=04%7C01%7CJasdeep.Dhillon%40amd.com%7C8a3d537d1fb64bac26c308d9cef28d37%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637768363894413584%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=rpKQjmK4atQn0U3RMCmkMTe9Yyj0dq4YDBhQeA48lyM%3D&reserved=0</a> -- Isabella Basso </div> </div> </body> </html>