Re: [PATCH 1/2] drm/nouveau: Add DRM_IOCTL_NOUVEAU_GET_ZCULL_INFO

[Danilo Krummrich <dakr@kernel.org>]

On Tue, Mar 25, 2025 at 07:40:56PM -0400, M Henning wrote:
> Okay, that sounds reasonable since I don't expect this to change very quickly.
> 
> Since I don't fully understand, is the suggestion here to:
> 1) add the interface as a function on nvkm_gr using the nvkm_gr_func
> vtable and store the actual data on r535_gr
> or
> 2) add the interface to NVIF (which IF?) and store the actual data on nvkm_gr
> ?

I think we want both.

1) I think the suggestion was to store the data directly in nvkm_gr, however the
   structure is indeed specific to r535, so I think, unfortunately, we need the
   vtable and store that data in r535_gr.

2) Yes, this should be passed through nvif. Unfortunately, I think it would need
   to be a whole new one (similar to the fifo one).

Maybe Ben can provide you some additional pointers one this? Mayber he can
suggest a shortcut, since he has patches queued to simplify the whole interface.

> 
> (Sorry, I don't understand how these layers are intended to fit together.))
> 
> On Thu, Mar 20, 2025 at 4:02 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > On Fri, Mar 21, 2025 at 05:57:55AM +1000, Ben Skeggs wrote:
> > > On 21/3/25 04:18, Danilo Krummrich wrote:
> > >
> > > > Hi Mel,
> > > >
> > > > On Wed, Mar 12, 2025 at 05:36:14PM -0400, Mel Henning wrote:
> > >
> > > > > @@ -72,6 +75,9 @@ struct nvkm_device {
> > > > >                   bool armed;
> > > > >                   bool legacy_done;
> > > > >           } intr;
> > > > > +
> > > > > + bool has_zcull_info;
> > > > > + struct drm_nouveau_get_zcull_info zcull_info;
> > > > This is bypassing the nvif layer entirely. I think you should store the contents
> > > > of struct NV2080_CTRL_GR_GET_ZCULL_INFO_PARAMS in struct r535_gr and have an
> > > > nvif interface to access the data.
> > >
> > > I agree here, though nvkm_gr would be a better choice for a couple of
> > > reasons, not least that it's possible (and should be reasonably trivial) to
> > > support this on earlier GPUs - should someone desire to at a later point.
> >
> > I agree, if the interface is stable enough -- I don't know whether this is prone
> > to change or not.