On 17/08/16 12:35, Tvrtko Ursulin wrote:
> On 17/08/16 12:24, Goel, Akash wrote:
[snip]
>> Won't the 2nd memcpy (from the copy on stack to the relay buffer) be
>> really fast ?
>> The copy on stack (16 bytes) will most likely be in the CPU cache and
>> the same area is used for all 3 buffer types.
>
> Yes I realized later you use it more in later patches.
>
> I don't think it is slow but was just wondering if it could be made
> tidier by getting rid of one copy.
>
> Would have to apply the series to see how the final loop looks like, but
> would something like the below be possible:
>
> if (!log_buffer_snapshot_state) {
> .. read_ptr update from wc ..
> continue;
> }
>
> memcpy from wc to final buffer
>
> .. the rest of processing you do reading from the copy ..
>
> ?
Not even compile tested and possibly incorrectly refactored,
but what do you think of:
static bool
guc_log_check_overflow(struct intel_guc *guc,
enum guc_log_buffer_type type,
struct guc_log_buffer *snapshot)
{
unsigned int full_cnt, prev_full_cnt;
bool overflow = false;
guc->log.flush_count[type]++;
full_cnt = snapshot.buffer_full_cnt;
prev_full_cnt = guc->log.prev_overflow_count[type];
if (full_cnt != prev_full_cnt) {
overflow = true;
guc->log.prev_overflow_count[type] = full_cnt;
guc->log.total_overflow_count[type] += full_cnt - prev_full_cnt;
if (full_cnt < prev_full_cnt) {
/* buffer_full_cnt is a 4 bit counter */
guc->log.total_overflow_count[type] += 16;
}
}
return overflow;
}
static void guc_read_update_log_buffer(struct intel_guc *guc)
{
struct guc_log_buffer *log_buffer, *snapshot;
void *src_data, *dst_data;
enum guc_log_buffer_type type;
if (WARN_ON(!guc->log.buf_addr))
return;
/* Get the pointer to shared GuC log buffer */
log_buffer = src_data = guc->log.buf_addr;
/* Get the pointer to local buffer to store the logs */
dst_data = snapshot = guc_get_write_buffer(guc);
/* Actual logs are present from the 2nd page */
src_data += PAGE_SIZE;
dst_data += PAGE_SIZE;
for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER;
type++, log_buffer++) {
unsigned int buffer_size, bytes_to_copy;
unsigned int read_offset, write_offset;
bool new_overflow;
/* Clear the 'flush to file' flag */
log_buffer->flush_to_file = 0;
if (unlikely(!snapshot)) {
/* Update the read pointer in the shared log buffer */
log_buffer->read_ptr = log_buffer.sampled_write_ptr;
continue;
}
memcpy(snapshot, &log_buffer, sizeof(*snapshot));
buffer_size = guc_get_log_buffer_size(type);
read_offset = snapshot.read_ptr;
write_offset = snapshot.sampled_write_ptr;
/* The write pointer could have been updated by the GuC
* firmware, after sending the flush interrupt to Host,
* for consistency set the write pointer value to same
* value of sampled_write_ptr in the snapshot buffer.
*/
snapshot->write_ptr = write_offset;
new_overflow = guc_log_check_overflow(guc, type, snapshot);
if (unlikely(new_overflow)) {
DRM_ERROR_RATELIMITED("GuC log buffer overflow\n");
/* copy the whole buffer in case of overflow */
read_offset = 0;
write_offset = buffer_size;
} else if (unlikely((read_offset > buffer_size) ||
(write_offset > buffer_size))) {
DRM_ERROR("invalid log buffer state\n");
/* copy whole buffer as offsets are unreliable */
read_offset = 0;
write_offset = buffer_size;
}
/* Just copy the newly written data */
if (read_offset > write_offset) {
bytes_to_copy = buffer_size - read_offset;
i915_memcpy_from_wc(dst_data, src_data, write_offset);
} else {
bytes_to_copy = write_offset - read_offset;
}
i915_memcpy_from_wc(dst_data + read_offset,
src_data + read_offset, bytes_to_copy);
src_data += buffer_size;
dst_data += buffer_size;
/* Update the read pointer in the shared log buffer */
log_buffer->read_ptr = snapshot.sampled_write_ptr;
snapshot++;
}
if (snapshot) {
guc_move_to_next_buf(guc);
} else {
/* Used rate limited to avoid deluge of messages, logs might be
* getting consumed by User at a slow rate.
*/
DRM_ERROR_RATELIMITED("no sub-buffer to capture log buffer\n");
guc->log.capture_miss_count++;
}
}
Removes the double memcpy and shortens some variable names in order
to make the whole thing more readable.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
