Hi Tim,
What's the goal behind the split. Please add a couple of words in the
commit message.
On 31 July 2017 at 20:40, Tim Rowley <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 :-)
> - 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>
> --- /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
> +*
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.
> --- 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.
-Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
