nv50,nvc0: Use u_pipe_screen_get_param_defaults
authorAlyssa Rosenzweig <alyssa@collabora.com>
Fri, 31 Mar 2023 00:31:20 +0000 (20:31 -0400)
committerMarge Bot <emma+marge@anholt.net>
Thu, 6 Apr 2023 14:13:00 +0000 (14:13 +0000)
commit2242bb703ecaa46e2cda65cba2d0f66e58929a4a
tree18d530cca16430b3edb6bbed4cef88f209c28003
parentfe60fa898e61c0bd4ddc408709700814a073d562
nv50,nvc0: Use u_pipe_screen_get_param_defaults

Other than nouveau, every single Gallium driver relies on
u_pipe_screen_get_param_defaults to get the default values of CAPs.

For the driver, this is much more concise. Unsupported new features -- or
supported features that virtually all Gallium drivers support -- do not need to
written out explicitly. Their absence (or presence) is implied as the default.
If there's any doubt over whether the CAP is exposed, it's easy to check in
u_pipe_screen_get_param_defaults.

For the Gallium tree in general, this brings a number of benefits:

* Unused CAPs are easy to delete, because there is only a single place
  (u_pipe_screen_get_param_defaults) where they are referenced and need to be
  deleted from.

* New CAPs are easy to introduce, for the same reason.

* It's straightforward to audit which drivers support (or don't support) a given
  CAP by grepping for the name (for example, when determining whether a CAP is
  unused and can be garbage collected, or a CAP is so widely supported that it
  can be made default.). You still need to check the source code in case it's
  conditionally exposed (common for layered drivers) but the search space is
  limited to drivers that reference the CAP by name.

Unfortunately, all of these benefits rely on all Gallium drivers cooperating.
The status quo is much less nice:

* Unused CAPs need to be deleted both from common code, and also specially from
  nouveau. Why is nouveau special?

* New CAPs need to be added both to common code, and also specially to nouveau.
  Again, why is nouveau special?

* When grepping for CAPs, nouveau (only) needs to be ignored, since it's
  spurious.  Unless sometimes it's not, in which case you need to open nouveau
  source code anyway to check.

Compounding on the fun, you have to do the special nouveau step twice, once for
nvc0 and once for nv50.

Why might it be benefical to list CAPs explicitly instead of relying on the
defaults?

* Maybe easier auditing nouveau driver for CAP correctness? In practice this has
  not been an issue for any of the drivers I've worked on, especially because
  the defaults are quite reasonable.

* Maybe forcing people adding CAPs to think about nouveau specially? This isn't
  fair to the tree in general, why should nouveau get this special
  treatment? Instead, CAPs are generally added to gate functionality that may
  not be supported on all drivers, and the default is disabling the new
  functionality until a developer for a given driver can wire it up. There's
  already no expectation that the person adding CAPs needs to also add the
  functionality to nouveau (if that's even possible) -- unless the CAP is being
  added for the particular nouveau's benefit of course -- so this isn't helpful.

* Maybe forcing people removing CAPs to think about nouveau specially? Similar
  issues apply here, and it's not clear how this would even work.

* Maybe keeping novueau developers aware of CAP churn? Again nouveau should not
  be special here and it isn't sustainable to do this for every driver. So, if
  this is something that nouveau developers want to do -- and they choose not to
  follow Gallium-tagged merge requests -- then the git log of
  src/gallium/include/pipe/p_defines.h or indeed
  src/gallium/auxiliary/util/u_screen.c may be consulted.

So, without an excellent reason why nouveau should be treated specially, and
with several reasons why it should not, let's bring nouveau in line with the
rest of Gallium and rely on the defaults.

I've left in CAPs with attached comments even when they are returning the
default value to preserve information from before the commit. Otherwise, this
commit aims to remove explicit cases that match the default value, as other
drivers generally aim to do.

Signed-off-by: Alyssa Rosenzweig <alyssa@collabora.com>
Reviewed-by: Karol Herbst <kherbst@redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22245>
src/gallium/drivers/nouveau/nv50/nv50_screen.c
src/gallium/drivers/nouveau/nvc0/nvc0_screen.c