Re: [Nouveau] [PATCH drm-next v4 14/14] drm/nouveau: debugfs: implement DRM GPU VA debugfs
* Danilo Krummrich [230606 18:32]:
> Provide the driver indirection iterating over all DRM GPU VA spaces to
> enable the common 'gpuvas' debugfs file for dumping DRM GPU VA spaces.
>
> Signed-off-by: Danilo Krummrich
> ---
> drivers/gpu/drm/nouveau/nouveau_debugfs.c | 39 +++
> 1 file changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
> b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
> index 99d022a91afc..053f703f2f68 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
> @@ -203,6 +203,44 @@ nouveau_debugfs_pstate_open(struct inode *inode, struct
> file *file)
> return single_open(file, nouveau_debugfs_pstate_get, inode->i_private);
> }
>
> +static void
> +nouveau_debugfs_gpuva_regions(struct seq_file *m, struct nouveau_uvmm *uvmm)
> +{
> + MA_STATE(mas, >region_mt, 0, 0);
> + struct nouveau_uvma_region *reg;
> +
> + seq_puts (m, " VA regions | start | range |
> end\n");
> + seq_puts (m,
> "\n");
rcu_read_lock();
> + mas_for_each(, reg, ULONG_MAX)
> + seq_printf(m, " | 0x%016llx | 0x%016llx |
> 0x%016llx\n",
> +reg->va.addr, reg->va.range, reg->va.addr +
> reg->va.range);
rcu_read_unlock();
> +}
> +
> +static int
> +nouveau_debugfs_gpuva(struct seq_file *m, void *data)
> +{
> + struct drm_info_node *node = (struct drm_info_node *) m->private;
> + struct nouveau_drm *drm = nouveau_drm(node->minor->dev);
> + struct nouveau_cli *cli;
> +
> + mutex_lock(>clients_lock);
> + list_for_each_entry(cli, >clients, head) {
> + struct nouveau_uvmm *uvmm = nouveau_cli_uvmm(cli);
> +
> + if (!uvmm)
> + continue;
> +
> + nouveau_uvmm_lock(uvmm);
> + drm_debugfs_gpuva_info(m, >umgr);
> + seq_puts(m, "\n");
> + nouveau_debugfs_gpuva_regions(m, uvmm);
> + nouveau_uvmm_unlock(uvmm);
> + }
> + mutex_unlock(>clients_lock);
> +
> + return 0;
> +}
> +
> static const struct file_operations nouveau_pstate_fops = {
> .owner = THIS_MODULE,
> .open = nouveau_debugfs_pstate_open,
> @@ -214,6 +252,7 @@ static const struct file_operations nouveau_pstate_fops =
> {
> static struct drm_info_list nouveau_debugfs_list[] = {
> { "vbios.rom", nouveau_debugfs_vbios_image, 0, NULL },
> { "strap_peek", nouveau_debugfs_strap_peek, 0, NULL },
> + DRM_DEBUGFS_GPUVA_INFO(nouveau_debugfs_gpuva, NULL),
> };
> #define NOUVEAU_DEBUGFS_ENTRIES ARRAY_SIZE(nouveau_debugfs_list)
>
> --
> 2.40.1
>
Re: [Nouveau] [PATCH drm-next v4 03/14] drm: manager to keep track of GPUs VA mappings
* Danilo Krummrich [230606 18:32]: > Add infrastructure to keep track of GPU virtual address (VA) mappings > with a decicated VA space manager implementation. > > New UAPIs, motivated by Vulkan sparse memory bindings graphics drivers > start implementing, allow userspace applications to request multiple and > arbitrary GPU VA mappings of buffer objects. The DRM GPU VA manager is > intended to serve the following purposes in this context. > > 1) Provide infrastructure to track GPU VA allocations and mappings, >making use of the maple_tree. > > 2) Generically connect GPU VA mappings to their backing buffers, in >particular DRM GEM objects. > > 3) Provide a common implementation to perform more complex mapping >operations on the GPU VA space. In particular splitting and merging >of GPU VA mappings, e.g. for intersecting mapping requests or partial >unmap requests. > > Suggested-by: Dave Airlie > Signed-off-by: Danilo Krummrich > --- > Documentation/gpu/drm-mm.rst| 31 + > drivers/gpu/drm/Makefile|1 + > drivers/gpu/drm/drm_gem.c |3 + > drivers/gpu/drm/drm_gpuva_mgr.c | 1687 +++ > include/drm/drm_drv.h |6 + > include/drm/drm_gem.h | 75 ++ > include/drm/drm_gpuva_mgr.h | 681 + > 7 files changed, 2484 insertions(+) > create mode 100644 drivers/gpu/drm/drm_gpuva_mgr.c > create mode 100644 include/drm/drm_gpuva_mgr.h > > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst > index a52e6f4117d6..c9f120cfe730 100644 > --- a/Documentation/gpu/drm-mm.rst > +++ b/Documentation/gpu/drm-mm.rst > @@ -466,6 +466,37 @@ DRM MM Range Allocator Function References > .. kernel-doc:: drivers/gpu/drm/drm_mm.c > :export: > > +DRM GPU VA Manager > +== > + > +Overview > + > + > +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c > + :doc: Overview > + > +Split and Merge > +--- > + > +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c > + :doc: Split and Merge > + > +Locking > +--- > + > +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c > + :doc: Locking > + > + > +DRM GPU VA Manager Function References > +-- > + > +.. kernel-doc:: include/drm/drm_gpuva_mgr.h > + :internal: > + > +.. kernel-doc:: drivers/gpu/drm/drm_gpuva_mgr.c > + :export: > + > DRM Buddy Allocator > === > > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 9c6446eb3c83..8eeed446a078 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -45,6 +45,7 @@ drm-y := \ > drm_vblank.o \ > drm_vblank_work.o \ > drm_vma_manager.o \ > + drm_gpuva_mgr.o \ > drm_writeback.o > drm-$(CONFIG_DRM_LEGACY) += \ > drm_agpsupport.o \ > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 1a5a2cd0d4ec..cd878ebddbd0 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -164,6 +164,9 @@ void drm_gem_private_object_init(struct drm_device *dev, > if (!obj->resv) > obj->resv = >_resv; > > + if (drm_core_check_feature(dev, DRIVER_GEM_GPUVA)) > + drm_gem_gpuva_init(obj); > + > drm_vma_node_reset(>vma_node); > INIT_LIST_HEAD(>lru_node); > } > diff --git a/drivers/gpu/drm/drm_gpuva_mgr.c b/drivers/gpu/drm/drm_gpuva_mgr.c > new file mode 100644 > index ..dd8dd7fef14b > --- /dev/null > +++ b/drivers/gpu/drm/drm_gpuva_mgr.c > @@ -0,0 +1,1687 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2022 Red Hat. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > + * OTHER DEALINGS IN THE SOFTWARE. > + * > + * Authors: > + * Danilo Krummrich > + * > + */ > + > +#include > +#include > + > +/** > + * DOC: Overview > + * > + * The DRM GPU VA Manager, represented by struct drm_gpuva_manager keeps > track > + * of
Re: [Nouveau] [PATCH 01/10] drm/nouveau/nvkm: fini object children in reverse order
For the whole series:
Reviewed-by: Lyude Paul
On Thu, 2023-05-25 at 10:30 +1000, Ben Skeggs wrote:
> From: Ben Skeggs
>
> Turns out, we're currently tearing down the disp core channel *before*
> the satellite channels (wndw, etc) during suspend.
>
> This makes RM return NV_ERR_NOT_SUPPORTED on attempting to reallocate
> the core channel on resume for some reason, but we probably shouldn't
> be doing it on HW either.
>
> Tear down children in the reverse of allocation order instead.
>
> Signed-off-by: Ben Skeggs
> ---
> drivers/gpu/drm/nouveau/nvkm/core/object.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/core/object.c
> b/drivers/gpu/drm/nouveau/nvkm/core/object.c
> index 301a5e5b5f7f..7c554c14e884 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/core/object.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/core/object.c
> @@ -185,7 +185,7 @@ nvkm_object_fini(struct nvkm_object *object, bool suspend)
>
> nvif_debug(object, "%s children...\n", action);
> time = ktime_to_us(ktime_get());
> - list_for_each_entry(child, >tree, head) {
> + list_for_each_entry_reverse(child, >tree, head) {
> ret = nvkm_object_fini(child, suspend);
> if (ret && suspend)
> goto fail_child;
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Re: [Nouveau] [PATCH 10/10] drm/nouveau/kms: don't call drm_dp_cec_set_edid() on TMDS
On Thu, May 25, 2023 at 2:31 AM Ben Skeggs wrote:
>
> From: Ben Skeggs
>
> RM complains very loudly at the aux transaction attempts.
>
> Signed-off-by: Ben Skeggs
> ---
> drivers/gpu/drm/nouveau/nouveau_connector.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 086b66b60d91..4c0cb32f6f2c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -619,7 +619,10 @@ nouveau_connector_detect(struct drm_connector
> *connector, bool force)
>
> nouveau_connector_set_encoder(connector, nv_encoder);
> conn_status = connector_status_connected;
> - drm_dp_cec_set_edid(_connector->aux, nv_connector->edid);
> +
> + if (nv_encoder->dcb->type == DCB_OUTPUT_DP)
> + drm_dp_cec_set_edid(_connector->aux,
> nv_connector->edid);
> +
> goto out;
> } else {
> nouveau_connector_set_edid(nv_connector, NULL);
> --
> 2.40.1
>
Reviewed-by: Karol Herbst
Re: [Nouveau] [PATCH 09/10] drm/nouveau/nvif: fix potential double-free
On Thu, May 25, 2023 at 2:31 AM Ben Skeggs wrote:
>
> From: Ben Skeggs
>
> DRM cleanup paths unconditionally call nvif_mmu_dtor() for clients,
> which would result in a double-free if nvif_mmu_ctor()'d previously
> failed.
>
> Signed-off-by: Ben Skeggs
> ---
> drivers/gpu/drm/nouveau/nvif/mmu.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nvif/mmu.c
> b/drivers/gpu/drm/nouveau/nvif/mmu.c
> index 3709cbbc19a1..c9dd3cff49a0 100644
> --- a/drivers/gpu/drm/nouveau/nvif/mmu.c
> +++ b/drivers/gpu/drm/nouveau/nvif/mmu.c
> @@ -27,6 +27,9 @@
> void
> nvif_mmu_dtor(struct nvif_mmu *mmu)
> {
> + if (!nvif_object_constructed(>object))
> + return;
> +
nvif_mmu_ctor seems to be calling into this in its clean up path, so
this could now leaks memory in case nvif_mmu_ctor fails, no?
> kfree(mmu->kind);
> kfree(mmu->type);
> kfree(mmu->heap);
> --
> 2.40.1
>
Re: [Nouveau] [PATCH 08/10] drm/nouveau/fifo/ga100-: add per-runlist nonstall intr handling
On Thu, May 25, 2023 at 2:31 AM Ben Skeggs wrote:
>
> From: Ben Skeggs
>
> GSP-RM will enforce this, so implement on HW too so we can share code.
>
> Signed-off-by: Ben Skeggs
> ---
> .../drm/nouveau/include/nvkm/core/engine.h| 1 +
> .../gpu/drm/nouveau/nvkm/engine/ce/ga100.c| 10
> .../gpu/drm/nouveau/nvkm/engine/ce/ga102.c| 1 +
> drivers/gpu/drm/nouveau/nvkm/engine/ce/priv.h | 1 +
> .../gpu/drm/nouveau/nvkm/engine/fifo/base.c | 32 +-
> .../gpu/drm/nouveau/nvkm/engine/fifo/ga100.c | 59 +++
> .../gpu/drm/nouveau/nvkm/engine/fifo/runl.h | 6 ++
> .../gpu/drm/nouveau/nvkm/engine/fifo/uchan.c | 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/base.c | 12
> .../gpu/drm/nouveau/nvkm/engine/gr/ga102.c| 7 +++
> .../gpu/drm/nouveau/nvkm/engine/gr/gf100.c| 12
> .../gpu/drm/nouveau/nvkm/engine/gr/gf100.h| 1 +
> drivers/gpu/drm/nouveau/nvkm/engine/gr/priv.h | 1 +
> 13 files changed, 116 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/core/engine.h
> b/drivers/gpu/drm/nouveau/include/nvkm/core/engine.h
> index 8041fe03237e..738899fcf30b 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/core/engine.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/core/engine.h
> @@ -22,6 +22,7 @@ struct nvkm_engine_func {
> int (*init)(struct nvkm_engine *);
> int (*fini)(struct nvkm_engine *, bool suspend);
> int (*reset)(struct nvkm_engine *);
> + int (*nonstall)(struct nvkm_engine *);
> void (*intr)(struct nvkm_engine *);
> void (*tile)(struct nvkm_engine *, int region, struct nvkm_fb_tile *);
> bool (*chsw_load)(struct nvkm_engine *);
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/ce/ga100.c
> b/drivers/gpu/drm/nouveau/nvkm/engine/ce/ga100.c
> index 6648ed62daa6..315a69f7fdd1 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/ce/ga100.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/ce/ga100.c
> @@ -35,6 +35,15 @@ ga100_ce_intr(struct nvkm_inth *inth)
> return IRQ_NONE;
> }
>
> +int
> +ga100_ce_nonstall(struct nvkm_engine *engine)
> +{
> + struct nvkm_subdev *subdev = >subdev;
> + struct nvkm_device *device = subdev->device;
> +
> + return nvkm_rd32(device, 0x104424 + (subdev->inst * 0x80)) &
> 0x0fff;
> +}
> +
> int
> ga100_ce_fini(struct nvkm_engine *engine, bool suspend)
> {
> @@ -67,6 +76,7 @@ ga100_ce = {
> .oneinit = ga100_ce_oneinit,
> .init = ga100_ce_init,
> .fini = ga100_ce_fini,
> + .nonstall = ga100_ce_nonstall,
> .cclass = _ce_cclass,
> .sclass = {
> { -1, -1, AMPERE_DMA_COPY_A },
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/ce/ga102.c
> b/drivers/gpu/drm/nouveau/nvkm/engine/ce/ga102.c
> index 9f3448ad625f..461b73c7e2e0 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/ce/ga102.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/ce/ga102.c
> @@ -28,6 +28,7 @@ ga102_ce = {
> .oneinit = ga100_ce_oneinit,
> .init = ga100_ce_init,
> .fini = ga100_ce_fini,
> + .nonstall = ga100_ce_nonstall,
> .cclass = _ce_cclass,
> .sclass = {
> { -1, -1, AMPERE_DMA_COPY_A },
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/ce/priv.h
> b/drivers/gpu/drm/nouveau/nvkm/engine/ce/priv.h
> index 53ba2abe0bf5..0be72c463b21 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/ce/priv.h
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/ce/priv.h
> @@ -12,4 +12,5 @@ extern const struct nvkm_object_func gv100_ce_cclass;
> int ga100_ce_oneinit(struct nvkm_engine *);
> int ga100_ce_init(struct nvkm_engine *);
> int ga100_ce_fini(struct nvkm_engine *, bool);
> +int ga100_ce_nonstall(struct nvkm_engine *);
> #endif
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/base.c
> b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/base.c
> index 5ea9a2ff0663..5db37247dc29 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/base.c
> @@ -283,11 +283,21 @@ nvkm_fifo_oneinit(struct nvkm_engine *engine)
> }
>
> /* Initialise non-stall intr handling. */
> - if (fifo->func->nonstall_ctor) {
> - ret = fifo->func->nonstall_ctor(fifo);
> - if (ret) {
> - nvkm_error(subdev, "nonstall %d\n", ret);
> + if (fifo->func->nonstall) {
> + if (fifo->func->nonstall_ctor) {
> + ret = fifo->func->nonstall_ctor(fifo);
> + if (ret < 0) {
> + nvkm_error(subdev, "nonstall %d\n", ret);
> + return ret;
> + }
> + } else {
> + ret = 1;
> }
> +
> + ret = nvkm_event_init(fifo->func->nonstall,
> >engine.subdev, 1, ret,
> + >nonstall.event);
> + if (ret)
> +
Re: [Nouveau] [PATCH 07/10] drm/nouveau/fifo/ga100-: remove individual runlists rather than failing oneinit
On Thu, May 25, 2023 at 2:31 AM Ben Skeggs wrote:
>
> From: Ben Skeggs
>
> We're adding better support for the non-stall interrupt, which will need
> to fetch the interrupt vector from the runlist's primary engine.
>
> NVKM doesn't support all target engines (ie. NVDEC etc), and it wouldn't
> be ideal to completely fail initialisation in this case.
>
> Instead. Remove runlists where we can't determine all the needed info.
>
> Signed-off-by: Ben Skeggs
> ---
> .../gpu/drm/nouveau/nvkm/engine/fifo/ga100.c | 46 ++-
> 1 file changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/ga100.c
> b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/ga100.c
> index 12a5d99d5e77..a729f8b7f0da 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/ga100.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/ga100.c
> @@ -429,7 +429,9 @@ static int
> ga100_runl_new(struct nvkm_fifo *fifo, int id, u32 addr, struct nvkm_runl
> **prunl)
> {
> struct nvkm_device *device = fifo->engine.subdev.device;
> + struct nvkm_top_device *tdev;
> struct nvkm_runl *runl;
> + struct nvkm_engn *engn;
> u32 chcfg = nvkm_rd32(device, addr + 0x004);
> u32 chnum = 1 << (chcfg & 0x000f);
> u32 chaddr = (chcfg & 0xfff0);
> @@ -437,26 +439,50 @@ ga100_runl_new(struct nvkm_fifo *fifo, int id, u32
> addr, struct nvkm_runl **prun
> u32 vector = nvkm_rd32(device, addr + 0x160);
> int i, ret;
>
> - runl = *prunl = nvkm_runl_new(fifo, id, addr, chnum);
> + runl = nvkm_runl_new(fifo, id, addr, chnum);
> if (IS_ERR(runl))
> return PTR_ERR(runl);
>
> + *prunl = runl;
> +
> for (i = 0; i < 2; i++) {
> u32 pbcfg = nvkm_rd32(device, addr + 0x010 + (i * 0x04));
> if (pbcfg & 0x8000) {
> runl->runq[runl->runq_nr] =
> nvkm_runq_new(fifo, ((pbcfg & 0x03fffc00) -
> 0x04) / 0x800);
> - if (!runl->runq[runl->runq_nr])
> + if (!runl->runq[runl->runq_nr]) {
> + RUNL_ERROR(runl, "runq %d", runl->runq_nr);
> return -ENOMEM;
> + }
>
> runl->runq_nr++;
> }
> }
>
> + nvkm_list_foreach(tdev, >top->device, head, tdev->runlist ==
> runl->addr) {
> + if (tdev->engine < 0) {
> + RUNL_DEBUG(runl, "engn !top");
> + return -EINVAL;
> + }
> +
> + engn = nvkm_runl_add(runl, tdev->engine, (tdev->type ==
> NVKM_ENGINE_CE) ?
> +fifo->func->engn_ce : fifo->func->engn,
> +tdev->type, tdev->inst);
> + if (!engn)
> + return -EINVAL;
> + }
> +
> + if (list_empty(>engns)) {
> + RUNL_DEBUG(runl, "!engns");
> + return -EINVAL;
> + }
> +
> ret = nvkm_inth_add(>vfn->intr, vector & 0x0fff,
> NVKM_INTR_PRIO_NORMAL,
> >engine.subdev, ga100_runl_intr,
> >inth);
> - if (ret)
> + if (ret) {
> + RUNL_ERROR(runl, "inth %d", ret);
> return ret;
> + }
>
> runl->chan = chaddr;
> runl->doorbell = dbcfg >> 16;
> @@ -514,15 +540,13 @@ ga100_fifo_runl_ctor(struct nvkm_fifo *fifo)
> runl = nvkm_runl_get(fifo, -1, tdev->runlist);
> if (!runl) {
> ret = ga100_runl_new(fifo, id++, tdev->runlist,
> );
> - if (ret)
> - return ret;
> - }
> -
> - if (tdev->engine < 0)
> - continue;
> + if (ret) {
> + if (runl)
> + nvkm_runl_del(runl);
>
> - nvkm_runl_add(runl, tdev->engine, (tdev->type ==
> NVKM_ENGINE_CE) ?
> - fifo->func->engn_ce : fifo->func->engn,
> tdev->type, tdev->inst);
> + continue;
> + }
> + }
> }
>
> return 0;
> --
> 2.40.1
>
Reviewed-by: Karol Herbst
Re: [Nouveau] [PATCH 06/10] drm/nouveau/fifo: return ERR_PTR from nvkm_runl_new()
On Thu, May 25, 2023 at 2:31 AM Ben Skeggs wrote:
>
> From: Ben Skeggs
>
> Callers expect this - not NULL.
>
> Signed-off-by: Ben Skeggs
> ---
> drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c
> b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c
> index 93d628d7d508..454a481a0aef 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c
> @@ -399,7 +399,7 @@ nvkm_runl_new(struct nvkm_fifo *fifo, int runi, u32 addr,
> int id_nr)
> int ret;
>
> if (!(runl = kzalloc(sizeof(*runl), GFP_KERNEL)))
> - return NULL;
> + return ERR_PTR(-ENOMEM);
>
> runl->func = fifo->func->runl;
> runl->fifo = fifo;
> @@ -419,7 +419,7 @@ nvkm_runl_new(struct nvkm_fifo *fifo, int runi, u32 addr,
> int id_nr)
> (ret = nvkm_chid_new(_chan_event, subdev, id_nr, 0,
> id_nr, >chid))) {
> RUNL_ERROR(runl, "cgid/chid: %d", ret);
> nvkm_runl_del(runl);
> - return NULL;
> + return ERR_PTR(ret);
> }
> } else {
> runl->cgid = nvkm_chid_ref(fifo->cgid);
> --
> 2.40.1
>
Reviewed-by: Karol Herbst
Re: [Nouveau] [PATCH 05/10] drm/nouveau/fifo: remove left-over references to nvkm_fifo_chan
On Thu, May 25, 2023 at 2:31 AM Ben Skeggs wrote:
>
> From: Ben Skeggs
>
> This was renamed to nvkm_chan in the host rework.
>
> Signed-off-by: Ben Skeggs
> ---
> drivers/gpu/drm/nouveau/include/nvkm/core/engine.h | 5 ++---
> drivers/gpu/drm/nouveau/include/nvkm/core/os.h | 5 -
> drivers/gpu/drm/nouveau/include/nvkm/engine/falcon.h | 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/ce/priv.h| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/base.c| 3 +--
> drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c | 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv04.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv10.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv10.h| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv20.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv25.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv2a.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv30.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv34.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv35.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv40.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv40.h| 4 ++--
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv50.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/nv50.h| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/gr/priv.h| 4 ++--
> drivers/gpu/drm/nouveau/nvkm/engine/mpeg/nv31.c | 3 +--
> drivers/gpu/drm/nouveau/nvkm/engine/mpeg/nv31.h | 4 ++--
> drivers/gpu/drm/nouveau/nvkm/engine/mpeg/nv44.c | 5 ++---
> drivers/gpu/drm/nouveau/nvkm/engine/mpeg/priv.h | 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/sw/base.c| 3 +--
> drivers/gpu/drm/nouveau/nvkm/engine/sw/chan.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/sw/chan.h| 4 ++--
> drivers/gpu/drm/nouveau/nvkm/engine/sw/gf100.c | 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/sw/nv04.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/sw/nv10.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/sw/nv50.c| 2 +-
> drivers/gpu/drm/nouveau/nvkm/engine/sw/priv.h| 2 +-
> 32 files changed, 37 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/core/engine.h
> b/drivers/gpu/drm/nouveau/include/nvkm/core/engine.h
> index b67b9c1a6b4e..8041fe03237e 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/core/engine.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/core/engine.h
> @@ -3,7 +3,7 @@
> #define __NVKM_ENGINE_H__
> #define nvkm_engine(p) container_of((p), struct nvkm_engine, subdev)
> #include
> -struct nvkm_fifo_chan;
> +struct nvkm_chan;
> struct nvkm_fb_tile;
>
> extern const struct nvkm_subdev_func nvkm_engine;
> @@ -32,8 +32,7 @@ struct nvkm_engine_func {
> } base;
>
> struct {
> - int (*cclass)(struct nvkm_fifo_chan *,
> - const struct nvkm_oclass *,
> + int (*cclass)(struct nvkm_chan *, const struct nvkm_oclass *,
> struct nvkm_object **);
> int (*sclass)(struct nvkm_oclass *, int index);
> } fifo;
> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/core/os.h
> b/drivers/gpu/drm/nouveau/include/nvkm/core/os.h
> index 4486d9862849..3fd5c007a663 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/core/os.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/core/os.h
> @@ -49,9 +49,4 @@ nvkm_blob_dtor(struct nvkm_blob *blob)
> (p = container_of((h), typeof(*p), m), nvkm_list_find_next(p, (h), m,
> (c)))
> #define nvkm_list_foreach(p,h,m,c)
> \
> for (p = nvkm_list_find(p, (h), m, (c)); p; p =
> nvkm_list_find_next(p, (h), m, (c)))
> -
> -/*FIXME: remove after */
> -#define nvkm_fifo_chan nvkm_chan
> -#define nvkm_fifo_chan_func nvkm_chan_func
> -#define nvkm_fifo_cgrp nvkm_cgrp
> #endif
> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/engine/falcon.h
> b/drivers/gpu/drm/nouveau/include/nvkm/engine/falcon.h
> index cd86d9198e4a..b7bb8a29a729 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/engine/falcon.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/engine/falcon.h
> @@ -3,7 +3,7 @@
> #define __NVKM_FLCNEN_H__
> #define nvkm_falcon(p) container_of((p), struct nvkm_falcon, engine)
> #include
> -struct nvkm_fifo_chan;
> +struct nvkm_chan;
>
> enum nvkm_falcon_dmaidx {
> FALCON_DMAIDX_UCODE = 0,
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/ce/priv.h
> b/drivers/gpu/drm/nouveau/nvkm/engine/ce/priv.h
> index c4c046916fa6..53ba2abe0bf5 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/ce/priv.h
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/ce/priv.h
> @@ -3,7 +3,7 @@
> #define __NVKM_CE_PRIV_H__
> #include
>
> -void gt215_ce_intr(struct nvkm_falcon *, struct nvkm_fifo_chan *);
> +void gt215_ce_intr(struct nvkm_falcon *, struct nvkm_chan *);
Re: [Nouveau] [PATCH 03/10] drm/nouveau/fb/gp102-ga100: switch to simpler vram size detection method
On Thu, May 25, 2023 at 2:31 AM Ben Skeggs wrote:
>
> From: Ben Skeggs
>
> Also exposes this for use by upcoming GSP-RM initialisation code.
>
> Signed-off-by: Ben Skeggs
> ---
> .../gpu/drm/nouveau/include/nvkm/subdev/fb.h | 1 +
> drivers/gpu/drm/nouveau/nvkm/subdev/fb/Kbuild | 1 +
> drivers/gpu/drm/nouveau/nvkm/subdev/fb/base.c | 12 +
> .../gpu/drm/nouveau/nvkm/subdev/fb/ga100.c| 3 +-
> .../gpu/drm/nouveau/nvkm/subdev/fb/gp102.c| 17 ++-
> .../gpu/drm/nouveau/nvkm/subdev/fb/gv100.c| 3 +-
> drivers/gpu/drm/nouveau/nvkm/subdev/fb/priv.h | 5 ++
> drivers/gpu/drm/nouveau/nvkm/subdev/fb/ram.h | 1 +
> .../gpu/drm/nouveau/nvkm/subdev/fb/ramgp102.c | 50 +++
> .../gpu/drm/nouveau/nvkm/subdev/fb/tu102.c| 3 +-
> 10 files changed, 92 insertions(+), 4 deletions(-)
> create mode 100644 drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgp102.c
>
> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/fb.h
> b/drivers/gpu/drm/nouveau/include/nvkm/subdev/fb.h
> index 01a22a13b452..1755b0df3cc1 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/fb.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/fb.h
> @@ -59,6 +59,7 @@ struct nvkm_fb {
> struct nvkm_memory *mmu_wr;
> };
>
> +u64 nvkm_fb_vidmem_size(struct nvkm_device *);
> int nvkm_fb_mem_unlock(struct nvkm_fb *);
>
> void nvkm_fb_tile_init(struct nvkm_fb *, int region, u32 addr, u32 size,
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/Kbuild
> b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/Kbuild
> index 6ba5120a2ebe..11dbfc4a381a 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/Kbuild
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/Kbuild
> @@ -55,6 +55,7 @@ nvkm-y += nvkm/subdev/fb/ramgk104.o
> nvkm-y += nvkm/subdev/fb/ramgm107.o
> nvkm-y += nvkm/subdev/fb/ramgm200.o
> nvkm-y += nvkm/subdev/fb/ramgp100.o
> +nvkm-y += nvkm/subdev/fb/ramgp102.o
> nvkm-y += nvkm/subdev/fb/ramga102.o
> nvkm-y += nvkm/subdev/fb/sddr2.o
> nvkm-y += nvkm/subdev/fb/sddr3.o
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/base.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/base.c
> index 0955340cc421..8a286a9349ac 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/base.c
> @@ -174,6 +174,18 @@ nvkm_fb_mem_unlock(struct nvkm_fb *fb)
> return 0;
> }
>
> +u64
> +nvkm_fb_vidmem_size(struct nvkm_device *device)
> +{
> + struct nvkm_fb *fb = device->fb;
> +
> + if (fb && fb->func->vidmem.size)
> + return fb->func->vidmem.size(fb);
> +
> + WARN_ON(1);
> + return 0;
> +}
> +
> static int
> nvkm_fb_init(struct nvkm_subdev *subdev)
> {
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ga100.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ga100.c
> index a7456e786463..12037fd4fdf2 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ga100.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ga100.c
> @@ -30,7 +30,8 @@ ga100_fb = {
> .init_page = gv100_fb_init_page,
> .init_unkn = gp100_fb_init_unkn,
> .sysmem.flush_page_init = gf100_fb_sysmem_flush_page_init,
> - .ram_new = gp100_ram_new,
> + .vidmem.size = gp102_fb_vidmem_size,
> + .ram_new = gp102_ram_new,
> .default_bigpage = 16,
> };
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gp102.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gp102.c
> index 14d942e8b857..534553c64805 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gp102.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gp102.c
> @@ -40,6 +40,20 @@ gp102_fb_vpr_scrub_required(struct nvkm_fb *fb)
> return (nvkm_rd32(device, 0x100cd0) & 0x0010) != 0;
> }
>
> +u64
> +gp102_fb_vidmem_size(struct nvkm_fb *fb)
> +{
> + const u32 data = nvkm_rd32(fb->subdev.device, 0x100ce0);
Do we have any kind of documentation for this register?
> + const u32 lmag = (data & 0x03f0) >> 4;
> + const u32 lsca = (data & 0x000f);
> + const u64 size = (u64)lmag << (lsca + 20);
> +
> + if (data & 0x4000)
> + return size / 16 * 15;
> +
> + return size;
> +}
> +
> int
> gp102_fb_oneinit(struct nvkm_fb *fb)
> {
> @@ -59,9 +73,10 @@ gp102_fb = {
> .init_remapper = gp100_fb_init_remapper,
> .init_page = gm200_fb_init_page,
> .sysmem.flush_page_init = gf100_fb_sysmem_flush_page_init,
> + .vidmem.size = gp102_fb_vidmem_size,
> .vpr.scrub_required = gp102_fb_vpr_scrub_required,
> .vpr.scrub = gp102_fb_vpr_scrub,
> - .ram_new = gp100_ram_new,
> + .ram_new = gp102_ram_new,
> };
>
> int
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gv100.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gv100.c
> index 4d8a286a7a34..f422564bee5b 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gv100.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/gv100.c
> @@ -36,9 +36,10 @@ gv100_fb = {
> .init_page = gv100_fb_init_page,
>
Re: [Nouveau] [PATCH 02/10] drm/nouveau/nvkm: punt spurious irq messages to debug level
On Thu, May 25, 2023 at 2:31 AM Ben Skeggs wrote:
>
> From: Ben Skeggs
>
> This can be completely normal in some situations (ie. non-stall intrs
> when nothing is waiting on them).
>
> Signed-off-by: Ben Skeggs
> ---
> drivers/gpu/drm/nouveau/nvkm/core/intr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/core/intr.c
> b/drivers/gpu/drm/nouveau/nvkm/core/intr.c
> index e20b7ca218c3..36a747f0039e 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/core/intr.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/core/intr.c
> @@ -212,8 +212,8 @@ nvkm_intr(int irq, void *arg)
> list_for_each_entry(intr, >intr.intr, head) {
> for (leaf = 0; leaf < intr->leaves; leaf++) {
> if (intr->stat[leaf]) {
> - nvkm_warn(intr->subdev, "intr%d:
> %08x\n",
> - leaf, intr->stat[leaf]);
> + nvkm_debug(intr->subdev, "intr%d:
> %08x\n",
> + leaf, intr->stat[leaf]);
> nvkm_intr_block_locked(intr, leaf,
> intr->stat[leaf]);
> }
> }
> --
> 2.40.1
>
Reviewed-by: Karol Herbst
Re: [Nouveau] [PATCH 01/10] drm/nouveau/nvkm: fini object children in reverse order
On Thu, May 25, 2023 at 2:31 AM Ben Skeggs wrote:
>
> From: Ben Skeggs
>
> Turns out, we're currently tearing down the disp core channel *before*
> the satellite channels (wndw, etc) during suspend.
>
> This makes RM return NV_ERR_NOT_SUPPORTED on attempting to reallocate
> the core channel on resume for some reason, but we probably shouldn't
> be doing it on HW either.
>
> Tear down children in the reverse of allocation order instead.
>
> Signed-off-by: Ben Skeggs
> ---
> drivers/gpu/drm/nouveau/nvkm/core/object.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/core/object.c
> b/drivers/gpu/drm/nouveau/nvkm/core/object.c
> index 301a5e5b5f7f..7c554c14e884 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/core/object.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/core/object.c
> @@ -185,7 +185,7 @@ nvkm_object_fini(struct nvkm_object *object, bool suspend)
>
> nvif_debug(object, "%s children...\n", action);
> time = ktime_to_us(ktime_get());
> - list_for_each_entry(child, >tree, head) {
> + list_for_each_entry_reverse(child, >tree, head) {
> ret = nvkm_object_fini(child, suspend);
> if (ret && suspend)
> goto fail_child;
> --
> 2.40.1
>
Reviewed-by: Karol Herbst
Re: [Nouveau] [PATCH] drm/nouveau/kms/nv50-: Fix drm_dp_remove_payload() invocation
On Tue, Jun 13, 2023 at 11:05 PM Lyude Paul wrote:
>
> We changed the semantics for this in:
>
> e761cc20946a ("drm/display/dp_mst: Handle old/new payload states in
> drm_dp_remove_payload()")
>
> But I totally forgot to update this properly in nouveau. So, let's do that.
>
> Signed-off-by: Lyude Paul
Reviewed-by: Karol Herbst
> ---
> drivers/gpu/drm/nouveau/dispnv50/disp.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 5bb777ff13130..1637e08b548c2 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -909,15 +909,19 @@ nv50_msto_prepare(struct drm_atomic_state *state,
> struct nouveau_drm *drm = nouveau_drm(msto->encoder.dev);
> struct nv50_mstc *mstc = msto->mstc;
> struct nv50_mstm *mstm = mstc->mstm;
> - struct drm_dp_mst_atomic_payload *payload;
> + struct drm_dp_mst_topology_state *old_mst_state;
> + struct drm_dp_mst_atomic_payload *payload, *old_payload;
>
> NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
>
> + old_mst_state = drm_atomic_get_old_mst_topology_state(state, mgr);
> +
> payload = drm_atomic_get_mst_payload_state(mst_state, mstc->port);
> + old_payload = drm_atomic_get_mst_payload_state(old_mst_state,
> mstc->port);
>
> // TODO: Figure out if we want to do a better job of handling VCPI
> allocation failures here?
> if (msto->disabled) {
> - drm_dp_remove_payload(mgr, mst_state, payload, payload);
> + drm_dp_remove_payload(mgr, mst_state, old_payload, payload);
>
> nvif_outp_dp_mst_vcpi(>outp->outp,
> msto->head->base.index, 0, 0, 0, 0);
> } else {
> --
> 2.40.1
>
[Nouveau] [PATCH] drm/nouveau/kms/nv50-: Fix drm_dp_remove_payload() invocation
We changed the semantics for this in:
e761cc20946a ("drm/display/dp_mst: Handle old/new payload states in
drm_dp_remove_payload()")
But I totally forgot to update this properly in nouveau. So, let's do that.
Signed-off-by: Lyude Paul
---
drivers/gpu/drm/nouveau/dispnv50/disp.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 5bb777ff13130..1637e08b548c2 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -909,15 +909,19 @@ nv50_msto_prepare(struct drm_atomic_state *state,
struct nouveau_drm *drm = nouveau_drm(msto->encoder.dev);
struct nv50_mstc *mstc = msto->mstc;
struct nv50_mstm *mstm = mstc->mstm;
- struct drm_dp_mst_atomic_payload *payload;
+ struct drm_dp_mst_topology_state *old_mst_state;
+ struct drm_dp_mst_atomic_payload *payload, *old_payload;
NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
+ old_mst_state = drm_atomic_get_old_mst_topology_state(state, mgr);
+
payload = drm_atomic_get_mst_payload_state(mst_state, mstc->port);
+ old_payload = drm_atomic_get_mst_payload_state(old_mst_state,
mstc->port);
// TODO: Figure out if we want to do a better job of handling VCPI
allocation failures here?
if (msto->disabled) {
- drm_dp_remove_payload(mgr, mst_state, payload, payload);
+ drm_dp_remove_payload(mgr, mst_state, old_payload, payload);
nvif_outp_dp_mst_vcpi(>outp->outp,
msto->head->base.index, 0, 0, 0, 0);
} else {
--
2.40.1
Re: [Nouveau] [PATCH] nouveau_connector: add nv_encoder pointer check for NULL
Nice catch!
Reviewed-by: Lyude Paul
Will push upstream
On Fri, 2023-05-12 at 13:33 +0300, Natalia Petrova wrote:
> Pointer nv_encoder could be dereferenced at nouveau_connector.c
> in case it's equal to NULL by jumping to goto label.
> This patch adds a NULL-check to avoid it.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 3195c5f9784a ("drm/nouveau: set encoder for lvds")
> Signed-off-by: Natalia Petrova
> ---
> drivers/gpu/drm/nouveau/nouveau_connector.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 43a9d1e1cf71..90ba6d0a9c80 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -729,7 +729,8 @@ nouveau_connector_detect_lvds(struct drm_connector
> *connector, bool force)
> #endif
>
> nouveau_connector_set_edid(nv_connector, edid);
> - nouveau_connector_set_encoder(connector, nv_encoder);
> + if (nv_encoder)
> + nouveau_connector_set_encoder(connector, nv_encoder);
> return status;
> }
>
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Re: [Nouveau] [PATCH] drm/nouveau/dp: check for NULL nv_connector->native_mode
Reviewed-by: Lyude Paul
Will push upstream in a bit
On Fri, 2023-05-12 at 14:15 +0300, Natalia Petrova wrote:
> Add checking for NULL before calling nouveau_connector_detect_depth() in
> nouveau_connector_get_modes() function because nv_connector->native_mode
> could be dereferenced there since connector pointer passed to
> nouveau_connector_detect_depth() and the same value of
> nv_connector->native_mode is used there.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: d4c2c99bdc83 ("drm/nouveau/dp: remove broken display depth function,
> use the improved one")
>
> Signed-off-by: Natalia Petrova
> ---
> drivers/gpu/drm/nouveau/nouveau_connector.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 086b66b60d91..5dbf025e6873 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -966,7 +966,7 @@ nouveau_connector_get_modes(struct drm_connector
> *connector)
> /* Determine display colour depth for everything except LVDS now,
>* DP requires this before mode_valid() is called.
>*/
> - if (connector->connector_type != DRM_MODE_CONNECTOR_LVDS)
> + if (connector->connector_type != DRM_MODE_CONNECTOR_LVDS &&
> nv_connector->native_mode)
> nouveau_connector_detect_depth(connector);
>
> /* Find the native mode if this is a digital panel, if we didn't
> @@ -987,7 +987,7 @@ nouveau_connector_get_modes(struct drm_connector
> *connector)
>* "native" mode as some VBIOS tables require us to use the
>* pixel clock as part of the lookup...
>*/
> - if (connector->connector_type == DRM_MODE_CONNECTOR_LVDS)
> + if (connector->connector_type == DRM_MODE_CONNECTOR_LVDS &&
> nv_connector->native_mode)
> nouveau_connector_detect_depth(connector);
>
> if (nv_encoder->dcb->type == DCB_OUTPUT_TV)
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
Re: [Nouveau] [PATCH drm-next v4 02/14] maple_tree: split up MA_STATE() macro
* Danilo Krummrich [230606 18:31]:
> Split up the MA_STATE() macro such that components using the maple tree
> can easily inherit from struct ma_state and build custom tree walk
> macros to hide their internals from users.
>
> Example:
>
> struct sample_iterator {
> struct ma_state mas;
> struct sample_mgr *mgr;
> };
>
> \#define SAMPLE_ITERATOR(name, __mgr, start) \
> struct sample_iterator name = { \
> .mas = MA_STATE_INIT(&(__mgr)->mt, start, 0), \
> .mgr = __mgr, \
> }
>
> \#define sample_iter_for_each_range(it__, entry__, end__) \
> mas_for_each(&(it__).mas, entry__, end__)
>
> --
>
> struct sample *sample;
> SAMPLE_ITERATOR(si, min);
>
> sample_iter_for_each_range(, sample, max) {
> frob(mgr, sample);
> }
>
> Signed-off-by: Danilo Krummrich
Reviewed-by: Liam R. Howlett
> ---
> include/linux/maple_tree.h | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/maple_tree.h b/include/linux/maple_tree.h
> index 1fadb5f5978b..87d55334f1c2 100644
> --- a/include/linux/maple_tree.h
> +++ b/include/linux/maple_tree.h
> @@ -423,8 +423,8 @@ struct ma_wr_state {
> #define MA_ERROR(err) \
> ((struct maple_enode *)(((unsigned long)err << 2) | 2UL))
>
> -#define MA_STATE(name, mt, first, end)
> \
> - struct ma_state name = {\
> +#define MA_STATE_INIT(mt, first, end)
> \
> + { \
> .tree = mt, \
> .index = first, \
> .last = end,\
> @@ -435,6 +435,9 @@ struct ma_wr_state {
> .mas_flags = 0, \
> }
>
> +#define MA_STATE(name, mt, first, end)
> \
> + struct ma_state name = MA_STATE_INIT(mt, first, end)
> +
> #define MA_WR_STATE(name, ma_state, wr_entry)
> \
> struct ma_wr_state name = { \
> .mas = ma_state,\
> --
> 2.40.1
>
>
Re: [Nouveau] [PATCH drm-next v4 00/14] [RFC] DRM GPUVA Manager & Nouveau VM_BIND UAPI
Hi Donald, On 6/9/23 13:56, Donald Robson wrote: On Wed, 2023-06-07 at 00:31 +0200, Danilo Krummrich wrote: Christian König (1): drm: execution context for GEM buffers v4 Danilo Krummrich (13): maple_tree: split up MA_STATE() macro drm: manager to keep track of GPUs VA mappings I have tested the drm GPUVA manager as part of using it with our new driver. The link below shows use of the drm_gpuva_sm_[un]map() functions. I think this is based on the v3 patches, but I have also tried it locally using v4 patches. We will be submitting this driver for review soon. That's awesome - thank your for taking the effort! https://gitlab.freedesktop.org/sarah-walker-imgtec/powervr/-/blob/dev/v3/drivers/gpu/drm/imagination/pvr_vm.c In a previous incarnation, I used the drm_gpuva_insert() and drm_gpuva_remove() functions directly. In some now abandoned work I used the drm_gpuva_sm_[un]map_ops_create() route. The only problem I encountered along the way was the maple tree init issue already reported by Boris and fixed in v4. One caveat - as our driver is a work in progress our testing is limited to certain Sascha Willem tests. I did find it quite difficult to get the prealloc route with drm_gpuva_sm_[un]map() working. I'm not sure to what degree this reflects me being a novice on matters DRM, but I did find myself wishing for more direction, even with Boris's help. I'm definitely up improving the existing documentation. Anything in particular you think should be described in more detail? - Danilo Tested-by: Donald Robson drm: debugfs: provide infrastructure to dump a DRM GPU VA space drm/nouveau: new VM_BIND uapi interfaces drm/nouveau: get vmm via nouveau_cli_vmm() drm/nouveau: bo: initialize GEM GPU VA interface drm/nouveau: move usercopy helpers to nouveau_drv.h drm/nouveau: fence: separate fence alloc and emit drm/nouveau: fence: fail to emit when fence context is killed drm/nouveau: chan: provide nouveau_channel_kill() drm/nouveau: nvkm/vmm: implement raw ops to manage uvmm drm/nouveau: implement new VM_BIND uAPI drm/nouveau: debugfs: implement DRM GPU VA debugfs
