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

Reply via email to