* [PATCH v2 0/2] drm/nouveau: zcull support
@ 2026-02-05 18:56 ` Mel Henning
0 siblings, 0 replies; 18+ messages in thread
From: Mel Henning @ 2026-02-05 18:56 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Mary Guillemard
Cc: dri-devel, nouveau, linux-kernel
These patches add kernel-side support for using the zcull hardware in nvidia
gpus. zcull aims to improve memory bandwidth by using an early approximate
depth test, similar to hierarchical Z on an AMD card. These patches add a
new ioctl on nouveau devices, which is currently only supported when using
gsp firmware.
Corresponding userspace changes for NVK are available here:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33861
This series is also available in git:
https://gitlab.freedesktop.org/mhenning/linux/-/commits/zcull3
(v2): Split the first patch into two - one for fetching information from
the hardware and the other for adding the UAPI. Also, drop the
change adding DRM_NOUVEAU_SET_ZCULL_CTXSW_BUFFER - it isn't
necessary to get zcull working and will be pursued as a separate
performance improvement in the future.
Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
---
Mel Henning (2):
drm/nouveau: Fetch zcull info from device
drm/nouveau: Add DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO
drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h | 19 +++++++
drivers/gpu/drm/nouveau/nouveau_abi16.c | 29 ++++++++++
drivers/gpu/drm/nouveau/nouveau_abi16.h | 1 +
drivers/gpu/drm/nouveau/nouveau_drm.c | 1 +
.../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c | 9 ++-
.../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c | 32 ++++++++++-
.../drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h | 19 +++++++
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h | 2 +-
include/uapi/drm/nouveau_drm.h | 66 ++++++++++++++++++++++
9 files changed, 172 insertions(+), 6 deletions(-)
---
base-commit: 18f7fcd5e69a04df57b563360b88be72471d6b62
change-id: 20260205-zcull3-9065115cd238
Best regards,
--
Mel Henning <mhenning@darkrefraction.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 0/2] drm/nouveau: zcull support
@ 2026-02-05 18:56 ` Mel Henning
0 siblings, 0 replies; 18+ messages in thread
From: Mel Henning @ 2026-02-05 18:56 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Mary Guillemard
Cc: dri-devel, nouveau, linux-kernel, Mel Henning
These patches add kernel-side support for using the zcull hardware in nvidia
gpus. zcull aims to improve memory bandwidth by using an early approximate
depth test, similar to hierarchical Z on an AMD card. These patches add a
new ioctl on nouveau devices, which is currently only supported when using
gsp firmware.
Corresponding userspace changes for NVK are available here:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33861
This series is also available in git:
https://gitlab.freedesktop.org/mhenning/linux/-/commits/zcull3
(v2): Split the first patch into two - one for fetching information from
the hardware and the other for adding the UAPI. Also, drop the
change adding DRM_NOUVEAU_SET_ZCULL_CTXSW_BUFFER - it isn't
necessary to get zcull working and will be pursued as a separate
performance improvement in the future.
Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
---
Mel Henning (2):
drm/nouveau: Fetch zcull info from device
drm/nouveau: Add DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO
drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h | 19 +++++++
drivers/gpu/drm/nouveau/nouveau_abi16.c | 29 ++++++++++
drivers/gpu/drm/nouveau/nouveau_abi16.h | 1 +
drivers/gpu/drm/nouveau/nouveau_drm.c | 1 +
.../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c | 9 ++-
.../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c | 32 ++++++++++-
.../drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h | 19 +++++++
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h | 2 +-
include/uapi/drm/nouveau_drm.h | 66 ++++++++++++++++++++++
9 files changed, 172 insertions(+), 6 deletions(-)
---
base-commit: 18f7fcd5e69a04df57b563360b88be72471d6b62
change-id: 20260205-zcull3-9065115cd238
Best regards,
--
Mel Henning <mhenning@darkrefraction.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device
2026-02-05 18:56 ` Mel Henning
@ 2026-02-05 18:56 ` Mel Henning
-1 siblings, 0 replies; 18+ messages in thread
From: Mel Henning @ 2026-02-05 18:56 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Mary Guillemard
Cc: dri-devel, nouveau, linux-kernel
This information will be exposed to userspace in the following commit.
Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
---
drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h | 19 +++++++++++++
.../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c | 9 ++++--
.../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c | 32 ++++++++++++++++++++--
.../drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h | 19 +++++++++++++
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h | 2 +-
5 files changed, 75 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
index a2333cfe6955..490ce410f6cb 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
@@ -3,9 +3,28 @@
#define __NVKM_GR_H__
#include <core/engine.h>
+struct nvkm_gr_zcull_info {
+ __u32 width_align_pixels;
+ __u32 height_align_pixels;
+ __u32 pixel_squares_by_aliquots;
+ __u32 aliquot_total;
+ __u32 zcull_region_byte_multiplier;
+ __u32 zcull_region_header_size;
+ __u32 zcull_subregion_header_size;
+ __u32 subregion_count;
+ __u32 subregion_width_align_pixels;
+ __u32 subregion_height_align_pixels;
+
+ __u32 ctxsw_size;
+ __u32 ctxsw_align;
+};
+
struct nvkm_gr {
const struct nvkm_gr_func *func;
struct nvkm_engine engine;
+
+ struct nvkm_gr_zcull_info zcull_info;
+ bool has_zcull_info;
};
u64 nvkm_gr_units(struct nvkm_gr *);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
index ddb57d5e73d6..73844e1e7294 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
@@ -249,7 +249,7 @@ r535_gr_get_ctxbuf_info(struct r535_gr *gr, int i,
}
static int
-r535_gr_get_ctxbufs_info(struct r535_gr *gr)
+r535_gr_get_ctxbufs_and_zcull_info(struct r535_gr *gr)
{
NV2080_CTRL_INTERNAL_STATIC_GR_GET_CONTEXT_BUFFERS_INFO_PARAMS *info;
struct nvkm_subdev *subdev = &gr->base.engine.subdev;
@@ -265,6 +265,9 @@ r535_gr_get_ctxbufs_info(struct r535_gr *gr)
r535_gr_get_ctxbuf_info(gr, i, &info->engineContextBuffersInfo[0].engine[i]);
nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, info);
+
+ gr->base.has_zcull_info = false;
+
return 0;
}
@@ -312,7 +315,7 @@ r535_gr_oneinit(struct nvkm_gr *base)
*
* Also build the information that'll be used to create channel contexts.
*/
- ret = rm->api->gr->get_ctxbufs_info(gr);
+ ret = rm->api->gr->get_ctxbufs_and_zcull_info(gr);
if (ret)
goto done;
@@ -352,5 +355,5 @@ r535_gr_dtor(struct nvkm_gr *base)
const struct nvkm_rm_api_gr
r535_gr = {
- .get_ctxbufs_info = r535_gr_get_ctxbufs_info,
+ .get_ctxbufs_and_zcull_info = r535_gr_get_ctxbufs_and_zcull_info,
};
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
index b6cced9b8aa1..3e7af2ffece9 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
@@ -164,9 +164,10 @@ r570_gr_scrubber_init(struct r535_gr *gr)
}
static int
-r570_gr_get_ctxbufs_info(struct r535_gr *gr)
+r570_gr_get_ctxbufs_and_zcull_info(struct r535_gr *gr)
{
NV2080_CTRL_INTERNAL_STATIC_GR_GET_CONTEXT_BUFFERS_INFO_PARAMS *info;
+ NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS *zcull_info;
struct nvkm_subdev *subdev = &gr->base.engine.subdev;
struct nvkm_gsp *gsp = subdev->device->gsp;
@@ -179,13 +180,40 @@ r570_gr_get_ctxbufs_info(struct r535_gr *gr)
for (int i = 0; i < ARRAY_SIZE(info->engineContextBuffersInfo[0].engine); i++)
r535_gr_get_ctxbuf_info(gr, i, &info->engineContextBuffersInfo[0].engine[i]);
+ NV2080_CTRL_INTERNAL_ENGINE_CONTEXT_BUFFER_INFO zcull = info->engineContextBuffersInfo[0]
+ .engine[NV0080_CTRL_FIFO_GET_ENGINE_CONTEXT_PROPERTIES_ENGINE_ID_GRAPHICS_ZCULL];
+ gr->base.zcull_info.ctxsw_size = zcull.size;
+ gr->base.zcull_info.ctxsw_align = zcull.alignment;
+
nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, info);
+
+ zcull_info = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.subdevice,
+ NV2080_CTRL_CMD_GR_GET_ZCULL_INFO,
+ sizeof(*zcull_info));
+ if (WARN_ON(IS_ERR(zcull_info)))
+ return PTR_ERR(zcull_info);
+
+ gr->base.zcull_info.width_align_pixels = zcull_info->widthAlignPixels;
+ gr->base.zcull_info.height_align_pixels = zcull_info->heightAlignPixels;
+ gr->base.zcull_info.pixel_squares_by_aliquots = zcull_info->pixelSquaresByAliquots;
+ gr->base.zcull_info.aliquot_total = zcull_info->aliquotTotal;
+ gr->base.zcull_info.zcull_region_byte_multiplier = zcull_info->zcullRegionByteMultiplier;
+ gr->base.zcull_info.zcull_region_header_size = zcull_info->zcullRegionHeaderSize;
+ gr->base.zcull_info.zcull_subregion_header_size = zcull_info->zcullSubregionHeaderSize;
+ gr->base.zcull_info.subregion_count = zcull_info->subregionCount;
+ gr->base.zcull_info.subregion_width_align_pixels = zcull_info->subregionWidthAlignPixels;
+ gr->base.zcull_info.subregion_height_align_pixels = zcull_info->subregionHeightAlignPixels;
+
+ nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, zcull_info);
+
+ gr->base.has_zcull_info = true;
+
return 0;
}
const struct nvkm_rm_api_gr
r570_gr = {
- .get_ctxbufs_info = r570_gr_get_ctxbufs_info,
+ .get_ctxbufs_and_zcull_info = r570_gr_get_ctxbufs_and_zcull_info,
.scrubber.init = r570_gr_scrubber_init,
.scrubber.fini = r570_gr_scrubber_fini,
};
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h
index feed1dabd9d2..a7a15a4de9d5 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h
@@ -76,4 +76,23 @@ typedef struct NV2080_CTRL_INTERNAL_GR_INIT_BUG4208224_WAR_PARAMS {
} NV2080_CTRL_INTERNAL_GR_INIT_BUG4208224_WAR_PARAMS;
#define NV2080_CTRL_CMD_INTERNAL_KGR_INIT_BUG4208224_WAR (0x20800a46) /* finn: Evaluated from "(FINN_NV20_SUBDEVICE_0_INTERNAL_INTERFACE_ID << 8) | NV2080_CTRL_INTERNAL_KGR_INIT_BUG4208224_WAR_PARAMS_MESSAGE_ID" */
+
+#define NV2080_CTRL_CMD_GR_GET_ZCULL_INFO (0x20801206U) /* finn: Evaluated from "(FINN_NV20_SUBDEVICE_0_GR_INTERFACE_ID << 8) | NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS_MESSAGE_ID" */
+
+#define NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS_SUBREGION_SUPPORTED
+#define NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS_MESSAGE_ID (0x6U)
+
+typedef struct NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS {
+ NvU32 widthAlignPixels;
+ NvU32 heightAlignPixels;
+ NvU32 pixelSquaresByAliquots;
+ NvU32 aliquotTotal;
+ NvU32 zcullRegionByteMultiplier;
+ NvU32 zcullRegionHeaderSize;
+ NvU32 zcullSubregionHeaderSize;
+ NvU32 subregionCount;
+ NvU32 subregionWidthAlignPixels;
+ NvU32 subregionHeightAlignPixels;
+} NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS;
+
#endif
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
index 393ea775941f..0fb0e67406c6 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
@@ -124,7 +124,7 @@ struct nvkm_rm_api {
} *ce, *nvdec, *nvenc, *nvjpg, *ofa;
const struct nvkm_rm_api_gr {
- int (*get_ctxbufs_info)(struct r535_gr *);
+ int (*get_ctxbufs_and_zcull_info)(struct r535_gr *);
struct {
int (*init)(struct r535_gr *);
void (*fini)(struct r535_gr *);
--
2.52.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device
@ 2026-02-05 18:56 ` Mel Henning
0 siblings, 0 replies; 18+ messages in thread
From: Mel Henning @ 2026-02-05 18:56 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Mary Guillemard
Cc: dri-devel, nouveau, linux-kernel, Mel Henning
This information will be exposed to userspace in the following commit.
Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
---
drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h | 19 +++++++++++++
.../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c | 9 ++++--
.../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c | 32 ++++++++++++++++++++--
.../drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h | 19 +++++++++++++
drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h | 2 +-
5 files changed, 75 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
index a2333cfe6955..490ce410f6cb 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
@@ -3,9 +3,28 @@
#define __NVKM_GR_H__
#include <core/engine.h>
+struct nvkm_gr_zcull_info {
+ __u32 width_align_pixels;
+ __u32 height_align_pixels;
+ __u32 pixel_squares_by_aliquots;
+ __u32 aliquot_total;
+ __u32 zcull_region_byte_multiplier;
+ __u32 zcull_region_header_size;
+ __u32 zcull_subregion_header_size;
+ __u32 subregion_count;
+ __u32 subregion_width_align_pixels;
+ __u32 subregion_height_align_pixels;
+
+ __u32 ctxsw_size;
+ __u32 ctxsw_align;
+};
+
struct nvkm_gr {
const struct nvkm_gr_func *func;
struct nvkm_engine engine;
+
+ struct nvkm_gr_zcull_info zcull_info;
+ bool has_zcull_info;
};
u64 nvkm_gr_units(struct nvkm_gr *);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
index ddb57d5e73d6..73844e1e7294 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
@@ -249,7 +249,7 @@ r535_gr_get_ctxbuf_info(struct r535_gr *gr, int i,
}
static int
-r535_gr_get_ctxbufs_info(struct r535_gr *gr)
+r535_gr_get_ctxbufs_and_zcull_info(struct r535_gr *gr)
{
NV2080_CTRL_INTERNAL_STATIC_GR_GET_CONTEXT_BUFFERS_INFO_PARAMS *info;
struct nvkm_subdev *subdev = &gr->base.engine.subdev;
@@ -265,6 +265,9 @@ r535_gr_get_ctxbufs_info(struct r535_gr *gr)
r535_gr_get_ctxbuf_info(gr, i, &info->engineContextBuffersInfo[0].engine[i]);
nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, info);
+
+ gr->base.has_zcull_info = false;
+
return 0;
}
@@ -312,7 +315,7 @@ r535_gr_oneinit(struct nvkm_gr *base)
*
* Also build the information that'll be used to create channel contexts.
*/
- ret = rm->api->gr->get_ctxbufs_info(gr);
+ ret = rm->api->gr->get_ctxbufs_and_zcull_info(gr);
if (ret)
goto done;
@@ -352,5 +355,5 @@ r535_gr_dtor(struct nvkm_gr *base)
const struct nvkm_rm_api_gr
r535_gr = {
- .get_ctxbufs_info = r535_gr_get_ctxbufs_info,
+ .get_ctxbufs_and_zcull_info = r535_gr_get_ctxbufs_and_zcull_info,
};
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
index b6cced9b8aa1..3e7af2ffece9 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
@@ -164,9 +164,10 @@ r570_gr_scrubber_init(struct r535_gr *gr)
}
static int
-r570_gr_get_ctxbufs_info(struct r535_gr *gr)
+r570_gr_get_ctxbufs_and_zcull_info(struct r535_gr *gr)
{
NV2080_CTRL_INTERNAL_STATIC_GR_GET_CONTEXT_BUFFERS_INFO_PARAMS *info;
+ NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS *zcull_info;
struct nvkm_subdev *subdev = &gr->base.engine.subdev;
struct nvkm_gsp *gsp = subdev->device->gsp;
@@ -179,13 +180,40 @@ r570_gr_get_ctxbufs_info(struct r535_gr *gr)
for (int i = 0; i < ARRAY_SIZE(info->engineContextBuffersInfo[0].engine); i++)
r535_gr_get_ctxbuf_info(gr, i, &info->engineContextBuffersInfo[0].engine[i]);
+ NV2080_CTRL_INTERNAL_ENGINE_CONTEXT_BUFFER_INFO zcull = info->engineContextBuffersInfo[0]
+ .engine[NV0080_CTRL_FIFO_GET_ENGINE_CONTEXT_PROPERTIES_ENGINE_ID_GRAPHICS_ZCULL];
+ gr->base.zcull_info.ctxsw_size = zcull.size;
+ gr->base.zcull_info.ctxsw_align = zcull.alignment;
+
nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, info);
+
+ zcull_info = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.subdevice,
+ NV2080_CTRL_CMD_GR_GET_ZCULL_INFO,
+ sizeof(*zcull_info));
+ if (WARN_ON(IS_ERR(zcull_info)))
+ return PTR_ERR(zcull_info);
+
+ gr->base.zcull_info.width_align_pixels = zcull_info->widthAlignPixels;
+ gr->base.zcull_info.height_align_pixels = zcull_info->heightAlignPixels;
+ gr->base.zcull_info.pixel_squares_by_aliquots = zcull_info->pixelSquaresByAliquots;
+ gr->base.zcull_info.aliquot_total = zcull_info->aliquotTotal;
+ gr->base.zcull_info.zcull_region_byte_multiplier = zcull_info->zcullRegionByteMultiplier;
+ gr->base.zcull_info.zcull_region_header_size = zcull_info->zcullRegionHeaderSize;
+ gr->base.zcull_info.zcull_subregion_header_size = zcull_info->zcullSubregionHeaderSize;
+ gr->base.zcull_info.subregion_count = zcull_info->subregionCount;
+ gr->base.zcull_info.subregion_width_align_pixels = zcull_info->subregionWidthAlignPixels;
+ gr->base.zcull_info.subregion_height_align_pixels = zcull_info->subregionHeightAlignPixels;
+
+ nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, zcull_info);
+
+ gr->base.has_zcull_info = true;
+
return 0;
}
const struct nvkm_rm_api_gr
r570_gr = {
- .get_ctxbufs_info = r570_gr_get_ctxbufs_info,
+ .get_ctxbufs_and_zcull_info = r570_gr_get_ctxbufs_and_zcull_info,
.scrubber.init = r570_gr_scrubber_init,
.scrubber.fini = r570_gr_scrubber_fini,
};
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h
index feed1dabd9d2..a7a15a4de9d5 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h
@@ -76,4 +76,23 @@ typedef struct NV2080_CTRL_INTERNAL_GR_INIT_BUG4208224_WAR_PARAMS {
} NV2080_CTRL_INTERNAL_GR_INIT_BUG4208224_WAR_PARAMS;
#define NV2080_CTRL_CMD_INTERNAL_KGR_INIT_BUG4208224_WAR (0x20800a46) /* finn: Evaluated from "(FINN_NV20_SUBDEVICE_0_INTERNAL_INTERFACE_ID << 8) | NV2080_CTRL_INTERNAL_KGR_INIT_BUG4208224_WAR_PARAMS_MESSAGE_ID" */
+
+#define NV2080_CTRL_CMD_GR_GET_ZCULL_INFO (0x20801206U) /* finn: Evaluated from "(FINN_NV20_SUBDEVICE_0_GR_INTERFACE_ID << 8) | NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS_MESSAGE_ID" */
+
+#define NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS_SUBREGION_SUPPORTED
+#define NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS_MESSAGE_ID (0x6U)
+
+typedef struct NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS {
+ NvU32 widthAlignPixels;
+ NvU32 heightAlignPixels;
+ NvU32 pixelSquaresByAliquots;
+ NvU32 aliquotTotal;
+ NvU32 zcullRegionByteMultiplier;
+ NvU32 zcullRegionHeaderSize;
+ NvU32 zcullSubregionHeaderSize;
+ NvU32 subregionCount;
+ NvU32 subregionWidthAlignPixels;
+ NvU32 subregionHeightAlignPixels;
+} NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS;
+
#endif
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
index 393ea775941f..0fb0e67406c6 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h
@@ -124,7 +124,7 @@ struct nvkm_rm_api {
} *ce, *nvdec, *nvenc, *nvjpg, *ofa;
const struct nvkm_rm_api_gr {
- int (*get_ctxbufs_info)(struct r535_gr *);
+ int (*get_ctxbufs_and_zcull_info)(struct r535_gr *);
struct {
int (*init)(struct r535_gr *);
void (*fini)(struct r535_gr *);
--
2.52.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/2] drm/nouveau: Add DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO
2026-02-05 18:56 ` Mel Henning
@ 2026-02-05 18:56 ` Mel Henning
-1 siblings, 0 replies; 18+ messages in thread
From: Mel Henning @ 2026-02-05 18:56 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Mary Guillemard
Cc: dri-devel, nouveau, linux-kernel
Add kernel-side support for using the zcull hardware in nvidia gpus.
zcull aims to improve memory bandwidth by using an early approximate
depth test, similar to hierarchical Z on an AMD card.
Add a new ioctl that exposes zcull information that has been read
from the hardware. Userspace uses each of these parameters either
in a heuristic for determining zcull region parameters or in the
calculation of a buffer size.
It appears the hardware hasn't changed its structure for these
values since FERMI_C (circa 2011), so the assumption is that it
won't change on us too quickly, and is therefore reasonable to
include in UAPI.
This bypasses the nvif layer and instead accesses nvkm_gr directly,
which mirrors existing usage of nvkm_gr_units(). There is no nvif
object for nvkm_gr yet, and adding one is not trivial.
Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
---
drivers/gpu/drm/nouveau/nouveau_abi16.c | 29 +++++++++++++++
drivers/gpu/drm/nouveau/nouveau_abi16.h | 1 +
drivers/gpu/drm/nouveau/nouveau_drm.c | 1 +
include/uapi/drm/nouveau_drm.h | 66 +++++++++++++++++++++++++++++++++
4 files changed, 97 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index a3ba07fc48a0..91d6f51c5a2f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -333,6 +333,35 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
return 0;
}
+int
+nouveau_abi16_ioctl_get_zcull_info(ABI16_IOCTL_ARGS)
+{
+ struct nouveau_drm *drm = nouveau_drm(dev);
+ struct nvkm_gr *gr = nvxx_gr(drm);
+ struct drm_nouveau_get_zcull_info *out = data;
+
+ if (gr->has_zcull_info) {
+ const struct nvkm_gr_zcull_info *i = &gr->zcull_info;
+
+ out->width_align_pixels = i->width_align_pixels;
+ out->height_align_pixels = i->height_align_pixels;
+ out->pixel_squares_by_aliquots = i->pixel_squares_by_aliquots;
+ out->aliquot_total = i->aliquot_total;
+ out->zcull_region_byte_multiplier = i->zcull_region_byte_multiplier;
+ out->zcull_region_header_size = i->zcull_region_header_size;
+ out->zcull_subregion_header_size = i->zcull_subregion_header_size;
+ out->subregion_count = i->subregion_count;
+ out->subregion_width_align_pixels = i->subregion_width_align_pixels;
+ out->subregion_height_align_pixels = i->subregion_height_align_pixels;
+ out->ctxsw_size = i->ctxsw_size;
+ out->ctxsw_align = i->ctxsw_align;
+
+ return 0;
+ } else {
+ return -ENODEV;
+ }
+}
+
int
nouveau_abi16_ioctl_channel_alloc(ABI16_IOCTL_ARGS)
{
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.h b/drivers/gpu/drm/nouveau/nouveau_abi16.h
index af6b4e1cefd2..134b3ab58719 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.h
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.h
@@ -6,6 +6,7 @@
struct drm_device *dev, void *data, struct drm_file *file_priv
int nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS);
+int nouveau_abi16_ioctl_get_zcull_info(ABI16_IOCTL_ARGS);
int nouveau_abi16_ioctl_channel_alloc(ABI16_IOCTL_ARGS);
int nouveau_abi16_ioctl_channel_free(ABI16_IOCTL_ARGS);
int nouveau_abi16_ioctl_grobj_alloc(ABI16_IOCTL_ARGS);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 1527b801f013..b698ac38e947 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1272,6 +1272,7 @@ nouveau_ioctls[] = {
DRM_IOCTL_DEF_DRV(NOUVEAU_GROBJ_ALLOC, nouveau_abi16_ioctl_grobj_alloc, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(NOUVEAU_NOTIFIEROBJ_ALLOC, nouveau_abi16_ioctl_notifierobj_alloc, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(NOUVEAU_GPUOBJ_FREE, nouveau_abi16_ioctl_gpuobj_free, DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(NOUVEAU_GET_ZCULL_INFO, nouveau_abi16_ioctl_get_zcull_info, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(NOUVEAU_SVM_INIT, nouveau_svmm_init, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(NOUVEAU_SVM_BIND, nouveau_svmm_bind, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_NEW, nouveau_gem_ioctl_new, DRM_RENDER_ALLOW),
diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
index dd87f8f30793..1fa82fa6af38 100644
--- a/include/uapi/drm/nouveau_drm.h
+++ b/include/uapi/drm/nouveau_drm.h
@@ -432,6 +432,69 @@ struct drm_nouveau_exec {
__u64 push_ptr;
};
+struct drm_nouveau_get_zcull_info {
+ /**
+ * @width_align_pixels: required alignment for region widths, in pixels
+ * (typically #TPC's * 16).
+ */
+ __u32 width_align_pixels;
+ /**
+ * @height_align_pixels: required alignment for region heights, in
+ * pixels (typically 32).
+ */
+ __u32 height_align_pixels;
+ /**
+ * @pixel_squares_by_aliquots: the pixel area covered by an aliquot
+ * (typically #Zcull_banks * 16 * 16).
+ */
+ __u32 pixel_squares_by_aliquots;
+ /**
+ * @aliquot_total: the total aliquot pool available in hardware
+ */
+ __u32 aliquot_total;
+ /**
+ * @zcull_region_byte_multiplier: the size of an aliquot in bytes, which
+ * is used for save/restore operations on a region
+ */
+ __u32 zcull_region_byte_multiplier;
+ /**
+ * @zcull_region_header_size: the region header size in bytes, which is
+ * used for save/restore operations on a region
+ */
+ __u32 zcull_region_header_size;
+ /**
+ * @zcull_subregion_header_size: the subregion header size in bytes,
+ * which is used for save/restore operations on a region
+ */
+ __u32 zcull_subregion_header_size;
+ /**
+ * @subregion_count: the total number of subregions the hardware
+ * supports
+ */
+ __u32 subregion_count;
+ /**
+ * @subregion_width_align_pixels: required alignment for subregion
+ * widths, in pixels (typically #TPC's * 16).
+ */
+ __u32 subregion_width_align_pixels;
+ /**
+ * @subregion_height_align_pixels: required alignment for subregion
+ * heights, in pixels
+ */
+ __u32 subregion_height_align_pixels;
+
+ /**
+ * @ctxsw_size: the size, in bytes, of a zcull context switching region.
+ * Will be zero if the kernel does not support zcull context switching.
+ */
+ __u32 ctxsw_size;
+ /**
+ * @ctxsw_align: the alignment, in bytes, of a zcull context switching
+ * region
+ */
+ __u32 ctxsw_align;
+};
+
#define DRM_NOUVEAU_GETPARAM 0x00
#define DRM_NOUVEAU_SETPARAM 0x01 /* deprecated */
#define DRM_NOUVEAU_CHANNEL_ALLOC 0x02
@@ -445,6 +508,7 @@ struct drm_nouveau_exec {
#define DRM_NOUVEAU_VM_INIT 0x10
#define DRM_NOUVEAU_VM_BIND 0x11
#define DRM_NOUVEAU_EXEC 0x12
+#define DRM_NOUVEAU_GET_ZCULL_INFO 0x13
#define DRM_NOUVEAU_GEM_NEW 0x40
#define DRM_NOUVEAU_GEM_PUSHBUF 0x41
#define DRM_NOUVEAU_GEM_CPU_PREP 0x42
@@ -513,6 +577,8 @@ struct drm_nouveau_svm_bind {
#define DRM_IOCTL_NOUVEAU_VM_INIT DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_VM_INIT, struct drm_nouveau_vm_init)
#define DRM_IOCTL_NOUVEAU_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_VM_BIND, struct drm_nouveau_vm_bind)
#define DRM_IOCTL_NOUVEAU_EXEC DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_EXEC, struct drm_nouveau_exec)
+
+#define DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO DRM_IOR (DRM_COMMAND_BASE + DRM_NOUVEAU_GET_ZCULL_INFO, struct drm_nouveau_get_zcull_info)
#if defined(__cplusplus)
}
#endif
--
2.52.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v2 2/2] drm/nouveau: Add DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO
@ 2026-02-05 18:56 ` Mel Henning
0 siblings, 0 replies; 18+ messages in thread
From: Mel Henning @ 2026-02-05 18:56 UTC (permalink / raw)
To: Lyude Paul, Danilo Krummrich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Mary Guillemard
Cc: dri-devel, nouveau, linux-kernel, Mel Henning
Add kernel-side support for using the zcull hardware in nvidia gpus.
zcull aims to improve memory bandwidth by using an early approximate
depth test, similar to hierarchical Z on an AMD card.
Add a new ioctl that exposes zcull information that has been read
from the hardware. Userspace uses each of these parameters either
in a heuristic for determining zcull region parameters or in the
calculation of a buffer size.
It appears the hardware hasn't changed its structure for these
values since FERMI_C (circa 2011), so the assumption is that it
won't change on us too quickly, and is therefore reasonable to
include in UAPI.
This bypasses the nvif layer and instead accesses nvkm_gr directly,
which mirrors existing usage of nvkm_gr_units(). There is no nvif
object for nvkm_gr yet, and adding one is not trivial.
Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
---
drivers/gpu/drm/nouveau/nouveau_abi16.c | 29 +++++++++++++++
drivers/gpu/drm/nouveau/nouveau_abi16.h | 1 +
drivers/gpu/drm/nouveau/nouveau_drm.c | 1 +
include/uapi/drm/nouveau_drm.h | 66 +++++++++++++++++++++++++++++++++
4 files changed, 97 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index a3ba07fc48a0..91d6f51c5a2f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -333,6 +333,35 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
return 0;
}
+int
+nouveau_abi16_ioctl_get_zcull_info(ABI16_IOCTL_ARGS)
+{
+ struct nouveau_drm *drm = nouveau_drm(dev);
+ struct nvkm_gr *gr = nvxx_gr(drm);
+ struct drm_nouveau_get_zcull_info *out = data;
+
+ if (gr->has_zcull_info) {
+ const struct nvkm_gr_zcull_info *i = &gr->zcull_info;
+
+ out->width_align_pixels = i->width_align_pixels;
+ out->height_align_pixels = i->height_align_pixels;
+ out->pixel_squares_by_aliquots = i->pixel_squares_by_aliquots;
+ out->aliquot_total = i->aliquot_total;
+ out->zcull_region_byte_multiplier = i->zcull_region_byte_multiplier;
+ out->zcull_region_header_size = i->zcull_region_header_size;
+ out->zcull_subregion_header_size = i->zcull_subregion_header_size;
+ out->subregion_count = i->subregion_count;
+ out->subregion_width_align_pixels = i->subregion_width_align_pixels;
+ out->subregion_height_align_pixels = i->subregion_height_align_pixels;
+ out->ctxsw_size = i->ctxsw_size;
+ out->ctxsw_align = i->ctxsw_align;
+
+ return 0;
+ } else {
+ return -ENODEV;
+ }
+}
+
int
nouveau_abi16_ioctl_channel_alloc(ABI16_IOCTL_ARGS)
{
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.h b/drivers/gpu/drm/nouveau/nouveau_abi16.h
index af6b4e1cefd2..134b3ab58719 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.h
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.h
@@ -6,6 +6,7 @@
struct drm_device *dev, void *data, struct drm_file *file_priv
int nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS);
+int nouveau_abi16_ioctl_get_zcull_info(ABI16_IOCTL_ARGS);
int nouveau_abi16_ioctl_channel_alloc(ABI16_IOCTL_ARGS);
int nouveau_abi16_ioctl_channel_free(ABI16_IOCTL_ARGS);
int nouveau_abi16_ioctl_grobj_alloc(ABI16_IOCTL_ARGS);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 1527b801f013..b698ac38e947 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1272,6 +1272,7 @@ nouveau_ioctls[] = {
DRM_IOCTL_DEF_DRV(NOUVEAU_GROBJ_ALLOC, nouveau_abi16_ioctl_grobj_alloc, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(NOUVEAU_NOTIFIEROBJ_ALLOC, nouveau_abi16_ioctl_notifierobj_alloc, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(NOUVEAU_GPUOBJ_FREE, nouveau_abi16_ioctl_gpuobj_free, DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(NOUVEAU_GET_ZCULL_INFO, nouveau_abi16_ioctl_get_zcull_info, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(NOUVEAU_SVM_INIT, nouveau_svmm_init, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(NOUVEAU_SVM_BIND, nouveau_svmm_bind, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_NEW, nouveau_gem_ioctl_new, DRM_RENDER_ALLOW),
diff --git a/include/uapi/drm/nouveau_drm.h b/include/uapi/drm/nouveau_drm.h
index dd87f8f30793..1fa82fa6af38 100644
--- a/include/uapi/drm/nouveau_drm.h
+++ b/include/uapi/drm/nouveau_drm.h
@@ -432,6 +432,69 @@ struct drm_nouveau_exec {
__u64 push_ptr;
};
+struct drm_nouveau_get_zcull_info {
+ /**
+ * @width_align_pixels: required alignment for region widths, in pixels
+ * (typically #TPC's * 16).
+ */
+ __u32 width_align_pixels;
+ /**
+ * @height_align_pixels: required alignment for region heights, in
+ * pixels (typically 32).
+ */
+ __u32 height_align_pixels;
+ /**
+ * @pixel_squares_by_aliquots: the pixel area covered by an aliquot
+ * (typically #Zcull_banks * 16 * 16).
+ */
+ __u32 pixel_squares_by_aliquots;
+ /**
+ * @aliquot_total: the total aliquot pool available in hardware
+ */
+ __u32 aliquot_total;
+ /**
+ * @zcull_region_byte_multiplier: the size of an aliquot in bytes, which
+ * is used for save/restore operations on a region
+ */
+ __u32 zcull_region_byte_multiplier;
+ /**
+ * @zcull_region_header_size: the region header size in bytes, which is
+ * used for save/restore operations on a region
+ */
+ __u32 zcull_region_header_size;
+ /**
+ * @zcull_subregion_header_size: the subregion header size in bytes,
+ * which is used for save/restore operations on a region
+ */
+ __u32 zcull_subregion_header_size;
+ /**
+ * @subregion_count: the total number of subregions the hardware
+ * supports
+ */
+ __u32 subregion_count;
+ /**
+ * @subregion_width_align_pixels: required alignment for subregion
+ * widths, in pixels (typically #TPC's * 16).
+ */
+ __u32 subregion_width_align_pixels;
+ /**
+ * @subregion_height_align_pixels: required alignment for subregion
+ * heights, in pixels
+ */
+ __u32 subregion_height_align_pixels;
+
+ /**
+ * @ctxsw_size: the size, in bytes, of a zcull context switching region.
+ * Will be zero if the kernel does not support zcull context switching.
+ */
+ __u32 ctxsw_size;
+ /**
+ * @ctxsw_align: the alignment, in bytes, of a zcull context switching
+ * region
+ */
+ __u32 ctxsw_align;
+};
+
#define DRM_NOUVEAU_GETPARAM 0x00
#define DRM_NOUVEAU_SETPARAM 0x01 /* deprecated */
#define DRM_NOUVEAU_CHANNEL_ALLOC 0x02
@@ -445,6 +508,7 @@ struct drm_nouveau_exec {
#define DRM_NOUVEAU_VM_INIT 0x10
#define DRM_NOUVEAU_VM_BIND 0x11
#define DRM_NOUVEAU_EXEC 0x12
+#define DRM_NOUVEAU_GET_ZCULL_INFO 0x13
#define DRM_NOUVEAU_GEM_NEW 0x40
#define DRM_NOUVEAU_GEM_PUSHBUF 0x41
#define DRM_NOUVEAU_GEM_CPU_PREP 0x42
@@ -513,6 +577,8 @@ struct drm_nouveau_svm_bind {
#define DRM_IOCTL_NOUVEAU_VM_INIT DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_VM_INIT, struct drm_nouveau_vm_init)
#define DRM_IOCTL_NOUVEAU_VM_BIND DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_VM_BIND, struct drm_nouveau_vm_bind)
#define DRM_IOCTL_NOUVEAU_EXEC DRM_IOWR(DRM_COMMAND_BASE + DRM_NOUVEAU_EXEC, struct drm_nouveau_exec)
+
+#define DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO DRM_IOR (DRM_COMMAND_BASE + DRM_NOUVEAU_GET_ZCULL_INFO, struct drm_nouveau_get_zcull_info)
#if defined(__cplusplus)
}
#endif
--
2.52.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device
2026-02-05 18:56 ` Mel Henning
@ 2026-02-13 17:12 ` Danilo Krummrich
-1 siblings, 0 replies; 18+ messages in thread
From: Danilo Krummrich @ 2026-02-13 17:12 UTC (permalink / raw)
To: Mel Henning
Cc: Maarten Lankhorst, Maxime Ripard, Simona Vetter, Mary Guillemard,
dri-devel, nouveau, linux-kernel
On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote:
> This information will be exposed to userspace in the following commit.
>
> Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
For someone looking at this commit, this commit message is not very useful.
Please add at least a brief explanation of what the patch does and - even more
important - why it does it. See also [1].
[1] https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> ---
> drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h | 19 +++++++++++++
> .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c | 9 ++++--
> .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c | 32 ++++++++++++++++++++--
> .../drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h | 19 +++++++++++++
> drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h | 2 +-
> 5 files changed, 75 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
> index a2333cfe6955..490ce410f6cb 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
> @@ -3,9 +3,28 @@
> #define __NVKM_GR_H__
> #include <core/engine.h>
>
> +struct nvkm_gr_zcull_info {
> + __u32 width_align_pixels;
> + __u32 height_align_pixels;
> + __u32 pixel_squares_by_aliquots;
> + __u32 aliquot_total;
> + __u32 zcull_region_byte_multiplier;
> + __u32 zcull_region_header_size;
> + __u32 zcull_subregion_header_size;
> + __u32 subregion_count;
> + __u32 subregion_width_align_pixels;
> + __u32 subregion_height_align_pixels;
> +
> + __u32 ctxsw_size;
> + __u32 ctxsw_align;
> +};
> +
> struct nvkm_gr {
> const struct nvkm_gr_func *func;
> struct nvkm_engine engine;
> +
> + struct nvkm_gr_zcull_info zcull_info;
> + bool has_zcull_info;
> };
>
> u64 nvkm_gr_units(struct nvkm_gr *);
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
> index ddb57d5e73d6..73844e1e7294 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
> @@ -249,7 +249,7 @@ r535_gr_get_ctxbuf_info(struct r535_gr *gr, int i,
> }
>
> static int
> -r535_gr_get_ctxbufs_info(struct r535_gr *gr)
> +r535_gr_get_ctxbufs_and_zcull_info(struct r535_gr *gr)
Why did you combine those two callbacks? Why not extend struct nvkm_rm_api_gr
with another callback?
> {
> NV2080_CTRL_INTERNAL_STATIC_GR_GET_CONTEXT_BUFFERS_INFO_PARAMS *info;
> struct nvkm_subdev *subdev = &gr->base.engine.subdev;
> @@ -265,6 +265,9 @@ r535_gr_get_ctxbufs_info(struct r535_gr *gr)
> r535_gr_get_ctxbuf_info(gr, i, &info->engineContextBuffersInfo[0].engine[i]);
>
> nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, info);
> +
> + gr->base.has_zcull_info = false;
> +
> return 0;
> }
>
> @@ -312,7 +315,7 @@ r535_gr_oneinit(struct nvkm_gr *base)
> *
> * Also build the information that'll be used to create channel contexts.
> */
> - ret = rm->api->gr->get_ctxbufs_info(gr);
> + ret = rm->api->gr->get_ctxbufs_and_zcull_info(gr);
> if (ret)
> goto done;
>
> @@ -352,5 +355,5 @@ r535_gr_dtor(struct nvkm_gr *base)
>
> const struct nvkm_rm_api_gr
> r535_gr = {
> - .get_ctxbufs_info = r535_gr_get_ctxbufs_info,
> + .get_ctxbufs_and_zcull_info = r535_gr_get_ctxbufs_and_zcull_info,
> };
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
> index b6cced9b8aa1..3e7af2ffece9 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
> @@ -164,9 +164,10 @@ r570_gr_scrubber_init(struct r535_gr *gr)
> }
>
> static int
> -r570_gr_get_ctxbufs_info(struct r535_gr *gr)
> +r570_gr_get_ctxbufs_and_zcull_info(struct r535_gr *gr)
> {
> NV2080_CTRL_INTERNAL_STATIC_GR_GET_CONTEXT_BUFFERS_INFO_PARAMS *info;
> + NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS *zcull_info;
> struct nvkm_subdev *subdev = &gr->base.engine.subdev;
> struct nvkm_gsp *gsp = subdev->device->gsp;
>
> @@ -179,13 +180,40 @@ r570_gr_get_ctxbufs_info(struct r535_gr *gr)
> for (int i = 0; i < ARRAY_SIZE(info->engineContextBuffersInfo[0].engine); i++)
> r535_gr_get_ctxbuf_info(gr, i, &info->engineContextBuffersInfo[0].engine[i]);
>
> + NV2080_CTRL_INTERNAL_ENGINE_CONTEXT_BUFFER_INFO zcull = info->engineContextBuffersInfo[0]
> + .engine[NV0080_CTRL_FIFO_GET_ENGINE_CONTEXT_PROPERTIES_ENGINE_ID_GRAPHICS_ZCULL];
> + gr->base.zcull_info.ctxsw_size = zcull.size;
> + gr->base.zcull_info.ctxsw_align = zcull.alignment;
> +
> nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, info);
> +
> + zcull_info = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.subdevice,
> + NV2080_CTRL_CMD_GR_GET_ZCULL_INFO,
> + sizeof(*zcull_info));
> + if (WARN_ON(IS_ERR(zcull_info)))
What justifies this WARN_ON()? To me this seems like normal error handling, i.e.
it is not a violation of some API invariant, etc. Also, this is in the driver's
probe() path.
> + return PTR_ERR(zcull_info);
> +
> + gr->base.zcull_info.width_align_pixels = zcull_info->widthAlignPixels;
> + gr->base.zcull_info.height_align_pixels = zcull_info->heightAlignPixels;
> + gr->base.zcull_info.pixel_squares_by_aliquots = zcull_info->pixelSquaresByAliquots;
> + gr->base.zcull_info.aliquot_total = zcull_info->aliquotTotal;
> + gr->base.zcull_info.zcull_region_byte_multiplier = zcull_info->zcullRegionByteMultiplier;
> + gr->base.zcull_info.zcull_region_header_size = zcull_info->zcullRegionHeaderSize;
> + gr->base.zcull_info.zcull_subregion_header_size = zcull_info->zcullSubregionHeaderSize;
> + gr->base.zcull_info.subregion_count = zcull_info->subregionCount;
> + gr->base.zcull_info.subregion_width_align_pixels = zcull_info->subregionWidthAlignPixels;
> + gr->base.zcull_info.subregion_height_align_pixels = zcull_info->subregionHeightAlignPixels;
> +
> + nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, zcull_info);
> +
> + gr->base.has_zcull_info = true;
> +
> return 0;
> }
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device
@ 2026-02-13 17:12 ` Danilo Krummrich
0 siblings, 0 replies; 18+ messages in thread
From: Danilo Krummrich @ 2026-02-13 17:12 UTC (permalink / raw)
To: Mel Henning
Cc: Lyude Paul, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Mary Guillemard, dri-devel, nouveau,
linux-kernel
On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote:
> This information will be exposed to userspace in the following commit.
>
> Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
For someone looking at this commit, this commit message is not very useful.
Please add at least a brief explanation of what the patch does and - even more
important - why it does it. See also [1].
[1] https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> ---
> drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h | 19 +++++++++++++
> .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c | 9 ++++--
> .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c | 32 ++++++++++++++++++++--
> .../drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h | 19 +++++++++++++
> drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h | 2 +-
> 5 files changed, 75 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
> index a2333cfe6955..490ce410f6cb 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
> @@ -3,9 +3,28 @@
> #define __NVKM_GR_H__
> #include <core/engine.h>
>
> +struct nvkm_gr_zcull_info {
> + __u32 width_align_pixels;
> + __u32 height_align_pixels;
> + __u32 pixel_squares_by_aliquots;
> + __u32 aliquot_total;
> + __u32 zcull_region_byte_multiplier;
> + __u32 zcull_region_header_size;
> + __u32 zcull_subregion_header_size;
> + __u32 subregion_count;
> + __u32 subregion_width_align_pixels;
> + __u32 subregion_height_align_pixels;
> +
> + __u32 ctxsw_size;
> + __u32 ctxsw_align;
> +};
> +
> struct nvkm_gr {
> const struct nvkm_gr_func *func;
> struct nvkm_engine engine;
> +
> + struct nvkm_gr_zcull_info zcull_info;
> + bool has_zcull_info;
> };
>
> u64 nvkm_gr_units(struct nvkm_gr *);
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
> index ddb57d5e73d6..73844e1e7294 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
> @@ -249,7 +249,7 @@ r535_gr_get_ctxbuf_info(struct r535_gr *gr, int i,
> }
>
> static int
> -r535_gr_get_ctxbufs_info(struct r535_gr *gr)
> +r535_gr_get_ctxbufs_and_zcull_info(struct r535_gr *gr)
Why did you combine those two callbacks? Why not extend struct nvkm_rm_api_gr
with another callback?
> {
> NV2080_CTRL_INTERNAL_STATIC_GR_GET_CONTEXT_BUFFERS_INFO_PARAMS *info;
> struct nvkm_subdev *subdev = &gr->base.engine.subdev;
> @@ -265,6 +265,9 @@ r535_gr_get_ctxbufs_info(struct r535_gr *gr)
> r535_gr_get_ctxbuf_info(gr, i, &info->engineContextBuffersInfo[0].engine[i]);
>
> nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, info);
> +
> + gr->base.has_zcull_info = false;
> +
> return 0;
> }
>
> @@ -312,7 +315,7 @@ r535_gr_oneinit(struct nvkm_gr *base)
> *
> * Also build the information that'll be used to create channel contexts.
> */
> - ret = rm->api->gr->get_ctxbufs_info(gr);
> + ret = rm->api->gr->get_ctxbufs_and_zcull_info(gr);
> if (ret)
> goto done;
>
> @@ -352,5 +355,5 @@ r535_gr_dtor(struct nvkm_gr *base)
>
> const struct nvkm_rm_api_gr
> r535_gr = {
> - .get_ctxbufs_info = r535_gr_get_ctxbufs_info,
> + .get_ctxbufs_and_zcull_info = r535_gr_get_ctxbufs_and_zcull_info,
> };
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
> index b6cced9b8aa1..3e7af2ffece9 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
> @@ -164,9 +164,10 @@ r570_gr_scrubber_init(struct r535_gr *gr)
> }
>
> static int
> -r570_gr_get_ctxbufs_info(struct r535_gr *gr)
> +r570_gr_get_ctxbufs_and_zcull_info(struct r535_gr *gr)
> {
> NV2080_CTRL_INTERNAL_STATIC_GR_GET_CONTEXT_BUFFERS_INFO_PARAMS *info;
> + NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS *zcull_info;
> struct nvkm_subdev *subdev = &gr->base.engine.subdev;
> struct nvkm_gsp *gsp = subdev->device->gsp;
>
> @@ -179,13 +180,40 @@ r570_gr_get_ctxbufs_info(struct r535_gr *gr)
> for (int i = 0; i < ARRAY_SIZE(info->engineContextBuffersInfo[0].engine); i++)
> r535_gr_get_ctxbuf_info(gr, i, &info->engineContextBuffersInfo[0].engine[i]);
>
> + NV2080_CTRL_INTERNAL_ENGINE_CONTEXT_BUFFER_INFO zcull = info->engineContextBuffersInfo[0]
> + .engine[NV0080_CTRL_FIFO_GET_ENGINE_CONTEXT_PROPERTIES_ENGINE_ID_GRAPHICS_ZCULL];
> + gr->base.zcull_info.ctxsw_size = zcull.size;
> + gr->base.zcull_info.ctxsw_align = zcull.alignment;
> +
> nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, info);
> +
> + zcull_info = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.subdevice,
> + NV2080_CTRL_CMD_GR_GET_ZCULL_INFO,
> + sizeof(*zcull_info));
> + if (WARN_ON(IS_ERR(zcull_info)))
What justifies this WARN_ON()? To me this seems like normal error handling, i.e.
it is not a violation of some API invariant, etc. Also, this is in the driver's
probe() path.
> + return PTR_ERR(zcull_info);
> +
> + gr->base.zcull_info.width_align_pixels = zcull_info->widthAlignPixels;
> + gr->base.zcull_info.height_align_pixels = zcull_info->heightAlignPixels;
> + gr->base.zcull_info.pixel_squares_by_aliquots = zcull_info->pixelSquaresByAliquots;
> + gr->base.zcull_info.aliquot_total = zcull_info->aliquotTotal;
> + gr->base.zcull_info.zcull_region_byte_multiplier = zcull_info->zcullRegionByteMultiplier;
> + gr->base.zcull_info.zcull_region_header_size = zcull_info->zcullRegionHeaderSize;
> + gr->base.zcull_info.zcull_subregion_header_size = zcull_info->zcullSubregionHeaderSize;
> + gr->base.zcull_info.subregion_count = zcull_info->subregionCount;
> + gr->base.zcull_info.subregion_width_align_pixels = zcull_info->subregionWidthAlignPixels;
> + gr->base.zcull_info.subregion_height_align_pixels = zcull_info->subregionHeightAlignPixels;
> +
> + nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, zcull_info);
> +
> + gr->base.has_zcull_info = true;
> +
> return 0;
> }
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] drm/nouveau: Add DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO
2026-02-05 18:56 ` Mel Henning
@ 2026-02-13 17:13 ` Danilo Krummrich
-1 siblings, 0 replies; 18+ messages in thread
From: Danilo Krummrich @ 2026-02-13 17:13 UTC (permalink / raw)
To: Mel Henning
Cc: Maarten Lankhorst, Maxime Ripard, Simona Vetter, Mary Guillemard,
dri-devel, nouveau, linux-kernel
On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote:
> +int
> +nouveau_abi16_ioctl_get_zcull_info(ABI16_IOCTL_ARGS)
> +{
> + struct nouveau_drm *drm = nouveau_drm(dev);
> + struct nvkm_gr *gr = nvxx_gr(drm);
> + struct drm_nouveau_get_zcull_info *out = data;
> +
> + if (gr->has_zcull_info) {
> + const struct nvkm_gr_zcull_info *i = &gr->zcull_info;
> +
> + out->width_align_pixels = i->width_align_pixels;
> + out->height_align_pixels = i->height_align_pixels;
> + out->pixel_squares_by_aliquots = i->pixel_squares_by_aliquots;
> + out->aliquot_total = i->aliquot_total;
> + out->zcull_region_byte_multiplier = i->zcull_region_byte_multiplier;
> + out->zcull_region_header_size = i->zcull_region_header_size;
> + out->zcull_subregion_header_size = i->zcull_subregion_header_size;
> + out->subregion_count = i->subregion_count;
> + out->subregion_width_align_pixels = i->subregion_width_align_pixels;
> + out->subregion_height_align_pixels = i->subregion_height_align_pixels;
> + out->ctxsw_size = i->ctxsw_size;
> + out->ctxsw_align = i->ctxsw_align;
> +
> + return 0;
> + } else {
> + return -ENODEV;
ENODEV usually means that that device fell off the bus, I think ENOTTY would be
a better fit.
> + }
> +}
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] drm/nouveau: Add DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO
@ 2026-02-13 17:13 ` Danilo Krummrich
0 siblings, 0 replies; 18+ messages in thread
From: Danilo Krummrich @ 2026-02-13 17:13 UTC (permalink / raw)
To: Mel Henning
Cc: Lyude Paul, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Mary Guillemard, dri-devel, nouveau,
linux-kernel
On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote:
> +int
> +nouveau_abi16_ioctl_get_zcull_info(ABI16_IOCTL_ARGS)
> +{
> + struct nouveau_drm *drm = nouveau_drm(dev);
> + struct nvkm_gr *gr = nvxx_gr(drm);
> + struct drm_nouveau_get_zcull_info *out = data;
> +
> + if (gr->has_zcull_info) {
> + const struct nvkm_gr_zcull_info *i = &gr->zcull_info;
> +
> + out->width_align_pixels = i->width_align_pixels;
> + out->height_align_pixels = i->height_align_pixels;
> + out->pixel_squares_by_aliquots = i->pixel_squares_by_aliquots;
> + out->aliquot_total = i->aliquot_total;
> + out->zcull_region_byte_multiplier = i->zcull_region_byte_multiplier;
> + out->zcull_region_header_size = i->zcull_region_header_size;
> + out->zcull_subregion_header_size = i->zcull_subregion_header_size;
> + out->subregion_count = i->subregion_count;
> + out->subregion_width_align_pixels = i->subregion_width_align_pixels;
> + out->subregion_height_align_pixels = i->subregion_height_align_pixels;
> + out->ctxsw_size = i->ctxsw_size;
> + out->ctxsw_align = i->ctxsw_align;
> +
> + return 0;
> + } else {
> + return -ENODEV;
ENODEV usually means that that device fell off the bus, I think ENOTTY would be
a better fit.
> + }
> +}
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device
2026-02-13 17:12 ` Danilo Krummrich
@ 2026-02-13 21:48 ` M Henning
-1 siblings, 0 replies; 18+ messages in thread
From: M Henning @ 2026-02-13 21:48 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Maarten Lankhorst, Maxime Ripard, Simona Vetter, Mary Guillemard,
dri-devel, nouveau, linux-kernel
On Fri, Feb 13, 2026 at 12:12 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote:
> > This information will be exposed to userspace in the following commit.
> >
> > Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
>
> For someone looking at this commit, this commit message is not very useful.
>
> Please add at least a brief explanation of what the patch does and - even more
> important - why it does it. See also [1].
>
> [1] https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
What I'm struggling with is that I don't know how to do this without
repeating myself. If you want, I can copy-paste my explanation of
zcull here too and then it will appear three times, once in each
commit and once in the cover letter. But that kind of repetition
doesn't seem very helpful to me.
> > ---
> > drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h | 19 +++++++++++++
> > .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c | 9 ++++--
> > .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c | 32 ++++++++++++++++++++--
> > .../drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h | 19 +++++++++++++
> > drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h | 2 +-
> > 5 files changed, 75 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
> > index a2333cfe6955..490ce410f6cb 100644
> > --- a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
> > +++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
> > @@ -3,9 +3,28 @@
> > #define __NVKM_GR_H__
> > #include <core/engine.h>
> >
> > +struct nvkm_gr_zcull_info {
> > + __u32 width_align_pixels;
> > + __u32 height_align_pixels;
> > + __u32 pixel_squares_by_aliquots;
> > + __u32 aliquot_total;
> > + __u32 zcull_region_byte_multiplier;
> > + __u32 zcull_region_header_size;
> > + __u32 zcull_subregion_header_size;
> > + __u32 subregion_count;
> > + __u32 subregion_width_align_pixels;
> > + __u32 subregion_height_align_pixels;
> > +
> > + __u32 ctxsw_size;
> > + __u32 ctxsw_align;
> > +};
> > +
> > struct nvkm_gr {
> > const struct nvkm_gr_func *func;
> > struct nvkm_engine engine;
> > +
> > + struct nvkm_gr_zcull_info zcull_info;
> > + bool has_zcull_info;
> > };
> >
> > u64 nvkm_gr_units(struct nvkm_gr *);
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
> > index ddb57d5e73d6..73844e1e7294 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
> > @@ -249,7 +249,7 @@ r535_gr_get_ctxbuf_info(struct r535_gr *gr, int i,
> > }
> >
> > static int
> > -r535_gr_get_ctxbufs_info(struct r535_gr *gr)
> > +r535_gr_get_ctxbufs_and_zcull_info(struct r535_gr *gr)
>
> Why did you combine those two callbacks? Why not extend struct nvkm_rm_api_gr
> with another callback?
ctxsw_size and ctxsw_align come from
NV2080_CTRL_CMD_INTERNAL_STATIC_KGR_GET_CONTEXT_BUFFERS_INFO, which is
already called by r570_gr_get_ctxbufs., while the rest of the zcull
info comes from
NV0080_CTRL_FIFO_GET_ENGINE_CONTEXT_PROPERTIES_ENGINE_ID_GRAPHICS_ZCULL.
If we add another callback then we either need to:
1) Call GET_CONTEXT_BUFFERS_INFO twice, once for each callback. This
is a little slower and more verbose than calling it once.
or
2) fill out zcull_info partially in r570_gr_get_ctxbufs and partially
in the new callback. Since we fill out only some of the info in each
we now need to handle edge cases where one function is called but not
the other as well as them being called in an arbitrary order.
Because of this, I decided that it was simplest to combine them in a
single call, which avoids repeated rpc calls to the gpu without the
complexity of handling partially complete states.
> > {
> > NV2080_CTRL_INTERNAL_STATIC_GR_GET_CONTEXT_BUFFERS_INFO_PARAMS *info;
> > struct nvkm_subdev *subdev = &gr->base.engine.subdev;
> > @@ -265,6 +265,9 @@ r535_gr_get_ctxbufs_info(struct r535_gr *gr)
> > r535_gr_get_ctxbuf_info(gr, i, &info->engineContextBuffersInfo[0].engine[i]);
> >
> > nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, info);
> > +
> > + gr->base.has_zcull_info = false;
> > +
> > return 0;
> > }
> >
> > @@ -312,7 +315,7 @@ r535_gr_oneinit(struct nvkm_gr *base)
> > *
> > * Also build the information that'll be used to create channel contexts.
> > */
> > - ret = rm->api->gr->get_ctxbufs_info(gr);
> > + ret = rm->api->gr->get_ctxbufs_and_zcull_info(gr);
> > if (ret)
> > goto done;
> >
> > @@ -352,5 +355,5 @@ r535_gr_dtor(struct nvkm_gr *base)
> >
> > const struct nvkm_rm_api_gr
> > r535_gr = {
> > - .get_ctxbufs_info = r535_gr_get_ctxbufs_info,
> > + .get_ctxbufs_and_zcull_info = r535_gr_get_ctxbufs_and_zcull_info,
> > };
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
> > index b6cced9b8aa1..3e7af2ffece9 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
> > @@ -164,9 +164,10 @@ r570_gr_scrubber_init(struct r535_gr *gr)
> > }
> >
> > static int
> > -r570_gr_get_ctxbufs_info(struct r535_gr *gr)
> > +r570_gr_get_ctxbufs_and_zcull_info(struct r535_gr *gr)
> > {
> > NV2080_CTRL_INTERNAL_STATIC_GR_GET_CONTEXT_BUFFERS_INFO_PARAMS *info;
> > + NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS *zcull_info;
> > struct nvkm_subdev *subdev = &gr->base.engine.subdev;
> > struct nvkm_gsp *gsp = subdev->device->gsp;
> >
> > @@ -179,13 +180,40 @@ r570_gr_get_ctxbufs_info(struct r535_gr *gr)
> > for (int i = 0; i < ARRAY_SIZE(info->engineContextBuffersInfo[0].engine); i++)
> > r535_gr_get_ctxbuf_info(gr, i, &info->engineContextBuffersInfo[0].engine[i]);
> >
> > + NV2080_CTRL_INTERNAL_ENGINE_CONTEXT_BUFFER_INFO zcull = info->engineContextBuffersInfo[0]
> > + .engine[NV0080_CTRL_FIFO_GET_ENGINE_CONTEXT_PROPERTIES_ENGINE_ID_GRAPHICS_ZCULL];
> > + gr->base.zcull_info.ctxsw_size = zcull.size;
> > + gr->base.zcull_info.ctxsw_align = zcull.alignment;
> > +
> > nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, info);
> > +
> > + zcull_info = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.subdevice,
> > + NV2080_CTRL_CMD_GR_GET_ZCULL_INFO,
> > + sizeof(*zcull_info));
> > + if (WARN_ON(IS_ERR(zcull_info)))
>
> What justifies this WARN_ON()? To me this seems like normal error handling, i.e.
> it is not a violation of some API invariant, etc. Also, this is in the driver's
> probe() path.
I was just copying the error handling that already exists in this function.
I do think these are weird error cases though - they mean that the gpu
was partially but not fully initialized which shouldn't happen during
normal usage. The only cases I can think of that would trigger this
warning are a kernel bug or an intermittent PCI link, which I think
are both reasonable to warn on.
> > + return PTR_ERR(zcull_info);
> > +
> > + gr->base.zcull_info.width_align_pixels = zcull_info->widthAlignPixels;
> > + gr->base.zcull_info.height_align_pixels = zcull_info->heightAlignPixels;
> > + gr->base.zcull_info.pixel_squares_by_aliquots = zcull_info->pixelSquaresByAliquots;
> > + gr->base.zcull_info.aliquot_total = zcull_info->aliquotTotal;
> > + gr->base.zcull_info.zcull_region_byte_multiplier = zcull_info->zcullRegionByteMultiplier;
> > + gr->base.zcull_info.zcull_region_header_size = zcull_info->zcullRegionHeaderSize;
> > + gr->base.zcull_info.zcull_subregion_header_size = zcull_info->zcullSubregionHeaderSize;
> > + gr->base.zcull_info.subregion_count = zcull_info->subregionCount;
> > + gr->base.zcull_info.subregion_width_align_pixels = zcull_info->subregionWidthAlignPixels;
> > + gr->base.zcull_info.subregion_height_align_pixels = zcull_info->subregionHeightAlignPixels;
> > +
> > + nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, zcull_info);
> > +
> > + gr->base.has_zcull_info = true;
> > +
> > return 0;
> > }
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device
@ 2026-02-13 21:48 ` M Henning
0 siblings, 0 replies; 18+ messages in thread
From: M Henning @ 2026-02-13 21:48 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Lyude Paul, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Mary Guillemard, dri-devel, nouveau,
linux-kernel
On Fri, Feb 13, 2026 at 12:12 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote:
> > This information will be exposed to userspace in the following commit.
> >
> > Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
>
> For someone looking at this commit, this commit message is not very useful.
>
> Please add at least a brief explanation of what the patch does and - even more
> important - why it does it. See also [1].
>
> [1] https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
What I'm struggling with is that I don't know how to do this without
repeating myself. If you want, I can copy-paste my explanation of
zcull here too and then it will appear three times, once in each
commit and once in the cover letter. But that kind of repetition
doesn't seem very helpful to me.
> > ---
> > drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h | 19 +++++++++++++
> > .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c | 9 ++++--
> > .../gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c | 32 ++++++++++++++++++++--
> > .../drm/nouveau/nvkm/subdev/gsp/rm/r570/nvrm/gr.h | 19 +++++++++++++
> > drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/rm.h | 2 +-
> > 5 files changed, 75 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
> > index a2333cfe6955..490ce410f6cb 100644
> > --- a/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
> > +++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/gr.h
> > @@ -3,9 +3,28 @@
> > #define __NVKM_GR_H__
> > #include <core/engine.h>
> >
> > +struct nvkm_gr_zcull_info {
> > + __u32 width_align_pixels;
> > + __u32 height_align_pixels;
> > + __u32 pixel_squares_by_aliquots;
> > + __u32 aliquot_total;
> > + __u32 zcull_region_byte_multiplier;
> > + __u32 zcull_region_header_size;
> > + __u32 zcull_subregion_header_size;
> > + __u32 subregion_count;
> > + __u32 subregion_width_align_pixels;
> > + __u32 subregion_height_align_pixels;
> > +
> > + __u32 ctxsw_size;
> > + __u32 ctxsw_align;
> > +};
> > +
> > struct nvkm_gr {
> > const struct nvkm_gr_func *func;
> > struct nvkm_engine engine;
> > +
> > + struct nvkm_gr_zcull_info zcull_info;
> > + bool has_zcull_info;
> > };
> >
> > u64 nvkm_gr_units(struct nvkm_gr *);
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
> > index ddb57d5e73d6..73844e1e7294 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/gr.c
> > @@ -249,7 +249,7 @@ r535_gr_get_ctxbuf_info(struct r535_gr *gr, int i,
> > }
> >
> > static int
> > -r535_gr_get_ctxbufs_info(struct r535_gr *gr)
> > +r535_gr_get_ctxbufs_and_zcull_info(struct r535_gr *gr)
>
> Why did you combine those two callbacks? Why not extend struct nvkm_rm_api_gr
> with another callback?
ctxsw_size and ctxsw_align come from
NV2080_CTRL_CMD_INTERNAL_STATIC_KGR_GET_CONTEXT_BUFFERS_INFO, which is
already called by r570_gr_get_ctxbufs., while the rest of the zcull
info comes from
NV0080_CTRL_FIFO_GET_ENGINE_CONTEXT_PROPERTIES_ENGINE_ID_GRAPHICS_ZCULL.
If we add another callback then we either need to:
1) Call GET_CONTEXT_BUFFERS_INFO twice, once for each callback. This
is a little slower and more verbose than calling it once.
or
2) fill out zcull_info partially in r570_gr_get_ctxbufs and partially
in the new callback. Since we fill out only some of the info in each
we now need to handle edge cases where one function is called but not
the other as well as them being called in an arbitrary order.
Because of this, I decided that it was simplest to combine them in a
single call, which avoids repeated rpc calls to the gpu without the
complexity of handling partially complete states.
> > {
> > NV2080_CTRL_INTERNAL_STATIC_GR_GET_CONTEXT_BUFFERS_INFO_PARAMS *info;
> > struct nvkm_subdev *subdev = &gr->base.engine.subdev;
> > @@ -265,6 +265,9 @@ r535_gr_get_ctxbufs_info(struct r535_gr *gr)
> > r535_gr_get_ctxbuf_info(gr, i, &info->engineContextBuffersInfo[0].engine[i]);
> >
> > nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, info);
> > +
> > + gr->base.has_zcull_info = false;
> > +
> > return 0;
> > }
> >
> > @@ -312,7 +315,7 @@ r535_gr_oneinit(struct nvkm_gr *base)
> > *
> > * Also build the information that'll be used to create channel contexts.
> > */
> > - ret = rm->api->gr->get_ctxbufs_info(gr);
> > + ret = rm->api->gr->get_ctxbufs_and_zcull_info(gr);
> > if (ret)
> > goto done;
> >
> > @@ -352,5 +355,5 @@ r535_gr_dtor(struct nvkm_gr *base)
> >
> > const struct nvkm_rm_api_gr
> > r535_gr = {
> > - .get_ctxbufs_info = r535_gr_get_ctxbufs_info,
> > + .get_ctxbufs_and_zcull_info = r535_gr_get_ctxbufs_and_zcull_info,
> > };
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
> > index b6cced9b8aa1..3e7af2ffece9 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/gr.c
> > @@ -164,9 +164,10 @@ r570_gr_scrubber_init(struct r535_gr *gr)
> > }
> >
> > static int
> > -r570_gr_get_ctxbufs_info(struct r535_gr *gr)
> > +r570_gr_get_ctxbufs_and_zcull_info(struct r535_gr *gr)
> > {
> > NV2080_CTRL_INTERNAL_STATIC_GR_GET_CONTEXT_BUFFERS_INFO_PARAMS *info;
> > + NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS *zcull_info;
> > struct nvkm_subdev *subdev = &gr->base.engine.subdev;
> > struct nvkm_gsp *gsp = subdev->device->gsp;
> >
> > @@ -179,13 +180,40 @@ r570_gr_get_ctxbufs_info(struct r535_gr *gr)
> > for (int i = 0; i < ARRAY_SIZE(info->engineContextBuffersInfo[0].engine); i++)
> > r535_gr_get_ctxbuf_info(gr, i, &info->engineContextBuffersInfo[0].engine[i]);
> >
> > + NV2080_CTRL_INTERNAL_ENGINE_CONTEXT_BUFFER_INFO zcull = info->engineContextBuffersInfo[0]
> > + .engine[NV0080_CTRL_FIFO_GET_ENGINE_CONTEXT_PROPERTIES_ENGINE_ID_GRAPHICS_ZCULL];
> > + gr->base.zcull_info.ctxsw_size = zcull.size;
> > + gr->base.zcull_info.ctxsw_align = zcull.alignment;
> > +
> > nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, info);
> > +
> > + zcull_info = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.subdevice,
> > + NV2080_CTRL_CMD_GR_GET_ZCULL_INFO,
> > + sizeof(*zcull_info));
> > + if (WARN_ON(IS_ERR(zcull_info)))
>
> What justifies this WARN_ON()? To me this seems like normal error handling, i.e.
> it is not a violation of some API invariant, etc. Also, this is in the driver's
> probe() path.
I was just copying the error handling that already exists in this function.
I do think these are weird error cases though - they mean that the gpu
was partially but not fully initialized which shouldn't happen during
normal usage. The only cases I can think of that would trigger this
warning are a kernel bug or an intermittent PCI link, which I think
are both reasonable to warn on.
> > + return PTR_ERR(zcull_info);
> > +
> > + gr->base.zcull_info.width_align_pixels = zcull_info->widthAlignPixels;
> > + gr->base.zcull_info.height_align_pixels = zcull_info->heightAlignPixels;
> > + gr->base.zcull_info.pixel_squares_by_aliquots = zcull_info->pixelSquaresByAliquots;
> > + gr->base.zcull_info.aliquot_total = zcull_info->aliquotTotal;
> > + gr->base.zcull_info.zcull_region_byte_multiplier = zcull_info->zcullRegionByteMultiplier;
> > + gr->base.zcull_info.zcull_region_header_size = zcull_info->zcullRegionHeaderSize;
> > + gr->base.zcull_info.zcull_subregion_header_size = zcull_info->zcullSubregionHeaderSize;
> > + gr->base.zcull_info.subregion_count = zcull_info->subregionCount;
> > + gr->base.zcull_info.subregion_width_align_pixels = zcull_info->subregionWidthAlignPixels;
> > + gr->base.zcull_info.subregion_height_align_pixels = zcull_info->subregionHeightAlignPixels;
> > +
> > + nvkm_gsp_rm_ctrl_done(&gsp->internal.device.subdevice, zcull_info);
> > +
> > + gr->base.has_zcull_info = true;
> > +
> > return 0;
> > }
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] drm/nouveau: Add DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO
2026-02-13 17:13 ` Danilo Krummrich
@ 2026-02-13 22:06 ` M Henning
-1 siblings, 0 replies; 18+ messages in thread
From: M Henning @ 2026-02-13 22:06 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Maarten Lankhorst, Maxime Ripard, Simona Vetter, Mary Guillemard,
dri-devel, nouveau, linux-kernel
On Fri, Feb 13, 2026 at 12:14 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote:
> > +int
> > +nouveau_abi16_ioctl_get_zcull_info(ABI16_IOCTL_ARGS)
> > +{
> > + struct nouveau_drm *drm = nouveau_drm(dev);
> > + struct nvkm_gr *gr = nvxx_gr(drm);
> > + struct drm_nouveau_get_zcull_info *out = data;
> > +
> > + if (gr->has_zcull_info) {
> > + const struct nvkm_gr_zcull_info *i = &gr->zcull_info;
> > +
> > + out->width_align_pixels = i->width_align_pixels;
> > + out->height_align_pixels = i->height_align_pixels;
> > + out->pixel_squares_by_aliquots = i->pixel_squares_by_aliquots;
> > + out->aliquot_total = i->aliquot_total;
> > + out->zcull_region_byte_multiplier = i->zcull_region_byte_multiplier;
> > + out->zcull_region_header_size = i->zcull_region_header_size;
> > + out->zcull_subregion_header_size = i->zcull_subregion_header_size;
> > + out->subregion_count = i->subregion_count;
> > + out->subregion_width_align_pixels = i->subregion_width_align_pixels;
> > + out->subregion_height_align_pixels = i->subregion_height_align_pixels;
> > + out->ctxsw_size = i->ctxsw_size;
> > + out->ctxsw_align = i->ctxsw_align;
> > +
> > + return 0;
> > + } else {
> > + return -ENODEV;
>
> ENODEV usually means that that device fell off the bus, I think ENOTTY would be
> a better fit.
Done.
> > + }
> > +}
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 2/2] drm/nouveau: Add DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO
@ 2026-02-13 22:06 ` M Henning
0 siblings, 0 replies; 18+ messages in thread
From: M Henning @ 2026-02-13 22:06 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Lyude Paul, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Mary Guillemard, dri-devel, nouveau,
linux-kernel
On Fri, Feb 13, 2026 at 12:14 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote:
> > +int
> > +nouveau_abi16_ioctl_get_zcull_info(ABI16_IOCTL_ARGS)
> > +{
> > + struct nouveau_drm *drm = nouveau_drm(dev);
> > + struct nvkm_gr *gr = nvxx_gr(drm);
> > + struct drm_nouveau_get_zcull_info *out = data;
> > +
> > + if (gr->has_zcull_info) {
> > + const struct nvkm_gr_zcull_info *i = &gr->zcull_info;
> > +
> > + out->width_align_pixels = i->width_align_pixels;
> > + out->height_align_pixels = i->height_align_pixels;
> > + out->pixel_squares_by_aliquots = i->pixel_squares_by_aliquots;
> > + out->aliquot_total = i->aliquot_total;
> > + out->zcull_region_byte_multiplier = i->zcull_region_byte_multiplier;
> > + out->zcull_region_header_size = i->zcull_region_header_size;
> > + out->zcull_subregion_header_size = i->zcull_subregion_header_size;
> > + out->subregion_count = i->subregion_count;
> > + out->subregion_width_align_pixels = i->subregion_width_align_pixels;
> > + out->subregion_height_align_pixels = i->subregion_height_align_pixels;
> > + out->ctxsw_size = i->ctxsw_size;
> > + out->ctxsw_align = i->ctxsw_align;
> > +
> > + return 0;
> > + } else {
> > + return -ENODEV;
>
> ENODEV usually means that that device fell off the bus, I think ENOTTY would be
> a better fit.
Done.
> > + }
> > +}
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device
2026-02-13 21:48 ` M Henning
@ 2026-02-13 22:22 ` Danilo Krummrich
-1 siblings, 0 replies; 18+ messages in thread
From: Danilo Krummrich @ 2026-02-13 22:22 UTC (permalink / raw)
To: M Henning
Cc: Maarten Lankhorst, Maxime Ripard, Simona Vetter, Mary Guillemard,
dri-devel, nouveau, linux-kernel
On Fri Feb 13, 2026 at 10:48 PM CET, M Henning wrote:
> On Fri, Feb 13, 2026 at 12:12 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote:
>> > This information will be exposed to userspace in the following commit.
>> >
>> > Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
>>
>> For someone looking at this commit, this commit message is not very useful.
>>
>> Please add at least a brief explanation of what the patch does and - even more
>> important - why it does it. See also [1].
>>
>> [1] https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
>
> What I'm struggling with is that I don't know how to do this without
> repeating myself. If you want, I can copy-paste my explanation of
> zcull here too and then it will appear three times, once in each
> commit and once in the cover letter. But that kind of repetition
> doesn't seem very helpful to me.
Again, the commit message should explain what the commit does and why. For
instance, I asked you why you did combine those two callbacks below.
The commit message could mention this, e.g. it could be something along the lines
of:
"Add struct nvkm_gr_zcull_info, which serves as abstraction layer between the
corresponding uAPI (added in a subsequent patch) and the firmware (version
specific) structure.
This is needed in order to not leak the uAPI layer into nvkm. Also note that we
are bypassing the nvif layer, since ...
Also note that we reuse the get_ctxbufs_info() callback, since ..."
I.e. make it obvious to maintainers what's going on and what's the motivation
for the patch and it's implementation details.
> Because of this, I decided that it was simplest to combine them in a
> single call, which avoids repeated rpc calls to the gpu without the
> complexity of handling partially complete states.
Ok, that seems reasonable.
>> > + if (WARN_ON(IS_ERR(zcull_info)))
>>
>> What justifies this WARN_ON()? To me this seems like normal error handling, i.e.
>> it is not a violation of some API invariant, etc. Also, this is in the driver's
>> probe() path.
>
> I was just copying the error handling that already exists in this function.
>
> I do think these are weird error cases though - they mean that the gpu
> was partially but not fully initialized which shouldn't happen during
> normal usage. The only cases I can think of that would trigger this
> warning are a kernel bug or an intermittent PCI link, which I think
> are both reasonable to warn on.
It could also be that the firmware is buggy, etc. In any case, I don't see that
a WARN_ON() is justified. Please use dev_err() instead.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device
@ 2026-02-13 22:22 ` Danilo Krummrich
0 siblings, 0 replies; 18+ messages in thread
From: Danilo Krummrich @ 2026-02-13 22:22 UTC (permalink / raw)
To: M Henning
Cc: Lyude Paul, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Mary Guillemard, dri-devel, nouveau,
linux-kernel
On Fri Feb 13, 2026 at 10:48 PM CET, M Henning wrote:
> On Fri, Feb 13, 2026 at 12:12 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote:
>> > This information will be exposed to userspace in the following commit.
>> >
>> > Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
>>
>> For someone looking at this commit, this commit message is not very useful.
>>
>> Please add at least a brief explanation of what the patch does and - even more
>> important - why it does it. See also [1].
>>
>> [1] https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
>
> What I'm struggling with is that I don't know how to do this without
> repeating myself. If you want, I can copy-paste my explanation of
> zcull here too and then it will appear three times, once in each
> commit and once in the cover letter. But that kind of repetition
> doesn't seem very helpful to me.
Again, the commit message should explain what the commit does and why. For
instance, I asked you why you did combine those two callbacks below.
The commit message could mention this, e.g. it could be something along the lines
of:
"Add struct nvkm_gr_zcull_info, which serves as abstraction layer between the
corresponding uAPI (added in a subsequent patch) and the firmware (version
specific) structure.
This is needed in order to not leak the uAPI layer into nvkm. Also note that we
are bypassing the nvif layer, since ...
Also note that we reuse the get_ctxbufs_info() callback, since ..."
I.e. make it obvious to maintainers what's going on and what's the motivation
for the patch and it's implementation details.
> Because of this, I decided that it was simplest to combine them in a
> single call, which avoids repeated rpc calls to the gpu without the
> complexity of handling partially complete states.
Ok, that seems reasonable.
>> > + if (WARN_ON(IS_ERR(zcull_info)))
>>
>> What justifies this WARN_ON()? To me this seems like normal error handling, i.e.
>> it is not a violation of some API invariant, etc. Also, this is in the driver's
>> probe() path.
>
> I was just copying the error handling that already exists in this function.
>
> I do think these are weird error cases though - they mean that the gpu
> was partially but not fully initialized which shouldn't happen during
> normal usage. The only cases I can think of that would trigger this
> warning are a kernel bug or an intermittent PCI link, which I think
> are both reasonable to warn on.
It could also be that the firmware is buggy, etc. In any case, I don't see that
a WARN_ON() is justified. Please use dev_err() instead.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device
2026-02-13 22:22 ` Danilo Krummrich
@ 2026-02-13 23:11 ` M Henning
-1 siblings, 0 replies; 18+ messages in thread
From: M Henning @ 2026-02-13 23:11 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Maarten Lankhorst, Maxime Ripard, Simona Vetter, Mary Guillemard,
dri-devel, nouveau, linux-kernel
On Fri, Feb 13, 2026 at 5:22 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Fri Feb 13, 2026 at 10:48 PM CET, M Henning wrote:
> > On Fri, Feb 13, 2026 at 12:12 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >>
> >> On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote:
> >> > This information will be exposed to userspace in the following commit.
> >> >
> >> > Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
> >>
> >> For someone looking at this commit, this commit message is not very useful.
> >>
> >> Please add at least a brief explanation of what the patch does and - even more
> >> important - why it does it. See also [1].
> >>
> >> [1] https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> >
> > What I'm struggling with is that I don't know how to do this without
> > repeating myself. If you want, I can copy-paste my explanation of
> > zcull here too and then it will appear three times, once in each
> > commit and once in the cover letter. But that kind of repetition
> > doesn't seem very helpful to me.
>
> Again, the commit message should explain what the commit does and why. For
> instance, I asked you why you did combine those two callbacks below.
>
> The commit message could mention this, e.g. it could be something along the lines
> of:
>
> "Add struct nvkm_gr_zcull_info, which serves as abstraction layer between the
> corresponding uAPI (added in a subsequent patch) and the firmware (version
> specific) structure.
>
> This is needed in order to not leak the uAPI layer into nvkm. Also note that we
> are bypassing the nvif layer, since ...
>
> Also note that we reuse the get_ctxbufs_info() callback, since ..."
>
> I.e. make it obvious to maintainers what's going on and what's the motivation
> for the patch and it's implementation details.
Okay, I'll give it another go.
> > Because of this, I decided that it was simplest to combine them in a
> > single call, which avoids repeated rpc calls to the gpu without the
> > complexity of handling partially complete states.
>
> Ok, that seems reasonable.
>
> >> > + if (WARN_ON(IS_ERR(zcull_info)))
> >>
> >> What justifies this WARN_ON()? To me this seems like normal error handling, i.e.
> >> it is not a violation of some API invariant, etc. Also, this is in the driver's
> >> probe() path.
> >
> > I was just copying the error handling that already exists in this function.
> >
> > I do think these are weird error cases though - they mean that the gpu
> > was partially but not fully initialized which shouldn't happen during
> > normal usage. The only cases I can think of that would trigger this
> > warning are a kernel bug or an intermittent PCI link, which I think
> > are both reasonable to warn on.
>
> It could also be that the firmware is buggy, etc. In any case, I don't see that
> a WARN_ON() is justified. Please use dev_err() instead.
Okay. I've now written this as:
if (IS_ERR(zcull_info)) {
nvdev_error(gr->base.engine.subdev.device, "could not fetch zcull info\n");
return PTR_ERR(zcull_info);
}
since nouveau seems to use its own nvdev_error() macro over dev_err()
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device
@ 2026-02-13 23:11 ` M Henning
0 siblings, 0 replies; 18+ messages in thread
From: M Henning @ 2026-02-13 23:11 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Lyude Paul, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Mary Guillemard, dri-devel, nouveau,
linux-kernel
On Fri, Feb 13, 2026 at 5:22 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Fri Feb 13, 2026 at 10:48 PM CET, M Henning wrote:
> > On Fri, Feb 13, 2026 at 12:12 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >>
> >> On Thu Feb 5, 2026 at 7:56 PM CET, Mel Henning wrote:
> >> > This information will be exposed to userspace in the following commit.
> >> >
> >> > Signed-off-by: Mel Henning <mhenning@darkrefraction.com>
> >>
> >> For someone looking at this commit, this commit message is not very useful.
> >>
> >> Please add at least a brief explanation of what the patch does and - even more
> >> important - why it does it. See also [1].
> >>
> >> [1] https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> >
> > What I'm struggling with is that I don't know how to do this without
> > repeating myself. If you want, I can copy-paste my explanation of
> > zcull here too and then it will appear three times, once in each
> > commit and once in the cover letter. But that kind of repetition
> > doesn't seem very helpful to me.
>
> Again, the commit message should explain what the commit does and why. For
> instance, I asked you why you did combine those two callbacks below.
>
> The commit message could mention this, e.g. it could be something along the lines
> of:
>
> "Add struct nvkm_gr_zcull_info, which serves as abstraction layer between the
> corresponding uAPI (added in a subsequent patch) and the firmware (version
> specific) structure.
>
> This is needed in order to not leak the uAPI layer into nvkm. Also note that we
> are bypassing the nvif layer, since ...
>
> Also note that we reuse the get_ctxbufs_info() callback, since ..."
>
> I.e. make it obvious to maintainers what's going on and what's the motivation
> for the patch and it's implementation details.
Okay, I'll give it another go.
> > Because of this, I decided that it was simplest to combine them in a
> > single call, which avoids repeated rpc calls to the gpu without the
> > complexity of handling partially complete states.
>
> Ok, that seems reasonable.
>
> >> > + if (WARN_ON(IS_ERR(zcull_info)))
> >>
> >> What justifies this WARN_ON()? To me this seems like normal error handling, i.e.
> >> it is not a violation of some API invariant, etc. Also, this is in the driver's
> >> probe() path.
> >
> > I was just copying the error handling that already exists in this function.
> >
> > I do think these are weird error cases though - they mean that the gpu
> > was partially but not fully initialized which shouldn't happen during
> > normal usage. The only cases I can think of that would trigger this
> > warning are a kernel bug or an intermittent PCI link, which I think
> > are both reasonable to warn on.
>
> It could also be that the firmware is buggy, etc. In any case, I don't see that
> a WARN_ON() is justified. Please use dev_err() instead.
Okay. I've now written this as:
if (IS_ERR(zcull_info)) {
nvdev_error(gr->base.engine.subdev.device, "could not fetch zcull info\n");
return PTR_ERR(zcull_info);
}
since nouveau seems to use its own nvdev_error() macro over dev_err()
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-02-13 23:11 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-05 18:56 [PATCH v2 0/2] drm/nouveau: zcull support Mel Henning
2026-02-05 18:56 ` Mel Henning
2026-02-05 18:56 ` [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device Mel Henning
2026-02-05 18:56 ` Mel Henning
2026-02-13 17:12 ` Danilo Krummrich
2026-02-13 17:12 ` Danilo Krummrich
2026-02-13 21:48 ` M Henning
2026-02-13 21:48 ` M Henning
2026-02-13 22:22 ` Danilo Krummrich
2026-02-13 22:22 ` Danilo Krummrich
2026-02-13 23:11 ` M Henning
2026-02-13 23:11 ` M Henning
2026-02-05 18:56 ` [PATCH v2 2/2] drm/nouveau: Add DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO Mel Henning
2026-02-05 18:56 ` Mel Henning
2026-02-13 17:13 ` Danilo Krummrich
2026-02-13 17:13 ` Danilo Krummrich
2026-02-13 22:06 ` M Henning
2026-02-13 22:06 ` M Henning
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.