On Tue, 2023-04-18 at 12:25 +0300, Lisovskiy, Stanislav wrote: > 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 <[email protected]> > > --- > > 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
Yes. Not much explanation about the addition of 500. I just blindly followed what was in that Bspec. Yes ideally div round_up is (n + d -1) / d. So what is the point of having 500 if the purpose is a rounding up unless it is accounting for some "other" factor. Anyway it is nice to get the clarification. So the "other" factor I assume is that pcode is using this formula to calculate QGV point peak BW. So in MTL as we pass this peak BW to pcode for compare and select the QGV point, driver and pcode calculations need to match. BR Vinod > > > sp->t_rp = REG_FIELD_GET(MTL_TRP_MASK, val); > > sp->t_rcd = REG_FIELD_GET(MTL_TRCD_MASK, val); > > > > -- > > 2.34.1 > >
