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 > --- > 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; > }
