On Thu, 28 Sept 2023 at 07:55, Faith Ekstrand
<faith.ekstrand@xxxxxxxxxxxxx> wrote:
>
> On Wed, 2023-09-27 at 03:22 +0200, Danilo Krummrich wrote:
> > Report the maximum number of IBs that can be pushed with a single
> > DRM_IOCTL_NOUVEAU_EXEC through DRM_IOCTL_NOUVEAU_GETPARAM.
> >
> > While the maximum number of IBs per ring might vary between chipsets,
> > the kernel will make sure that userspace can only push a fraction of
> > the
> > maximum number of IBs per ring per job, such that we avoid a
> > situation
> > where there's only a single job occupying the ring, which could
> > potentially lead to the ring run dry.
> >
> > Using DRM_IOCTL_NOUVEAU_GETPARAM to report the maximum number of IBs
> > that can be pushed with a single DRM_IOCTL_NOUVEAU_EXEC implies that
> > all channels of a given device have the same ring size.
>
> There's a bunch of nouveau kernel details I don't know here but the
> interface looks good and I prefer it to a #define in the header.
>
> Acked-by: Faith Ekstrand <faith.ekstrand@xxxxxxxxxxxxx>
For the series
Reviewed-by: Dave Airlie <airlied@xxxxxxxxxx>
we should probably land this in drm-misc-fixes, since it would be good
to have in 6.6
Dave.
>
>
> > Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx>
> > ---
> > drivers/gpu/drm/nouveau/nouveau_abi16.c | 19 +++++++++++++++++++
> > drivers/gpu/drm/nouveau/nouveau_chan.c | 2 +-
> > drivers/gpu/drm/nouveau/nouveau_dma.h | 3 +++
> > drivers/gpu/drm/nouveau/nouveau_exec.c | 7 ++++---
> > drivers/gpu/drm/nouveau/nouveau_exec.h | 5 +++++
> > include/uapi/drm/nouveau_drm.h | 10 ++++++++++
> > 6 files changed, 42 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c
> > b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> > index 30afbec9e3b1..1a198689b391 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
> > @@ -31,6 +31,7 @@
> >
> > #include "nouveau_drv.h"
> > #include "nouveau_dma.h"
> > +#include "nouveau_exec.h"
> > #include "nouveau_gem.h"
> > #include "nouveau_chan.h"
> > #include "nouveau_abi16.h"
> > @@ -183,6 +184,20 @@ nouveau_abi16_fini(struct nouveau_abi16 *abi16)
> > cli->abi16 = NULL;
> > }
> >
> > +static inline unsigned int
> > +getparam_dma_ib_max(struct nvif_device *device)
> > +{
> > + const struct nvif_mclass dmas[] = {
> > + { NV03_CHANNEL_DMA, 0 },
> > + { NV10_CHANNEL_DMA, 0 },
> > + { NV17_CHANNEL_DMA, 0 },
> > + { NV40_CHANNEL_DMA, 0 },
> > + {}
> > + };
> > +
> > + return nvif_mclass(&device->object, dmas) < 0 ?
> > NV50_DMA_IB_MAX : 0;
> > +}
> > +
> > int
> > nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
> > {
> > @@ -247,6 +262,10 @@ nouveau_abi16_ioctl_getparam(ABI16_IOCTL_ARGS)
> > case NOUVEAU_GETPARAM_GRAPH_UNITS:
> > getparam->value = nvkm_gr_units(gr);
> > break;
> > + case NOUVEAU_GETPARAM_EXEC_PUSH_MAX:
> > + getparam->value = getparam_dma_ib_max(device) /
> > + NOUVEAU_EXEC_PUSH_MAX_DIV;
> > + break;
> > default:
> > NV_PRINTK(dbg, cli, "unknown parameter %lld\n",
> > getparam->param);
> > return -EINVAL;
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
> > b/drivers/gpu/drm/nouveau/nouveau_chan.c
> > index ac56f4689ee3..c3c2ab887978 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_chan.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
> > @@ -456,7 +456,7 @@ nouveau_channel_init(struct nouveau_channel
> > *chan, u32 vram, u32 gart)
> > chan->user_get = 0x44;
> > chan->user_get_hi = 0x60;
> > chan->dma.ib_base = 0x10000 / 4;
> > - chan->dma.ib_max = (0x02000 / 8) - 1;
> > + chan->dma.ib_max = NV50_DMA_IB_MAX;
> > chan->dma.ib_put = 0;
> > chan->dma.ib_free = chan->dma.ib_max - chan-
> > >dma.ib_put;
> > chan->dma.max = chan->dma.ib_base;
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_dma.h
> > b/drivers/gpu/drm/nouveau/nouveau_dma.h
> > index 1744d95b233e..c52cda82353e 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_dma.h
> > +++ b/drivers/gpu/drm/nouveau/nouveau_dma.h
> > @@ -49,6 +49,9 @@ void nv50_dma_push(struct nouveau_channel *, u64
> > addr, u32 length,
> > /* Maximum push buffer size. */
> > #define NV50_DMA_PUSH_MAX_LENGTH 0x7fffff
> >
> > +/* Maximum IBs per ring. */
> > +#define NV50_DMA_IB_MAX ((0x02000 / 8) - 1)
> > +
> > /* Object handles - for stuff that's doesn't use handle == oclass.
> > */
> > enum {
> > NvDmaFB = 0x80000002,
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.c
> > b/drivers/gpu/drm/nouveau/nouveau_exec.c
> > index ba6913a3efb6..5b5c4a77b8e6 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_exec.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_exec.c
> > @@ -346,7 +346,7 @@ nouveau_exec_ioctl_exec(struct drm_device *dev,
> > struct nouveau_channel *chan = NULL;
> > struct nouveau_exec_job_args args = {};
> > struct drm_nouveau_exec *req = data;
> > - int ret = 0;
> > + int push_max, ret = 0;
> >
> > if (unlikely(!abi16))
> > return -ENOMEM;
> > @@ -371,9 +371,10 @@ nouveau_exec_ioctl_exec(struct drm_device *dev,
> > if (!chan->dma.ib_max)
> > return nouveau_abi16_put(abi16, -ENOSYS);
> >
> > - if (unlikely(req->push_count > NOUVEAU_GEM_MAX_PUSH)) {
> > + push_max = chan->dma.ib_max / NOUVEAU_EXEC_PUSH_MAX_DIV;
> > + if (unlikely(req->push_count > push_max)) {
> > NV_PRINTK(err, cli, "pushbuf push count exceeds
> > limit: %d max %d\n",
> > - req->push_count, NOUVEAU_GEM_MAX_PUSH);
> > + req->push_count, push_max);
> > return nouveau_abi16_put(abi16, -EINVAL);
> > }
> >
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_exec.h
> > b/drivers/gpu/drm/nouveau/nouveau_exec.h
> > index b815de2428f3..c6270452e4b5 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_exec.h
> > +++ b/drivers/gpu/drm/nouveau/nouveau_exec.h
> > @@ -6,6 +6,11 @@
> > #include "nouveau_drv.h"
> > #include "nouveau_sched.h"
> >
> > +/* Divider to limit the number of IBs per job to half the size of
> > the ring in
> > + * order to avoid the ring running dry between submissions.
> > + */
> > +#define NOUVEAU_EXEC_PUSH_MAX_DIV 2
> > +
> > struct nouveau_exec_job_args {
> > struct drm_file *file_priv;
> > struct nouveau_sched_entity *sched_entity;
> > diff --git a/include/uapi/drm/nouveau_drm.h
> > b/include/uapi/drm/nouveau_drm.h
> > index 8d7402c13e56..eaf9f248619f 100644
> > --- a/include/uapi/drm/nouveau_drm.h
> > +++ b/include/uapi/drm/nouveau_drm.h
> > @@ -44,6 +44,16 @@ extern "C" {
> > #define NOUVEAU_GETPARAM_PTIMER_TIME 14
> > #define NOUVEAU_GETPARAM_HAS_BO_USAGE 15
> > #define NOUVEAU_GETPARAM_HAS_PAGEFLIP 16
> > +
> > +/**
> > + * @NOUVEAU_GETPARAM_EXEC_PUSH_MAX
> > + *
> > + * Query the maximum amount of IBs that can be pushed through a
> > single
> > + * &drm_nouveau_exec structure and hence a single
> > &DRM_IOCTL_NOUVEAU_EXEC
> > + * ioctl().
> > + */
> > +#define NOUVEAU_GETPARAM_EXEC_PUSH_MAX 17
> > +
> > struct drm_nouveau_getparam {
> > __u64 param;
> > __u64 value;
>
