[Intel-gfx] [PATCH v2 08/16] drm/i915: Split watermark programming into pre and post steps
Paulo Zanoni
przanoni at gmail.com
Tue Jun 3 22:51:01 CEST 2014
2014-05-22 11:48 GMT-03:00 <ville.syrjala at linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> We need to perform watermark programming before and after changing the
> plane configuration. Add two new vfuncs to do that. The pre phase is
> supposed to switch over to the intermediate watermarks which are
> computed so that they can deal with both the old and new plane
> configurations. The post phase will arm the vblank based update
> systems to switch over to the optimal target watermarks after the
> plane configuration has for sure changed.
>
> v2: Pass around intel_crtc and s/intel_crtc/crtc/
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 5 +++
> drivers/gpu/drm/i915/intel_drv.h | 11 +++++
> drivers/gpu/drm/i915/intel_pm.c | 88 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 104 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c90d5ac..d4f8ae8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -407,6 +407,7 @@ struct intel_plane_config;
> struct intel_crtc;
> struct intel_limit;
> struct dpll;
> +struct intel_crtc_wm_config;
>
> struct drm_i915_display_funcs {
> bool (*fbc_enabled)(struct drm_device *dev);
> @@ -437,6 +438,10 @@ struct drm_i915_display_funcs {
> struct drm_crtc *crtc,
> uint32_t sprite_width, int pixel_size,
> bool enable, bool scaled);
> + void (*program_wm_pre)(struct intel_crtc *crtc,
> + const struct intel_crtc_wm_config *config);
> + void (*program_wm_post)(struct intel_crtc *crtc,
> + const struct intel_crtc_wm_config *config);
> 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_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 72f01b1..4b59be3 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -358,6 +358,13 @@ struct intel_pipe_wm {
> bool sprites_scaled;
> };
>
> +struct intel_crtc_wm_config {
> + /* target watermarks for the pipe */
> + struct intel_pipe_wm target;
> + /* intermediate watermarks for pending/active->target transition */
> + struct intel_pipe_wm intm;
It seems you always prefer shorter names such as "intm", and I usually
prefer the longer ones like "intermediate". Looks like this is a
common topic for my bikesheddings on your patches. When I read "intm"
my brain parses it as "Int M" and then aborts execution =P
With or without that changed: Reviewed-by: Paulo Zanoni
<paulo.r.zanoni at intel.com>
> +};
> +
> struct intel_crtc {
> struct drm_crtc base;
> enum pipe pipe;
> @@ -1001,6 +1008,10 @@ 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 ilk_update_pipe_wm(struct drm_device *dev, enum pipe pipe);
> +void intel_program_watermarks_pre(struct intel_crtc *crtc,
> + const struct intel_crtc_wm_config *config);
> +void intel_program_watermarks_post(struct intel_crtc *crtc,
> + const struct intel_crtc_wm_config *config);
>
>
> /* intel_sdvo.c */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6fc6416..ccf920a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2950,6 +2950,20 @@ void ilk_update_pipe_wm(struct drm_device *dev, enum pipe pipe)
> spin_unlock(&crtc->wm.lock);
> }
>
> +static void ilk_wm_cancel(struct intel_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->base.dev;
> +
> + assert_spin_locked(&crtc->wm.lock);
> +
> + crtc->wm.dirty = false;
> +
> + if (crtc->wm.vblank) {
> + drm_vblank_put(dev, crtc->pipe);
> + crtc->wm.vblank = false;
> + }
> +}
> +
> static void ilk_update_sprite_wm(struct drm_plane *plane,
> struct drm_crtc *crtc,
> uint32_t sprite_width, int pixel_size,
> @@ -3084,6 +3098,24 @@ void intel_update_sprite_watermarks(struct drm_plane *plane,
> pixel_size, enabled, scaled);
> }
>
> +void intel_program_watermarks_pre(struct intel_crtc *crtc,
> + const struct intel_crtc_wm_config *config)
> +{
> + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> +
> + if (dev_priv->display.program_wm_pre)
> + dev_priv->display.program_wm_pre(crtc, config);
> +}
> +
> +void intel_program_watermarks_post(struct intel_crtc *crtc,
> + const struct intel_crtc_wm_config *config)
> +{
> + struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> +
> + if (dev_priv->display.program_wm_post)
> + dev_priv->display.program_wm_post(crtc, config);
> +}
> +
> static struct drm_i915_gem_object *
> intel_alloc_context_page(struct drm_device *dev)
> {
> @@ -6522,6 +6554,60 @@ void intel_fini_runtime_pm(struct drm_i915_private *dev_priv)
> pm_runtime_disable(device);
> }
>
> +static void ilk_program_wm_pre(struct intel_crtc *crtc,
> + const struct intel_crtc_wm_config *config)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + mutex_lock(&dev_priv->wm.mutex);
> +
> + spin_lock_irq(&crtc->wm.lock);
> + ilk_wm_cancel(crtc);
> + spin_unlock_irq(&crtc->wm.lock);
> +
> + /* pending update (if any) got cancelled */
> + crtc->wm.pending = crtc->wm.active;
> +
> + if (!memcmp(&crtc->wm.active, &config->intm, sizeof(config->intm)))
> + goto unlock;
> +
> + crtc->wm.active = config->intm;
> +
> + /* use the most up to date watermarks for other pipes */
> + ilk_refresh_pending_watermarks(dev);
> +
> + /* switch over to the intermediate watermarks */
> + ilk_program_watermarks(dev);
> +
> + unlock:
> + mutex_unlock(&dev_priv->wm.mutex);
> +}
> +
> +static void ilk_program_wm_post(struct intel_crtc *crtc,
> + const struct intel_crtc_wm_config *config)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + u32 vbl_count;
> +
> + /*
> + * FIXME sample this inside the atomic section to avoid
> + * needlessly long periods w/ sub-par watermarks
> + */
> + vbl_count = dev->driver->get_vblank_counter(dev, crtc->pipe);
> +
> + mutex_lock(&dev_priv->wm.mutex);
> +
> + /*
> + * We can switch over to the target
> + * watermarks after the next vblank.
> + */
> + ilk_setup_pending_watermarks(crtc, &config->target, vbl_count);
> +
> + mutex_unlock(&dev_priv->wm.mutex);
> +}
> +
> /* Set up chip specific power management-related functions */
> void intel_init_pm(struct drm_device *dev)
> {
> @@ -6569,6 +6655,8 @@ void intel_init_pm(struct drm_device *dev)
> 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;
> + dev_priv->display.program_wm_pre = ilk_program_wm_pre;
> + dev_priv->display.program_wm_post = ilk_program_wm_post;
> } else {
> DRM_DEBUG_KMS("Failed to read display plane latency. "
> "Disable CxSR\n");
> --
> 1.8.5.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
More information about the Intel-gfx
mailing list