Commit 47f6b1305cc3752f318a555b932e194e1500c1d8 completely broke this
test due to the fread() assertion. When we're reading the debugfs file
we really don't care about how many bytes we read because the number
is not constant and we just use strstr() later. Change the assertion
to make it check for at least 1 byte read, just to make sure no one
changes that again.
Regression introduced by:
commit 47f6b1305cc3752f318a555b932e194e1500c1d8
Author: Thomas Wood <thomas.wood@intel.com>
Date: Wed Mar 25 16:42:57 2015 +0000
igt.cocci: check the return values of various functions
Cc: Thomas Wood <thomas.wood@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
While it is nice to have shorter names for the most-accessed
variables, it makes the code more difficult to read since it's not
clear to the code reader whether that "gem_handle" is from some FB or
something else. The reader also has to audit the code to see if, for
example, the value of data->handle[0] stays consistent with
data->fb[0].gem_handle all the tame or if at some point the value is
replaced with something else. So remove the redundant information,
making it explicit that we're using the gem handles and FB IDs of the
framebuffers all the time.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
The code has a common pattern of "wait 300ms, then check if FBC is
enabled". Most of the time FBC is enabled in either 50ms or 0ms, so
introduce wait_for_fbc_enabled(), which can return much earlier if FBC
is actually enabled before the 300ms timeout.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Debugfs i915_fbc_status shows "FBC unsupported on this chipset"
not "unsupported by this chipset" if the platform doesn't support
FBC feature. That typo will cause case fail on some platforms such
as byt, bsw.
Signed-off-by: Lei Liu <lei.a.liu@intel.com>
Use 1 as the element size to check the number of bytes returned is
greater than 0, rather than checking the number of elements returned.
This fixes a regression from commit 47f6b13 (igt.cocci: check the
return values of various functions).
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89833
Signed-off-by: Thomas Wood <thomas.wood@intel.com>
Add rules to fix unused-result warnings when compiling with
_FORTIFY_SOURCE defined and apply them to the library and tests.
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Thomas Wood <thomas.wood@intel.com>
Tests should positively check for crc matches, not for mismatches.
Enforce this by only exposing and igt_assert function for comparing
crcs.
For the few tests which didn't just do this as consistency checks but
to do functional tests add FIXME comments that some reference crc
values are missing.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Because of hash collisions tests should only ever compare crc
checksums for equality. Checking for inequality can result in random
failures.
To ensure this only expose and igt_assert function and use that.
Follow-up patches will rework the code for tests which don't follow
this requirement and try to compare for CRC inequality.
v2: Rebase on top of Matt's kms_plane changes.
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
This converts the IGT API only, underneath legacy set_tiling is still used.
v2: One got away in kms_flip.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
The problem is that this causes writes to registers, and if the pipe
is off they might go nowhere (e.g. when runtime pm is enabled).
Furthermore we can only really check once the modeset setup is done,
but again most tests set up the CRC structure before calling
igt_commit and friends. We could add crc restore support to the
kernel's rpm code, but that will end up being rather invasive and
fragile hard-to-test code.
Now originally this was needed back when CRC support wasn't available
everywhere. But that's fixed now.
So given all this just drop that sanity check and make sure that we
only touch the debugfs file (and so the hw state) when we know the
pipe is running in the desired configuration.
A complementary kernel patch will try to catch offenders by returning
-EIO if the pipe is off.
v2: Forgot to git add one hunk.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86092
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
These tests require DRM master right, so make sure they have it from the
beginning. This gives an early indication if another DRM master is running
and makes the given test skip (with a proper explanation of the reason)
instead of exiting with error.
Signed-off-by: Imre Deak <imre.deak@intel.com>
Since relocations are variable size, depending upon generation, it is
easier to handle the resizing of the batch request inside the
BEGIN_BATCH macro. This still leaves us with having to resize commands
in a few places - which still need adaption for gen8+.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
This reveal that quite a few locations were writing relocation offsets
but only allowing for 32 bit addresses. To reveal such places in active
tests, we also now double check that we do not use more batch space than
declared.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Damien dodged this ...
Also run the script while at it.
v2: Don't just capture identifiers for pipe, but also expressions.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Switch to XY_COLOR_BLT from COLOR_BLT and use the appropriate
macros to make the code work on BDW.
Also make the blit 8bpp instead if 16bpp. 8bpp is what it was
supposed to use all along.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76307
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
All the cases that simply dump some debug information and couldn't be
converted to some of the fancier macros.
Some information output removed when it's redundant with the subtest
status.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
If a subtest fails it'll leave the display in a state that may prevent
the next subtest from working. So reset the display state between
subtests.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
There's no need to keep an array of pipe_crc objects around. Just keep
one for the duration of the specific crtc/connector/test combo.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Don't skip the entire subtest if FBC only works on some of the primary
planes, as is the case on pre-gen4 and hsw+. Only skip the entire subtest
if all crtc/connector combinations skip.
Also print some kind of status for all otherwise valid crtc/connector combos
if they skip due to FBC being disabled or CRC support not being there.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Shorter and more in line with our general igt_ prefix for everything
which isn't somehow intel or i915-gem or otherwise hw specific - these
helpers here are all fully generic framebuffer handling functions
based on kms + cairo.
Well, the actual buffer alloc is done with i915 gem, but meh ;-)
Two special cases:
- bpp_depth_to_drm_format and drm_format_to_bpp completely lacked
prefixes, so just add igt_.
- write_fb was a bit misleading given that we have gem_write for
uploading to buffers. Rename that to write_fb_to_png to make it
crystal clear what this thing does even without looking at docs.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Use the new-style function using drm fourcc codes instead everywhere.
To easily use thew fourcc based interface also expose
bpp_depth_to_drm_format from the library. Finally include drm_fourcc.h
from the igt_kms.h header since pretty much everyone needs this now.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Step one to properly namespace the rendercpy/mediafill functions. Als
give the buf_height/width helpers a proper igt_ prefix.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
And move the public interfaces into intel_batchbuffer.[hc].
A bit messy since we are fairly inconsistent with our header #include
handling.
Also exclude rendercopy.h from the documentation.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Functions which provide feature checks through igt_skip should be of
the form <prefix>_require_<feature>.
Otoh feature checks which return in a boolean whether the feature is
available should be of the form <prefix>_has_<feature>, e.g.
gem_has_blt.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Also add a missing igt_assert to kms_fbc_crc and again add the missing
Returns: section to the api doc.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
kms_cursor_crc and kms_fbc_crc don't need glib.h. This was just some
copy-paste error on my part.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
cairo_t is the short lived drawing context, whereas cairo_surface_t is
the heavyweight object that persists and is also tied to underlying GEM
objects. So make the kmstest API reflect the different weights and fix
the lifetime and underlying object reference leaks.
Based on the fix by Paulo Zanoni.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
No need to duplicate this all over the place.
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Or more precisely: It already has an igt_require. So we cant ditch it
from tests.
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
kms_fbc_crc will perform various write operations to the scanout buffer
whilc FBC is enabled. CRC checks will be used to make sure the
modifcations to scanout buffer are detected.
The operations include:
- page flip
- GTT mmap
- CPU mmap
- blit
- rendercopy
- context switch + rendercopy
- combination of a page flip and each operation listed above
v2: Use gem_sw_finish instead of drmModeDirtyFB after CPU access
v3: Drop pwrite tests, call gem_bo_busy() after rendering, drop
set_domain() calls after mmap access, wait for 2 vblanks
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
