Just some style comments, feel free to ignore them.

On 20/07/17 12:35, aravindan.muthuku...@intel.com wrote:
From: Aravindan Muthukumar <aravindan.muthuku...@intel.com>

This patch improves CPI Rate(Cycles per Instruction)
and branch mispredict for i965. The function check_state()
was showing CPI retired rate.

Performance stats with android:
CPI retired lowered by 28% (lower is better)
Branch missprediction lowered by 13% (lower is better)
3DMark improved by 2%

The dissassembly doesn't show difference, although above
results were observed with patch.

Signed-off-by: Aravindan Muthukumar <aravindan.muthuku...@intel.com>
Signedd-off-by: Yogesh Marathe <yogesh.mara...@intel.com>
Tested-by: Asish <as...@intel.com>
---

Changes since V1:
- Removed memset() change
- Changed commit message as per review comments

  src/mesa/drivers/dri/i965/brw_defines.h      |  4 ++++
  src/mesa/drivers/dri/i965/brw_state_upload.c | 12 ++++++++----
  2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h 
b/src/mesa/drivers/dri/i965/brw_defines.h
index 2a8dbf8..8c9a510 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -1687,3 +1687,7 @@ enum brw_pixel_shader_coverage_mask_mode {
  # define CSDBG2_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE (1 << 4)
#endif
+
+/* Checking the state of mesa and brw before emitting atoms */
+#define CHECK_BRW_STATE(a,b) ((a.mesa & b.mesa) | (a.brw & b.brw))
+
diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c 
b/src/mesa/drivers/dri/i965/brw_state_upload.c
index acaa97e..1c8b969 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -443,10 +443,8 @@ check_and_emit_atom(struct brw_context *brw,
                      struct brw_state_flags *state,
                      const struct brw_tracked_state *atom)
  {
-   if (check_state(state, &atom->dirty)) {
        atom->emit(brw);
        merge_ctx_state(brw, state);
-   }

You might want to re-indent this.
Also maybe that function can be rename since it won't check anything anymore.

  }
static inline void
@@ -541,7 +539,10 @@ brw_upload_pipeline_state(struct brw_context *brw,
         const struct brw_tracked_state *atom = &atoms[i];
         struct brw_state_flags generated;
- check_and_emit_atom(brw, &state, atom);
+         /* Checking the state and emitting atoms */
+         if (CHECK_BRW_STATE(state, atom->dirty)) {
+            check_and_emit_atom(brw, &state, atom);
+         }
accumulate_state(&examined, &atom->dirty); @@ -558,7 +559,10 @@ brw_upload_pipeline_state(struct brw_context *brw,
        for (i = 0; i < num_atoms; i++) {
         const struct brw_tracked_state *atom = &atoms[i];
- check_and_emit_atom(brw, &state, atom);
+         /* Checking the state and emitting atoms */
+         if (CHECK_BRW_STATE(state, atom->dirty)) {
+            check_and_emit_atom(brw, &state, atom);
+         }
        }
     }


Why not replacing the last call to check_state() by CHECK_BRW_STATE() and get rid of that function altogether?

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to