On 11/10/2016 05:23 PM, Tvrtko Ursulin wrote:
>
> On 10/11/2016 13:17, Tomeu Vizoso wrote:
>> On 1 November 2016 at 16:44, Tvrtko Ursulin <tursulin@xxxxxxxxxxx> wrote:
>>>
>>> Hi,
>>>
>>>
>>>
>>> On 02/03/16 14:00, Tomeu Vizoso wrote:
>>>>
>>>> igt_create_bo_with_dimensions() is intended to abstract differences
>>>> between drivers in buffer object creation.
>>>>
>>>> The driver-specific ioctls will be called if the driver that is being
>>>> tested can satisfy the needs of the calling subtest, or it will be
>>>> skipped otherwise.
>>>>
>>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@xxxxxxxxxxxxx>
>>>> ---
>>>>
>>>> lib/igt_fb.c | 83
>>>> ++++++++++++++++++++++++++++++++++++++++++------------------
>>>> lib/igt_fb.h | 6 +++++
>>>> 2 files changed, 65 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
>>>> index cd1605308308..0a3526f4e4ea 100644
>>>> --- a/lib/igt_fb.c
>>>> +++ b/lib/igt_fb.c
>>>> @@ -174,30 +174,66 @@ void igt_calc_fb_size(int fd, int width, int height,
>>>> int bpp, uint64_t tiling,
>>>> *size_ret = size;
>>>> }
>>>>
>>>> +int igt_create_bo_with_dimensions(int fd, int width, int height,
>>>> + uint32_t format, uint64_t modifier,
>>>> + unsigned stride, unsigned *size_ret,
>>>> + unsigned *stride_ret, bool *is_dumb)
>>>> +{
>>>> + int bpp = igt_drm_format_to_bpp(format);
>>>> + int bo;
>>>> +
>>>> + igt_assert((modifier && stride) || (!modifier && !stride));
>>>> +
>>>> + if (modifier) {
>>>> + unsigned size, calculated_stride;
>>>> +
>>>> + igt_calc_fb_size(fd, width, height, bpp, modifier, &size,
>>>> + &calculated_stride);
>>>> + if (stride == 0)
>>>> + stride = calculated_stride;
>>>> +
>>>> + if (is_dumb)
>>>> + *is_dumb = false;
>>>> +
>>>> + if (is_i915_device(fd)) {
>>>> +
>>>> + bo = gem_create(fd, size);
>>>> + gem_set_tiling(fd, bo, modifier, stride);
>>>
>>>
>>> This is broken, gem_set_tiling does not take a fb modifier but an object
>>> tiling mode.
>>>
>>> You can demonstrate the failure if you got a Skylake system with:
>>>
>>> tests/kms_flip_tiling --r flip-changes-tiling-Yf
>>
>> Hi,
>>
>> that subtest fails occasionally here due to CRC issues, but the one
>> that fails due to the tiling constant passed to gem_set_tiling is
>> flip-Yf-tiled.
>>
>> Have fixed it by converting the modifier to a tiling mode, but I also
>> needed to make sure that we don't pass to the kernel the Yf or Ys
>> constants as the kernel doesn't know about those.
>>
>> Both fixes have been pushed already.
>
> With the two patches you pushed flip-changes-tiling-Yf is still broken
> due the tiling mode mismatch, not the CRC.
>
> Perhaps you tested with all three patches you posted today?
Just before pushing I tested with this:
IGT-Version: 1.16-g050c00d (x86_64) (Linux: 4.5.5-300.fc24.x86_64 x86_64)
So I can only think of a difference in the hw causing a different
codepath to execute (it's a i3-6100U), or a difference in the kernel.
This is a remote box and I don't have yet all the infrastructure in
place so I could monitor the boot, nor power cycle it, so I cannot test
with a newer kernel yet.
If you can confirm that we should be passing I915_TILING_NONE to
set_tiling when the FB has I915_FORMAT_MOD_Yf_TILED, I can fix that, but
I would also like to understand why the test is failing for you.
Regards,
Tomeu
>>> I would like to be able to tell you that all subtests there should pass,
>>> since they have been at some point, but I have a feeling that there are
>>> other breakages affecting it these days. :(
>>
>> Yeah, at least I can say that they are passing now in this particular
>> machine (i3-6100U).
>>
>> To make such issues less likely in the future, I have sent a patch
>> that makes clear when a variable contains a fb modifier constant, and
>> when a tiling mode. Haven't pushed it because I'm not 100% sure it's
>> worth it, so I would appreciate any opinions.
>
> I will look at that one later.
>
> Regards,
>
> Tvrtko
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
