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