[Mesa-dev] [PATCH] anv/device: Add limits for InterpolationOffset

[Jason Ekstrand]

On Thu, Aug 11, 2016 at 10:47 AM, Anuj Phogat <anuj.phogat at gmail.com> wrote:

> On Fri, Jul 29, 2016 at 2:30 PM, Anuj Phogat <anuj.phogat at gmail.com>
> wrote:
> >
> >
> > On Fri, Jul 29, 2016 at 12:32 PM, Anuj Phogat <anuj.phogat at gmail.com>
> wrote:
> >>
> >>
> >>
> >> On Thu, Jul 28, 2016 at 6:31 PM, Jason Ekstrand <jason at jlekstrand.net>
> >> wrote:
> >>>
> >>> On Jul 28, 2016 7:37 PM, "Anuj Phogat" <anuj.phogat at gmail.com> wrote:
> >>> >
> >>> > Fixes the vulkan cts regression in test
> >>> > dEQP-VK.api.info.device.properties
> >>> >
> >>> > Cc: Mark Janes <mark.a.janes at intel.com>
> >>> > Cc: Jason Ekstrand <jason at jlekstrand.net>
> >>> > Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> >>> > ---
> >>> >  src/intel/vulkan/anv_device.c | 6 +++---
> >>> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >>> >
> >>> > diff --git a/src/intel/vulkan/anv_device.c
> >>> > b/src/intel/vulkan/anv_device.c
> >>> > index c2c5153..9fd9683 100644
> >>> > --- a/src/intel/vulkan/anv_device.c
> >>> > +++ b/src/intel/vulkan/anv_device.c
> >>> > @@ -524,9 +524,9 @@ void anv_GetPhysicalDeviceProperties(
> >>> >        .maxTexelOffset                           = 7,
> >>> >        .minTexelGatherOffset                     = -8,
> >>> >        .maxTexelGatherOffset                     = 7,
> >>> > -      .minInterpolationOffset                   = 0, /* FIXME */
> >>> > -      .maxInterpolationOffset                   = 0, /* FIXME */
> >>> > -      .subPixelInterpolationOffsetBits          = 0, /* FIXME */
> >>> > +      .minInterpolationOffset                   = -0.5,
> >>> > +      .maxInterpolationOffset                   = 0.5,
> >
> > Correction: maxInterpolationOffset = 0.4375
> > From Vulkan 1.0 spec:
> > "
> > If subPixelInterpolationOffsetBits is 4, this provides increments of
> (1/24)
> > = 0.0625, and thus the range of supported interpolation offsets would be
> > [-0.5, 0.4375].
> > "
> Jason, other than above correction, is there anything else you want me
> to address in V2 of this patch ?
>

No, I think what you have is fine.  Assuming that's the only change, v2
will be

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


> >>>
> >>> > +      .subPixelInterpolationOffsetBits          = 4,
> >>>
> >>> I looked and neither gen7_pipeline.c nor gen8_pipeline.c set the
> subpixel
> >>> precision so we get a default of enum value 0 which is 8 bits.  We
> should
> >>> probably be explicitly setting it to something.  I think 4 is probably
> fine
> >>> but we should be seeing it.
> >>
> >> I think you're talking about "Vertex Sub Pixel Precision Select" in
> >> 3DSTATE_SF. Yes, we are setting a default value of zero which is 8 bits
> in
> >> both OpenGL and Vulkan. This meets the minimum requirement of 4 bits for
> >> GL_SUBPIXEL_BITS. Yes, it'll be nicer to make it more obvious what we
> are
> >> setting here.
> >>
> >> The value we are defining in this patch is
> subPixelInterpolationOffsetBits
> >> (GL_FRAGMENT_INTERPOLATION_OFFSET_BITS in OpenGL). I found no option of
> >> setting it on Intel Hardware. I just replicated what we do in i965 and
> made
> >> the CTS test happy again.
> >>
> >> Looks unrelated but there is this another thing 'viewportSubPixelBits'
> >> which is initialized to 13 in Vulkan but 0 (required min. value) in
> i965.
> >> Again found no option of setting on Intel hardware.So, a similar case to
> >> subPixelInterpolationOffsetBits.
> >>>
> >>> As a side note, I think we could probably combine 3DSTATE_SF and
> >>> 3DSTATE_RASTER setup between gen7 and gen8 (and move it to
> >>> genX_pipeline_until.h).  We'd probably have to do some #define trick
> like we
> >>> did for 3DSTATE_SBE and 3DSTATE_SBE_SWIZ because the two packets are
> >>> combined on gen7 and split on gen8.
> >>
> >> I'll look in to it.
> >>
> >>>
> >>> >        .maxFramebufferWidth                      = (1 << 14),
> >>> >        .maxFramebufferHeight                     = (1 << 14),
> >>> >        .maxFramebufferLayers                     = (1 << 10),
> >>> > --
> >>> > 2.5.5
> >>> >
> >>
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160812/0a04cddb/attachment.html>