On Thu, May 09, 2013 at 04:55:50PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zan...@intel.com>
> 
> The spec says the linetime watermarks must be programmed before
> enabling any display low power watermarks, but we're currently
> updating the linetime watermarks after we call intel_update_watermarks
> (and only at crtc_mode_set, not at crtc_{enable,disable}). So IMHO the
> best way guarantee the linetime watermarks will be updated before the
> low power watermarks is inside the update_wm function, because it's
> the function that enables low power watermarks. And since Haswell is
> the only platform that has linetime watermarks, let's completely kill
> the "intel_update_linetime_watermarks" abstraction and just use the
> intel_update_watermarks abstraction by creating haswell_update_wm.
> 
> For now haswell_update_wm is still calling sandybridge_update_wm, but
> in the future I plan to implement a function specific to Haswell.
> 
> v2: - Rename patch
>     - Disable LP watermarks before changing linetime WMs (Chris)
>     - Add a comment explaining that this is just temporary code.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zan...@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    2 --
>  drivers/gpu/drm/i915/intel_display.c |    2 --
>  drivers/gpu/drm/i915/intel_drv.h     |    2 --
>  drivers/gpu/drm/i915/intel_pm.c      |   43 
> ++++++++++++++++++++++++----------
>  4 files changed, 30 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c81100c..8606103 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -316,8 +316,6 @@ struct drm_i915_display_funcs {
>       void (*update_wm)(struct drm_device *dev);
>       void (*update_sprite_wm)(struct drm_device *dev, int pipe,
>                                uint32_t sprite_width, int pixel_size);
> -     void (*update_linetime_wm)(struct drm_device *dev, int pipe,
> -                              struct drm_display_mode *mode);
>       void (*modeset_global_resources)(struct drm_device *dev);
>       /* Returns the active state of the crtc, and if the crtc is active,
>        * fills out the pipe-config with the hw state. */
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index b796752..7bcd60c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6026,8 +6026,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>  
>       intel_update_watermarks(dev);
>  
> -     intel_update_linetime_watermarks(dev, pipe, adjusted_mode);
> -
>       return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index be9ad39..80b417a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -727,8 +727,6 @@ extern void intel_update_watermarks(struct drm_device 
> *dev);
>  extern void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
>                                          uint32_t sprite_width,
>                                          int pixel_size);
> -extern void intel_update_linetime_watermarks(struct drm_device *dev, int 
> pipe,
> -                      struct drm_display_mode *mode);
>  
>  extern unsigned long intel_gen4_compute_page_offset(int *x, int *y,
>                                                   unsigned int tiling_mode,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 93b8f70..236cd99 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2069,12 +2069,19 @@ static void ivybridge_update_wm(struct drm_device 
> *dev)
>  }
>  
>  static void
> -haswell_update_linetime_wm(struct drm_device *dev, int pipe,
> -                              struct drm_display_mode *mode)
> +haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
>  {
>       struct drm_i915_private *dev_priv = dev->dev_private;
> +     struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +     enum pipe pipe = intel_crtc->pipe;
> +     struct drm_display_mode *mode = &intel_crtc->config.adjusted_mode;
>       u32 temp;
>  
> +     if (!intel_crtc_active(crtc)) {
> +             I915_WRITE(PIPE_WM_LINETIME(pipe), 0);
> +             return;
> +     }
> +
>       temp = I915_READ(PIPE_WM_LINETIME(pipe));
>       temp &= ~PIPE_WM_LINETIME_MASK;
>  
> @@ -2095,6 +2102,26 @@ haswell_update_linetime_wm(struct drm_device *dev, int 
> pipe,
>       I915_WRITE(PIPE_WM_LINETIME(pipe), temp);
>  }
>  
> +static void haswell_update_wm(struct drm_device *dev)
> +{
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +     struct drm_crtc *crtc;
> +     enum pipe pipe;
> +
> +     /* Disable the LP WMs before changine the linetime registers. This is
> +      * just a temporary code that will be replaced soon. */
> +     I915_WRITE(WM3_LP_ILK, 0);
> +     I915_WRITE(WM2_LP_ILK, 0);
> +     I915_WRITE(WM1_LP_ILK, 0);
> +
> +     for_each_pipe(pipe) {
> +             crtc = dev_priv->pipe_to_crtc_mapping[pipe];

list_for_each_entry(crtc, ...)

Otherwise:
Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

> +             haswell_update_linetime_wm(dev, crtc);
> +     }
> +
> +     sandybridge_update_wm(dev);
> +}
> +
>  static bool
>  sandybridge_compute_sprite_wm(struct drm_device *dev, int plane,
>                             uint32_t sprite_width, int pixel_size,
> @@ -2290,15 +2317,6 @@ void intel_update_watermarks(struct drm_device *dev)
>               dev_priv->display.update_wm(dev);
>  }
>  
> -void intel_update_linetime_watermarks(struct drm_device *dev,
> -             int pipe, struct drm_display_mode *mode)
> -{
> -     struct drm_i915_private *dev_priv = dev->dev_private;
> -
> -     if (dev_priv->display.update_linetime_wm)
> -             dev_priv->display.update_linetime_wm(dev, pipe, mode);
> -}
> -
>  void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
>                                   uint32_t sprite_width, int pixel_size)
>  {
> @@ -4553,9 +4571,8 @@ void intel_init_pm(struct drm_device *dev)
>                       dev_priv->display.init_clock_gating = 
> ivybridge_init_clock_gating;
>               } else if (IS_HASWELL(dev)) {
>                       if (SNB_READ_WM0_LATENCY()) {
> -                             dev_priv->display.update_wm = 
> sandybridge_update_wm;
> +                             dev_priv->display.update_wm = haswell_update_wm;
>                               dev_priv->display.update_sprite_wm = 
> sandybridge_update_sprite_wm;
> -                             dev_priv->display.update_linetime_wm = 
> haswell_update_linetime_wm;
>                       } else {
>                               DRM_DEBUG_KMS("Failed to read display plane 
> latency. "
>                                             "Disable CxSR\n");
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to