| Message ID | 20230522230759.153112-3-vinod.govindapillai@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mtl: add support for pmdemand | expand |
On Tue, 23 May 2023, Vinod Govindapillai <vinod.govindapillai@intel.com> 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. > > v2: use DIV_ROUND_* macro for the calculations (Ville) > > Bspec: 64636 > > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> > Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@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 ab405c48ca3a..c8075a37c3ab 100644 > --- a/drivers/gpu/drm/i915/display/intel_bw.c > +++ b/drivers/gpu/drm/i915/display/intel_bw.c > @@ -182,7 +182,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 = DIV_ROUND_CLOSEST(16667 * dclk + 500, 1000); What's with the +500 there? BR, Jani. > sp->t_rp = REG_FIELD_GET(MTL_TRP_MASK, val); > sp->t_rcd = REG_FIELD_GET(MTL_TRCD_MASK, val);
On Tue, 2023-05-23 at 12:01 +0300, Jani Nikula wrote: > On Tue, 23 May 2023, Vinod Govindapillai <vinod.govindapillai@intel.com> 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. > > > > v2: use DIV_ROUND_* macro for the calculations (Ville) > > > > Bspec: 64636 > > > > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> > > Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@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 ab405c48ca3a..c8075a37c3ab 100644 > > --- a/drivers/gpu/drm/i915/display/intel_bw.c > > +++ b/drivers/gpu/drm/i915/display/intel_bw.c > > @@ -182,7 +182,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 = DIV_ROUND_CLOSEST(16667 * dclk + 500, 1000); > > What's with the +500 there? This is what pcode expects. Somehow pcode use this formula and we have to exactly match this. Got this confirmed from Art. BR Vinod > > BR, > Jani. > > > > sp->t_rp = REG_FIELD_GET(MTL_TRP_MASK, val); > > sp->t_rcd = REG_FIELD_GET(MTL_TRCD_MASK, val); >
On Tue, 23 May 2023, "Govindapillai, Vinod" <vinod.govindapillai@intel.com> wrote: > On Tue, 2023-05-23 at 12:01 +0300, Jani Nikula wrote: >> On Tue, 23 May 2023, Vinod Govindapillai <vinod.govindapillai@intel.com> 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. >> > >> > v2: use DIV_ROUND_* macro for the calculations (Ville) >> > >> > Bspec: 64636 >> > >> > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> >> > Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@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 ab405c48ca3a..c8075a37c3ab 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_bw.c >> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c >> > @@ -182,7 +182,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 = DIV_ROUND_CLOSEST(16667 * dclk + 500, 1000); >> >> What's with the +500 there? > > This is what pcode expects. Somehow pcode use this formula and we have to exactly match this. Got > this confirmed from Art. I'm guessing all it really means is to round to closest, and they specified it like that instead of saying "round to closest". Essentially (x + (div/2)) / div is what DIV_ROUND_CLOSEST() does. It's odd to do it *twice*, and surely not what they expect. BR, Jani. > > BR > Vinod > >> >> BR, >> Jani. >> >> >> > sp->t_rp = REG_FIELD_GET(MTL_TRP_MASK, val); >> > sp->t_rcd = REG_FIELD_GET(MTL_TRCD_MASK, val); >> >
On Tue, 2023-05-23 at 16:14 +0300, Jani Nikula wrote: > On Tue, 23 May 2023, "Govindapillai, Vinod" <vinod.govindapillai@intel.com> wrote: > > On Tue, 2023-05-23 at 12:01 +0300, Jani Nikula wrote: > > > On Tue, 23 May 2023, Vinod Govindapillai <vinod.govindapillai@intel.com> 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. > > > > > > > > v2: use DIV_ROUND_* macro for the calculations (Ville) > > > > > > > > Bspec: 64636 > > > > > > > > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com> > > > > Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@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 ab405c48ca3a..c8075a37c3ab 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_bw.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_bw.c > > > > @@ -182,7 +182,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 = DIV_ROUND_CLOSEST(16667 * dclk + 500, 1000); > > > > > > What's with the +500 there? > > > > This is what pcode expects. Somehow pcode use this formula and we have to exactly match this. > > Got > > this confirmed from Art. > > I'm guessing all it really means is to round to closest, and they > specified it like that instead of saying "round to closest". > > Essentially (x + (div/2)) / div is what DIV_ROUND_CLOSEST() does. > > It's odd to do it *twice*, and surely not what they expect. > > BR, > Jani. Ah.. ok! Thanks! will update this! BR Vinod > > > > > BR > > Vinod > > > > > > > > BR, > > > Jani. > > > > > > > > > > sp->t_rp = REG_FIELD_GET(MTL_TRP_MASK, val); > > > > sp->t_rcd = REG_FIELD_GET(MTL_TRCD_MASK, val); > > > > > >
diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c index ab405c48ca3a..c8075a37c3ab 100644 --- a/drivers/gpu/drm/i915/display/intel_bw.c +++ b/drivers/gpu/drm/i915/display/intel_bw.c @@ -182,7 +182,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 = DIV_ROUND_CLOSEST(16667 * dclk + 500, 1000); sp->t_rp = REG_FIELD_GET(MTL_TRP_MASK, val); sp->t_rcd = REG_FIELD_GET(MTL_TRCD_MASK, val);