[Intel-gfx] [PATCH 4/6] drm/i915: set ILK_DPFC_FENCE_YOFF to 0 on SNB

[Paulo Zanoni]

2015-07-09 14:10 GMT-03:00 Daniel Vetter <daniel at ffwll.ch>:
> On Wed, Jul 08, 2015 at 05:58:57PM -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at 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.zanoni at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch



-- 
Paulo Zanoni