Re: [PATCH v2 1/2] drm/nouveau: Fetch zcull info from device
From: Danilo Krummrich
Date: Fri Feb 13 2026 - 12:13:00 EST
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@xxxxxxxxxxxxxxxxxx>
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;
> }