GPU: Add platform parameter for GPU architecture #112566

Jason Fielder · 2023-09-19T12:10:52+02:00

Jason Fielder commented

2023-09-19 12:10:52 +02:00

Authored by Apple: Michael Parkin-White

Jason Fielder added 1 commit 2023-09-19 12:11:04 +02:00

8b5c9e5077 GPU: Add platform parameter for GPU architecture

Authored by Apple: Michael Parkin-White

Jason Fielder requested review from Jeroen Bakker 2023-09-19 12:11:34 +02:00

Jason Fielder requested review from Clément Foucault 2023-09-19 12:11:43 +02:00

Jeroen Bakker reviewed 2023-09-19 12:41:57 +02:00

source/blender/gpu/GPU_platform.h Outdated

						
				@ -56,2 +56,4 @@

				} eGPUSupportLevel;

				typedef enum eGPUArchitectureType {

				  GPU_ARCHITECTURE_IMR = 0,

Jeroen Bakker commented

2023-09-19 12:41:57 +02:00

These terms are not clear.
If I look further in the code it is unified memory or not.

Think eGPUMemoryModel would be more appropriate as an GPU architecture is typically used to refer to a processor type or series. Possible values might be
GPU_MEMORY_MODEL_INTEGRATED
GPU_MEMORY_MODEL_DEDICATED
GPU_MEMORY_MODEL_UNIFIED

These terms are not clear. If I look further in the code it is unified memory or not. Think eGPUMemoryModel would be more appropriate as an GPU architecture is typically used to refer to a processor type or series. Possible values might be GPU_MEMORY_MODEL_INTEGRATED GPU_MEMORY_MODEL_DEDICATED GPU_MEMORY_MODEL_UNIFIED

Clément Foucault commented

2023-09-20 15:12:37 +02:00

I think it is more about the way how the chip is build to render. So it has nothing to do with the memory model. Some intel chip might have some dedicated memory but still work as tile renderer.

Michael Parkin-White commented

2023-09-20 15:38:18 +02:00

First-time contributor

The memory model would also likely be a valid capability to expose, but would cover different use-cases.

Currently, the use-case for architecture type would be to determine which implementation of virtual shadow map updating to use as per: #111283 (Metal: EEVEE Next: Optimize Virtual shadow maps for Apple Silicon).

Granted, the memory model could be used to achieve the same switch, but the implementation of the technique is mostly depending on the architecture style of the GPU, as Clement outlined.

It could still theoretically be possible to have either type of architecture with any other type of memory model.

This architecture check may also be relevant for other APIs in future, especially also for windows machines running with Qualcomm Snapdragon GPUs. (Qualcomm is also a little iffy as technically these GPUs can also run in immediate mode, depending on the bits-per-pixel and workload being rendered)

Perhaps it's a technicality atm, as atm, there is a correlation between tile-based architectures and system-on-chip Unified memory architecture, so I guess it depends which parameter best represents this.

I see two options:

Add both capabilities
Use eGPUMemoryModel with the assumption that a unified memory implies a tile-based renderer, which can use tile-shaders and on-tile per-pixel storage.

The memory model would also likely be a valid capability to expose, but would cover different use-cases. Currently, the use-case for architecture type would be to determine which implementation of virtual shadow map updating to use as per: https://projects.blender.org/blender/blender/pulls/111283 (Metal: EEVEE Next: Optimize Virtual shadow maps for Apple Silicon). Granted, the memory model could be used to achieve the same switch, but the implementation of the technique is mostly depending on the architecture style of the GPU, as Clement outlined. It could still theoretically be possible to have either type of architecture with any other type of memory model. This architecture check may also be relevant for other APIs in future, especially also for windows machines running with Qualcomm Snapdragon GPUs. (Qualcomm is also a little iffy as technically these GPUs can also run in immediate mode, depending on the bits-per-pixel and workload being rendered) Perhaps it's a technicality atm, as atm, there is a correlation between tile-based architectures and system-on-chip Unified memory architecture, so I guess it depends which parameter best represents this. I see two options: 1) Add both capabilities 2) Use eGPUMemoryModel with the assumption that a unified memory implies a tile-based renderer, which can use tile-shaders and on-tile per-pixel storage.

Jeroen Bakker commented

2023-09-21 14:10:29 +02:00

In that case it makes more sense to have a GPU capability setting where we can check if it uses tiled rendering. As this is still an internal property think it is enough to use a bool.
GPU_tiled_renderer_support() or something similar.

In that case it makes more sense to have a GPU capability setting where we can check if it uses tiled rendering. As this is still an internal property think it is enough to use a bool. `GPU_tiled_renderer_support()` or something similar.

Jeroen Bakker commented

2023-09-19 12:44:54 +02:00

Can you elaborate a bit on the usage/use case you have in mind. We might be able to start with just a GPU_capability for the memory model which each backend can set.

Jeroen Bakker referenced this pull request

2023-09-25 11:19:41 +02:00

Face overlay still not resolve. #110048

Kent Davis referenced this pull request

2023-09-25 17:34:44 +02:00

Face overlay still not resolve. #110048

Jeroen Bakker requested changes 2023-09-26 13:04:08 +02:00

Jeroen Bakker left a comment

I did some reading on this topic. GPU architecture seems to be the correct term according to wikipedia and blogs. https://www.rastergrid.com/blog/gpu-tech/2021/07/gpu-architecture-types-explained/

I am fine with adding this to GPG as it currently is.

I would suggest to add defined to the shader codes. As that could be used to enable/disable the depth bias.
#define GPU_ARCHITECTURE_TBDR
#define GPU_ARCHITECTURE_IMR

Perhaps just add a comment what TBDR and IMR means (Tile-based deferred rendering. and Immediate mode rendering.
The rest seems fine. And sorry for the confusion due to lacking information on my side.

I did some reading on this topic. GPU architecture seems to be the correct term according to wikipedia and blogs. https://www.rastergrid.com/blog/gpu-tech/2021/07/gpu-architecture-types-explained/ I am fine with adding this to GPG as it currently is. I would suggest to add defined to the shader codes. As that could be used to enable/disable the depth bias. `#define GPU_ARCHITECTURE_TBDR` `#define GPU_ARCHITECTURE_IMR` Perhaps just add a comment what TBDR and IMR means (`Tile-based deferred rendering.` and `Immediate mode rendering`. The rest seems fine. And sorry for the confusion due to lacking information on my side.

Michael Parkin-White commented

2023-09-26 13:06:22 +02:00

First-time contributor

I did some reading on this topic. GPU architecture seems to be the correct term according to wikipedia and blogs. https://www.rastergrid.com/blog/gpu-tech/2021/07/gpu-architecture-types-explained/

I am fine with adding this to GPG as it currently is.

I would suggest to add defined to the shader codes. As that could be used to enable/disable the depth bias.
#define GPU_ARCHITECTURE_TBDR
#define GPU_ARCHITECTURE_IMR

Perhaps just add a comment what TBDR and IMR means (Tile-based deferred rendering. and Immediate mode rendering.
The rest seems fine. And sorry for the confusion due to lacking information on my side.

No problem! Thanks for all of the feedback, so in conclusion, best to revert to the original method (I realise I updated my patch locally to the GPU_tiled_renderer_support , but believe I must have forgotten to push the patch -- possibly pushed it to the shadow branch instead of this one).

Good idea on adding the changes to the shader definitions as well, can look at getting that sorted.

> I did some reading on this topic. GPU architecture seems to be the correct term according to wikipedia and blogs. https://www.rastergrid.com/blog/gpu-tech/2021/07/gpu-architecture-types-explained/ > > I am fine with adding this to GPG as it currently is. > > I would suggest to add defined to the shader codes. As that could be used to enable/disable the depth bias. > `#define GPU_ARCHITECTURE_TBDR` > `#define GPU_ARCHITECTURE_IMR` > > Perhaps just add a comment what TBDR and IMR means (`Tile-based deferred rendering.` and `Immediate mode rendering`. > The rest seems fine. And sorry for the confusion due to lacking information on my side. > No problem! Thanks for all of the feedback, so in conclusion, best to revert to the original method (I realise I updated my patch locally to the `GPU_tiled_renderer_support` , but believe I must have forgotten to push the patch -- possibly pushed it to the shadow branch instead of this one). Good idea on adding the changes to the shader definitions as well, can look at getting that sorted.

Jason Fielder added 3 commits 2023-09-27 22:27:35 +02:00

fde03be518 Merge branch 'main' into GPUPlatformArchitectureParam

0907c62a9f merge branch main

e5f9a451ba Add architecture type descriptions.

Clément Foucault approved these changes 2023-09-27 22:34:42 +02:00

source/blender/gpu/GPU_platform.h Outdated

						
				@ -55,6 +55,11 @@ typedef enum eGPUSupportLevel {

				  GPU_SUPPORT_LEVEL_UNSUPPORTED,

				} eGPUSupportLevel;

				typedef enum eGPUArchitectureType {