Bug 94442

Summary: Add ARB_vertex_attrib_64bit support for the i965 backend
Product: Mesa Reporter: Alejandro Piñeiro (freenode IRC: apinheiro) <apinheiro>
Component: Drivers/DRI/i965Assignee: Alejandro Piñeiro (freenode IRC: apinheiro) <apinheiro>
Status: RESOLVED FIXED QA Contact: Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity: normal    
Priority: medium CC: agomez, jajcus, jason, jasuarez, notasas
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Bug Depends on: 92760    
Bug Blocks:    

Description Alejandro Piñeiro (freenode IRC: apinheiro) 2016-03-08 11:30:50 UTC
This issue is to track progress for the implementation of ARB_vertex_attrib_64bit support for the i965 backend.

https://www.opengl.org/registry/specs/ARB/vertex_attrib_64bit.txt

This extension is already supported on softpipe and llvmpipe, so the GL API is already in place. There are also piglit tests for this extension, but they will be extended if needed.
Comment 1 Alejandro Piñeiro (freenode IRC: apinheiro) 2016-03-08 11:35:52 UTC
Forgot to mention that this work depends on FP64 support for i965, that is it still not in master. In any case, the implementation for that support is mostly finished, and they are already sending patches to the list to review, so it is possible to start the work on this extension, based on the private branches that implements FP64.

Setting dependency with bug 92760.
Comment 2 Grazvydas Ignotas 2016-04-08 22:31:25 UTC
As suggested by Iago on bug 92760 , I ran a bisect on my Skylake machine on today's i965-attrib64 and it points to this:

commit 2cb3aec172550912eb75bca2074f7c7a1ae6206f
Author: Juan A. Suarez Romero <jasuarez@igalia.com>
Date:   Mon Apr 4 12:47:57 2016 +0200

    arb_vertex_attrib_64bit: use attribute slots instead of attribute


This commit introduces XOrg font rendering issues.
Comment 3 Alejandro Piñeiro (freenode IRC: apinheiro) 2016-04-09 17:45:34 UTC
(In reply to Grazvydas Ignotas from comment #2)
> As suggested by Iago on bug 92760 , I ran a bisect on my Skylake machine on
> today's i965-attrib64 and it points to this:
> 
> commit 2cb3aec172550912eb75bca2074f7c7a1ae6206f
> Author: Juan A. Suarez Romero <jasuarez@igalia.com>
> Date:   Mon Apr 4 12:47:57 2016 +0200
> 
>     arb_vertex_attrib_64bit: use attribute slots instead of attribute
> 
> 
> This commit introduces XOrg font rendering issues.

Thanks for the feedback!

As Iago said on bug 92760 this branch is still a WIP, so this kind of regressions are expected. Having said so, we will take your discovery into account, and check again when we got a code we are happy with.

Again, thanks for the feedback.
Comment 4 Alejandro Piñeiro (freenode IRC: apinheiro) 2016-04-11 08:03:48 UTC
BTW, making the most of comment 2 and comment 3, and as we have been working some time now, here an update.

We have a functional implementation of vertex_attrib_64bit for gen8. If someone is curious:

https://github.com/Igalia/mesa/tree/i965-attrib64

Right now it passing all the piglit tests for that extension. That includes those already included on piglit master, plus those that we wrote while working on this extension, and that we also plan to send for review:

https://github.com/Igalia/piglit/tree/i965-attrib64

In any case, as mentioned on comment 3, they are still wip, and not ready for sending to review. We are still testing and finding corner cases to test. And as comment 2 points, there is a regression with Xorg font rendering.
Comment 5 Juan A. Suarez 2016-04-14 10:31:16 UTC
(In reply to Grazvydas Ignotas from comment #2)
> As suggested by Iago on bug 92760 , I ran a bisect on my Skylake machine on
> today's i965-attrib64 and it points to this:
> 
> commit 2cb3aec172550912eb75bca2074f7c7a1ae6206f
> Author: Juan A. Suarez Romero <jasuarez@igalia.com>
> Date:   Mon Apr 4 12:47:57 2016 +0200
> 
>     arb_vertex_attrib_64bit: use attribute slots instead of attribute
> 
> 
> This commit introduces XOrg font rendering issues.


While this is still a WIP, we pushed several commits that addresses some bugs we found. Probably some of these commits could fix the font rendering issues.

Do you mind to test it again? I'd appreciate very much.
Comment 6 Grazvydas Ignotas 2016-04-14 23:17:26 UTC
(In reply to Juan A. Suarez from comment #5)
> 
> Do you mind to test it again? I'd appreciate very much.

Seems to be fine now.
Comment 7 Alejandro Piñeiro (freenode IRC: apinheiro) 2016-04-15 08:02:48 UTC
(In reply to Grazvydas Ignotas from comment #6)
> (In reply to Juan A. Suarez from comment #5)
> > 
> > Do you mind to test it again? I'd appreciate very much.
> 
> Seems to be fine now.

Thanks for confirming.
Comment 8 Alejandro Piñeiro (freenode IRC: apinheiro) 2016-04-27 11:02:36 UTC
New update. 

The support for this extension is mostly finished for gen8. We are just doing some cleanings, but the functionality would remain the same:
https://github.com/Igalia/mesa/tree/i965-attrib64

We also have a branch that provides the support for gen7:
https://github.com/Igalia/mesa/tree/i965-attrib64-gen7

Having said so, there is still a possibility of the fp64 support for gen7 changing in the future, so it is possible that attrib64 support for gen7 would need to be adapted for it.

In both cases, there are no regressions doing a full piglit run, neither on gles3/gles31 dEQP.

Additionally, we also worked on extending the tests specific for this extension on piglit:
https://github.com/Igalia/piglit/tree/i965-attrib64

For those new tests, there are some failing ones, but they are known cases, as some of those tests use more attributes that the hw can handle.
Comment 9 Antia Puentes 2016-05-02 10:33:57 UTC
We have sent to the mailing list the series implementing the support for this extension on i965 gen8+ scalar backend.

You can find the thread here:
https://lists.freedesktop.org/archives/mesa-dev/2016-April/114871.html

Notice that this series is on top of the fp64 work (for scalar gen8+) that is being tracked on bug 92760.
Comment 10 Alejandro Piñeiro (freenode IRC: apinheiro) 2016-05-17 11:12:47 UTC
(In reply to Antia Puentes from comment #9)
> We have sent to the mailing list the series implementing the support for
> this extension on i965 gen8+ scalar backend.
> 
> You can find the thread here:
> https://lists.freedesktop.org/archives/mesa-dev/2016-April/114871.html
> 
> Notice that this series is on top of the fp64 work (for scalar gen8+) that
> is being tracked on bug 92760.

Most of the patches (except two) got a Rb. We have just pushed them.

In the following days we will send to review the piglit series, and send an alternative version for the NAK'ed patches.
Comment 11 Alejandro Piñeiro (freenode IRC: apinheiro) 2016-09-28 13:06:41 UTC
(In reply to Alejandro Piñeiro (freenode IRC: apinheiro) from comment #10)
> (In reply to Antia Puentes from comment #9)
> > We have sent to the mailing list the series implementing the support for
> > this extension on i965 gen8+ scalar backend.
> > 
> > You can find the thread here:
> > https://lists.freedesktop.org/archives/mesa-dev/2016-April/114871.html
> > 
> > Notice that this series is on top of the fp64 work (for scalar gen8+) that
> > is being tracked on bug 92760.
> 
> Most of the patches (except two) got a Rb. We have just pushed them.
> 
> In the following days we will send to review the piglit series, and send an
> alternative version for the NAK'ed patches.

FWIW, and just in case someone is wondering, all that it's done. gen8+ has support for this extension (and allowed to expose OpenGL 4.2 at that moment).

The bug is still open because there is an ongoing work to get this extension supported on gen6/gen7. Both are blocked by the fp64 work. In any case gen7 support is really advanced.
Comment 12 Samuel Iglesias Gonsálvez 2017-06-22 08:19:48 UTC
ARB_vertex_attrib_64bit support was already added to both i965's vec4 and scalar backends in mesa master. Closing the bug.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.