From 994d1db079b4947e6f10ab22a4b366a676382345 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 30 Jul 2009 12:32:40 -0700 Subject: i915: Let i915_program_error take a format string, and don't use _mesa_problem. It's misleading to report things like the program having too many native instructions as a Mesa implementation error, when the program may just be too big for the hardware. --- src/mesa/drivers/dri/i915/i915_program.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'src/mesa/drivers/dri/i915/i915_program.c') diff --git a/src/mesa/drivers/dri/i915/i915_program.c b/src/mesa/drivers/dri/i915/i915_program.c index e87700f8e0..85a1b0cf5d 100644 --- a/src/mesa/drivers/dri/i915/i915_program.c +++ b/src/mesa/drivers/dri/i915/i915_program.c @@ -424,12 +424,21 @@ i915_emit_param4fv(struct i915_fragment_program * p, const GLfloat * values) return 0; } - - +/* Warning the user about program errors seems to be quite valuable, from + * our bug reports. It unfortunately means piglit reporting errors + * when we fall back to software due to an unsupportable program, though. + */ void -i915_program_error(struct i915_fragment_program *p, const char *msg) +i915_program_error(struct i915_fragment_program *p, const char *fmt, ...) { - _mesa_problem(NULL, "i915_program_error: %s", msg); + va_list args; + + fprintf(stderr, "i915_program_error: "); + va_start(args, fmt); + vfprintf(stderr, fmt, args); + va_end(args); + + fprintf(stderr, "\n"); p->error = 1; } -- cgit v1.2.3 From 4ff816751f74b0645c198372937eec589c458a60 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 29 Jul 2009 22:39:15 -0700 Subject: i915: Bail when the fragment program has too many total instructions. Previously, we'd go trashing the heap. --- src/mesa/drivers/dri/i915/i915_program.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'src/mesa/drivers/dri/i915/i915_program.c') diff --git a/src/mesa/drivers/dri/i915/i915_program.c b/src/mesa/drivers/dri/i915/i915_program.c index 85a1b0cf5d..6ccc9eea3e 100644 --- a/src/mesa/drivers/dri/i915/i915_program.c +++ b/src/mesa/drivers/dri/i915/i915_program.c @@ -186,6 +186,11 @@ i915_emit_arith(struct i915_fragment_program * p, p->utemp_flag = old_utemp_flag; /* restore */ } + if (p->csr >= p->program + I915_PROGRAM_SIZE) { + i915_program_error(p, "Program contains too many instructions"); + return UREG_BAD; + } + *(p->csr++) = (op | A0_DEST(dest) | mask | saturate | A0_SRC0(src0)); *(p->csr++) = (A1_SRC0(src0) | A1_SRC1(src1)); *(p->csr++) = (A2_SRC1(src1) | A2_SRC2(src2)); @@ -270,6 +275,11 @@ GLuint i915_emit_texld( struct i915_fragment_program *p, p->register_phases[GET_UREG_NR(coord)] == p->nr_tex_indirect) p->nr_tex_indirect++; + if (p->csr >= p->program + I915_PROGRAM_SIZE) { + i915_program_error(p, "Program contains too many instructions"); + return UREG_BAD; + } + *(p->csr++) = (op | T0_DEST( dest ) | T0_SAMPLER( sampler )); -- cgit v1.2.3 From 61b512c47c9888f3ff117faf3aceccfb52d59c3a Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 29 Jul 2009 23:37:04 -0700 Subject: i915: Update and translate the fragment program along with state updates. Previously, we were doing it in the midst of the pipeline run, which gave an opportunity to enable/disable fallbacks, which is certainly the wrong time to be doing so. This manifested itself in a NULL dereference for PutRow after transitioning out of a fallback during a run_pipeline in glean glsl1. --- src/mesa/drivers/dri/i915/i915_context.c | 3 +++ src/mesa/drivers/dri/i915/i915_fragprog.c | 32 +++++++++++++++++++++---------- src/mesa/drivers/dri/i915/i915_program.c | 2 -- src/mesa/drivers/dri/i915/i915_program.h | 3 +-- 4 files changed, 26 insertions(+), 14 deletions(-) (limited to 'src/mesa/drivers/dri/i915/i915_program.c') diff --git a/src/mesa/drivers/dri/i915/i915_context.c b/src/mesa/drivers/dri/i915/i915_context.c index 3ab7d682ee..b342503772 100644 --- a/src/mesa/drivers/dri/i915/i915_context.c +++ b/src/mesa/drivers/dri/i915/i915_context.c @@ -40,6 +40,7 @@ #include "utils.h" #include "i915_reg.h" +#include "i915_program.h" #include "intel_regions.h" #include "intel_batchbuffer.h" @@ -80,6 +81,8 @@ i915InvalidateState(GLcontext * ctx, GLuint new_state) i915_update_stencil(ctx); if (new_state & (_NEW_LIGHT)) i915_update_provoking_vertex(ctx); + if (new_state & (_NEW_PROGRAM | _NEW_PROGRAM_CONSTANTS)) + i915_update_program(ctx); } diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c b/src/mesa/drivers/dri/i915/i915_fragprog.c index beed8ef197..929a6eb835 100644 --- a/src/mesa/drivers/dri/i915/i915_fragprog.c +++ b/src/mesa/drivers/dri/i915/i915_fragprog.c @@ -1058,6 +1058,28 @@ i915ProgramStringNotify(GLcontext * ctx, _tnl_program_string(ctx, target, prog); } +void +i915_update_program(GLcontext *ctx) +{ + struct intel_context *intel = intel_context(ctx); + struct i915_context *i915 = i915_context(&intel->ctx); + struct i915_fragment_program *fp = + (struct i915_fragment_program *) ctx->FragmentProgram._Current; + + if (i915->current_program != fp) { + if (i915->current_program) { + i915->current_program->on_hardware = 0; + i915->current_program->params_uptodate = 0; + } + + i915->current_program = fp; + } + + if (!fp->translated) + translate_program(fp); + + FALLBACK(&i915->intel, I915_FALLBACK_PROGRAM, fp->error); +} void i915ValidateFragmentProgram(struct i915_context *i915) @@ -1075,16 +1097,6 @@ i915ValidateFragmentProgram(struct i915_context *i915) GLuint s2 = S2_TEXCOORD_NONE; int i, offset = 0; - if (i915->current_program != p) { - if (i915->current_program) { - i915->current_program->on_hardware = 0; - i915->current_program->params_uptodate = 0; - } - - i915->current_program = p; - } - - /* Important: */ VB->AttribPtr[VERT_ATTRIB_POS] = VB->NdcPtr; diff --git a/src/mesa/drivers/dri/i915/i915_program.c b/src/mesa/drivers/dri/i915/i915_program.c index 6ccc9eea3e..fd3019b234 100644 --- a/src/mesa/drivers/dri/i915/i915_program.c +++ b/src/mesa/drivers/dri/i915/i915_program.c @@ -530,8 +530,6 @@ i915_upload_program(struct i915_context *i915, GLuint program_size = p->csr - p->program; GLuint decl_size = p->decl - p->declarations; - FALLBACK(&i915->intel, I915_FALLBACK_PROGRAM, p->error); - /* Could just go straight to the batchbuffer from here: */ if (i915->state.ProgramSize != (program_size + decl_size) || diff --git a/src/mesa/drivers/dri/i915/i915_program.h b/src/mesa/drivers/dri/i915/i915_program.h index 1074d24f9e..0d17d04865 100644 --- a/src/mesa/drivers/dri/i915/i915_program.h +++ b/src/mesa/drivers/dri/i915/i915_program.h @@ -155,7 +155,6 @@ extern void i915_upload_program(struct i915_context *i915, extern void i915_fini_program(struct i915_fragment_program *p); - - +extern void i915_update_program(GLcontext *ctx); #endif -- cgit v1.2.3 From 96a3c69d48bb7c021181e061d010cca08198ae4c Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 30 Jul 2009 00:03:21 -0700 Subject: i915: Increase maximum program size to the hardware limits. This fixes potential heap trashing if the program of choice exceeds limits, and fixes the native instructions limit being lower than what can be used by valid programs. --- src/mesa/drivers/dri/i915/i915_context.h | 15 ++++++++++----- src/mesa/drivers/dri/i915/i915_program.c | 8 ++++++-- 2 files changed, 16 insertions(+), 7 deletions(-) (limited to 'src/mesa/drivers/dri/i915/i915_program.c') diff --git a/src/mesa/drivers/dri/i915/i915_context.h b/src/mesa/drivers/dri/i915/i915_context.h index 8de4a9d0d3..082d614442 100644 --- a/src/mesa/drivers/dri/i915/i915_context.h +++ b/src/mesa/drivers/dri/i915/i915_context.h @@ -121,10 +121,14 @@ enum { #define I915_MAX_CONSTANT 32 #define I915_CONSTANT_SIZE (2+(4*I915_MAX_CONSTANT)) +#define I915_MAX_INSN (I915_MAX_DECL_INSN + \ + I915_MAX_TEX_INSN + \ + I915_MAX_ALU_INSN) -#define I915_PROGRAM_SIZE 192 - -#define I915_MAX_INSN (I915_MAX_TEX_INSN+I915_MAX_ALU_INSN) +/* Maximum size of the program packet, which matches the limits on + * decl, tex, and ALU instructions. + */ +#define I915_PROGRAM_SIZE (I915_MAX_INSN * 3 + 1) /* Hardware version of a parsed fragment program. "Derived" from the * mesa fragment_program struct. @@ -154,8 +158,9 @@ struct i915_fragment_program */ GLcontext *ctx; - GLuint declarations[I915_PROGRAM_SIZE]; - GLuint program[I915_PROGRAM_SIZE]; + /* declarations contains the packet header. */ + GLuint declarations[I915_MAX_DECL_INSN * 3 + 1]; + GLuint program[(I915_MAX_TEX_INSN + I915_MAX_ALU_INSN) * 3]; GLfloat constant[I915_MAX_CONSTANT][4]; GLuint constant_flags[I915_MAX_CONSTANT]; diff --git a/src/mesa/drivers/dri/i915/i915_program.c b/src/mesa/drivers/dri/i915/i915_program.c index fd3019b234..e7908bd48f 100644 --- a/src/mesa/drivers/dri/i915/i915_program.c +++ b/src/mesa/drivers/dri/i915/i915_program.c @@ -130,6 +130,7 @@ i915_emit_decl(struct i915_fragment_program *p, *(p->decl++) = (D0_DCL | D0_DEST(reg) | d0_flags); *(p->decl++) = D1_MBZ; *(p->decl++) = D2_MBZ; + assert(p->decl <= p->declarations + ARRAY_SIZE(p->declarations)); p->nr_decl_insn++; return reg; @@ -186,7 +187,7 @@ i915_emit_arith(struct i915_fragment_program * p, p->utemp_flag = old_utemp_flag; /* restore */ } - if (p->csr >= p->program + I915_PROGRAM_SIZE) { + if (p->csr >= p->program + ARRAY_SIZE(p->program)) { i915_program_error(p, "Program contains too many instructions"); return UREG_BAD; } @@ -275,7 +276,7 @@ GLuint i915_emit_texld( struct i915_fragment_program *p, p->register_phases[GET_UREG_NR(coord)] == p->nr_tex_indirect) p->nr_tex_indirect++; - if (p->csr >= p->program + I915_PROGRAM_SIZE) { + if (p->csr >= p->program + ARRAY_SIZE(p->program)) { i915_program_error(p, "Program contains too many instructions"); return UREG_BAD; } @@ -530,6 +531,9 @@ i915_upload_program(struct i915_context *i915, GLuint program_size = p->csr - p->program; GLuint decl_size = p->decl - p->declarations; + if (p->error) + return; + /* Could just go straight to the batchbuffer from here: */ if (i915->state.ProgramSize != (program_size + decl_size) || -- cgit v1.2.3