Re: [Intel-gfx] [PATCH v1 2/4] drm/i915: update the QGV point frequency calculations

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Sun, Apr 16, 2023 at 06:54:15PM +0300, Vinod Govindapillai wrote:
> From MTL onwwards, pcode locks the QGV point based on peak BW of
> the intended QGV point passed by the driver. So the peak BW
> calculation must match the value expected by the pcode. Update
> the calculations as per the Bspec.
> 
> Bspec: 64636
> 
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 5fa599b04ca5..57f8204162dd 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -179,7 +179,7 @@ static int mtl_read_qgv_point_info(struct drm_i915_private *dev_priv,
>  	val2 = intel_uncore_read(&dev_priv->uncore,
>  				 MTL_MEM_SS_INFO_QGV_POINT_HIGH(point));
>  	dclk = REG_FIELD_GET(MTL_DCLK_MASK, val);
> -	sp->dclk = DIV_ROUND_UP((16667 * dclk), 1000);
> +	sp->dclk = (16667 * dclk + 500) / 1000;

Hmm, wonder does it at least partly now intersects with what I'm doing in 
https://patchwork.freedesktop.org/series/114982/

I remember we were discussing if this "+500" is actually also rounding up
itself.

The thing is that the way how rounding up is done for instance in DIV_ROUND_UP
also, if you check, if you lets say want to divide n by d, however you want to round
up the result, you add n = n + (d - 1) and then divide by d. This is how DIV_ROUND_UP works.
That effectively means that if n would be anything more than m*d, result would be not m,
but m + 1(note flooring would give m)

Adding 500, when dividing by 1000 is also rouding up, however it is a bit weaker.
In example above that would mean, if we want to divide n by d, we first add n = n + d / 2
and then divide by d.
That effectively means that if n would be anything more than m*d + 500, result would not m,
but again m + 1(again note, that true flooeing would have given m, not m + 1)

So it is still rounding up, but just being weaker/less precise though.

If we would want to truly floor that division, we would want to get m, but not m + 1 from
above examples, which means that we should just divide n / d, without adding anything.
So in my opinion, if we want to floor (16667 * dclk / 1000) result - it should not have
both "DIV_ROUND_UP" and " + 500" things - thats what I've done in series which also was touching
this code as well.

I think it would be nice to raise issue and clarify from HW team, if it was initial intention,
because adding + 500 is clearly doing rounding up as well, but it is just now on +-500(d/2) granularity now,
while DIV_ROUND_UP worked with +-1 granularity. However both things are essentially "rounding up".
So in that case I would really want to challenge or clarify, what is written in BSpec.

Stan

>  	sp->t_rp = REG_FIELD_GET(MTL_TRP_MASK, val);
>  	sp->t_rcd = REG_FIELD_GET(MTL_TRCD_MASK, val);
>  
> -- 
> 2.34.1
>