From: Matt Roper <matthew.d.roper@intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH v2 07/11] drm/i915: Add a atomic evasion step to watermark programming, v2.
Date: Wed, 26 Oct 2016 16:10:26 -0700 [thread overview]
Message-ID: <20161026231026.GB5924@intel.com> (raw)
In-Reply-To: <1477489299-25777-8-git-send-email-maarten.lankhorst@linux.intel.com>
Minor grammar fix in the headline...it should be "an atomic" rather than
"a atomic." Sorry I missed that the first time around.
I know i915 tends to be somewhat lax about adhering to the 80-character
line limit, but the lines that add the extra parameter are starting to
get _really_ long now, so it would be good to wrap them if possible.
Otherwise the change here look good to me, so with the minor cosmetic
stuff fixed,
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Matt
On Wed, Oct 26, 2016 at 03:41:35PM +0200, Maarten Lankhorst wrote:
> Allow the driver to write watermarks during atomic evasion.
> This will make it possible to write the watermarks in a cleaner
> way on gen9+.
>
> intel_atomic_state is not used here yet, but will be used when
> we program all watermarks as a separate step during evasion.
>
> This also writes linetime all the time, while before it was only
> done during plane updates. This looks like this could be a bugfix,
> but I'm not sure what it affects.
>
> Changes since v1:
> - Add comment about atomic evasion to commit message.
> - Unwrap I915_WRITE call. (Lyude)
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 6 ++++--
> drivers/gpu/drm/i915/intel_display.c | 20 +++++++++-----------
> drivers/gpu/drm/i915/intel_pm.c | 18 ++++++++++++++++--
> 3 files changed, 29 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7a621c74254e..7a477d6a486e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -485,6 +485,7 @@ struct sdvo_device_mapping {
>
> struct intel_connector;
> struct intel_encoder;
> +struct intel_atomic_state;
> struct intel_crtc_state;
> struct intel_initial_plane_config;
> struct intel_crtc;
> @@ -498,8 +499,9 @@ struct drm_i915_display_funcs {
> int (*compute_intermediate_wm)(struct drm_device *dev,
> struct intel_crtc *intel_crtc,
> struct intel_crtc_state *newstate);
> - void (*initial_watermarks)(struct intel_crtc_state *cstate);
> - void (*optimize_watermarks)(struct intel_crtc_state *cstate);
> + void (*initial_watermarks)(struct intel_atomic_state *state, struct intel_crtc_state *cstate);
> + void (*atomic_evade_watermarks)(struct intel_atomic_state *state, struct intel_crtc_state *cstate);
> + void (*optimize_watermarks)(struct intel_atomic_state *state, struct intel_crtc_state *cstate);
> int (*compute_global_watermarks)(struct drm_atomic_state *state);
> void (*update_wm)(struct drm_crtc *crtc);
> int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9ea9a3bc0bc0..48eb0635ec0f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5170,7 +5170,7 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
> * us to.
> */
> if (dev_priv->display.initial_watermarks != NULL)
> - dev_priv->display.initial_watermarks(pipe_config);
> + dev_priv->display.initial_watermarks(to_intel_atomic_state(old_state), pipe_config);
> else if (pipe_config->update_wm_pre)
> intel_update_watermarks(&crtc->base);
> }
> @@ -5384,7 +5384,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config,
> intel_color_load_luts(&pipe_config->base);
>
> if (dev_priv->display.initial_watermarks != NULL)
> - dev_priv->display.initial_watermarks(intel_crtc->config);
> + dev_priv->display.initial_watermarks(to_intel_atomic_state(old_state), intel_crtc->config);
> intel_enable_pipe(intel_crtc);
>
> if (intel_crtc->config->has_pch_encoder)
> @@ -5490,7 +5490,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config,
> intel_ddi_enable_transcoder_func(crtc);
>
> if (dev_priv->display.initial_watermarks != NULL)
> - dev_priv->display.initial_watermarks(pipe_config);
> + dev_priv->display.initial_watermarks(to_intel_atomic_state(old_state), pipe_config);
> else
> intel_update_watermarks(crtc);
>
> @@ -14526,7 +14526,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> intel_cstate = to_intel_crtc_state(crtc->state);
>
> if (dev_priv->display.optimize_watermarks)
> - dev_priv->display.optimize_watermarks(intel_cstate);
> + dev_priv->display.optimize_watermarks(intel_state, intel_cstate);
> }
>
> for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
> @@ -14934,7 +14934,6 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
> struct intel_crtc_state *old_intel_state =
> to_intel_crtc_state(old_crtc_state);
> bool modeset = needs_modeset(crtc->state);
> - enum pipe pipe = intel_crtc->pipe;
>
> /* Perform vblank evasion around commit operation */
> intel_pipe_update_start(intel_crtc);
> @@ -14947,14 +14946,13 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc,
> intel_color_load_luts(crtc->state);
> }
>
> - if (intel_cstate->update_pipe) {
> + if (intel_cstate->update_pipe)
> intel_update_pipe_config(intel_crtc, old_intel_state);
> - } else if (INTEL_GEN(dev_priv) >= 9) {
> + else if (INTEL_GEN(dev_priv) >= 9)
> skl_detach_scalers(intel_crtc);
>
> - I915_WRITE(PIPE_WM_LINETIME(pipe),
ma> - intel_cstate->wm.skl.optimal.linetime);
> - }
> + if (dev_priv->display.atomic_evade_watermarks)
> + dev_priv->display.atomic_evade_watermarks(to_intel_atomic_state(old_crtc_state->state), intel_cstate);
> }
>
> static void intel_finish_crtc_commit(struct drm_crtc *crtc,
> @@ -16408,7 +16406,7 @@ static void sanitize_watermarks(struct drm_device *dev)
> struct intel_crtc_state *cs = to_intel_crtc_state(cstate);
>
> cs->wm.need_postvbl_update = true;
> - dev_priv->display.optimize_watermarks(cs);
> + dev_priv->display.optimize_watermarks(to_intel_atomic_state(state), cs);
> }
>
> put_state:
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 41953cc41809..5ff35833b258 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4199,6 +4199,17 @@ skl_compute_wm(struct drm_atomic_state *state)
> return 0;
> }
>
> +static void skl_evade_crtc_wm(struct intel_atomic_state *state,
> + struct intel_crtc_state *cstate)
> +{
> + struct intel_crtc *crtc = to_intel_crtc(cstate->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> + struct skl_pipe_wm *pipe_wm = &cstate->wm.skl.optimal;
> + enum pipe pipe = crtc->pipe;
> +
> + I915_WRITE(PIPE_WM_LINETIME(pipe), pipe_wm->linetime);
> +}
> +
> static void skl_update_wm(struct drm_crtc *crtc)
> {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -4292,7 +4303,8 @@ static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
> ilk_write_wm_values(dev_priv, &results);
> }
>
> -static void ilk_initial_watermarks(struct intel_crtc_state *cstate)
> +static void ilk_initial_watermarks(struct intel_atomic_state *state,
> + struct intel_crtc_state *cstate)
> {
> struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> @@ -4303,7 +4315,8 @@ static void ilk_initial_watermarks(struct intel_crtc_state *cstate)
> mutex_unlock(&dev_priv->wm.wm_mutex);
> }
>
> -static void ilk_optimize_watermarks(struct intel_crtc_state *cstate)
> +static void ilk_optimize_watermarks(struct intel_atomic_state *state,
> + struct intel_crtc_state *cstate)
> {
> struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
> @@ -7741,6 +7754,7 @@ void intel_init_pm(struct drm_device *dev)
> if (INTEL_INFO(dev)->gen >= 9) {
> skl_setup_wm_latency(dev);
> dev_priv->display.update_wm = skl_update_wm;
> + dev_priv->display.atomic_evade_watermarks = skl_evade_crtc_wm;
> dev_priv->display.compute_global_watermarks = skl_compute_wm;
> } else if (HAS_PCH_SPLIT(dev_priv)) {
> ilk_setup_wm_latency(dev);
> --
> 2.7.4
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-10-26 23:10 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-26 13:41 [PATCH v2 00/11] drm/i915/gen9+: Atomic watermark fixes Maarten Lankhorst
2016-10-26 13:41 ` [PATCH v2 01/11] drm/i915/skl+: Prepare for removing data rate from skl watermark state, v2 Maarten Lankhorst
2016-10-27 7:12 ` Daniel Vetter
2016-10-27 8:31 ` Maarten Lankhorst
2016-10-27 17:48 ` Paulo Zanoni
2016-10-27 17:55 ` Matt Roper
2016-10-26 13:41 ` [PATCH v2 02/11] drm/i915/gen9+: Use cstate plane mask instead of crtc->state Maarten Lankhorst
2016-10-27 14:50 ` Matt Roper
2016-10-26 13:41 ` [PATCH v2 03/11] drm/i915/gen9+: Use for_each_intel_plane_on_crtc in skl_print_wm_changes Maarten Lankhorst
2016-10-27 13:03 ` Ville Syrjälä
2016-10-27 17:47 ` Matt Roper
2016-10-27 17:57 ` Paulo Zanoni
2016-11-01 11:04 ` [PATCH v2.1 03/11] drm/i915/gen9+: Use for_each_intel_plane_on_crtc in skl_print_wm_changes, v2 Maarten Lankhorst
2016-11-01 12:33 ` Paulo Zanoni
2016-10-26 13:41 ` [PATCH v2 04/11] drm/i915/skl+: Remove data_rate from watermark struct, v2 Maarten Lankhorst
2016-10-27 18:14 ` Paulo Zanoni
2016-10-26 13:41 ` [PATCH v2 05/11] drm/i915/skl+: Remove minimum block allocation from crtc state Maarten Lankhorst
2016-10-26 13:41 ` [PATCH v2 06/11] drm/i915/skl+: Clean up minimum allocations, v2 Maarten Lankhorst
2016-10-27 19:25 ` Paulo Zanoni
2016-10-26 13:41 ` [PATCH v2 07/11] drm/i915: Add a atomic evasion step to watermark programming, v2 Maarten Lankhorst
2016-10-26 16:19 ` Ville Syrjälä
2016-10-27 7:10 ` Daniel Vetter
2016-10-27 10:37 ` [PATCH v2.1 07/11] drm/i915: Add a atomic evasion step to watermark programming, v3 Maarten Lankhorst
2016-10-26 23:10 ` Matt Roper [this message]
2016-10-26 13:41 ` [PATCH v2 08/11] drm/i915/gen9+: Use the watermarks from crtc_state for everything, v2 Maarten Lankhorst
2016-10-26 23:13 ` Matt Roper
2016-10-26 13:41 ` [PATCH v2 09/11] drm/i915/gen9+: Program watermarks as a separate step during evasion, v2 Maarten Lankhorst
2016-10-26 16:05 ` Lyude Paul
2016-10-26 23:24 ` Matt Roper
2016-10-27 8:29 ` Maarten Lankhorst
2016-10-27 11:45 ` [PATCH v2.1 09/11] drm/i915/gen9+: Program watermarks as a separate step during evasion, v3 Maarten Lankhorst
2016-10-26 13:41 ` [PATCH v2 10/11] drm/i915/gen9+: Preserve old allocation from crtc_state Maarten Lankhorst
2016-10-26 13:41 ` [PATCH v2 11/11] drm/i915/gen9+: Kill off hw_ddb from intel_crtc Maarten Lankhorst
2016-10-26 14:16 ` ✓ Fi.CI.BAT: success for drm/i915/gen9+: Atomic watermark fixes Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161026231026.GB5924@intel.com \
--to=matthew.d.roper@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=paulo.r.zanoni@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.