Re: [PATCH 12/24] drm/i915: Refactor ilk_validate_pipe_wm()

Paulo Zanoni
2014-04-23T21:23:33Z

2014-03-07 13:32 GMT-03:00 <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Pull the LP0 validate out from intel_compute_pipe_wm() into a separate
> function. We will have further need for such a function to validate
> both the final watermarks and the intermediate watermarks we will be
> using while the plane(s) transition between different configurations.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 30 +++++++++++++++++++-----------
> 1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e519578a1..e142095 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2172,6 +2172,24 @@ static void ilk_compute_wm_config(struct drm_device *dev,
> }
> }
>
> +static bool ilk_validate_pipe_wm(struct drm_device *dev,
> + struct intel_pipe_wm *pipe_wm)
> +{
> + /* LP0 watermark maximums depend on this pipe alone */
> + const struct intel_wm_config config = {
> + .num_pipes_active = 1,
> + .sprites_enabled = pipe_wm->sprites_enabled,
> + .sprites_scaled = pipe_wm->sprites_scaled,
> + };
> + struct ilk_wm_maximums max;
> +
> + /* LP0 watermarks always use 1/2 DDB partitioning */
> + ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max);
> +
> + /* At least LP0 must be valid */

I would remove this comment, and keep the original one on the caller.
IMHO it makes more sense there.

With or without that change:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> + return ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]);
> +}
> +
> /* Compute new watermarks for the pipe */
> static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
> const struct ilk_pipe_wm_parameters *params,
> @@ -2180,12 +2198,6 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
> struct drm_device *dev = crtc->dev;
> const struct drm_i915_private *dev_priv = dev->dev_private;
> int level, max_level = ilk_wm_max_level(dev);
> - /* LP0 watermark maximums depend on this pipe alone */
> - struct intel_wm_config config = {
> - .num_pipes_active = 1,
> - .sprites_enabled = params->spr.enabled,
> - .sprites_scaled = params->spr.scaled,
> - };
> struct ilk_wm_maximums max;
>
> pipe_wm->pipe_enabled = params->active;
> @@ -2205,11 +2217,7 @@ static bool intel_compute_pipe_wm(struct drm_crtc *crtc,
> if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> pipe_wm->linetime = hsw_compute_linetime_wm(dev, crtc);
>
> - /* LP0 watermarks always use 1/2 DDB partitioning */
> - ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max);
> -
> - /* At least LP0 must be valid */
> - if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]))
> + if (!ilk_validate_pipe_wm(dev, pipe_wm))
> return false;
>
> ilk_compute_wm_reg_maximums(dev, 1, &max);
> --
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Paulo Zanoni