On 12/28/2012 09:57 AM, Mark Zhang wrote:
On 12/28/2012 03:45 PM, Arto Merilainen wrote:
On 12/28/2012 08:47 AM, Mark Zhang wrote:
+
+ /* Add fences */
+ if (num_fences) {
+
+ tegra_stream_push(stream,
+ nvhost_opcode_setclass(NV_HOST1X_CLASS_ID,
+ host1x_uclass_wait_syncpt_r(), num_fences));
+
+ for (; num_fences; num_fences--, fence++) {
+ assert(tegra_fence_is_valid(fence));
This is useless. We already add "1 + num_fences" to num_words above. So
move this "assert" before adding "1 + num_fences" to num_words makes
sense.
The assertion checks the validity of a single fence - not if there is
room in the command buffer.
The goal is to prevent having invalid fences in the command stream. If
this check were not here it would be possible to initialise a fence with
tegra_fence_clear() and put that fence into the stream.
My idea is, if one fence is invalid, then we should not count this in
"num_words". In current code, if one fence is invalid, then this fence
will not be pushed into the command stream, and the "num_words" shows a
wrong command buffer size.
So I think we should:
- validate the fences, remove the invalid fence
- update num_words
- then you don't need to check fence here - I mean, before push a host1x
syncpt wait command into the active buffer of stream.
In my opinion asking tegra_stream_begin() to put a bad fence into the
stream is a case we should never be. assert() kills the application
immediately (in debug builds) and usually this helps the programmer for
1) finding bugs 2) not doing bad code.
"Silencing" is not a good solution especially in this case:
tegra_stream_flush() returns an invalid fence when flushing fails. If
the application chains submits (i.e. do a blit and then do another using
the output of the first blit) it is crucial to be sure the first submit
has been performed before starting the second one.
- Arto
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
