Re: [PATCH] drm/amd/display: fix documentation warnings for mpc.h

2024-04-30 Thread Rodrigo Siqueira Jordao

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

2024-04-23 Thread Rodrigo Siqueira Jordao




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

2024-04-23 Thread Rodrigo Siqueira Jordao




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

2024-04-23 Thread Rodrigo Siqueira Jordao




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

2024-02-28 Thread Rodrigo Siqueira Jordao




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()

2024-02-21 Thread Rodrigo Siqueira Jordao




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

2024-02-21 Thread Rodrigo Siqueira Jordao

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

2024-01-08 Thread Rodrigo Siqueira Jordao




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

2023-12-06 Thread Rodrigo Siqueira Jordao




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

2023-12-06 Thread Rodrigo Siqueira Jordao




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

2023-10-30 Thread Rodrigo Siqueira Jordao

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.

2023-10-04 Thread Rodrigo Siqueira Jordao




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

2023-09-13 Thread Rodrigo Siqueira Jordao




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

2023-09-13 Thread Rodrigo Siqueira Jordao




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

2023-09-13 Thread Rodrigo Siqueira Jordao




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()

2023-05-17 Thread Rodrigo Siqueira Jordao




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()

2023-05-17 Thread Rodrigo Siqueira Jordao




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()

2023-05-17 Thread Rodrigo Siqueira Jordao




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

2023-03-16 Thread Rodrigo Siqueira Jordao




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

2023-03-10 Thread Rodrigo Siqueira Jordao

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()

2023-03-10 Thread Rodrigo Siqueira Jordao




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

2023-03-07 Thread Rodrigo Siqueira Jordao




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

2023-01-12 Thread Rodrigo Siqueira Jordao




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

2023-01-12 Thread Rodrigo Siqueira Jordao




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

2023-01-10 Thread Rodrigo Siqueira Jordao




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

2023-01-10 Thread Rodrigo Siqueira Jordao




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

2022-10-20 Thread Rodrigo Siqueira Jordao




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()

2022-10-04 Thread Rodrigo Siqueira Jordao




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

2022-10-04 Thread Rodrigo Siqueira Jordao




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'

2022-10-04 Thread Rodrigo Siqueira Jordao




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

2022-10-03 Thread Rodrigo Siqueira Jordao




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

2022-09-15 Thread Rodrigo Siqueira Jordao

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

2022-09-13 Thread Rodrigo Siqueira Jordao




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

2022-09-12 Thread Rodrigo Siqueira Jordao




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

2022-09-06 Thread Rodrigo Siqueira Jordao




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()

2022-09-06 Thread Rodrigo Siqueira Jordao




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()

2022-09-06 Thread Rodrigo Siqueira Jordao




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()

2022-09-01 Thread Rodrigo Siqueira Jordao




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

2022-08-10 Thread Rodrigo Siqueira Jordao

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

2022-08-04 Thread Rodrigo Siqueira Jordao




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

2022-08-04 Thread Rodrigo Siqueira Jordao




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

2022-08-04 Thread Rodrigo Siqueira Jordao




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

2022-08-04 Thread Rodrigo Siqueira Jordao




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

2022-08-03 Thread Rodrigo Siqueira Jordao




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

2022-08-02 Thread Rodrigo Siqueira Jordao




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

2022-08-02 Thread Rodrigo Siqueira Jordao




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

2022-08-02 Thread Rodrigo Siqueira Jordao




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

2022-08-02 Thread Rodrigo Siqueira Jordao




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

2022-07-27 Thread Rodrigo Siqueira Jordao




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

2022-07-22 Thread Rodrigo Siqueira Jordao




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

2022-07-21 Thread Rodrigo Siqueira Jordao




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

2022-07-21 Thread Rodrigo Siqueira Jordao




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

2022-07-21 Thread Rodrigo Siqueira Jordao




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

2022-07-21 Thread Rodrigo Siqueira Jordao




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

2022-07-21 Thread Rodrigo Siqueira Jordao




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

2022-07-21 Thread Rodrigo Siqueira Jordao




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

2022-06-30 Thread Rodrigo Siqueira Jordao




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

2022-06-22 Thread Rodrigo Siqueira Jordao

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

2022-04-26 Thread Rodrigo Siqueira Jordao




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

2022-04-07 Thread Rodrigo Siqueira Jordao

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

2022-03-30 Thread Rodrigo Siqueira Jordao




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

2022-03-30 Thread Rodrigo Siqueira Jordao




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

2022-03-16 Thread Rodrigo Siqueira Jordao




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

2022-02-23 Thread Rodrigo Siqueira Jordao




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]

2022-01-07 Thread Rodrigo Siqueira Jordao

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

2021-12-13 Thread Rodrigo Siqueira Jordao




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

2021-12-11 Thread Rodrigo Siqueira Jordao




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

2021-12-10 Thread Rodrigo Siqueira Jordao




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

2021-12-08 Thread Rodrigo Siqueira Jordao




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

2021-12-08 Thread Rodrigo Siqueira Jordao




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

2021-11-30 Thread Rodrigo Siqueira Jordao




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

2021-11-30 Thread Rodrigo Siqueira Jordao




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

2021-11-30 Thread Rodrigo Siqueira Jordao




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

2021-11-25 Thread Rodrigo Siqueira Jordao




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

2021-11-24 Thread Rodrigo Siqueira Jordao




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