[Intel-gfx] [PATCH 1/9] drm/i915/cmdparser: Make initialisation failure non-fatal

[Chris Wilson]

On Mon, Aug 15, 2016 at 12:56:44PM +0100, Matthew Auld wrote:
> On 12 August 2016 at 16:07, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > If the developer adds a register in the wrong order, we BUG during boot.
> > That makes development and testing very difficult. Let's be a bit more
> > friendly and disable the command parser with a big warning if the tables
> > are invalid.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_cmd_parser.c | 30 ++++++++++++++++++------------
> >  drivers/gpu/drm/i915/i915_drv.h        |  2 +-
> >  drivers/gpu/drm/i915/intel_engine_cs.c |  6 ++++--
> >  3 files changed, 23 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > index a1f4683f5c35..1882dc28c750 100644
> > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > @@ -746,17 +746,15 @@ static void fini_hash_table(struct intel_engine_cs *engine)
> >   * Optionally initializes fields related to batch buffer command parsing in the
> >   * struct intel_engine_cs based on whether the platform requires software
> >   * command parsing.
> > - *
> > - * Return: non-zero if initialization fails
> >   */
> > -int intel_engine_init_cmd_parser(struct intel_engine_cs *engine)
> > +void intel_engine_init_cmd_parser(struct intel_engine_cs *engine)
> >  {
> >         const struct drm_i915_cmd_table *cmd_tables;
> >         int cmd_table_count;
> >         int ret;
> >
> >         if (!IS_GEN7(engine->i915))
> > -               return 0;
> > +               return;
> >
> >         switch (engine->id) {
> >         case RCS:
> > @@ -811,24 +809,32 @@ int intel_engine_init_cmd_parser(struct intel_engine_cs *engine)
> >                 break;
> >         default:
> >                 MISSING_CASE(engine->id);
> > -               BUG();
> > +               return;
> >         }
> >
> > -       BUG_ON(!validate_cmds_sorted(engine, cmd_tables, cmd_table_count));
> > -       BUG_ON(!validate_regs_sorted(engine));
> > +       if (!hash_empty(engine->cmd_hash)) {
> > +               DRM_DEBUG_DRIVER("%s: no commands?\n", engine->name);
> > +               return;
> > +       }
> "no commands?", !hash_empty should mean we already have commands, not
> that we don't, right?

Oh, it should be "inited twice?"; return;

We can just kill it since we should catch that error earlier and now the
command parser is initialised from the common engine setup the error is
even less likely to ever be hit.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre