[Mesa-dev] [PATCH 03/12] i965: Configure bufmgr debug options from intel_screen.c

[Kristian Høgsberg]

On Thu, Oct 8, 2015 at 6:52 AM, Iago Toral <itoral at igalia.com> wrote:
> On Thu, 2015-10-08 at 10:07 +0300, Pohjolainen, Topi wrote:
>> On Wed, Oct 07, 2015 at 07:11:43AM -0700, Kristian H?gsberg Kristensen wrote:
>> > We need the debug flag parsing and INTEL_DEBUG in the compiler, but we
>> > don't want the dependency on bufmgr (libdrm_intel) in there. Move to
>> > intel_screen.c.
>> >
>> > Signed-off-by: Kristian Høgsberg Kristensen <krh at bitplanet.net>
>> > ---
>> >  src/mesa/drivers/dri/i965/intel_debug.c  | 14 +-------------
>> >  src/mesa/drivers/dri/i965/intel_debug.h  |  4 +---
>> >  src/mesa/drivers/dri/i965/intel_screen.c | 14 +++++++++++++-
>> >  3 files changed, 15 insertions(+), 17 deletions(-)
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/intel_debug.c b/src/mesa/drivers/dri/i965/intel_debug.c
>> > index 3120189..f7c02c8 100644
>> > --- a/src/mesa/drivers/dri/i965/intel_debug.c
>> > +++ b/src/mesa/drivers/dri/i965/intel_debug.c
>> > @@ -92,22 +92,10 @@ intel_debug_flag_for_shader_stage(gl_shader_stage stage)
>> >  }
>> >
>> >  void
>> > -brw_process_intel_debug_variable(struct intel_screen *screen)
>> > +brw_process_intel_debug_variable(void)
>> >  {
>> >     uint64_t intel_debug = parse_debug_string(getenv("INTEL_DEBUG"), debug_control);
>> >     (void) p_atomic_cmpxchg(&INTEL_DEBUG, 0, intel_debug);
>> > -
>> > -   if (INTEL_DEBUG & DEBUG_BUFMGR)
>> > -      dri_bufmgr_set_debug(screen->bufmgr, true);
>> > -
>> > -   if ((INTEL_DEBUG & DEBUG_SHADER_TIME) && screen->devinfo->gen < 7) {
>> > -      fprintf(stderr,
>> > -              "shader_time debugging requires gen7 (Ivybridge) or better.\n");
>> > -      INTEL_DEBUG &= ~DEBUG_SHADER_TIME;
>> > -   }
>> > -
>> > -   if (INTEL_DEBUG & DEBUG_AUB)
>> > -      drm_intel_bufmgr_gem_set_aub_dump(screen->bufmgr, true);
>> >  }
>> >
>> >  /**
>> > diff --git a/src/mesa/drivers/dri/i965/intel_debug.h b/src/mesa/drivers/dri/i965/intel_debug.h
>> > index b7d0c82..0a6e1b9 100644
>> > --- a/src/mesa/drivers/dri/i965/intel_debug.h
>> > +++ b/src/mesa/drivers/dri/i965/intel_debug.h
>> > @@ -115,8 +115,6 @@ extern uint64_t INTEL_DEBUG;
>> >
>> >  extern uint64_t intel_debug_flag_for_shader_stage(gl_shader_stage stage);
>> >
>> > -struct intel_screen;
>> > -
>> > -extern void brw_process_intel_debug_variable(struct intel_screen *);
>> > +extern void brw_process_intel_debug_variable(void);
>> >
>> >  extern bool brw_env_var_as_boolean(const char *var_name, bool default_value);
>> > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
>> > index 1783835..590c45d 100644
>> > --- a/src/mesa/drivers/dri/i965/intel_screen.c
>> > +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>> > @@ -1421,7 +1421,19 @@ __DRIconfig **intelInitScreen2(__DRIscreen *psp)
>> >     if (!intelScreen->devinfo)
>> >        return false;
>> >
>> > -   brw_process_intel_debug_variable(intelScreen);
>> > +   brw_process_intel_debug_variable();
>>
>> This is the only caller for brw_process_intel_debug_variable(). Are you
>> planning to use it from somewhere else or could we just move the two lines
>> left in it directly here?
>
> Nothing else in this series will call that function so I think it is
> probably okay to just move the two remaining lines here and remove the
> function.
>
> With that change (or an argument against) this is
> Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

I agree that the split is a little funny and I was on the fence about
what to do here. In the end, I left the tiny function in intel_debug.c
so I wouldn't have to export the 'debug_control' variable outside
intel_debug.c.

Kristian

>> > +
>> > +   if (INTEL_DEBUG & DEBUG_BUFMGR)
>> > +      dri_bufmgr_set_debug(intelScreen->bufmgr, true);
>> > +
>> > +   if ((INTEL_DEBUG & DEBUG_SHADER_TIME) && intelScreen->devinfo->gen < 7) {
>> > +      fprintf(stderr,
>> > +              "shader_time debugging requires gen7 (Ivybridge) or better.\n");
>> > +      INTEL_DEBUG &= ~DEBUG_SHADER_TIME;
>> > +   }
>> > +
>> > +   if (INTEL_DEBUG & DEBUG_AUB)
>> > +      drm_intel_bufmgr_gem_set_aub_dump(intelScreen->bufmgr, true);
>> >
>> >     intelScreen->hw_must_use_separate_stencil = intelScreen->devinfo->gen >= 7;
>> >
>> > --
>> > 2.4.3
>> >
>> > _______________________________________________
>> > mesa-dev mailing list
>> > mesa-dev at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>