| Message ID | 20220818234202.451742-14-radhakrishna.sripada@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Initial Meteorlake Support | expand |
On Thu, Aug 18, 2022 at 04:41:54PM -0700, Radhakrishna Sripada wrote: > Watermark latency is adjusted in cases when latency is 0us for level > greater than 1, the subsequent levels are disabled. Extract this logic > into its own function. > > Suggested-by: Matt Roper <matthew.d.roper@intel.com> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 88 ++++++++++++++++++--------------- > 1 file changed, 48 insertions(+), 40 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index ef7553b494ea..898e56d2eaf7 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2861,15 +2861,59 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, > result->enable = true; > } > > +static void > +adjust_wm_latency(u16 wm[], int max_level, int read_latency, > + bool wm_lv_0_adjust_needed) > +{ > + int i, level; > + > + /* > + * If a level n (n > 1) has a 0us latency, all levels m (m >= n) > + * need to be disabled. We make sure to sanitize the values out > + * of the punit to satisfy this requirement. > + */ > + for (level = 1; level <= max_level; level++) { > + if (wm[level] == 0) { > + for (i = level + 1; i <= max_level; i++) > + wm[i] = 0; > + > + max_level = level - 1; > + break; > + } > + } > + > + /* > + * WaWmMemoryReadLatency > + * > + * punit doesn't take into account the read latency so we need > + * to add proper adjustement to each valid level we retrieve > + * from the punit when level 0 response data is 0us. > + */ > + if (wm[0] == 0) { > + for (level = 0; level <= max_level; level++) > + wm[level] += read_latency; > + } > + > + /* > + * WA Level-0 adjustment for 16GB DIMMs: SKL+ > + * If we could not get dimm info enable this WA to prevent from > + * any underrun. If not able to get Dimm info assume 16GB dimm > + * to avoid any underrun. > + */ > + if (wm_lv_0_adjust_needed) > + wm[0] += 1; > +} > + > static void intel_read_wm_latency(struct drm_i915_private *dev_priv, > u16 wm[]) > { > struct intel_uncore *uncore = &dev_priv->uncore; > > if (DISPLAY_VER(dev_priv) >= 9) { > + int read_latency = DISPLAY_VER(dev_priv) >= 12 ? 3 : 2; > u32 val; > - int ret, i; > - int level, max_level = ilk_wm_max_level(dev_priv); > + int ret; > + int max_level = ilk_wm_max_level(dev_priv); > int mult = IS_DG2(dev_priv) ? 2 : 1; > > /* read the first set of memory latencies[0:3] */ > @@ -2909,44 +2953,8 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv, > wm[7] = ((val >> GEN9_MEM_LATENCY_LEVEL_3_7_SHIFT) & > GEN9_MEM_LATENCY_LEVEL_MASK) * mult; > > - /* > - * If a level n (n > 1) has a 0us latency, all levels m (m >= n) > - * need to be disabled. We make sure to sanitize the values out > - * of the punit to satisfy this requirement. > - */ > - for (level = 1; level <= max_level; level++) { > - if (wm[level] == 0) { > - for (i = level + 1; i <= max_level; i++) > - wm[i] = 0; > - > - max_level = level - 1; > - > - break; > - } > - } > - > - /* > - * WaWmMemoryReadLatency > - * > - * punit doesn't take into account the read latency so we need > - * to add proper adjustement to each valid level we retrieve > - * from the punit when level 0 response data is 0us. > - */ > - if (wm[0] == 0) { > - u8 adjust = DISPLAY_VER(dev_priv) >= 12 ? 3 : 2; > - > - for (level = 0; level <= max_level; level++) > - wm[level] += adjust; > - } > - > - /* > - * WA Level-0 adjustment for 16GB DIMMs: SKL+ > - * If we could not get dimm info enable this WA to prevent from > - * any underrun. If not able to get Dimm info assume 16GB dimm > - * to avoid any underrun. > - */ > - if (dev_priv->dram_info.wm_lv_0_adjust_needed) > - wm[0] += 1; > + adjust_wm_latency(wm, max_level, read_latency, > + dev_priv->dram_info.wm_lv_0_adjust_needed); Would it be cleaner to just pass the dev_priv as a parameter to the function and have it extract the structure field? As far as I can tell, we're never going to pass anything except exactly this field to this function. Aside from that, Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > u64 sskpd = intel_uncore_read64(uncore, MCH_SSKPD); > > -- > 2.25.1 >
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ef7553b494ea..898e56d2eaf7 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2861,15 +2861,59 @@ static void ilk_compute_wm_level(const struct drm_i915_private *dev_priv, result->enable = true; } +static void +adjust_wm_latency(u16 wm[], int max_level, int read_latency, + bool wm_lv_0_adjust_needed) +{ + int i, level; + + /* + * If a level n (n > 1) has a 0us latency, all levels m (m >= n) + * need to be disabled. We make sure to sanitize the values out + * of the punit to satisfy this requirement. + */ + for (level = 1; level <= max_level; level++) { + if (wm[level] == 0) { + for (i = level + 1; i <= max_level; i++) + wm[i] = 0; + + max_level = level - 1; + break; + } + } + + /* + * WaWmMemoryReadLatency + * + * punit doesn't take into account the read latency so we need + * to add proper adjustement to each valid level we retrieve + * from the punit when level 0 response data is 0us. + */ + if (wm[0] == 0) { + for (level = 0; level <= max_level; level++) + wm[level] += read_latency; + } + + /* + * WA Level-0 adjustment for 16GB DIMMs: SKL+ + * If we could not get dimm info enable this WA to prevent from + * any underrun. If not able to get Dimm info assume 16GB dimm + * to avoid any underrun. + */ + if (wm_lv_0_adjust_needed) + wm[0] += 1; +} + static void intel_read_wm_latency(struct drm_i915_private *dev_priv, u16 wm[]) { struct intel_uncore *uncore = &dev_priv->uncore; if (DISPLAY_VER(dev_priv) >= 9) { + int read_latency = DISPLAY_VER(dev_priv) >= 12 ? 3 : 2; u32 val; - int ret, i; - int level, max_level = ilk_wm_max_level(dev_priv); + int ret; + int max_level = ilk_wm_max_level(dev_priv); int mult = IS_DG2(dev_priv) ? 2 : 1; /* read the first set of memory latencies[0:3] */ @@ -2909,44 +2953,8 @@ static void intel_read_wm_latency(struct drm_i915_private *dev_priv, wm[7] = ((val >> GEN9_MEM_LATENCY_LEVEL_3_7_SHIFT) & GEN9_MEM_LATENCY_LEVEL_MASK) * mult; - /* - * If a level n (n > 1) has a 0us latency, all levels m (m >= n) - * need to be disabled. We make sure to sanitize the values out - * of the punit to satisfy this requirement. - */ - for (level = 1; level <= max_level; level++) { - if (wm[level] == 0) { - for (i = level + 1; i <= max_level; i++) - wm[i] = 0; - - max_level = level - 1; - - break; - } - } - - /* - * WaWmMemoryReadLatency - * - * punit doesn't take into account the read latency so we need - * to add proper adjustement to each valid level we retrieve - * from the punit when level 0 response data is 0us. - */ - if (wm[0] == 0) { - u8 adjust = DISPLAY_VER(dev_priv) >= 12 ? 3 : 2; - - for (level = 0; level <= max_level; level++) - wm[level] += adjust; - } - - /* - * WA Level-0 adjustment for 16GB DIMMs: SKL+ - * If we could not get dimm info enable this WA to prevent from - * any underrun. If not able to get Dimm info assume 16GB dimm - * to avoid any underrun. - */ - if (dev_priv->dram_info.wm_lv_0_adjust_needed) - wm[0] += 1; + adjust_wm_latency(wm, max_level, read_latency, + dev_priv->dram_info.wm_lv_0_adjust_needed); } else if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { u64 sskpd = intel_uncore_read64(uncore, MCH_SSKPD);
Watermark latency is adjusted in cases when latency is 0us for level greater than 1, the subsequent levels are disabled. Extract this logic into its own function. Suggested-by: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> --- drivers/gpu/drm/i915/intel_pm.c | 88 ++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 40 deletions(-)