| Message ID | 1443023547-19896-6-git-send-email-paulo.r.zanoni@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 23, 2015 at 12:52:25PM -0300, Paulo Zanoni wrote: > According to my experiments, the maximum sizes mentioned in the > specification delimit how far in the buffer the hardware tracking can > go. And the hardware seems to calculate the size based on the plane > address and x/y offsets we specify to it. So adjust the code to do the > proper checks. > > 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 > be if FBC is disabled. > > Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence (SKL) > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Same query again for using src_[wh]. Do we have tests for FBC + panel fitting? Did I miss some explanation in the code that isn't visible in the diffs? -Chris
Em Qua, 2015-09-23 às 17:59 +0100, Chris Wilson escreveu: > On Wed, Sep 23, 2015 at 12:52:25PM -0300, Paulo Zanoni wrote: > > According to my experiments, the maximum sizes mentioned in the > > specification delimit how far in the buffer the hardware tracking > > can > > go. And the hardware seems to calculate the size based on the plane > > address and x/y offsets we specify to it. So adjust the code to do > > the > > proper checks. > > > > 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 > > be if FBC is disabled. > > > > Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence (SKL) > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Same query again for using src_[wh]. After clarification from the HW guys (which CCd you), I wrote new versions of these patches. > Do we have tests for FBC + panel > fitting? The --use-small-modes option forces a 1024x768 mode for eDP, which triggers the panel fitter in case its native resolution is not 1024x768. This is not the default option, but I run it eventually. For this specific patch, the real "failure" is when you do a GTT write and then the hardware tracking doesn't detect it and you end with a CRC error. For the other patch that's related with sizes (the CFB size patch) what we need is a stolen memory checker, like we have for slabs: allocate an extra page at the exact end of the CFB, write magic bits to it, enable FBC, check if the magic bits are still there. > > Did I miss some explanation in the code that isn't visible in the > diffs? > -Chris >
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 0c7b59b..2cc2528 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -758,7 +758,7 @@ static bool pixel_format_is_valid(struct drm_framebuffer *fb) static bool size_is_valid(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; - unsigned int max_w, max_h; + unsigned int tracked_w, tracked_h, max_w, max_h; if (INTEL_INFO(dev_priv)->gen >= 8 || IS_HASWELL(dev_priv)) { max_w = 4096; @@ -771,8 +771,10 @@ static bool 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; + tracked_w = (crtc->base.primary->state->src_w >> 16) + crtc->adjusted_x; + tracked_h = (crtc->base.primary->state->src_h >> 16) + crtc->adjusted_y; + + return tracked_w <= max_w && tracked_h <= max_h; } /**
According to my experiments, the maximum sizes mentioned in the specification delimit how far in the buffer the hardware tracking can go. And the hardware seems to calculate the size based on the plane address and x/y offsets we specify to it. So adjust the code to do the proper checks. 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 be if FBC is disabled. Testcase: igt/kms_frontbuffer_tracking/fbc-farfromfence (SKL) Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> --- drivers/gpu/drm/i915/intel_fbc.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)