On Fri, Feb 13, 2026 at 12:12 PM Danilo Krummrich <[email protected]> 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 <[email protected]>
>
> 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;
> > }
