Skip to content
Snippets Groups Projects

Delete Mesa Classic

Merged Dylan Baker requested to merge dbaker/mesa:submit/delete-classic into main

As proposed, here is the concrete removal of the Mesa classic drivers and infrastructure. I've tested with Meson and things continue to compile tests pass. I have tried to break the commits up into logical chunks, removing one driver at a time, then common code, then libmesa classic itself.

There are the following additional changes

  • move the dri megadriver code to gallium/auxilliary. I'm not sure where else to put it, it's used to two gallium targets
  • remove the core mesa dispatch sanity test. This would require a pretty substantial rework to still be useful, as the code that it uses for setup (but not the code it tests) is being deleted. I suspect that the code tested will be refactored out in the near future, so that work wouldn't be that useful.
  • remove classic xlib. I know that the gallium-xlib code isn't quite as good as classic xlib, but it's not a super common feature anyway.]

I have moved the PCI IDs that both iris and i965 supports into the iris pci_ids header, and removed them from i965. I have left the actual header in place, along with the rest of the Intel GFX version 4-7.5 code, as I assume that the crocus effort can use that.

I've done my best to keep the Android.mk working, but I'm sure I've missed something.

Finally, if there are any requests to re-author any patches, please let me know and I'll re-author them.

This should not land before the 21.1 branch point, but shortly after instead.

Edited by Dylan Baker

Merge request reports

Checking pipeline status.

Approval is optional

Merged by Marge BotMarge Bot 3 years ago (Dec 4, 2021 12:40am UTC)

Merge details

  • Changes merged into main with ea8fa10e.
  • Deleted the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Author Developer

    I should add, as a follow up to this I'd like to move src/mesa into src/gallium/frontends, but there's additional pain there, especially in the mesa <-> glsl exchange. I have a branch I've started hacking on if anyone is interested

  • Dylan Baker changed the description

    changed the description

  • I've done my best to keep the Android.mk working, but I'm sure I've missed something.

    That's funny, the rest of us are trying our best to keep breaking Android.mk :upside_down: !9728 (merged)

    • remove classic xlib. I know that the gallium-xlib code isn't quite as good as classic xlib, but it's not a super common feature anyway.

      Enh. The due diligence in !7464 (closed) about trying to get to parity is maybe a bit hard-headed. The only real regressions in the glx sense are dealing with the front buffer interactions, which is pretty far into Doctor It Hurts When I Do This territory. And the rendering regressions from swrastc to llvmpipe are mostly the kinds of weird corners that even hardware drivers get wrong.

      I do want to see a few more of the checkboxes in !8015 (closed) addressed before this lands though. The biggest issue is getting the xserver / 2d driver to select the right glvnd vendor for the screen when it's using an LTS-branch Mesa. I think @kbrenneman had a need to get this out of EGL as well for multigpu cases. And, we should pick a vendor name for that branch that distros can default to so main and LTS mesas can easily parallel install.

    • Do you mean figuring out a vendor name to send back for a GLX_VENDOR_NAMES_EXT query?

    • Yes. Shouldn't be much more complicated than querying it out of EGL and using that for the GLX vendor name too, I think.

    • That should work, yes. I'm working on an EGL extension that would let an EGL client look up a vendor name from an EGLDeviceEXT that would correlate to GLX.

      GLX_VENDOR_NAMES_EXT is also allowed to be a space-separated list of names. So, even without an EGL extension, the server could send back both versions of Mesa, and then libglvnd will use the first one that it can load and which claims to support that X screen.

    • Author Developer

      What's the status of this?

    • The new EGL extension was checked in a few days ago. The new query that matters here is the EGL_DRIVER_NAME_EXT string for eglQueryDeviceStringEXT:

      https://www.khronos.org/registry/EGL/extensions/EXT/EGL_EXT_device_persistent_id.txt

    • @kbrenneman Mesa's driver name isn't the same as its glvnd vendor name though.

    • EGL_DRIVER_NAME_EXT isn't the name of an internal driver, it's the name of a vendor library. Its purpose is to disambiguate device UUID's, and on libglvnd-based systems, to correlate to the GLX vendor name.

      Issues 2 and 3 in the extension spec go into more detail on that.

    • Please register or sign in to reply
  • Dylan Baker added 111 commits

    added 111 commits

    • d1d35246...544044b2 - 100 commits from branch mesa:master
    • 1d40e59a - classic/r100: Delete driver
    • d6073409 - classic/r200: Delete driver
    • fc6b7f47 - classic/nouveau: Remove driver
    • daff86ba - classic/i915: Remove driver
    • 89dcd2fa - classic/i965: Remove driver
    • 60910eb7 - include/pci_ids: Move PCI ids supported by both i965 and iris to iris
    • 923da478 - mesa: move dri_common to gallium
    • bdf82226 - xlib: delete classic xlib
    • b0786bbf - mesa/main/tests: remove dispatch sanity
    • 9c37940f - classic: delete common code
    • 659fad55 - mesa: Delete libmesa_classic

    Compare with previous version

    • Hi,

      I just wanted to know if i915g (gallium) is considered a modern driver and it will be keep on main branch.

      I just compiled mesa 21.0.2 and I had a bug #4637 (closed)

      but I can run glxgears and xmoto. I would like to run piglit or something but it would be nice if you could provide some documentation to get that driver more stable and properly tested.

      thanks :)

      Edited by mercuriete
    • Author Developer

      We don't (currently) have plans to remove i915g. TBH, i915c is a better driver right now, although there's been work going into i915g recently, so it might be better in the future.

    • I'm glad to hear that. thanks :)

    • Please register or sign in to reply
  • Dylan Baker added 373 commits

    added 373 commits

    • 659fad55...7d234da6 - 362 commits from branch mesa:master
    • 4e3377b2 - mesa/x11: Remove the swrast-classic-based fake libGL
    • eafb6faf - classic/r100: Delete driver
    • ad3ed75b - classic/r200: Delete driver
    • fd249029 - classic/nouveau: Remove driver
    • a706af51 - classic/i915: Remove driver
    • 1fa8b1dc - classic/i965: Remove driver
    • c57ba861 - include/pci_ids: Move PCI ids supported by both i965 and iris to iris
    • 84021331 - mesa: move dri_common to gallium
    • d3af0c8f - mesa/main/tests: remove dispatch sanity
    • f309d30f - classic: delete common code
    • 331f9750 - mesa: Delete libmesa_classic

    Compare with previous version

  • added nv-vieux label

  • Dylan Baker added 11 commits

    added 11 commits

    • 6a73a524 - mesa/x11: Remove the swrast-classic-based fake libGL
    • d7d46268 - classic/r100: Delete driver
    • b58fd69e - classic/r200: Delete driver
    • 13735bdb - classic/nouveau: Remove driver
    • c69a3b6a - classic/i915: Remove driver
    • 8ba2d0bb - classic/i965: Remove driver
    • 49998d96 - include/pci_ids: Move PCI ids supported by both i965 and iris to iris
    • 032c6c05 - mesa: move dri_common to gallium
    • bca5143b - mesa/main/tests: remove dispatch sanity
    • 2f163469 - classic: delete common code
    • 9f17bc82 - mesa: Delete libmesa_classic

    Compare with previous version

    • Just a comment on one of the patches.

      Commit mesa/main/tests: remove dispatch sanity reads:

      Thsi test uses a bunch of the classic infastructure, which is about to
      be deleted. Since gallium will be the sole user, it will likely be
      refactored out anyway.

      Unless my grep-foo has failed, from the ~20 function called only one is getting removed - _mesa_init_driver_functions(), which can be replaced with st_init_driver_functions() (didn't test). Everything else is used in mesa/core and/or mesa/st.

      Looking at the bigger picture, this test has prevented regressions in the past and afaict does not impede any refactoring. So I'd suggest fixing the function call and leaving the test in.

      Edit: And yes, the includes used by the test are fairly misleading.

      Edited by Emil Velikov
    • Author Developer

      I guess I can leave it, I just figured getting rid of layer of abstraction that is now useless would be high on @mareko's list of things to delete.

    • I think we need the dispatch sanity test because it's a mapi/glapi test.

      I expect some of the mesa/main abstractions to be reworked regardless of the outcome of this MR, so that we have a more straightforward path to gallium drivers. We wouldn't rework the abstractions just for the sake of doing it. We would only do it where it benefits the majority.

    • Author Developer

      that's actually a pretty non-trivial amount of work, because adding an st requirement means moving the test out of mesa/main due to the gymnastics we're going to get mesa built before st/mesa. It's probably easier to just write mock stub functions that using the st version.

    • Please register or sign in to reply
    • There's another issue we need to sort out before this lands. @shadeslayer just landed !5594 (merged) which adds GL_EXT_external_objects support to i965. The immediate issue here is that it landed after the branch point so, if we throw away i965 now, we'll loose his work. Cherry-picking his MR into LTS shouldn't be much of a problem but there's a bigger issue...

      The way the GL_EXT_external_objects extension works, it assumes that the GL and Vulkan driver come from the same build. More specifically, it assumes that they have exactly the same surface layout code. With a code-base that's constantly in flux, the best way to ensure this is to ensure that they have the same build. If we fork off i965 away from ANV, this lands us in a bit of a pickle. I see a few potential options, each with their own problems:

      1. Leave i965 in the tree for a while. None of us particularly like this plan.

      2. Fork off Gen7 ANV as well. This would mean that Gen7 ANV and i965 build from the same code-base, solving the out-of-sync problem. However, it would also mean that Gen7 ANV would stop getting new features. The only reason why it gets new features today is because it's often trivial (or free) to add the Gen7 code at the same time as we enable it on BDW+. This would also mean we'd end up with the inverse problem as soon as crocus lands.

      3. Leave ANV in master, fork off i965, make the reported driverUUID some fixed value for Gen7 and CHV, and pray ISL never changes on those platforms. If ISL ever did change, we'd have to back-port the change to LTS, update the fixed value, and do an LTS release. We'd also want to make sure GL_EXT_external_objects is only advertised on Gen7 and CHV in i965.

      Thoughts?

      Cc @airlied @imirkin

    • 3 sounds pretty decent to me. I have difficulty imagining a change ISL would need to absorb that would break gen7 to begin with, since definitionally it already works as well as it ever did so there's no reason to change unless somehow gen7 support would also get way better. But, by that point, whatever change that is has probably happened after crocus lands, so just leave LTS alone and ask people to move to crocus in mesa main. EDIT: to be clear: you wouldn't need to do an LTS release, I don't think, because there's no question of interop between gen7's GL and gen8's Vulkan. Unless one of those gens is DG1+, I guess, but... do you really think it'd work if you tried to share between that and a broadwell?

      By the last sentence, you mean i965 in LTS should only advertise GL_EXT_external_objects on those two and not anything newer? Yes, I would agree, I would in fact want that for i965 even without that extension, in the sense that for anything newer the supported driver is iris, thank you, and it's in main not in LTS, so there's no question of what i965 does on gen8 because the driver will refuse to bind to gen8. Similarly LTS's build of anv would refuse to bind.

      Edited by Adam Jackson
    • Please register or sign in to reply
  • Adam Jackson mentioned in merge request !8015 (closed)

    mentioned in merge request !8015 (closed)

  • The i915 driver removal missed removing include/pci_ids/i915_pci_ids.h, though arguably i915g should be using it.

    Edited by Adam Jackson
  • Some related work in !10554 (merged) and !10557 (merged).

  • Roman Stratiienko mentioned in merge request !10183 (merged)

    mentioned in merge request !10183 (merged)

  • Jordan Justen changed target branch from master to main

    changed target branch from master to main

  • Ryan Houdek mentioned in merge request !9868 (merged)

    mentioned in merge request !9868 (merged)

  • Emma Anholt mentioned in merge request !10874 (closed)

    mentioned in merge request !10874 (closed)

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading