The calculations of row_pitch, the row pitch's alignment, surface size,
and base_alignment were mixed together. This patch moves the calculation
of row_pitch and its alignment to occur before the calculation of
surface_size and base_alignment.
This simplifies a follow-on patch that adds a new member, 'row_pitch',
to struct isl_surf_init_info.
v2:
- Also extract the row pitch alignment.
- More helper functions that will later help validate the row pitch.
Reviewed-by: Nanley Chery <nanley.g.chery@intel.com> (v2)
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> (v2)
isl has a giant comment that explains the hardware's padding
requirements. (Hint: Cache lines and page faults). But the comment is in
the wrong place, in isl_calc_linear_row_pitch(), which is unrelated to
padding.
The important parts of that comment were copied to
isl_apply_surface_padding() long ago. So drop the misplaced comment.
Reviewed-by: Nanley Chery <nanley.g.chery@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
This was used for aubdumping (deleted a while ago) and INTEL_DEBUG=bat
decoding (deleted recently).
While we're changing parameters, delete the wrapper macro and make the
actual function brw_state_batch instead of __brw_state_batch.
This subsumes a patch by Emil Velikov to drop this from BLORP.
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
This will be used for color output in debug messages.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
When the iterator encounters a structure field, it now looks up the
gen_group for that structure definition and saves a pointer to it.
This lets us drop a lot of ridiculous code in the caller, which looked
at item->value (<struct NAME dword>), strtok'd the structure name back
out, and looked it up itself.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
The iterator code already computed this value, then we stored it in
the structure name, strtok'd it back out, and also manually computed
it when printing dword headers.
Just put the value in the struct and use it. Way simpler.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
It made more sense when decode_group() took a bunch of extra options,
but now that there's only one...we may as well pass 0 and call it a day.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
I added this flag in 65a9d5eabb but
it was completely unused. Both callers appear to have printed dword
headers, so we can just drop the flag and continue doing it
unconditionally.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
When decoding a structure field within a group, we may want to look up
that structure type. Having a gen_spec pointer makes it easy to do so.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
gen_field_iterator_next() produces a string representing the value of
the field. For enum values, it also produced a separate "description"
string containing the textual name of the enum.
The only caller of this function combines the two, printing enums as
"<numeric value> (<texture enum name>)". We may as well just store
that in item->value directly, eliminating the description field, and
a layer of wrapping.
v2: Use non-overlapping source and destination strings in snprintf.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
The PRMs state that this packet is 16 DWORDS long. Ensure that the last
three DWORDS are zeroed as required by the hardware when allocating a
null surface state.
Cc: <mesa-stable@lists.freedesktop.org>
Signed-off-by: Nanley Chery <nanley.g.chery@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Reviewed-by: Anuj Phogat <anuj.phogat@gmail.com>
The crash is due to NULL pColorBlendState, which is legal if the
pipeline has rasterization disabled or if the subpass of the render pass
the pipeline is created against does not use any color attachments.
Test: Sample subpasses from LunarG can run without crash
Signed-off-by: Xu,Randy <randy.xu@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Cc: "17.0 13.0" <mesa-stable@lists.freedesktop.org>
In the end, pipeline statistics queries look a lot like occlusion
queries only with between 1 and 11 begin/end pairs being generated
instead of just the one.
Reviewed-By: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
In order to get accurate statistics, we need to disable statistics for
blits, clears, and the surface state memcpy at the top of each secondary
command buffer. There are two possible approaches to this:
1) Disable before the blit/memcpy and re-enable afterwards
2) Move emitting 3DSTATE_VF_STATISTICS from initialization and make it
part of pipeline state and then just disabale statistics before
blits and memcpy operations.
Emitting 3DSTATE_VF_STATISTICS should be fairly cheap so it doesn't
really matter which path we take. We choose the second option as it's
more consistent with the way the rest of the statistics are enabled and
disabled.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
It's in 3DSTATE_CLIP, so it doesn't really need the extra detail. This
matches what we do for VS, FS, etc.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
The new version is a nice GPU parallel to cpu_write_query_result and it
nicely handles things like dealing with 32 vs. 64-bit offsets in the
destination buffer.
Reviewed-By: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Not all queries are the same. Even the two queries we support today
require a different amount of data per slot. Once we introduce pipeline
statistics queries, the size will vary wildly.
Reviewed-By: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
We're about to make slots variable-length and always having the
available bits at the front makes certain operations substantially
easier once we do that.
Reviewed-By: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
From the Vulkan 1.0.39 Specification:
"If VK_QUERY_RESULT_64_BIT is not set and the result overflows a
32-bit value, the value may either wrap or saturate."
So we can either clamp or wrap. Wrapping is both easier and what the
user gets if they use vkCmdCopyQueryPoolResults and we should be
consistent. We could make vkCmdCopyQueryPoolResults clamp but it's
annoying and ends up burning extra batch for something the spec clearly
doesn't require.
Reviewed-By: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Now that there's a timebase_scale in gen_device_info which is
effectively the 'period' this switches anv_GetPhysicalDeviceProperties
to using this common device info to initialize the timestampPeriod
device limit.
Signed-off-by: Robert Bragg <robert@sixbynine.org>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Prior to Skylake the Gen HW timestamps were driven by a 12.5MHz clock
with the convenient property of being able to scale by an integer (80)
to nanosecond units.
For Skylake the frequency is 12MHz or a scale factor of 83.333333
This updates gen_device_info to track a floating point timebase_scale
factor and makes corresponding _queryobj.c changes to no longer assume a
scale factor of 80 works across all gens.
Although the gen6_ code could have been been left alone, the changes
keep the code more comparable, and it now shares a few utility functions
for scaling raw timestamps and calculating deltas. The utility for
calculating deltas takes into account 32 or 36bit overflow depending on
the current kernel version.
Note: this leaves the timestamp handling of ARB_query_buffer_object
untouched, which continues to use an incorrect scale of 80 on Skylake
for now. This is more awkward to solve since the scaling is currently
done using a very limited uint64 ALU available to the command parser
that doesn't support multiply or divide where it's already taking a
large number of instructions just to effectively multiple by 80.
This fixes piglit arb_timer_query-timestamp-get on Skylake
v2: (Ken) Update timebase_scale for platforms past Skylake/Broxton too.
Signed-off-by: Robert Bragg <robert@sixbynine.org>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Older versions of GCC don't like compound literals in static const
variable declarations because they don't think it's an actual constant
value.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
The Vulkan spec is fairly clear about when we should and should not
write query pool results. We're also supposed to return VK_NOT_READY if
VK_QUERY_RESULT_PARTIAL_BIT is not set and we come across any queries
which are not yet finished. This fixes rendering corruptions on The
Talos Principle where geometry flickers in and out due to bogus query
results being returned by the driver. These issues are most noticable
on Sky Lake GT4 2hen running on "ultra" settings.
Reviewed-By: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100182
Cc: "17.0 13.0" <mesa-stable@lists.freedesktop.org>
There's not much point to having them or not having them but this
reduces some pointless diff from the version we can auto-generate
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
The code for decoding structures and commands was almost identical.
The only differences are: we print dword headers for commands, and
we skip the first one (with the command opcode and lengths).
So, generalize decode_structure to add a starting DWord, and a flag
for printing the DWord headers, and reuse it.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
handle_struct_decode() is just a wrapper around decode_structure()
with a NULL check. But the only caller already does that NULL check.
So, just use decode_structure() directly.
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Specifically, report 'out of memory' errors that might have happened while
emitting the pipeline's batch.
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
These can fail to allocate device memory, however, the driver can recover
from this error by allocating a new binding table block and trying again.
v2:
- Instead of tracking the errors in these functions and making callers
reset the batch's status before attempting to allocate a new block
for the binding table, simply make callers responsible for setting
the error status if they fail to allocate memory during the second
attempt (Jason).
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Also, we had a couple of instances in flush_descriptor_sets() were
we were returning a VkResult directly upon error, but the return
value of this function is not a VkResult but a uint32_t dirty mask,
so simply return 0 in these cases which reduces the amount of
work the driver will do after the error has been raised.
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Instead of asserting inside the function, and then use use that information
to return early from its callers upon failure.
v2:
- Make sure that clear_color_attachment() and
clear_depth_stencil_attachment() get the VkResult as well so they
avoid executing the batch if an error happened. (Topi)
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
Any errors that may have happened during the command buffer recording are
reported by vkEndCommandBuffer() and it is the application's reponsibility
to not submit broken commands to a queue.
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
v2: Assert on secondary commands, applications should've called
vkEndCommandBuffer() and received an error for them before (Jason)
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>
