From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH 1/3] drm/i915: BUG_ON() when cmd/reg tables are not sorted Date: Wed, 02 Apr 2014 12:11:37 +0300 Message-ID: <87sipwayeu.fsf@intel.com> References: <1395945820-20376-1-git-send-email-bradley.d.volkin@intel.com> <1395945820-20376-2-git-send-email-bradley.d.volkin@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 28C126EA01 for ; Wed, 2 Apr 2014 02:11:08 -0700 (PDT) In-Reply-To: <1395945820-20376-2-git-send-email-bradley.d.volkin@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: bradley.d.volkin@intel.com, intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 27 Mar 2014, bradley.d.volkin@intel.com wrote: > From: Brad Volkin > > As suggested during review, this makes it much more obvious > when the tables are not sorted. Let's look at the optimizations later. Reviewed-by: Jani Nikula > > Cc: Jani Nikula > Signed-off-by: Brad Volkin > --- > drivers/gpu/drm/i915/i915_cmd_parser.c | 31 +++++++++++++++++++++---------- > 1 file changed, 21 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c > index 788bd96..8a93db3 100644 > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c > @@ -493,12 +493,13 @@ static u32 gen7_blt_get_cmd_length_mask(u32 cmd_header) > return 0; > } > > -static void validate_cmds_sorted(struct intel_ring_buffer *ring) > +static bool validate_cmds_sorted(struct intel_ring_buffer *ring) > { > int i; > + bool ret = true; > > if (!ring->cmd_tables || ring->cmd_table_count == 0) > - return; > + return true; > > for (i = 0; i < ring->cmd_table_count; i++) { > const struct drm_i915_cmd_table *table = &ring->cmd_tables[i]; > @@ -510,35 +511,45 @@ static void validate_cmds_sorted(struct intel_ring_buffer *ring) > &table->table[i]; > u32 curr = desc->cmd.value & desc->cmd.mask; > > - if (curr < previous) > + if (curr < previous) { > DRM_ERROR("CMD: table not sorted ring=%d table=%d entry=%d cmd=0x%08X prev=0x%08X\n", > ring->id, i, j, curr, previous); > + ret = false; > + } > > previous = curr; > } > } > + > + return ret; > } > > -static void check_sorted(int ring_id, const u32 *reg_table, int reg_count) > +static bool check_sorted(int ring_id, const u32 *reg_table, int reg_count) > { > int i; > u32 previous = 0; > + bool ret = true; > > for (i = 0; i < reg_count; i++) { > u32 curr = reg_table[i]; > > - if (curr < previous) > + if (curr < previous) { > DRM_ERROR("CMD: table not sorted ring=%d entry=%d reg=0x%08X prev=0x%08X\n", > ring_id, i, curr, previous); > + ret = false; > + } > > previous = curr; > } > + > + return ret; > } > > -static void validate_regs_sorted(struct intel_ring_buffer *ring) > +static bool validate_regs_sorted(struct intel_ring_buffer *ring) > { > - check_sorted(ring->id, ring->reg_table, ring->reg_count); > - check_sorted(ring->id, ring->master_reg_table, ring->master_reg_count); > + return check_sorted(ring->id, ring->reg_table, ring->reg_count) && > + check_sorted(ring->id, ring->master_reg_table, > + ring->master_reg_count); > } > > /** > @@ -617,8 +628,8 @@ void i915_cmd_parser_init_ring(struct intel_ring_buffer *ring) > BUG(); > } > > - validate_cmds_sorted(ring); > - validate_regs_sorted(ring); > + BUG_ON(!validate_cmds_sorted(ring)); > + BUG_ON(!validate_regs_sorted(ring)); > } > > static const struct drm_i915_cmd_descriptor* > -- > 1.8.3.2 > -- Jani Nikula, Intel Open Source Technology Center