On Thu, Jan 14, 2016 at 06:37:32PM -0800, Matt Roper wrote: > On Thu, Jan 14, 2016 at 02:53:35PM +0200, [email protected] wrote: > > From: Ville Syrjälä <[email protected]> > > > > ilk_program_watermarks() is supposed to merge the active watermarks from > > all pipes. Thus we need to use the active config too instead of some > > precomputed stuff. > > So to clarify, the bug you're fixing here would be if we have racing > commits that operate on disjoint sets of CRTC's; in that case the second > one that actually gets into wm_mutex will fail to see the config changes > made by the first commit, right?
That, or I suppose cases where the intermediate and optimal watermarks have a different idea about the sprite enabled/scaled flags. I assume what it was doing is picking those flags always based on the optimal wms even when programming the intermedidate wms. > > Seems like we could go ahead and remove dev_priv->wm.config (and > calc_watermark_data() that builds it) since it's not actually doing us > any good. Although it's probably fine to hold that off to a subsequent > patch. IIRC I was a reference to that stuff in the SKL code too, so I didn't bother trying to kill it without knowing what it was doing. And I was too lazy to take a deeper look at the code right now. > > Both of your patches are > > Reviewed-by: Matt Roper <[email protected]> > > CI results do report SKL failures, but those are clearly bogus since SKL > doesn't even run any of the functions that you're changing in these two > patches; as you noted, it looks more like the machine had some kind of > bizarre hardware failure that was unrelated to the patchset. The other > results look clean. Given that, I've gone ahead and pushed your patches > to dinq. Thanks! Cheers. > > > Matt > > > > > > Fixes: aa363136866c ("drm/i915: Calculate watermark configuration during > > atomic check (v2)") > > Cc: Matt Roper <[email protected]> > > Signed-off-by: Ville Syrjälä <[email protected]> > > --- > > drivers/gpu/drm/i915/intel_pm.c | 32 ++++++++++++++++++++++++++------ > > 1 file changed, 26 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > b/drivers/gpu/drm/i915/intel_pm.c > > index e9f4e6e7b736..f44a961183d7 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -3680,23 +3680,43 @@ static void skl_update_wm(struct drm_crtc *crtc) > > dev_priv->wm.skl_hw = *results; > > } > > > > +static void ilk_compute_wm_config(struct drm_device *dev, > > + struct intel_wm_config *config) > > +{ > > + struct intel_crtc *crtc; > > + > > + /* Compute the currently _active_ config */ > > + for_each_intel_crtc(dev, crtc) { > > + const struct intel_pipe_wm *wm = &crtc->wm.active.ilk; > > + > > + if (!wm->pipe_enabled) > > + continue; > > + > > + config->sprites_enabled |= wm->sprites_enabled; > > + config->sprites_scaled |= wm->sprites_scaled; > > + config->num_pipes_active++; > > + } > > +} > > + > > static void ilk_program_watermarks(struct drm_i915_private *dev_priv) > > { > > struct drm_device *dev = dev_priv->dev; > > struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm; > > struct ilk_wm_maximums max; > > - struct intel_wm_config *config = &dev_priv->wm.config; > > + struct intel_wm_config config = {}; > > struct ilk_wm_values results = {}; > > enum intel_ddb_partitioning partitioning; > > > > - ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_1_2, &max); > > - ilk_wm_merge(dev, config, &max, &lp_wm_1_2); > > + ilk_compute_wm_config(dev, &config); > > + > > + ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_1_2, &max); > > + ilk_wm_merge(dev, &config, &max, &lp_wm_1_2); > > > > /* 5/6 split only in single pipe config on IVB+ */ > > if (INTEL_INFO(dev)->gen >= 7 && > > - config->num_pipes_active == 1 && config->sprites_enabled) { > > - ilk_compute_wm_maximums(dev, 1, config, INTEL_DDB_PART_5_6, > > &max); > > - ilk_wm_merge(dev, config, &max, &lp_wm_5_6); > > + config.num_pipes_active == 1 && config.sprites_enabled) { > > + ilk_compute_wm_maximums(dev, 1, &config, INTEL_DDB_PART_5_6, > > &max); > > + ilk_wm_merge(dev, &config, &max, &lp_wm_5_6); > > > > best_lp_wm = ilk_find_best_result(dev, &lp_wm_1_2, &lp_wm_5_6); > > } else { > > -- > > 2.4.10 > > > > -- > Matt Roper > Graphics Software Engineer > IoTG Platform Enabling & Development > Intel Corporation > (916) 356-2795 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
