On Mon, Dec 21, 2015 at 11:32:50AM -0800, Matt Roper wrote:
> Storing the max_level in dev_priv as VLV/CHV already do is a bit simpler
> than calling this standalone function, especially since some of the
> callsites need to special-case the call to check whether they're running
> on VLV/CHV.
>
> This function was further confusing since it wasn't actually specific to
> ILK-style platforms as its name implied (it was also used on Skylake and
> would even be called on pre-ILK platforms via debugfs). Aside from this
> function, our watermark programming across various platform-types
> doesn't really have any overlap.
>
> Note that we do change debugfs behavior slightly with this patch;
> i915_*_wm_latency files will all now appear empty on pre-ILK platforms
> rather than showing two watermark levels of 0.
>
> Signed-off-by: Matt Roper <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 18 ++-------------
> drivers/gpu/drm/i915/intel_pm.c | 44
> +++++++++++++++++--------------------
> 2 files changed, 22 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 0fc38bb..ae36656 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4445,14 +4445,7 @@ static void wm_latency_show(struct seq_file *m, const
> uint16_t wm[8])
> {
> struct drm_device *dev = m->private;
> int level;
> - int num_levels;
> -
> - if (IS_CHERRYVIEW(dev))
> - num_levels = 3;
> - else if (IS_VALLEYVIEW(dev))
> - num_levels = 1;
> - else
> - num_levels = ilk_wm_max_level(dev) + 1;
> + int num_levels = to_i915(dev)->wm.max_level + 1;
>
> drm_modeset_lock_all(dev);
>
> @@ -4560,18 +4553,11 @@ static ssize_t wm_latency_write(struct file *file,
> const char __user *ubuf,
> struct seq_file *m = file->private_data;
> struct drm_device *dev = m->private;
> uint16_t new[8] = { 0 };
> - int num_levels;
> + int num_levels = to_i915(dev)->wm.max_level + 1;
> int level;
> int ret;
> char tmp[32];
>
> - if (IS_CHERRYVIEW(dev))
> - num_levels = 3;
> - else if (IS_VALLEYVIEW(dev))
> - num_levels = 1;
> - else
> - num_levels = ilk_wm_max_level(dev) + 1;
> -
One idea I had at some point would be adjust dev_priv->wm.max_level based
on what the user writes to this file. Ie. it would allow the user to
enable/disable specific wm levels in a fairly nice way. In theory you
can do that already, but a bunch of stuff in the wm code might
scream at/not like eg. a latency value of 0. If we would adjust max_levels
appropritely we should be able to avoid those problems.
To do that we should probably keep the ilk_wm_max_level() (or something
like it) around and it would return the hardware limit, whereas dev_priv
contains the sw limit.
> if (len >= sizeof(tmp))
> return -EINVAL;
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b076580..71d814c 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2027,7 +2027,7 @@ static void intel_read_wm_latency(struct drm_device
> *dev, uint16_t wm[8])
> if (IS_GEN9(dev)) {
> uint32_t val;
> int ret, i;
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = dev_priv->wm.max_level;
>
> /* read the first set of memory latencies[0:3] */
> val = 0; /* data0 to be programmed to 0 for first set */
> @@ -2142,24 +2142,11 @@ static void intel_fixup_cur_wm_latency(struct
> drm_device *dev, uint16_t wm[5])
> wm[3] *= 2;
> }
>
> -int ilk_wm_max_level(const struct drm_device *dev)
> -{
> - /* how many WM levels are we expecting */
> - if (INTEL_INFO(dev)->gen >= 9)
> - return 7;
> - else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> - return 4;
> - else if (INTEL_INFO(dev)->gen >= 6)
> - return 3;
> - else
> - return 2;
> -}
> -
> static void intel_print_wm_latency(struct drm_device *dev,
> const char *name,
> const uint16_t wm[8])
> {
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = to_i915(dev)->wm.max_level;
>
> for (level = 0; level <= max_level; level++) {
> unsigned int latency = wm[level];
> @@ -2188,7 +2175,7 @@ static void intel_print_wm_latency(struct drm_device
> *dev,
> static bool ilk_increase_wm_latency(struct drm_i915_private *dev_priv,
> uint16_t wm[5], uint16_t min)
> {
> - int level, max_level = ilk_wm_max_level(dev_priv->dev);
> + int level, max_level = dev_priv->wm.max_level;
>
> if (wm[0] >= min)
> return false;
> @@ -2242,6 +2229,13 @@ static void ilk_setup_wm_latency(struct drm_device
> *dev)
>
> if (IS_GEN6(dev))
> snb_wm_latency_quirk(dev);
> +
> + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> + dev_priv->wm.max_level = 4;
> + else if (INTEL_INFO(dev)->gen >= 6)
> + dev_priv->wm.max_level = 3;
> + else
> + dev_priv->wm.max_level = 2;
Maybe we could just compute this based on how many non-zero wm latency
values we managed to read out?
> }
>
> static void skl_setup_wm_latency(struct drm_device *dev)
> @@ -2250,6 +2244,8 @@ static void skl_setup_wm_latency(struct drm_device *dev)
>
> intel_read_wm_latency(dev, dev_priv->wm.skl_latency);
> intel_print_wm_latency(dev, "Gen9 Plane", dev_priv->wm.skl_latency);
> +
> + dev_priv->wm.max_level = 7;
> }
>
> /* Compute new watermarks for the pipe */
> @@ -2265,7 +2261,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc
> *intel_crtc,
> struct intel_plane_state *pristate = NULL;
> struct intel_plane_state *sprstate = NULL;
> struct intel_plane_state *curstate = NULL;
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = dev_priv->wm.max_level;
> /* LP0 watermark maximums depend on this pipe alone */
> struct intel_wm_config config = {
> .num_pipes_active = 1,
> @@ -2389,7 +2385,7 @@ static void ilk_wm_merge(struct drm_device *dev,
> struct intel_pipe_wm *merged)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = dev_priv->wm.max_level;
> int last_enabled_level = max_level;
>
> /* ILK/SNB/IVB: LP1+ watermarks only w/ single pipe */
> @@ -2530,7 +2526,7 @@ static struct intel_pipe_wm
> *ilk_find_best_result(struct drm_device *dev,
> struct intel_pipe_wm *r1,
> struct intel_pipe_wm *r2)
> {
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = to_i915(dev)->wm.max_level;
> int level1 = 0, level2 = 0;
>
> for (level = 1; level <= max_level; level++) {
> @@ -3232,7 +3228,7 @@ static void skl_compute_pipe_wm(struct intel_crtc_state
> *cstate,
> {
> struct drm_device *dev = cstate->base.crtc->dev;
> const struct drm_i915_private *dev_priv = dev->dev_private;
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = dev_priv->wm.max_level;
>
> for (level = 0; level <= max_level; level++) {
> skl_compute_wm_level(dev_priv, ddb, cstate,
> @@ -3248,7 +3244,7 @@ static void skl_compute_wm_results(struct drm_device
> *dev,
> struct skl_wm_values *r,
> struct intel_crtc *intel_crtc)
> {
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = to_i915(dev)->wm.max_level;
> enum pipe pipe = intel_crtc->pipe;
> uint32_t temp;
> int i;
> @@ -3317,7 +3313,7 @@ static void skl_write_wm_values(struct drm_i915_private
> *dev_priv,
> struct intel_crtc *crtc;
>
> for_each_intel_crtc(dev, crtc) {
> - int i, level, max_level = ilk_wm_max_level(dev);
> + int i, level, max_level = dev_priv->wm.max_level;
> enum pipe pipe = crtc->pipe;
>
> if (!new->dirty[pipe])
> @@ -3711,7 +3707,7 @@ static void skl_pipe_wm_get_hw_state(struct drm_crtc
> *crtc)
> int level, i, max_level;
> uint32_t temp;
>
> - max_level = ilk_wm_max_level(dev);
> + max_level = dev_priv->wm.max_level;
>
> hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
>
> @@ -3801,7 +3797,7 @@ static void ilk_pipe_wm_get_hw_state(struct drm_crtc
> *crtc)
> active->wm[0].cur_val = tmp & WM0_PIPE_CURSOR_MASK;
> active->linetime = hw->wm_linetime[pipe];
> } else {
> - int level, max_level = ilk_wm_max_level(dev);
> + int level, max_level = dev_priv->wm.max_level;
>
> /*
> * For inactive pipes, all watermark levels
> --
> 2.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
