[Intel-gfx] [PATCH 3/3] drm/i915: Simplify watermark/init_clock_gating setup

[Paulo Zanoni]

2014/1/7  <ville.syrjala at linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Avoid duplicating the same piece of code several times by separating
> the watemark vfunc setup from the init_clock_gating vfunc setup on PCH
> platforms.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 78 +++++++++--------------------------------
>  1 file changed, 16 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 04e1e29..a177a93 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5575,73 +5575,27 @@ void intel_init_pm(struct drm_device *dev)
>         if (HAS_PCH_SPLIT(dev)) {
>                 intel_setup_wm_latency(dev);
>
> -               if (IS_GEN5(dev)) {
> -                       if (dev_priv->wm.pri_latency[1] &&
> -                           dev_priv->wm.spr_latency[1] &&
> -                           dev_priv->wm.cur_latency[1]) {
> -                               dev_priv->display.update_wm = ilk_update_wm;
> -                               dev_priv->display.update_sprite_wm =
> -                                       ilk_update_sprite_wm;
> -                       } else {
> -                               DRM_DEBUG_KMS("Failed to get proper latency. "
> -                                             "Disable CxSR\n");
> -                               dev_priv->display.update_wm = NULL;
> -                       }
> +               if ((IS_GEN5(dev) && dev_priv->wm.pri_latency[1] &&
> +                    dev_priv->wm.spr_latency[1] && dev_priv->wm.cur_latency[1]) ||
> +                   (!IS_GEN5(dev) && dev_priv->wm.pri_latency[0] &&
> +                    dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) {
> +                       dev_priv->display.update_wm = ilk_update_wm;
> +                       dev_priv->display.update_sprite_wm = ilk_update_sprite_wm;

My tiny little brain doesn't remember why on gen5 we check for
latency[1] instead of latency[0]. I know this is not the goal if your
patch, but maybe a follow-up patch adding a little comment would be
nice :)

For the 3 patches on the series:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>


> +               } else {
> +                       DRM_DEBUG_KMS("Failed to read display plane latency. "
> +                                     "Disable CxSR\n");
> +               }
> +
> +               if (IS_GEN5(dev))
>                         dev_priv->display.init_clock_gating = ironlake_init_clock_gating;
> -               } else if (IS_GEN6(dev)) {
> -                       if (dev_priv->wm.pri_latency[0] &&
> -                           dev_priv->wm.spr_latency[0] &&
> -                           dev_priv->wm.cur_latency[0]) {
> -                               dev_priv->display.update_wm = ilk_update_wm;
> -                               dev_priv->display.update_sprite_wm =
> -                                       ilk_update_sprite_wm;
> -                       } else {
> -                               DRM_DEBUG_KMS("Failed to read display plane latency. "
> -                                             "Disable CxSR\n");
> -                               dev_priv->display.update_wm = NULL;
> -                       }
> +               else if (IS_GEN6(dev))
>                         dev_priv->display.init_clock_gating = gen6_init_clock_gating;
> -               } else if (IS_IVYBRIDGE(dev)) {
> -                       if (dev_priv->wm.pri_latency[0] &&
> -                           dev_priv->wm.spr_latency[0] &&
> -                           dev_priv->wm.cur_latency[0]) {
> -                               dev_priv->display.update_wm = ilk_update_wm;
> -                               dev_priv->display.update_sprite_wm =
> -                                       ilk_update_sprite_wm;
> -                       } else {
> -                               DRM_DEBUG_KMS("Failed to read display plane latency. "
> -                                             "Disable CxSR\n");
> -                               dev_priv->display.update_wm = NULL;
> -                       }
> +               else if (IS_IVYBRIDGE(dev))
>                         dev_priv->display.init_clock_gating = ivybridge_init_clock_gating;
> -               } else if (IS_HASWELL(dev)) {
> -                       if (dev_priv->wm.pri_latency[0] &&
> -                           dev_priv->wm.spr_latency[0] &&
> -                           dev_priv->wm.cur_latency[0]) {
> -                               dev_priv->display.update_wm = ilk_update_wm;
> -                               dev_priv->display.update_sprite_wm =
> -                                       ilk_update_sprite_wm;
> -                       } else {
> -                               DRM_DEBUG_KMS("Failed to read display plane latency. "
> -                                             "Disable CxSR\n");
> -                               dev_priv->display.update_wm = NULL;
> -                       }
> +               else if (IS_HASWELL(dev))
>                         dev_priv->display.init_clock_gating = haswell_init_clock_gating;
> -               } else if (INTEL_INFO(dev)->gen == 8) {
> -                       if (dev_priv->wm.pri_latency[0] &&
> -                           dev_priv->wm.spr_latency[0] &&
> -                           dev_priv->wm.cur_latency[0]) {
> -                               dev_priv->display.update_wm = ilk_update_wm;
> -                               dev_priv->display.update_sprite_wm =
> -                                       ilk_update_sprite_wm;
> -                       } else {
> -                               DRM_DEBUG_KMS("Failed to read display plane latency. "
> -                                             "Disable CxSR\n");
> -                               dev_priv->display.update_wm = NULL;
> -                       }
> +               else if (INTEL_INFO(dev)->gen == 8)
>                         dev_priv->display.init_clock_gating = gen8_init_clock_gating;
> -               } else
> -                       dev_priv->display.update_wm = NULL;
>         } else if (IS_VALLEYVIEW(dev)) {
>                 dev_priv->display.update_wm = valleyview_update_wm;
>                 dev_priv->display.init_clock_gating =
> --
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni