Re: [i-g-t PATCH v1 08/14] lib: Add igt_create_bo_with_dimensions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/11/2016 11:23, Tomeu Vizoso wrote:
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.

It should definitely be I915_TILING_NONE. If you look at the i915 code, intel_display.c/intel_framebuffer_init will reject attempts to add a framebuffer where object tiling does not match the framebuffer modifier. And the intel_fb_modifier_to_tiling helper translates the Yf/Ys modifiers to NONE tiling.

So I have no idea how it could have possibly worked for you. :)

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux