Re: [PATCH 3/5] drm/amd/display: Remove unused dce60_clk_mgr register definitions

[Rodrigo Siqueira <siqueira@igalia.com>]

On 01/18, Timur Kristóf wrote:
> It turned out that these were actually not necessary.
> 
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>  .../display/dc/clk_mgr/dce60/dce60_clk_mgr.c  | 30 ++-----------------
>  .../amd/display/dc/inc/hw/clk_mgr_internal.h  | 11 -------
>  2 files changed, 3 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/dce60/dce60_clk_mgr.c b/drivers/gpu/drm/amd/display/dc/clk_mgr/dce60/dce60_clk_mgr.c
> index 69dd80d9f738..1fdf344efe1a 100644
> --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/dce60/dce60_clk_mgr.c
> +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/dce60/dce60_clk_mgr.c
> @@ -43,30 +43,6 @@
>  #include "dce/dce_6_0_d.h"
>  #include "dce/dce_6_0_sh_mask.h"
>  
> -#define REG(reg) \
> -	(clk_mgr->regs->reg)
> -
> -#undef FN
> -#define FN(reg_name, field_name) \
> -	clk_mgr->clk_mgr_shift->field_name, clk_mgr->clk_mgr_mask->field_name
> -
> -/* set register offset */
> -#define SR(reg_name)\
> -	.reg_name = mm ## reg_name
> -
> -static const struct clk_mgr_registers disp_clk_regs = {
> -		CLK_COMMON_REG_LIST_DCE60_BASE()
> -};
> -
> -static const struct clk_mgr_shift disp_clk_shift = {
> -		CLK_COMMON_MASK_SH_LIST_DCE60_COMMON_BASE(__SHIFT)
> -};
> -
> -static const struct clk_mgr_mask disp_clk_mask = {
> -		CLK_COMMON_MASK_SH_LIST_DCE60_COMMON_BASE(_MASK)
> -};
> -
> -
>  /* Max clock values for each state indexed by "enum clocks_state": */
>  static const struct state_dependent_clocks dce60_max_clks_by_state[] = {
>  /* ClocksStateInvalid - should not be used */
> @@ -155,9 +131,9 @@ void dce60_clk_mgr_construct(
>  		dce60_max_clks_by_state,
>  		sizeof(dce60_max_clks_by_state));
>  
> -	clk_mgr->regs = &disp_clk_regs;
> -	clk_mgr->clk_mgr_shift = &disp_clk_shift;
> -	clk_mgr->clk_mgr_mask = &disp_clk_mask;
> +	clk_mgr->regs = NULL;
> +	clk_mgr->clk_mgr_shift = NULL;
> +	clk_mgr->clk_mgr_mask = NULL;

After looking into the next commit, I got why you did this change. Maybe
for future patches, expand the commit message and mention that this is a
transition step. Anyway:

Reviewed-by: Rodrigo Siqueira <siqueira@igalia.com>

>  	clk_mgr->base.funcs = &dce60_funcs;
>  
>  	base->clks.max_supported_dispclk_khz =
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr_internal.h b/drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr_internal.h
> index bac8febad69a..836a28134d41 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr_internal.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/clk_mgr_internal.h
> @@ -89,11 +89,6 @@ enum dentist_divider_range {
>  	.DPREFCLK_CNTL = mmDPREFCLK_CNTL, \
>  	.DENTIST_DISPCLK_CNTL = mmDENTIST_DISPCLK_CNTL
>  
> -#if defined(CONFIG_DRM_AMD_DC_SI)
> -#define CLK_COMMON_REG_LIST_DCE60_BASE() \
> -	SR(DENTIST_DISPCLK_CNTL)
> -#endif
> -
>  #define CLK_COMMON_REG_LIST_DCN_BASE() \
>  	SR(DENTIST_DISPCLK_CNTL)
>  
> @@ -119,12 +114,6 @@ enum dentist_divider_range {
>  	CLK_SF(DPREFCLK_CNTL, DPREFCLK_SRC_SEL, mask_sh), \
>  	CLK_SF(DENTIST_DISPCLK_CNTL, DENTIST_DPREFCLK_WDIVIDER, mask_sh)
>  
> -#if defined(CONFIG_DRM_AMD_DC_SI)
> -#define CLK_COMMON_MASK_SH_LIST_DCE60_COMMON_BASE(mask_sh) \
> -	CLK_SF(DENTIST_DISPCLK_CNTL, DENTIST_DISPCLK_WDIVIDER, mask_sh),\
> -	CLK_SF(DENTIST_DISPCLK_CNTL, DENTIST_DISPCLK_CHG_DONE, mask_sh)
> -#endif
> -
>  #define CLK_COMMON_MASK_SH_LIST_DCN_COMMON_BASE(mask_sh) \
>  	CLK_SF(DENTIST_DISPCLK_CNTL, DENTIST_DISPCLK_WDIVIDER, mask_sh),\
>  	CLK_SF(DENTIST_DISPCLK_CNTL, DENTIST_DISPCLK_CHG_DONE, mask_sh)
> -- 
> 2.52.0
> 

-- 
Rodrigo Siqueira
https://siqueira.tech