Re: [PATCH] drm/amd/display: fix documentation warnings for mpc.h
Hi Marcelo, First of all, thanks a lot for your patch! Please check some of my inline comments. On 4/27/24 10:05 AM, Marcelo Mendes Spessoto Junior wrote: Fix most of the display documentation compile warnings by documenting struct mpc_funcs functions in dc/inc/hw/mpc.h file. Could you add the warnings that you fixed in the commit message? I think some of your changes in this patch address some of the issues reported by Stephan Rothwell: https://lore.kernel.org/all/20240130134954.04fcf...@canb.auug.org.au/ Please include Stephan in the new version of this patch. Also, use the Fixes tags pointing to: b8c1c3a82e75 ("Documentation/gpu: Add kernel doc entry for MPC") Signed-off-by: Marcelo Mendes Spessoto Junior --- drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 372 +++- 1 file changed, 369 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h index 34a398f23..388b96c32 100644 --- a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h @@ -1,4 +1,5 @@ -/* Copyright 2012-15 Advanced Micro Devices, Inc. +/* + * Copyright 2012-15 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"), @@ -282,6 +283,21 @@ struct mpcc_state { * struct mpc_funcs - funcs */ struct mpc_funcs { + /** + * @read_mpcc_state: + * + * Read register content from given MPCC physical instance. It looks like your text editor left some extra spaces at the end of the string in many parts of this patch. Remove this extra space at the end of some of the strings in this patch for the next version. If you use git log -p after applying your patch, you can visually see where the extra space is at the end of each line. + * + * Parameters: + * + * - [in/out] mpc - MPC context + * - [in] mpcc_instance - MPC context instance + * - [in] mpcc_state - MPC context state + * + * Return: + * + * void + */ Use the same indentation as the field/functions that you are documenting. Finally, ensure you are using the latest amd-staging-drm-next to base your patch. I believe you have some merge conflicts since mpc.h has some changes. Thanks Siqueira void (*read_mpcc_state)( struct mpc *mpc, int mpcc_inst, @@ -346,12 +362,22 @@ struct mpc_funcs { * Parameters: * * - [in/out] mpc - MPC context. -* +* * Return: * * void */ void (*mpc_init)(struct mpc *mpc); + + /** + * @mpc_init_single_inst: + * + * Initialize given MPCC physical instance. + * + * Parameters: + * - [in/out] mpc - MPC context. + * - [in] mpcc_id - The MPCC physical instance to be initialized. + */ void (*mpc_init_single_inst)( struct mpc *mpc, unsigned int mpcc_id); @@ -449,62 +475,282 @@ struct mpc_funcs { struct mpc_tree *tree, struct mpcc *mpcc); + /** + * @get_mpcc_for_dpp_from_secondary: + * + * Find, if it exists, a MPCC from a given 'secondary' MPC tree that + * is associated with specified plane. + * + * Parameters: + * - [in/out] tree - MPC tree structure to search for plane. + * - [in] dpp_id - DPP to be searched. + * + * Return: + * + * struct mpcc* - pointer to plane or NULL if no plane found. + */ struct mpcc* (*get_mpcc_for_dpp_from_secondary)( struct mpc_tree *tree, int dpp_id); + /** + * @get_mpcc_for_dpp: + * + * Find, if it exists, a MPCC from a given MPC tree that + * is associated with specified plane. + * + * Parameters: + * - [in/out] tree - MPC tree structure to search for plane. + * - [in] dpp_id - DPP to be searched. + * + * Return: + * + * struct mpcc* - pointer to plane or NULL if no plane found. + */ struct mpcc* (*get_mpcc_for_dpp)( struct mpc_tree *tree, int dpp_id); + /** + * @wait_for_idle: + * + * Wait for a MPCC in MPC context to enter idle state. + * + * Parameters: + * - [in/out] mpc - MPC Context. + * - [in] id - MPCC to wait for idle state. + * + * Return: + * + * void + */ void (*wait_for_idle)(struct mpc *mpc, int id); + /** + * @assert_mpcc_idle_before_connect: + * + * Assert if MPCC in MPC context is in idle state. + * + * Parameters: + * - [in/out] mpc - MPC context. + * - [in] id - MPCC to assert idle state. + * + * Return: + * + * void + */ void (*assert_mpcc_idle_before_connect)(struct mpc *mpc, int mpcc_id); + /** + * @init_mpcc_list_from_hw: + * + * Iterate through the MPCC array from a given MPC context
Re: [PATCH 1/2] drm/amd/display: clean inconsistent indenting
On 2/13/24 3:43 PM, Joao Paulo Pereira da Silva wrote: From: jppaulo Clean some wrong indenting that throw errors in checkpatch. Signed-off-by: Joao Paulo Pereira da Silva --- drivers/gpu/drm/amd/display/dc/core/dc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index aa7c02ba948e..7832832b973d 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -962,7 +962,7 @@ static bool dc_construct(struct dc *dc, goto fail; } -dc_ctx = dc->ctx; + dc_ctx = dc->ctx; /* Resource should construct all asic specific resources. * This should be the only place where we need to parse the asic id @@ -3155,10 +3155,10 @@ static void commit_planes_do_stream_update(struct dc *dc, if (stream_update->mst_bw_update->is_increase) dc->link_srv->increase_mst_payload(pipe_ctx, stream_update->mst_bw_update->mst_stream_bw); - else + else dc->link_srv->reduce_mst_payload(pipe_ctx, stream_update->mst_bw_update->mst_stream_bw); - } + } if (stream_update->pending_test_pattern) { dc_link_dp_set_test_pattern(stream->link, Hi Joao, Could part of this patch not apply to amd-staging-drm-next (the second part is already present)? Could you rebase your change and squash these two commits in a single one? Thanks Siqueira
Re: [PATCH] drm/amd/display: Remove duplicated function signature from dcn3.01 DCCG
On 2/22/24 7:19 AM, David Tadokoro wrote: In the header file dc/dcn301/dcn301_dccg.h, the function dccg301_create is declared twice, so remove duplication. Signed-off-by: David Tadokoro --- drivers/gpu/drm/amd/display/dc/dcn301/dcn301_dccg.h | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_dccg.h b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_dccg.h index 73db962dbc03..067e49cb238e 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_dccg.h +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_dccg.h @@ -56,10 +56,4 @@ struct dccg *dccg301_create( const struct dccg_shift *dccg_shift, const struct dccg_mask *dccg_mask); -struct dccg *dccg301_create( - struct dc_context *ctx, - const struct dccg_registers *regs, - const struct dccg_shift *dccg_shift, - const struct dccg_mask *dccg_mask); - #endif //__DCN301_DCCG_H__ Hi David, Thanks a lot for your patch. Reviewed-by: Rodrigo Siqueira I already merged your patch into the asdn. Thanks Siqueira
Re: [PATCH RESEND] drm/amd/display: Fix division by zero in setup_dsc_config
On 4/22/24 8:35 AM, Jose Fernandez wrote: When slice_height is 0, the division by slice_height in the calculation of the number of slices will cause a division by zero driver crash. This leaves the kernel in a state that requires a reboot. This patch adds a check to avoid the division by zero. The stack trace below is for the 6.8.4 Kernel. I reproduced the issue on a Z16 Gen 2 Lenovo Thinkpad with a Apple Studio Display monitor connected via Thunderbolt. The amdgpu driver crashed with this exception when I rebooted the system with the monitor connected. kernel: ? die (arch/x86/kernel/dumpstack.c:421 arch/x86/kernel/dumpstack.c:434 arch/x86/kernel/dumpstack.c:447) kernel: ? do_trap (arch/x86/kernel/traps.c:113 arch/x86/kernel/traps.c:154) kernel: ? setup_dsc_config (drivers/gpu/drm/amd/amdgpu/../display/dc/dsc/dc_dsc.c:1053) amdgpu kernel: ? do_error_trap (./arch/x86/include/asm/traps.h:58 arch/x86/kernel/traps.c:175) kernel: ? setup_dsc_config (drivers/gpu/drm/amd/amdgpu/../display/dc/dsc/dc_dsc.c:1053) amdgpu kernel: ? exc_divide_error (arch/x86/kernel/traps.c:194 (discriminator 2)) kernel: ? setup_dsc_config (drivers/gpu/drm/amd/amdgpu/../display/dc/dsc/dc_dsc.c:1053) amdgpu kernel: ? asm_exc_divide_error (./arch/x86/include/asm/idtentry.h:548) kernel: ? setup_dsc_config (drivers/gpu/drm/amd/amdgpu/../display/dc/dsc/dc_dsc.c:1053) amdgpu kernel: dc_dsc_compute_config (drivers/gpu/drm/amd/amdgpu/../display/dc/dsc/dc_dsc.c:1109) amdgpu After applying this patch, the driver no longer crashes when the monitor is connected and the system is rebooted. I believe this is the same issue reported for 3113. Signed-off-by: Jose Fernandez Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3113 --- drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c index ac41f9c0a283..597d5425d6cb 100644 --- a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c +++ b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c @@ -1055,7 +1055,12 @@ static bool setup_dsc_config( if (!is_dsc_possible) goto done; - dsc_cfg->num_slices_v = pic_height/slice_height; + if (slice_height > 0) + dsc_cfg->num_slices_v = pic_height/slice_height; Hi Jose, First of all, thanks a lot for your patch. Your patch looks good; I just added {} in the if and spaces around '/' before merging your patch. Anyway, Reviewed-by: Rodrigo Siqueira Thanks Siqueira + else { + is_dsc_possible = false; + goto done; + } if (target_bandwidth_kbps > 0) { is_dsc_possible = decide_dsc_target_bpp_x16(
Re: [PATCH v5 7/8] drm/amd/display: Introduce KUnit tests to dc_dmub_srv library
On 2/26/24 04:12, Jani Nikula wrote: On Thu, 22 Feb 2024, Rodrigo Siqueira wrote: diff --git a/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig b/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig index eb6f81601757..4c5861ad58bd 100644 --- a/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig +++ b/drivers/gpu/drm/amd/display/test/kunit/.kunitconfig @@ -4,5 +4,6 @@ CONFIG_DRM=y CONFIG_DRM_AMDGPU=y CONFIG_DRM_AMD_DC=y CONFIG_AMD_DC_BASICS_KUNIT_TEST=y +CONFIG_AMD_DC_KUNIT_TEST=y CONFIG_DCE_KUNIT_TEST=y CONFIG_DML_KUNIT_TEST=y A bit random patch to comment on, but this hunk demonstrates the point: Should all the configs have DRM_AMD_ prefix to put them in a "namespace"? You are right! I'll fix this in the next version. Thanks Siqueira BR, Jani.
Re: [PATCH] drm/amd/display: Use kcalloc() instead of kzalloc()
On 1/28/24 02:04, Lenko Donchev wrote: We are trying to get rid of all multiplications from allocation functions to prevent integer overflows. Here the multiplication is obviously safe, but using kcalloc() is more appropriate and improves readability. This patch has no effect on runtime behavior. Link: https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments Link: https://github.com/KSPP/linux/issues/162 Signed-off-by: Lenko Donchev --- drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c index 5c9a30211c10..b67cd78e7c58 100644 --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dpcd.c @@ -164,7 +164,7 @@ static void dpcd_extend_address_range( if (new_addr_range.start != in_address || new_addr_range.end != end_address) { *out_address = new_addr_range.start; *out_size = ADDRESS_RANGE_SIZE(new_addr_range.start, new_addr_range.end); - *out_data = kzalloc(*out_size * sizeof(**out_data), GFP_KERNEL); + *out_data = kcalloc(*out_size, sizeof(**out_data), GFP_KERNEL); } } lgtm, Reviewed-by: Rodrigo Siqueira
Re: [PATCH] drm/amd/display: clean unnecessary braces
Hi Túlio, First of all thanks for your patch. See my comments inline. On 2/17/24 13:20, Túlio Fernandes wrote: Clean unnecessary braces in dc/dcn32/dcn32_resource_helpers.c and dc/dcn32/dcn201_link_encoder.c Did you identify this issue with checkpatch? If so, I recommend you paste the error message in the commit message. Iirc, checkpatch provides the file and the function name, which can make this commit message more informative. Also, wrap the commit message under 70 characters. Signed-off-by: Túlio Fernandes --- .../display/dc/dcn32/dcn32_resource_helpers.c| 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c index 87760600e154..e179dea148e7 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c +++ b/drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource_helpers.c @@ -110,14 +110,12 @@ uint32_t dcn32_helper_calculate_num_ways_for_subvp( struct dc_state *context) { if (context->bw_ctx.bw.dcn.mall_subvp_size_bytes > 0) { - if (dc->debug.force_subvp_num_ways) { + if (dc->debug.force_subvp_num_ways) return dc->debug.force_subvp_num_ways; - } else { + else return dcn32_helper_mall_bytes_to_ways(dc, context->bw_ctx.bw.dcn.mall_subvp_size_bytes); - } - } else { + } else Actually, we want to keep the braces around the else part to keep the braces balanced with the if condition. Thanks Siqueira return 0; - } } void dcn32_merge_pipes_for_subvp(struct dc *dc, @@ -250,9 +248,9 @@ bool dcn32_is_psr_capable(struct pipe_ctx *pipe) { bool psr_capable = false; - if (pipe->stream && pipe->stream->link->psr_settings.psr_version != DC_PSR_VERSION_UNSUPPORTED) { + if (pipe->stream && pipe->stream->link->psr_settings.psr_version != DC_PSR_VERSION_UNSUPPORTED) psr_capable = true; - } + return psr_capable; } @@ -278,9 +276,9 @@ static void override_det_for_subvp(struct dc *dc, struct dc_state *context, uint if (pipe_ctx->stream && pipe_ctx->plane_state && dc_state_get_pipe_subvp_type(context, pipe_ctx) != SUBVP_PHANTOM) { if (dcn32_allow_subvp_high_refresh_rate(dc, context, pipe_ctx)) { -if (pipe_ctx->stream->timing.v_addressable == 1080 && pipe_ctx->stream->timing.h_addressable == 1920) { + if (pipe_ctx->stream->timing.v_addressable == 1080 && pipe_ctx->stream->timing.h_addressable == 1920) fhd_count++; - } + subvp_high_refresh_count++; } }
Re: [PATCH] drm/amd/display: cleanup inconsistent indenting in amdgpu_dm_color
On 1/5/24 15:02, Melissa Wen wrote: smatch warnings: amdgpu_dm_update_plane_color_mgmt() warn: inconsistent indenting Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202401051643.ppdbmg1u-...@intel.com/ Signed-off-by: Melissa Wen --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c index 9b527bffe11a..c87b64e464ed 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c @@ -1239,7 +1239,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc, if (has_crtc_cm_degamma && ret != -EINVAL) { drm_dbg_kms(crtc->base.crtc->dev, "doesn't support plane and CRTC degamma at the same time\n"); - return -EINVAL; + return -EINVAL; } /* If we are here, it means we don't have plane degamma settings, check Reviewed-by: Rodrigo Siqueira Change also applied to asdn. Thanks Siqueira
Re: [PATCH v3 3/9] drm/amd/display: read gamut remap matrix in fixed-point 31.32 format
On 11/28/23 10:52, Melissa Wen wrote: Instead of read gamut remap data from hw values, convert HW register values (S2D13) into a fixed-point 31.32 matrix for color state log. Change DCN10 log to print data in the format of the gamut remap matrix. Signed-off-by: Melissa Wen --- .../amd/display/dc/hwss/dcn10/dcn10_hwseq.c | 38 +-- drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h | 3 ++ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c index 9b801488eb9d..f0a9f8818909 100644 --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c @@ -289,20 +289,26 @@ static void dcn10_log_color_state(struct dc *dc, struct resource_pool *pool = dc->res_pool; int i; - DTN_INFO("DPP:IGAM format IGAM modeDGAM modeRGAM mode" - " GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 " - "C31 C32 C33 C34\n"); + DTN_INFO("DPP:IGAM formatIGAM modeDGAM modeRGAM mode" +" GAMUT adjust " +"C11C12C13C14" +"C21C22C23C24" +"C31C32C33C34\n"); for (i = 0; i < pool->pipe_count; i++) { struct dpp *dpp = pool->dpps[i]; struct dcn_dpp_state s = {0}; dpp->funcs->dpp_read_state(dpp, ); + dpp->funcs->dpp_get_gamut_remap(dpp, _remap); if (!s.is_enabled) continue; - DTN_INFO("[%2d]: %11xh %-11s %-11s %-11s" - "%8x%08xh %08xh %08xh %08xh %08xh %08xh", + DTN_INFO("[%2d]: %11xh %11s%9s%9s" +" %12s " +"%010lld %010lld %010lld %010lld " +"%010lld %010lld %010lld %010lld " +"%010lld %010lld %010lld %010lld", dpp->inst, s.igam_input_format, (s.igam_lut_mode == 0) ? "BypassFixed" : @@ -322,13 +328,21 @@ static void dcn10_log_color_state(struct dc *dc, ((s.rgam_lut_mode == 3) ? "RAM" : ((s.rgam_lut_mode == 4) ? "RAM" : "Unknown", - s.gamut_remap_mode, - s.gamut_remap_c11_c12, - s.gamut_remap_c13_c14, - s.gamut_remap_c21_c22, - s.gamut_remap_c23_c24, - s.gamut_remap_c31_c32, - s.gamut_remap_c33_c34); + (s.gamut_remap.gamut_adjust_type == 0) ? "Bypass" : + ((s.gamut_remap.gamut_adjust_type == 1) ? "HW" : + "SW"), + s.gamut_remap.temperature_matrix[0].value, + s.gamut_remap.temperature_matrix[1].value, + s.gamut_remap.temperature_matrix[2].value, + s.gamut_remap.temperature_matrix[3].value, + s.gamut_remap.temperature_matrix[4].value, + s.gamut_remap.temperature_matrix[5].value, + s.gamut_remap.temperature_matrix[6].value, + s.gamut_remap.temperature_matrix[7].value, + s.gamut_remap.temperature_matrix[8].value, + s.gamut_remap.temperature_matrix[9].value, + s.gamut_remap.temperature_matrix[10].value, + s.gamut_remap.temperature_matrix[11].value); DTN_INFO("\n"); } DTN_INFO("\n"); diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h b/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h index 597ebdb4da4c..b6acfd86642a 100644 --- a/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h @@ -141,6 +141,7 @@ struct dcn_dpp_state { uint32_t igam_input_format; uint32_t dgam_lut_mode; uint32_t rgam_lut_mode; + // gamut_remap data for dcn10_get_cm_states() uint32_t gamut_remap_mode; uint32_t gamut_remap_c11_c12; uint32_t gamut_remap_c13_c14; @@ -148,6 +149,8 @@ struct dcn_dpp_state { uint32_t gamut_remap_c23_c24; uint32_t gamut_remap_c31_c32; uint32_t gamut_remap_c33_c34; + // gamut_remap data for dcn*_log_color_state() + struct
Re: [PATCH v3 1/9] drm/amd/display: decouple color state from hw state log
On 11/28/23 10:52, Melissa Wen wrote: Prepare to hook up color state log according to the DCN version. v3: - put functions in single line (Siqueira) Signed-off-by: Melissa Wen --- .../amd/display/dc/hwss/dcn10/dcn10_hwseq.c | 26 +-- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c index 2b8b8366538e..9b801488eb9d 100644 --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn10/dcn10_hwseq.c @@ -282,19 +282,13 @@ static void dcn10_log_hubp_states(struct dc *dc, void *log_ctx) DTN_INFO("\n"); } -void dcn10_log_hw_state(struct dc *dc, - struct dc_log_buffer_ctx *log_ctx) +static void dcn10_log_color_state(struct dc *dc, + struct dc_log_buffer_ctx *log_ctx) { struct dc_context *dc_ctx = dc->ctx; struct resource_pool *pool = dc->res_pool; int i; - DTN_INFO_BEGIN(); - - dcn10_log_hubbub_state(dc, log_ctx); - - dcn10_log_hubp_states(dc, log_ctx); - DTN_INFO("DPP:IGAM format IGAM modeDGAM modeRGAM mode" " GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 " "C31 C32 C33 C34\n"); @@ -351,6 +345,22 @@ void dcn10_log_hw_state(struct dc *dc, s.idle); } DTN_INFO("\n"); +} + +void dcn10_log_hw_state(struct dc *dc, + struct dc_log_buffer_ctx *log_ctx) +{ + struct dc_context *dc_ctx = dc->ctx; + struct resource_pool *pool = dc->res_pool; + int i; + + DTN_INFO_BEGIN(); + + dcn10_log_hubbub_state(dc, log_ctx); + + dcn10_log_hubp_states(dc, log_ctx); + + dcn10_log_color_state(dc, log_ctx); DTN_INFO("OTG: v_bs v_be v_ss v_se vpol vmax vmin vmax_sel vmin_sel h_bs h_be h_ss h_se hpol htot vtot underflow blank_en\n"); Reviewed-by: Rodrigo Siqueira
Re: [PATCH] drm/amd/display: add kernel docs for dc_stream_forward_crc_window
Hi Sagar, First of all, thanks for your patch. On 10/25/23 08:04, Sagar Vashnav wrote: Add kernel documentation for the dc_stream_forward_crc_window Signed-off-by: Sagar Vashnav --- drivers/gpu/drm/amd/display/dc/core/dc.c | 13 + 1 file changed, 13 insertions(+) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index 1729fb727..5ab35e482 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -528,6 +528,19 @@ dc_stream_forward_dmcu_crc_window(struct dmcu *dmcu, dmcu->funcs->forward_crc_window(dmcu, rect, mux_mapping); } +/** + * dc_stream_forward_crc_window() - Forward CRC window configuration to DMUB or DMCU. Add an empty comment line between the summary and the parameter description. + * @stream: The stream state to forward CRC window configuration for. + * @rect: Pointer to the rectangle defining the CRC window coordinates. + * @is_stop: Flag indicating whether the CRC capture should be stopped. + You need to add `*` in the above line. + * This function is responsible for forwarding the CRC window configuration + * for a given stream to either the DMUB or DMCU, depending on their availability. + Same as my previous comment. + * Return: + * %true if the CRC window configuration was successfully forwarded; + * %false if the stream was not found or CRC forwarding is not supported. Afaik, we don't use `%` in the kernel-doc. Maybe just use 'True' and 'False'? Thanks Siqueira + */ bool dc_stream_forward_crc_window(struct dc_stream_state *stream, struct rect *rect, bool is_stop)
Re: [PATCH 0/5] drm/amd/display: Remove migrate-disable and move memory allocation.
On 9/21/23 08:15, Sebastian Andrzej Siewior wrote: Hi, I stumbled uppon the amdgpu driver via a bugzilla report. The actual fix is #4 + #5 and the rest was made while looking at the code. Sebastian Hi Sebastian, Thanks a lot for this patchset. We tested it on multiple devices, and everything looks good. I also reviewed it and lgtm. Reviewed-by: Rodrigo Siqueira Thanks Siqueira
Re: [RFC PATCH v2 0/5] drm/amd/display: improve DTN color state log
On 9/13/23 10:43, Melissa Wen wrote: Hi, This is an update of previous RFC [0] improving the data collection of Gamma Correction and Blend Gamma color blocks. As I mentioned in the last version, I'm updating the color state part of DTN log to match DCN3.0 HW better. Currently, the DTN log considers the DCN10 color pipeline, which is useless for DCN3.0 because of all the differences in color caps between DCN versions. In addition to new color blocks and caps, some semantic differences made the DCN10 output not fit DCN30. In this RFC, the first patch adds new color state elements to DPP and implements the reading of registers according to HW blocks. Similarly to MPC, the second patch also creates a DCN3-specific function to read the MPC state and add the MPC color state logging to it. With DPP and MPC color-register reading, I detach DCN10 color state logging from the HW log and create a `.log_color_state` hook for logging color state according to HW color blocks with DCN30 as the first use case. Finally, the last patch adds DPP and MPC color caps output to facilitate understanding of the color state log. This version works well with the driver-specific color properties[1] and steamdeck/gamescope[2] together, where we can see color state changing from default values. Here is a before vs. after example: Without this series: === DPP:IGAM format IGAM modeDGAM modeRGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34 [ 0]:0h BypassFixed Bypass Bypass0h h h h h h [ 3]:0h BypassFixed Bypass Bypass0h h h h h h MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE [ 0]: 0h 0h 3h 3 20 0 0 [ 3]: 0h 3h fh 2 20 0 0 With this series (Steamdeck/Gamescope): == DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34 [ 0]:1 sRGBBypassRAM A RAM B 12-bit17x17x17 RAM A 0 h h h h h h [ 1]:1 sRGBBypassRAM B RAM A 12-bit17x17x17 RAM A 0 h h h h h h [ 2]:1 sRGBBypassRAM B RAM A 12-bit17x17x17 RAM A 0 h h h h h h [ 3]:1 sRGBBypassRAM A RAM B 12-bit17x17x17 RAM A 0 h h h h h h DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1 dgam_rom_for_yuv:0 3d_lut:1 blnd_lut:1 oscs:0 MPCC: OPP DPP MPCCBOT MODE ALPHA_MODE PREMULT OVERLAP_ONLY IDLE SHAPER mode 3DLUT_mode 3DLUT bit-depth 3DLUT size OGAM mode OGAM LUT GAMUT mode C11 C12 C33 C34 [ 0]: 0h 0h 2h 3 01 0 0 Bypass Bypass 12-bit17x17x17RAM A 0 h h [ 1]: 0h 1h fh 2 20 0 0 Bypass Bypass 12-bit17x17x17 Bypass A 0 h h [ 2]: 0h 2h 3h 3 01 0 0 Bypass Bypass 12-bit17x17x17 Bypass A 0 h h [ 3]: 0h 3h 1h 3 20 0 0 Bypass Bypass 12-bit17x17x17 Bypass A 0 h h MPC Color Caps: gamut_remap:1, 3dlut:2, ogam_ram:1, ocsc:1 I liked this new visualization. At some point, we need to document this information as a kernel-doc to make it easy to understand this DTN LOG. With this series (Steamdeck/KDE): DPP: DGAM ROM DGAM ROM type DGAM LUT SHAPER mode 3DLUT mode 3DLUT bit depth 3DLUT size RGAM mode GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 C31 C32 C33 C34 [ 0]:0 sRGBBypass Bypass Bypass 12-bit 9x9x9 Bypass 0 h h h h h h [ 3]:0 sRGBBypass Bypass Bypass 12-bit 9x9x9 Bypass 0 h h h h h h DPP Color Caps: input_lut_shared:0 icsc:1 dgam_ram:0 dgam_rom: srgb:1,bt2020:1,gamma2_2:1,pq:1,hlg:1 post_csc:1 gamcor:1
Re: [RFC PATCH v2 2/5] drm/amd/display: fill up DCN3 DPP color state
On 9/13/23 10:43, Melissa Wen wrote: DCN3 DPP color state was uncollected and some state elements from DCN1 doesn't fit DCN3. Create new elements according to DCN3 color caps and fill them up for DTN log output. rfc-v2: - fix reading of gamcor and blnd gamma states Signed-off-by: Melissa Wen --- .../gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c | 45 +-- drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h | 8 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c index 50dc83404644..a26b33c84ae0 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c @@ -44,11 +44,50 @@ void dpp30_read_state(struct dpp *dpp_base, struct dcn_dpp_state *s) { struct dcn3_dpp *dpp = TO_DCN30_DPP(dpp_base); + uint32_t gamcor_lut_mode, rgam_lut_mode; REG_GET(DPP_CONTROL, - DPP_CLOCK_ENABLE, >is_enabled); - - // TODO: Implement for DCN3 + DPP_CLOCK_ENABLE, >is_enabled); + // Pre-degamma (ROM) + REG_GET_2(PRE_DEGAM, + PRE_DEGAM_MODE, >pre_dgam_mode, + PRE_DEGAM_SELECT, >pre_dgam_select); nitpick: Add a new line before each comment in this function. + // Gamma Correction (RAM) + REG_GET(CM_GAMCOR_CONTROL, + CM_GAMCOR_MODE_CURRENT, >gamcor_mode); + if (s->gamcor_mode) { + REG_GET(CM_GAMCOR_CONTROL, CM_GAMCOR_SELECT_CURRENT, _lut_mode); + if (!gamcor_lut_mode) + s->gamcor_mode = LUT_RAM_A; // Otherwise, LUT_RAM_B + } + // Shaper LUT (RAM), 3D LUT (mode, bit-depth, size) + REG_GET(CM_SHAPER_CONTROL, + CM_SHAPER_LUT_MODE, >shaper_lut_mode); + REG_GET(CM_3DLUT_MODE, + CM_3DLUT_MODE_CURRENT, >lut3d_mode); + REG_GET(CM_3DLUT_READ_WRITE_CONTROL, + CM_3DLUT_30BIT_EN, >lut3d_bit_depth); + REG_GET(CM_3DLUT_MODE, + CM_3DLUT_SIZE, >lut3d_size); + // Gamut Remap Matrix (3x4) + REG_GET(CM_GAMUT_REMAP_CONTROL, + CM_GAMUT_REMAP_MODE, >gamut_remap_mode); + if (s->gamut_remap_mode) { + s->gamut_remap_c11_c12 = REG_READ(CM_GAMUT_REMAP_C11_C12); + s->gamut_remap_c13_c14 = REG_READ(CM_GAMUT_REMAP_C13_C14); + s->gamut_remap_c21_c22 = REG_READ(CM_GAMUT_REMAP_C21_C22); + s->gamut_remap_c23_c24 = REG_READ(CM_GAMUT_REMAP_C23_C24); + s->gamut_remap_c31_c32 = REG_READ(CM_GAMUT_REMAP_C31_C32); + s->gamut_remap_c33_c34 = REG_READ(CM_GAMUT_REMAP_C33_C34); + } + // Blend/Out Gamma (RAM) + REG_GET(CM_BLNDGAM_CONTROL, + CM_BLNDGAM_MODE_CURRENT, >rgam_lut_mode); + if (s->rgam_lut_mode){ + REG_GET(CM_BLNDGAM_CONTROL, CM_BLNDGAM_SELECT_CURRENT, _lut_mode); + if (!rgam_lut_mode) + s->rgam_lut_mode = LUT_RAM_A; // Otherwise, LUT_RAM_B + } } /*program post scaler scs block in dpp CM*/ void dpp3_program_post_csc( diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h b/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h index f4aa76e02518..1dfe08dc4364 100644 --- a/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/dpp.h @@ -148,6 +148,14 @@ struct dcn_dpp_state { uint32_t gamut_remap_c23_c24; uint32_t gamut_remap_c31_c32; uint32_t gamut_remap_c33_c34; + uint32_t shaper_lut_mode; + uint32_t lut3d_mode; + uint32_t lut3d_bit_depth; + uint32_t lut3d_size; + uint32_t blnd_lut_mode; + uint32_t pre_dgam_mode; + uint32_t pre_dgam_select; + uint32_t gamcor_mode; }; struct CM_bias_params {
Re: [RFC PATCH v2 1/5] drm/amd/display: detach color state from hw state logging
On 9/13/23 10:43, Melissa Wen wrote: Prepare to hook color state logging according to DCN version. Signed-off-by: Melissa Wen --- .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 27 +-- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c index 79befa17bb03..a0c489ed218c 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c @@ -279,19 +279,14 @@ static void dcn10_log_hubp_states(struct dc *dc, void *log_ctx) DTN_INFO("\n"); } -void dcn10_log_hw_state(struct dc *dc, - struct dc_log_buffer_ctx *log_ctx) +static void +dcn10_log_color_state(struct dc *dc, + struct dc_log_buffer_ctx *log_ctx) nitpick: put the function definition in a single line. { struct dc_context *dc_ctx = dc->ctx; struct resource_pool *pool = dc->res_pool; int i; - DTN_INFO_BEGIN(); - - dcn10_log_hubbub_state(dc, log_ctx); - - dcn10_log_hubp_states(dc, log_ctx); - DTN_INFO("DPP:IGAM format IGAM modeDGAM modeRGAM mode" " GAMUT mode C11 C12 C13 C14 C21 C22 C23 C24 " "C31 C32 C33 C34\n"); @@ -348,6 +343,22 @@ void dcn10_log_hw_state(struct dc *dc, s.idle); } DTN_INFO("\n"); +} + +void dcn10_log_hw_state(struct dc *dc, + struct dc_log_buffer_ctx *log_ctx) ditto +{ + struct dc_context *dc_ctx = dc->ctx; + struct resource_pool *pool = dc->res_pool; + int i; + + DTN_INFO_BEGIN(); + + dcn10_log_hubbub_state(dc, log_ctx); + + dcn10_log_hubp_states(dc, log_ctx); + + dcn10_log_color_state(dc, log_ctx); DTN_INFO("OTG: v_bs v_be v_ss v_se vpol vmax vmin vmax_sel vmin_sel h_bs h_be h_ss h_se hpol htot vtot underflow blank_en\n");
Re: [PATCH 3/3] drm/amd/display: drop unused count variable in create_eml_sink()
On 5/17/23 12:33, Hamza Mahfooz wrote: Since, we are only interested in having drm_edid_override_connector_update(), update the value of connector->edid_blob_ptr. We don't care about the return value of drm_edid_override_connector_update() here. So, drop count. Fixes: 068553e14f86 ("drm/amd/display: assign edid_blob_ptr with edid from debugfs") Reported-by: kernel test robot Signed-off-by: Hamza Mahfooz --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 14b296e1d0f6..5a2d04f47276 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -6396,9 +6396,8 @@ static void create_eml_sink(struct amdgpu_dm_connector *aconnector) /* if connector->edid_override valid, pass * it to edid_override to edid_blob_ptr */ - int count; - count = drm_edid_override_connector_update(>base); + drm_edid_override_connector_update(>base); if (!aconnector->base.edid_blob_ptr) { DRM_ERROR("No EDID firmware found on connector: %s ,forcing to OFF!\n", Reviewed-by: Rodrigo Siqueira
Re: [PATCH 2/3] drm/amd/display: drop unused function set_abm_event()
On 5/17/23 12:33, Hamza Mahfooz wrote: set_abm_event() is never actually used. So, drop it. Fixes: b46c01aa0329 ("drm/amd/display: Refactor ABM feature") Reported-by: kernel test robot Reported-by: Tom Rix Signed-off-by: Hamza Mahfooz --- drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c | 12 drivers/gpu/drm/amd/display/dc/inc/hw/abm.h | 2 -- 2 files changed, 14 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c b/drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c index a66f83a61402..2fb9572ce25d 100644 --- a/drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c +++ b/drivers/gpu/drm/amd/display/dc/dce/dmub_abm.c @@ -131,17 +131,6 @@ static bool dmub_abm_set_pipe_ex(struct abm *abm, uint32_t otg_inst, uint32_t op return ret; } -static bool dmub_abm_set_event_ex(struct abm *abm, unsigned int full_screen, unsigned int video_mode, - unsigned int hdr_mode, unsigned int panel_inst) -{ - bool ret = false; - unsigned int feature_support; - - feature_support = abm_feature_support(abm, panel_inst); - - return ret; -} - static bool dmub_abm_set_backlight_level_pwm_ex(struct abm *abm, unsigned int backlight_pwm_u16_16, unsigned int frame_ramp, @@ -167,7 +156,6 @@ static const struct abm_funcs abm_funcs = { .init_abm_config = dmub_abm_init_config_ex, .set_abm_pause = dmub_abm_set_pause_ex, .set_pipe_ex = dmub_abm_set_pipe_ex, - .set_abm_event = dmub_abm_set_event_ex, .set_backlight_level_pwm = dmub_abm_set_backlight_level_pwm_ex, }; diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/abm.h b/drivers/gpu/drm/amd/display/dc/inc/hw/abm.h index db5cf9acafe6..d2190a3320f6 100644 --- a/drivers/gpu/drm/amd/display/dc/inc/hw/abm.h +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/abm.h @@ -59,8 +59,6 @@ struct abm_funcs { unsigned int otg_inst, unsigned int option, unsigned int panel_inst); - bool (*set_abm_event)(struct abm *abm, unsigned int full_screen, unsigned int video_mode, - unsigned int hdr_mode, unsigned int panel_inst); }; #endif Reviewed-by: Rodrigo Siqueira
Re: [PATCH 1/3] drm/amd/display: drop redundant memset() in get_available_dsc_slices()
On 5/17/23 12:33, Hamza Mahfooz wrote: get_available_dsc_slices() returns the number of indices set, and all of the users of get_available_dsc_slices() don't cross the returned bound when iterating over available_slices[]. So, the memset() in get_available_dsc_slices() is redundant and can be dropped. Fixes: 97bda0322b8a ("drm/amd/display: Add DSC support for Navi (v2)") Reported-by: Christophe JAILLET Signed-off-by: Hamza Mahfooz --- drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c index b9a05bb025db..58dd62cce4bb 100644 --- a/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c +++ b/drivers/gpu/drm/amd/display/dc/dsc/dc_dsc.c @@ -645,8 +645,6 @@ static int get_available_dsc_slices(union dsc_enc_slice_caps slice_caps, int *av { int idx = 0; - memset(available_slices, -1, MIN_AVAILABLE_SLICES_SIZE); - if (slice_caps.bits.NUM_SLICES_1) available_slices[idx++] = 1; Reviewed-by: Rodrigo Siqueira
Re: [PATCH v3 0/6] drm/amd/display: Pass proper parent for DM backlight device v3
On 3/12/23 13:17, Hans de Goede wrote: Hi All, Here is version 3 of my patch series to pass the proper parent device to backlight_device_register(). Changes in v3: - Make amdgpu_dm_register_backlight_device() check bl_idx != 1 before registering the backlight since amdgpu_dm_connector_late_register() now calls it for _all_ connectors. Changes in v2: - Patches 1 - 5 are new, reworking the code a bit to allow delaying the registering, so this has turned from a single patch into a 6 patch set. - Patch 6 now delays the registering of the backlight_dev till after the drm_connector is registered by doing it from drm_connector_funcs.late_register. Note this no longer is RFC since this has been successfully tested on 3 laptops which hit the affected code path. Version 3 has also been tested on my personal AMD Ryzen 7 5700G APU desktop machine and now no longer tries to register a backlight device for each connector there. Regards, Hans Hans de Goede (6): drm/amd/display/amdgpu_dm: Fix backlight_device_register() error handling drm/amd/display/amdgpu_dm: Refactor register_backlight_device() drm/amd/display/amdgpu_dm: Add a bl_idx to amdgpu_dm_connector drm/amd/display/amdgpu_dm: Move most backlight setup into setup_backlight_device() drm/amd/display/amdgpu_dm: Make amdgpu_dm_register_backlight_device() take an amdgpu_dm_connector drm/amd/display/amdgpu_dm: Pass proper parent for backlight device registration v3 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 100 -- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 + 2 files changed, 46 insertions(+), 55 deletions(-) Hi, First of all, thanks a lot for this patchset. I run your series in our CI (IGT-based), and I also conducted some manual tests in my ASICs. Everything looks fine. I also reviewed this series, and it LGTM: Reviewed-by: Rodrigo Siqueira Finally, I pushed it to amd-staging-drm-next. Thanks Siqueira
Re: [RFC v2 0/6] drm/amd/display: Pass proper parent for DM backlight device v2
Hi Hans, Which AMD device do you have available for testing this series? P.s.: If you have a new version of this series, could you also Cc me? Thanks for your patchset. Siqueira On 3/8/23 14:58, Hans de Goede wrote: Hi All, Here is version 2 of my patch series to pass the proper parent device to backlight_device_register(). New in version 2 is delaying the registering of the backlight_dev till after the drm_connector is registered by doing it from drm_connector_funcs.late_register. This involves first reworking the code a bit to allow delaying the registering, so this has turned from a single patch into a 6 patch set. Regards, Hans Hans de Goede (6): drm/amd/display/amdgpu_dm: Fix backlight_device_register() error handling drm/amd/display/amdgpu_dm: Refactor register_backlight_device() drm/amd/display/amdgpu_dm: Add a bl_idx to amdgpu_dm_connector drm/amd/display/amdgpu_dm: Move most backlight setup into setup_backlight_device() drm/amd/display/amdgpu_dm: Make amdgpu_dm_register_backlight_device() take an amdgpu_dm_connector drm/amd/display: Pass proper parent for DM backlight device registration v2 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 99 --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 1 + 2 files changed, 44 insertions(+), 56 deletions(-)
Re: [PATCH] drm/amd/display: use a more accurate check in dm_helpers_dp_read_dpcd()
On 3/9/23 14:30, Hamza Mahfooz wrote: We should be checking if drm_dp_dpcd_read() returns the size that we are asking it to read instead of just checking if it is greater than zero. Also, we should WARN_ON() here since this condition is only ever met, if there is an issue worth investigating. So, compare the return value of drm_dp_dpcd_read() to size and WARN_ON() if they aren't equal. Signed-off-by: Hamza Mahfooz --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 8d598b322e5b..ed2ed7b1d869 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -511,8 +511,8 @@ bool dm_helpers_dp_read_dpcd( return false; } - return drm_dp_dpcd_read(>dm_dp_aux.aux, address, - data, size) > 0; + return !WARN_ON(drm_dp_dpcd_read(>dm_dp_aux.aux, address, +data, size) != size); } bool dm_helpers_dp_write_dpcd( Reviewed-by: Rodrigo Siqueira and pushed to amd-staging-drm-next. Thanks Siqueira
Re: [PATCH] drm/amd/display: remove legacy fields of dc_plane_cap struct
On 3/7/23 15:53, David Tadokoro wrote: The fields blends_with_above and blends_with_below of struct dc_plane_cap (defined in dc/dc.h) are boolean and set to true by default. All instances of a dc_plane_cap maintain the default values of both. Also, there is only one if statement that checks those fields and there would be the same effect if it was deleted (assuming that those fields are always going to be true). For this reason, considering both fields as legacy ones, this commit removes them and the aforementioned if statement. Signed-off-by: David Tadokoro --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 --- drivers/gpu/drm/amd/display/dc/dc.h | 2 -- drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 3 --- drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn201/dcn201_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn21/dcn21_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn30/dcn30_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn301/dcn301_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn302/dcn302_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn303/dcn303_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn314/dcn314_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn32/dcn32_resource.c | 2 -- drivers/gpu/drm/amd/display/dc/dcn321/dcn321_resource.c | 2 -- 17 files changed, 36 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index b472931cb7ca..fdcb375e908a 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4354,9 +4354,6 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev) if (plane->type != DC_PLANE_TYPE_DCN_UNIVERSAL) continue; - if (!plane->blends_with_above || !plane->blends_with_below) - continue; - if (!plane->pixel_format_support.argb) continue; diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h index f0a1934ebf8c..ccc27d482640 100644 --- a/drivers/gpu/drm/amd/display/dc/dc.h +++ b/drivers/gpu/drm/amd/display/dc/dc.h @@ -82,8 +82,6 @@ enum det_size { struct dc_plane_cap { enum dc_plane_type type; - uint32_t blends_with_above : 1; - uint32_t blends_with_below : 1; uint32_t per_pixel_alpha : 1; struct { uint32_t argb : 1; diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c index f808315b2835..a4a45a6ce61e 100644 --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c @@ -401,8 +401,6 @@ static const struct resource_caps stoney_resource_cap = { static const struct dc_plane_cap plane_cap = { .type = DC_PLANE_TYPE_DCE_RGB, - .blends_with_below = true, - .blends_with_above = true, .per_pixel_alpha = 1, .pixel_format_support = { @@ -428,7 +426,6 @@ static const struct dc_plane_cap plane_cap = { static const struct dc_plane_cap underlay_plane_cap = { .type = DC_PLANE_TYPE_DCE_UNDERLAY, - .blends_with_above = true, .per_pixel_alpha = 1, .pixel_format_support = { diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c index 6bfac8088ab0..2bb8e11f26e0 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c @@ -504,8 +504,6 @@ static const struct resource_caps rv2_res_cap = { static const struct dc_plane_cap plane_cap = { .type = DC_PLANE_TYPE_DCN_UNIVERSAL, - .blends_with_above = true, - .blends_with_below = true, .per_pixel_alpha = true, .pixel_format_support = { 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 3af24ef9cb2d..00668df0938e 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c @@ -670,8 +670,6 @@ static const struct resource_caps res_cap_nv10 = { static const struct dc_plane_cap plane_cap = { .type = DC_PLANE_TYPE_DCN_UNIVERSAL, - .blends_with_above = true, - .blends_with_below = true, .per_pixel_alpha = true, .pixel_format_support = { diff --git
Re: [PATCH] drm/amd/display: Fix set scaling doesn's work
On 1/11/23 10:19, Harry Wentland wrote: On 1/10/23 10:58, Rodrigo Siqueira Jordao wrote: On 11/22/22 06:20, hongao wrote: [Why] Setting scaling does not correctly update CRTC state. As a result dc stream state's src (composition area) && dest (addressable area) was not calculated as expected. This causes set scaling doesn's work. [How] Correctly update CRTC state when setting scaling property. Signed-off-by: hongao diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 3e1ecca72430..a88a6f758748 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -9386,8 +9386,8 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, goto fail; } - if (dm_old_con_state->abm_level != - dm_new_con_state->abm_level) + if (dm_old_con_state->abm_level != dm_new_con_state->abm_level || + dm_old_con_state->scaling != dm_new_con_state->scaling) new_crtc_state->connectors_changed = true; } Hi, This change lgtm, and I also run it in our CI, and from IGT perspective, we are good. Harry, do you have any comment about this change? LGTM Reviewed-by: Harry Wentland Harry Thanks Siqueira Thanks, patch applied to amd-staging-drm-next.
Re: [PATCH] drm: amd: display: Fix memory leakage
On 11/29/22 21:50, Konstantin Meskhidze wrote: This commit fixes memory leakage in dc_construct_ctx() function. Signed-off-by: Konstantin Meskhidze --- drivers/gpu/drm/amd/display/dc/core/dc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index 997ab031f816..359e28d3567e 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -878,6 +878,7 @@ static bool dc_construct_ctx(struct dc *dc, dc_ctx->perf_trace = dc_perf_trace_create(); if (!dc_ctx->perf_trace) { + kfree(dc_ctx); ASSERT_CRITICAL(false); return false; } Reviewed-by: Rodrigo Siqueira And applied to amd-staging-drm-next. Thanks Siqueira
Re: [PATCH] drm/amd/display: Fix set scaling doesn's work
On 11/22/22 06:20, hongao wrote: [Why] Setting scaling does not correctly update CRTC state. As a result dc stream state's src (composition area) && dest (addressable area) was not calculated as expected. This causes set scaling doesn's work. [How] Correctly update CRTC state when setting scaling property. Signed-off-by: hongao diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 3e1ecca72430..a88a6f758748 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -9386,8 +9386,8 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, goto fail; } - if (dm_old_con_state->abm_level != - dm_new_con_state->abm_level) + if (dm_old_con_state->abm_level != dm_new_con_state->abm_level || + dm_old_con_state->scaling != dm_new_con_state->scaling) new_crtc_state->connectors_changed = true; } Hi, This change lgtm, and I also run it in our CI, and from IGT perspective, we are good. Harry, do you have any comment about this change? Thanks Siqueira
Re: [PATCH] drm/amd/display: No need for Null pointer check before kfree
On 12/27/22 13:39, Deepak R Varma wrote: kfree() & vfree() internally performs NULL check on the pointer handed to it and take no action if it indeed is NULL. Hence there is no need for a pre-check of the memory pointer before handing it to kfree()/vfree(). Issue reported by ifnullfree.cocci Coccinelle semantic patch script. Signed-off-by: Deepak R Varma --- drivers/gpu/drm/amd/display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c | 3 +-- drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c index 3ce0ee0d012f..694a9d3d92ae 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c @@ -577,8 +577,7 @@ void dcn3_clk_mgr_construct( void dcn3_clk_mgr_destroy(struct clk_mgr_internal *clk_mgr) { - if (clk_mgr->base.bw_params) - kfree(clk_mgr->base.bw_params); + kfree(clk_mgr->base.bw_params); if (clk_mgr->wm_range_table) dm_helpers_free_gpu_mem(clk_mgr->base.ctx, DC_MEM_ALLOC_TYPE_GART, diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c index 200fcec19186..ba9814f88f48 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn32/dcn32_clk_mgr.c @@ -783,8 +783,7 @@ void dcn32_clk_mgr_construct( void dcn32_clk_mgr_destroy(struct clk_mgr_internal *clk_mgr) { - if (clk_mgr->base.bw_params) - kfree(clk_mgr->base.bw_params); + kfree(clk_mgr->base.bw_params); if (clk_mgr->wm_range_table) dm_helpers_free_gpu_mem(clk_mgr->base.ctx, DC_MEM_ALLOC_TYPE_GART, -- 2.34.1 Hi, Reviewed-by: Rodrigo Siqueira And applied to amd-staging-drm-next. Thanks Siqueira
Re: [PATCH] drm/amd/display: Revert logic for plane modifiers
On 2022-10-19 11:15, Joaquín Ignacio Aramendía wrote: This file was split in commit 5d945cbcd4b16a29d6470a80dfb19738f9a4319f ("drm/amd/display: Create a file dedicated to planes") the logic in dm_plane_format_mod_supported() function got changed by a switch logic. That change broke drm_plane modifiers setting on series 5000 APUs (tested on OXP mini AMD 5800U and HP Dev One 5850U PRO) leading to Gamescope not working as reproted on GitHub[1] To reproduce the issue, enter a TTY and run: $ gamescope -- vkcube With said commit applied it will abort. This one restores the old logic, fixing the issue that affects Gamescope. [1](https://github.com/Plagman/gamescope/issues/624>> Signed-off-by: Joaquín Ignacio Aramendía --- .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 50 +++ 1 file changed, 8 insertions(+), 42 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c index dfd3be49eac8..81c1fc9468b8 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c @@ -1371,6 +1371,8 @@ static bool dm_plane_format_mod_supported(struct drm_plane *plane, const struct drm_format_info *info = drm_format_info(format); struct hw_asic_id asic_id = adev->dm.dc->ctx->asic_id; Hi, First of all, thanks a lot for this patch. You can drop asic_id since it is not used anymore. Thanks Siqueira + int i; + enum dm_micro_swizzle microtile = modifier_gfx9_swizzle_mode(modifier) & 3; if (!info) @@ -1386,49 +1388,13 @@ static bool dm_plane_format_mod_supported(struct drm_plane *plane, return true; } - /* check if swizzle mode is supported by this version of DCN */ - switch (asic_id.chip_family) { - case FAMILY_SI: - case FAMILY_CI: - case FAMILY_KV: - case FAMILY_CZ: - case FAMILY_VI: - /* asics before AI does not have modifier support */ - return false; - case FAMILY_AI: - case FAMILY_RV: - case FAMILY_NV: - case FAMILY_VGH: - case FAMILY_YELLOW_CARP: - case AMDGPU_FAMILY_GC_10_3_6: - case AMDGPU_FAMILY_GC_10_3_7: - switch (AMD_FMT_MOD_GET(TILE, modifier)) { - case AMD_FMT_MOD_TILE_GFX9_64K_R_X: - case AMD_FMT_MOD_TILE_GFX9_64K_D_X: - case AMD_FMT_MOD_TILE_GFX9_64K_S_X: - case AMD_FMT_MOD_TILE_GFX9_64K_D: - return true; - default: - return false; - } - break; - case AMDGPU_FAMILY_GC_11_0_0: - case AMDGPU_FAMILY_GC_11_0_1: - switch (AMD_FMT_MOD_GET(TILE, modifier)) { - case AMD_FMT_MOD_TILE_GFX11_256K_R_X: - case AMD_FMT_MOD_TILE_GFX9_64K_R_X: - case AMD_FMT_MOD_TILE_GFX9_64K_D_X: - case AMD_FMT_MOD_TILE_GFX9_64K_S_X: - case AMD_FMT_MOD_TILE_GFX9_64K_D: - return true; - default: - return false; - } - break; - default: - ASSERT(0); /* Unknown asic */ - break; + /* Check that the modifier is on the list of the plane's supported modifiers. */ + for (i = 0; i < plane->modifier_count; i++) { + if (modifier == plane->modifiers[i]) + break; } + if (i == plane->modifier_count) + return false; /* * For D swizzle the canonical modifier depends on the bpp, so check
Re: [PATCH] drm/amdgpu/dm/mst: Fix incorrect usage of drm_dp_add_payload_part2()
On 2022-10-04 16:24, Lyude Paul wrote: Yikes, it appears somehow I totally made a mistake here. We're currently checking to see if drm_dp_add_payload_part2() returns a non-zero value to indicate success. That's totally wrong though, as this function only returns a zero value on success - not the other way around. So, fix that. Signed-off-by: Lyude Paul Issue: https://gitlab.freedesktop.org/drm/amd/-/issues/2171 Fixes: 4d07b0bc4034 ("drm/display/dp_mst: Move all payload info into the atomic state") --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index b8077fcd4651..00598def5b39 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -297,7 +297,7 @@ bool dm_helpers_dp_mst_send_payload_allocation( clr_flag = MST_ALLOCATE_NEW_PAYLOAD; } - if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, payload)) { + if (enable && drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, payload) == 0) { amdgpu_dm_set_mst_status(>mst_status, set_flag, false); } else { Hi Lyude, Maybe I'm missing something, but I can't find the drm_dp_add_payload_part2() function on amd-staging-drm-next. Which repo are you using? Thanks Siqueira
Re: [PATCH] drm/amd/display: Remove unused struct i2c_id_config_access
On 2022-09-27 09:39, Yuan Can wrote: After commit 5a8132b9f606("drm/amd/display: remove dead dc vbios code"), no one use struct i2c_id_config_access, so remove it. Signed-off-by: Yuan Can --- drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c | 7 --- 1 file changed, 7 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 5d70f9901d13..d380cf98b844 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c +++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c @@ -50,13 +50,6 @@ #define LAST_RECORD_TYPE 0xff #define SMU9_SYSPLL0_ID 0 -struct i2c_id_config_access { - uint8_t bfI2C_LineMux:4; - uint8_t bfHW_EngineID:3; - uint8_t bfHW_Capable:1; - uint8_t ucAccess; -}; - static enum bp_result get_gpio_i2c_info(struct bios_parser *bp, struct atom_i2c_record *record, struct graphics_object_i2c_info *info); Reviewed-by: Rodrigo Siqueira and applied to amd-staging-drm-next. Thanks Siqueira
Re: [PATCH -next] drm/amd/display: Removed unused variable 'sdp_stream_enable'
On 2022-09-30 02:38, Dong Chenchen wrote: Kernel test robot throws below warning -> drivers/gpu/drm/amd/amdgpu/../display/dc/dcn31/dcn31_hpo_dp_stream_encoder.c: In function 'dcn31_hpo_dp_stream_enc_update_dp_info_packets': drivers/gpu/drm/amd/amdgpu/../display/dc/dcn31/dcn31_hpo_dp_stream_encoder.c:439:14: warning: variable 'sdp_stream_enable' set but not used [-Wunused-but-set-variable] 439 | bool sdp_stream_enable = false; Removed unused variable 'sdp_stream_enable'. Reported-by: kernel test robot Signed-off-by: Dong Chenchen --- .../dc/dcn31/dcn31_hpo_dp_stream_encoder.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hpo_dp_stream_encoder.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hpo_dp_stream_encoder.c index 23621ff08c90..7daafbab98da 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hpo_dp_stream_encoder.c +++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_hpo_dp_stream_encoder.c @@ -436,32 +436,28 @@ static void dcn31_hpo_dp_stream_enc_update_dp_info_packets( { struct dcn31_hpo_dp_stream_encoder *enc3 = DCN3_1_HPO_DP_STREAM_ENC_FROM_HPO_STREAM_ENC(enc); uint32_t dmdata_packet_enabled = 0; - bool sdp_stream_enable = false; - if (info_frame->vsc.valid) { + if (info_frame->vsc.valid) enc->vpg->funcs->update_generic_info_packet( enc->vpg, 0, /* packetIndex */ _frame->vsc, true); - sdp_stream_enable = true; - } - if (info_frame->spd.valid) { + + if (info_frame->spd.valid) enc->vpg->funcs->update_generic_info_packet( enc->vpg, 2, /* packetIndex */ _frame->spd, true); - sdp_stream_enable = true; - } - if (info_frame->hdrsmd.valid) { + + if (info_frame->hdrsmd.valid) enc->vpg->funcs->update_generic_info_packet( enc->vpg, 3, /* packetIndex */ _frame->hdrsmd, true); - sdp_stream_enable = true; - } + /* enable/disable transmission of packet(s). * If enabled, packet transmission begins on the next frame */ Thanks a lot for your patch, Reviewed-by: Rodrigo Siqueira and applied to amd-staging-drm-next. Thanks Siqueira
Re: [PATCH] drm: amd: clean up dcn32_fpu.c kernel-doc
On 2022-10-01 00:33, Randy Dunlap wrote: Rectify multiple kernel-doc warnings in dcn32_fpu.c. E.g.: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:247: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst * Finds dummy_latency_index when MCLK switching using firmware based drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:484: warning: Function parameter or member 'phantom_stream' not described in 'dcn32_set_phantom_stream_timing' drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:601: warning: Function parameter or member 'dc' not described in 'dcn32_assign_subvp_pipe' drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:601: warning: Function parameter or member 'context' not described in 'dcn32_assign_subvp_pipe' drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:601: warning: Function parameter or member 'index' not described in 'dcn32_assign_subvp_pipe' drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:2140: warning: Function parameter or member 'dc' not described in 'dcn32_update_bw_bounding_box_fpu' drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:2140: warning: Function parameter or member 'bw_params' not described in 'dcn32_update_bw_bounding_box_fpu' drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/dcn32_fpu.c:2140: warning: expecting prototype for dcn32_update_bw_bounding_box(). Prototype was for dcn32_update_bw_bounding_box_fpu() instead Reported-by: kernel test robot Signed-off-by: Randy Dunlap Cc: George Shen Cc: Alvin Lee Cc: Nevenko Stupar Cc: Harry Wentland Cc: Leo Li Cc: Rodrigo Siqueira Cc: amd-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Cc: Alex Deucher Cc: Christian König Cc: "Pan, Xinhui" --- drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c | 116 -- 1 file changed, 49 insertions(+), 67 deletions(-) --- a/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn32/dcn32_fpu.c @@ -243,7 +243,7 @@ void dcn32_build_wm_range_table_fpu(stru clk_mgr->base.bw_params->wm_table.nv_entries[WM_D].pmfw_breakdown.max_uclk = 0x; } -/** +/* * Finds dummy_latency_index when MCLK switching using firmware based * vblank stretch is enabled. This function will iterate through the * table of dummy pstate latencies until the lowest value that allows @@ -290,15 +290,14 @@ int dcn32_find_dummy_latency_index_for_f /** * dcn32_helper_populate_phantom_dlg_params - Get DLG params for phantom pipes * and populate pipe_ctx with those params. - * - * This function must be called AFTER the phantom pipes are added to context - * and run through DML (so that the DLG params for the phantom pipes can be - * populated), and BEFORE we program the timing for the phantom pipes. - * * @dc: [in] current dc state * @context: [in] new dc state * @pipes: [in] DML pipe params array * @pipe_cnt: [in] DML pipe count + * + * This function must be called AFTER the phantom pipes are added to context + * and run through DML (so that the DLG params for the phantom pipes can be + * populated), and BEFORE we program the timing for the phantom pipes. */ void dcn32_helper_populate_phantom_dlg_params(struct dc *dc, struct dc_state *context, @@ -331,8 +330,9 @@ void dcn32_helper_populate_phantom_dlg_p } /** - * *** - * dcn32_predict_pipe_split: Predict if pipe split will occur for a given DML pipe + * dcn32_predict_pipe_split - Predict if pipe split will occur for a given DML pipe + * @context: [in] New DC state to be programmed + * @pipe_e2e: [in] DML pipe end to end context * * This function takes in a DML pipe (pipe_e2e) and predicts if pipe split is required (both * ODM and MPC). For pipe split, ODM combine is determined by the ODM mode, and MPC combine is @@ -343,12 +343,7 @@ void dcn32_helper_populate_phantom_dlg_p * - MPC combine is only chosen if there is no ODM combine requirements / policy in place, and * MPC is required * - * @param [in]: context: New DC state to be programmed - * @param [in]: pipe_e2e: DML pipe end to end context - * - * @return: Number of splits expected (1 for 2:1 split, 3 for 4:1 split, 0 for no splits). - * - * *** + * Return: Number of splits expected (1 for 2:1 split, 3 for 4:1 split, 0 for no splits). */ uint8_t dcn32_predict_pipe_split(struct dc_state *context, display_e2e_pipe_params_st *pipe_e2e) @@ -504,7 +499,14 @@ void insert_entry_into_table_sorted(stru } /** - * dcn32_set_phantom_stream_timing: Set timing params for the phantom stream + * dcn32_set_phantom_stream_timing - Set
Re: [PATCH V3 46/47] drm/amd/display: Fix failures of disabling primary plans
Hi Michel, First of all, thanks a lot for your review. I want to take this opportunity to discuss this topic in more depth and learn more from you and others. +(Nick, Leo, Daniel, Mark, Dave, Sean, Simon) On 2022-09-15 04:55, Michel Dänzer wrote: On 2022-09-14 22:08, Alex Hung wrote: On 2022-09-14 10:55, Michel Dänzer wrote: On 2022-09-14 18:30, Alex Hung wrote: On 2022-09-14 07:40, Michel Dänzer wrote: On 2022-09-14 15:31, Michel Dänzer wrote: On 2022-09-14 07:10, Wayne Lin wrote: From: Alex Hung [Why & How] This fixes kernel errors when IGT disables primary planes during the tests kms_universal_plane::functional_test_pipe/pageflip_test_pipe. NAK. This essentially reverts commit b836a274b797 ("drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is") (except that it goes even further and completely removes the requirement for any HW plane to be enabled when the HW cursor is), so it would reintroduce the issues described in that commit log. Actually not exactly the same issues, due to going even further than reverting my fix. Instead, the driver will claim that an atomic commit which enables the CRTC and the cursor plane, while leaving all other KMS planes disabled, succeeds. But the HW cursor will not actually be visible. I did not observe problems with cursors (GNOME 42.4 - desktop and youtube/mpv video playback: windowed/fullscreen). Are there steps to reproduce cursor problems? As described in my last follow-up e-mail: An atomic KMS commit which enables the CRTC and the cursor plane, but disables all other KMS planes for the CRTC. The commit will succeed, but will not result in the HW cursor being actually visible. (I don't know of any specific application or test which exercises this) Did you mean cursor plane depends on primary plane (i.e. no primary plane = no visible HW cursor)? Sort of. I understand the HW cursor isn't an actual separate plane in AMD HW. Instead, the HW cursor can be displayed as part of any other HW plane. This means that the HW cursor can only be visible if any other plane is enabled. The commit that you mentioned (b836a274b797) was created to address some issues reported by the user. Please, correct me if I'm wrong, but the original issue could be reproduced by following these steps on Gnome with Ellesmere: 1. Lock the screen and wait for suspending; 2. Wake up the system a few minutes later; 3. See two cursors, one that can be used and another one that is not working. I tried to reproduce this issue using an Ellesmere board (+this patchset), and Daniel has tested it in multiple ASICs; we never repro that issue (Gnome and ChromeOS). It is not evident to me why we cannot reproduce this problem. Do you have some suggestions? If we find a case showing this bug, we can add it as part of our tests. I feel that the commit b836a274b797 is not directly related to that specific bug. I mean, it might make sense to have it, but for other reasons. If there is no primary plane, what scenario would it be required to draw a cursor? If this is a rare case, would it still be a concern? Yes. In the KMS API, the cursor plane is like any other plane. A Wayland compositor or other user space may legitimately try to display something (not necessarily a "cursor") using only the cursor plane. The driver must accurately signal that this cannot work. The established way to do so (if a bit indirectly) is to require the primary plane to be enabled whenever the CRTC is. Is there any real case for this scenario? Is this scenario strong enough to say that a driver does not support CRTC enabled without planes? Also see the commit log of bc92c06525e5 ("drm/amd/display: Allow commits with no planes active"). Does it mean dm_crtc_helper_atomic_check does not need to return -EINVAL if there is no active cursor because there are no cursors to be shown anyways, [...] This was considered in the review discussion for b836a274b797 ("drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is"), see https://patchwork.freedesktop.org/patch/387230/ . TL;DR: There were already other KMS drivers requiring the primary plane to be enabled whenever the CRTC is, and there's a special case for that in atomic_remove_fb. iirc, this requiring is only available in drm_simple_kms_helper, and drivers under the tiny folder are the only ones using it. Adding another special case for the cursor plane would make things much more complicated for common DRM code and user space (and possibly even introduce issues which cannot be solved at all). If IGT tests disable the primary plane while leaving the CRTC enabled, those tests are broken and need to be fixed instead. There are IGT cursor tests fixed by this: igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size It also reduces amdgpu workaround in IGT's kms_concurrent:
Re: [PATCH 0/5] drm/amd/display: Reduce stack usage for clang
On 2022-09-12 18:02, Nathan Chancellor wrote: Hi Rodrigo, On Mon, Sep 12, 2022 at 05:50:31PM -0400, Rodrigo Siqueira Jordao wrote: On 2022-08-30 16:34, Nathan Chancellor wrote: Hi all, This series aims to address the following warnings, which are visible when building x86_64 allmodconfig with clang after commit 3876a8b5e241 ("drm/amd/display: Enable building new display engine with KCOV enabled"). drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_mode_vba_30.c:3542:6: error: stack frame size (2200) exceeds limit (2048) in 'dml30_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml30_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c:3908:6: error: stack frame size (2216) exceeds limit (2048) in 'dml31_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml31_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1721:6: error: stack frame size (2152) exceeds limit (2048) in 'dml32_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml32_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. This series is based on commit b3235e8635e1 ("drm/amd/display: clean up some inconsistent indentings"). These warnings are fatal for allmodconfig due to CONFIG_WERROR so ideally, I would like to see these patches cherry-picked to a branch targeting mainline to allow our builds to go back to green. However, since this series is not exactly trivial in size, I can understand not wanting to apply these to mainline during the -rc cycle. If they cannot be cherry-picked to mainline, I can add a patch raising the value of -Wframe-larger-than for these files that can be cherry-picked to 6.0/mainline then add a revert of that change as the last patch in the stack so everything goes back to normal for -next/6.1. I am open to other options though! I have built this series against clang 16.0.0 (ToT) and GCC 12.2.0 for x86_64. It has seen no runtime testing, as my only test system with AMD graphics is a Renoir one, which as far as I understand it uses DCN 2.1. Nathan Chancellor (5): drm/amd/display: Reduce number of arguments of dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport() drm/amd/display: Reduce number of arguments of dml32_CalculatePrefetchSchedule() drm/amd/display: Reduce number of arguments of dml31's CalculateWatermarksAndDRAMSpeedChangeSupport() drm/amd/display: Reduce number of arguments of dml31's CalculateFlipSchedule() drm/amd/display: Mark dml30's UseMinimumDCFCLK() as noinline for stack usage .../dc/dml/dcn30/display_mode_vba_30.c| 2 +- .../dc/dml/dcn31/display_mode_vba_31.c| 420 +- .../dc/dml/dcn32/display_mode_vba_32.c| 236 +++--- .../dc/dml/dcn32/display_mode_vba_util_32.c | 323 ++ .../dc/dml/dcn32/display_mode_vba_util_32.h | 51 +-- 5 files changed, 318 insertions(+), 714 deletions(-) base-commit: b3235e8635e1dd7ac1a27a73330e9880dfe05154 Hi Nathan, First of all, thanks a lot for your patchset! Sorry for the delay; it took me more time than I expected to review and run a couple of tests in this patchset (most of them were IGT). Anyway, I'm good with this change; this series is: Reviewed-by: Rodrigo Siqueira And I applied it to amd-staging-drm-next. We will run some extra tests this week; if we find some issues, I'll debug them. Also, thanks, Maíra, for checking this patch as well. No worries on the delay, the series is not exactly the smallest one I have ever sent :) While the changes were mostly mechanical, I could have definitely messed something up and I appreciate you taking the time to review it and run it through some tests. Please let me know if I can be of further assistance on that front. If you have any thoughts on the blurb I had in the cover letter around how to handle the warnings this series resolves with regards to mainline, I would love to hear them. Actually, I think it will be valuable to add a kernel-doc about DML and create a Troubleshoot section where we can have some of the things you described in the cover letter as part of this doc. I'll work on that, and when I have a V1, I'll Cc you. Cheers, Nathan
Re: [PATCH 0/5] drm/amd/display: Reduce stack usage for clang
On 2022-08-30 16:34, Nathan Chancellor wrote: Hi all, This series aims to address the following warnings, which are visible when building x86_64 allmodconfig with clang after commit 3876a8b5e241 ("drm/amd/display: Enable building new display engine with KCOV enabled"). drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn30/display_mode_vba_30.c:3542:6: error: stack frame size (2200) exceeds limit (2048) in 'dml30_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml30_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c:3908:6: error: stack frame size (2216) exceeds limit (2048) in 'dml31_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml31_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1721:6: error: stack frame size (2152) exceeds limit (2048) in 'dml32_ModeSupportAndSystemConfigurationFull' [-Werror,-Wframe-larger-than] void dml32_ModeSupportAndSystemConfigurationFull(struct display_mode_lib *mode_lib) ^ 1 error generated. This series is based on commit b3235e8635e1 ("drm/amd/display: clean up some inconsistent indentings"). These warnings are fatal for allmodconfig due to CONFIG_WERROR so ideally, I would like to see these patches cherry-picked to a branch targeting mainline to allow our builds to go back to green. However, since this series is not exactly trivial in size, I can understand not wanting to apply these to mainline during the -rc cycle. If they cannot be cherry-picked to mainline, I can add a patch raising the value of -Wframe-larger-than for these files that can be cherry-picked to 6.0/mainline then add a revert of that change as the last patch in the stack so everything goes back to normal for -next/6.1. I am open to other options though! I have built this series against clang 16.0.0 (ToT) and GCC 12.2.0 for x86_64. It has seen no runtime testing, as my only test system with AMD graphics is a Renoir one, which as far as I understand it uses DCN 2.1. Nathan Chancellor (5): drm/amd/display: Reduce number of arguments of dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport() drm/amd/display: Reduce number of arguments of dml32_CalculatePrefetchSchedule() drm/amd/display: Reduce number of arguments of dml31's CalculateWatermarksAndDRAMSpeedChangeSupport() drm/amd/display: Reduce number of arguments of dml31's CalculateFlipSchedule() drm/amd/display: Mark dml30's UseMinimumDCFCLK() as noinline for stack usage .../dc/dml/dcn30/display_mode_vba_30.c| 2 +- .../dc/dml/dcn31/display_mode_vba_31.c| 420 +- .../dc/dml/dcn32/display_mode_vba_32.c| 236 +++--- .../dc/dml/dcn32/display_mode_vba_util_32.c | 323 ++ .../dc/dml/dcn32/display_mode_vba_util_32.h | 51 +-- 5 files changed, 318 insertions(+), 714 deletions(-) base-commit: b3235e8635e1dd7ac1a27a73330e9880dfe05154 Hi Nathan, First of all, thanks a lot for your patchset! Sorry for the delay; it took me more time than I expected to review and run a couple of tests in this patchset (most of them were IGT). Anyway, I'm good with this change; this series is: Reviewed-by: Rodrigo Siqueira And I applied it to amd-staging-drm-next. We will run some extra tests this week; if we find some issues, I'll debug them. Also, thanks, Maíra, for checking this patch as well. Best Regards, Siqueira
Re: [PATCH linux-next] drm/amd/display: Remove the unneeded result variable
On 2022-09-02 03:54, cgel@gmail.com wrote: From: zhang songyi Return the enable_link_dp() directly instead of storing it in another redundant variable. Reported-by: Zeal Robot Signed-off-by: zhang songyi --- drivers/gpu/drm/amd/display/dc/core/dc_link.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c b/drivers/gpu/drm/amd/display/dc/core/dc_link.c index f9b798b7933c..4ab27e231337 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c @@ -2077,11 +2077,7 @@ static enum dc_status enable_link_edp( struct dc_state *state, struct pipe_ctx *pipe_ctx) { - enum dc_status status; - - status = enable_link_dp(state, pipe_ctx); - - return status; + return enable_link_dp(state, pipe_ctx); } static enum dc_status enable_link_dp_mst( LGTM, Reviewed-by: Rodrigo Siqueira and applied to amd-staging-drm-next. Thanks Siqueira
Re: [PATCH] drm/amd/display: fix memory leak when using debugfs_lookup()
On 2022-09-06 11:06, Greg Kroah-Hartman wrote: On Tue, Sep 06, 2022 at 10:52:28AM -0400, Rodrigo Siqueira Jordao wrote: On 2022-09-02 09:10, Greg Kroah-Hartman wrote: On Fri, Sep 02, 2022 at 03:01:05PM +0200, Greg Kroah-Hartman wrote: When calling debugfs_lookup() the result must have dput() called on it, otherwise the memory will leak over time. Fix this up by properly calling dput(). Cc: Harry Wentland Cc: Leo Li Cc: Rodrigo Siqueira Cc: Alex Deucher Cc: "Christian König" Cc: "Pan, Xinhui" Cc: David Airlie Cc: Daniel Vetter Cc: Wayne Lin Cc: hersen wu Cc: Wenjing Liu Cc: Patrik Jakobsson Cc: Thelford Williams Cc: Fangzhi Zuo Cc: Yongzhi Liu Cc: Mikita Lipski Cc: Jiapeng Chong Cc: Bhanuprakash Modem Cc: Sean Paul Cc: amd-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Greg Kroah-Hartman --- Despite a zillion cc: items, I forgot to cc: stable on this. Can the maintainer add that here, or do you all want me to resend it with that item added? thanks, greg k-h Hi Greg, Reviewed-by: Rodrigo Siqueira Is 'Cc: sta...@vger.kernel.org' enough here? I can make this change before I merge it into amd-staging-drm-next. Yes, please add the cc: stable@ line to the body of the patch, sorry I forgot that. Change applied to amd-staging-drm-next, with the Cc to the stable branch. Thanks Siqueira thanks, greg k-h
Re: [PATCH] drm/amd/display: fix memory leak when using debugfs_lookup()
On 2022-09-02 09:10, Greg Kroah-Hartman wrote: On Fri, Sep 02, 2022 at 03:01:05PM +0200, Greg Kroah-Hartman wrote: When calling debugfs_lookup() the result must have dput() called on it, otherwise the memory will leak over time. Fix this up by properly calling dput(). Cc: Harry Wentland Cc: Leo Li Cc: Rodrigo Siqueira Cc: Alex Deucher Cc: "Christian König" Cc: "Pan, Xinhui" Cc: David Airlie Cc: Daniel Vetter Cc: Wayne Lin Cc: hersen wu Cc: Wenjing Liu Cc: Patrik Jakobsson Cc: Thelford Williams Cc: Fangzhi Zuo Cc: Yongzhi Liu Cc: Mikita Lipski Cc: Jiapeng Chong Cc: Bhanuprakash Modem Cc: Sean Paul Cc: amd-...@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: Greg Kroah-Hartman --- Despite a zillion cc: items, I forgot to cc: stable on this. Can the maintainer add that here, or do you all want me to resend it with that item added? thanks, greg k-h Hi Greg, Reviewed-by: Rodrigo Siqueira Is 'Cc: sta...@vger.kernel.org' enough here? I can make this change before I merge it into amd-staging-drm-next. Thanks Siqueira
Re: [PATCH v2] drm/amd/display: fix indentation in commit_planes_for_stream()
On 2022-09-01 10:15, Hamza Mahfooz wrote: Address the following warning: drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:3508:9: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation] 3508 | if (update_type != UPDATE_TYPE_FAST) | ^~ drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:3510:17: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’ 3510 | if (update_type != UPDATE_TYPE_FAST) | ^~ Signed-off-by: Hamza Mahfooz --- v2: implement feedback from Alvin --- drivers/gpu/drm/amd/display/dc/core/dc.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c index b49237390cce..9860bf38c547 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c @@ -3507,9 +3507,10 @@ static void commit_planes_for_stream(struct dc *dc, if (update_type != UPDATE_TYPE_FAST) dc->hwss.post_unlock_program_front_end(dc, context); - if (update_type != UPDATE_TYPE_FAST) - if (dc->hwss.commit_subvp_config) - dc->hwss.commit_subvp_config(dc, context); + + if (update_type != UPDATE_TYPE_FAST) + if (dc->hwss.commit_subvp_config) + dc->hwss.commit_subvp_config(dc, context); /* Since phantom pipe programming is moved to post_unlock_program_front_end, * move the SubVP lock to after the phantom pipes have been setup Reviewed-by: Rodrigo Siqueira and applied to amd-staging-drm-next. Thanks Siqueira
Re: [PATCH v5] drm: Add initial ci/ subdirectory
Hi Tomeu, First of all, nice patch! I just saw it, and I have some basic questions (I don't understand many of these CI details). I also CC some CI folks from the display team at AMD. On 2022-07-26 14:16, Tomeu Vizoso wrote: And use it to store expectations about what the DRM drivers are supposed to pass in the IGT test suite. Also include a configuration file that points to the out-of-tree CI scripts. By storing the test expectations along the code we can make sure both stay in sync with each other, and so we can know when a code change breaks those expectations. This will allow all contributors to drm to reuse the infrastructure already in gitlab.freedesktop.org to test the driver on several generations of the hardware. v2: - Fix names of result expectation files to match SoC - Don't execute tests that are going to skip on all boards v3: - Remove tracking of dmesg output during test execution v4: - Move up to drivers/gpu/drm - Add support for a bunch of other drivers - Explain how to incorporate fixes for CI from a ${TARGET_BRANCH}-external-fixes branch - Remove tests that pass from expected results file, to reduce the size of in-tree files - Add docs about how to deal with outages in automated testing labs - Specify the exact SHA of the CI scripts to be used v5: - Remove unneeded skips from Meson expectations file - Use a more advanced runner that detects flakes automatically - Use a more succint format for the expectations - Run many more tests (and use sharding to finish in time) - Use skip lists to avoid hanging machines - Add some build testing - Build IGT in each pipeline for faster uprevs - List failures in the GitLab UI Signed-off-by: Tomeu Vizoso Reviewed-by: Neil Armstrong --- Documentation/gpu/automated_testing.rst | 84 ++ drivers/gpu/drm/ci/amdgpu-stoney-fails.txt| 13 +++ drivers/gpu/drm/ci/amdgpu-stoney-flakes.txt | 20 + drivers/gpu/drm/ci/amdgpu-stoney-skips.txt| 2 + drivers/gpu/drm/ci/gitlab-ci.yml | 13 +++ drivers/gpu/drm/ci/i915-amly-flakes.txt | 32 +++ drivers/gpu/drm/ci/i915-amly-skips.txt| 2 + drivers/gpu/drm/ci/i915-apl-fails.txt | 29 +++ drivers/gpu/drm/ci/i915-apl-flakes.txt| 1 + drivers/gpu/drm/ci/i915-apl-skips.txt | 2 + drivers/gpu/drm/ci/i915-cml-flakes.txt| 36 drivers/gpu/drm/ci/i915-glk-flakes.txt| 40 + drivers/gpu/drm/ci/i915-glk-skips.txt | 2 + drivers/gpu/drm/ci/i915-kbl-fails.txt | 8 ++ drivers/gpu/drm/ci/i915-kbl-flakes.txt| 24 ++ drivers/gpu/drm/ci/i915-kbl-skips.txt | 2 + drivers/gpu/drm/ci/i915-tgl-fails.txt | 19 drivers/gpu/drm/ci/i915-tgl-flakes.txt| 6 ++ drivers/gpu/drm/ci/i915-tgl-skips.txt | 8 ++ drivers/gpu/drm/ci/i915-whl-fails.txt | 30 +++ drivers/gpu/drm/ci/i915-whl-flakes.txt| 1 + drivers/gpu/drm/ci/mediatek-mt8173-fails.txt | 29 +++ drivers/gpu/drm/ci/mediatek-mt8183-fails.txt | 10 +++ drivers/gpu/drm/ci/mediatek-mt8183-flakes.txt | 14 +++ drivers/gpu/drm/ci/meson-g12b-fails.txt | 5 ++ drivers/gpu/drm/ci/meson-g12b-flakes.txt | 4 + drivers/gpu/drm/ci/msm-apq8016-fails.txt | 15 drivers/gpu/drm/ci/msm-apq8016-flakes.txt | 4 + drivers/gpu/drm/ci/msm-apq8096-fails.txt | 2 + drivers/gpu/drm/ci/msm-apq8096-flakes.txt | 4 + drivers/gpu/drm/ci/msm-apq8096-skips.txt | 2 + drivers/gpu/drm/ci/msm-sc7180-fails.txt | 22 + drivers/gpu/drm/ci/msm-sc7180-flakes.txt | 14 +++ drivers/gpu/drm/ci/msm-sc7180-skips.txt | 18 drivers/gpu/drm/ci/msm-sdm845-fails.txt | 44 ++ drivers/gpu/drm/ci/msm-sdm845-flakes.txt | 33 +++ drivers/gpu/drm/ci/msm-sdm845-skips.txt | 2 + drivers/gpu/drm/ci/rockchip-rk3288-fails.txt | 75 drivers/gpu/drm/ci/rockchip-rk3288-flakes.txt | 5 ++ drivers/gpu/drm/ci/rockchip-rk3288-skips.txt | 46 ++ drivers/gpu/drm/ci/rockchip-rk3399-fails.txt | 86 +++ drivers/gpu/drm/ci/rockchip-rk3399-flakes.txt | 25 ++ drivers/gpu/drm/ci/rockchip-rk3399-skips.txt | 5 ++ drivers/gpu/drm/ci/virtio_gpu-none-fails.txt | 38 drivers/gpu/drm/ci/virtio_gpu-none-flakes.txt | 0 drivers/gpu/drm/ci/virtio_gpu-none-skips.txt | 6 ++ 46 files changed, 882 insertions(+) create mode 100644 Documentation/gpu/automated_testing.rst create mode 100644 drivers/gpu/drm/ci/amdgpu-stoney-fails.txt create mode 100644 drivers/gpu/drm/ci/amdgpu-stoney-flakes.txt create mode 100644 drivers/gpu/drm/ci/amdgpu-stoney-skips.txt create mode 100644 drivers/gpu/drm/ci/gitlab-ci.yml create mode 100644 drivers/gpu/drm/ci/i915-amly-flakes.txt create mode 100644 drivers/gpu/drm/ci/i915-amly-skips.txt create mode 100644
Re: [PATCH v2 4/4] Documentation/gpu/amdgpu/amdgpu_dm: add DM docs for pixel blend mode
On 2022-08-04 11:01, Melissa Wen wrote: AMD GPU display manager (DM) maps DRM pixel blend modes (None, Pre-multiplied, Coverage) to MPC hw blocks through blend configuration options. Describe relevant elements and how to set and test them to get the expected DRM blend mode on DCN hw. v2: - add ref tag (Tales) Signed-off-by: Melissa Wen Reviewed-by: Tales Aparecida --- .../gpu/amdgpu/display/display-manager.rst| 98 +++ Documentation/gpu/drm-kms.rst | 2 + 2 files changed, 100 insertions(+) diff --git a/Documentation/gpu/amdgpu/display/display-manager.rst b/Documentation/gpu/amdgpu/display/display-manager.rst index 88e2c08c7014..b7abb18cfc82 100644 --- a/Documentation/gpu/amdgpu/display/display-manager.rst +++ b/Documentation/gpu/amdgpu/display/display-manager.rst @@ -83,3 +83,101 @@ schemas. **DCN 3.0 family color caps and mapping** .. kernel-figure:: dcn3_cm_drm_current.svg + +Blend Mode Properties += + +Pixel blend mode is a DRM plane composition property of :c:type:`drm_plane` used to +describes how pixels from a foreground plane (fg) are composited with the +background plane (bg). Here, we present main concepts of DRM blend mode to help +to understand how this property is mapped to AMD DC interface. See more about +this DRM property and the alpha blending equations in :ref:`DRM Plane +Composition Properties `. + +Basically, a blend mode sets the alpha blending equation for plane +composition that fits the mode in which the alpha channel affects the state of +pixel color values and, therefore, the resulted pixel color. For +example, consider the following elements of the alpha blending equation: + +- *fg.rgb*: Each of the RGB component values from the foreground's pixel. +- *fg.alpha*: Alpha component value from the foreground's pixel. +- *bg.rgb*: Each of the RGB component values from the background. +- *plane_alpha*: Plane alpha value set by the **plane "alpha" property**, see + more in :ref:`DRM Plane Composition Properties `. + +in the basic alpha blending equation:: + + out.rgb = alpha * fg.rgb + (1 - alpha) * bg.rgb + +the alpha channel value of each pixel in a plane is ignored and only the plane +alpha affects the resulted pixel color values. + +DRM has three blend mode to define the blend formula in the plane composition: + +* **None**: Blend formula that ignores the pixel alpha. + +* **Pre-multiplied**: Blend formula that assumes the pixel color values in a + plane was already pre-multiplied by its own alpha channel before storage. + +* **Coverage**: Blend formula that assumes the pixel color values were not + pre-multiplied with the alpha channel values. + +and pre-multiplied is the default pixel blend mode, that means, when no blend +mode property is created or defined, DRM considers the plane's pixels has +pre-multiplied color values. On IGT GPU tools, the kms_plane_alpha_blend test +provides a set of subtests to verify plane alpha and blend mode properties. + +The DRM blend mode and its elements are then mapped by AMDGPU display manager +(DM) to program the blending configuration of the Multiple Pipe/Plane Combined +(MPC), as follows: + +.. kernel-doc:: drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h + :doc: mpc-overview + +.. kernel-doc:: drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h + :functions: mpcc_blnd_cfg + +Therefore, the blending configuration for a single MPCC instance on the MPC +tree is defined by :c:type:`mpcc_blnd_cfg`, where +:c:type:`pre_multiplied_alpha` is the alpha pre-multiplied mode flag used to +set :c:type:`MPCC_ALPHA_MULTIPLIED_MODE`. It controls whether alpha is +multiplied (true/false), being only true for DRM pre-multiplied blend mode. +:c:type:`mpcc_alpha_blend_mode` defines the alpha blend mode regarding pixel +alpha and plane alpha values. It sets one of the three modes for +:c:type:`MPCC_ALPHA_BLND_MODE`, as described below. + +.. kernel-doc:: drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h + :functions: mpcc_alpha_blend_mode + +DM then maps the elements of `enum mpcc_alpha_blend_mode` to those in the DRM +blend formula, as follows: + +* *MPC pixel alpha* matches *DRM fg.alpha* as the alpha component value + from the plane's pixel +* *MPC global alpha* matches *DRM plane_alpha* when the pixel alpha should + be ignored and, therefore, pixel values are not pre-multiplied +* *MPC global gain* assumes *MPC global alpha* value when both *DRM + fg.alpha* and *DRM plane_alpha* participate in the blend equation + +In short, *fg.alpha* is ignored by selecting +:c:type:`MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA`. On the other hand, (plane_alpha * +fg.alpha) component becomes available by selecting +:c:type:`MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN`. And the +:c:type:`MPCC_ALPHA_MULTIPLIED_MODE` defines if the pixel color values are +pre-multiplied by alpha or not. + +Blend configuration flow + + +The alpha blending equation is configured from DRM to
Re: [PATCH v2 3/4] drm/amd/display: add doc entries for MPC blending configuration
On 2022-08-04 11:01, Melissa Wen wrote: Describe structs and enums used to set blend mode properties to MPC blocks. Some pieces of information are already available as code comments, and were just formatted. Others were collected and summarised from discussions on AMD issue tracker[1][2]. [1] https://gitlab.freedesktop.org/drm/amd/-/issues/1734 [2] https://gitlab.freedesktop.org/drm/amd/-/issues/1769 v2: - fix typos (Tales) - add MPCC to MPC entry in the glossary Signed-off-by: Melissa Wen Reviewed-by: Tales Aparecida --- .../gpu/amdgpu/display/dc-glossary.rst| 2 +- drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h | 91 --- 2 files changed, 78 insertions(+), 15 deletions(-) diff --git a/Documentation/gpu/amdgpu/display/dc-glossary.rst b/Documentation/gpu/amdgpu/display/dc-glossary.rst index 116f5f0942fd..0b0ffd428dd2 100644 --- a/Documentation/gpu/amdgpu/display/dc-glossary.rst +++ b/Documentation/gpu/amdgpu/display/dc-glossary.rst @@ -170,7 +170,7 @@ consider asking in the amdgfx and update this page. MC Memory Controller -MPC +MPC/MPCC Multiple pipes and plane combine MPO diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h index 5097037e3962..8d86159d9de0 100644 --- a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h @@ -22,6 +22,16 @@ * */ +/** + * DOC: mpc-overview + * + * Multiple Pipe/Plane Combined (MPC) is a component in the hardware pipeline + * that performs blending of multiple planes, using global and per-pixel alpha. + * It also performs post-blending color correction operations according to the + * hardware capabilities, such as color transformation matrix and gamma 1D and + * 3D LUT. + */ + #ifndef __DC_MPCC_H__ #define __DC_MPCC_H__ @@ -48,14 +58,39 @@ enum mpcc_blend_mode { MPCC_BLEND_MODE_TOP_BOT_BLENDING }; +/** + * enum mpcc_alpha_blend_mode - define the alpha blend mode regarding pixel + * alpha and plane alpha values + */ enum mpcc_alpha_blend_mode { + /** +* @MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA: per pixel alpha using DPP +* alpha value +*/ MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA, + /** +* @MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN: per +* pixel alpha using DPP alpha value multiplied by a global gain (plane +* alpha) +*/ MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN, + /** +* @MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA: global alpha value, ignores +* pixel alpha and consider only plane alpha +*/ MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA }; -/* - * MPCC blending configuration +/** + * struct mpcc_blnd_cfg - MPCC blending configuration + * + * @black_color: background color + * @alpha_mode: alpha blend mode (MPCC_ALPHA_BLND_MODE) + * @pre_multiplied_alpha: whether pixel color values were pre-multiplied by the + * alpha channel (MPCC_ALPHA_MULTIPLIED_MODE) + * @global_gain: used when blend mode considers both pixel alpha and plane + * alpha value and assumes the global alpha value. + * @global_alpha: plane alpha value */ struct mpcc_blnd_cfg { struct tg_color black_color;/* background color */ @@ -107,8 +142,15 @@ struct mpc_dwb_flow_control { int flow_ctrl_cnt1; }; -/* - * MPCC connection and blending configuration for a single MPCC instance. +/** + * struct mpcc - MPCC connection and blending configuration for a single MPCC instance. + * @mpcc_id: MPCC physical instance + * @dpp_id: DPP input to this MPCC + * @mpcc_bot: pointer to bottom layer MPCC. NULL when not connected. + * @blnd_cfg: the blending configuration for this MPCC + * @sm_cfg: stereo mix setting for this MPCC + * @shared_bottom: if MPCC output to both OPP and DWB endpoints, true. Otherwise, false. + * * This struct is used as a node in an MPC tree. */ struct mpcc { @@ -120,8 +162,12 @@ struct mpcc { bool shared_bottom; /* TRUE if MPCC output to both OPP and DWB endpoints, else FALSE */ }; -/* - * MPC tree represents all MPCC connections for a pipe. +/** + * struct mpc_tree - MPC tree represents all MPCC connections for a pipe. + * + * @opp_id: the OPP instance that owns this MPC tree + * @opp_list: the top MPCC layer of the MPC tree that outputs to OPP endpoint + * */ struct mpc_tree { int opp_id; /* The OPP instance that owns this MPC tree */ @@ -149,13 +195,18 @@ struct mpcc_state { uint32_t busy; }; +/** + * struct mpc_funcs - funcs + */ struct mpc_funcs { void (*read_mpcc_state)( struct mpc *mpc, int mpcc_inst, struct mpcc_state *s); - /* + /** +* @insert_plane: +* * Insert DPP into MPC tree based on specified blending position.
Re: [PATCH v2 2/4] Documentation/amdgpu/display: add DC color caps info
On 2022-08-04 11:01, Melissa Wen wrote: Add details about color correction capabilities and explain a bit about differences between DC hw generations and also how they are mapped between DRM and DC interface. Two schemas for DCN 2.0 and 3.0 (converted to svg from the original png) is included to illustrate it. They were obtained from a discussion[1] in the amd-gfx mailing list. [1] https://lore.kernel.org/amd-gfx/20220422142811.dm6vtk6v64jcw...@mail.igalia.com/>> v1: - remove redundant comments (Harry) - fix typos (Harry) v2: - reword introduction of color section - add co-dev tag for Harry - who provided most of the info - fix typos (Tales) - describe missing struct parameters (Tales and Siqueira) Co-developed-by: Harry Wentland Signed-off-by: Harry Wentland Signed-off-by: Melissa Wen Reviewed-by: Tales Aparecida --- .../amdgpu/display/dcn2_cm_drm_current.svg| 1370 +++ .../amdgpu/display/dcn3_cm_drm_current.svg| 1529 + .../gpu/amdgpu/display/display-manager.rst| 34 + drivers/gpu/drm/amd/display/dc/dc.h | 77 +- 4 files changed, 2997 insertions(+), 13 deletions(-) create mode 100644 Documentation/gpu/amdgpu/display/dcn2_cm_drm_current.svg create mode 100644 Documentation/gpu/amdgpu/display/dcn3_cm_drm_current.svg diff --git a/Documentation/gpu/amdgpu/display/dcn2_cm_drm_current.svg b/Documentation/gpu/amdgpu/display/dcn2_cm_drm_current.svg new file mode 100644 index ..315ffc5a1a4b --- /dev/null +++ b/Documentation/gpu/amdgpu/display/dcn2_cm_drm_current.svg @@ -0,0 +1,1370 @@ + + + +http://www.inkscape.org/namespaces/inkscape>> + xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd>> + xmlns="http://www.w3.org/2000/svg>> + xmlns:svg="http://www.w3.org/2000/svg>> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +Matrix +1D LUT +3D LUT +Unpacking +Other +drm_framebuffer +format +drm_plane +drm_crtc +Stream +MPC +DPP + +Blender +Degamma +CTM +Gamma +format +bias_and_scale +color space matrix +input_csc_color_matrix +in_transfer_func +hdr_mult +gamut_remap_matrix +in_shaper_func +lut3d_func +blend_tf +Blender +gamut_remap_matrix +func_shaper +lut3d_func +out_transfer_func +csc_color_matrix +bit_depth_param +clamping +output_color_space +Plane +Legend +DCN 2.0 +DC Interface +DRM Interface + +CNVC +Input CSC +DeGammaRAM and ROM(sRGB, BT2020 +HDR Multiply +Gamut Remap +Shaper LUTRAM +3D LUTRAM +Blend Gamma +Blender +GammaRAM +OCSC + + +color_encoding + +pixel_blend_mode + +color_range + + + + + + + + + + + + + + diff --git a/Documentation/gpu/amdgpu/display/dcn3_cm_drm_current.svg b/Documentation/gpu/amdgpu/display/dcn3_cm_drm_current.svg new file mode 100644 index ..7299ee9b6d64 --- /dev/null +++ b/Documentation/gpu/amdgpu/display/dcn3_cm_drm_current.svg @@ -0,0 +1,1529 @@ + + + +http://www.inkscape.org/namespaces/inkscape>> + xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd>> + xmlns="http://www.w3.org/2000/svg>> + xmlns:svg="http://www.w3.org/2000/svg>> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +Matrix +1D LUT +3D LUT +Unpacking +Other +drm_framebuffer +format +drm_plane +drm_crtc +Stream +MPC +DPP + +Blender +Degamma +CTM +Gamma +format +bias_and_scale +color space matrix +input_csc_color_matrix +in_transfer_func +hdr_mult +gamut_remap_matrix +in_shaper_func +lut3d_func +blend_tf +Blender +gamut_remap_matrix +func_shaper +lut3d_func +out_transfer_func +csc_color_matrix +bit_depth_param +clamping +output_color_space +Plane +Legend +DCN 3.0 +DC Interface +DRM
Re: [PATCH v2 1/4] Documentation/amdgpu_dm: Add DM color correction documentation
On 2022-08-04 11:01, Melissa Wen wrote: AMDGPU DM maps DRM color management properties (degamma, ctm and gamma) to DC color correction entities. Part of this mapping is already documented as code comments and can be converted as kernel docs. v2: - rebase to amd-staging-drm-next - fix typos (Tales) - undo kernel-docs inside functions (Tales) Signed-off-by: Melissa Wen Reviewed-by: Harry Wentland Reviewed-by: Tales Aparecida --- .../gpu/amdgpu/display/display-manager.rst| 9 ++ .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 109 +- 2 files changed, 90 insertions(+), 28 deletions(-) diff --git a/Documentation/gpu/amdgpu/display/display-manager.rst b/Documentation/gpu/amdgpu/display/display-manager.rst index 7ce31f89d9a0..b1b0f11aed83 100644 --- a/Documentation/gpu/amdgpu/display/display-manager.rst +++ b/Documentation/gpu/amdgpu/display/display-manager.rst @@ -40,3 +40,12 @@ Atomic Implementation .. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c :functions: amdgpu_dm_atomic_check amdgpu_dm_atomic_commit_tail + +Color Management Properties +=== + +.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c + :doc: overview + +.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c + :internal: diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c index a71177305bcd..a4cb23d059bd 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c @@ -29,7 +29,9 @@ #include "modules/color/color_gamma.h" #include "basics/conversion.h" -/* +/** + * DOC: overview + * * The DC interface to HW gives us the following color management blocks * per pipe (surface): * @@ -71,8 +73,8 @@ #define MAX_DRM_LUT_VALUE 0x -/* - * Initialize the color module. +/** + * amdgpu_dm_init_color_mod - Initialize the color module. * * We're not using the full color module, only certain components. * Only call setup functions for components that we need. @@ -82,7 +84,14 @@ void amdgpu_dm_init_color_mod(void) setup_x_points_distribution(); } -/* Extracts the DRM lut and lut size from a blob. */ +/** + * __extract_blob_lut - Extracts the DRM lut and lut size from a blob. + * @blob: DRM color mgmt property blob + * @size: lut size + * + * Returns: + * DRM LUT or NULL + */ static const struct drm_color_lut * __extract_blob_lut(const struct drm_property_blob *blob, uint32_t *size) { @@ -90,13 +99,18 @@ __extract_blob_lut(const struct drm_property_blob *blob, uint32_t *size) return blob ? (struct drm_color_lut *)blob->data : NULL; } -/* - * Return true if the given lut is a linear mapping of values, i.e. it acts - * like a bypass LUT. +/** + * __is_lut_linear - check if the given lut is a linear mapping of values + * @lut: given lut to check values + * @size: lut size * * It is considered linear if the lut represents: - * f(a) = (0xFF00/MAX_COLOR_LUT_ENTRIES-1)a; for integer a in - * [0, MAX_COLOR_LUT_ENTRIES) + * f(a) = (0xFF00/MAX_COLOR_LUT_ENTRIES-1)a; for integer a in [0, + * MAX_COLOR_LUT_ENTRIES) + * + * Returns: + * True if the given lut is a linear mapping of values, i.e. it acts like a + * bypass LUT. Otherwise, false. */ static bool __is_lut_linear(const struct drm_color_lut *lut, uint32_t size) { @@ -119,9 +133,13 @@ static bool __is_lut_linear(const struct drm_color_lut *lut, uint32_t size) return true; } -/* - * Convert the drm_color_lut to dc_gamma. The conversion depends on the size - * of the lut - whether or not it's legacy. +/** + * __drm_lut_to_dc_gamma - convert the drm_color_lut to dc_gamma. + * @lut: DRM lookup table for color conversion + * @gamma: DC gamma to set entries + * @is_legacy: legacy or atomic gamma + * + * The conversion depends on the size of the lut - whether or not it's legacy. */ static void __drm_lut_to_dc_gamma(const struct drm_color_lut *lut, struct dc_gamma *gamma, bool is_legacy) @@ -154,8 +172,11 @@ static void __drm_lut_to_dc_gamma(const struct drm_color_lut *lut, } } -/* - * Converts a DRM CTM to a DC CSC float matrix. +/** + * __drm_ctm_to_dc_matrix - converts a DRM CTM to a DC CSC float matrix + * @ctm: DRM color transformation matrix + * @matrix: DC CSC float matrix + * * The matrix needs to be a 3x4 (12 entry) matrix. */ static void __drm_ctm_to_dc_matrix(const struct drm_color_ctm *ctm, @@ -189,7 +210,18 @@ static void __drm_ctm_to_dc_matrix(const struct drm_color_ctm *ctm, } } -/* Calculates the legacy transfer function - only for sRGB input space. */ +/** + * __set_legacy_tf - Calculates the legacy transfer function + * @func: transfer function + * @lut: lookup table that defines the color space + * @lut_size: size of
Re: [PATCH 2/3] drm/amd/display: Fix 'no previous prototype' compiler warns in amdgpu_dm_plane.c
On 2022-08-03 08:41, Imre Deak wrote: On Tue, Aug 02, 2022 at 12:57:24PM -0400, Rodrigo Siqueira Jordao wrote: On 2022-08-01 09:52, Imre Deak wrote: Fix compiler warnings like the following triggered by '-Wmissing-prototypes': CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_plane.o drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_plane.c:83:31: warning: no previous prototype for ‘amd_get_format_info’ [-Wmissing-prototypes] I see "‘" around "amd_get_format_info"; I'm not sure if my email client adds that or if there is something wrong in the commit message. Yes, it's a copy-paste from http://gfx-ci.fi.intel.com/archive/deploy/CI_DRM_11953/build_failure.log>> should be 'amd_get_format_info' and can be fixed while applying the patch. With the commit message change: Reviewed-by: Rodrigo Siqueira Thanks for the review. Could this and patch 3/3 be merged via the amd tree? Sure, Patch 2 and 3 applied to amd-staging-drm-next. Thanks Siqueira const struct drm_format_info *amd_get_format_info(const struct drm_mode_fb_cmd2 *cmd) Fixes: 5d945cbcd4b1 ("drm/amd/display: Create a file dedicated to planes") Cc: Harry Wentland Cc: Alan Liu Cc: Rodrigo Siqueira Signed-off-by: Imre Deak --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c index 5eb5d31e591de..da3b086b0d6ef 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c @@ -33,6 +33,7 @@ #include "amdgpu.h" #include "dal_asic_id.h" #include "amdgpu_display.h" +#include "amdgpu_dm_plane.h" #include "amdgpu_dm_trace.h" #include "gc/gc_11_0_0_offset.h" #include "gc/gc_11_0_0_sh_mask.h"
Re: [PATCH 3/3] drm/amd/display: Fix static declaration follows non-static declaration compiler warn
On 2022-08-01 09:52, Imre Deak wrote: Fix the drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_plane.c:951:13: error: static declaration of ‘get_min_max_dc_plane_scaling’ follows non-static declaration 951 | static void get_min_max_dc_plane_scaling(struct drm_device *dev, | ^~~~ In file included from drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_plane.c:36: drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_plane.h:39:6: note: previous declaration of ‘get_min_max_dc_plane_scaling’ with type ‘void(struct drm_device *, struct drm_framebuffer *, int *, int *)’ 39 | void get_min_max_dc_plane_scaling(struct drm_device *dev, complier warning. Fixes: 5d945cbcd4b1 ("drm/amd/display: Create a file dedicated to planes") Cc: Rodrigo Siqueira Cc: Harry Wentland Cc: Alan Liu Signed-off-by: Imre Deak --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h index 95168c2cfa6fa..c391f4b53 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.h @@ -36,10 +36,6 @@ int fill_dc_scaling_info(struct amdgpu_device *adev, const struct drm_plane_state *state, struct dc_scaling_info *scaling_info); -void get_min_max_dc_plane_scaling(struct drm_device *dev, - struct drm_framebuffer *fb, - int *min_downscale, int *max_upscale); - int dm_plane_helper_check_state(struct drm_plane_state *state, struct drm_crtc_state *new_crtc_state); Reviewed-by: Rodrigo Siqueira
Re: [PATCH 2/3] drm/amd/display: Fix 'no previous prototype' compiler warns in amdgpu_dm_plane.c
On 2022-08-01 09:52, Imre Deak wrote: Fix compiler warnings like the following triggered by '-Wmissing-prototypes': CC [M] drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_plane.o drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_plane.c:83:31: warning: no previous prototype for ‘amd_get_format_info’ [-Wmissing-prototypes] I see "‘" around "amd_get_format_info"; I'm not sure if my email client adds that or if there is something wrong in the commit message. With the commit message change: Reviewed-by: Rodrigo Siqueira const struct drm_format_info *amd_get_format_info(const struct drm_mode_fb_cmd2 *cmd) Fixes: 5d945cbcd4b1 ("drm/amd/display: Create a file dedicated to planes") Cc: Harry Wentland Cc: Alan Liu Cc: Rodrigo Siqueira Signed-off-by: Imre Deak --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c index 5eb5d31e591de..da3b086b0d6ef 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c @@ -33,6 +33,7 @@ #include "amdgpu.h" #include "dal_asic_id.h" #include "amdgpu_display.h" +#include "amdgpu_dm_plane.h" #include "amdgpu_dm_trace.h" #include "gc/gc_11_0_0_offset.h" #include "gc/gc_11_0_0_sh_mask.h"
Re: [PATCH v2 1/3] drm/amd/display: make variables static
On 2022-08-02 09:31, André Almeida wrote: Às 22:06 de 29/07/22, Magali Lemes escreveu: As "dcn3_1_soc", "dcn3_15_soc", and "dcn3_16_soc" are not used outside of their corresponding "dcn3*_fpu.c", make them static and remove their extern declaration. Fixes: 26f4712aedbd ("drm/amd/display: move FPU related code from dcn31 to dml/dcn31 folder") Fixes: fa896297b31b ("drm/amd/display: move FPU related code from dcn315 to dml/dcn31 folder") Fixes: 3f8951cc123f ("drm/amd/display: move FPU related code from dcn316 to dml/dcn31 folder") Signed-off-by: Magali Lemes Reviewed-by: Maíra Canal Reviewed-by: Melissa Wen --- Series is Reviewed-by: André Almeida Series also Reviewed-by: Rodrigo Siqueira Applied to amd-staging-drm-next. Thanks Siqueira
Re: [PATCH] drm/amd/display: remove DML Makefile duplicate lines
On 2022-08-02 09:05, Harry Wentland wrote: On 2022-08-02 08:04, Magali Lemes wrote: There are two identical CFLAGS entries for "display_mode_vba_20.o", so remove one of them. Also, as there's already an entry for "display_mode_lib.o" CFLAGS, regardless of CONFIG_DRM_AMD_DC_DCN being defined or not, remove the one entry between CONFIG_DRM_AMD_DC_DCN ifdef guards. Signed-off-by: Magali Lemes Reviewed-by: Harry Wentland Harry Applied to amd-staging-drm-next. Thanks Siqueira --- drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile b/drivers/gpu/drm/amd/display/dc/dml/Makefile index 359f6e9a1da0..41bb6c3cc2d8 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile @@ -61,7 +61,6 @@ CFLAGS_$(AMDDALPATH)/dc/dml/display_mode_vba.o := $(dml_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml/dcn10/dcn10_fpu.o := $(dml_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/dcn20_fpu.o := $(dml_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_mode_vba_20.o := $(dml_ccflags) -CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_mode_vba_20.o := $(dml_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_rq_dlg_calc_20.o := $(dml_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_mode_vba_20v2.o := $(dml_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml/dcn20/display_rq_dlg_calc_20v2.o := $(dml_ccflags) @@ -82,7 +81,6 @@ CFLAGS_$(AMDDALPATH)/dc/dml/dcn301/dcn301_fpu.o := $(dml_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml/dcn302/dcn302_fpu.o := $(dml_ccflags) CFLAGS_$(AMDDALPATH)/dc/dml/dcn303/dcn303_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
Re: [PATCH -next] drm/amd/display: remove unneeded semicolon
On 2022-07-26 18:28, Yang Li wrote: Eliminate the following coccicheck warning: ./drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c:2344:67-68: Unneeded semicolon Reported-by: Abaci Robot Signed-off-by: Yang Li --- drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c index 39428488a052..ca44df4fca74 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn20/dcn20_fpu.c @@ -2341,7 +2341,7 @@ void dcn201_populate_dml_writeback_from_context_fpu(struct dc *dc, dout_wb.wb_dst_width = wb_info->dwb_params.dest_width; dout_wb.wb_dst_height = wb_info->dwb_params.dest_height; dout_wb.wb_htaps_luma = wb_info->dwb_params.scaler_taps.h_taps; - dout_wb.wb_vtaps_luma = wb_info->dwb_params.scaler_taps.v_taps;; + dout_wb.wb_vtaps_luma = wb_info->dwb_params.scaler_taps.v_taps; dout_wb.wb_htaps_chroma = wb_info->dwb_params.scaler_taps.h_taps_c; dout_wb.wb_vtaps_chroma = wb_info->dwb_params.scaler_taps.v_taps_c; dout_wb.wb_hratio = wb_info->dwb_params.cnv_params.crop_en ? Reviewed-by: Rodrigo Siqueira And applied to amd-staging-drm-next. Thanks Siqueira
Re: [PATCH 1/4] Documentation/amdgpu_dm: Add DM color correction documentation
On 2022-07-20 18:54, Melissa Wen wrote: On 07/17, Tales Lelo da Aparecida wrote: On 16/07/2022 19:25, Melissa Wen wrote: AMDGPU DM maps DRM color management properties (degamma, ctm and gamma) to DC color correction entities. Part of this mapping is already documented as code comments and can be converted as kernel docs. v2: - rebase to amd-staging-drm-next Reviewed-by: Harry Wentland Signed-off-by: Melissa Wen --- .../gpu/amdgpu/display/display-manager.rst| 9 ++ .../amd/display/amdgpu_dm/amdgpu_dm_color.c | 121 +- 2 files changed, 98 insertions(+), 32 deletions(-) diff --git a/Documentation/gpu/amdgpu/display/display-manager.rst b/Documentation/gpu/amdgpu/display/display-manager.rst index 7ce31f89d9a0..b1b0f11aed83 100644 --- a/Documentation/gpu/amdgpu/display/display-manager.rst +++ b/Documentation/gpu/amdgpu/display/display-manager.rst @@ -40,3 +40,12 @@ Atomic Implementation .. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c :functions: amdgpu_dm_atomic_check amdgpu_dm_atomic_commit_tail + +Color Management Properties +=== + +.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c + :doc: overview + +.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c + :internal: diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c index a71177305bcd..93c813089bff 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c @@ -29,7 +29,9 @@ #include "modules/color/color_gamma.h" #include "basics/conversion.h" -/* +/** + * DOC: overview + * * The DC interface to HW gives us the following color management blocks * per pipe (surface): * @@ -71,8 +73,8 @@ #define MAX_DRM_LUT_VALUE 0x -/* - * Initialize the color module. +/** + * amdgpu_dm_init_color_mod - Initialize the color module. * * We're not using the full color module, only certain components. * Only call setup functions for components that we need. @@ -82,7 +84,14 @@ void amdgpu_dm_init_color_mod(void) setup_x_points_distribution(); } -/* Extracts the DRM lut and lut size from a blob. */ +/** + * __extract_blob_lut - Extracts the DRM lut and lut size from a blob. + * @blob: DRM color mgmt property blob + * @size: lut size + * + * Returns: + * DRM LUT or NULL + */ static const struct drm_color_lut * __extract_blob_lut(const struct drm_property_blob *blob, uint32_t *size) { @@ -90,13 +99,18 @@ __extract_blob_lut(const struct drm_property_blob *blob, uint32_t *size) return blob ? (struct drm_color_lut *)blob->data : NULL; } I don't think everyone would approve using actual kernel-doc for these static functions, but I can appreciate they being formatted as such. Consider replacing /** with /*. IMHO, although they are static, they provide info to understand the AMD DM programming of DRM color correction properties. I see the value for external contributors, but I'm not sure about kernel-doc rules about it. Yeah... I agree, I don't mind seeing kernel-doc for some static functions. Iirc, DRM documentation also documents some static functions. -/* - * Return true if the given lut is a linear mapping of values, i.e. it acts - * like a bypass LUT. +/** + * __is_lut_linear - check if the given lut is a linear mapping of values + * @lut: given lut to check values + * @size: lut size * * It is considered linear if the lut represents: - * f(a) = (0xFF00/MAX_COLOR_LUT_ENTRIES-1)a; for integer a in - * [0, MAX_COLOR_LUT_ENTRIES) + * f(a) = (0xFF00/MAX_COLOR_LUT_ENTRIES-1)a; for integer a in [0, + * MAX_COLOR_LUT_ENTRIES) + * + * Returns: + * True if the given lut is a linear mapping of values, i.e. it acts like a + * bypass LUT. Otherwise, false. */ static bool __is_lut_linear(const struct drm_color_lut *lut, uint32_t size) { @@ -119,9 +133,13 @@ static bool __is_lut_linear(const struct drm_color_lut *lut, uint32_t size) return true; } -/* - * Convert the drm_color_lut to dc_gamma. The conversion depends on the size - * of the lut - whether or not it's legacy. +/** + * __drm_lut_to_dc_gamma - convert the drm_color_lut to dc_gamma. + * @lut: DRM lookup table for color conversion + * @gamma: DC gamma to set entries + * @is_legacy: legacy or atomic gamma + * + * The conversion depends on the size of the lut - whether or not it's legacy. */ static void __drm_lut_to_dc_gamma(const struct drm_color_lut *lut, struct dc_gamma *gamma, bool is_legacy) @@ -154,8 +172,11 @@ static void __drm_lut_to_dc_gamma(const struct drm_color_lut *lut, } } -/* - * Converts a DRM CTM to a DC CSC float matrix. +/** + * __drm_ctm_to_dc_matrix - converts a DRM CTM to a DC CSC float matrix + * @ctm: DRM color transformation matrix
Re: [PATCH 0/5] drm/amd/display: FPU cleanup in clk_mgr files for powerpc
On 2022-07-20 15:32, Melissa Wen wrote: An initial report from Guenter[1] shows some soft-fp vs hard-fp error from DCN31 clk mgr for powerpc. I was not able to reproduce it cross-compiling with gcc-powerpc-linux-gnu and gcc-11.3, but thanks to Maíra tips, I can reproduce the issue using make.cross, as follows: - wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross - chmod +x ~/bin/make.cross - mkdir build_dir - COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 ~/make.cross O=build_dir ARCH=powerpc SHELL=/bin/bash Hi Melissa, I didn't know about these steps, I was trying to reproduce this issue by using the standard cross compile package provided by my distro (Debian testing and ArchLinux), and as a result, I was never able to see the problem. Anyway, I can now reproduce this issue, thanks a lot. with a config file generate by allmodconfig So, the first patch fix the issue reported by Guenter. The second is just a cleanup in dcn31_resource file to remove useless DC_FP_ wrapper. Finally, the last three patches I'm removing the -mno-gnu-attribute option, that was just hiding FPU-associated code in clk mgr files of dcn21/30/301, and moving them to DML folder. This series doesn't cover recent drivers dcn32/314. I validated this series in our internal CI by running multiple IGT tests in numerous ASICs. Tomorrow we will also send some extra patches associated with this FPU effort; hopefully, after that, we will finally have all the FPU code under DML. Again, thanks a lot for your effort! Thanks Siqueira Thanks Guenter, Maíra, Siqueira and Alex for all inputs on this debugging process. Let me know your thoughts on this approach. Melissa [1] https://lore.kernel.org/amd-gfx/20220618232737.2036722-1-li...@roeck-us.net/>> Melissa Wen (5): drm/amd/display: fix soft-fp vs hard-fp on DCN 3.1 family for powerpc drm/amd/display: remove useless FPU protection wrapper from dcn31_resource file drm/amd/display: move FPU code on dcn21 clk_mgr drm/amd/display: move FPU code from dcn30 clk mgr to DML folder drm/amd/display: move FPU code from dcn301 clk mgr to DML folder .../gpu/drm/amd/display/dc/clk_mgr/Makefile | 18 -- .../amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c | 234 + .../amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.h | 7 + .../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c | 63 + .../display/dc/clk_mgr/dcn301/vg_clk_mgr.c| 86 +-- .../display/dc/clk_mgr/dcn301/vg_clk_mgr.h| 3 + .../drm/amd/display/dc/dcn31/dcn31_resource.c | 11 +- .../amd/display/dc/dcn315/dcn315_resource.c | 5 +- .../amd/display/dc/dcn316/dcn316_resource.c | 5 +- .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 235 ++ .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.h | 2 + .../drm/amd/display/dc/dml/dcn30/dcn30_fpu.c | 63 - .../drm/amd/display/dc/dml/dcn30/dcn30_fpu.h | 1 + .../amd/display/dc/dml/dcn301/dcn301_fpu.c| 74 ++ .../drm/amd/display/dc/dml/dcn31/dcn31_fpu.c | 11 + .../drm/amd/display/dc/dml/dcn31/dcn31_fpu.h | 3 + 16 files changed, 423 insertions(+), 398 deletions(-)
Re: [PATCH 5/5] drm/amd/display: move FPU code from dcn301 clk mgr to DML folder
On 2022-07-20 15:32, Melissa Wen wrote: The -mno-gnu-attribute option in dcn301 clk mgr makefile hides a soft vs hard fp error for powerpc. After removing this flag, we can see some FPU code remains there: gcc-11.3.0-nolibc/powerpc64-linux/bin/powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn301/vg_clk_mgr.o uses soft float Therefore, remove the -mno-gnu-attribute flag for dcn301/powerpc and move FPU-associated code to DML folder. Signed-off-by: Melissa Wen --- .../gpu/drm/amd/display/dc/clk_mgr/Makefile | 6 -- .../display/dc/clk_mgr/dcn301/vg_clk_mgr.c| 86 ++- .../display/dc/clk_mgr/dcn301/vg_clk_mgr.h| 3 + .../amd/display/dc/dml/dcn301/dcn301_fpu.c| 74 4 files changed, 84 insertions(+), 85 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile b/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile index 15b660a951a5..271d8e573181 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile @@ -123,12 +123,6 @@ AMD_DISPLAY_FILES += $(AMD_DAL_CLK_MGR_DCN30) ### CLK_MGR_DCN301 = vg_clk_mgr.o dcn301_smu.o -# prevent build errors regarding soft-float vs hard-float FP ABI tags -# this code is currently unused on ppc64, as it applies to VanGogh APUs only -ifdef CONFIG_PPC64 -CFLAGS_$(AMDDALPATH)/dc/clk_mgr/dcn301/vg_clk_mgr.o := $(call cc-option,-mno-gnu-attribute) -endif - AMD_DAL_CLK_MGR_DCN301 = $(addprefix $(AMDDALPATH)/dc/clk_mgr/dcn301/,$(CLK_MGR_DCN301)) AMD_DISPLAY_FILES += $(AMD_DAL_CLK_MGR_DCN301) diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn301/vg_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn301/vg_clk_mgr.c index f310b0d25a07..65f224af03c0 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn301/vg_clk_mgr.c +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn301/vg_clk_mgr.c @@ -32,6 +32,10 @@ // For dcn20_update_clocks_update_dpp_dto #include "dcn20/dcn20_clk_mgr.h" +// For DML FPU code +#include "dml/dcn20/dcn20_fpu.h" +#include "dml/dcn301/dcn301_fpu.h" + #include "vg_clk_mgr.h" #include "dcn301_smu.h" #include "reg_helper.h" @@ -526,81 +530,6 @@ static struct clk_bw_params vg_bw_params = { }; -static struct wm_table ddr4_wm_table = { - .entries = { - { - .wm_inst = WM_A, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.72, - .sr_exit_time_us = 6.09, - .sr_enter_plus_exit_time_us = 7.14, - .valid = true, - }, - { - .wm_inst = WM_B, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.72, - .sr_exit_time_us = 10.12, - .sr_enter_plus_exit_time_us = 11.48, - .valid = true, - }, - { - .wm_inst = WM_C, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.72, - .sr_exit_time_us = 10.12, - .sr_enter_plus_exit_time_us = 11.48, - .valid = true, - }, - { - .wm_inst = WM_D, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.72, - .sr_exit_time_us = 10.12, - .sr_enter_plus_exit_time_us = 11.48, - .valid = true, - }, - } -}; - -static struct wm_table lpddr5_wm_table = { - .entries = { - { - .wm_inst = WM_A, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.65333, - .sr_exit_time_us = 13.5, - .sr_enter_plus_exit_time_us = 16.5, - .valid = true, - }, - { - .wm_inst = WM_B, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.65333, - .sr_exit_time_us = 13.5, - .sr_enter_plus_exit_time_us = 16.5, - .valid = true, - }, - { - .wm_inst = WM_C, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.65333, - .sr_exit_time_us = 13.5, - .sr_enter_plus_exit_time_us = 16.5, - .valid = true, - }, - { - .wm_inst = WM_D, -
Re: [PATCH 4/5] drm/amd/display: move FPU code from dcn30 clk mgr to DML folder
On 2022-07-20 15:32, Melissa Wen wrote: The -mno-gnu-attribute option in clk mgr makefile for dcn30 hides a soft vs hard fp error for powerpc. After removing this flag, we can see some FPU code remains there: gcc-11.3.0-nolibc/powerpc64-linux/bin/powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.o uses soft float Therefore, remove the -mno-gnu-attribute flag for dcn30/powerpc and move FPU-associated code to DML folder. Signed-off-by: Melissa Wen --- .../gpu/drm/amd/display/dc/clk_mgr/Makefile | 6 -- .../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c | 63 ++- .../drm/amd/display/dc/dml/dcn30/dcn30_fpu.c | 63 ++- .../drm/amd/display/dc/dml/dcn30/dcn30_fpu.h | 1 + 4 files changed, 68 insertions(+), 65 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile b/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile index 66dc02c426e9..15b660a951a5 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile @@ -115,12 +115,6 @@ AMD_DISPLAY_FILES += $(AMD_DAL_CLK_MGR_DCN21) ### CLK_MGR_DCN30 = dcn30_clk_mgr.o dcn30_clk_mgr_smu_msg.o -# prevent build errors regarding soft-float vs hard-float FP ABI tags -# this code is currently unused on ppc64, as it applies to VanGogh APUs only -ifdef CONFIG_PPC64 -CFLAGS_$(AMDDALPATH)/dc/clk_mgr/dcn30/dcn30_clk_mgr.o := $(call cc-option,-mno-gnu-attribute) -endif - AMD_DAL_CLK_MGR_DCN30 = $(addprefix $(AMDDALPATH)/dc/clk_mgr/dcn30/,$(CLK_MGR_DCN30)) AMD_DISPLAY_FILES += $(AMD_DAL_CLK_MGR_DCN30) diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c index 914708cefc79..3ce0ee0d012f 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c @@ -29,6 +29,7 @@ #include "dcn20/dcn20_clk_mgr.h" #include "dce100/dce_clk_mgr.h" #include "dcn30/dcn30_clk_mgr.h" +#include "dml/dcn30/dcn30_fpu.h" #include "reg_helper.h" #include "core_types.h" #include "dm_helpers.h" @@ -97,65 +98,11 @@ static void dcn3_init_single_clock(struct clk_mgr_internal *clk_mgr, uint32_t cl } } -static noinline void dcn3_build_wm_range_table(struct clk_mgr_internal *clk_mgr) +static void dcn3_build_wm_range_table(struct clk_mgr_internal *clk_mgr) { - /* defaults */ - double pstate_latency_us = clk_mgr->base.ctx->dc->dml.soc.dram_clock_change_latency_us; - double sr_exit_time_us = clk_mgr->base.ctx->dc->dml.soc.sr_exit_time_us; - double sr_enter_plus_exit_time_us = clk_mgr->base.ctx->dc->dml.soc.sr_enter_plus_exit_time_us; - uint16_t min_uclk_mhz = clk_mgr->base.bw_params->clk_table.entries[0].memclk_mhz; - - /* Set A - Normal - default values*/ - clk_mgr->base.bw_params->wm_table.nv_entries[WM_A].valid = true; - clk_mgr->base.bw_params->wm_table.nv_entries[WM_A].dml_input.pstate_latency_us = pstate_latency_us; - clk_mgr->base.bw_params->wm_table.nv_entries[WM_A].dml_input.sr_exit_time_us = sr_exit_time_us; - clk_mgr->base.bw_params->wm_table.nv_entries[WM_A].dml_input.sr_enter_plus_exit_time_us = sr_enter_plus_exit_time_us; - clk_mgr->base.bw_params->wm_table.nv_entries[WM_A].pmfw_breakdown.wm_type = WATERMARKS_CLOCK_RANGE; - clk_mgr->base.bw_params->wm_table.nv_entries[WM_A].pmfw_breakdown.min_dcfclk = 0; - clk_mgr->base.bw_params->wm_table.nv_entries[WM_A].pmfw_breakdown.max_dcfclk = 0x; - clk_mgr->base.bw_params->wm_table.nv_entries[WM_A].pmfw_breakdown.min_uclk = min_uclk_mhz; - clk_mgr->base.bw_params->wm_table.nv_entries[WM_A].pmfw_breakdown.max_uclk = 0x; - - /* Set B - Performance - higher minimum clocks */ -// clk_mgr->base.bw_params->wm_table.nv_entries[WM_B].valid = true; -// clk_mgr->base.bw_params->wm_table.nv_entries[WM_B].dml_input.pstate_latency_us = pstate_latency_us; -// clk_mgr->base.bw_params->wm_table.nv_entries[WM_B].dml_input.sr_exit_time_us = sr_exit_time_us; -// clk_mgr->base.bw_params->wm_table.nv_entries[WM_B].dml_input.sr_enter_plus_exit_time_us = sr_enter_plus_exit_time_us; -// clk_mgr->base.bw_params->wm_table.nv_entries[WM_B].pmfw_breakdown.wm_type = WATERMARKS_CLOCK_RANGE; -// clk_mgr->base.bw_params->wm_table.nv_entries[WM_B].pmfw_breakdown.min_dcfclk = TUNED VALUE; -// clk_mgr->base.bw_params->wm_table.nv_entries[WM_B].pmfw_breakdown.max_dcfclk = 0x; -// clk_mgr->base.bw_params->wm_table.nv_entries[WM_B].pmfw_breakdown.min_uclk = TUNED VALUE; -// clk_mgr->base.bw_params->wm_table.nv_entries[WM_B].pmfw_breakdown.max_uclk = 0x; - - /* Set C - Dummy P-State - P-State latency set
Re: [PATCH 3/5] drm/amd/display: move FPU code on dcn21 clk_mgr
On 2022-07-20 15:32, Melissa Wen wrote: The -mno-gnu-attribute option in dcn21 clk mgr makefile hides a soft vs hard fp error for powerpc. After removing this flag, we can see some FPU code remains there: /gcc-11.3.0-nolibc/powerpc64-linux/bin/powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/dc/clk_mgr/dcn21/rn_clk_mgr.o uses soft float Therefore, remove the -mno-gnu-attribute flag for dcn21/powerpc and move FPU-associated code to DML folder. Signed-off-by: Melissa Wen --- .../gpu/drm/amd/display/dc/clk_mgr/Makefile | 6 - .../amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c | 234 + .../amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.h | 7 + .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 235 ++ .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.h | 2 + 5 files changed, 248 insertions(+), 236 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile b/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile index a48453612d10..66dc02c426e9 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile @@ -107,12 +107,6 @@ AMD_DISPLAY_FILES += $(AMD_DAL_CLK_MGR_DCN201) ### CLK_MGR_DCN21 = rn_clk_mgr.o rn_clk_mgr_vbios_smu.o -# prevent build errors regarding soft-float vs hard-float FP ABI tags -# this code is currently unused on ppc64, as it applies to Renoir APUs only -ifdef CONFIG_PPC64 -CFLAGS_$(AMDDALPATH)/dc/clk_mgr/dcn21/rn_clk_mgr.o := $(call cc-option,-mno-gnu-attribute) -endif - AMD_DAL_CLK_MGR_DCN21 = $(addprefix $(AMDDALPATH)/dc/clk_mgr/dcn21/,$(CLK_MGR_DCN21)) AMD_DISPLAY_FILES += $(AMD_DAL_CLK_MGR_DCN21) diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c index cf1b5f354ae9..0202dc682682 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c @@ -26,10 +26,9 @@ #include "dccg.h" #include "clk_mgr_internal.h" - #include "dcn20/dcn20_clk_mgr.h" #include "rn_clk_mgr.h" - +#include "dml/dcn20/dcn20_fpu.h" #include "dce100/dce_clk_mgr.h" #include "rn_clk_mgr_vbios_smu.h" @@ -45,7 +44,6 @@ /* Constants */ -#define LPDDR_MEM_RETRAIN_LATENCY 4.977 /* Number obtained from LPDDR4 Training Counter Requirement doc */ #define SMU_VER_55_51_0 0x373300 /* SMU Version that is able to set DISPCLK below 100MHz */ /* Macros */ @@ -613,228 +611,6 @@ static struct clk_bw_params rn_bw_params = { }; -static struct wm_table ddr4_wm_table_gs = { - .entries = { - { - .wm_inst = WM_A, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.72, - .sr_exit_time_us = 7.09, - .sr_enter_plus_exit_time_us = 8.14, - .valid = true, - }, - { - .wm_inst = WM_B, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.72, - .sr_exit_time_us = 10.12, - .sr_enter_plus_exit_time_us = 11.48, - .valid = true, - }, - { - .wm_inst = WM_C, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.72, - .sr_exit_time_us = 10.12, - .sr_enter_plus_exit_time_us = 11.48, - .valid = true, - }, - { - .wm_inst = WM_D, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.72, - .sr_exit_time_us = 10.12, - .sr_enter_plus_exit_time_us = 11.48, - .valid = true, - }, - } -}; - -static struct wm_table lpddr4_wm_table_gs = { - .entries = { - { - .wm_inst = WM_A, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.65333, - .sr_exit_time_us = 5.32, - .sr_enter_plus_exit_time_us = 6.38, - .valid = true, - }, - { - .wm_inst = WM_B, - .wm_type = WM_TYPE_PSTATE_CHG, - .pstate_latency_us = 11.65333, - .sr_exit_time_us = 9.82, - .sr_enter_plus_exit_time_us = 11.196, - .valid = true, - }, - { - .wm_inst = WM_C, - .wm_type = WM_TYPE_PSTATE_CHG, -
Re: [PATCH 2/5] drm/amd/display: remove useless FPU protection wrapper from dcn31_resource file
On 2022-07-20 15:32, Melissa Wen wrote: Many lines of code in dcn31_resource_construct are wrapped by DC_FP macro to protect FPU operations; however, there is no FPU in this region. Therefore, just remove the wrapper for clarity. Signed-off-by: Melissa Wen --- drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c index 929b712cbada..6d25fcf865bf 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c @@ -1863,8 +1863,6 @@ static bool dcn31_resource_construct( struct dc_context *ctx = dc->ctx; struct irq_service_init_data init_data; - DC_FP_START(); - ctx->dc_bios->regs = _regs; pool->base.res_cap = _cap_dcn31; @@ -2175,13 +2173,9 @@ static bool dcn31_resource_construct( dc->dcn_ip->max_num_dpp = dcn3_1_ip.max_num_dpp; - DC_FP_END(); - return true; create_fail: - - DC_FP_END(); dcn31_resource_destruct(pool); return false; Very nice catch! Reviewed-by: Rodrigo Siqueira
Re: [PATCH 1/5] drm/amd/display: fix soft-fp vs hard-fp on DCN 3.1 family for powerpc
On 2022-07-20 15:32, Melissa Wen wrote: Move remaining FPU code to DML folder that caused compilation error for powerpc. This patch depends on [1] to prevent the error below: /gcc-11.3.0-nolibc/powerpc64-linux/bin/powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/dc/dcn31/dcn31_resource.o uses soft float /gcc-11.3.0-nolibc/powerpc64-linux/bin/powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dcn31/dcn31_resource.o /gcc-11.3.0-nolibc/powerpc64-linux/bin/powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/dc/dcn315/dcn315_resource.o uses soft float /gcc-11.3.0-nolibc/powerpc64-linux/bin/powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dcn315/dcn315_resource.o /gcc-11.3.0-nolibc/powerpc64-linux/bin/powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/dc/dcn316/dcn316_resource.o uses soft float /gcc-11.3.0-nolibc/powerpc64-linux/bin/powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dcn316/dcn316_resource.o [1] https://lore.kernel.org/amd-gfx/20220716195144.342960-1-m...@igalia.com/ Reported-by: Guenter Roeck Signed-off-by: Melissa Wen --- drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c | 5 +++-- .../gpu/drm/amd/display/dc/dcn315/dcn315_resource.c | 5 +++-- .../gpu/drm/amd/display/dc/dcn316/dcn316_resource.c | 5 +++-- drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c | 11 +++ drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.h | 3 +++ 5 files changed, 23 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c index 178d40c0d70a..929b712cbada 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c @@ -1663,11 +1663,12 @@ int dcn31_populate_dml_pipes_from_context( pipes[pipe_cnt].pipe.src.immediate_flip = true; pipes[pipe_cnt].pipe.src.unbounded_req_mode = false; pipes[pipe_cnt].pipe.src.gpuvm = true; - pipes[pipe_cnt].pipe.src.dcc_fraction_of_zs_req_luma = 0; - pipes[pipe_cnt].pipe.src.dcc_fraction_of_zs_req_chroma = 0; pipes[pipe_cnt].pipe.dest.vfront_porch = timing->v_front_porch; pipes[pipe_cnt].pipe.src.dcc_rate = 3; pipes[pipe_cnt].dout.dsc_input_bpc = 0; + DC_FP_START(); + dcn31_zero_pipe_dcc_fraction(pipes, pipe_cnt); + DC_FP_END(); if (dc->debug.dml_hostvm_override == DML_HOSTVM_NO_OVERRIDE) pipes[pipe_cnt].pipe.src.hostvm = dc->res_pool->hubbub->riommu_active; diff --git a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c index df2abd8fe2eb..1a5f5977f962 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c @@ -1658,11 +1658,12 @@ static int dcn315_populate_dml_pipes_from_context( pipes[pipe_cnt].pipe.src.unbounded_req_mode = false; pipes[pipe_cnt].pipe.src.gpuvm = true; - pipes[pipe_cnt].pipe.src.dcc_fraction_of_zs_req_luma = 0; - pipes[pipe_cnt].pipe.src.dcc_fraction_of_zs_req_chroma = 0; pipes[pipe_cnt].pipe.dest.vfront_porch = timing->v_front_porch; pipes[pipe_cnt].pipe.src.dcc_rate = 3; pipes[pipe_cnt].dout.dsc_input_bpc = 0; + DC_FP_START(); + dcn31_zero_pipe_dcc_fraction(pipes, pipe_cnt); + DC_FP_END(); if (pipes[pipe_cnt].dout.dsc_enable) { switch (timing->display_color_depth) { diff --git a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.c b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.c index 070fe10a004e..53dea466348f 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.c @@ -1661,11 +1661,12 @@ static int dcn316_populate_dml_pipes_from_context( pipes[pipe_cnt].pipe.src.unbounded_req_mode = false; pipes[pipe_cnt].pipe.src.gpuvm = true; - pipes[pipe_cnt].pipe.src.dcc_fraction_of_zs_req_luma = 0; - pipes[pipe_cnt].pipe.src.dcc_fraction_of_zs_req_chroma = 0; pipes[pipe_cnt].pipe.dest.vfront_porch = timing->v_front_porch; pipes[pipe_cnt].pipe.src.dcc_rate = 3; pipes[pipe_cnt].dout.dsc_input_bpc = 0; + DC_FP_START(); +
Re: [PATCH] drm/amd/display: Add missing hard-float compile flags for PPC64 builds
On 2022-06-18 19:27, Guenter Roeck wrote: ppc:allmodconfig builds fail with the following error. powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/dc/dcn31/dcn31_resource.o uses soft float powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dcn31/dcn31_resource.o powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/dc/dcn315/dcn315_resource.o uses soft float powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dcn315/dcn315_resource.o powerpc64-linux-ld: drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o uses hard float, drivers/gpu/drm/amd/amdgpu/../display/dc/dcn316/dcn316_resource.o uses soft float powerpc64-linux-ld: failed to merge target specific data of file drivers/gpu/drm/amd/amdgpu/../display/dc/dcn316/dcn316_resource.o The problem was introduced with commit 41b7a347bf14 ("powerpc: Book3S 64-bit outline-only KASAN support") which adds support for KASAN. This commit in turn enables DRM_AMD_DC_DCN because KCOV_INSTRUMENT_ALL and KCOV_ENABLE_COMPARISONS are no longer enabled. As result, new files are compiled which lack the selection of hard-float. Fixes: 41b7a347bf14 ("powerpc: Book3S 64-bit outline-only KASAN support") Cc: Michael Ellerman Cc: Daniel Axtens Signed-off-by: Guenter Roeck --- drivers/gpu/drm/amd/display/dc/dcn31/Makefile | 4 drivers/gpu/drm/amd/display/dc/dcn315/Makefile | 4 drivers/gpu/drm/amd/display/dc/dcn316/Makefile | 4 3 files changed, 12 insertions(+) diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/Makefile b/drivers/gpu/drm/amd/display/dc/dcn31/Makefile index ec041e3cda30..74be02114ae4 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn31/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dcn31/Makefile @@ -15,6 +15,10 @@ DCN31 = dcn31_resource.o dcn31_hubbub.o dcn31_hwseq.o dcn31_init.o dcn31_hubp.o dcn31_apg.o dcn31_hpo_dp_stream_encoder.o dcn31_hpo_dp_link_encoder.o \ dcn31_afmt.o dcn31_vpg.o +ifdef CONFIG_PPC64 +CFLAGS_$(AMDDALPATH)/dc/dcn31/dcn31_resource.o := -mhard-float -maltivec +endif + AMD_DAL_DCN31 = $(addprefix $(AMDDALPATH)/dc/dcn31/,$(DCN31)) AMD_DISPLAY_FILES += $(AMD_DAL_DCN31) diff --git a/drivers/gpu/drm/amd/display/dc/dcn315/Makefile b/drivers/gpu/drm/amd/display/dc/dcn315/Makefile index 59381d24800b..1395c1ced8c5 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn315/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dcn315/Makefile @@ -25,6 +25,10 @@ DCN315 = dcn315_resource.o +ifdef CONFIG_PPC64 +CFLAGS_$(AMDDALPATH)/dc/dcn315/dcn315_resource.o := -mhard-float -maltivec +endif + AMD_DAL_DCN315 = $(addprefix $(AMDDALPATH)/dc/dcn315/,$(DCN315)) AMD_DISPLAY_FILES += $(AMD_DAL_DCN315) diff --git a/drivers/gpu/drm/amd/display/dc/dcn316/Makefile b/drivers/gpu/drm/amd/display/dc/dcn316/Makefile index 819d44a9439b..c3d2dd78f1e2 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn316/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dcn316/Makefile @@ -25,6 +25,10 @@ DCN316 = dcn316_resource.o +ifdef CONFIG_PPC64 +CFLAGS_$(AMDDALPATH)/dc/dcn316/dcn316_resource.o := -mhard-float -maltivec +endif + AMD_DAL_DCN316 = $(addprefix $(AMDDALPATH)/dc/dcn316/,$(DCN316)) AMD_DISPLAY_FILES += $(AMD_DAL_DCN316) Hi, I don't want to re-introduce those FPU flags for DCN31/DCN314/DCN316 since we fully isolate FPU operations for those ASICs inside the DML folder. Notice that we have the PPC64 in the DML Makefile: https://gitlab.freedesktop.org/agd5f/linux/-/blob/amd-staging-drm-next/drivers/gpu/drm/amd/display/dc/dml/Makefile Could you share what you see without your patch in the amd-staging-drm-next? Also: * Are you using cross-compilation? If so, could you share your setup? * Which GCC/Clang version are you using? Thanks Siqueira
Re: [RFC 0/3] drm/amd/display: Introduce KUnit to Display Mode Library
Hi, First of all, thanks a lot for exploring the introduction of kunit inside amdgpu. See my inline comments On 2022-06-18 05:08, David Gow wrote: On Sat, Jun 18, 2022 at 4:24 AM Maíra Canal wrote: On 6/17/22 04:55, David Gow wrote: On Fri, Jun 17, 2022 at 6:41 AM Maíra Canal wrote: Hi David, Thank you for your feedback! On 6/16/22 11:39, David Gow wrote: On Wed, Jun 8, 2022 at 9:08 AM Maíra Canal wrote: As kunit_test_suites() defines itself as an init_module(), it conflicts with the existing one at amdgpu_drv. So, if we use kunit_test_suites(), we won't be able to compile the tests as modules and, therefore, won't be able to use IGT to run the tests. This problem with kunit_test_suites() was already discussed in the KUnit mailing list, as can be seen in [7]. I'm not sure I fully understand why these tests need to be part of the amdgpu module, though admittedly I've not played with IGT much. Would it be possible to compile these tests as separate modules, which could depend on amdgpu (or maybe include the DML stuff directly), and therefore not have this conflict? I definitely was able to get these tests working under kunit_tool (albeit as built-ins) by using kunit_test_suites(). If each suite were built as a separate module (or indeed, even if all the tests were in one module, with one list of suites), then it should be possible to avoid the init_module() conflict. That'd also make it possible to run these tests without actually needing the driver to initialise, which seems like it might require actual hardware(?) In the Display code for amdgpu, we heavily rely on IGT for automated tests. We have some internal CI where we run a large set of IGT tests per patch, and I'm sure many other DRM developers also use IGT. In this sense, if we can have an interface inside IGT that can easily run those kunit tests, we can enable kunit tests in our CI pipeline almost for free :) We already have a prototype for this sort of integration at: https://patchwork.freedesktop.org/series/105294/ Initially, we tried the kunit_test_suites() approach. And it did work pretty well for the kunit_tool (although we didn't test any hardware-specific unit test). But when compiling the test as a module, we would get a linking error, pointing out multiple definitions of 'init_module'/'cleanup_module' at kunit_test_suites(). At this point, we thought about a couple of options to resolve this problem: - Add EXPORT_SYMBOL to the functions we would test. But, this doesn't scale pretty well, because it would pollute AMDGPU code as the tests expand. - Take the Thunderbolt path and add the tests to the driver stack. We end up taking the Thunderbolt path as it would be more maintainable. Compiling the tests as a module is essential to make the tests run at IGT, as IGT essentially loads the module, runs it, and parses the output (a very very simplified explanation of what IGT does). IGT is a very known tool for DRI developers, so we believe that IGT support is crucial for this project. If you have any other options on how to make the module compilation viable without using the 'thunderbolt'-style, we would be glad to hear your suggestions. As you point out, there are really two separate problems with splitting the tests out totally: - It's ugly and pollutes the symbol namespace to have EXPORT_SYMBOL() everywhere. - It's impossible to have multiple init_module() "calls" in the same module. The first of these is, I think, the harder to solve generally. (There are some ways to mitigate the namespace pollution part of it by either hiding the EXPORT_SYMBOL() directives behind #ifdef CONFIG_KUNIT or similar, or by using symbol namespaces: https://www.kernel.org/doc/html/latest/core-api/symbol-namespaces.html -- or both -- but they don't solve the issue entirely.) That being said, it's as much a matter of taste as anything, so if keeping things in the amdgpu module works well, don't let me stop you. Either way should work, and have their own advantages and disadvantages. I want to avoid making changes inside the dc code [1] (or keep it minimal) for enabling kunit. Aligned with the IGT comment, I'm more inclined to a solution where we treat the kunit tests for DML as a module. However, I'm not sure yet if it is possible to have something like that... Does it make things easier if we have a single module that handles the dml-kunit interface, and we can control which test to invoke via kernel parameter? 1. https://gitlab.freedesktop.org/agd5f/linux/-/tree/amd-staging-drm-next/drivers/gpu/drm/amd/display/dc The latter is just a quirk of the current KUnit implementation of kunit_test_suites(). This multiple-definition issue will go away in the not-too-distant future. So my suggestion here would be to make sure any changes you make to work around the issue with multiple init_module definitions are easy to remove. I suspect you could probably significantly simplify the whole dml_test.{c,h}
Re: [PATCH] drm/amd/display: protect remaining FPU-code calls on dcn3.1.x
On 2022-03-30 19:02, Melissa Wen wrote: From [1], I realized two other calls to dcn30 code are associated with FPU operations and are not protected by DC_FP_* macros: * dcn30_populate_dml_writeback_from_context() * dcn30_set_mcif_arb_params() So, since FPU-associated code is not fully isolated in dcn30, and dcn3.1.x reuses them, let's wrap their calls properly. Note: this patch complements the fix from [1]. [1] https://lore.kernel.org/amd-gfx/20220329082957.1662655-1-chandan.vurdigerenata...@amd.com/ Signed-off-by: Melissa Wen --- .../drm/amd/display/dc/dcn31/dcn31_resource.c | 25 +-- .../drm/amd/display/dc/dcn31/dcn31_resource.h | 9 +++ .../amd/display/dc/dcn315/dcn315_resource.c | 4 +-- .../amd/display/dc/dcn316/dcn316_resource.c | 4 +-- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c index bf130b2435ab..afdfec74ed08 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.c @@ -1735,6 +1735,27 @@ void dcn31_calculate_wm_and_dlg( DC_FP_END(); } +void +dcn31_populate_dml_writeback_from_context(struct dc *dc, + struct resource_context *res_ctx, + display_e2e_pipe_params_st *pipes) +{ + DC_FP_START(); + dcn30_populate_dml_writeback_from_context(dc, res_ctx, pipes); + DC_FP_END(); +} + +void +dcn31_set_mcif_arb_params(struct dc *dc, + struct dc_state *context, + display_e2e_pipe_params_st *pipes, + int pipe_cnt) +{ + DC_FP_START(); + dcn30_set_mcif_arb_params(dc, context, pipes, pipe_cnt); + DC_FP_END(); +} + bool dcn31_validate_bandwidth(struct dc *dc, struct dc_state *context, bool fast_validate) @@ -1806,8 +1827,8 @@ static struct resource_funcs dcn31_res_pool_funcs = { .add_stream_to_ctx = dcn30_add_stream_to_ctx, .add_dsc_to_stream_resource = dcn20_add_dsc_to_stream_resource, .remove_stream_from_ctx = dcn20_remove_stream_from_ctx, - .populate_dml_writeback_from_context = dcn30_populate_dml_writeback_from_context, - .set_mcif_arb_params = dcn30_set_mcif_arb_params, + .populate_dml_writeback_from_context = dcn31_populate_dml_writeback_from_context, + .set_mcif_arb_params = dcn31_set_mcif_arb_params, .find_first_free_match_stream_enc_for_link = dcn10_find_first_free_match_stream_enc_for_link, .acquire_post_bldn_3dlut = dcn30_acquire_post_bldn_3dlut, .release_post_bldn_3dlut = dcn30_release_post_bldn_3dlut, diff --git a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.h b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.h index 1ce6509c1ed1..393458015d6a 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.h +++ b/drivers/gpu/drm/amd/display/dc/dcn31/dcn31_resource.h @@ -50,6 +50,15 @@ int dcn31_populate_dml_pipes_from_context( struct dc *dc, struct dc_state *context, display_e2e_pipe_params_st *pipes, bool fast_validate); +void +dcn31_populate_dml_writeback_from_context(struct dc *dc, + struct resource_context *res_ctx, + display_e2e_pipe_params_st *pipes); +void +dcn31_set_mcif_arb_params(struct dc *dc, + struct dc_state *context, + display_e2e_pipe_params_st *pipes, + int pipe_cnt); void dcn31_update_soc_for_wm_a(struct dc *dc, struct dc_state *context); struct resource_pool *dcn31_create_resource_pool( diff --git a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c index fadb89326999..06dd064e5997 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn315/dcn315_resource.c @@ -1726,8 +1726,8 @@ static struct resource_funcs dcn315_res_pool_funcs = { .add_stream_to_ctx = dcn30_add_stream_to_ctx, .add_dsc_to_stream_resource = dcn20_add_dsc_to_stream_resource, .remove_stream_from_ctx = dcn20_remove_stream_from_ctx, - .populate_dml_writeback_from_context = dcn30_populate_dml_writeback_from_context, - .set_mcif_arb_params = dcn30_set_mcif_arb_params, + .populate_dml_writeback_from_context = dcn31_populate_dml_writeback_from_context, + .set_mcif_arb_params = dcn31_set_mcif_arb_params, .find_first_free_match_stream_enc_for_link = dcn10_find_first_free_match_stream_enc_for_link, .acquire_post_bldn_3dlut = dcn30_acquire_post_bldn_3dlut, .release_post_bldn_3dlut = dcn30_release_post_bldn_3dlut, diff --git a/drivers/gpu/drm/amd/display/dc/dcn316/dcn316_resource.c
Re: [PATCH v2] drm/amd/display: don't ignore alpha property on pre-multiplied mode
Patch merged to amd-staging-drm-next. Thanks a lot! On 2022-04-05 15:32, Simon Ser wrote: I've tested this patch and it fixes my bug [1]. Thanks! Tested-by: Simon Ser [1]: https://gitlab.freedesktop.org/drm/amd/-/issues/1734>
Re: [PATCH 0/2] remove DC_FP_* wrappers in dml files
On 2022-03-26 16:24, Melissa Wen wrote: From FPU documentation, developers must not use DC_FP_START/END in dml files, but invoke it when calling FPU-associated functions (isolated in dml folder). Therefore, the first patch renames dcn10_validate_bandwidth in dml/calcs to dcn_ for generalization, declares dcn10_validate_bandwidth in dcn10 - that calls dcn_validate_bandwidth and wraps with DC_FP_* accordingly. The second patch removes invocations of DC_FP_* from dml files and properly wraps FPU functions in dc code outside dml folder. Melissa Wen (2): drm/amd/display: detach fpu operations from dcn10_validate_bandwidth in calcs drm/amd/display: remove DC_FP_* wrapper from dml folder .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 10 -- .../drm/amd/display/dc/dcn10/dcn10_resource.c | 16 .../drm/amd/display/dc/dml/calcs/dcn_calcs.c | 19 +-- .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 2 -- .../gpu/drm/amd/display/dc/inc/dcn_calcs.h| 2 +- 5 files changed, 26 insertions(+), 23 deletions(-) Hi, Thanks a lot for your patch! I reviewed and tested this series and lgtm. Applied to amd-staging-drm-next. Btw, I agree with Christian. Can you try to find a way to add a compilation error or warning if the developer tries to add DC_FP_* inside DML? Also, about the question of recursive calling to DC_FP_*, it should be safe if using DC_FP_*. Thanks Siqueira
Re: [PATCH v2] drm/amd/display: don't ignore alpha property on pre-multiplied mode
On 2022-03-29 16:18, Melissa Wen wrote: "Pre-multiplied" is the default pixel blend mode for KMS/DRM, as documented in supported_modes of drm_plane_create_blend_mode_property(): https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/drm_blend.c In this mode, both 'pixel alpha' and 'plane alpha' participate in the calculation, as described by the pixel blend mode formula in KMS/DRM documentation: out.rgb = plane_alpha * fg.rgb + (1 - (plane_alpha * fg.alpha)) * bg.rgb Considering the blend config mechanisms we have in the driver so far, the alpha mode that better fits this blend mode is the _PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN, where the value for global_gain is the plane alpha (global_alpha). With this change, alpha property stops to be ignored. It also addresses Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1734 v2: * keep the 8-bit value for global_alpha_value (Nicholas) * correct the logical ordering for combined global gain (Nicholas) * apply to dcn10 too (Nicholas) Signed-off-by: Melissa Wen --- .../drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c | 14 +- drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c | 14 +- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c index ad757b59e00e..b1034e6258c8 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c @@ -2528,14 +2528,18 @@ void dcn10_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx) struct mpc *mpc = dc->res_pool->mpc; struct mpc_tree *mpc_tree_params = &(pipe_ctx->stream_res.opp->mpc_tree_params); - if (per_pixel_alpha) - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA; - else - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA; - blnd_cfg.overlap_only = false; blnd_cfg.global_gain = 0xff; + if (per_pixel_alpha && pipe_ctx->plane_state->global_alpha) { + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN; + blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value; + } else if (per_pixel_alpha) { + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA; + } else { + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA; + } + if (pipe_ctx->plane_state->global_alpha) blnd_cfg.global_alpha = pipe_ctx->plane_state->global_alpha_value; else diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c index 4290eaf11a04..b627c41713cc 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c @@ -2344,14 +2344,18 @@ void dcn20_update_mpcc(struct dc *dc, struct pipe_ctx *pipe_ctx) struct mpc *mpc = dc->res_pool->mpc; struct mpc_tree *mpc_tree_params = &(pipe_ctx->stream_res.opp->mpc_tree_params); - if (per_pixel_alpha) - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA; - else - blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA; - blnd_cfg.overlap_only = false; blnd_cfg.global_gain = 0xff; + if (per_pixel_alpha && pipe_ctx->plane_state->global_alpha) { + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA_COMBINED_GLOBAL_GAIN; + blnd_cfg.global_gain = pipe_ctx->plane_state->global_alpha_value; + } else if (per_pixel_alpha) { + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_PER_PIXEL_ALPHA; + } else { + blnd_cfg.alpha_mode = MPCC_ALPHA_BLEND_MODE_GLOBAL_ALPHA; + } + Hi Melissa, Thanks a lot for this patch. I run your patch in our CI, and everything looks good from the IGT test result. In this sense: Tested-by: Rodrigo Siqueira However, I think it is still necessary to have someone else review. Harry, Nick, or Zhan, what do you think about this change? Thanks Siqueira if (pipe_ctx->plane_state->global_alpha) blnd_cfg.global_alpha = pipe_ctx->plane_state->global_alpha_value; else
Re: [PATCH 0/3] Move FPU related code from DCN3.1x drivers to DML folder
On 2022-03-07 10:47, Melissa Wen wrote: This series moves FPU code from DCN 3.1x drivers to dml/dcn31 folder to isolate FPU operations. For this, it creates dcn31_fpu files to centralize FPU operations and structs from dcn31x drivers, that include: - _vcs_dpi_ip_params_st and _vcs_dpi_soc_bounding_box_st structs - dcn31x_update_bw_bounding_box() functions - dcn31_calculate_wm_and_dlg_fp() Also, it adds dc_assert_fp_enabled() in public dml-fpu functions, as required, and I've checked if their calls are properly wrapped by DC_FP_START/END (and removed when inside dml/fpu files too). Melissa Wen (3): drm/amd/dicplay: move FPU related code from dcn31 to dml/dcn31 folder drm/amd/display: move FPU related code from dcn315 to dml/dcn31 folder drm/amd/display: move FPU related code from dcn316 to dml/dcn31 folder drivers/gpu/drm/amd/display/dc/dcn31/Makefile | 26 - .../drm/amd/display/dc/dcn31/dcn31_resource.c | 355 +-- .../drm/amd/display/dc/dcn31/dcn31_resource.h | 4 +- .../gpu/drm/amd/display/dc/dcn315/Makefile| 26 - .../amd/display/dc/dcn315/dcn315_resource.c | 232 + .../amd/display/dc/dcn315/dcn315_resource.h | 3 + .../gpu/drm/amd/display/dc/dcn316/Makefile| 26 - .../amd/display/dc/dcn316/dcn316_resource.c | 231 + .../amd/display/dc/dcn316/dcn316_resource.h | 3 + drivers/gpu/drm/amd/display/dc/dml/Makefile | 2 + .../drm/amd/display/dc/dml/dcn31/dcn31_fpu.c | 863 ++ .../drm/amd/display/dc/dml/dcn31/dcn31_fpu.h | 44 + 12 files changed, 921 insertions(+), 894 deletions(-) create mode 100644 drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.c create mode 100644 drivers/gpu/drm/amd/display/dc/dml/dcn31/dcn31_fpu.h Hi Melissa, Thanks a lot for your patchset. We have already tested it with IGT, and we are running some manual tests to ensure that everything is fine. We will include your patch in this week's DC upstream, and if everything is alright, we will merge it next Monday.
Re: [PATCH] drm/amd/display: move FPU-related code from dcn20 to dml folder
On 2022-02-21 06:31, Melissa Wen wrote: Move parts of dcn20 code that uses FPU to dml folder. It aims to isolate FPU operations as described by series: drm/amd/display: Introduce FPU directory inside DC https://patchwork.freedesktop.org/series/93042/ This patch moves the following functions from dcn20_resource to dml/dcn20_fpu and calls of public functions in dcn20_resource are wrapped by DC_FP_START/END(): - void dcn20_populate_dml_writeback_from_context - static bool is_dtbclk_required() - static enum dcn_zstate_support_state() - void dcn20_calculate_dlg_params() - static void swizzle_to_dml_params() - int dcn20_populate_dml_pipes_from_context() - void dcn20_calculate_wm() - void dcn20_cap_soc_clocks() - void dcn20_update_bounding_box() - void dcn20_patch_bounding_box() - bool dcn20_validate_bandwidth_fp() This movement also affects dcn30/31, as dcn20_calculate_dlg_params() is used by dcn30 and dcn31. For this reason, I included dcn20_fpu headers in dcn20_resource headers to make dcn20_calculate_dlg_params() visible to dcn30/31. Three new functions are created to isolate well-delimited FPU operations: - void dcn20_fpu_set_wb_arb_params(): set cli_watermark, pstate_watermark and time_per_pixel from wb_arb_params (struct mcif_arb_params), since those uses FPU operations on double types: WritebackUrgentWatermark, WritebackDRAMClockChangeWatermark, '16.0'. - void dcn20_fpu_set_wm_ranges(): set min_fill_clk_mhz and max_fill_clk_mhz involves FPU calcs on dram_speed_mts (double type); - void dcn20_fpu_adjust_dppclk(): adjust operation on RequiredDPPCLK that is a double. Signed-off-by: Melissa Wen --- drivers/gpu/drm/amd/display/dc/dcn20/Makefile | 25 - .../drm/amd/display/dc/dcn20/dcn20_resource.c | 1370 +--- .../drm/amd/display/dc/dcn20/dcn20_resource.h | 30 +- .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.c | 1385 + .../drm/amd/display/dc/dml/dcn20/dcn20_fpu.h | 42 +- 5 files changed, 1451 insertions(+), 1401 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/Makefile b/drivers/gpu/drm/amd/display/dc/dcn20/Makefile index 5fcaf78334ff..abaed2121feb 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dcn20/Makefile @@ -9,31 +9,6 @@ DCN20 = dcn20_resource.o dcn20_init.o dcn20_hwseq.o dcn20_dpp.o dcn20_dpp_cm.o d DCN20 += dcn20_dsc.o -ifdef CONFIG_X86 -CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o := -mhard-float -msse -endif - -ifdef CONFIG_PPC64 -CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o := -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). -CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o += -mpreferred-stack-boundary=4 -else -CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o += -msse2 -endif -endif - AMD_DAL_DCN20 = $(addprefix $(AMDDALPATH)/dc/dcn20/,$(DCN20)) AMD_DISPLAY_FILES += $(AMD_DAL_DCN20) 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 dfe2e1c25a26..63c50bee0144 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c @@ -63,7 +63,6 @@ #include "dcn20_dccg.h" #include "dcn20_vmid.h" #include "dc_link_ddc.h" -#include "dc_link_dp.h" #include "dce/dce_panel_cntl.h" #include "navi10_ip_offset.h" @@ -93,367 +92,6 @@ #define DC_LOGGER_INIT(logger) -struct _vcs_dpi_ip_params_st dcn2_0_ip = { - .odm_capable = 1, - .gpuvm_enable = 0, - .hostvm_enable = 0, - .gpuvm_max_page_table_levels = 4, - .hostvm_max_page_table_levels = 4, - .hostvm_cached_page_table_levels = 0, - .pte_group_size_bytes = 2048, - .num_dsc = 6, - .rob_buffer_size_kbytes = 168, - .det_buffer_size_kbytes = 164, - .dpte_buffer_size_in_pte_reqs_luma = 84, - .pde_proc_buffer_size_64k_reqs = 48, - .dpp_output_buffer_pixels = 2560, - .opp_output_buffer_lines = 1, - .pixel_chunk_size_kbytes = 8, - .pte_chunk_size_kbytes = 2, - .meta_chunk_size_kbytes = 2, - .writeback_chunk_size_kbytes = 2, - .line_buffer_size_bits = 789504, - .is_line_buffer_bpp_fixed = 0, - .line_buffer_fixed_bpp = 0, - .dcc_supported = true, - .max_line_buffer_lines = 12, - .writeback_luma_buffer_size_kbytes = 12, - .writeback_chroma_buffer_size_kbytes = 8, - .writeback_chroma_line_buffer_width_pixels = 4, - .writeback_max_hscl_ratio = 1, - .writeback_max_vscl_ratio = 1, - .writeback_min_hscl_ratio = 1, - .writeback_min_vscl_ratio = 1, - .writeback_max_hscl_taps = 12, - .writeback_max_vscl_taps = 12, -
Re: [PATCH 0/2]
Hi Zhenneng, + some display folks First of all, thanks a lot for your patch. We had a similar patch in the past, but we had to revert it because we cannot simply enable DCN for ARM-based systems. You can refer to this commit message to get a better context: https://gitlab.freedesktop.org/agd5f/linux/-/commit/c241ed2f0ea549c18cff62a3708b43846b84dae3 Before enabling ARM, we first need to isolate all FPU code in the DML folder fully. You can read more about our strategy at the below link: https://patchwork.freedesktop.org/series/93042/ And you can see some examples of this effort in the below links: - https://patchwork.freedesktop.org/series/95504/ - https://patchwork.freedesktop.org/patch/455465/?series=94441=3 - https://patchwork.freedesktop.org/series/98247/ Soon we will submit another series that isolate DCN302, but we still need to isolate code from DCN20, DCN10, DCN303, and DCN301. If you want to help us speed up this process, feel free to look at DCN301 or DCN10. Best Regards Siqueira On 2022-01-07 4:57 a.m., Zhenneng Li wrote: For adapting radeon rx6600 xt on arm64 platform, there report some compile errors. Zhenneng Li (2): drm/amdgpu: fix compile error for dcn on arm64 drm/amdgpu: enable dcn support on arm64 drivers/gpu/drm/amd/display/Kconfig | 2 +- drivers/gpu/drm/amd/display/dc/calcs/Makefile | 6 + .../gpu/drm/amd/display/dc/clk_mgr/Makefile | 7 ++ drivers/gpu/drm/amd/display/dc/dcn10/Makefile | 4 +++ drivers/gpu/drm/amd/display/dc/dcn20/Makefile | 4 +++ .../gpu/drm/amd/display/dc/dcn201/Makefile| 6 + drivers/gpu/drm/amd/display/dc/dcn21/Makefile | 4 +++ drivers/gpu/drm/amd/display/dc/dcn30/Makefile | 6 + .../gpu/drm/amd/display/dc/dcn302/Makefile| 6 + .../gpu/drm/amd/display/dc/dcn303/Makefile| 6 + drivers/gpu/drm/amd/display/dc/dcn31/Makefile | 6 + drivers/gpu/drm/amd/display/dc/dml/Makefile | 25 +++ drivers/gpu/drm/amd/display/dc/dsc/Makefile | 7 ++ 13 files changed, 88 insertions(+), 1 deletion(-)
Re: [PATCH 1/2] drm/amd/display: Reduce stack size for dml31_ModeSupportAndSystemConfigurationFull
On 2021-12-13 4:46 a.m., Michel Dänzer wrote: On 2021-12-11 13:20, Rodrigo Siqueira Jordao wrote: On 2021-12-09 11:46 a.m., Michel Dänzer wrote: From: Michel Dänzer Move code using the Pipe struct to a new helper function. Works around[0] this warning (resulting in failure to build a RHEL debug kernel with Werror enabled): ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c: In function ‘dml31_ModeSupportAndSystemConfigurationFull’: ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c:5740:1: warning: the frame size of 2144 bytes is larger than 2048 bytes [-Wframe-larger-than=] The culprit seems to be the Pipe struct, so pull the relevant block out into its own sub-function. (This is porting a62427ef9b55 "drm/amd/display: Reduce stack size for dml21_ModeSupportAndSystemConfigurationFull" from dml31 to dml21) [0] AFAICT this doesn't actually reduce the total amount of stack which can be used, just moves some of it from dml31_ModeSupportAndSystemConfigurationFull to the new helper function, so the former happens to no longer exceed the limit for a single function. Signed-off-by: Michel Dänzer --- .../dc/dml/dcn31/display_mode_vba_31.c | 185 ++ 1 file changed, 99 insertions(+), 86 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c index 7e937bdcea00..8965f9af9d0a 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c @@ -3949,6 +3949,102 @@ static double TruncToValidBPP( return BPP_INVALID; } +static noinline void CalculatePrefetchSchedulePerPlane( + struct display_mode_lib *mode_lib, + double HostVMInefficiencyFactor, + int i, + unsigned j, + unsigned k) +{ + struct vba_vars_st *v = _lib->vba; + Pipe myPipe; + + myPipe.DPPCLK = v->RequiredDPPCLK[i][j][k]; + myPipe.DISPCLK = v->RequiredDISPCLK[i][j]; + myPipe.PixelClock = v->PixelClock[k]; + myPipe.DCFCLKDeepSleep = v->ProjectedDCFCLKDeepSleep[i][j]; + myPipe.DPPPerPlane = v->NoOfDPP[i][j][k]; + myPipe.ScalerEnabled = v->ScalerEnabled[k]; + myPipe.SourceScan = v->SourceScan[k]; + myPipe.BlockWidth256BytesY = v->Read256BlockWidthY[k]; + myPipe.BlockHeight256BytesY = v->Read256BlockHeightY[k]; + myPipe.BlockWidth256BytesC = v->Read256BlockWidthC[k]; + myPipe.BlockHeight256BytesC = v->Read256BlockHeightC[k]; + myPipe.InterlaceEnable = v->Interlace[k]; + myPipe.NumberOfCursors = v->NumberOfCursors[k]; + myPipe.VBlank = v->VTotal[k] - v->VActive[k]; + myPipe.HTotal = v->HTotal[k]; + myPipe.DCCEnable = v->DCCEnable[k]; + myPipe.ODMCombineIsEnabled = v->ODMCombineEnablePerState[i][k] == dm_odm_combine_mode_4to1 + || v->ODMCombineEnablePerState[i][k] == dm_odm_combine_mode_2to1; + myPipe.SourcePixelFormat = v->SourcePixelFormat[k]; + myPipe.BytePerPixelY = v->BytePerPixelY[k]; + myPipe.BytePerPixelC = v->BytePerPixelC[k]; + myPipe.ProgressiveToInterlaceUnitInOPP = v->ProgressiveToInterlaceUnitInOPP; + v->NoTimeForPrefetch[i][j][k] = CalculatePrefetchSchedule( + mode_lib, + HostVMInefficiencyFactor, + , + v->DSCDelayPerState[i][k], + v->DPPCLKDelaySubtotal + v->DPPCLKDelayCNVCFormater, + v->DPPCLKDelaySCL, + v->DPPCLKDelaySCLLBOnly, + v->DPPCLKDelayCNVCCursor, + v->DISPCLKDelaySubtotal, + v->SwathWidthYThisState[k] / v->HRatio[k], + v->OutputFormat[k], + v->MaxInterDCNTileRepeaters, + dml_min(v->MaxVStartup, v->MaximumVStartup[i][j][k]), + v->MaximumVStartup[i][j][k], + v->GPUVMMaxPageTableLevels, + v->GPUVMEnable, + v->HostVMEnable, + v->HostVMMaxNonCachedPageTableLevels, + v->HostVMMinPageSize, + v->DynamicMetadataEnable[k], + v->DynamicMetadataVMEnabled, + v->DynamicMetadataLinesBeforeActiveRequired[k], + v->DynamicMetadataTransmittedBytes[k], + v->UrgLatency[i], + v->ExtraLatency, + v->TimeCalc, + v->PDEAndMetaPTEBytesPerFrame[i][j][k], + v->MetaRowBytes[i][j][k], + v->DPTEBytesPerRow[i][j][k], + v->PrefetchLinesY[i][j][k], + v->SwathWidthYThisState[k], + v->PrefillY[k], + v->MaxNumSwY[k], + v->PrefetchLinesC[i][j][k], + v->SwathWidthCThisState[k], + v->PrefillC[k], + v->MaxNumSwC[k], + v->swath_width_luma_ub_this_state[k], + v->swath_width_chroma_ub_this_state[k], + v->SwathHeightYThisState[k], + v->SwathHeightCThisState[k], + v->TWait, +
Re: [PATCH 1/2] drm/amd/display: Reduce stack size for dml31_ModeSupportAndSystemConfigurationFull
On 2021-12-09 11:46 a.m., Michel Dänzer wrote: From: Michel Dänzer Move code using the Pipe struct to a new helper function. Works around[0] this warning (resulting in failure to build a RHEL debug kernel with Werror enabled): ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c: In function ‘dml31_ModeSupportAndSystemConfigurationFull’: ../drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn31/display_mode_vba_31.c:5740:1: warning: the frame size of 2144 bytes is larger than 2048 bytes [-Wframe-larger-than=] The culprit seems to be the Pipe struct, so pull the relevant block out into its own sub-function. (This is porting a62427ef9b55 "drm/amd/display: Reduce stack size for dml21_ModeSupportAndSystemConfigurationFull" from dml31 to dml21) [0] AFAICT this doesn't actually reduce the total amount of stack which can be used, just moves some of it from dml31_ModeSupportAndSystemConfigurationFull to the new helper function, so the former happens to no longer exceed the limit for a single function. Signed-off-by: Michel Dänzer --- .../dc/dml/dcn31/display_mode_vba_31.c| 185 ++ 1 file changed, 99 insertions(+), 86 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c index 7e937bdcea00..8965f9af9d0a 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c @@ -3949,6 +3949,102 @@ static double TruncToValidBPP( return BPP_INVALID; } +static noinline void CalculatePrefetchSchedulePerPlane( + struct display_mode_lib *mode_lib, + double HostVMInefficiencyFactor, + int i, + unsigned j, + unsigned k) +{ + struct vba_vars_st *v = _lib->vba; + Pipe myPipe; + + myPipe.DPPCLK = v->RequiredDPPCLK[i][j][k]; + myPipe.DISPCLK = v->RequiredDISPCLK[i][j]; + myPipe.PixelClock = v->PixelClock[k]; + myPipe.DCFCLKDeepSleep = v->ProjectedDCFCLKDeepSleep[i][j]; + myPipe.DPPPerPlane = v->NoOfDPP[i][j][k]; + myPipe.ScalerEnabled = v->ScalerEnabled[k]; + myPipe.SourceScan = v->SourceScan[k]; + myPipe.BlockWidth256BytesY = v->Read256BlockWidthY[k]; + myPipe.BlockHeight256BytesY = v->Read256BlockHeightY[k]; + myPipe.BlockWidth256BytesC = v->Read256BlockWidthC[k]; + myPipe.BlockHeight256BytesC = v->Read256BlockHeightC[k]; + myPipe.InterlaceEnable = v->Interlace[k]; + myPipe.NumberOfCursors = v->NumberOfCursors[k]; + myPipe.VBlank = v->VTotal[k] - v->VActive[k]; + myPipe.HTotal = v->HTotal[k]; + myPipe.DCCEnable = v->DCCEnable[k]; + myPipe.ODMCombineIsEnabled = v->ODMCombineEnablePerState[i][k] == dm_odm_combine_mode_4to1 + || v->ODMCombineEnablePerState[i][k] == dm_odm_combine_mode_2to1; + myPipe.SourcePixelFormat = v->SourcePixelFormat[k]; + myPipe.BytePerPixelY = v->BytePerPixelY[k]; + myPipe.BytePerPixelC = v->BytePerPixelC[k]; + myPipe.ProgressiveToInterlaceUnitInOPP = v->ProgressiveToInterlaceUnitInOPP; + v->NoTimeForPrefetch[i][j][k] = CalculatePrefetchSchedule( + mode_lib, + HostVMInefficiencyFactor, + , + v->DSCDelayPerState[i][k], + v->DPPCLKDelaySubtotal + v->DPPCLKDelayCNVCFormater, + v->DPPCLKDelaySCL, + v->DPPCLKDelaySCLLBOnly, + v->DPPCLKDelayCNVCCursor, + v->DISPCLKDelaySubtotal, + v->SwathWidthYThisState[k] / v->HRatio[k], + v->OutputFormat[k], + v->MaxInterDCNTileRepeaters, + dml_min(v->MaxVStartup, v->MaximumVStartup[i][j][k]), + v->MaximumVStartup[i][j][k], + v->GPUVMMaxPageTableLevels, + v->GPUVMEnable, + v->HostVMEnable, + v->HostVMMaxNonCachedPageTableLevels, + v->HostVMMinPageSize, + v->DynamicMetadataEnable[k], + v->DynamicMetadataVMEnabled, + v->DynamicMetadataLinesBeforeActiveRequired[k], + v->DynamicMetadataTransmittedBytes[k], + v->UrgLatency[i], + v->ExtraLatency, + v->TimeCalc, + v->PDEAndMetaPTEBytesPerFrame[i][j][k], + v->MetaRowBytes[i][j][k], + v->DPTEBytesPerRow[i][j][k], + v->PrefetchLinesY[i][j][k], + v->SwathWidthYThisState[k], + v->PrefillY[k], + v->MaxNumSwY[k], + v->PrefetchLinesC[i][j][k], + v->SwathWidthCThisState[k], + v->PrefillC[k], + v->MaxNumSwC[k], + v->swath_width_luma_ub_this_state[k], + v->swath_width_chroma_ub_this_state[k], +
Re: [PATCH v4 0/6] Expand display core documentation
On 2021-12-09 4:04 p.m., Yann Dirson wrote: Thanks for this. It's really good to see this. Reviewed-by: Harry Wentland Hearfully seconded, let's get this rolling :) Reviewed-by: Yann Dirson Series applied to amd-staging-drm-next Thanks a lot! Harry On 2021-12-09 09:20, Rodrigo Siqueira wrote: Display Core (DC) is one of the components under amdgpu, and it has multiple features directly related to the KMS API. Unfortunately, we don't have enough documentation about DC in the upstream, which makes the life of some external contributors a little bit more challenging. For these reasons, this patchset reworks part of the DC documentation and introduces a new set of details on how the display core works on DCN IP. Another improvement that this documentation effort tries to bring is making explicit some of our hardware-specific details to guide user-space developers better. In my view, it is easier to review this series if you apply it in your local kernel and build the HTML version (make htmldocs). I'm suggesting this approach because I added a few SVG diagrams that will be easier to see in the HTML version. If you cannot build the documentation, try to open the SVG images while reviewing the content. In summary, in this series, you will find: 1. Patch 1: Re-arrange of display core documentation. This is preparation work for the other patches, but it is also a way to expand this documentation. 2. Patch 2 to 4: Document some common debug options related to display. 3. Patch 5: This patch provides an overview of how our display core next works and a brief explanation of each component. 4. Patch 6: We use a lot of acronyms in our driver; for this reason, we exposed a glossary with common terms used by display core. Please let us know what you think we can improve this series and what kind of content you want to see for the next series. Changes since V3: - Add new acronyms to amdgpu glossary - Add link between dc and amdgpu glossary Changes since V2: - Add a comment about MMHUBBUB Changes since V1: - Group amdgpu documentation together. - Create index pages. - Mirror display folder in the documentation. - Divide glossary based on driver context. - Make terms more consistent and update CPLIB - Add new acronyms to the glossary Thanks Siqueira Rodrigo Siqueira (6): Documentation/gpu: Reorganize DC documentation Documentation/gpu: Document amdgpu_dm_visual_confirm debugfs entry Documentation/gpu: Document pipe split visual confirmation Documentation/gpu: How to collect DTN log Documentation/gpu: Add basic overview of DC pipeline Documentation/gpu: Add amdgpu and dc glossary Documentation/gpu/amdgpu-dc.rst | 74 -- Documentation/gpu/amdgpu/amdgpu-glossary.rst | 87 ++ .../gpu/amdgpu/display/config_example.svg | 414 ++ Documentation/gpu/amdgpu/display/dc-debug.rst | 77 ++ .../gpu/amdgpu/display/dc-glossary.rst| 237 .../amdgpu/display/dc_pipeline_overview.svg | 1125 + .../gpu/amdgpu/display/dcn-overview.rst | 171 +++ .../gpu/amdgpu/display/display-manager.rst| 42 + .../gpu/amdgpu/display/global_sync_vblank.svg | 485 +++ Documentation/gpu/amdgpu/display/index.rst| 29 + .../gpu/{amdgpu.rst => amdgpu/index.rst} | 25 +- Documentation/gpu/drivers.rst |3 +- 12 files changed, 2690 insertions(+), 79 deletions(-) delete mode 100644 Documentation/gpu/amdgpu-dc.rst create mode 100644 Documentation/gpu/amdgpu/amdgpu-glossary.rst create mode 100644 Documentation/gpu/amdgpu/display/config_example.svg create mode 100644 Documentation/gpu/amdgpu/display/dc-debug.rst create mode 100644 Documentation/gpu/amdgpu/display/dc-glossary.rst create mode 100644 Documentation/gpu/amdgpu/display/dc_pipeline_overview.svg create mode 100644 Documentation/gpu/amdgpu/display/dcn-overview.rst create mode 100644 Documentation/gpu/amdgpu/display/display-manager.rst create mode 100644 Documentation/gpu/amdgpu/display/global_sync_vblank.svg create mode 100644 Documentation/gpu/amdgpu/display/index.rst rename Documentation/gpu/{amdgpu.rst => amdgpu/index.rst} (95%)
Re: [PATCH v2 6/6] Documentation/gpu: Add amdgpu and dc glossary
On 2021-12-08 11:16 a.m., Yann Dirson wrote: Hi Rodrigo, On 2021-12-07 2:49 p.m., Yann Dirson wrote: On Thu, Dec 02, 2021 at 11:01:32AM -0500, Rodrigo Siqueira wrote: In the DC driver, we have multiple acronyms that are not obvious most of the time; the same idea is valid for amdgpu. This commit introduces a DC and amdgpu glossary in order to make it easier to navigate through our driver. Changes since V1: - Yann: Divide glossary based on driver context. - Alex: Make terms more consistent and update CPLIB - Add new acronyms to the glossary If you're rerolling, it could be a good time to include the additional (and detailed) entries from Alex's answer to "Looking for clarifications around gfx/kcq/kiq". Finding a way to fit the other details not fitting directly in the glossary will likely take more rounds, though, so we can wait for the first round to be merged before dealing with them. Hi Yann, I will send another version to address Daniel's comment, and I'll also expand the amdgpu acronyms glossary based on your mail thread with Alex. However, I don't want to add more details about that discussion in this series because I don't want to lose focus in this patchset since my main goal is to start to expand display documentation. fair enough By the way, I think you could consider writing a kernel-doc based on your discussion with Alex. This way, you can try to consolidate what you discover and get reviews in the content. that's the idea, though I'm not sure I have enough material to start with yet :) As you know, right now, we have zero documentation about that, which means that if you add something, it is an excellent improvement in the current status :) Thanks Siqueira Signed-off-by: Rodrigo Siqueira --- Documentation/gpu/amdgpu/amdgpu-glossary.rst | 47 .../gpu/amdgpu/display/dc-glossary.rst| 243 ++ Documentation/gpu/amdgpu/display/index.rst| 1 + Documentation/gpu/amdgpu/index.rst| 7 + 4 files changed, 298 insertions(+) create mode 100644 Documentation/gpu/amdgpu/amdgpu-glossary.rst create mode 100644 Documentation/gpu/amdgpu/display/dc-glossary.rst diff --git a/Documentation/gpu/amdgpu/amdgpu-glossary.rst b/Documentation/gpu/amdgpu/amdgpu-glossary.rst new file mode 100644 index ..e635851025e7 --- /dev/null +++ b/Documentation/gpu/amdgpu/amdgpu-glossary.rst @@ -0,0 +1,47 @@ +=== +AMDGPU Glossary +=== + +Here you can find some generic acronyms used in the amdgpu driver. Notice that +we have a dedicated glossary for Display Core. Maybe add a link to that here so it's easier to find? sphinx autogenerates header targets so pretty easy (if the heading is unique at least). -Daniel + +.. glossary:: + +CPLIB + Content Protection Library + +DFS + Digital Frequency Synthesizer + +ECP + Enhanced Content Protection + +EOP + End Of Pipe/Pipeline + +HQD + Hardware Queue Descriptor + +KCQ + Kernel Compute Queue + +KGQ + Kernel Graphics Queue + +KIQ + Kernel Interface Queue + +MQD + Memory Queue Descriptor + +PPLib + PowerPlay Library - PowerPlay is the power management component. + +SMU + System Management Unit + +VCE + Video Compression Engine + +VCN + Video Codec Next diff --git a/Documentation/gpu/amdgpu/display/dc-glossary.rst b/Documentation/gpu/amdgpu/display/dc-glossary.rst new file mode 100644 index ..547c0bfbb3e2 --- /dev/null +++ b/Documentation/gpu/amdgpu/display/dc-glossary.rst @@ -0,0 +1,243 @@ +=== +DC Glossary +=== + +On this page, we try to keep track of acronyms related to the display +component. If you do not find what you are looking for, look at the amdgpu +glossary; if you cannot find it anywhere, consider asking in the amdgfx and +update this page. + +.. glossary:: + +ABM + Adaptive Backlight Modulation + +APU + Accelerated Processing Unit + +ASIC + Application-Specific Integrated Circuit + +ASSR + Alternate Scrambler Seed Reset + +AZ + Azalia (HD audio DMA engine) + +BPC + Bits Per Colour/Component + +BPP + Bits Per Pixel + +Clocks + * PCLK: Pixel Clock + * SYMCLK: Symbol Clock + * SOCCLK: GPU Engine Clock + * DISPCLK: Display Clock + * DPPCLK: DPP Clock + * DCFCLK: Display Controller Fabric Clock + * REFCLK: Real Time Reference Clock + * PPLL: Pixel PLL + * FCLK: Fabric Clock + * MCLK: Memory Clock + +CRC + Cyclic Redundancy Check + +CRTC + Cathode Ray Tube Controller - commonly called "Controller" - Generates + raw stream of pixels, clocked at pixel clock + +CVT + Coordinated Video Timings + +DAL + Display Abstraction layer + +DC (Software) + Display Core + +DC (Hardware) + Display Controller + +DCC + Delta
Re: [PATCH v2 6/6] Documentation/gpu: Add amdgpu and dc glossary
On 2021-12-07 2:49 p.m., Yann Dirson wrote: On Thu, Dec 02, 2021 at 11:01:32AM -0500, Rodrigo Siqueira wrote: In the DC driver, we have multiple acronyms that are not obvious most of the time; the same idea is valid for amdgpu. This commit introduces a DC and amdgpu glossary in order to make it easier to navigate through our driver. Changes since V1: - Yann: Divide glossary based on driver context. - Alex: Make terms more consistent and update CPLIB - Add new acronyms to the glossary If you're rerolling, it could be a good time to include the additional (and detailed) entries from Alex's answer to "Looking for clarifications around gfx/kcq/kiq". Finding a way to fit the other details not fitting directly in the glossary will likely take more rounds, though, so we can wait for the first round to be merged before dealing with them. Hi Yann, I will send another version to address Daniel's comment, and I'll also expand the amdgpu acronyms glossary based on your mail thread with Alex. However, I don't want to add more details about that discussion in this series because I don't want to lose focus in this patchset since my main goal is to start to expand display documentation. By the way, I think you could consider writing a kernel-doc based on your discussion with Alex. This way, you can try to consolidate what you discover and get reviews in the content. Thanks Siqueira Signed-off-by: Rodrigo Siqueira --- Documentation/gpu/amdgpu/amdgpu-glossary.rst | 47 .../gpu/amdgpu/display/dc-glossary.rst| 243 ++ Documentation/gpu/amdgpu/display/index.rst| 1 + Documentation/gpu/amdgpu/index.rst| 7 + 4 files changed, 298 insertions(+) create mode 100644 Documentation/gpu/amdgpu/amdgpu-glossary.rst create mode 100644 Documentation/gpu/amdgpu/display/dc-glossary.rst diff --git a/Documentation/gpu/amdgpu/amdgpu-glossary.rst b/Documentation/gpu/amdgpu/amdgpu-glossary.rst new file mode 100644 index ..e635851025e7 --- /dev/null +++ b/Documentation/gpu/amdgpu/amdgpu-glossary.rst @@ -0,0 +1,47 @@ +=== +AMDGPU Glossary +=== + +Here you can find some generic acronyms used in the amdgpu driver. Notice that +we have a dedicated glossary for Display Core. Maybe add a link to that here so it's easier to find? sphinx autogenerates header targets so pretty easy (if the heading is unique at least). -Daniel + +.. glossary:: + +CPLIB + Content Protection Library + +DFS + Digital Frequency Synthesizer + +ECP + Enhanced Content Protection + +EOP + End Of Pipe/Pipeline + +HQD + Hardware Queue Descriptor + +KCQ + Kernel Compute Queue + +KGQ + Kernel Graphics Queue + +KIQ + Kernel Interface Queue + +MQD + Memory Queue Descriptor + +PPLib + PowerPlay Library - PowerPlay is the power management component. + +SMU + System Management Unit + +VCE + Video Compression Engine + +VCN + Video Codec Next diff --git a/Documentation/gpu/amdgpu/display/dc-glossary.rst b/Documentation/gpu/amdgpu/display/dc-glossary.rst new file mode 100644 index ..547c0bfbb3e2 --- /dev/null +++ b/Documentation/gpu/amdgpu/display/dc-glossary.rst @@ -0,0 +1,243 @@ +=== +DC Glossary +=== + +On this page, we try to keep track of acronyms related to the display +component. If you do not find what you are looking for, look at the amdgpu +glossary; if you cannot find it anywhere, consider asking in the amdgfx and +update this page. + +.. glossary:: + +ABM + Adaptive Backlight Modulation + +APU + Accelerated Processing Unit + +ASIC + Application-Specific Integrated Circuit + +ASSR + Alternate Scrambler Seed Reset + +AZ + Azalia (HD audio DMA engine) + +BPC + Bits Per Colour/Component + +BPP + Bits Per Pixel + +Clocks + * PCLK: Pixel Clock + * SYMCLK: Symbol Clock + * SOCCLK: GPU Engine Clock + * DISPCLK: Display Clock + * DPPCLK: DPP Clock + * DCFCLK: Display Controller Fabric Clock + * REFCLK: Real Time Reference Clock + * PPLL: Pixel PLL + * FCLK: Fabric Clock + * MCLK: Memory Clock + +CRC + Cyclic Redundancy Check + +CRTC + Cathode Ray Tube Controller - commonly called "Controller" - Generates + raw stream of pixels, clocked at pixel clock + +CVT + Coordinated Video Timings + +DAL + Display Abstraction layer + +DC (Software) + Display Core + +DC (Hardware) + Display Controller + +DCC + Delta Colour Compression + +DCE + Display Controller Engine + +DCHUB + Display Controller HUB + +ARB + Arbiter + +VTG + Vertical Timing Generator + +DCN + Display Core Next + +DCCG + Display Clock Generator block + +DDC + Display Data Channel + +DIO + Display IO
Re: [PATCH 1/6] Documentation/gpu: Reorganize DC documentation
On 2021-11-30 10:48 a.m., Harry Wentland wrote: On 2021-11-30 10:46, Rodrigo Siqueira Jordao wrote: On 2021-11-29 7:06 a.m., Jani Nikula wrote: On Fri, 26 Nov 2021, Daniel Vetter wrote: On Thu, Nov 25, 2021 at 10:38:25AM -0500, Rodrigo Siqueira wrote: Display core documentation is not well organized, and it is hard to find information due to the lack of sections. This commit reorganizes the documentation layout, and it is preparation work for future changes. Signed-off-by: Rodrigo Siqueira --- Documentation/gpu/amdgpu-dc.rst | 74 --- .../gpu/amdgpu-dc/amdgpu-dc-debug.rst | 4 + Documentation/gpu/amdgpu-dc/amdgpu-dc.rst | 29 Documentation/gpu/amdgpu-dc/amdgpu-dm.rst | 42 +++ Documentation/gpu/drivers.rst | 2 +- 5 files changed, 76 insertions(+), 75 deletions(-) delete mode 100644 Documentation/gpu/amdgpu-dc.rst create mode 100644 Documentation/gpu/amdgpu-dc/amdgpu-dc-debug.rst create mode 100644 Documentation/gpu/amdgpu-dc/amdgpu-dc.rst create mode 100644 Documentation/gpu/amdgpu-dc/amdgpu-dm.rst diff --git a/Documentation/gpu/amdgpu-dc.rst b/Documentation/gpu/amdgpu-dc.rst deleted file mode 100644 index f7ff7e1309de.. --- a/Documentation/gpu/amdgpu-dc.rst +++ /dev/null @@ -1,74 +0,0 @@ -=== -drm/amd/display - Display Core (DC) -=== - -*placeholder - general description of supported platforms, what dc is, etc.* - -Because it is partially shared with other operating systems, the Display Core -Driver is divided in two pieces. - -1. **Display Core (DC)** contains the OS-agnostic components. Things like - hardware programming and resource management are handled here. -2. **Display Manager (DM)** contains the OS-dependent components. Hooks to the - amdgpu base driver and DRM are implemented here. - -It doesn't help that the entire package is frequently referred to as DC. But -with the context in mind, it should be clear. - -When CONFIG_DRM_AMD_DC is enabled, DC will be initialized by default for -supported ASICs. To force disable, set `amdgpu.dc=0` on kernel command line. -Likewise, to force enable on unsupported ASICs, set `amdgpu.dc=1`. - -To determine if DC is loaded, search dmesg for the following entry: - -``Display Core initialized with `` - -AMDgpu Display Manager -== - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :doc: overview - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h - :internal: - -Lifecycle -- - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :doc: DM Lifecycle - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :functions: dm_hw_init dm_hw_fini - -Interrupts --- - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c - :doc: overview - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c - :internal: - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :functions: register_hpd_handlers dm_crtc_high_irq dm_pflip_high_irq - -Atomic Implementation -- - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :doc: atomic - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :functions: amdgpu_dm_atomic_check amdgpu_dm_atomic_commit_tail - -Display Core - - -**WIP** - -FreeSync Video --- - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :doc: FreeSync Video diff --git a/Documentation/gpu/amdgpu-dc/amdgpu-dc-debug.rst b/Documentation/gpu/amdgpu-dc/amdgpu-dc-debug.rst new file mode 100644 index ..bbb8c3fc8eee --- /dev/null +++ b/Documentation/gpu/amdgpu-dc/amdgpu-dc-debug.rst @@ -0,0 +1,4 @@ +Display Core Debug tools + + +TODO diff --git a/Documentation/gpu/amdgpu-dc/amdgpu-dc.rst b/Documentation/gpu/amdgpu-dc/amdgpu-dc.rst new file mode 100644 index ..3685b3b1ad64 --- /dev/null +++ b/Documentation/gpu/amdgpu-dc/amdgpu-dc.rst While we bikeshed names, I think it'd would make sense to call this overview.rst or intro.rst or similar, since it's meant to contain the overall toctree for everything amdgpu related (maybe there will be more in the future). index.rst? Hi, Thanks a lot for the suggestions; I will prepare a V2 that addresses all your comments. Ps.: If there is no objection, I'll rename amdgpu-dc to index as Jani suggested. SGTM, you mean amdgpu/index.rst, right? Yeah, but I'm also thinking about this new organization: 1. Create an amdgpu folder. 2. Inside amdgpu folder, I want to create a display folder. 3. Move all display documentation to the display folder and keep other amdgpu generic things under amdgpu. 4. Finally, inside the amdgpu folder, I'll create the index.rst for amdgpu, and inside the display folder, I will create a similar file
Re: [PATCH 6/6] Documentation/gpu: Add DC glossary
On 2021-11-29 3:48 p.m., ydir...@free.fr wrote: Hi Rodrigo, That will really be helpful! I know drawing the line is a difficult problem (and can even make things harder when searching), but maybe it would make sense to keep generic acronyms not specific to amdgpu in a separate list. I bet a number of them would be useful in the scope of other drm drivers (e.g. CRTC, DCC, MST), and some are not restricted to the drm subsystem at all (e.g. FEC, LUT), but still have value as not necessarily easy to look up. Maybe "DC glossary" should just be "Glossary", since quite some entries help to read adm/amdgpu/ too. Which brings me to the result of my recent searches as suggested entries: KIQ (Kernel Interface Queue), MQD (memory queue descriptor), HQD (hardware queue descriptor), EOP (still no clue :) Maybe some more specific ones just to be spelled out in clear where they are used ? KCQ (compute queue?), KGQ (gfx queue?) More suggestions inlined. Best regards, Hi all, I'll address all the highlighted problems in the V2. Thanks a lot for all the feedback. Yann, For the generic acronyms, how about keeping it in this patch for now? After it gets merged, I can prepare a new documentation patch that creates a glossary for DRM where I move the generic acronyms to the DRM documentation. I prefer this approach to keep the improvement small and manageable. Best Regards Siqueira
Re: [PATCH 1/6] Documentation/gpu: Reorganize DC documentation
On 2021-11-29 7:06 a.m., Jani Nikula wrote: On Fri, 26 Nov 2021, Daniel Vetter wrote: On Thu, Nov 25, 2021 at 10:38:25AM -0500, Rodrigo Siqueira wrote: Display core documentation is not well organized, and it is hard to find information due to the lack of sections. This commit reorganizes the documentation layout, and it is preparation work for future changes. Signed-off-by: Rodrigo Siqueira --- Documentation/gpu/amdgpu-dc.rst | 74 --- .../gpu/amdgpu-dc/amdgpu-dc-debug.rst | 4 + Documentation/gpu/amdgpu-dc/amdgpu-dc.rst | 29 Documentation/gpu/amdgpu-dc/amdgpu-dm.rst | 42 +++ Documentation/gpu/drivers.rst | 2 +- 5 files changed, 76 insertions(+), 75 deletions(-) delete mode 100644 Documentation/gpu/amdgpu-dc.rst create mode 100644 Documentation/gpu/amdgpu-dc/amdgpu-dc-debug.rst create mode 100644 Documentation/gpu/amdgpu-dc/amdgpu-dc.rst create mode 100644 Documentation/gpu/amdgpu-dc/amdgpu-dm.rst diff --git a/Documentation/gpu/amdgpu-dc.rst b/Documentation/gpu/amdgpu-dc.rst deleted file mode 100644 index f7ff7e1309de.. --- a/Documentation/gpu/amdgpu-dc.rst +++ /dev/null @@ -1,74 +0,0 @@ -=== -drm/amd/display - Display Core (DC) -=== - -*placeholder - general description of supported platforms, what dc is, etc.* - -Because it is partially shared with other operating systems, the Display Core -Driver is divided in two pieces. - -1. **Display Core (DC)** contains the OS-agnostic components. Things like - hardware programming and resource management are handled here. -2. **Display Manager (DM)** contains the OS-dependent components. Hooks to the - amdgpu base driver and DRM are implemented here. - -It doesn't help that the entire package is frequently referred to as DC. But -with the context in mind, it should be clear. - -When CONFIG_DRM_AMD_DC is enabled, DC will be initialized by default for -supported ASICs. To force disable, set `amdgpu.dc=0` on kernel command line. -Likewise, to force enable on unsupported ASICs, set `amdgpu.dc=1`. - -To determine if DC is loaded, search dmesg for the following entry: - -``Display Core initialized with `` - -AMDgpu Display Manager -== - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :doc: overview - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h - :internal: - -Lifecycle -- - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :doc: DM Lifecycle - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :functions: dm_hw_init dm_hw_fini - -Interrupts --- - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c - :doc: overview - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c - :internal: - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :functions: register_hpd_handlers dm_crtc_high_irq dm_pflip_high_irq - -Atomic Implementation -- - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :doc: atomic - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :functions: amdgpu_dm_atomic_check amdgpu_dm_atomic_commit_tail - -Display Core - - -**WIP** - -FreeSync Video --- - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :doc: FreeSync Video diff --git a/Documentation/gpu/amdgpu-dc/amdgpu-dc-debug.rst b/Documentation/gpu/amdgpu-dc/amdgpu-dc-debug.rst new file mode 100644 index ..bbb8c3fc8eee --- /dev/null +++ b/Documentation/gpu/amdgpu-dc/amdgpu-dc-debug.rst @@ -0,0 +1,4 @@ +Display Core Debug tools + + +TODO diff --git a/Documentation/gpu/amdgpu-dc/amdgpu-dc.rst b/Documentation/gpu/amdgpu-dc/amdgpu-dc.rst new file mode 100644 index ..3685b3b1ad64 --- /dev/null +++ b/Documentation/gpu/amdgpu-dc/amdgpu-dc.rst While we bikeshed names, I think it'd would make sense to call this overview.rst or intro.rst or similar, since it's meant to contain the overall toctree for everything amdgpu related (maybe there will be more in the future). index.rst? Hi, Thanks a lot for the suggestions; I will prepare a V2 that addresses all your comments. Ps.: If there is no objection, I'll rename amdgpu-dc to index as Jani suggested. Thanks.
Re: [PATCH] drm/amd/display: fix application of sizeof to pointer
On 2021-11-23 10:04 p.m., cgel@gmail.com wrote: From: Lv Ruyi Both of split and merge are pointers, not arrays. Reported-by: Zeal Robot Signed-off-by: Lv Ruyi --- drivers/gpu/drm/amd/display/dc/dml/dml_wrapper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dml_wrapper.c b/drivers/gpu/drm/amd/display/dc/dml/dml_wrapper.c index ece34b0b8a46..91810aaee5a3 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dml_wrapper.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dml_wrapper.c @@ -1223,8 +1223,8 @@ static void dml_full_validate_bw_helper(struct dc *dc, *pipe_cnt = dml_populate_dml_pipes_from_context(dc, context, pipes, false); *vlevel = dml_get_voltage_level(>bw_ctx.dml, pipes, *pipe_cnt); if (*vlevel < context->bw_ctx.dml.soc.num_states) { - memset(split, 0, sizeof(split)); - memset(merge, 0, sizeof(merge)); + memset(split, 0, MAX_PIPES * sizeof(*split)); + memset(merge, 0, MAX_PIPES * sizeof(*merge)); *vlevel = dml_validate_apply_pipe_split_flags(dc, context, *vlevel, split, merge); } Nice catch! Reviewed-by: Rodrigo Siqueira and applied to amd-staging-drm-next Thanks Siqueira
Re: [PATCH] drm/amd/display: Fix warning comparing pointer to 0
On 2021-11-24 5:20 a.m., Jiapeng Chong wrote: Fix the following coccicheck warning: ./drivers/gpu/drm/amd/display/dc/dml/dsc/rc_calc_fpu.c:96:14-15: WARNING comparing pointer to 0. Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong --- drivers/gpu/drm/amd/display/dc/dml/dsc/rc_calc_fpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/dc/dml/dsc/rc_calc_fpu.c b/drivers/gpu/drm/amd/display/dc/dml/dsc/rc_calc_fpu.c index 122ba29..ec636d0 100644 --- a/drivers/gpu/drm/amd/display/dc/dml/dsc/rc_calc_fpu.c +++ b/drivers/gpu/drm/amd/display/dc/dml/dsc/rc_calc_fpu.c @@ -93,7 +93,7 @@ static void get_qp_set(qp_set qps, enum colour_mode cm, enum bits_per_comp bpc, TABLE_CASE(420, 12, min); } - if (table == 0) + if (!table) return; index = (bpp - table[0].bpp) * 2; Reviewed-by: Rodrigo Siqueira Applied to amd-staging-drm-next Thanks