[Mesa-dev] [PATCH] Integrate precise trig into configuration infrastructure

[Kenneth Graunke]

On Friday, May 6, 2016 11:53:53 PM PDT Jordan Justen wrote:
> On 2016-04-27 14:52:34, Gurchetan Singh wrote:
> > With this change, to enable precise SIN and COS instructions
> > on Intel hardware, one can put
> > 
> > <option name="intel_precise_trig" value="true"/>
> > 
> > in the proper drirc file.
> > ---
> >  src/mesa/drivers/dri/common/xmlpool/t_options.h | 5 +++++
> >  src/mesa/drivers/dri/i965/brw_compiler.c        | 2 --
> >  src/mesa/drivers/dri/i965/brw_context.c         | 3 +++
> >  src/mesa/drivers/dri/i965/intel_screen.c        | 2 ++
> 
> No apps in mind for src/mesa/drivers/dri/common/drirc?
> 
> >  4 files changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/common/xmlpool/t_options.h b/src/mesa/
drivers/dri/common/xmlpool/t_options.h
> > index e5cbc46..615491a 100644
> > --- a/src/mesa/drivers/dri/common/xmlpool/t_options.h
> > +++ b/src/mesa/drivers/dri/common/xmlpool/t_options.h
> > @@ -158,6 +158,11 @@ DRI_CONF_OPT_BEGIN_B(force_s3tc_enable, def) \
> >          DRI_CONF_DESC(en,gettext("Enable S3TC texture compression even if 
software support is not available")) \
> >  DRI_CONF_OPT_END
> >  
> > +#define DRI_CONF_INTEL_PRECISE_TRIG(def) \
> > +DRI_CONF_OPT_BEGIN_B(intel_precise_trig, def) \
> 
> The other conf options are more generically named, even if they were
> added for hardware specific reasons.
> 
> Example: b3340cd32acf5935891f19833de0cfc500a93e0b
> 
> So, maybe precise_trig instead?
> 
> > +        DRI_CONF_DESC(en,gettext("Minimize maximum error for Intel's SIN 
and COS instructions")) \
> 
> Is the above is changed, then maybe "Prefer accuracy over performance
> in trig functions" instead.
> 
> > +DRI_CONF_OPT_END
> > +
> >  #define DRI_CONF_COLOR_REDUCTION_ROUND 0
> >  #define DRI_CONF_COLOR_REDUCTION_DITHER 1
> >  #define DRI_CONF_COLOR_REDUCTION(def) \
> > diff --git a/src/mesa/drivers/dri/i965/brw_compiler.c b/src/mesa/drivers/
dri/i965/brw_compiler.c
> > index 4496699..8753197 100644
> > --- a/src/mesa/drivers/dri/i965/brw_compiler.c
> > +++ b/src/mesa/drivers/dri/i965/brw_compiler.c
> > @@ -148,8 +148,6 @@ brw_compiler_create(void *mem_ctx, const struct 
brw_device_info *devinfo)
> >     brw_fs_alloc_reg_sets(compiler);
> >     brw_vec4_alloc_reg_set(compiler);
> >  
> > -   compiler->precise_trig = env_var_as_boolean("INTEL_PRECISE_TRIG", 
false);
> > -
> 
> Ken, I take it the dri conf would work fine for you instead of the env
> var?
> 
> -Jordan

Yep, it seems reasonable.  I wasn't sure about putting "intel" in the
name of a drirc option in general code...but I wasn't sure about
generalizing it either.  I like your suggestions.

Thanks for taking care of this, Gurchetan!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160507/6199ad39/attachment.sig>