| Message ID | 20171108092922.58142-1-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 08, 2017 at 10:29:20AM +0100, Maarten Lankhorst wrote: > Fixes the following error when fastset is enabled, caught by CI: > > [drm:ilk_validate_wm_level.part.8 [i915]] Sprite WM0 too large 56 (max 0) > [drm:ilk_validate_pipe_wm [i915]] LP0 watermark invalid > [drm:intel_crtc_atomic_check [i915]] No valid intermediate pipe watermarks are possible > > Triggered on debugfs_test.read_all_entries, but could have been any igt > test depending on ordering. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> So I'm no expert on this, but why is this not needed for g4x and vlv intermediate wm? I think the commit message should explain that. I think it should also explain why simply shutting the above warnings up is safe. I think once that's fixed I understand why we need this and why it works, but probably better to get an Ack from Ville to make sure. -Daniel > --- > drivers/gpu/drm/i915/intel_pm.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 46440e2ecb33..9e8a0a9cac02 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3137,7 +3137,9 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev, > * and after the vblank. > */ > *a = newstate->wm.ilk.optimal; > - if (!newstate->base.active || drm_atomic_crtc_needs_modeset(&newstate->base)) > + if (!newstate->base.active || > + drm_atomic_crtc_needs_modeset(&newstate->base) || > + oldstate->base.mode.private_flags & I915_MODE_FLAG_INHERITED) > return 0; > > a->pipe_enabled |= b->pipe_enabled; > -- > 2.15.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Op 08-11-17 om 16:41 schreef Daniel Vetter: > On Wed, Nov 08, 2017 at 10:29:20AM +0100, Maarten Lankhorst wrote: >> Fixes the following error when fastset is enabled, caught by CI: >> >> [drm:ilk_validate_wm_level.part.8 [i915]] Sprite WM0 too large 56 (max 0) >> [drm:ilk_validate_pipe_wm [i915]] LP0 watermark invalid >> [drm:intel_crtc_atomic_check [i915]] No valid intermediate pipe watermarks are possible >> >> Triggered on debugfs_test.read_all_entries, but could have been any igt >> test depending on ordering. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > So I'm no expert on this, but why is this not needed for g4x and vlv > intermediate wm? I think the commit message should explain that. I think > it should also explain why simply shutting the above warnings up is safe. > > I think once that's fixed I understand why we need this and why it works, > but probably better to get an Ack from Ville to make sure. > -Daniel True, reason it's not needed there is because intermediate calculation can't fail. And yeah best say because BIOS wm are bogus here. >> --- >> drivers/gpu/drm/i915/intel_pm.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c >> index 46440e2ecb33..9e8a0a9cac02 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -3137,7 +3137,9 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev, >> * and after the vblank. >> */ >> *a = newstate->wm.ilk.optimal; >> - if (!newstate->base.active || drm_atomic_crtc_needs_modeset(&newstate->base)) >> + if (!newstate->base.active || >> + drm_atomic_crtc_needs_modeset(&newstate->base) || >> + oldstate->base.mode.private_flags & I915_MODE_FLAG_INHERITED) >> return 0; >> >> a->pipe_enabled |= b->pipe_enabled; >> -- >> 2.15.0 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Nov 08, 2017 at 05:09:33PM +0100, Maarten Lankhorst wrote: > Op 08-11-17 om 16:41 schreef Daniel Vetter: > > On Wed, Nov 08, 2017 at 10:29:20AM +0100, Maarten Lankhorst wrote: > >> Fixes the following error when fastset is enabled, caught by CI: > >> > >> [drm:ilk_validate_wm_level.part.8 [i915]] Sprite WM0 too large 56 (max 0) > >> [drm:ilk_validate_pipe_wm [i915]] LP0 watermark invalid > >> [drm:intel_crtc_atomic_check [i915]] No valid intermediate pipe watermarks are possible > >> > >> Triggered on debugfs_test.read_all_entries, but could have been any igt > >> test depending on ordering. > >> > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > So I'm no expert on this, but why is this not needed for g4x and vlv > > intermediate wm? I think the commit message should explain that. I think > > it should also explain why simply shutting the above warnings up is safe. > > > > I think once that's fixed I understand why we need this and why it works, > > but probably better to get an Ack from Ville to make sure. > > -Daniel > True, reason it's not needed there is because intermediate calculation can't fail. > And yeah best say because BIOS wm are bogus here. Ok, with that (maybe a bit more verbose) explained in the commit message: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > >> --- > >> drivers/gpu/drm/i915/intel_pm.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > >> index 46440e2ecb33..9e8a0a9cac02 100644 > >> --- a/drivers/gpu/drm/i915/intel_pm.c > >> +++ b/drivers/gpu/drm/i915/intel_pm.c > >> @@ -3137,7 +3137,9 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev, > >> * and after the vblank. > >> */ > >> *a = newstate->wm.ilk.optimal; > >> - if (!newstate->base.active || drm_atomic_crtc_needs_modeset(&newstate->base)) > >> + if (!newstate->base.active || > >> + drm_atomic_crtc_needs_modeset(&newstate->base) || > >> + oldstate->base.mode.private_flags & I915_MODE_FLAG_INHERITED) > >> return 0; > >> > >> a->pipe_enabled |= b->pipe_enabled; > >> -- > >> 2.15.0 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
Op 09-11-17 om 11:41 schreef Daniel Vetter: > On Wed, Nov 08, 2017 at 05:09:33PM +0100, Maarten Lankhorst wrote: >> Op 08-11-17 om 16:41 schreef Daniel Vetter: >>> On Wed, Nov 08, 2017 at 10:29:20AM +0100, Maarten Lankhorst wrote: >>>> Fixes the following error when fastset is enabled, caught by CI: >>>> >>>> [drm:ilk_validate_wm_level.part.8 [i915]] Sprite WM0 too large 56 (max 0) >>>> [drm:ilk_validate_pipe_wm [i915]] LP0 watermark invalid >>>> [drm:intel_crtc_atomic_check [i915]] No valid intermediate pipe watermarks are possible >>>> >>>> Triggered on debugfs_test.read_all_entries, but could have been any igt >>>> test depending on ordering. >>>> >>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> >>> So I'm no expert on this, but why is this not needed for g4x and vlv >>> intermediate wm? I think the commit message should explain that. I think >>> it should also explain why simply shutting the above warnings up is safe. >>> >>> I think once that's fixed I understand why we need this and why it works, >>> but probably better to get an Ack from Ville to make sure. >>> -Daniel >> True, reason it's not needed there is because intermediate calculation can't fail. >> And yeah best say because BIOS wm are bogus here. > Ok, with that (maybe a bit more verbose) explained in the commit message: > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Hm, something fishy is going on. This should have been caught by sanitize_watermarks() early during boot. More investigation is needed...
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 46440e2ecb33..9e8a0a9cac02 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3137,7 +3137,9 @@ static int ilk_compute_intermediate_wm(struct drm_device *dev, * and after the vblank. */ *a = newstate->wm.ilk.optimal; - if (!newstate->base.active || drm_atomic_crtc_needs_modeset(&newstate->base)) + if (!newstate->base.active || + drm_atomic_crtc_needs_modeset(&newstate->base) || + oldstate->base.mode.private_flags & I915_MODE_FLAG_INHERITED) return 0; a->pipe_enabled |= b->pipe_enabled;
Fixes the following error when fastset is enabled, caught by CI: [drm:ilk_validate_wm_level.part.8 [i915]] Sprite WM0 too large 56 (max 0) [drm:ilk_validate_pipe_wm [i915]] LP0 watermark invalid [drm:intel_crtc_atomic_check [i915]] No valid intermediate pipe watermarks are possible Triggered on debugfs_test.read_all_entries, but could have been any igt test depending on ordering. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_pm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)