Re: [Intel-xe] [PATCH] drm/xe/mtl: Use 16.67 Mhz freq scale factor to get rpX

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Wed, 01 Nov 2023 08:17:56 -0700, Badal Nilawar wrote:
>

Hi Badal,

> For mtl and above 16.67 Mhz is the scale factor to calculate
> rpX frequencies.
>
> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_guc_pc.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
> index 74247e0d3674..90e3e1b56c27 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> @@ -306,14 +306,15 @@ static int pc_set_max_freq(struct xe_guc_pc *pc, u32 freq)
>  static void mtl_update_rpe_value(struct xe_guc_pc *pc)
>  {
>	struct xe_gt *gt = pc_to_gt(pc);
> -	u32 reg;
> +	u32 freq;
>
>	if (xe_gt_is_media_type(gt))
> -		reg = xe_mmio_read32(gt, MTL_MPE_FREQUENCY);
> +		freq = xe_mmio_read32(gt, MTL_MPE_FREQUENCY);
>	else
> -		reg = xe_mmio_read32(gt, MTL_GT_RPE_FREQUENCY);
> +		freq = xe_mmio_read32(gt, MTL_GT_RPE_FREQUENCY);

Generally I think we should avoid making these kind of variable name
changes, if only to keep the patch small and reduce the review
effort.

In this case, isn't 'reg' more appropriate anyway? If we retained reg this
would be a single line change in this function.

>
> -	pc->rpe_freq = REG_FIELD_GET(MTL_RPE_MASK, reg) * GT_FREQUENCY_MULTIPLIER;
> +	freq = REG_FIELD_GET(MTL_RPE_MASK, freq);
> +	pc->rpe_freq = decode_freq(freq);
>  }
>
>  static void tgl_update_rpe_value(struct xe_guc_pc *pc)
> @@ -645,18 +646,20 @@ static const struct attribute *pc_attrs[] = {
>  static void mtl_init_fused_rp_values(struct xe_guc_pc *pc)
>  {
>	struct xe_gt *gt = pc_to_gt(pc);
> -	u32 reg;
> +	u32 freq;
>
>	xe_device_assert_mem_access(pc_to_xe(pc));
>
>	if (xe_gt_is_media_type(gt))
> -		reg = xe_mmio_read32(gt, MTL_MEDIAP_STATE_CAP);
> +		freq = xe_mmio_read32(gt, MTL_MEDIAP_STATE_CAP);
>	else
> -		reg = xe_mmio_read32(gt, MTL_RP_STATE_CAP);
> -	pc->rp0_freq = REG_FIELD_GET(MTL_RP0_CAP_MASK, reg) *
> -		GT_FREQUENCY_MULTIPLIER;
> -	pc->rpn_freq = REG_FIELD_GET(MTL_RPN_CAP_MASK, reg) *
> -		GT_FREQUENCY_MULTIPLIER;
> +		freq = xe_mmio_read32(gt, MTL_RP_STATE_CAP);
> +
> +	freq = REG_FIELD_GET(MTL_RP0_CAP_MASK, freq);
> +	pc->rp0_freq = decode_freq(freq);
> +
> +	freq = REG_FIELD_GET(MTL_RPN_CAP_MASK, freq);
> +	pc->rpn_freq = decode_freq(freq);

Similar comments here too.

>  }
>
>  static void tgl_init_fused_rp_values(struct xe_guc_pc *pc)
> --
> 2.25.1
>

Thanks.
--
Ashutosh