| Message ID | 1431386860-1038-1-git-send-email-abhay.kumar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 11, 2015 at 04:27:40PM -0700, abhay.kumar@intel.com wrote: > From: Abhay <abhay.kumar@intel.com> > > Current WM calculation is causing regression on SR residency. > Recomputing WM using new formula as provided by VPG Well, really it's the same formula that's been in use since forever for watermarks. I think we have at least a couple of copies if that formula in the wm code (should unify a bit at some point). However to get DDR DVFS enabled we'll need to go quite a bit beyond this. In the meantime I suppose we might try to just use the old formula with some kind of pessimistic latency value (not sure where the 23usec came from, last time I heard it was 33usec for DDR DVFS, and 11usec for PM5). But that would probably mean we'd already need to hardcode the drain latency to a small value. In fact the current drain latency values we're using still seem insufficient in some cases. > > Change-Id: I9dbd6a7b70c84454748dee41738130934230b763 > Signed-off-by: Abhay <abhay.kumar@intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++----------- > 1 file changed, 15 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index a960cdd..01a5d79 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -1546,6 +1546,13 @@ static int vlv_compute_wm(struct intel_crtc *crtc, > int fifo_size) > { > int clock, entries, pixel_size; > + uint32_t line_time = 0,buffer_wm = 0; > + > + /* using memory DVFS latency of 23usec */ > + int latency = 23000; > + > + int htotal = crtc->config.adjusted_mode.crtc_htotal; > + int hdisplay = to_intel_crtc(crtc)->config.pipe_src_w; > > /* > * FIXME the plane might have an fb > @@ -1557,18 +1564,15 @@ static int vlv_compute_wm(struct intel_crtc *crtc, > pixel_size = drm_format_plane_cpp(plane->base.fb->pixel_format, 0); > clock = crtc->config.adjusted_mode.crtc_clock; > > - entries = DIV_ROUND_UP(clock, 1000) * pixel_size; > + line_time = max(htotal * 1000 / clock, 1); > + > + /* Max FIFO size */ > + fifo_size = 96 * 1024; > + > + buffer_wm = (fifo_size - (((latency /line_time /1000) + 1 ) * > + hdisplay * pixel_size)); > + return buffer_wm; > > - /* > - * Set up the watermark such that we don't start issuing memory > - * requests until we are within PND's max deadline value (256us). > - * Idea being to be idle as long as possible while still taking > - * advatange of PND's deadline scheduling. The limit of 8 > - * cachelines (used when the FIFO will anyway drain in less time > - * than 256us) should match what we would be done if trickle > - * feed were enabled. > - */ > - return fifo_size - clamp(DIV_ROUND_UP(256 * entries, 64), 0, fifo_size - 8); > } > > static bool vlv_compute_sr_wm(struct drm_device *dev, > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
We received this 23usec number from the power team and same is being used by other BSW products. With this number we are getting 78% SRR From current 8%. Regards, Abhay Kumar -----Original Message----- From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] Sent: Tuesday, May 12, 2015 11:17 AM To: Kumar, Abhay Cc: Intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/chv: Recomputing CHV watermark. On Mon, May 11, 2015 at 04:27:40PM -0700, abhay.kumar@intel.com wrote: > From: Abhay <abhay.kumar@intel.com> > > Current WM calculation is causing regression on SR residency. > Recomputing WM using new formula as provided by VPG Well, really it's the same formula that's been in use since forever for watermarks. I think we have at least a couple of copies if that formula in the wm code (should unify a bit at some point). However to get DDR DVFS enabled we'll need to go quite a bit beyond this. In the meantime I suppose we might try to just use the old formula with some kind of pessimistic latency value (not sure where the 23usec came from, last time I heard it was 33usec for DDR DVFS, and 11usec for PM5). But that would probably mean we'd already need to hardcode the drain latency to a small value. In fact the current drain latency values we're using still seem insufficient in some cases. > > Change-Id: I9dbd6a7b70c84454748dee41738130934230b763 > Signed-off-by: Abhay <abhay.kumar@intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++----------- > 1 file changed, 15 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c index a960cdd..01a5d79 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -1546,6 +1546,13 @@ static int vlv_compute_wm(struct intel_crtc *crtc, > int fifo_size) > { > int clock, entries, pixel_size; > + uint32_t line_time = 0,buffer_wm = 0; > + > + /* using memory DVFS latency of 23usec */ > + int latency = 23000; > + > + int htotal = crtc->config.adjusted_mode.crtc_htotal; > + int hdisplay = to_intel_crtc(crtc)->config.pipe_src_w; > > /* > * FIXME the plane might have an fb > @@ -1557,18 +1564,15 @@ static int vlv_compute_wm(struct intel_crtc *crtc, > pixel_size = drm_format_plane_cpp(plane->base.fb->pixel_format, 0); > clock = crtc->config.adjusted_mode.crtc_clock; > > - entries = DIV_ROUND_UP(clock, 1000) * pixel_size; > + line_time = max(htotal * 1000 / clock, 1); > + > + /* Max FIFO size */ > + fifo_size = 96 * 1024; > + > + buffer_wm = (fifo_size - (((latency /line_time /1000) + 1 ) * > + hdisplay * pixel_size)); > + return buffer_wm; > > - /* > - * Set up the watermark such that we don't start issuing memory > - * requests until we are within PND's max deadline value (256us). > - * Idea being to be idle as long as possible while still taking > - * advatange of PND's deadline scheduling. The limit of 8 > - * cachelines (used when the FIFO will anyway drain in less time > - * than 256us) should match what we would be done if trickle > - * feed were enabled. > - */ > - return fifo_size - clamp(DIV_ROUND_UP(256 * entries, 64), 0, fifo_size - 8); > } > > static bool vlv_compute_sr_wm(struct drm_device *dev, > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index a960cdd..01a5d79 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -1546,6 +1546,13 @@ static int vlv_compute_wm(struct intel_crtc *crtc, int fifo_size) { int clock, entries, pixel_size; + uint32_t line_time = 0,buffer_wm = 0; + + /* using memory DVFS latency of 23usec */ + int latency = 23000; + + int htotal = crtc->config.adjusted_mode.crtc_htotal; + int hdisplay = to_intel_crtc(crtc)->config.pipe_src_w; /* * FIXME the plane might have an fb @@ -1557,18 +1564,15 @@ static int vlv_compute_wm(struct intel_crtc *crtc, pixel_size = drm_format_plane_cpp(plane->base.fb->pixel_format, 0); clock = crtc->config.adjusted_mode.crtc_clock; - entries = DIV_ROUND_UP(clock, 1000) * pixel_size; + line_time = max(htotal * 1000 / clock, 1); + + /* Max FIFO size */ + fifo_size = 96 * 1024; + + buffer_wm = (fifo_size - (((latency /line_time /1000) + 1 ) * + hdisplay * pixel_size)); + return buffer_wm; - /* - * Set up the watermark such that we don't start issuing memory - * requests until we are within PND's max deadline value (256us). - * Idea being to be idle as long as possible while still taking - * advatange of PND's deadline scheduling. The limit of 8 - * cachelines (used when the FIFO will anyway drain in less time - * than 256us) should match what we would be done if trickle - * feed were enabled. - */ - return fifo_size - clamp(DIV_ROUND_UP(256 * entries, 64), 0, fifo_size - 8); } static bool vlv_compute_sr_wm(struct drm_device *dev,