On Thu, Sep 04, 2014 at 12:27:16PM +0100, Damien Lespiau wrote:
> From: Pradeep Bhat <pradeep.b...@intel.com>
> 
> This patch provides the implementation for reading the pipe wm HW
> state.
> 
> v2: Incorporated Damien's review comments and also made modifications
>     to incorporate the plane/cursor split.
> 
> v3: No need to ident a line that was fitting 80 chars
>     Return early instead of indenting the remaining of a function
>     (Damien)
> 
> v4: Rebase on top of nightly (minor conflect in intel_drv.h)
> 
> Signed-off-by: Pradeep Bhat <pradeep.b...@intel.com>
> Signed-off-by: Damien Lespiau <damien.lesp...@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   4 +-
>  drivers/gpu/drm/i915/intel_drv.h     |   1 +
>  drivers/gpu/drm/i915/intel_pm.c      | 104 
> +++++++++++++++++++++++++++++++++++
>  3 files changed, 108 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 3d9f447..69e023a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13375,7 +13375,9 @@ void intel_modeset_setup_hw_state(struct drm_device 
> *dev,
>               pll->on = false;
>       }
>  
> -     if (HAS_PCH_SPLIT(dev))
> +     if (IS_GEN9(dev))
> +             skl_wm_get_hw_state(dev);
> +     else if (HAS_PCH_SPLIT(dev))
>               ilk_wm_get_hw_state(dev);
>  
>       if (force_restore) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 268087f..559b747 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1102,6 +1102,7 @@ void intel_runtime_pm_put(struct drm_i915_private 
> *dev_priv);
>  void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
>  void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
>  void ilk_wm_get_hw_state(struct drm_device *dev);
> +void skl_wm_get_hw_state(struct drm_device *dev);
>  
>  
>  /* intel_sdvo.c */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 756ff16..1e56067 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3591,6 +3591,110 @@ ilk_update_sprite_wm(struct drm_plane *plane,
>       ilk_update_wm(crtc);
>  }
>  
> +static void skl_pipe_wm_active_state(uint32_t val,
> +                                  struct skl_pipe_wm *active,
> +                                  bool is_transwm,
> +                                  bool is_cursor,
> +                                  int i,
> +                                  int level)
> +{
> +     bool is_enabled = (val & PLANE_WM_EN) != 0;
> +
> +     if (!is_transwm) {
> +             if (!is_cursor) {
> +                     active->wm[level].plane_en[i] = is_enabled;
> +                     active->wm[level].plane_res_b[i] =
> +                                     val & PLANE_WM_BLOCKS_MASK;
> +                     active->wm[level].plane_res_l[i] =
> +                                     (val >> PLANE_WM_LINES_SHIFT) &
> +                                             PLANE_WM_LINES_MASK;
> +             } else {
> +                     active->wm[level].cursor_en = is_enabled;
> +                     active->wm[level].cursor_res_b =
> +                                     val & PLANE_WM_BLOCKS_MASK;
> +                     active->wm[level].cursor_res_l =
> +                                     (val >> PLANE_WM_LINES_SHIFT) &
> +                                             PLANE_WM_LINES_MASK;
> +             }
> +     } else {
> +             if (!is_cursor) {
> +                     active->trans_wm.plane_en[i] = is_enabled;
> +                     active->trans_wm.plane_res_b[i] =
> +                                     val & PLANE_WM_BLOCKS_MASK;
> +                     active->trans_wm.plane_res_l[i] =
> +                                     (val >> PLANE_WM_LINES_SHIFT) &
> +                                             PLANE_WM_LINES_MASK;
> +             } else {
> +                     active->trans_wm.cursor_en = is_enabled;
> +                     active->trans_wm.cursor_res_b =
> +                                     val & PLANE_WM_BLOCKS_MASK;
> +                     active->trans_wm.cursor_res_l =
> +                                     (val >> PLANE_WM_LINES_SHIFT) &
> +                                             PLANE_WM_LINES_MASK;
> +             }
> +     }
> +}

Am I imagining it or could this function be reduced to four lines if you
would just pass the target struct as a parameter instead of the
(is_transwm,is_cursor,i,level) tuple? Ah no, crap, SoA strikes back. So I
think I mentioned it already during my first round of reviews that I'd
like make a bunch of this stuff AoS instead. But that's a recipe for
massive conflicts so let's get the current stuff in first before we go
tearing into those structures.

Apart from that horror of a function this looks like it does what it's
meant to so:
Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

> +static void skl_pipe_wm_get_hw_state(struct drm_crtc *crtc)
> +{
> +     struct drm_device *dev = crtc->dev;
> +     struct drm_i915_private *dev_priv = dev->dev_private;
> +     struct skl_wm_values *hw = &dev_priv->wm.skl_hw;
> +     struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +     struct skl_pipe_wm *active = &intel_crtc->wm.skl_active;
> +     enum pipe pipe = intel_crtc->pipe;
> +     int level, i, max_level;
> +     uint32_t temp;
> +
> +     max_level = ilk_wm_max_level(dev);
> +
> +     hw->wm_linetime[pipe] = I915_READ(PIPE_WM_LINETIME(pipe));
> +
> +     for (level = 0; level <= max_level; level++) {
> +             for (i = 0; i < intel_num_planes(intel_crtc); i++)
> +                     hw->plane[pipe][i][level] =
> +                                     I915_READ(PLANE_WM(pipe, i, level));
> +             hw->cursor[pipe][level] = I915_READ(CUR_WM(pipe, level));
> +     }
> +
> +     for (i = 0; i < intel_num_planes(intel_crtc); i++)
> +             hw->plane_trans[pipe][i] = I915_READ(PLANE_WM_TRANS(pipe, i));
> +     hw->cursor_trans[pipe] = I915_READ(CUR_WM_TRANS(pipe));
> +
> +     if (!intel_crtc_active(crtc))
> +             return;
> +
> +     hw->dirty[pipe] = true;
> +
> +     active->linetime = hw->wm_linetime[pipe];
> +
> +     for (level = 0; level <= max_level; level++) {
> +             for (i = 0; i < intel_num_planes(intel_crtc); i++) {
> +                     temp = hw->plane[pipe][i][level];
> +                     skl_pipe_wm_active_state(temp, active, false,
> +                                             false, i, level);
> +             }
> +             temp = hw->cursor[pipe][level];
> +             skl_pipe_wm_active_state(temp, active, false, true, i, level);
> +     }
> +
> +     for (i = 0; i < intel_num_planes(intel_crtc); i++) {
> +             temp = hw->plane_trans[pipe][i];
> +             skl_pipe_wm_active_state(temp, active, true, false, i, 0);
> +     }
> +
> +     temp = hw->cursor_trans[pipe];
> +     skl_pipe_wm_active_state(temp, active, true, true, i, 0);
> +}
> +
> +void skl_wm_get_hw_state(struct drm_device *dev)
> +{
> +     struct drm_crtc *crtc;
> +
> +     list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> +             skl_pipe_wm_get_hw_state(crtc);
> +}
> +
>  static void ilk_pipe_wm_get_hw_state(struct drm_crtc *crtc)
>  {
>       struct drm_device *dev = crtc->dev;
> -- 
> 1.8.3.1
> 
> _______________________________________________
> 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