| Message ID | 1456826842-32553-1-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 01, 2016 at 11:07:21AM +0100, Maarten Lankhorst wrote: > As Paulo has noted we can help bisectability by separating computing > watermarks on a noop in 2 separate commits. > > This patch no longer clears the crtc watermark state, but recalculates > it completely. Regardless whether a level is used the full values for > each level are calculated. If a level is invalid wm[level].enable is > unset. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index d33de954a2e4..1b8ba777d2b3 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2300,7 +2300,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, > struct intel_plane_state *pristate = NULL; > struct intel_plane_state *sprstate = NULL; > struct intel_plane_state *curstate = NULL; > - int level, max_level = ilk_wm_max_level(dev); > + int level, max_level = ilk_wm_max_level(dev), usable_level; > struct ilk_wm_maximums max; > > cstate = intel_atomic_get_crtc_state(state, intel_crtc); > @@ -2308,7 +2308,6 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, > return PTR_ERR(cstate); > > pipe_wm = &cstate->wm.optimal.ilk; > - memset(pipe_wm, 0, sizeof(*pipe_wm)); > > for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { > ps = drm_atomic_get_plane_state(state, > @@ -2330,13 +2329,15 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, > (drm_rect_width(&sprstate->dst) != drm_rect_width(&sprstate->src) >> 16 || > drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16); > > + usable_level = max_level; > + > /* ILK/SNB: LP2+ watermarks only w/o sprites */ > if (INTEL_INFO(dev)->gen <= 6 && sprstate->visible) > - max_level = 1; > + usable_level = 1; > > /* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */ > if (pipe_wm->sprites_scaled) > - max_level = 0; > + usable_level = 0; > > ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, > pristate, sprstate, curstate, &pipe_wm->wm[0]); > @@ -2350,20 +2351,21 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, > ilk_compute_wm_reg_maximums(dev, 1, &max); > > for (level = 1; level <= max_level; level++) { > - struct intel_wm_level wm = {}; > + struct intel_wm_level *wm = &pipe_wm->wm[level]; > > ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, > - pristate, sprstate, curstate, &wm); > + pristate, sprstate, curstate, wm); > > /* > * Disable any watermark level that exceeds the > * register maximums since such watermarks are > * always invalid. > */ > - if (!ilk_validate_wm_level(level, &max, &wm)) > - break; > - > - pipe_wm->wm[level] = wm; > + if (!ilk_validate_wm_level(level, &max, wm)) { > + usable_level = level; > + wm->enable = false; > + } else if (level > usable_level) > + wm->enable = false; That seems to be the wrong way around. I don't think there's any point in calling ilk_validate_wm_level() if the level already exceeds the max. > } > > return 0; > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 02-03-16 om 12:17 schreef Ville Syrjälä: > On Tue, Mar 01, 2016 at 11:07:21AM +0100, Maarten Lankhorst wrote: >> As Paulo has noted we can help bisectability by separating computing >> watermarks on a noop in 2 separate commits. >> >> This patch no longer clears the crtc watermark state, but recalculates >> it completely. Regardless whether a level is used the full values for >> each level are calculated. If a level is invalid wm[level].enable is >> unset. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_pm.c | 22 ++++++++++++---------- >> 1 file changed, 12 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index d33de954a2e4..1b8ba777d2b3 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -2300,7 +2300,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, >> struct intel_plane_state *pristate = NULL; >> struct intel_plane_state *sprstate = NULL; >> struct intel_plane_state *curstate = NULL; >> - int level, max_level = ilk_wm_max_level(dev); >> + int level, max_level = ilk_wm_max_level(dev), usable_level; >> struct ilk_wm_maximums max; >> >> cstate = intel_atomic_get_crtc_state(state, intel_crtc); >> @@ -2308,7 +2308,6 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, >> return PTR_ERR(cstate); >> >> pipe_wm = &cstate->wm.optimal.ilk; >> - memset(pipe_wm, 0, sizeof(*pipe_wm)); >> >> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { >> ps = drm_atomic_get_plane_state(state, >> @@ -2330,13 +2329,15 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, >> (drm_rect_width(&sprstate->dst) != drm_rect_width(&sprstate->src) >> 16 || >> drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16); >> >> + usable_level = max_level; >> + >> /* ILK/SNB: LP2+ watermarks only w/o sprites */ >> if (INTEL_INFO(dev)->gen <= 6 && sprstate->visible) >> - max_level = 1; >> + usable_level = 1; >> >> /* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */ >> if (pipe_wm->sprites_scaled) >> - max_level = 0; >> + usable_level = 0; >> >> ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, >> pristate, sprstate, curstate, &pipe_wm->wm[0]); >> @@ -2350,20 +2351,21 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, >> ilk_compute_wm_reg_maximums(dev, 1, &max); >> >> for (level = 1; level <= max_level; level++) { >> - struct intel_wm_level wm = {}; >> + struct intel_wm_level *wm = &pipe_wm->wm[level]; >> >> ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, >> - pristate, sprstate, curstate, &wm); >> + pristate, sprstate, curstate, wm); >> >> /* >> * Disable any watermark level that exceeds the >> * register maximums since such watermarks are >> * always invalid. >> */ >> - if (!ilk_validate_wm_level(level, &max, &wm)) >> - break; >> - >> - pipe_wm->wm[level] = wm; >> + if (!ilk_validate_wm_level(level, &max, wm)) { >> + usable_level = level; >> + wm->enable = false; >> + } else if (level > usable_level) >> + wm->enable = false; > That seems to be the wrong way around. I don't think there's > any point in calling ilk_validate_wm_level() if the level > already exceeds the max. > Indeed! Will fix.
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d33de954a2e4..1b8ba777d2b3 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2300,7 +2300,7 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, struct intel_plane_state *pristate = NULL; struct intel_plane_state *sprstate = NULL; struct intel_plane_state *curstate = NULL; - int level, max_level = ilk_wm_max_level(dev); + int level, max_level = ilk_wm_max_level(dev), usable_level; struct ilk_wm_maximums max; cstate = intel_atomic_get_crtc_state(state, intel_crtc); @@ -2308,7 +2308,6 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, return PTR_ERR(cstate); pipe_wm = &cstate->wm.optimal.ilk; - memset(pipe_wm, 0, sizeof(*pipe_wm)); for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) { ps = drm_atomic_get_plane_state(state, @@ -2330,13 +2329,15 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, (drm_rect_width(&sprstate->dst) != drm_rect_width(&sprstate->src) >> 16 || drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16); + usable_level = max_level; + /* ILK/SNB: LP2+ watermarks only w/o sprites */ if (INTEL_INFO(dev)->gen <= 6 && sprstate->visible) - max_level = 1; + usable_level = 1; /* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */ if (pipe_wm->sprites_scaled) - max_level = 0; + usable_level = 0; ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate, pristate, sprstate, curstate, &pipe_wm->wm[0]); @@ -2350,20 +2351,21 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc, ilk_compute_wm_reg_maximums(dev, 1, &max); for (level = 1; level <= max_level; level++) { - struct intel_wm_level wm = {}; + struct intel_wm_level *wm = &pipe_wm->wm[level]; ilk_compute_wm_level(dev_priv, intel_crtc, level, cstate, - pristate, sprstate, curstate, &wm); + pristate, sprstate, curstate, wm); /* * Disable any watermark level that exceeds the * register maximums since such watermarks are * always invalid. */ - if (!ilk_validate_wm_level(level, &max, &wm)) - break; - - pipe_wm->wm[level] = wm; + if (!ilk_validate_wm_level(level, &max, wm)) { + usable_level = level; + wm->enable = false; + } else if (level > usable_level) + wm->enable = false; } return 0;
As Paulo has noted we can help bisectability by separating computing watermarks on a noop in 2 separate commits. This patch no longer clears the crtc watermark state, but recalculates it completely. Regardless whether a level is used the full values for each level are calculated. If a level is invalid wm[level].enable is unset. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_pm.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)