On Jul 31, 2017, at 3:18 PM, Emil Velikov <emil.l.veli...@gmail.com<mailto:emil.l.veli...@gmail.com>> wrote:
Hi Tim, What's the goal behind the split. Please add a couple of words in the commit message. Will do. On 31 July 2017 at 20:40, Tim Rowley <timothy.o.row...@intel.com<mailto:timothy.o.row...@intel.com>> wrote: --- src/gallium/drivers/swr/Makefile.am | 3 +- src/gallium/drivers/swr/SConscript | 4 +- .../drivers/swr/rasterizer/codegen/gen_knobs.py | 14 +- .../swr/rasterizer/codegen/templates/gen_knobs.cpp | 112 +--------------- .../swr/rasterizer/codegen/templates/gen_knobs.h | 147 +++++++++++++++++++++ .../drivers/swr/rasterizer/core/knobs_init.h | 12 +- 6 files changed, 166 insertions(+), 126 deletions(-) create mode 100644 src/gallium/drivers/swr/rasterizer/codegen/templates/gen_knobs.h diff --git a/src/gallium/drivers/swr/Makefile.am b/src/gallium/drivers/swr/Makefile.am index 73fe904..b20f128 100644 --- a/src/gallium/drivers/swr/Makefile.am +++ b/src/gallium/drivers/swr/Makefile.am @@ -115,7 +115,7 @@ rasterizer/codegen/gen_knobs.cpp: rasterizer/codegen/gen_knobs.py rasterizer/cod --output rasterizer/codegen/gen_knobs.cpp \ --gen_cpp -rasterizer/codegen/gen_knobs.h: rasterizer/codegen/gen_knobs.py rasterizer/codegen/knob_defs.py rasterizer/codegen/templates/gen_knobs.cpp rasterizer/codegen/gen_common.py +rasterizer/codegen/gen_knobs.h: rasterizer/codegen/gen_knobs.py rasterizer/codegen/knob_defs.py rasterizer/codegen/templates/gen_knobs.h rasterizer/codegen/gen_common.py $(MKDIR_GEN) $(PYTHON_GEN) \ $(srcdir)/rasterizer/codegen/gen_knobs.py \ @@ -347,5 +347,6 @@ EXTRA_DIST = \ rasterizer/codegen/templates/gen_builder.hpp \ rasterizer/codegen/templates/gen_header_init.hpp \ rasterizer/codegen/templates/gen_knobs.cpp \ + rasterizer/codegen/templates/gen_knobs.h \ rasterizer/codegen/templates/gen_llvm.hpp \ rasterizer/codegen/templates/gen_rasterizer.cpp diff --git a/src/gallium/drivers/swr/SConscript b/src/gallium/drivers/swr/SConscript index a32807d..b394cbc 100644 --- a/src/gallium/drivers/swr/SConscript +++ b/src/gallium/drivers/swr/SConscript @@ -53,8 +53,8 @@ env.CodeGenerate( source = '', command = python_cmd + ' $SCRIPT --output $TARGET --gen_h' ) -Depends('rasterizer/codegen/gen_knobs.cpp', Seems like this should have been gen_knobs.h in the first place - oops :-) Yep, noticed that bug when updating the rule - I’ve pulled the fix into a separate commit. - swrroot + 'rasterizer/codegen/templates/gen_knobs.cpp') +Depends('rasterizer/codegen/gen_knobs.h', + swrroot + 'rasterizer/codegen/templates/gen_knobs.h') The build bits are Reviewed-by: Emil Velikov <emil.veli...@collabora.com<mailto:emil.veli...@collabora.com>> --- /dev/null +++ b/src/gallium/drivers/swr/rasterizer/codegen/templates/gen_knobs.h @@ -0,0 +1,147 @@ +/****************************************************************************** +* +* Copyright 2015-2017 +* Intel Corporation +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http ://www.apache.org/licenses/LICENSE-2.0<http://www.apache.org/licenses/LICENSE-2.0> +* I'm not a lawyer so I'm not sure if having Apache licensed code is fine with rest of Mesa. Considering that rest of SWR (barring the original gen_knobs.cpp where this is comes from) uses MIT X11/Expat I'd stay consistent and re-license this/these files. If possible, of course. Adding files with another license was unintentional - I’ve added a relicense commit prior to the split. --- a/src/gallium/drivers/swr/rasterizer/core/knobs_init.h +++ b/src/gallium/drivers/swr/rasterizer/core/knobs_init.h @@ -91,16 +91,18 @@ static inline void ConvertEnvToKnob(const char* pOverride, std::string& knobValu template <typename T> static inline void InitKnob(T& knob) { - - // TODO, read registry first - - // Second, read environment variables + // Read environment variables const char* pOverride = getenv(knob.Name()); if (pOverride) { - auto knobValue = knob.Value(); + auto knobValue = knob.DefaultValue(); ConvertEnvToKnob(pOverride, knobValue); knob.Value(knobValue); } + else + { + // Set default value + knob.Value(knob.DefaultValue()); This and the underlying code seems to have changed a bit. Would be nice to keep "dummy split" and functionality changes as separate patches. Then again: it's not my code, so please don't read too much into my suggestion. I’ve unwoven this commit into five commits for the upcoming v2 of the patchset: commit 566ea1983277bf62f07ea02571854009b667081f Author: Tim Rowley <timothy.o.row...@intel.com<mailto:timothy.o.row...@intel.com>> Date: Mon Jul 31 17:22:54 2017 -0500 swr/rast: simplify knob default value setup commit f83d47cd01d987600b59106828bb75c672ea610c Author: Tim Rowley <timothy.o.row...@intel.com<mailto:timothy.o.row...@intel.com>> Date: Mon Jul 31 17:22:12 2017 -0500 swr/rast: split gen_knobs templates into .h/.cpp Switch to a 1:1 mapping template:generated for future maintenance. commit 51af078588be9e5f25d8f7d31135cbdf0e526c37 Author: Tim Rowley <timothy.o.row...@intel.com<mailto:timothy.o.row...@intel.com>> Date: Mon Jul 31 17:01:54 2017 -0500 swr/rast: gen_knobs template code style commit fdb5c7018c159d31233e452250938afca7add189 Author: Tim Rowley <timothy.o.row...@intel.com<mailto:timothy.o.row...@intel.com>> Date: Mon Jul 31 17:48:12 2017 -0500 swr/rast: switch gen_knobs.cpp license Unintentionally added with an apache2 license; relicense to match the rest of the tree. commit 5191465819bb9c860a805b7163fe2f2ca2b49620 Author: Tim Rowley <timothy.o.row...@intel.com<mailto:timothy.o.row...@intel.com>> Date: Mon Jul 31 16:59:06 2017 -0500 swr/rast: fix scons gen_knobs.h dependency Copy/paste error was duplicating a gen_knobs.cpp rule. -Emil
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
