On Thu, Oct 8, 2015 at 6:52 AM, Iago Toral <ito...@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 <k...@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 <ito...@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@lists.freedesktop.org >> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev