On Thu, Sep 04, 2014 at 12:27:16PM +0100, Damien Lespiau wrote:
> From: Pradeep Bhat <pradeep.bhat@xxxxxxxxx>
>
> 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.bhat@xxxxxxxxx>
> Signed-off-by: Damien Lespiau <damien.lespiau@xxxxxxxxx>
> ---
> 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.syrjala@xxxxxxxxxxxxxxx>
> +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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
