Re: [PATCH 01/25] drm/i915: Move verify_wm_state() to heap

Ville Syrjälä
2019-02-19T12:56:56Z

On Tue, Feb 19, 2019 at 12:21:51PM +0000, Chris Wilson wrote:
> The stack usage exceeded 1024 bytes prompting warnings on conservative
> setups, so move the temporary allocation for HW readback onto the heap.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---
> drivers/gpu/drm/i915/intel_display.c | 48 ++++++++++++++++++----------
> 1 file changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3b4a6eeb4573..415d8968f2c5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12227,12 +12227,15 @@ static void verify_wm_state(struct drm_crtc *crtc,
> struct drm_crtc_state *new_state)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> - struct skl_ddb_allocation hw_ddb, *sw_ddb;
> - struct skl_pipe_wm hw_wm, *sw_wm;
> - struct skl_plane_wm *hw_plane_wm, *sw_plane_wm;
> + struct skl_hw_state {
> + struct skl_ddb_entry ddb_y[I915_MAX_PLANES];
> + struct skl_ddb_entry ddb_uv[I915_MAX_PLANES];
> + struct skl_ddb_allocation ddb;
> + struct skl_pipe_wm wm;
> + } *hw;
> + struct skl_ddb_allocation *sw_ddb;
> + struct skl_pipe_wm *sw_wm;
> struct skl_ddb_entry *hw_ddb_entry, *sw_ddb_entry;
> - struct skl_ddb_entry hw_ddb_y[I915_MAX_PLANES];
> - struct skl_ddb_entry hw_ddb_uv[I915_MAX_PLANES];
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> const enum pipe pipe = intel_crtc->pipe;
> int plane, level, max_level = ilk_wm_max_level(dev_priv);
> @@ -12240,22 +12243,29 @@ static void verify_wm_state(struct drm_crtc *crtc,
> if (INTEL_GEN(dev_priv) < 9 || !new_state->active)
> return;
>
> - skl_pipe_wm_get_hw_state(intel_crtc, &hw_wm);
> + hw = kzalloc(sizeof(*hw), GFP_KERNEL);
> + if (!hw)
> + return;
> +
> + skl_pipe_wm_get_hw_state(intel_crtc, &hw->wm);
> sw_wm = &to_intel_crtc_state(new_state)->wm.skl.optimal;
>
> - skl_pipe_ddb_get_hw_state(intel_crtc, hw_ddb_y, hw_ddb_uv);
> + skl_pipe_ddb_get_hw_state(intel_crtc, hw->ddb_y, hw->ddb_uv);
>
> - skl_ddb_get_hw_state(dev_priv, &hw_ddb);
> + skl_ddb_get_hw_state(dev_priv, &hw->ddb);
> sw_ddb = &dev_priv->wm.skl_hw.ddb;
>
> - if (INTEL_GEN(dev_priv) >= 11)
> - if (hw_ddb.enabled_slices != sw_ddb->enabled_slices)
> - DRM_ERROR("mismatch in DBUF Slices (expected %u, got %u)\n",
> - sw_ddb->enabled_slices,
> - hw_ddb.enabled_slices);
> + if (INTEL_GEN(dev_priv) >= 11 &&
> + hw->ddb.enabled_slices != sw_ddb->enabled_slices)
> + DRM_ERROR("mismatch in DBUF Slices (expected %u, got %u)\n",
> + sw_ddb->enabled_slices,
> + hw->ddb.enabled_slices);
> +
> /* planes */
> for_each_universal_plane(dev_priv, pipe, plane) {
> - hw_plane_wm = &hw_wm.planes[plane];
> + struct skl_plane_wm *hw_plane_wm, *sw_plane_wm;
> +
> + hw_plane_wm = &hw->wm.planes[plane];
> sw_plane_wm = &sw_wm->planes[plane];
>
> /* Watermarks */
> @@ -12287,7 +12297,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
> }
>
> /* DDB */
> - hw_ddb_entry = &hw_ddb_y[plane];
> + hw_ddb_entry = &hw->ddb_y[plane];
> sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[plane];
>
> if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
> @@ -12305,7 +12315,9 @@ static void verify_wm_state(struct drm_crtc *crtc,
> * once the plane becomes visible, we can skip this check
> */
> if (1) {
> - hw_plane_wm = &hw_wm.planes[PLANE_CURSOR];
> + struct skl_plane_wm *hw_plane_wm, *sw_plane_wm;
> +
> + hw_plane_wm = &hw->wm.planes[PLANE_CURSOR];
> sw_plane_wm = &sw_wm->planes[PLANE_CURSOR];
>
> /* Watermarks */
> @@ -12337,7 +12349,7 @@ static void verify_wm_state(struct drm_crtc *crtc,
> }
>
> /* DDB */
> - hw_ddb_entry = &hw_ddb_y[PLANE_CURSOR];
> + hw_ddb_entry = &hw->ddb_y[PLANE_CURSOR];
> sw_ddb_entry = &to_intel_crtc_state(new_state)->wm.skl.plane_ddb_y[PLANE_CURSOR];
>
> if (!skl_ddb_entry_equal(hw_ddb_entry, sw_ddb_entry)) {
> @@ -12347,6 +12359,8 @@ static void verify_wm_state(struct drm_crtc *crtc,
> hw_ddb_entry->start, hw_ddb_entry->end);
> }
> }
> +
> + kfree(hw);
> }
>
> static void
> --
> 2.20.1

--
Ville Syrjälä
Intel