* [PATCH v2 0/3] drm/i915: Enable fastboot, v3!
@ 2017-11-20 12:23 Maarten Lankhorst
2017-11-20 12:23 ` [PATCH v2 1/3] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v2 Maarten Lankhorst
` (7 more replies)
0 siblings, 8 replies; 22+ messages in thread
From: Maarten Lankhorst @ 2017-11-20 12:23 UTC (permalink / raw)
To: intel-gfx
Updates to IPS fixes, and then flipping the switch. :)
Maarten Lankhorst (3):
drm/i915: Make ips_enabled a property depending on whether IPS is
enabled, v2.
drm/i915: Enable IPS with only sprite plane visible too, v2.
drm/i915: Re-enable fastboot by default
drivers/gpu/drm/i915/i915_params.h | 2 +-
drivers/gpu/drm/i915/intel_cdclk.c | 2 +-
drivers/gpu/drm/i915/intel_display.c | 114 ++++++++++++++++++----------------
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_pipe_crc.c | 2 -
5 files changed, 63 insertions(+), 58 deletions(-)
--
2.15.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH v2 1/3] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v2. 2017-11-20 12:23 [PATCH v2 0/3] drm/i915: Enable fastboot, v3! Maarten Lankhorst @ 2017-11-20 12:23 ` Maarten Lankhorst 2017-11-20 14:01 ` Ville Syrjälä 2017-11-20 12:23 ` [PATCH v2 2/3] drm/i915: Enable IPS with only sprite plane visible too, v2 Maarten Lankhorst ` (6 subsequent siblings) 7 siblings, 1 reply; 22+ messages in thread From: Maarten Lankhorst @ 2017-11-20 12:23 UTC (permalink / raw) To: intel-gfx ips_enabled was used as a variable of whether IPS can be enabled or not, but should be used to test whether IPS is actually enabled. Changes since v1: - Call needs_modeset on new crtc state. (Ville) - IPS can be enabled with sprite plane enabled too. (Ville) - Fix CDCLK vs IPS workaround. (Ville) Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_cdclk.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 119 +++++++++++++++++++--------------- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_pipe_crc.c | 2 - 4 files changed, 67 insertions(+), 57 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index e8884c2ade98..30affa7903d7 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1896,7 +1896,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) min_cdclk = intel_pixel_rate_to_cdclk(dev_priv, crtc_state->pixel_rate); /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ - if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) + if (IS_BROADWELL(dev_priv) && hsw_pipe_config_ips_capable(crtc_state)) min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 95); /* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3b3dec1e6640..0b9ba415839a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4870,7 +4870,7 @@ void hsw_enable_ips(const struct intel_crtc_state *crtc_state) struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); - if (!crtc->config->ips_enabled) + if (!crtc_state->ips_enabled) return; /* @@ -4966,14 +4966,6 @@ intel_post_enable_primary(struct drm_crtc *crtc, struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int pipe = intel_crtc->pipe; - /* - * FIXME IPS should be fine as long as one plane is - * enabled, but in practice it seems to have problems - * when going from primary only to sprite only and vice - * versa. - */ - hsw_enable_ips(new_crtc_state); - /* * Gen2 reports pipe underruns whenever all planes are disabled. * So don't enable underrun reporting before at least some planes @@ -4989,10 +4981,9 @@ intel_post_enable_primary(struct drm_crtc *crtc, intel_check_pch_fifo_underruns(dev_priv); } -/* FIXME move all this to pre_plane_update() with proper state tracking */ +/* FIXME get rid of this and use pre_plane_update */ static void -intel_pre_disable_primary(struct drm_crtc *crtc, - const struct intel_crtc_state *old_crtc_state) +intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = to_i915(dev); @@ -5001,32 +4992,12 @@ intel_pre_disable_primary(struct drm_crtc *crtc, /* * Gen2 reports pipe underruns whenever all planes are disabled. - * So diasble underrun reporting before all the planes get disabled. - * FIXME: Need to fix the logic to work when we turn off all planes - * but leave the pipe running. + * So disable underrun reporting before all the planes get disabled. */ if (IS_GEN2(dev_priv)) intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); - /* - * FIXME IPS should be fine as long as one plane is - * enabled, but in practice it seems to have problems - * when going from primary only to sprite only and vice - * versa. - */ - hsw_disable_ips(old_crtc_state); -} - -/* FIXME get rid of this and use pre_plane_update */ -static void -intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) -{ - struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = to_i915(dev); - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int pipe = intel_crtc->pipe; - - intel_pre_disable_primary(crtc, to_intel_crtc_state(crtc->state)); + hsw_disable_ips(to_intel_crtc_state(crtc->state)); /* * Vblank time updates from the shadow to live plane control register @@ -5058,6 +5029,11 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) if (pipe_config->update_wm_post && pipe_config->base.active) intel_update_watermarks(crtc); + if (!old_crtc_state->ips_enabled || + needs_modeset(&pipe_config->base) || + pipe_config->update_pipe) + hsw_enable_ips(pipe_config); + if (old_pri_state) { struct intel_plane_state *primary_state = intel_atomic_get_new_plane_state(to_intel_atomic_state(old_state), @@ -5088,6 +5064,9 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, struct intel_atomic_state *old_intel_state = to_intel_atomic_state(old_state); + if (needs_modeset(&pipe_config->base) || !pipe_config->ips_enabled) + hsw_disable_ips(old_crtc_state); + if (old_pri_state) { struct intel_plane_state *primary_state = intel_atomic_get_new_plane_state(old_intel_state, @@ -5096,10 +5075,13 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, to_intel_plane_state(old_pri_state); intel_fbc_pre_update(crtc, pipe_config, primary_state); - - if (old_primary_state->base.visible && + /* + * Gen2 reports pipe underruns whenever all planes are disabled. + * So disable underrun reporting before all the planes get disabled. + */ + if (IS_GEN2(dev_priv) && old_primary_state->base.visible && (modeset || !primary_state->base.visible)) - intel_pre_disable_primary(&crtc->base, old_crtc_state); + intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); } /* @@ -6228,15 +6210,42 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc, return ret; } -static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, - struct intel_crtc_state *pipe_config) +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config) { - if (pipe_config->ips_force_disable) + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc); + + /* IPS only exists on ULT machines and is tied to pipe A. */ + if (!hsw_crtc_supports_ips(crtc)) + return false; + + if (!i915_modparams.enable_ips) return false; if (pipe_config->pipe_bpp > 24) return false; + return true; +} + +static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) +{ + struct drm_i915_private *dev_priv = to_i915(pipe_config->base.crtc->dev); + + if (!hsw_pipe_config_ips_capable(pipe_config)) + return false; + + if (pipe_config->ips_force_disable) + return false; + + /* + * FIXME IPS should be fine as long as one plane is + * enabled, but in practice it seems to have problems + * when going from primary only to sprite only and vice + * versa. + */ + if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) + return false; + /* HSW can handle pixel rate up to cdclk? */ if (IS_HASWELL(dev_priv)) return true; @@ -6252,17 +6261,6 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, dev_priv->max_cdclk_freq * 95 / 100; } -static void hsw_compute_ips_config(struct intel_crtc *crtc, - struct intel_crtc_state *pipe_config) -{ - struct drm_device *dev = crtc->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); - - pipe_config->ips_enabled = i915_modparams.enable_ips && - hsw_crtc_supports_ips(crtc) && - pipe_config_supports_ips(dev_priv, pipe_config); -} - static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc) { const struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -6378,9 +6376,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, intel_crtc_compute_pixel_rate(pipe_config); - if (HAS_IPS(dev_priv)) - hsw_compute_ips_config(crtc, pipe_config); - if (pipe_config->has_pch_encoder) return ironlake_fdi_compute_config(crtc, pipe_config); @@ -9275,6 +9270,19 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, ironlake_get_pfit_config(crtc, pipe_config); } + if (hsw_crtc_supports_ips(crtc)) { + if (IS_HASWELL(dev_priv)) + pipe_config->ips_enabled = I915_READ(IPS_CTL) & IPS_ENABLE; + else { + /* + * We cannot readout IPS state on broadwell, set to + * true so we can set it to a defined state on first + * commit. + */ + pipe_config->ips_enabled = true; + } + } + if (pipe_config->cpu_transcoder != TRANSCODER_EDP && !transcoder_is_dsi(pipe_config->cpu_transcoder)) { pipe_config->pixel_multiplier = @@ -10488,6 +10496,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, pipe_config); } + if (HAS_IPS(dev_priv)) + pipe_config->ips_enabled = hsw_compute_ips_config(pipe_config); + return ret; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 69aab324aaa1..40417b20844e 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1485,6 +1485,7 @@ bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock, int chv_calc_dpll_params(int refclk, struct dpll *pll_clock); bool intel_crtc_active(struct intel_crtc *crtc); +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config); void hsw_enable_ips(const struct intel_crtc_state *crtc_state); void hsw_disable_ips(const struct intel_crtc_state *crtc_state); enum intel_display_power_domain intel_port_to_power_domain(enum port port); diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c index 61641d479b93..1f5cd572a7ff 100644 --- a/drivers/gpu/drm/i915/intel_pipe_crc.c +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c @@ -541,8 +541,6 @@ static void hsw_pipe_A_crc_wa(struct drm_i915_private *dev_priv, * completely disable it. */ pipe_config->ips_force_disable = enable; - if (pipe_config->ips_enabled == enable) - pipe_config->base.connectors_changed = true; } if (IS_HASWELL(dev_priv)) { -- 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v2. 2017-11-20 12:23 ` [PATCH v2 1/3] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v2 Maarten Lankhorst @ 2017-11-20 14:01 ` Ville Syrjälä 2017-11-20 16:21 ` Maarten Lankhorst 2017-11-22 15:07 ` [PATCH] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v3 Maarten Lankhorst 0 siblings, 2 replies; 22+ messages in thread From: Ville Syrjälä @ 2017-11-20 14:01 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Mon, Nov 20, 2017 at 01:23:35PM +0100, Maarten Lankhorst wrote: > ips_enabled was used as a variable of whether IPS can be enabled or not, > but should be used to test whether IPS is actually enabled. > > Changes since v1: > - Call needs_modeset on new crtc state. (Ville) > - IPS can be enabled with sprite plane enabled too. (Ville) > - Fix CDCLK vs IPS workaround. (Ville) > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_cdclk.c | 2 +- > drivers/gpu/drm/i915/intel_display.c | 119 +++++++++++++++++++--------------- > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_pipe_crc.c | 2 - > 4 files changed, 67 insertions(+), 57 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index e8884c2ade98..30affa7903d7 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -1896,7 +1896,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) > min_cdclk = intel_pixel_rate_to_cdclk(dev_priv, crtc_state->pixel_rate); > > /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ > - if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) > + if (IS_BROADWELL(dev_priv) && hsw_pipe_config_ips_capable(crtc_state)) > min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 95); I think either we should drop this entirely and make the ips_compute_config() check logical.cdclk instead of max_cdclk, or we stick to the max_cdclk based check but move it into hsw_pipe_config_ips_capable(). Otherwise we would end up rejecting any mode whose pixel rate exceed the 95% cdclk limit. > > /* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz, > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 3b3dec1e6640..0b9ba415839a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4870,7 +4870,7 @@ void hsw_enable_ips(const struct intel_crtc_state *crtc_state) > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > > - if (!crtc->config->ips_enabled) > + if (!crtc_state->ips_enabled) > return; > > /* > @@ -4966,14 +4966,6 @@ intel_post_enable_primary(struct drm_crtc *crtc, > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > int pipe = intel_crtc->pipe; > > - /* > - * FIXME IPS should be fine as long as one plane is > - * enabled, but in practice it seems to have problems > - * when going from primary only to sprite only and vice > - * versa. > - */ > - hsw_enable_ips(new_crtc_state); > - > /* > * Gen2 reports pipe underruns whenever all planes are disabled. > * So don't enable underrun reporting before at least some planes > @@ -4989,10 +4981,9 @@ intel_post_enable_primary(struct drm_crtc *crtc, > intel_check_pch_fifo_underruns(dev_priv); > } > > -/* FIXME move all this to pre_plane_update() with proper state tracking */ > +/* FIXME get rid of this and use pre_plane_update */ > static void > -intel_pre_disable_primary(struct drm_crtc *crtc, > - const struct intel_crtc_state *old_crtc_state) > +intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > @@ -5001,32 +4992,12 @@ intel_pre_disable_primary(struct drm_crtc *crtc, > > /* > * Gen2 reports pipe underruns whenever all planes are disabled. > - * So diasble underrun reporting before all the planes get disabled. > - * FIXME: Need to fix the logic to work when we turn off all planes > - * but leave the pipe running. > + * So disable underrun reporting before all the planes get disabled. > */ > if (IS_GEN2(dev_priv)) > intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); > > - /* > - * FIXME IPS should be fine as long as one plane is > - * enabled, but in practice it seems to have problems > - * when going from primary only to sprite only and vice > - * versa. > - */ > - hsw_disable_ips(old_crtc_state); > -} > - > -/* FIXME get rid of this and use pre_plane_update */ > -static void > -intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) > -{ > - struct drm_device *dev = crtc->dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - int pipe = intel_crtc->pipe; > - > - intel_pre_disable_primary(crtc, to_intel_crtc_state(crtc->state)); > + hsw_disable_ips(to_intel_crtc_state(crtc->state)); > > /* > * Vblank time updates from the shadow to live plane control register > @@ -5058,6 +5029,11 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) > if (pipe_config->update_wm_post && pipe_config->base.active) > intel_update_watermarks(crtc); > > + if (!old_crtc_state->ips_enabled || > + needs_modeset(&pipe_config->base) || > + pipe_config->update_pipe) Hmm. So thinking more about this I guess I figured out why you put the update_pipe check here. On BDW the initial old_crtc_state will have ips_enabled=true since the readout now always makes it so. Thus if we skip the initial modeset we would not turn on IPS even if a suitable set of planes is enabled. I think it would be more clear to check for the INHERITED thing specifically. Also that would avoid paying the cost of the pcode access every time we have update_pipe==true and ips_enabled==true. > + hsw_enable_ips(pipe_config); > + > if (old_pri_state) { > struct intel_plane_state *primary_state = > intel_atomic_get_new_plane_state(to_intel_atomic_state(old_state), > @@ -5088,6 +5064,9 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > struct intel_atomic_state *old_intel_state = > to_intel_atomic_state(old_state); > > + if (needs_modeset(&pipe_config->base) || !pipe_config->ips_enabled) > + hsw_disable_ips(old_crtc_state); > + > if (old_pri_state) { > struct intel_plane_state *primary_state = > intel_atomic_get_new_plane_state(old_intel_state, > @@ -5096,10 +5075,13 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > to_intel_plane_state(old_pri_state); > > intel_fbc_pre_update(crtc, pipe_config, primary_state); > - > - if (old_primary_state->base.visible && > + /* > + * Gen2 reports pipe underruns whenever all planes are disabled. > + * So disable underrun reporting before all the planes get disabled. > + */ > + if (IS_GEN2(dev_priv) && old_primary_state->base.visible && > (modeset || !primary_state->base.visible)) > - intel_pre_disable_primary(&crtc->base, old_crtc_state); > + intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); > } > > /* > @@ -6228,15 +6210,42 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc, > return ret; > } > > -static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, > - struct intel_crtc_state *pipe_config) > +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config) > { > - if (pipe_config->ips_force_disable) > + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc); > + > + /* IPS only exists on ULT machines and is tied to pipe A. */ > + if (!hsw_crtc_supports_ips(crtc)) > + return false; > + > + if (!i915_modparams.enable_ips) > return false; > > if (pipe_config->pipe_bpp > 24) > return false; > > + return true; > +} > + > +static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) > +{ > + struct drm_i915_private *dev_priv = to_i915(pipe_config->base.crtc->dev); > + > + if (!hsw_pipe_config_ips_capable(pipe_config)) > + return false; > + > + if (pipe_config->ips_force_disable) > + return false; > + > + /* > + * FIXME IPS should be fine as long as one plane is > + * enabled, but in practice it seems to have problems > + * when going from primary only to sprite only and vice > + * versa. > + */ > + if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) > + return false; > + > /* HSW can handle pixel rate up to cdclk? */ > if (IS_HASWELL(dev_priv)) > return true; > @@ -6252,17 +6261,6 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, > dev_priv->max_cdclk_freq * 95 / 100; > } > > -static void hsw_compute_ips_config(struct intel_crtc *crtc, > - struct intel_crtc_state *pipe_config) > -{ > - struct drm_device *dev = crtc->base.dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - > - pipe_config->ips_enabled = i915_modparams.enable_ips && > - hsw_crtc_supports_ips(crtc) && > - pipe_config_supports_ips(dev_priv, pipe_config); > -} > - > static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc) > { > const struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > @@ -6378,9 +6376,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, > > intel_crtc_compute_pixel_rate(pipe_config); > > - if (HAS_IPS(dev_priv)) > - hsw_compute_ips_config(crtc, pipe_config); > - > if (pipe_config->has_pch_encoder) > return ironlake_fdi_compute_config(crtc, pipe_config); > > @@ -9275,6 +9270,19 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, > ironlake_get_pfit_config(crtc, pipe_config); > } > > + if (hsw_crtc_supports_ips(crtc)) { > + if (IS_HASWELL(dev_priv)) > + pipe_config->ips_enabled = I915_READ(IPS_CTL) & IPS_ENABLE; > + else { > + /* > + * We cannot readout IPS state on broadwell, set to > + * true so we can set it to a defined state on first > + * commit. > + */ > + pipe_config->ips_enabled = true; > + } > + } > + > if (pipe_config->cpu_transcoder != TRANSCODER_EDP && > !transcoder_is_dsi(pipe_config->cpu_transcoder)) { > pipe_config->pixel_multiplier = > @@ -10488,6 +10496,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > pipe_config); > } > > + if (HAS_IPS(dev_priv)) > + pipe_config->ips_enabled = hsw_compute_ips_config(pipe_config); > + > return ret; > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 69aab324aaa1..40417b20844e 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1485,6 +1485,7 @@ bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock, > int chv_calc_dpll_params(int refclk, struct dpll *pll_clock); > > bool intel_crtc_active(struct intel_crtc *crtc); > +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config); > void hsw_enable_ips(const struct intel_crtc_state *crtc_state); > void hsw_disable_ips(const struct intel_crtc_state *crtc_state); > enum intel_display_power_domain intel_port_to_power_domain(enum port port); > diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c > index 61641d479b93..1f5cd572a7ff 100644 > --- a/drivers/gpu/drm/i915/intel_pipe_crc.c > +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c > @@ -541,8 +541,6 @@ static void hsw_pipe_A_crc_wa(struct drm_i915_private *dev_priv, > * completely disable it. > */ > pipe_config->ips_force_disable = enable; > - if (pipe_config->ips_enabled == enable) > - pipe_config->base.connectors_changed = true; > } > > if (IS_HASWELL(dev_priv)) { > -- > 2.15.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v2. 2017-11-20 14:01 ` Ville Syrjälä @ 2017-11-20 16:21 ` Maarten Lankhorst 2017-11-22 15:07 ` [PATCH] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v3 Maarten Lankhorst 1 sibling, 0 replies; 22+ messages in thread From: Maarten Lankhorst @ 2017-11-20 16:21 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx Op 20-11-17 om 15:01 schreef Ville Syrjälä: > On Mon, Nov 20, 2017 at 01:23:35PM +0100, Maarten Lankhorst wrote: >> ips_enabled was used as a variable of whether IPS can be enabled or not, >> but should be used to test whether IPS is actually enabled. >> >> Changes since v1: >> - Call needs_modeset on new crtc state. (Ville) >> - IPS can be enabled with sprite plane enabled too. (Ville) >> - Fix CDCLK vs IPS workaround. (Ville) >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_cdclk.c | 2 +- >> drivers/gpu/drm/i915/intel_display.c | 119 +++++++++++++++++++--------------- >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> drivers/gpu/drm/i915/intel_pipe_crc.c | 2 - >> 4 files changed, 67 insertions(+), 57 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c >> index e8884c2ade98..30affa7903d7 100644 >> --- a/drivers/gpu/drm/i915/intel_cdclk.c >> +++ b/drivers/gpu/drm/i915/intel_cdclk.c >> @@ -1896,7 +1896,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) >> min_cdclk = intel_pixel_rate_to_cdclk(dev_priv, crtc_state->pixel_rate); >> >> /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ >> - if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) >> + if (IS_BROADWELL(dev_priv) && hsw_pipe_config_ips_capable(crtc_state)) >> min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 95); > I think either we should drop this entirely and make the > ips_compute_config() check logical.cdclk instead of max_cdclk, > or we stick to the max_cdclk based check but move it into > hsw_pipe_config_ips_capable(). Otherwise we would end up > rejecting any mode whose pixel rate exceed the 95% cdclk > limit. > >> >> /* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz, >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 3b3dec1e6640..0b9ba415839a 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -4870,7 +4870,7 @@ void hsw_enable_ips(const struct intel_crtc_state *crtc_state) >> struct drm_device *dev = crtc->base.dev; >> struct drm_i915_private *dev_priv = to_i915(dev); >> >> - if (!crtc->config->ips_enabled) >> + if (!crtc_state->ips_enabled) >> return; >> >> /* >> @@ -4966,14 +4966,6 @@ intel_post_enable_primary(struct drm_crtc *crtc, >> struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> int pipe = intel_crtc->pipe; >> >> - /* >> - * FIXME IPS should be fine as long as one plane is >> - * enabled, but in practice it seems to have problems >> - * when going from primary only to sprite only and vice >> - * versa. >> - */ >> - hsw_enable_ips(new_crtc_state); >> - >> /* >> * Gen2 reports pipe underruns whenever all planes are disabled. >> * So don't enable underrun reporting before at least some planes >> @@ -4989,10 +4981,9 @@ intel_post_enable_primary(struct drm_crtc *crtc, >> intel_check_pch_fifo_underruns(dev_priv); >> } >> >> -/* FIXME move all this to pre_plane_update() with proper state tracking */ >> +/* FIXME get rid of this and use pre_plane_update */ >> static void >> -intel_pre_disable_primary(struct drm_crtc *crtc, >> - const struct intel_crtc_state *old_crtc_state) >> +intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) >> { >> struct drm_device *dev = crtc->dev; >> struct drm_i915_private *dev_priv = to_i915(dev); >> @@ -5001,32 +4992,12 @@ intel_pre_disable_primary(struct drm_crtc *crtc, >> >> /* >> * Gen2 reports pipe underruns whenever all planes are disabled. >> - * So diasble underrun reporting before all the planes get disabled. >> - * FIXME: Need to fix the logic to work when we turn off all planes >> - * but leave the pipe running. >> + * So disable underrun reporting before all the planes get disabled. >> */ >> if (IS_GEN2(dev_priv)) >> intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); >> >> - /* >> - * FIXME IPS should be fine as long as one plane is >> - * enabled, but in practice it seems to have problems >> - * when going from primary only to sprite only and vice >> - * versa. >> - */ >> - hsw_disable_ips(old_crtc_state); >> -} >> - >> -/* FIXME get rid of this and use pre_plane_update */ >> -static void >> -intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) >> -{ >> - struct drm_device *dev = crtc->dev; >> - struct drm_i915_private *dev_priv = to_i915(dev); >> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> - int pipe = intel_crtc->pipe; >> - >> - intel_pre_disable_primary(crtc, to_intel_crtc_state(crtc->state)); >> + hsw_disable_ips(to_intel_crtc_state(crtc->state)); >> >> /* >> * Vblank time updates from the shadow to live plane control register >> @@ -5058,6 +5029,11 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) >> if (pipe_config->update_wm_post && pipe_config->base.active) >> intel_update_watermarks(crtc); >> >> + if (!old_crtc_state->ips_enabled || >> + needs_modeset(&pipe_config->base) || >> + pipe_config->update_pipe) > Hmm. So thinking more about this I guess I figured out why you put the > update_pipe check here. On BDW the initial old_crtc_state will have > ips_enabled=true since the readout now always makes it so. Thus if we > skip the initial modeset we would not turn on IPS even if a suitable > set of planes is enabled. > > I think it would be more clear to check for the INHERITED thing > specifically. Also that would avoid paying the cost of the pcode > access every time we have update_pipe==true and ips_enabled==true. In case of update_pipe = true we already avoided a modeset, which is an order more expensive. I'm not too worried about it. :p Maybe hsw_ips_pre/post_update with the relevant checks? _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v3. 2017-11-20 14:01 ` Ville Syrjälä 2017-11-20 16:21 ` Maarten Lankhorst @ 2017-11-22 15:07 ` Maarten Lankhorst 2017-11-22 15:34 ` Ville Syrjälä 1 sibling, 1 reply; 22+ messages in thread From: Maarten Lankhorst @ 2017-11-22 15:07 UTC (permalink / raw) To: intel-gfx ips_enabled was used as a variable of whether IPS can be enabled or not, but should be used to test whether IPS is actually enabled. Changes since v1: - Call needs_modeset on new crtc state. (Ville) - IPS can be enabled with sprite plane enabled too. (Ville) - Fix CDCLK vs IPS workaround. (Ville) Changes since v2: - Only re-enable fastset when inheriting mode. (Ville) - Put the conditions for enabling and disabling IPS in a helper. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_cdclk.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 151 +++++++++++++++++++++------------- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_pipe_crc.c | 2 - 4 files changed, 98 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index e8884c2ade98..30affa7903d7 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1896,7 +1896,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) min_cdclk = intel_pixel_rate_to_cdclk(dev_priv, crtc_state->pixel_rate); /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ - if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) + if (IS_BROADWELL(dev_priv) && hsw_pipe_config_ips_capable(crtc_state)) min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 95); /* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2007c69468b9..b62cb6e90669 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -489,7 +489,7 @@ static const struct intel_limit intel_limits_bxt = { }; static bool -needs_modeset(struct drm_crtc_state *state) +needs_modeset(const struct drm_crtc_state *state) { return drm_atomic_crtc_needs_modeset(state); } @@ -4870,7 +4870,7 @@ void hsw_enable_ips(const struct intel_crtc_state *crtc_state) struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); - if (!crtc->config->ips_enabled) + if (!crtc_state->ips_enabled) return; /* @@ -4966,14 +4966,6 @@ intel_post_enable_primary(struct drm_crtc *crtc, struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int pipe = intel_crtc->pipe; - /* - * FIXME IPS should be fine as long as one plane is - * enabled, but in practice it seems to have problems - * when going from primary only to sprite only and vice - * versa. - */ - hsw_enable_ips(new_crtc_state); - /* * Gen2 reports pipe underruns whenever all planes are disabled. * So don't enable underrun reporting before at least some planes @@ -4989,10 +4981,9 @@ intel_post_enable_primary(struct drm_crtc *crtc, intel_check_pch_fifo_underruns(dev_priv); } -/* FIXME move all this to pre_plane_update() with proper state tracking */ +/* FIXME get rid of this and use pre_plane_update */ static void -intel_pre_disable_primary(struct drm_crtc *crtc, - const struct intel_crtc_state *old_crtc_state) +intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = to_i915(dev); @@ -5001,32 +4992,12 @@ intel_pre_disable_primary(struct drm_crtc *crtc, /* * Gen2 reports pipe underruns whenever all planes are disabled. - * So diasble underrun reporting before all the planes get disabled. - * FIXME: Need to fix the logic to work when we turn off all planes - * but leave the pipe running. + * So disable underrun reporting before all the planes get disabled. */ if (IS_GEN2(dev_priv)) intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); - /* - * FIXME IPS should be fine as long as one plane is - * enabled, but in practice it seems to have problems - * when going from primary only to sprite only and vice - * versa. - */ - hsw_disable_ips(old_crtc_state); -} - -/* FIXME get rid of this and use pre_plane_update */ -static void -intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) -{ - struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = to_i915(dev); - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int pipe = intel_crtc->pipe; - - intel_pre_disable_primary(crtc, to_intel_crtc_state(crtc->state)); + hsw_disable_ips(to_intel_crtc_state(crtc->state)); /* * Vblank time updates from the shadow to live plane control register @@ -5042,6 +5013,38 @@ intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) intel_wait_for_vblank(dev_priv, pipe); } +static bool hsw_pre_update_disable_ips(const struct intel_crtc_state *old_crtc_state, + const struct intel_crtc_state *new_crtc_state) +{ + if (!old_crtc_state->ips_enabled) + return false; + + if (needs_modeset(&new_crtc_state->base)) + return true; + + return !new_crtc_state->ips_enabled; +} + +static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_state, + const struct intel_crtc_state *new_crtc_state) +{ + if (!new_crtc_state->ips_enabled) + return false; + + if (needs_modeset(&new_crtc_state->base)) + return true; + + /* + * We can't read out IPS on broadwell, assume the worst and + * forcibly enable IPS on the first fastset. + */ + if (new_crtc_state->update_pipe && + old_crtc_state->base.adjusted_mode.private_flags & I915_MODE_FLAG_INHERITED) + return true; + + return !old_crtc_state->ips_enabled; +} + static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) { struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc); @@ -5058,6 +5061,9 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) if (pipe_config->update_wm_post && pipe_config->base.active) intel_update_watermarks(crtc); + if (hsw_post_update_enable_ips(old_crtc_state, pipe_config)) + hsw_enable_ips(pipe_config); + if (old_pri_state) { struct intel_plane_state *primary_state = intel_atomic_get_new_plane_state(to_intel_atomic_state(old_state), @@ -5088,6 +5094,9 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, struct intel_atomic_state *old_intel_state = to_intel_atomic_state(old_state); + if (hsw_pre_update_disable_ips(old_crtc_state, pipe_config)) + hsw_disable_ips(old_crtc_state); + if (old_pri_state) { struct intel_plane_state *primary_state = intel_atomic_get_new_plane_state(old_intel_state, @@ -5096,10 +5105,13 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, to_intel_plane_state(old_pri_state); intel_fbc_pre_update(crtc, pipe_config, primary_state); - - if (old_primary_state->base.visible && + /* + * Gen2 reports pipe underruns whenever all planes are disabled. + * So disable underrun reporting before all the planes get disabled. + */ + if (IS_GEN2(dev_priv) && old_primary_state->base.visible && (modeset || !primary_state->base.visible)) - intel_pre_disable_primary(&crtc->base, old_crtc_state); + intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); } /* @@ -6228,15 +6240,42 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc, return ret; } -static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, - struct intel_crtc_state *pipe_config) +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config) { - if (pipe_config->ips_force_disable) + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc); + + /* IPS only exists on ULT machines and is tied to pipe A. */ + if (!hsw_crtc_supports_ips(crtc)) + return false; + + if (!i915_modparams.enable_ips) return false; if (pipe_config->pipe_bpp > 24) return false; + return true; +} + +static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) +{ + struct drm_i915_private *dev_priv = to_i915(pipe_config->base.crtc->dev); + + if (!hsw_pipe_config_ips_capable(pipe_config)) + return false; + + if (pipe_config->ips_force_disable) + return false; + + /* + * FIXME IPS should be fine as long as one plane is + * enabled, but in practice it seems to have problems + * when going from primary only to sprite only and vice + * versa. + */ + if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) + return false; + /* HSW can handle pixel rate up to cdclk? */ if (IS_HASWELL(dev_priv)) return true; @@ -6252,17 +6291,6 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, dev_priv->max_cdclk_freq * 95 / 100; } -static void hsw_compute_ips_config(struct intel_crtc *crtc, - struct intel_crtc_state *pipe_config) -{ - struct drm_device *dev = crtc->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); - - pipe_config->ips_enabled = i915_modparams.enable_ips && - hsw_crtc_supports_ips(crtc) && - pipe_config_supports_ips(dev_priv, pipe_config); -} - static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc) { const struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -6378,9 +6406,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, intel_crtc_compute_pixel_rate(pipe_config); - if (HAS_IPS(dev_priv)) - hsw_compute_ips_config(crtc, pipe_config); - if (pipe_config->has_pch_encoder) return ironlake_fdi_compute_config(crtc, pipe_config); @@ -9275,6 +9300,19 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, ironlake_get_pfit_config(crtc, pipe_config); } + if (hsw_crtc_supports_ips(crtc)) { + if (IS_HASWELL(dev_priv)) + pipe_config->ips_enabled = I915_READ(IPS_CTL) & IPS_ENABLE; + else { + /* + * We cannot readout IPS state on broadwell, set to + * true so we can set it to a defined state on first + * commit. + */ + pipe_config->ips_enabled = true; + } + } + if (pipe_config->cpu_transcoder != TRANSCODER_EDP && !transcoder_is_dsi(pipe_config->cpu_transcoder)) { pipe_config->pixel_multiplier = @@ -10489,6 +10527,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, pipe_config); } + if (HAS_IPS(dev_priv)) + pipe_config->ips_enabled = hsw_compute_ips_config(pipe_config); + return ret; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 69aab324aaa1..40417b20844e 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1485,6 +1485,7 @@ bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock, int chv_calc_dpll_params(int refclk, struct dpll *pll_clock); bool intel_crtc_active(struct intel_crtc *crtc); +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config); void hsw_enable_ips(const struct intel_crtc_state *crtc_state); void hsw_disable_ips(const struct intel_crtc_state *crtc_state); enum intel_display_power_domain intel_port_to_power_domain(enum port port); diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c index 61641d479b93..1f5cd572a7ff 100644 --- a/drivers/gpu/drm/i915/intel_pipe_crc.c +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c @@ -541,8 +541,6 @@ static void hsw_pipe_A_crc_wa(struct drm_i915_private *dev_priv, * completely disable it. */ pipe_config->ips_force_disable = enable; - if (pipe_config->ips_enabled == enable) - pipe_config->base.connectors_changed = true; } if (IS_HASWELL(dev_priv)) { -- 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v3. 2017-11-22 15:07 ` [PATCH] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v3 Maarten Lankhorst @ 2017-11-22 15:34 ` Ville Syrjälä 2017-11-22 18:39 ` [PATCH 1/2] " Maarten Lankhorst 0 siblings, 1 reply; 22+ messages in thread From: Ville Syrjälä @ 2017-11-22 15:34 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Wed, Nov 22, 2017 at 04:07:17PM +0100, Maarten Lankhorst wrote: > ips_enabled was used as a variable of whether IPS can be enabled or not, > but should be used to test whether IPS is actually enabled. > > Changes since v1: > - Call needs_modeset on new crtc state. (Ville) > - IPS can be enabled with sprite plane enabled too. (Ville) > - Fix CDCLK vs IPS workaround. (Ville) > Changes since v2: > - Only re-enable fastset when inheriting mode. (Ville) > - Put the conditions for enabling and disabling IPS in a helper. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_cdclk.c | 2 +- > drivers/gpu/drm/i915/intel_display.c | 151 +++++++++++++++++++++------------- > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_pipe_crc.c | 2 - > 4 files changed, 98 insertions(+), 58 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index e8884c2ade98..30affa7903d7 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -1896,7 +1896,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) > min_cdclk = intel_pixel_rate_to_cdclk(dev_priv, crtc_state->pixel_rate); > > /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ > - if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) > + if (IS_BROADWELL(dev_priv) && hsw_pipe_config_ips_capable(crtc_state)) > min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 95); > > /* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz, > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 2007c69468b9..b62cb6e90669 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -489,7 +489,7 @@ static const struct intel_limit intel_limits_bxt = { > }; > > static bool > -needs_modeset(struct drm_crtc_state *state) > +needs_modeset(const struct drm_crtc_state *state) > { > return drm_atomic_crtc_needs_modeset(state); > } > @@ -4870,7 +4870,7 @@ void hsw_enable_ips(const struct intel_crtc_state *crtc_state) > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > > - if (!crtc->config->ips_enabled) > + if (!crtc_state->ips_enabled) > return; > > /* > @@ -4966,14 +4966,6 @@ intel_post_enable_primary(struct drm_crtc *crtc, > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > int pipe = intel_crtc->pipe; > > - /* > - * FIXME IPS should be fine as long as one plane is > - * enabled, but in practice it seems to have problems > - * when going from primary only to sprite only and vice > - * versa. > - */ > - hsw_enable_ips(new_crtc_state); > - > /* > * Gen2 reports pipe underruns whenever all planes are disabled. > * So don't enable underrun reporting before at least some planes > @@ -4989,10 +4981,9 @@ intel_post_enable_primary(struct drm_crtc *crtc, > intel_check_pch_fifo_underruns(dev_priv); > } > > -/* FIXME move all this to pre_plane_update() with proper state tracking */ > +/* FIXME get rid of this and use pre_plane_update */ > static void > -intel_pre_disable_primary(struct drm_crtc *crtc, > - const struct intel_crtc_state *old_crtc_state) > +intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > @@ -5001,32 +4992,12 @@ intel_pre_disable_primary(struct drm_crtc *crtc, > > /* > * Gen2 reports pipe underruns whenever all planes are disabled. > - * So diasble underrun reporting before all the planes get disabled. > - * FIXME: Need to fix the logic to work when we turn off all planes > - * but leave the pipe running. > + * So disable underrun reporting before all the planes get disabled. > */ > if (IS_GEN2(dev_priv)) > intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); > > - /* > - * FIXME IPS should be fine as long as one plane is > - * enabled, but in practice it seems to have problems > - * when going from primary only to sprite only and vice > - * versa. > - */ > - hsw_disable_ips(old_crtc_state); > -} > - > -/* FIXME get rid of this and use pre_plane_update */ > -static void > -intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) > -{ > - struct drm_device *dev = crtc->dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - int pipe = intel_crtc->pipe; > - > - intel_pre_disable_primary(crtc, to_intel_crtc_state(crtc->state)); > + hsw_disable_ips(to_intel_crtc_state(crtc->state)); > > /* > * Vblank time updates from the shadow to live plane control register > @@ -5042,6 +5013,38 @@ intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) > intel_wait_for_vblank(dev_priv, pipe); > } > > +static bool hsw_pre_update_disable_ips(const struct intel_crtc_state *old_crtc_state, > + const struct intel_crtc_state *new_crtc_state) > +{ > + if (!old_crtc_state->ips_enabled) > + return false; > + > + if (needs_modeset(&new_crtc_state->base)) > + return true; > + > + return !new_crtc_state->ips_enabled; > +} > + > +static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_state, > + const struct intel_crtc_state *new_crtc_state) > +{ > + if (!new_crtc_state->ips_enabled) > + return false; > + > + if (needs_modeset(&new_crtc_state->base)) > + return true; > + > + /* > + * We can't read out IPS on broadwell, assume the worst and > + * forcibly enable IPS on the first fastset. > + */ > + if (new_crtc_state->update_pipe && > + old_crtc_state->base.adjusted_mode.private_flags & I915_MODE_FLAG_INHERITED) > + return true; > + > + return !old_crtc_state->ips_enabled; > +} > + > static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) > { > struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc); > @@ -5058,6 +5061,9 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) > if (pipe_config->update_wm_post && pipe_config->base.active) > intel_update_watermarks(crtc); > > + if (hsw_post_update_enable_ips(old_crtc_state, pipe_config)) > + hsw_enable_ips(pipe_config); > + > if (old_pri_state) { > struct intel_plane_state *primary_state = > intel_atomic_get_new_plane_state(to_intel_atomic_state(old_state), > @@ -5088,6 +5094,9 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > struct intel_atomic_state *old_intel_state = > to_intel_atomic_state(old_state); > > + if (hsw_pre_update_disable_ips(old_crtc_state, pipe_config)) > + hsw_disable_ips(old_crtc_state); > + > if (old_pri_state) { > struct intel_plane_state *primary_state = > intel_atomic_get_new_plane_state(old_intel_state, > @@ -5096,10 +5105,13 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > to_intel_plane_state(old_pri_state); > > intel_fbc_pre_update(crtc, pipe_config, primary_state); > - > - if (old_primary_state->base.visible && > + /* > + * Gen2 reports pipe underruns whenever all planes are disabled. > + * So disable underrun reporting before all the planes get disabled. > + */ > + if (IS_GEN2(dev_priv) && old_primary_state->base.visible && > (modeset || !primary_state->base.visible)) > - intel_pre_disable_primary(&crtc->base, old_crtc_state); > + intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); > } > > /* > @@ -6228,15 +6240,42 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc, > return ret; > } > > -static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, > - struct intel_crtc_state *pipe_config) > +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config) > { > - if (pipe_config->ips_force_disable) > + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc); > + > + /* IPS only exists on ULT machines and is tied to pipe A. */ > + if (!hsw_crtc_supports_ips(crtc)) > + return false; > + > + if (!i915_modparams.enable_ips) > return false; > > if (pipe_config->pipe_bpp > 24) > return false; > > + return true; The max_cdclk check should be here. Otherwise this looks quite nice. Maybe also re-add the ips_enabled vs. hw state assert on HSW? I guess intel_verify_planes() migth be a nice place for it, in case we ever decide to start doing these checks for pure plane updates as well... > +} > + > +static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) > +{ > + struct drm_i915_private *dev_priv = to_i915(pipe_config->base.crtc->dev); > + > + if (!hsw_pipe_config_ips_capable(pipe_config)) > + return false; > + > + if (pipe_config->ips_force_disable) > + return false; > + > + /* > + * FIXME IPS should be fine as long as one plane is > + * enabled, but in practice it seems to have problems > + * when going from primary only to sprite only and vice > + * versa. > + */ > + if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) > + return false; > + > /* HSW can handle pixel rate up to cdclk? */ > if (IS_HASWELL(dev_priv)) > return true; > @@ -6252,17 +6291,6 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, > dev_priv->max_cdclk_freq * 95 / 100; > } > > -static void hsw_compute_ips_config(struct intel_crtc *crtc, > - struct intel_crtc_state *pipe_config) > -{ > - struct drm_device *dev = crtc->base.dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - > - pipe_config->ips_enabled = i915_modparams.enable_ips && > - hsw_crtc_supports_ips(crtc) && > - pipe_config_supports_ips(dev_priv, pipe_config); > -} > - > static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc) > { > const struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > @@ -6378,9 +6406,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, > > intel_crtc_compute_pixel_rate(pipe_config); > > - if (HAS_IPS(dev_priv)) > - hsw_compute_ips_config(crtc, pipe_config); > - > if (pipe_config->has_pch_encoder) > return ironlake_fdi_compute_config(crtc, pipe_config); > > @@ -9275,6 +9300,19 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, > ironlake_get_pfit_config(crtc, pipe_config); > } > > + if (hsw_crtc_supports_ips(crtc)) { > + if (IS_HASWELL(dev_priv)) > + pipe_config->ips_enabled = I915_READ(IPS_CTL) & IPS_ENABLE; > + else { > + /* > + * We cannot readout IPS state on broadwell, set to > + * true so we can set it to a defined state on first > + * commit. > + */ > + pipe_config->ips_enabled = true; > + } > + } > + > if (pipe_config->cpu_transcoder != TRANSCODER_EDP && > !transcoder_is_dsi(pipe_config->cpu_transcoder)) { > pipe_config->pixel_multiplier = > @@ -10489,6 +10527,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > pipe_config); > } > > + if (HAS_IPS(dev_priv)) > + pipe_config->ips_enabled = hsw_compute_ips_config(pipe_config); > + > return ret; > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 69aab324aaa1..40417b20844e 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1485,6 +1485,7 @@ bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock, > int chv_calc_dpll_params(int refclk, struct dpll *pll_clock); > > bool intel_crtc_active(struct intel_crtc *crtc); > +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config); > void hsw_enable_ips(const struct intel_crtc_state *crtc_state); > void hsw_disable_ips(const struct intel_crtc_state *crtc_state); > enum intel_display_power_domain intel_port_to_power_domain(enum port port); > diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c > index 61641d479b93..1f5cd572a7ff 100644 > --- a/drivers/gpu/drm/i915/intel_pipe_crc.c > +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c > @@ -541,8 +541,6 @@ static void hsw_pipe_A_crc_wa(struct drm_i915_private *dev_priv, > * completely disable it. > */ > pipe_config->ips_force_disable = enable; > - if (pipe_config->ips_enabled == enable) > - pipe_config->base.connectors_changed = true; > } > > if (IS_HASWELL(dev_priv)) { > -- > 2.15.0 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/2] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v3. 2017-11-22 15:34 ` Ville Syrjälä @ 2017-11-22 18:39 ` Maarten Lankhorst 2017-11-23 14:36 ` Ville Syrjälä 0 siblings, 1 reply; 22+ messages in thread From: Maarten Lankhorst @ 2017-11-22 18:39 UTC (permalink / raw) To: intel-gfx ips_enabled was used as a variable of whether IPS can be enabled or not, but should be used to test whether IPS is actually enabled. Changes since v1: - Call needs_modeset on new crtc state. (Ville) - IPS can be enabled with sprite plane enabled too. (Ville) - Fix CDCLK vs IPS workaround. (Ville) Changes since v2: - Only re-enable fastset when inheriting mode. (Ville) - Put the conditions for enabling and disabling IPS in a helper. Changes since v3: - Keep the max_cdclk workaround working. (Ville) - Also check logical cdclk out of paranoia. - Remove planes check from IPS disable function for initial disable. - Remove assert_plane_enabled/disabled checks and use crtc_state->active_planes for hsw_enable_ips only, always allow calling hsw_disable_ips to disable it initially in hw. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_cdclk.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 168 ++++++++++++++++++++++------------ drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_pipe_crc.c | 2 - 4 files changed, 109 insertions(+), 64 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index e8884c2ade98..30affa7903d7 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -1896,7 +1896,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) min_cdclk = intel_pixel_rate_to_cdclk(dev_priv, crtc_state->pixel_rate); /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ - if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) + if (IS_BROADWELL(dev_priv) && hsw_pipe_config_ips_capable(crtc_state)) min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 95); /* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 60d0a2d8534c..95df5c2128b4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -489,7 +489,7 @@ static const struct intel_limit intel_limits_bxt = { }; static bool -needs_modeset(struct drm_crtc_state *state) +needs_modeset(const struct drm_crtc_state *state) { return drm_atomic_crtc_needs_modeset(state); } @@ -4833,7 +4833,7 @@ void hsw_enable_ips(const struct intel_crtc_state *crtc_state) struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); - if (!crtc->config->ips_enabled) + if (!crtc_state->ips_enabled) return; /* @@ -4841,8 +4841,7 @@ void hsw_enable_ips(const struct intel_crtc_state *crtc_state) * This function is called from post_plane_update, which is run after * a vblank wait. */ - - assert_plane_enabled(to_intel_plane(crtc->base.primary)); + WARN_ON(!(crtc_state->active_planes & ~BIT(PLANE_CURSOR))); if (IS_BROADWELL(dev_priv)) { mutex_lock(&dev_priv->pcu_lock); @@ -4877,8 +4876,6 @@ void hsw_disable_ips(const struct intel_crtc_state *crtc_state) if (!crtc_state->ips_enabled) return; - assert_plane_enabled(to_intel_plane(crtc->base.primary)); - if (IS_BROADWELL(dev_priv)) { mutex_lock(&dev_priv->pcu_lock); WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0)); @@ -4931,14 +4928,6 @@ intel_post_enable_primary(struct drm_crtc *crtc, struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int pipe = intel_crtc->pipe; - /* - * FIXME IPS should be fine as long as one plane is - * enabled, but in practice it seems to have problems - * when going from primary only to sprite only and vice - * versa. - */ - hsw_enable_ips(new_crtc_state); - /* * Gen2 reports pipe underruns whenever all planes are disabled. * So don't enable underrun reporting before at least some planes @@ -4954,10 +4943,9 @@ intel_post_enable_primary(struct drm_crtc *crtc, intel_check_pch_fifo_underruns(dev_priv); } -/* FIXME move all this to pre_plane_update() with proper state tracking */ +/* FIXME get rid of this and use pre_plane_update */ static void -intel_pre_disable_primary(struct drm_crtc *crtc, - const struct intel_crtc_state *old_crtc_state) +intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = to_i915(dev); @@ -4966,32 +4954,12 @@ intel_pre_disable_primary(struct drm_crtc *crtc, /* * Gen2 reports pipe underruns whenever all planes are disabled. - * So diasble underrun reporting before all the planes get disabled. - * FIXME: Need to fix the logic to work when we turn off all planes - * but leave the pipe running. + * So disable underrun reporting before all the planes get disabled. */ if (IS_GEN2(dev_priv)) intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); - /* - * FIXME IPS should be fine as long as one plane is - * enabled, but in practice it seems to have problems - * when going from primary only to sprite only and vice - * versa. - */ - hsw_disable_ips(old_crtc_state); -} - -/* FIXME get rid of this and use pre_plane_update */ -static void -intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) -{ - struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = to_i915(dev); - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int pipe = intel_crtc->pipe; - - intel_pre_disable_primary(crtc, to_intel_crtc_state(crtc->state)); + hsw_disable_ips(to_intel_crtc_state(crtc->state)); /* * Vblank time updates from the shadow to live plane control register @@ -5007,6 +4975,38 @@ intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) intel_wait_for_vblank(dev_priv, pipe); } +static bool hsw_pre_update_disable_ips(const struct intel_crtc_state *old_crtc_state, + const struct intel_crtc_state *new_crtc_state) +{ + if (!old_crtc_state->ips_enabled) + return false; + + if (needs_modeset(&new_crtc_state->base)) + return true; + + return !new_crtc_state->ips_enabled; +} + +static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_state, + const struct intel_crtc_state *new_crtc_state) +{ + if (!new_crtc_state->ips_enabled) + return false; + + if (needs_modeset(&new_crtc_state->base)) + return true; + + /* + * We can't read out IPS on broadwell, assume the worst and + * forcibly enable IPS on the first fastset. + */ + if (new_crtc_state->update_pipe && + old_crtc_state->base.adjusted_mode.private_flags & I915_MODE_FLAG_INHERITED) + return true; + + return !old_crtc_state->ips_enabled; +} + static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) { struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc); @@ -5023,6 +5023,9 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) if (pipe_config->update_wm_post && pipe_config->base.active) intel_update_watermarks(crtc); + if (hsw_post_update_enable_ips(old_crtc_state, pipe_config)) + hsw_enable_ips(pipe_config); + if (old_pri_state) { struct intel_plane_state *primary_state = intel_atomic_get_new_plane_state(to_intel_atomic_state(old_state), @@ -5053,6 +5056,9 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, struct intel_atomic_state *old_intel_state = to_intel_atomic_state(old_state); + if (hsw_pre_update_disable_ips(old_crtc_state, pipe_config)) + hsw_disable_ips(old_crtc_state); + if (old_pri_state) { struct intel_plane_state *primary_state = intel_atomic_get_new_plane_state(old_intel_state, @@ -5061,10 +5067,13 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, to_intel_plane_state(old_pri_state); intel_fbc_pre_update(crtc, pipe_config, primary_state); - - if (old_primary_state->base.visible && + /* + * Gen2 reports pipe underruns whenever all planes are disabled. + * So disable underrun reporting before all the planes get disabled. + */ + if (IS_GEN2(dev_priv) && old_primary_state->base.visible && (modeset || !primary_state->base.visible)) - intel_pre_disable_primary(&crtc->base, old_crtc_state); + intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); } /* @@ -6195,18 +6204,20 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc, return ret; } -static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, - struct intel_crtc_state *pipe_config) +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config) { - if (pipe_config->ips_force_disable) + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc); + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + + /* IPS only exists on ULT machines and is tied to pipe A. */ + if (!hsw_crtc_supports_ips(crtc)) return false; - if (pipe_config->pipe_bpp > 24) + if (!i915_modparams.enable_ips) return false; - /* HSW can handle pixel rate up to cdclk? */ - if (IS_HASWELL(dev_priv)) - return true; + if (pipe_config->pipe_bpp > 24) + return false; /* * We compare against max which means we must take @@ -6215,19 +6226,41 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, * * Should measure whether using a lower cdclk w/o IPS */ - return pipe_config->pixel_rate <= - dev_priv->max_cdclk_freq * 95 / 100; + if (IS_BROADWELL(dev_priv) && pipe_config->pixel_rate > + dev_priv->max_cdclk_freq * 95 / 100) + return false; + + return true; } -static void hsw_compute_ips_config(struct intel_crtc *crtc, - struct intel_crtc_state *pipe_config) +static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) { - struct drm_device *dev = crtc->base.dev; - struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_i915_private *dev_priv = + to_i915(pipe_config->base.crtc->dev); + struct intel_atomic_state *intel_state = + to_intel_atomic_state(pipe_config->base.state); + + if (!hsw_pipe_config_ips_capable(pipe_config)) + return false; + + if (pipe_config->ips_force_disable) + return false; + + /* + * FIXME IPS should be fine as long as one plane is + * enabled, but in practice it seems to have problems + * when going from primary only to sprite only and vice + * versa. + */ + if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) + return false; - pipe_config->ips_enabled = i915_modparams.enable_ips && - hsw_crtc_supports_ips(crtc) && - pipe_config_supports_ips(dev_priv, pipe_config); + /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ + if (IS_BROADWELL(dev_priv) && pipe_config->pixel_rate > + intel_state->cdclk.logical.cdclk * 95 / 100) + return false; + + return true; } static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc) @@ -6345,9 +6378,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, intel_crtc_compute_pixel_rate(pipe_config); - if (HAS_IPS(dev_priv)) - hsw_compute_ips_config(crtc, pipe_config); - if (pipe_config->has_pch_encoder) return ironlake_fdi_compute_config(crtc, pipe_config); @@ -9183,6 +9213,19 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, ironlake_get_pfit_config(crtc, pipe_config); } + if (hsw_crtc_supports_ips(crtc)) { + if (IS_HASWELL(dev_priv)) + pipe_config->ips_enabled = I915_READ(IPS_CTL) & IPS_ENABLE; + else { + /* + * We cannot readout IPS state on broadwell, set to + * true so we can set it to a defined state on first + * commit. + */ + pipe_config->ips_enabled = true; + } + } + if (pipe_config->cpu_transcoder != TRANSCODER_EDP && !transcoder_is_dsi(pipe_config->cpu_transcoder)) { pipe_config->pixel_multiplier = @@ -10436,6 +10479,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, pipe_config); } + if (HAS_IPS(dev_priv)) + pipe_config->ips_enabled = hsw_compute_ips_config(pipe_config); + return ret; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 635a96fcd788..4f4cf5128e0c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1485,6 +1485,7 @@ bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock, int chv_calc_dpll_params(int refclk, struct dpll *pll_clock); bool intel_crtc_active(struct intel_crtc *crtc); +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config); void hsw_enable_ips(const struct intel_crtc_state *crtc_state); void hsw_disable_ips(const struct intel_crtc_state *crtc_state); enum intel_display_power_domain intel_port_to_power_domain(enum port port); diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c index 61641d479b93..1f5cd572a7ff 100644 --- a/drivers/gpu/drm/i915/intel_pipe_crc.c +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c @@ -541,8 +541,6 @@ static void hsw_pipe_A_crc_wa(struct drm_i915_private *dev_priv, * completely disable it. */ pipe_config->ips_force_disable = enable; - if (pipe_config->ips_enabled == enable) - pipe_config->base.connectors_changed = true; } if (IS_HASWELL(dev_priv)) { -- 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v3. 2017-11-22 18:39 ` [PATCH 1/2] " Maarten Lankhorst @ 2017-11-23 14:36 ` Ville Syrjälä 2017-11-30 15:54 ` Maarten Lankhorst 0 siblings, 1 reply; 22+ messages in thread From: Ville Syrjälä @ 2017-11-23 14:36 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Wed, Nov 22, 2017 at 07:39:01PM +0100, Maarten Lankhorst wrote: > ips_enabled was used as a variable of whether IPS can be enabled or not, > but should be used to test whether IPS is actually enabled. > > Changes since v1: > - Call needs_modeset on new crtc state. (Ville) > - IPS can be enabled with sprite plane enabled too. (Ville) > - Fix CDCLK vs IPS workaround. (Ville) > Changes since v2: > - Only re-enable fastset when inheriting mode. (Ville) > - Put the conditions for enabling and disabling IPS in a helper. > Changes since v3: > - Keep the max_cdclk workaround working. (Ville) > - Also check logical cdclk out of paranoia. > - Remove planes check from IPS disable function for initial disable. > - Remove assert_plane_enabled/disabled checks and use > crtc_state->active_planes for hsw_enable_ips only, always allow > calling hsw_disable_ips to disable it initially in hw. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_cdclk.c | 2 +- > drivers/gpu/drm/i915/intel_display.c | 168 ++++++++++++++++++++++------------ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_pipe_crc.c | 2 - > 4 files changed, 109 insertions(+), 64 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index e8884c2ade98..30affa7903d7 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -1896,7 +1896,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) > min_cdclk = intel_pixel_rate_to_cdclk(dev_priv, crtc_state->pixel_rate); > > /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ > - if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) > + if (IS_BROADWELL(dev_priv) && hsw_pipe_config_ips_capable(crtc_state)) > min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 95); > > /* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz, > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 60d0a2d8534c..95df5c2128b4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -489,7 +489,7 @@ static const struct intel_limit intel_limits_bxt = { > }; > > static bool > -needs_modeset(struct drm_crtc_state *state) > +needs_modeset(const struct drm_crtc_state *state) > { > return drm_atomic_crtc_needs_modeset(state); > } > @@ -4833,7 +4833,7 @@ void hsw_enable_ips(const struct intel_crtc_state *crtc_state) > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > > - if (!crtc->config->ips_enabled) > + if (!crtc_state->ips_enabled) > return; > > /* > @@ -4841,8 +4841,7 @@ void hsw_enable_ips(const struct intel_crtc_state *crtc_state) > * This function is called from post_plane_update, which is run after > * a vblank wait. > */ > - > - assert_plane_enabled(to_intel_plane(crtc->base.primary)); > + WARN_ON(!(crtc_state->active_planes & ~BIT(PLANE_CURSOR))); > > if (IS_BROADWELL(dev_priv)) { > mutex_lock(&dev_priv->pcu_lock); > @@ -4877,8 +4876,6 @@ void hsw_disable_ips(const struct intel_crtc_state *crtc_state) > if (!crtc_state->ips_enabled) > return; > > - assert_plane_enabled(to_intel_plane(crtc->base.primary)); > - > if (IS_BROADWELL(dev_priv)) { > mutex_lock(&dev_priv->pcu_lock); > WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0)); > @@ -4931,14 +4928,6 @@ intel_post_enable_primary(struct drm_crtc *crtc, > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > int pipe = intel_crtc->pipe; > > - /* > - * FIXME IPS should be fine as long as one plane is > - * enabled, but in practice it seems to have problems > - * when going from primary only to sprite only and vice > - * versa. > - */ > - hsw_enable_ips(new_crtc_state); > - > /* > * Gen2 reports pipe underruns whenever all planes are disabled. > * So don't enable underrun reporting before at least some planes > @@ -4954,10 +4943,9 @@ intel_post_enable_primary(struct drm_crtc *crtc, > intel_check_pch_fifo_underruns(dev_priv); > } > > -/* FIXME move all this to pre_plane_update() with proper state tracking */ > +/* FIXME get rid of this and use pre_plane_update */ > static void > -intel_pre_disable_primary(struct drm_crtc *crtc, > - const struct intel_crtc_state *old_crtc_state) > +intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > @@ -4966,32 +4954,12 @@ intel_pre_disable_primary(struct drm_crtc *crtc, > > /* > * Gen2 reports pipe underruns whenever all planes are disabled. > - * So diasble underrun reporting before all the planes get disabled. > - * FIXME: Need to fix the logic to work when we turn off all planes > - * but leave the pipe running. > + * So disable underrun reporting before all the planes get disabled. > */ > if (IS_GEN2(dev_priv)) > intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); > > - /* > - * FIXME IPS should be fine as long as one plane is > - * enabled, but in practice it seems to have problems > - * when going from primary only to sprite only and vice > - * versa. > - */ > - hsw_disable_ips(old_crtc_state); > -} > - > -/* FIXME get rid of this and use pre_plane_update */ > -static void > -intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) > -{ > - struct drm_device *dev = crtc->dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - int pipe = intel_crtc->pipe; > - > - intel_pre_disable_primary(crtc, to_intel_crtc_state(crtc->state)); > + hsw_disable_ips(to_intel_crtc_state(crtc->state)); > > /* > * Vblank time updates from the shadow to live plane control register > @@ -5007,6 +4975,38 @@ intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) > intel_wait_for_vblank(dev_priv, pipe); > } > > +static bool hsw_pre_update_disable_ips(const struct intel_crtc_state *old_crtc_state, > + const struct intel_crtc_state *new_crtc_state) > +{ > + if (!old_crtc_state->ips_enabled) > + return false; > + > + if (needs_modeset(&new_crtc_state->base)) > + return true; > + > + return !new_crtc_state->ips_enabled; > +} > + > +static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_state, > + const struct intel_crtc_state *new_crtc_state) > +{ > + if (!new_crtc_state->ips_enabled) > + return false; > + > + if (needs_modeset(&new_crtc_state->base)) > + return true; > + > + /* > + * We can't read out IPS on broadwell, assume the worst and > + * forcibly enable IPS on the first fastset. > + */ > + if (new_crtc_state->update_pipe && Do we even need the update_pipe check now? Ie. why would we skip this if the initial update happens to update just the planes? > + old_crtc_state->base.adjusted_mode.private_flags & I915_MODE_FLAG_INHERITED) > + return true; > + > + return !old_crtc_state->ips_enabled; > +} > + > static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) > { > struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc); > @@ -5023,6 +5023,9 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) > if (pipe_config->update_wm_post && pipe_config->base.active) > intel_update_watermarks(crtc); > > + if (hsw_post_update_enable_ips(old_crtc_state, pipe_config)) > + hsw_enable_ips(pipe_config); > + > if (old_pri_state) { > struct intel_plane_state *primary_state = > intel_atomic_get_new_plane_state(to_intel_atomic_state(old_state), > @@ -5053,6 +5056,9 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > struct intel_atomic_state *old_intel_state = > to_intel_atomic_state(old_state); > > + if (hsw_pre_update_disable_ips(old_crtc_state, pipe_config)) > + hsw_disable_ips(old_crtc_state); > + > if (old_pri_state) { > struct intel_plane_state *primary_state = > intel_atomic_get_new_plane_state(old_intel_state, > @@ -5061,10 +5067,13 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, > to_intel_plane_state(old_pri_state); > > intel_fbc_pre_update(crtc, pipe_config, primary_state); > - > - if (old_primary_state->base.visible && > + /* > + * Gen2 reports pipe underruns whenever all planes are disabled. > + * So disable underrun reporting before all the planes get disabled. > + */ > + if (IS_GEN2(dev_priv) && old_primary_state->base.visible && > (modeset || !primary_state->base.visible)) > - intel_pre_disable_primary(&crtc->base, old_crtc_state); > + intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); > } > > /* > @@ -6195,18 +6204,20 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc, > return ret; > } > > -static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, > - struct intel_crtc_state *pipe_config) > +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config) Since you're touching pretty much everything, how about s/pipe_config/crtc_state/ while at it? > { > - if (pipe_config->ips_force_disable) > + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc); > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > + > + /* IPS only exists on ULT machines and is tied to pipe A. */ > + if (!hsw_crtc_supports_ips(crtc)) > return false; > > - if (pipe_config->pipe_bpp > 24) > + if (!i915_modparams.enable_ips) > return false; > > - /* HSW can handle pixel rate up to cdclk? */ > - if (IS_HASWELL(dev_priv)) > - return true; > + if (pipe_config->pipe_bpp > 24) > + return false; > > /* > * We compare against max which means we must take > @@ -6215,19 +6226,41 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, > * > * Should measure whether using a lower cdclk w/o IPS > */ > - return pipe_config->pixel_rate <= > - dev_priv->max_cdclk_freq * 95 / 100; > + if (IS_BROADWELL(dev_priv) && pipe_config->pixel_rate > > + dev_priv->max_cdclk_freq * 95 / 100) Splitting the line like that makes it pretty hard to read. if (IS_BROADWELL(...) && pipe_config->pixel_rate > dev_priv->max_cdclk_freq * 95 / 100) would look much better IMO. > + return false; > + > + return true; > } > > -static void hsw_compute_ips_config(struct intel_crtc *crtc, > - struct intel_crtc_state *pipe_config) > +static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) s/pipe_config/crtc_state/ again > { > - struct drm_device *dev = crtc->base.dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > + struct drm_i915_private *dev_priv = > + to_i915(pipe_config->base.crtc->dev); > + struct intel_atomic_state *intel_state = > + to_intel_atomic_state(pipe_config->base.state); > + > + if (!hsw_pipe_config_ips_capable(pipe_config)) > + return false; > + > + if (pipe_config->ips_force_disable) > + return false; > + > + /* > + * FIXME IPS should be fine as long as one plane is > + * enabled, but in practice it seems to have problems > + * when going from primary only to sprite only and vice > + * versa. > + */ > + if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) > + return false; > > - pipe_config->ips_enabled = i915_modparams.enable_ips && > - hsw_crtc_supports_ips(crtc) && > - pipe_config_supports_ips(dev_priv, pipe_config); > + /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ > + if (IS_BROADWELL(dev_priv) && pipe_config->pixel_rate > > + intel_state->cdclk.logical.cdclk * 95 / 100) > + return false; Another hard to read split. Apart from those nits, this lgtm so Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > + > + return true; > } > > static bool intel_crtc_supports_double_wide(const struct intel_crtc *crtc) > @@ -6345,9 +6378,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc, > > intel_crtc_compute_pixel_rate(pipe_config); > > - if (HAS_IPS(dev_priv)) > - hsw_compute_ips_config(crtc, pipe_config); > - > if (pipe_config->has_pch_encoder) > return ironlake_fdi_compute_config(crtc, pipe_config); > > @@ -9183,6 +9213,19 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, > ironlake_get_pfit_config(crtc, pipe_config); > } > > + if (hsw_crtc_supports_ips(crtc)) { > + if (IS_HASWELL(dev_priv)) > + pipe_config->ips_enabled = I915_READ(IPS_CTL) & IPS_ENABLE; > + else { > + /* > + * We cannot readout IPS state on broadwell, set to > + * true so we can set it to a defined state on first > + * commit. > + */ > + pipe_config->ips_enabled = true; > + } > + } > + > if (pipe_config->cpu_transcoder != TRANSCODER_EDP && > !transcoder_is_dsi(pipe_config->cpu_transcoder)) { > pipe_config->pixel_multiplier = > @@ -10436,6 +10479,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > pipe_config); > } > > + if (HAS_IPS(dev_priv)) > + pipe_config->ips_enabled = hsw_compute_ips_config(pipe_config); > + > return ret; > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 635a96fcd788..4f4cf5128e0c 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1485,6 +1485,7 @@ bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock, > int chv_calc_dpll_params(int refclk, struct dpll *pll_clock); > > bool intel_crtc_active(struct intel_crtc *crtc); > +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config); > void hsw_enable_ips(const struct intel_crtc_state *crtc_state); > void hsw_disable_ips(const struct intel_crtc_state *crtc_state); > enum intel_display_power_domain intel_port_to_power_domain(enum port port); > diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c > index 61641d479b93..1f5cd572a7ff 100644 > --- a/drivers/gpu/drm/i915/intel_pipe_crc.c > +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c > @@ -541,8 +541,6 @@ static void hsw_pipe_A_crc_wa(struct drm_i915_private *dev_priv, > * completely disable it. > */ > pipe_config->ips_force_disable = enable; > - if (pipe_config->ips_enabled == enable) > - pipe_config->base.connectors_changed = true; > } > > if (IS_HASWELL(dev_priv)) { > -- > 2.15.0 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v3. 2017-11-23 14:36 ` Ville Syrjälä @ 2017-11-30 15:54 ` Maarten Lankhorst 0 siblings, 0 replies; 22+ messages in thread From: Maarten Lankhorst @ 2017-11-30 15:54 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx Op 23-11-17 om 15:36 schreef Ville Syrjälä: > On Wed, Nov 22, 2017 at 07:39:01PM +0100, Maarten Lankhorst wrote: >> ips_enabled was used as a variable of whether IPS can be enabled or not, >> but should be used to test whether IPS is actually enabled. >> >> Changes since v1: >> - Call needs_modeset on new crtc state. (Ville) >> - IPS can be enabled with sprite plane enabled too. (Ville) >> - Fix CDCLK vs IPS workaround. (Ville) >> Changes since v2: >> - Only re-enable fastset when inheriting mode. (Ville) >> - Put the conditions for enabling and disabling IPS in a helper. >> Changes since v3: >> - Keep the max_cdclk workaround working. (Ville) >> - Also check logical cdclk out of paranoia. >> - Remove planes check from IPS disable function for initial disable. >> - Remove assert_plane_enabled/disabled checks and use >> crtc_state->active_planes for hsw_enable_ips only, always allow >> calling hsw_disable_ips to disable it initially in hw. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_cdclk.c | 2 +- >> drivers/gpu/drm/i915/intel_display.c | 168 ++++++++++++++++++++++------------ >> drivers/gpu/drm/i915/intel_drv.h | 1 + >> drivers/gpu/drm/i915/intel_pipe_crc.c | 2 - >> 4 files changed, 109 insertions(+), 64 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c >> index e8884c2ade98..30affa7903d7 100644 >> --- a/drivers/gpu/drm/i915/intel_cdclk.c >> +++ b/drivers/gpu/drm/i915/intel_cdclk.c >> @@ -1896,7 +1896,7 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state) >> min_cdclk = intel_pixel_rate_to_cdclk(dev_priv, crtc_state->pixel_rate); >> >> /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ >> - if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) >> + if (IS_BROADWELL(dev_priv) && hsw_pipe_config_ips_capable(crtc_state)) >> min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 95); >> >> /* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz, >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 60d0a2d8534c..95df5c2128b4 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -489,7 +489,7 @@ static const struct intel_limit intel_limits_bxt = { >> }; >> >> static bool >> -needs_modeset(struct drm_crtc_state *state) >> +needs_modeset(const struct drm_crtc_state *state) >> { >> return drm_atomic_crtc_needs_modeset(state); >> } >> @@ -4833,7 +4833,7 @@ void hsw_enable_ips(const struct intel_crtc_state *crtc_state) >> struct drm_device *dev = crtc->base.dev; >> struct drm_i915_private *dev_priv = to_i915(dev); >> >> - if (!crtc->config->ips_enabled) >> + if (!crtc_state->ips_enabled) >> return; >> >> /* >> @@ -4841,8 +4841,7 @@ void hsw_enable_ips(const struct intel_crtc_state *crtc_state) >> * This function is called from post_plane_update, which is run after >> * a vblank wait. >> */ >> - >> - assert_plane_enabled(to_intel_plane(crtc->base.primary)); >> + WARN_ON(!(crtc_state->active_planes & ~BIT(PLANE_CURSOR))); >> >> if (IS_BROADWELL(dev_priv)) { >> mutex_lock(&dev_priv->pcu_lock); >> @@ -4877,8 +4876,6 @@ void hsw_disable_ips(const struct intel_crtc_state *crtc_state) >> if (!crtc_state->ips_enabled) >> return; >> >> - assert_plane_enabled(to_intel_plane(crtc->base.primary)); >> - >> if (IS_BROADWELL(dev_priv)) { >> mutex_lock(&dev_priv->pcu_lock); >> WARN_ON(sandybridge_pcode_write(dev_priv, DISPLAY_IPS_CONTROL, 0)); >> @@ -4931,14 +4928,6 @@ intel_post_enable_primary(struct drm_crtc *crtc, >> struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> int pipe = intel_crtc->pipe; >> >> - /* >> - * FIXME IPS should be fine as long as one plane is >> - * enabled, but in practice it seems to have problems >> - * when going from primary only to sprite only and vice >> - * versa. >> - */ >> - hsw_enable_ips(new_crtc_state); >> - >> /* >> * Gen2 reports pipe underruns whenever all planes are disabled. >> * So don't enable underrun reporting before at least some planes >> @@ -4954,10 +4943,9 @@ intel_post_enable_primary(struct drm_crtc *crtc, >> intel_check_pch_fifo_underruns(dev_priv); >> } >> >> -/* FIXME move all this to pre_plane_update() with proper state tracking */ >> +/* FIXME get rid of this and use pre_plane_update */ >> static void >> -intel_pre_disable_primary(struct drm_crtc *crtc, >> - const struct intel_crtc_state *old_crtc_state) >> +intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) >> { >> struct drm_device *dev = crtc->dev; >> struct drm_i915_private *dev_priv = to_i915(dev); >> @@ -4966,32 +4954,12 @@ intel_pre_disable_primary(struct drm_crtc *crtc, >> >> /* >> * Gen2 reports pipe underruns whenever all planes are disabled. >> - * So diasble underrun reporting before all the planes get disabled. >> - * FIXME: Need to fix the logic to work when we turn off all planes >> - * but leave the pipe running. >> + * So disable underrun reporting before all the planes get disabled. >> */ >> if (IS_GEN2(dev_priv)) >> intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false); >> >> - /* >> - * FIXME IPS should be fine as long as one plane is >> - * enabled, but in practice it seems to have problems >> - * when going from primary only to sprite only and vice >> - * versa. >> - */ >> - hsw_disable_ips(old_crtc_state); >> -} >> - >> -/* FIXME get rid of this and use pre_plane_update */ >> -static void >> -intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) >> -{ >> - struct drm_device *dev = crtc->dev; >> - struct drm_i915_private *dev_priv = to_i915(dev); >> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> - int pipe = intel_crtc->pipe; >> - >> - intel_pre_disable_primary(crtc, to_intel_crtc_state(crtc->state)); >> + hsw_disable_ips(to_intel_crtc_state(crtc->state)); >> >> /* >> * Vblank time updates from the shadow to live plane control register >> @@ -5007,6 +4975,38 @@ intel_pre_disable_primary_noatomic(struct drm_crtc *crtc) >> intel_wait_for_vblank(dev_priv, pipe); >> } >> >> +static bool hsw_pre_update_disable_ips(const struct intel_crtc_state *old_crtc_state, >> + const struct intel_crtc_state *new_crtc_state) >> +{ >> + if (!old_crtc_state->ips_enabled) >> + return false; >> + >> + if (needs_modeset(&new_crtc_state->base)) >> + return true; >> + >> + return !new_crtc_state->ips_enabled; >> +} >> + >> +static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_state, >> + const struct intel_crtc_state *new_crtc_state) >> +{ >> + if (!new_crtc_state->ips_enabled) >> + return false; >> + >> + if (needs_modeset(&new_crtc_state->base)) >> + return true; >> + >> + /* >> + * We can't read out IPS on broadwell, assume the worst and >> + * forcibly enable IPS on the first fastset. >> + */ >> + if (new_crtc_state->update_pipe && > Do we even need the update_pipe check now? Ie. why would we skip this if the > initial update happens to update just the planes? The first update may update planes, but that keeps the I915_MODE_FLAG_INHERITED flag, we can either choose to keep updating IPS until first update_pipe is called, or only call ips_enabled after first update_pipe. >> + old_crtc_state->base.adjusted_mode.private_flags & I915_MODE_FLAG_INHERITED) >> + return true; >> + >> + return !old_crtc_state->ips_enabled; >> +} >> + >> static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) >> { >> struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc); >> @@ -5023,6 +5023,9 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) >> if (pipe_config->update_wm_post && pipe_config->base.active) >> intel_update_watermarks(crtc); >> >> + if (hsw_post_update_enable_ips(old_crtc_state, pipe_config)) >> + hsw_enable_ips(pipe_config); >> + >> if (old_pri_state) { >> struct intel_plane_state *primary_state = >> intel_atomic_get_new_plane_state(to_intel_atomic_state(old_state), >> @@ -5053,6 +5056,9 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, >> struct intel_atomic_state *old_intel_state = >> to_intel_atomic_state(old_state); >> >> + if (hsw_pre_update_disable_ips(old_crtc_state, pipe_config)) >> + hsw_disable_ips(old_crtc_state); >> + >> if (old_pri_state) { >> struct intel_plane_state *primary_state = >> intel_atomic_get_new_plane_state(old_intel_state, >> @@ -5061,10 +5067,13 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state, >> to_intel_plane_state(old_pri_state); >> >> intel_fbc_pre_update(crtc, pipe_config, primary_state); >> - >> - if (old_primary_state->base.visible && >> + /* >> + * Gen2 reports pipe underruns whenever all planes are disabled. >> + * So disable underrun reporting before all the planes get disabled. >> + */ >> + if (IS_GEN2(dev_priv) && old_primary_state->base.visible && >> (modeset || !primary_state->base.visible)) >> - intel_pre_disable_primary(&crtc->base, old_crtc_state); >> + intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); >> } >> >> /* >> @@ -6195,18 +6204,20 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc, >> return ret; >> } >> >> -static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, >> - struct intel_crtc_state *pipe_config) >> +bool hsw_pipe_config_ips_capable(const struct intel_crtc_state *pipe_config) > Since you're touching pretty much everything, how about > s/pipe_config/crtc_state/ while at it? > >> { >> - if (pipe_config->ips_force_disable) >> + struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc); >> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >> + >> + /* IPS only exists on ULT machines and is tied to pipe A. */ >> + if (!hsw_crtc_supports_ips(crtc)) >> return false; >> >> - if (pipe_config->pipe_bpp > 24) >> + if (!i915_modparams.enable_ips) >> return false; >> >> - /* HSW can handle pixel rate up to cdclk? */ >> - if (IS_HASWELL(dev_priv)) >> - return true; >> + if (pipe_config->pipe_bpp > 24) >> + return false; >> >> /* >> * We compare against max which means we must take >> @@ -6215,19 +6226,41 @@ static bool pipe_config_supports_ips(struct drm_i915_private *dev_priv, >> * >> * Should measure whether using a lower cdclk w/o IPS >> */ >> - return pipe_config->pixel_rate <= >> - dev_priv->max_cdclk_freq * 95 / 100; >> + if (IS_BROADWELL(dev_priv) && pipe_config->pixel_rate > >> + dev_priv->max_cdclk_freq * 95 / 100) > Splitting the line like that makes it pretty hard to read. > > if (IS_BROADWELL(...) && > pipe_config->pixel_rate > dev_priv->max_cdclk_freq * 95 / 100) > > would look much better IMO. > >> + return false; >> + >> + return true; >> } >> >> -static void hsw_compute_ips_config(struct intel_crtc *crtc, >> - struct intel_crtc_state *pipe_config) >> +static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) > s/pipe_config/crtc_state/ again > >> { >> - struct drm_device *dev = crtc->base.dev; >> - struct drm_i915_private *dev_priv = to_i915(dev); >> + struct drm_i915_private *dev_priv = >> + to_i915(pipe_config->base.crtc->dev); >> + struct intel_atomic_state *intel_state = >> + to_intel_atomic_state(pipe_config->base.state); >> + >> + if (!hsw_pipe_config_ips_capable(pipe_config)) >> + return false; >> + >> + if (pipe_config->ips_force_disable) >> + return false; >> + >> + /* >> + * FIXME IPS should be fine as long as one plane is >> + * enabled, but in practice it seems to have problems >> + * when going from primary only to sprite only and vice >> + * versa. >> + */ >> + if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) >> + return false; >> >> - pipe_config->ips_enabled = i915_modparams.enable_ips && >> - hsw_crtc_supports_ips(crtc) && >> - pipe_config_supports_ips(dev_priv, pipe_config); >> + /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ >> + if (IS_BROADWELL(dev_priv) && pipe_config->pixel_rate > >> + intel_state->cdclk.logical.cdclk * 95 / 100) >> + return false; > Another hard to read split. > > Apart from those nits, this lgtm so > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Thanks, moved the split and did s/pipe_config/crtc_state/ (including on function name). Pushed. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 2/3] drm/i915: Enable IPS with only sprite plane visible too, v2. 2017-11-20 12:23 [PATCH v2 0/3] drm/i915: Enable fastboot, v3! Maarten Lankhorst 2017-11-20 12:23 ` [PATCH v2 1/3] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v2 Maarten Lankhorst @ 2017-11-20 12:23 ` Maarten Lankhorst 2017-11-20 13:27 ` Ville Syrjälä 2017-11-20 12:23 ` [PATCH (resend) 3/3] drm/i915: Re-enable fastboot by default Maarten Lankhorst ` (5 subsequent siblings) 7 siblings, 1 reply; 22+ messages in thread From: Maarten Lankhorst @ 2017-11-20 12:23 UTC (permalink / raw) To: intel-gfx This comment predates atomic, and I think with the way we currently track IPS, it's safe to enable this for the case we switch too. Changes since v1: - Keep IPS enabled when switching planes. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 0b9ba415839a..09817bc6f9a8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6237,13 +6237,8 @@ static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) if (pipe_config->ips_force_disable) return false; - /* - * FIXME IPS should be fine as long as one plane is - * enabled, but in practice it seems to have problems - * when going from primary only to sprite only and vice - * versa. - */ - if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) + /* IPS should be fine as long as one plane is enabled. */ + if (hweight32(pipe_config->active_planes & ~BIT(PLANE_CURSOR)) != 1) return false; /* HSW can handle pixel rate up to cdclk? */ -- 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/3] drm/i915: Enable IPS with only sprite plane visible too, v2. 2017-11-20 12:23 ` [PATCH v2 2/3] drm/i915: Enable IPS with only sprite plane visible too, v2 Maarten Lankhorst @ 2017-11-20 13:27 ` Ville Syrjälä 2017-11-21 9:27 ` Maarten Lankhorst 2017-11-22 15:08 ` [PATCH] drm/i915: Enable IPS with only sprite plane visible too, v3 Maarten Lankhorst 0 siblings, 2 replies; 22+ messages in thread From: Ville Syrjälä @ 2017-11-20 13:27 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Mon, Nov 20, 2017 at 01:23:36PM +0100, Maarten Lankhorst wrote: > This comment predates atomic, and I think with the way we currently > track IPS, it's safe to enable this for the case we switch too. > > Changes since v1: > - Keep IPS enabled when switching planes. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 0b9ba415839a..09817bc6f9a8 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6237,13 +6237,8 @@ static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) > if (pipe_config->ips_force_disable) > return false; > > - /* > - * FIXME IPS should be fine as long as one plane is > - * enabled, but in practice it seems to have problems > - * when going from primary only to sprite only and vice > - * versa. > - */ > - if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) > + /* IPS should be fine as long as one plane is enabled. */ Maybe "... at least one plane is enabled." to make it less vague? > + if (hweight32(pipe_config->active_planes & ~BIT(PLANE_CURSOR)) != 1) I keep wondering if popcnt is cheaper than is_power_of_2()? If it is, maybe someone should actually make is_power_of_2() use popcnt... > return false; > > /* HSW can handle pixel rate up to cdclk? */ > -- > 2.15.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 2/3] drm/i915: Enable IPS with only sprite plane visible too, v2. 2017-11-20 13:27 ` Ville Syrjälä @ 2017-11-21 9:27 ` Maarten Lankhorst 2017-11-22 15:08 ` [PATCH] drm/i915: Enable IPS with only sprite plane visible too, v3 Maarten Lankhorst 1 sibling, 0 replies; 22+ messages in thread From: Maarten Lankhorst @ 2017-11-21 9:27 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx Op 20-11-17 om 14:27 schreef Ville Syrjälä: > On Mon, Nov 20, 2017 at 01:23:36PM +0100, Maarten Lankhorst wrote: >> This comment predates atomic, and I think with the way we currently >> track IPS, it's safe to enable this for the case we switch too. >> >> Changes since v1: >> - Keep IPS enabled when switching planes. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 9 ++------- >> 1 file changed, 2 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 0b9ba415839a..09817bc6f9a8 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -6237,13 +6237,8 @@ static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) >> if (pipe_config->ips_force_disable) >> return false; >> >> - /* >> - * FIXME IPS should be fine as long as one plane is >> - * enabled, but in practice it seems to have problems >> - * when going from primary only to sprite only and vice >> - * versa. >> - */ >> - if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) >> + /* IPS should be fine as long as one plane is enabled. */ > Maybe "... at least one plane is enabled." to make it less vague? > >> + if (hweight32(pipe_config->active_planes & ~BIT(PLANE_CURSOR)) != 1) > I keep wondering if popcnt is cheaper than is_power_of_2()? If it is, > maybe someone should actually make is_power_of_2() use popcnt... Crap, should be if (pipe_config->active_planes & ~BIT(PLANE_CURSOR)) > >> return false; >> >> /* HSW can handle pixel rate up to cdclk? */ >> -- >> 2.15.0 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] drm/i915: Enable IPS with only sprite plane visible too, v3. 2017-11-20 13:27 ` Ville Syrjälä 2017-11-21 9:27 ` Maarten Lankhorst @ 2017-11-22 15:08 ` Maarten Lankhorst 2017-11-22 15:28 ` Ville Syrjälä 1 sibling, 1 reply; 22+ messages in thread From: Maarten Lankhorst @ 2017-11-22 15:08 UTC (permalink / raw) To: intel-gfx This comment predates atomic, and I think with the way we currently track IPS, it's safe to enable this for the case we switch too. Changes since v1: - Keep IPS enabled when switching planes. Changes since v2: - Enable IPS when at least one plane is enabled. (Ville) Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b62cb6e90669..77778bc0f0ff 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6267,13 +6267,8 @@ static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) if (pipe_config->ips_force_disable) return false; - /* - * FIXME IPS should be fine as long as one plane is - * enabled, but in practice it seems to have problems - * when going from primary only to sprite only and vice - * versa. - */ - if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) + /* IPS should be fine as long as at least one plane is enabled. */ + if (pipe_config->active_planes & ~BIT(PLANE_CURSOR)) return false; /* HSW can handle pixel rate up to cdclk? */ -- 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] drm/i915: Enable IPS with only sprite plane visible too, v3. 2017-11-22 15:08 ` [PATCH] drm/i915: Enable IPS with only sprite plane visible too, v3 Maarten Lankhorst @ 2017-11-22 15:28 ` Ville Syrjälä 2017-11-22 18:39 ` [PATCH 2/2] drm/i915: Enable IPS with only sprite plane visible too, v4 Maarten Lankhorst 0 siblings, 1 reply; 22+ messages in thread From: Ville Syrjälä @ 2017-11-22 15:28 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Wed, Nov 22, 2017 at 04:08:03PM +0100, Maarten Lankhorst wrote: > This comment predates atomic, and I think with the way we currently > track IPS, it's safe to enable this for the case we switch too. > > Changes since v1: > - Keep IPS enabled when switching planes. > Changes since v2: > - Enable IPS when at least one plane is enabled. (Ville) > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index b62cb6e90669..77778bc0f0ff 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6267,13 +6267,8 @@ static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) > if (pipe_config->ips_force_disable) > return false; > > - /* > - * FIXME IPS should be fine as long as one plane is > - * enabled, but in practice it seems to have problems > - * when going from primary only to sprite only and vice > - * versa. > - */ > - if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) > + /* IPS should be fine as long as at least one plane is enabled. */ > + if (pipe_config->active_planes & ~BIT(PLANE_CURSOR)) == 0 missing > return false; > > /* HSW can handle pixel rate up to cdclk? */ > -- > 2.15.0 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 2/2] drm/i915: Enable IPS with only sprite plane visible too, v4. 2017-11-22 15:28 ` Ville Syrjälä @ 2017-11-22 18:39 ` Maarten Lankhorst 2017-11-23 14:37 ` Ville Syrjälä 0 siblings, 1 reply; 22+ messages in thread From: Maarten Lankhorst @ 2017-11-22 18:39 UTC (permalink / raw) To: intel-gfx This comment predates atomic, and I think with the way we currently track IPS, it's safe to enable this for the case we switch too. Changes since v1: - Keep IPS enabled when switching planes. Changes since v2: - Enable IPS when at least one plane is enabled. (Ville) Changes since v3: - Actually do what was advertised in v3, sigh! (Ville, CI) Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 95df5c2128b4..3b4ae922be3b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6246,13 +6246,8 @@ static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) if (pipe_config->ips_force_disable) return false; - /* - * FIXME IPS should be fine as long as one plane is - * enabled, but in practice it seems to have problems - * when going from primary only to sprite only and vice - * versa. - */ - if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) + /* IPS should be fine as long as at least one plane is enabled. */ + if (!(pipe_config->active_planes & ~BIT(PLANE_CURSOR))) return false; /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ -- 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] drm/i915: Enable IPS with only sprite plane visible too, v4. 2017-11-22 18:39 ` [PATCH 2/2] drm/i915: Enable IPS with only sprite plane visible too, v4 Maarten Lankhorst @ 2017-11-23 14:37 ` Ville Syrjälä 0 siblings, 0 replies; 22+ messages in thread From: Ville Syrjälä @ 2017-11-23 14:37 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx On Wed, Nov 22, 2017 at 07:39:06PM +0100, Maarten Lankhorst wrote: > This comment predates atomic, and I think with the way we currently > track IPS, it's safe to enable this for the case we switch too. > > Changes since v1: > - Keep IPS enabled when switching planes. > Changes since v2: > - Enable IPS when at least one plane is enabled. (Ville) > Changes since v3: > - Actually do what was advertised in v3, sigh! (Ville, CI) > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 95df5c2128b4..3b4ae922be3b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6246,13 +6246,8 @@ static bool hsw_compute_ips_config(struct intel_crtc_state *pipe_config) > if (pipe_config->ips_force_disable) > return false; > > - /* > - * FIXME IPS should be fine as long as one plane is > - * enabled, but in practice it seems to have problems > - * when going from primary only to sprite only and vice > - * versa. > - */ > - if (!(pipe_config->active_planes & BIT(PLANE_PRIMARY))) > + /* IPS should be fine as long as at least one plane is enabled. */ > + if (!(pipe_config->active_planes & ~BIT(PLANE_CURSOR))) > return false; Yep, now it makes sense ;) Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > /* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */ > -- > 2.15.0 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH (resend) 3/3] drm/i915: Re-enable fastboot by default 2017-11-20 12:23 [PATCH v2 0/3] drm/i915: Enable fastboot, v3! Maarten Lankhorst 2017-11-20 12:23 ` [PATCH v2 1/3] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v2 Maarten Lankhorst 2017-11-20 12:23 ` [PATCH v2 2/3] drm/i915: Enable IPS with only sprite plane visible too, v2 Maarten Lankhorst @ 2017-11-20 12:23 ` Maarten Lankhorst 2017-11-20 13:05 ` ✓ Fi.CI.BAT: success for drm/i915: Enable fastboot, v3! Patchwork ` (4 subsequent siblings) 7 siblings, 0 replies; 22+ messages in thread From: Maarten Lankhorst @ 2017-11-20 12:23 UTC (permalink / raw) To: intel-gfx; +Cc: Jani Nikula, Daniel Vetter This fix was originally reverted because it left a chromebook pixel black, and no immediate fix was available. This has been fixed in the meantime. Rather than trying to remove the parameter, set it to default to true for now, so we can always back out if required. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Testcase: kms_panel_fitting --- drivers/gpu/drm/i915/i915_params.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index c7292268ed43..b99cb58801e6 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -57,7 +57,7 @@ param(bool, alpha_support, IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT)) \ param(bool, enable_cmd_parser, true) \ param(bool, enable_hangcheck, true) \ - param(bool, fastboot, false) \ + param(bool, fastboot, true) \ param(bool, prefault_disable, false) \ param(bool, load_detect_test, false) \ param(bool, force_reset_modeset_test, false) \ -- 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 22+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Enable fastboot, v3! 2017-11-20 12:23 [PATCH v2 0/3] drm/i915: Enable fastboot, v3! Maarten Lankhorst ` (2 preceding siblings ...) 2017-11-20 12:23 ` [PATCH (resend) 3/3] drm/i915: Re-enable fastboot by default Maarten Lankhorst @ 2017-11-20 13:05 ` Patchwork 2017-11-20 14:15 ` ✓ Fi.CI.IGT: " Patchwork ` (3 subsequent siblings) 7 siblings, 0 replies; 22+ messages in thread From: Patchwork @ 2017-11-20 13:05 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx == Series Details == Series: drm/i915: Enable fastboot, v3! URL : https://patchwork.freedesktop.org/series/34098/ State : success == Summary == Series 34098v1 drm/i915: Enable fastboot, v3! https://patchwork.freedesktop.org/api/1.0/series/34098/revisions/1/mbox/ Test kms_busy: Subgroup basic-flip-a: fail -> PASS (fi-gdg-551) fdo#102654 Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-legacy: fail -> PASS (fi-gdg-551) fdo#102618 Test kms_pipe_crc_basic: Subgroup suspend-read-crc-pipe-b: pass -> INCOMPLETE (fi-snb-2520m) fdo#103713 fdo#102654 https://bugs.freedesktop.org/show_bug.cgi?id=102654 fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618 fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713 fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:441s fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:453s fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:382s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:545s fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:279s fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:507s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:508s fi-byt-j1900 total:289 pass:254 dwarn:0 dfail:0 fail:0 skip:35 time:496s fi-cfl-s2 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:604s fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:433s fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:268s fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:541s fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:429s fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:439s fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:434s fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:492s fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:458s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:484s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:536s fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:474s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:535s fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:584s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:447s fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:544s fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:563s fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:522s fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:497s fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:465s fi-snb-2520m total:246 pass:212 dwarn:0 dfail:0 fail:0 skip:33 fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:417s Blacklisted hosts: fi-glk-dsi total:289 pass:258 dwarn:0 dfail:0 fail:1 skip:30 time:503s 4b69b21e93c02a8b45781c490077dc1a35adc608 drm-tip: 2017y-11m-20d-10h-20m-58s UTC integration manifest b3b673ef83f2 drm/i915: Re-enable fastboot by default 0df89b40577b drm/i915: Enable IPS with only sprite plane visible too, v2. a0a4a79f52db drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v2. == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7202/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: Enable fastboot, v3! 2017-11-20 12:23 [PATCH v2 0/3] drm/i915: Enable fastboot, v3! Maarten Lankhorst ` (3 preceding siblings ...) 2017-11-20 13:05 ` ✓ Fi.CI.BAT: success for drm/i915: Enable fastboot, v3! Patchwork @ 2017-11-20 14:15 ` Patchwork 2017-11-22 15:38 ` ✗ Fi.CI.BAT: failure for drm/i915: Enable fastboot, v3! (rev3) Patchwork ` (2 subsequent siblings) 7 siblings, 0 replies; 22+ messages in thread From: Patchwork @ 2017-11-20 14:15 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx == Series Details == Series: drm/i915: Enable fastboot, v3! URL : https://patchwork.freedesktop.org/series/34098/ State : success == Summary == Test drv_module_reload: Subgroup basic-no-display: pass -> DMESG-WARN (shard-snb) fdo#102707 Test gem_eio: Subgroup in-flight-suspend: fail -> PASS (shard-hsw) fdo#103375 Test kms_cursor_legacy: Subgroup long-nonblocking-modeset-vs-cursor-atomic: pass -> SKIP (shard-hsw) fdo#103181 fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707 fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#103181 https://bugs.freedesktop.org/show_bug.cgi?id=103181 shard-hsw total:2585 pass:1473 dwarn:1 dfail:1 fail:10 skip:1100 time:9504s shard-snb total:2585 pass:1259 dwarn:2 dfail:1 fail:12 skip:1311 time:8015s Blacklisted hosts: shard-apl total:2565 pass:1603 dwarn:3 dfail:0 fail:22 skip:936 time:12910s shard-kbl total:2534 pass:1675 dwarn:8 dfail:0 fail:26 skip:823 time:10169s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7202/shards.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Enable fastboot, v3! (rev3) 2017-11-20 12:23 [PATCH v2 0/3] drm/i915: Enable fastboot, v3! Maarten Lankhorst ` (4 preceding siblings ...) 2017-11-20 14:15 ` ✓ Fi.CI.IGT: " Patchwork @ 2017-11-22 15:38 ` Patchwork 2017-11-22 19:27 ` ✓ Fi.CI.BAT: success for drm/i915: Enable fastboot, v3! (rev5) Patchwork 2017-11-22 21:11 ` ✓ Fi.CI.IGT: " Patchwork 7 siblings, 0 replies; 22+ messages in thread From: Patchwork @ 2017-11-22 15:38 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx == Series Details == Series: drm/i915: Enable fastboot, v3! (rev3) URL : https://patchwork.freedesktop.org/series/34098/ State : failure == Summary == Series 34098v3 drm/i915: Enable fastboot, v3! https://patchwork.freedesktop.org/api/1.0/series/34098/revisions/3/mbox/ Test debugfs_test: Subgroup read_all_entries: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Test gem_ctx_create: Subgroup basic-files: pass -> FAIL (fi-bsw-n3050) fdo#103706 Test gem_exec_suspend: Subgroup basic-s3: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-s4-devices: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Test gem_ringfill: Subgroup basic-default-hang: dmesg-warn -> PASS (fi-blb-e6850) fdo#101600 Test kms_busy: Subgroup basic-flip-a: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-flip-b: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-flip-c: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Test kms_cursor_legacy: Subgroup basic-busy-flip-before-cursor-atomic: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-busy-flip-before-cursor-legacy: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-flip-after-cursor-atomic: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-flip-after-cursor-legacy: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-flip-after-cursor-varying-size: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-flip-before-cursor-atomic: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-flip-before-cursor-legacy: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-flip-before-cursor-varying-size: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Test kms_flip: Subgroup basic-flip-vs-dpms: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-flip-vs-modeset: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-flip-vs-wf_vblank: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup basic-plain-flip: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Test kms_frontbuffer_tracking: Subgroup basic: pass -> DMESG-WARN (fi-bdw-5557u) fdo#102473 pass -> DMESG-WARN (fi-bdw-gvtdvm) Test kms_pipe_crc_basic: Subgroup hang-read-crc-pipe-a: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup hang-read-crc-pipe-b: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup hang-read-crc-pipe-c: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) pass -> FAIL (fi-cfl-s2) Subgroup nonblocking-crc-pipe-a: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup nonblocking-crc-pipe-a-frame-sequence: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup nonblocking-crc-pipe-b: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup nonblocking-crc-pipe-b-frame-sequence: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup nonblocking-crc-pipe-c: pass -> DMESG-WARN (fi-bdw-5557u) pass -> DMESG-WARN (fi-bdw-gvtdvm) Subgroup nonblocking-crc-pipe-c-frame-sequence: pass -> DMESG-WARN (fi-bdw-5557u) WARNING: Long output truncated fi-pnv-d510 failed to collect. IGT log at Patchwork_7235/fi-pnv-d510/igt.log 368de251cb3d8b5a69ea4e2731a2b53d329c8485 drm-tip: 2017y-11m-22d-11h-27m-26s UTC integration manifest 543608acf2b4 drm/i915: Re-enable fastboot by default 243c803c4365 drm/i915: Enable IPS with only sprite plane visible too, v3. 3d127369b401 drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v3. == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7235/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Enable fastboot, v3! (rev5) 2017-11-20 12:23 [PATCH v2 0/3] drm/i915: Enable fastboot, v3! Maarten Lankhorst ` (5 preceding siblings ...) 2017-11-22 15:38 ` ✗ Fi.CI.BAT: failure for drm/i915: Enable fastboot, v3! (rev3) Patchwork @ 2017-11-22 19:27 ` Patchwork 2017-11-22 21:11 ` ✓ Fi.CI.IGT: " Patchwork 7 siblings, 0 replies; 22+ messages in thread From: Patchwork @ 2017-11-22 19:27 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx == Series Details == Series: drm/i915: Enable fastboot, v3! (rev5) URL : https://patchwork.freedesktop.org/series/34098/ State : success == Summary == Series 34098v5 drm/i915: Enable fastboot, v3! https://patchwork.freedesktop.org/api/1.0/series/34098/revisions/5/mbox/ Test gem_exec_reloc: Subgroup basic-cpu-active: pass -> FAIL (fi-gdg-551) fdo#102582 +4 fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582 fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:449s fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:463s fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:380s fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:545s fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:278s fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:506s fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:513s fi-byt-j1900 total:289 pass:254 dwarn:0 dfail:0 fail:0 skip:35 time:499s fi-byt-n2820 total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:495s fi-cfl-s2 total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:620s fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:437s fi-gdg-551 total:289 pass:173 dwarn:1 dfail:0 fail:6 skip:109 time:266s fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:546s fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:429s fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:438s fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:430s fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:477s fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:463s fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:482s fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:533s fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:477s fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:536s fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:581s fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:454s fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:545s fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:561s fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:520s fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:504s fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:459s fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:568s fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:428s Blacklisted hosts: fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:554s fi-glk-dsi total:22 pass:21 dwarn:0 dfail:0 fail:0 skip:0 a3f080ee939e8befca924f1ab83deeabfa7ff98e drm-tip: 2017y-11m-22d-18h-37m-24s UTC integration manifest fb4472672320 drm/i915: Re-enable fastboot by default ccd6b721bda1 drm/i915: Enable IPS with only sprite plane visible too, v4. 17c63ce32a99 drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v3. == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7239/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: Enable fastboot, v3! (rev5) 2017-11-20 12:23 [PATCH v2 0/3] drm/i915: Enable fastboot, v3! Maarten Lankhorst ` (6 preceding siblings ...) 2017-11-22 19:27 ` ✓ Fi.CI.BAT: success for drm/i915: Enable fastboot, v3! (rev5) Patchwork @ 2017-11-22 21:11 ` Patchwork 7 siblings, 0 replies; 22+ messages in thread From: Patchwork @ 2017-11-22 21:11 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx == Series Details == Series: drm/i915: Enable fastboot, v3! (rev5) URL : https://patchwork.freedesktop.org/series/34098/ State : success == Summary == Test kms_frontbuffer_tracking: Subgroup fbc-1p-offscren-pri-shrfb-draw-blt: fail -> PASS (shard-snb) fdo#101623 Subgroup fbc-1p-pri-indfb-multidraw: incomplete -> PASS (shard-hsw) fdo#103167 Test kms_cursor_crc: Subgroup cursor-128x128-suspend: pass -> SKIP (shard-hsw) fdo#103375 Test drv_module_reload: Subgroup basic-no-display: pass -> DMESG-WARN (shard-hsw) fdo#102707 Test gem_busy: Subgroup close-race: fail -> PASS (shard-snb) fdo#103829 fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623 fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167 fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375 fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707 fdo#103829 https://bugs.freedesktop.org/show_bug.cgi?id=103829 shard-hsw total:2667 pass:1525 dwarn:2 dfail:0 fail:17 skip:1123 time:9411s shard-snb total:2646 pass:1289 dwarn:1 dfail:0 fail:18 skip:1337 time:7953s Blacklisted hosts: shard-apl total:2573 pass:1618 dwarn:4 dfail:0 fail:21 skip:927 time:12654s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7239/shards.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-11-30 15:54 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-11-20 12:23 [PATCH v2 0/3] drm/i915: Enable fastboot, v3! Maarten Lankhorst 2017-11-20 12:23 ` [PATCH v2 1/3] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v2 Maarten Lankhorst 2017-11-20 14:01 ` Ville Syrjälä 2017-11-20 16:21 ` Maarten Lankhorst 2017-11-22 15:07 ` [PATCH] drm/i915: Make ips_enabled a property depending on whether IPS is enabled, v3 Maarten Lankhorst 2017-11-22 15:34 ` Ville Syrjälä 2017-11-22 18:39 ` [PATCH 1/2] " Maarten Lankhorst 2017-11-23 14:36 ` Ville Syrjälä 2017-11-30 15:54 ` Maarten Lankhorst 2017-11-20 12:23 ` [PATCH v2 2/3] drm/i915: Enable IPS with only sprite plane visible too, v2 Maarten Lankhorst 2017-11-20 13:27 ` Ville Syrjälä 2017-11-21 9:27 ` Maarten Lankhorst 2017-11-22 15:08 ` [PATCH] drm/i915: Enable IPS with only sprite plane visible too, v3 Maarten Lankhorst 2017-11-22 15:28 ` Ville Syrjälä 2017-11-22 18:39 ` [PATCH 2/2] drm/i915: Enable IPS with only sprite plane visible too, v4 Maarten Lankhorst 2017-11-23 14:37 ` Ville Syrjälä 2017-11-20 12:23 ` [PATCH (resend) 3/3] drm/i915: Re-enable fastboot by default Maarten Lankhorst 2017-11-20 13:05 ` ✓ Fi.CI.BAT: success for drm/i915: Enable fastboot, v3! Patchwork 2017-11-20 14:15 ` ✓ Fi.CI.IGT: " Patchwork 2017-11-22 15:38 ` ✗ Fi.CI.BAT: failure for drm/i915: Enable fastboot, v3! (rev3) Patchwork 2017-11-22 19:27 ` ✓ Fi.CI.BAT: success for drm/i915: Enable fastboot, v3! (rev5) Patchwork 2017-11-22 21:11 ` ✓ Fi.CI.IGT: " Patchwork
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).