On Thu, Oct 01, 2015 at 07:57:12PM -0300, Paulo Zanoni wrote: > According to my experiments (and later confirmation from the hardware > developers), the maximum sizes mentioned in the specification delimit > how far in the buffer the hardware tracking can go. And the hardware > calculates the size based on the plane address we provide - and the > provided plane address might not be the real x:0,y:0 point due to the > compute_page_offset() function. > > On platforms that do the x/y offset adjustment trick it will be really > hard to reproduce a bug, but on the current SKL we can reproduce the > bug with igt/kms_frontbuffer_tracking/fbc-farfromfence. With this > patch, we'll go from "CRC assertion failure" to "FBC unexpectedly > disabled", which is still a failure on the test suite but is not a > perceived user bug - you will just not save as much power as you could > if FBC is disabled. > > v2, rewrite patch after clarification from the Hadware guys: > - Rename function so it's clear what the check is for. > - Use the new intel_fbc_get_plane_source_sizes() function in order > to get the proper sizes as seen by FBC. > v3: > - Rebase after the s/sizes/size/ on the previous patch. > - Adjust comment wording (Ville). > - s/used_/effective_/ (Ville). > > Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence (SKL) > Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com> > Signed-off-by: Paulo Zanoni <paulo.r.zan...@intel.com>
Merged these three patches to dinq, thanks. -Daniel > --- > drivers/gpu/drm/i915/intel_fbc.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c > b/drivers/gpu/drm/i915/intel_fbc.c > index 18e228b..cf47352 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -799,10 +799,16 @@ static bool pixel_format_is_valid(struct > drm_framebuffer *fb) > } > } > > -static bool pipe_size_is_valid(struct intel_crtc *crtc) > +/* > + * For some reason, the hardware tracking starts looking at whatever we > + * programmed as the display plane base address register. It does not look at > + * the X and Y offset registers. That's why we look at the > crtc->adjusted{x,y} > + * variables instead of just looking at the pipe/plane size. > + */ > +static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc) > { > struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; > - unsigned int max_w, max_h; > + unsigned int effective_w, effective_h, max_w, max_h; > > if (INTEL_INFO(dev_priv)->gen >= 8 || IS_HASWELL(dev_priv)) { > max_w = 4096; > @@ -815,8 +821,11 @@ static bool pipe_size_is_valid(struct intel_crtc *crtc) > max_h = 1536; > } > > - return crtc->config->pipe_src_w <= max_w && > - crtc->config->pipe_src_h <= max_h; > + intel_fbc_get_plane_source_size(crtc, &effective_w, &effective_h); > + effective_w += crtc->adjusted_x; > + effective_h += crtc->adjusted_y; > + > + return effective_w <= max_w && effective_h <= max_h; > } > > /** > @@ -893,7 +902,7 @@ static void __intel_fbc_update(struct drm_i915_private > *dev_priv) > goto out_disable; > } > > - if (!pipe_size_is_valid(intel_crtc)) { > + if (!intel_fbc_hw_tracking_covers_screen(intel_crtc)) { > set_no_fbc_reason(dev_priv, FBC_MODE_TOO_LARGE); > goto out_disable; > } > -- > 2.5.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx