[Mesa-dev] [PATCH 4/4] main: Use a derived value for the default sample count
Previously the framebuffer default sample count was taken directly from the value given by the application. On the i965 driver on HSW if the value wasn't one that is supported by the hardware it would hit an assert when it tried to program the state for it. This patch fixes it by adding a derived sample count to the state for the default framebuffer. The driver can then quantize this to one of the valid values in its UpdateState handler when the _NEW_BUFFERS state changes. _mesa_geometric_samples is changed to use the new derived value. Fixes the piglit test arb_framebuffer_no_attachments-query Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93957 Cc: Ilia Mirkin--- src/mesa/drivers/dri/i965/brw_context.c | 19 +++ src/mesa/main/framebuffer.h | 3 ++- src/mesa/main/mtypes.h | 4 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c index 1032e5a..44d2fe4 100644 --- a/src/mesa/drivers/dri/i965/brw_context.c +++ b/src/mesa/drivers/dri/i965/brw_context.c @@ -167,6 +167,19 @@ intel_viewport(struct gl_context *ctx) } static void +intel_update_framebuffer(struct gl_context *ctx, + struct gl_framebuffer *fb) +{ + struct brw_context *brw = brw_context(ctx); + + /* Quantize the derived default number of samples +*/ + fb->DefaultGeometry._NumSamples = + intel_quantize_num_samples(brw->intelScreen, + fb->DefaultGeometry.NumSamples); +} + +static void intel_update_state(struct gl_context * ctx, GLuint new_state) { struct brw_context *brw = brw_context(ctx); @@ -245,6 +258,12 @@ intel_update_state(struct gl_context * ctx, GLuint new_state) } _mesa_lock_context_textures(ctx); + + if (new_state & _NEW_BUFFERS) { + intel_update_framebuffer(ctx, ctx->DrawBuffer); + if (ctx->DrawBuffer != ctx->ReadBuffer) + intel_update_framebuffer(ctx, ctx->ReadBuffer); + } } #define flushFront(screen) ((screen)->image.loader ? (screen)->image.loader->flushFrontBuffer : (screen)->dri2.loader->flushFrontBuffer) diff --git a/src/mesa/main/framebuffer.h b/src/mesa/main/framebuffer.h index ab077ed..fa434d4 100644 --- a/src/mesa/main/framebuffer.h +++ b/src/mesa/main/framebuffer.h @@ -97,7 +97,8 @@ static inline GLuint _mesa_geometric_samples(const struct gl_framebuffer *buffer) { return buffer->_HasAttachments ? - buffer->Visual.samples : buffer->DefaultGeometry.NumSamples; + buffer->Visual.samples : + buffer->DefaultGeometry._NumSamples; } static inline GLuint diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 58064aa..745eeb8 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -3223,6 +3223,10 @@ struct gl_framebuffer struct { GLuint Width, Height, Layers, NumSamples; GLboolean FixedSampleLocations; + /* Derived from NumSamples by the driver so that it can choose a valid + * value for the hardware. + */ + GLuint _NumSamples; } DefaultGeometry; /** \name Drawing bounds (Intersection of buffer size and scissor box) -- 2.5.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/4] main: Use a derived value for the default sample count
On Thu, Feb 4, 2016 at 11:17 AM, Ilia Mirkinwrote: > On Thu, Feb 4, 2016 at 11:12 AM, Neil Roberts wrote: >> Previously the framebuffer default sample count was taken directly >> from the value given by the application. On the i965 driver on HSW if >> the value wasn't one that is supported by the hardware it would hit an >> assert when it tried to program the state for it. This patch fixes it >> by adding a derived sample count to the state for the default >> framebuffer. The driver can then quantize this to one of the valid >> values in its UpdateState handler when the _NEW_BUFFERS state changes. >> _mesa_geometric_samples is changed to use the new derived value. >> >> Fixes the piglit test arb_framebuffer_no_attachments-query >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93957 >> Cc: Ilia Mirkin >> --- >> src/mesa/drivers/dri/i965/brw_context.c | 19 +++ >> src/mesa/main/framebuffer.h | 3 ++- >> src/mesa/main/mtypes.h | 4 >> 3 files changed, 25 insertions(+), 1 deletion(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_context.c >> b/src/mesa/drivers/dri/i965/brw_context.c >> index 1032e5a..44d2fe4 100644 >> --- a/src/mesa/drivers/dri/i965/brw_context.c >> +++ b/src/mesa/drivers/dri/i965/brw_context.c >> @@ -167,6 +167,19 @@ intel_viewport(struct gl_context *ctx) >> } >> >> static void >> +intel_update_framebuffer(struct gl_context *ctx, >> + struct gl_framebuffer *fb) >> +{ >> + struct brw_context *brw = brw_context(ctx); >> + >> + /* Quantize the derived default number of samples >> +*/ > > Not a lot of effort either way, but this only needs to be done if > !fb->_HasAttachments right? BTW, whether you choose to condition it on _HasAttachments or not, the whole series is Reviewed-by: Ilia Mirkin > >> + fb->DefaultGeometry._NumSamples = >> + intel_quantize_num_samples(brw->intelScreen, >> + fb->DefaultGeometry.NumSamples); >> +} >> + >> +static void >> intel_update_state(struct gl_context * ctx, GLuint new_state) >> { >> struct brw_context *brw = brw_context(ctx); >> @@ -245,6 +258,12 @@ intel_update_state(struct gl_context * ctx, GLuint >> new_state) >> } >> >> _mesa_lock_context_textures(ctx); >> + >> + if (new_state & _NEW_BUFFERS) { >> + intel_update_framebuffer(ctx, ctx->DrawBuffer); >> + if (ctx->DrawBuffer != ctx->ReadBuffer) >> + intel_update_framebuffer(ctx, ctx->ReadBuffer); >> + } >> } >> >> #define flushFront(screen) ((screen)->image.loader ? >> (screen)->image.loader->flushFrontBuffer : >> (screen)->dri2.loader->flushFrontBuffer) >> diff --git a/src/mesa/main/framebuffer.h b/src/mesa/main/framebuffer.h >> index ab077ed..fa434d4 100644 >> --- a/src/mesa/main/framebuffer.h >> +++ b/src/mesa/main/framebuffer.h >> @@ -97,7 +97,8 @@ static inline GLuint >> _mesa_geometric_samples(const struct gl_framebuffer *buffer) >> { >> return buffer->_HasAttachments ? >> - buffer->Visual.samples : buffer->DefaultGeometry.NumSamples; >> + buffer->Visual.samples : >> + buffer->DefaultGeometry._NumSamples; >> } >> >> static inline GLuint >> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h >> index 58064aa..745eeb8 100644 >> --- a/src/mesa/main/mtypes.h >> +++ b/src/mesa/main/mtypes.h >> @@ -3223,6 +3223,10 @@ struct gl_framebuffer >> struct { >> GLuint Width, Height, Layers, NumSamples; >> GLboolean FixedSampleLocations; >> + /* Derived from NumSamples by the driver so that it can choose a valid >> + * value for the hardware. >> + */ >> + GLuint _NumSamples; >> } DefaultGeometry; >> >> /** \name Drawing bounds (Intersection of buffer size and scissor box) >> -- >> 2.5.0 >> ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/4] main: Use a derived value for the default sample count
On Thu, Feb 4, 2016 at 11:12 AM, Neil Robertswrote: > Previously the framebuffer default sample count was taken directly > from the value given by the application. On the i965 driver on HSW if > the value wasn't one that is supported by the hardware it would hit an > assert when it tried to program the state for it. This patch fixes it > by adding a derived sample count to the state for the default > framebuffer. The driver can then quantize this to one of the valid > values in its UpdateState handler when the _NEW_BUFFERS state changes. > _mesa_geometric_samples is changed to use the new derived value. > > Fixes the piglit test arb_framebuffer_no_attachments-query > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93957 > Cc: Ilia Mirkin > --- > src/mesa/drivers/dri/i965/brw_context.c | 19 +++ > src/mesa/main/framebuffer.h | 3 ++- > src/mesa/main/mtypes.h | 4 > 3 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_context.c > b/src/mesa/drivers/dri/i965/brw_context.c > index 1032e5a..44d2fe4 100644 > --- a/src/mesa/drivers/dri/i965/brw_context.c > +++ b/src/mesa/drivers/dri/i965/brw_context.c > @@ -167,6 +167,19 @@ intel_viewport(struct gl_context *ctx) > } > > static void > +intel_update_framebuffer(struct gl_context *ctx, > + struct gl_framebuffer *fb) > +{ > + struct brw_context *brw = brw_context(ctx); > + > + /* Quantize the derived default number of samples > +*/ Not a lot of effort either way, but this only needs to be done if !fb->_HasAttachments right? > + fb->DefaultGeometry._NumSamples = > + intel_quantize_num_samples(brw->intelScreen, > + fb->DefaultGeometry.NumSamples); > +} > + > +static void > intel_update_state(struct gl_context * ctx, GLuint new_state) > { > struct brw_context *brw = brw_context(ctx); > @@ -245,6 +258,12 @@ intel_update_state(struct gl_context * ctx, GLuint > new_state) > } > > _mesa_lock_context_textures(ctx); > + > + if (new_state & _NEW_BUFFERS) { > + intel_update_framebuffer(ctx, ctx->DrawBuffer); > + if (ctx->DrawBuffer != ctx->ReadBuffer) > + intel_update_framebuffer(ctx, ctx->ReadBuffer); > + } > } > > #define flushFront(screen) ((screen)->image.loader ? > (screen)->image.loader->flushFrontBuffer : > (screen)->dri2.loader->flushFrontBuffer) > diff --git a/src/mesa/main/framebuffer.h b/src/mesa/main/framebuffer.h > index ab077ed..fa434d4 100644 > --- a/src/mesa/main/framebuffer.h > +++ b/src/mesa/main/framebuffer.h > @@ -97,7 +97,8 @@ static inline GLuint > _mesa_geometric_samples(const struct gl_framebuffer *buffer) > { > return buffer->_HasAttachments ? > - buffer->Visual.samples : buffer->DefaultGeometry.NumSamples; > + buffer->Visual.samples : > + buffer->DefaultGeometry._NumSamples; > } > > static inline GLuint > diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h > index 58064aa..745eeb8 100644 > --- a/src/mesa/main/mtypes.h > +++ b/src/mesa/main/mtypes.h > @@ -3223,6 +3223,10 @@ struct gl_framebuffer > struct { > GLuint Width, Height, Layers, NumSamples; > GLboolean FixedSampleLocations; > + /* Derived from NumSamples by the driver so that it can choose a valid > + * value for the hardware. > + */ > + GLuint _NumSamples; > } DefaultGeometry; > > /** \name Drawing bounds (Intersection of buffer size and scissor box) > -- > 2.5.0 > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev