Re: [Intel-gfx] [PATCH 4/6] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB
2015-07-09 14:39 GMT-03:00 Ville Syrjälä ville.syrj...@linux.intel.com: On Thu, Jul 09, 2015 at 02:31:15PM -0300, Paulo Zanoni wrote: 2015-07-09 14:22 GMT-03:00 Ville Syrjälä ville.syrj...@linux.intel.com: On Thu, Jul 09, 2015 at 07:10:04PM +0200, Daniel Vetter wrote: On Wed, Jul 08, 2015 at 05:58:57PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com The doc is pretty clear that this register should be set to 0 on SNB. We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines below. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Hm, do we have testcases where we have a sufficiently big y offset? We can just allocate 128 lines more and use that as the offset, that should be big enough everywhere. Actually make that 129 lines to check the tile-size rounding ;-) Ofc this means we need to have two sets of testcases for all the affected tests (i.e. everything that tries to test the gtt hw tracking). Another funny corner case (which we're getting wrong on skl even without fbc) is x offsets 2048 pixels (since x/y offset registers don't hold bigger values and then it wraps). I.e. I'd like this patch (and the others) to be augmented with a Testcase: tag. I think the entire Y offset thing is currently being misprogrammed. IIRC the offset is from the display base address but we program in the offset from the start of the FB. After patch 3, all the current tests pass on BDW. Can you suggest a different test that won't pass? Ah patch 3 tries to fix it. It's not entirely accurate though since it simply relies on an implementation detail of intel_gen4_compute_page_offset(). Well, assuming my recollection of the hardware details is correct. Also IIRC intel_gen4_compute_page_offset() isn't even used on SKL/BXT currently, so it should fail on those platforms. Daniel clarified the problem to me, so I implemented the following test case: http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/commit/?id=04d1311fc3d2127d609b5c5e670bf9887652cb17 I hope this exercises the problem you're mentioning. So far I only tested BDW and it passes. -- Ville Syrjälä Intel OTC -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB
On Thu, Jul 09, 2015 at 07:10:04PM +0200, Daniel Vetter wrote: On Wed, Jul 08, 2015 at 05:58:57PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com The doc is pretty clear that this register should be set to 0 on SNB. We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines below. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Hm, do we have testcases where we have a sufficiently big y offset? We can just allocate 128 lines more and use that as the offset, that should be big enough everywhere. Actually make that 129 lines to check the tile-size rounding ;-) Ofc this means we need to have two sets of testcases for all the affected tests (i.e. everything that tries to test the gtt hw tracking). Another funny corner case (which we're getting wrong on skl even without fbc) is x offsets 2048 pixels (since x/y offset registers don't hold bigger values and then it wraps). I.e. I'd like this patch (and the others) to be augmented with a Testcase: tag. I think the entire Y offset thing is currently being misprogrammed. IIRC the offset is from the display base address but we program in the offset from the start of the FB. Cheers, Daniel --- drivers/gpu/drm/i915/intel_fbc.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 0373cbc..0a24e96 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -216,7 +216,12 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) dpfc_ctl |= obj-fence_reg; y_offset = get_crtc_fence_y_offset(crtc); - I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); + + if (IS_GEN5(dev_priv)) + I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); + else + I915_WRITE(ILK_DPFC_FENCE_YOFF, 0); + I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID); /* enable it... */ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); -- 2.1.4 ___ 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 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB
On Wed, Jul 08, 2015 at 05:58:57PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com The doc is pretty clear that this register should be set to 0 on SNB. We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines below. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Hm, do we have testcases where we have a sufficiently big y offset? We can just allocate 128 lines more and use that as the offset, that should be big enough everywhere. Actually make that 129 lines to check the tile-size rounding ;-) Ofc this means we need to have two sets of testcases for all the affected tests (i.e. everything that tries to test the gtt hw tracking). Another funny corner case (which we're getting wrong on skl even without fbc) is x offsets 2048 pixels (since x/y offset registers don't hold bigger values and then it wraps). I.e. I'd like this patch (and the others) to be augmented with a Testcase: tag. Cheers, Daniel --- drivers/gpu/drm/i915/intel_fbc.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 0373cbc..0a24e96 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -216,7 +216,12 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) dpfc_ctl |= obj-fence_reg; y_offset = get_crtc_fence_y_offset(crtc); - I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); + + if (IS_GEN5(dev_priv)) + I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); + else + I915_WRITE(ILK_DPFC_FENCE_YOFF, 0); + I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID); /* enable it... */ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); -- 2.1.4 ___ 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
Re: [Intel-gfx] [PATCH 4/6] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB
2015-07-09 14:10 GMT-03:00 Daniel Vetter dan...@ffwll.ch: On Wed, Jul 08, 2015 at 05:58:57PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com The doc is pretty clear that this register should be set to 0 on SNB. We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines below. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Hm, do we have testcases where we have a sufficiently big y offset? We can just allocate 128 lines more and use that as the offset, that should be big enough everywhere. Actually make that 129 lines to check the tile-size rounding ;-) Yes, it's 500x500. See FBS_MULTI on kms_frontbuffer_tracking. Subtests with sfb on their names create a Single frontbuffer for each possible primary plane (primary, secondary, offscreen), while subtests with mfb have Multiple pipes pointing to the same frontbuffer. See the small drawing inside kms_frontbuffer_tracking, at the top of create_big_fb(). By the way, it's on my TODO list to change that arrangement a little bit in order to avoid super-huge strides. Ofc this means we need to have two sets of testcases for all the affected tests (i.e. everything that tries to test the gtt hw tracking). We do. Another funny corner case (which we're getting wrong on skl even without fbc) is x offsets 2048 pixels (since x/y offset registers don't hold bigger values and then it wraps). Can you please clarify the sentence above? For the dual-screen subtests, if we use 2 screens of 1024 + the 500 pixel offset, we'll already be bigger than 2048 pixels. I.e. I'd like this patch (and the others) to be augmented with a Testcase: This one doesn't have a Testcase tag because I'm not testing SNB right now, so I can't confirm anything here. Patch 3 has the useful Testcases tags. tag. Cheers, Daniel --- drivers/gpu/drm/i915/intel_fbc.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 0373cbc..0a24e96 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -216,7 +216,12 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) dpfc_ctl |= obj-fence_reg; y_offset = get_crtc_fence_y_offset(crtc); - I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); + + if (IS_GEN5(dev_priv)) + I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); + else + I915_WRITE(ILK_DPFC_FENCE_YOFF, 0); + I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID); /* enable it... */ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); -- 2.1.4 ___ 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 -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB
2015-07-09 14:15 GMT-03:00 Paulo Zanoni przan...@gmail.com: 2015-07-09 14:10 GMT-03:00 Daniel Vetter dan...@ffwll.ch: On Wed, Jul 08, 2015 at 05:58:57PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com The doc is pretty clear that this register should be set to 0 on SNB. We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines below. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Hm, do we have testcases where we have a sufficiently big y offset? We can just allocate 128 lines more and use that as the offset, that should be big enough everywhere. Actually make that 129 lines to check the tile-size rounding ;-) Yes, it's 500x500. See FBS_MULTI on kms_frontbuffer_tracking. Subtests with sfb on their names create a Single frontbuffer for each possible primary plane (primary, secondary, offscreen), while subtests with mfb have Multiple pipes pointing to the same frontbuffer. See the small drawing inside kms_frontbuffer_tracking, at the top of create_big_fb(). By the way, it's on my TODO list to change that arrangement a little bit in order to avoid super-huge strides. Also notice that I tested values other than 500x500 while developing patch 3. 500x500 seems like a nice value since it's not aligned with any tiles and never part of the first tile. Ofc this means we need to have two sets of testcases for all the affected tests (i.e. everything that tries to test the gtt hw tracking). We do. Another funny corner case (which we're getting wrong on skl even without fbc) is x offsets 2048 pixels (since x/y offset registers don't hold bigger values and then it wraps). Can you please clarify the sentence above? For the dual-screen subtests, if we use 2 screens of 1024 + the 500 pixel offset, we'll already be bigger than 2048 pixels. I.e. I'd like this patch (and the others) to be augmented with a Testcase: This one doesn't have a Testcase tag because I'm not testing SNB right now, so I can't confirm anything here. Patch 3 has the useful Testcases tags. tag. Cheers, Daniel --- drivers/gpu/drm/i915/intel_fbc.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 0373cbc..0a24e96 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -216,7 +216,12 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) dpfc_ctl |= obj-fence_reg; y_offset = get_crtc_fence_y_offset(crtc); - I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); + + if (IS_GEN5(dev_priv)) + I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); + else + I915_WRITE(ILK_DPFC_FENCE_YOFF, 0); + I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID); /* enable it... */ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); -- 2.1.4 ___ 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 -- Paulo Zanoni -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB
On Thu, Jul 09, 2015 at 02:31:15PM -0300, Paulo Zanoni wrote: 2015-07-09 14:22 GMT-03:00 Ville Syrjälä ville.syrj...@linux.intel.com: On Thu, Jul 09, 2015 at 07:10:04PM +0200, Daniel Vetter wrote: On Wed, Jul 08, 2015 at 05:58:57PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com The doc is pretty clear that this register should be set to 0 on SNB. We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines below. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Hm, do we have testcases where we have a sufficiently big y offset? We can just allocate 128 lines more and use that as the offset, that should be big enough everywhere. Actually make that 129 lines to check the tile-size rounding ;-) Ofc this means we need to have two sets of testcases for all the affected tests (i.e. everything that tries to test the gtt hw tracking). Another funny corner case (which we're getting wrong on skl even without fbc) is x offsets 2048 pixels (since x/y offset registers don't hold bigger values and then it wraps). I.e. I'd like this patch (and the others) to be augmented with a Testcase: tag. I think the entire Y offset thing is currently being misprogrammed. IIRC the offset is from the display base address but we program in the offset from the start of the FB. After patch 3, all the current tests pass on BDW. Can you suggest a different test that won't pass? Ah patch 3 tries to fix it. It's not entirely accurate though since it simply relies on an implementation detail of intel_gen4_compute_page_offset(). Well, assuming my recollection of the hardware details is correct. Also IIRC intel_gen4_compute_page_offset() isn't even used on SKL/BXT currently, so it should fail on those platforms. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB
2015-07-09 14:22 GMT-03:00 Ville Syrjälä ville.syrj...@linux.intel.com: On Thu, Jul 09, 2015 at 07:10:04PM +0200, Daniel Vetter wrote: On Wed, Jul 08, 2015 at 05:58:57PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com The doc is pretty clear that this register should be set to 0 on SNB. We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines below. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Hm, do we have testcases where we have a sufficiently big y offset? We can just allocate 128 lines more and use that as the offset, that should be big enough everywhere. Actually make that 129 lines to check the tile-size rounding ;-) Ofc this means we need to have two sets of testcases for all the affected tests (i.e. everything that tries to test the gtt hw tracking). Another funny corner case (which we're getting wrong on skl even without fbc) is x offsets 2048 pixels (since x/y offset registers don't hold bigger values and then it wraps). I.e. I'd like this patch (and the others) to be augmented with a Testcase: tag. I think the entire Y offset thing is currently being misprogrammed. IIRC the offset is from the display base address but we program in the offset from the start of the FB. After patch 3, all the current tests pass on BDW. Can you suggest a different test that won't pass? Cheers, Daniel --- drivers/gpu/drm/i915/intel_fbc.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 0373cbc..0a24e96 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -216,7 +216,12 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) dpfc_ctl |= obj-fence_reg; y_offset = get_crtc_fence_y_offset(crtc); - I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); + + if (IS_GEN5(dev_priv)) + I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); + else + I915_WRITE(ILK_DPFC_FENCE_YOFF, 0); + I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID); /* enable it... */ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); -- 2.1.4 ___ 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 -- Ville Syrjälä Intel OTC -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB
From: Paulo Zanoni paulo.r.zan...@intel.com The doc is pretty clear that this register should be set to 0 on SNB. We already write y_offset to DPFC_CPU_FENCE_OFFSET a few lines below. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_fbc.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 0373cbc..0a24e96 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -216,7 +216,12 @@ static void ilk_fbc_enable(struct intel_crtc *crtc) dpfc_ctl |= obj-fence_reg; y_offset = get_crtc_fence_y_offset(crtc); - I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); + + if (IS_GEN5(dev_priv)) + I915_WRITE(ILK_DPFC_FENCE_YOFF, y_offset); + else + I915_WRITE(ILK_DPFC_FENCE_YOFF, 0); + I915_WRITE(ILK_FBC_RT_BASE, i915_gem_obj_ggtt_offset(obj) | ILK_FBC_RT_VALID); /* enable it... */ I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
