Em Qui, 2017-06-01 às 16:09 -0700, Manasi Navare escreveu: > The modified commit message looks good to me. > > Reviewed-by: Manasi Navare <[email protected]>
Reviewed-by: Paulo Zanoni <[email protected]> As a longer term plan we could try to think some way to reduce the complexity between the Kernel and IGT interaction here: maybe there's a way to make things work without having to preserve no_fbc_reason between multiple commits. But I don't have any specific ideas, so suggestions are welcome. > > 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 <[email protected]. > > uk> > > Signed-off-by: Gabriel Krisman Bertazi <[email protected]> > > Cc: Paulo Zanoni <[email protected]> > > --- > > 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
