Date: Mon, 27 May 2013 15:23:11 GMT From: Philip Withnall <prw35@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 229059 for review Message-ID: <201305271523.r4RFNBm8029295@skunkworks.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@229059?ac=10 Change 229059 by prw35@pwithnall_zenith on 2013/05/27 15:23:08 compositor: Optimise compositor_send_command() Eliminate some redundant bus reads and inline the function. This commit also fixes GetStatistics. Affected files ... .. //depot/projects/ctsrd/cheribsd/src/sys/dev/cheri/compositor/cheri_compositor_reg.c#3 edit Differences ... ==== //depot/projects/ctsrd/cheribsd/src/sys/dev/cheri/compositor/cheri_compositor_reg.c#3 (text+ko) ==== @@ -182,7 +182,7 @@ ((uint64_t) (address) & 0xfff) << 46 | \ ((uint64_t) (entry) & 0x3fffffffffff)) #define PAYLOAD_GET_STATISTICS(page) \ - ((uint64_t) (page) & 0x3) + ((uint64_t) (page) & 0x7) #define PAYLOAD_CONTROL_STATISTICS(reset, isPaused) \ (((uint64_t) (reset) & 0x1) << 1 | \ ((uint64_t) (isPaused) & 0x1)) @@ -203,14 +203,14 @@ uint64_t payload; } __packed; -static void +static inline void compositor_send_command(struct cheri_compositor_softc *sc, const struct compositor_command *command, - volatile struct compositor_response *response) + volatile struct compositor_response *response, + boolean_t expecting_response_payload) { const uint32_t *command_raw = (const uint32_t *) command; volatile uint32_t *response_raw = (volatile uint32_t *) response; - unsigned int i; CHERI_COMPOSITOR_ASSERT_LOCKED(sc); @@ -236,31 +236,31 @@ /* FIXME: Tidy up endianness; h64tole, le64toh etc. */ - /* Poll for the response. FIXME: This should use interrupts. - * Bail if this takes more than 100 attempts. */ - i = 0; - do { - CHERI_COMPOSITOR_DEBUG(sc, "Checking for response…"); + /* Read the response. The hardware guarantees that this is always + * available on the next clock cycle. */ + CHERI_COMPOSITOR_DEBUG(sc, "Checking for response…"); + + response_raw[0] = + bus_read_8(sc->compositor_reg_res, + CHERI_COMPOSITOR_RESPONSE_HEADER) & 0xffffffff; - response_raw[0] = - bus_read_8(sc->compositor_reg_res, - CHERI_COMPOSITOR_RESPONSE_HEADER) & 0xffffffff; + if (expecting_response_payload) { + /* This should get optimised if !expecting_response_payload. */ response->payload = bus_read_8(sc->compositor_reg_res, CHERI_COMPOSITOR_RESPONSE_PAYLOAD); + } - i++; - } while ((response->header.seq_num == 0 || - response->header.seq_num != command->header.seq_num) && - i <= 20); - - /* Timeout? */ - if (i >= 20) { +#if defined(CHERI_COMPOSITOR_DEBUG_VERBOSE) && CHERI_COMPOSITOR_DEBUG_VERBOSE + /* Error? */ + if (response->header.seq_num == 0 || + response->header.seq_num != command->header.seq_num) { CHERI_COMPOSITOR_ERROR(sc, - "Aborted sending compositor command due to timeout."); + "Aborted sending compositor command due to error."); } CHERI_COMPOSITOR_DEBUG(sc, "Acknowledging response."); +#endif /* Acknowledge the response. */ bus_write_8(sc->compositor_reg_res, @@ -930,7 +930,7 @@ command.payload = PAYLOAD_ALLOCATE_CFB(width_tiles, height_tiles, allocated_tiles_base); - compositor_send_command(sc, &command, &response); + compositor_send_command(sc, &command, &response, false); if (response.header.status != STATUS_SUCCESS) { retval = EIO; @@ -1047,7 +1047,7 @@ y * (MAX_X_RES / TILE_SIZE) + x /* address */, TILE_CACHE_ENTRY(is_opaque, tile_x_offset, tile_y_offset, allocated_tiles_base, width, height)); - compositor_send_command(sc, &command, &response); + compositor_send_command(sc, &command, &response, false); retval = (response.header.status == STATUS_SUCCESS) ? 0 : EIO; @@ -1631,7 +1631,7 @@ command.payload = PAYLOAD_SET_CONFIGURATION(configuration->x_resolution, configuration->y_resolution); - compositor_send_command(sc, &command, &response); + compositor_send_command(sc, &command, &response, false); if (response.header.status == STATUS_SUCCESS) { /* Update our copies in memory. */ @@ -1815,7 +1815,7 @@ command.header.seq_num = sc->seq_num; command.header.opcode = OPCODE_CONTROL_STATISTICS; command.payload = PAYLOAD_CONTROL_STATISTICS(0, 1); - compositor_send_command(sc, &command, &response); + compositor_send_command(sc, &command, &response, false); if (response.header.status != STATUS_SUCCESS) { retval = EIO; @@ -1858,7 +1858,7 @@ command.header.seq_num = sc->seq_num; command.header.opcode = OPCODE_CONTROL_STATISTICS; command.payload = PAYLOAD_CONTROL_STATISTICS(0, 0); - compositor_send_command(sc, &command, &response); + compositor_send_command(sc, &command, &response, false); if (response.header.status != STATUS_SUCCESS) { retval = EIO; @@ -1902,7 +1902,7 @@ command.header.seq_num = sc->seq_num; command.header.opcode = OPCODE_CONTROL_STATISTICS; command.payload = PAYLOAD_CONTROL_STATISTICS(1, sc->is_sampler_paused); - compositor_send_command(sc, &command, &response); + compositor_send_command(sc, &command, &response, false); if (response.header.status != STATUS_SUCCESS) { retval = EIO; @@ -1951,16 +1951,16 @@ command.header.seq_num = sc->seq_num; command.header.opcode = OPCODE_GET_STATISTICS; command.payload = PAYLOAD_GET_STATISTICS(page); - compositor_send_command(sc, &command, &response); + compositor_send_command(sc, &command, &response, true); if (response.header.status != STATUS_SUCCESS) { retval = EIO; goto done; } - /* Copy out the values. */ - val1 = (response.payload >> 32) & 0xffff; - val2 = (response.payload >> 0) & 0xffff; + /* Copy out the values. Each is 29b long. */ + val1 = (response.payload >> 32) & 0x1fffffff; + val2 = (response.payload >> 0) & 0x1fffffff; done: *val1_out = val1;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201305271523.r4RFNBm8029295>