The modified commit message looks good to me. Reviewed-by: Manasi Navare <manasi.d.navare@xxxxxxxxx> On Thu, Jun 01, 2017 at 12:36:08PM -0300, Gabriel Krisman Bertazi wrote: > If the atomic commit doesn't include any new plane, there is no need to > choose a new CRTC for FBC, and the intel_fbc_choose_crtc() will bail out > early. Although, if the FBC setup failed in the previous commit, if the > current commit doesn't include new plane update, it tries to overwrite > no_fbc_reason to "no suitable CRTC for FBC". For that scenario, it is > better that we simply keep the old message in-place to make debugging > easier. > > A scenario where this happens is with the > igt@kms_frontbuffer_tracking@fbc-suspend testcase when executed on a > Haswell system with not enough stolen memory. When enabling the CRTC, > the FBC setup will be correctly initialized to a specific CRTC, but > won't be enabled, since there is not enough memory. The testcase will > then enable CRC checking, which requires a quirk for Haswell, which > issues a new atomic commit that doesn't update the planes. Since that > update doesn't include any new planes (and the FBC wasn't enabled), > intel_fbc_choose_crtc() will not find any suitable CRTC, but update the > error message, hiding the lack of memory information, which is the > actual cause of the initialization failure. As a result, this causes > that test, which should skip, to fail on Haswell. > > Changes since v1: > - Update commit message (Manasi) > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100020 > Fixes: f7e9b004b8a3 ("drm/i915/fbc: inline intel_fbc_can_choose()") > Reported-by: Dorota Czaplejewicz <dorota.czaplejewicz@xxxxxxxxxxxxxxx> > Signed-off-by: Gabriel Krisman Bertazi <krisman@xxxxxxxxxxxxxxx> > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_fbc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c > index ded2add18b26..0c99c9b731ee 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -1045,6 +1045,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, > struct drm_plane *plane; > struct drm_plane_state *plane_state; > bool crtc_chosen = false; > + bool new_planes = false; > int i; > > mutex_lock(&fbc->lock); > @@ -1066,6 +1067,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, > to_intel_plane_state(plane_state); > struct intel_crtc_state *intel_crtc_state; > struct intel_crtc *crtc = to_intel_crtc(plane_state->crtc); > + new_planes = true; > > if (!intel_plane_state->base.visible) > continue; > @@ -1084,7 +1086,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, > break; > } > > - if (!crtc_chosen) > + if (new_planes && !crtc_chosen) > fbc->no_fbc_reason = "no suitable CRTC for FBC"; > > out: > -- > 2.11.0 > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
