Re: [PATCH 8/8] amdgpu/pm: Optimize emit_clock_levels for aldebaran - part 3

["Lazar, Lijo" <lijo.lazar@xxxxxxx>]

On 4/27/2023 11:57 AM, Darren Powell wrote:
>     split switch statement into two and consolidate the common
>     code for printing most of the types of clock speeds
>
> Signed-off-by: Darren Powell <darren.powell@xxxxxxx>
> ---
>   .../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c    | 82 ++++++-------------
>   1 file changed, 27 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> index b315353adfdf..9d37800bfae9 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c
> @@ -788,17 +788,6 @@ static int aldebaran_emit_clk_levels(struct smu_context *smu,
>   			freq_values[2] = max_clk;
>   			freq_values[1] = cur_value;
>   		}
> -
> -		for (i = 0; i < display_levels; i++) {
> -			clock_mhz = freq_values[i];
> -			freq_match = aldebaran_freqs_in_same_level(clock_mhz, cur_value);
> -			freq_match |= (display_levels == 1);
> -
> -			*offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i,
> -				clock_mhz,
> -				(freq_match) ? "*" : "");
> -		}
> -
>   		break;
>   
>   	case SMU_OD_MCLK:
> @@ -813,16 +802,6 @@ static int aldebaran_emit_clk_levels(struct smu_context *smu,
>   
>   		single_dpm_table = &(dpm_context->dpm_tables.uclk_table);
>   		aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> -
> -		for (i = 0; i < clocks.num_levels; i++) {
> -			clock_mhz = clocks.data[i].clocks_in_khz / 1000;
> -			freq_match = aldebaran_freqs_in_same_level(clock_mhz, cur_value);
> -			freq_match |= (clocks.num_levels == 1);
> -
> -			*offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
> -					i, clock_mhz,
> -					(freq_match) ? "*" : "");
> -		}
>   		break;
>   
>   	case SMU_SOCCLK:
> @@ -834,16 +813,6 @@ static int aldebaran_emit_clk_levels(struct smu_context *smu,
>   
>   		single_dpm_table = &(dpm_context->dpm_tables.soc_table);
>   		aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> -
> -		for (i = 0; i < clocks.num_levels; i++) {
> -			clock_mhz = clocks.data[i].clocks_in_khz / 1000;
> -			freq_match = aldebaran_freqs_in_same_level(clock_mhz, cur_value);
> -			freq_match |= (clocks.num_levels == 1);
> -
> -			*offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
> -					i, clock_mhz,
> -					(freq_match) ? "*" : "");
> -		}
>   		break;
>   
>   	case SMU_FCLK:
> @@ -855,16 +824,6 @@ static int aldebaran_emit_clk_levels(struct smu_context *smu,
>   
>   		single_dpm_table = &(dpm_context->dpm_tables.fclk_table);
>   		aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> -
> -		for (i = 0; i < clocks.num_levels; i++) {
> -			clock_mhz = clocks.data[i].clocks_in_khz / 1000;
> -			freq_match = aldebaran_freqs_in_same_level(clock_mhz, cur_value);
> -			freq_match |= (clocks.num_levels == 1);
> -
> -			*offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
> -					i, clock_mhz,
> -					(freq_match) ? "*" : "");
> -		}
>   		break;
>   
>   	case SMU_VCLK:
> @@ -876,16 +835,6 @@ static int aldebaran_emit_clk_levels(struct smu_context *smu,
>   
>   		single_dpm_table = &(dpm_context->dpm_tables.vclk_table);
>   		aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> -
> -		for (i = 0; i < clocks.num_levels; i++) {
> -			clock_mhz = clocks.data[i].clocks_in_khz / 1000;
> -			freq_match = aldebaran_freqs_in_same_level(clock_mhz, cur_value);
> -			freq_match |= (clocks.num_levels == 1);
> -
> -			*offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
> -					i, clock_mhz,
> -					(freq_match) ? "*" : "");
> -		}
>   		break;
>   
>   	case SMU_DCLK:
> @@ -897,21 +846,44 @@ static int aldebaran_emit_clk_levels(struct smu_context *smu,
>   
>   		single_dpm_table = &(dpm_context->dpm_tables.dclk_table);
>   		aldebaran_get_clk_table(smu, &clocks, single_dpm_table);
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
>   
> +	switch (type) {
> +	case SMU_OD_SCLK:
> +	case SMU_SCLK:
> +		for (i = 0; i < display_levels; i++) {
> +			clock_mhz = freq_values[i];
> +			freq_match = aldebaran_freqs_in_same_level(clock_mhz, cur_value);
> +			freq_match |= (display_levels == 1);
> +
> +			*offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n", i,
> +				clock_mhz,
> +				(freq_match) ? "*" : "");
> +		}
> +		break;
> +

Similar nits as in arcturus patches. Keep this as part of original 
switch and below ones as some inline func or goto.

Thanks,
Lijo

> +	case SMU_OD_MCLK:
> +	case SMU_MCLK:
> +	case SMU_SOCCLK:
> +	case SMU_FCLK:
> +	case SMU_VCLK:
> +	case SMU_DCLK:
>   		for (i = 0; i < clocks.num_levels; i++) {
>   			clock_mhz = clocks.data[i].clocks_in_khz / 1000;
>   			freq_match = aldebaran_freqs_in_same_level(clock_mhz, cur_value);
>   			freq_match |= (clocks.num_levels == 1);
>   
>   			*offset += sysfs_emit_at(buf, *offset, "%d: %uMhz %s\n",
> -					i, clock_mhz,
> -					(freq_match) ? "*" : "");
> +				i, clock_mhz,
> +				(freq_match) ? "*" : "");
>   		}
>   		break;
> -
>   	default:
>   		return -EINVAL;
> -		break;
>   	}
>   
>   	return 0;