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@xxxxxxxxx> > --- > 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 >
