Re: [Nouveau] [RFC] treewide: cleanup unreachable breaks
On Mon, Oct 19, 2020 at 12:42:15PM -0700, Nick Desaulniers wrote: > On Sat, Oct 17, 2020 at 10:43 PM Greg KH wrote: > > > > On Sat, Oct 17, 2020 at 09:09:28AM -0700, t...@redhat.com wrote: > > > From: Tom Rix > > > > > > This is a upcoming change to clean up a new warning treewide. > > > I am wondering if the change could be one mega patch (see below) or > > > normal patch per file about 100 patches or somewhere half way by > > > collecting > > > early acks. > > > > Please break it up into one-patch-per-subsystem, like normal, and get it > > merged that way. > > > > Sending us a patch, without even a diffstat to review, isn't going to > > get you very far... > > Tom, > If you're able to automate this cleanup, I suggest checking in a > script that can be run on a directory. Then for each subsystem you > can say in your commit "I ran scripts/fix_whatever.py on this subdir." > Then others can help you drive the tree wide cleanup. Then we can > enable -Wunreachable-code-break either by default, or W=2 right now > might be a good idea. I remember using clang-modernize in the past to fix issues very similar to this, if clang machinery can generate the warning, can't something like clang-tidy directly generate the patch? You can send me a patch for drivers/infiniband/* as well Thanks, Jason ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH] drm/nouveau: fix memory leak in iccsense/base.c
kmemleak report: unreferenced object 0x9071c65644e0 (size 96): comm "systemd-udevd", pid 347, jiffies 4294898424 (age 810.828s) hex dump (first 32 bytes): 02 01 00 00 00 00 00 00 00 00 10 00 02 04 00 00 00 00 00 00 00 00 a0 86 00 00 00 00 00 00 00 00 backtrace: [<7c0d0ac3>] __kmalloc+0x337/0x500 [<551bfaeb>] nvbios_iccsense_parse+0xf7/0x280 [nouveau] [] nvkm_iccsense_oneinit+0x6c/0x4e0 [nouveau] [<287e7701>] nvkm_subdev_init+0x58/0xd0 [nouveau] [<08e4793e>] nvkm_device_init+0x118/0x1a0 [nouveau] [<8cd3afa3>] nvkm_udevice_init+0x48/0x60 [nouveau] [<7e047aee>] nvkm_object_init+0x43/0x110 [nouveau] [<6c56b3a4>] nvkm_ioctl_new+0x184/0x210 [nouveau] [<80abc890>] nvkm_ioctl+0xf0/0x190 [nouveau] [ ] nvkm_client_ioctl+0x12/0x20 [nouveau] [<0f001008>] nvif_object_ioctl+0x4f/0x60 [nouveau] [<98d66807>] nvif_object_ctor+0xfb/0x160 [nouveau] [ ] nvif_device_ctor+0x24/0x70 [nouveau] [<878b3286>] nouveau_cli_init+0x1a3/0x460 [nouveau] [ ] nouveau_drm_device_init+0x77/0x740 [nouveau] [ ] nouveau_drm_probe+0x132/0x1f0 [nouveau] Fix nvkm_iccsense_oneinit(), to free stbl.rail post iteration. Signed-off-by: Vamshi K Sthambamkadi --- drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c index fecfa6a..23d91b6 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/iccsense/base.c @@ -291,6 +291,7 @@ nvkm_iccsense_oneinit(struct nvkm_subdev *subdev) list_add_tail(&rail->head, &iccsense->rails); } } + kfree(stbl.rail); return 0; } -- 2.7.4 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v4 05/10] drm/ttm: Add vmap/vunmap to TTM and TTM GEM helpers
Hi Christian
On 15.10.20 16:08, Christian König wrote:
> Am 15.10.20 um 14:38 schrieb Thomas Zimmermann:
>> The new functions ttm_bo_{vmap,vunmap}() map and unmap a TTM BO in kernel
>> address space. The mapping's address is returned as struct dma_buf_map.
>> Each function is a simplified version of TTM's existing kmap code. Both
>> functions respect the memory's location ani/or writecombine flags.
>>
>> On top TTM's functions, GEM TTM helpers got drm_gem_ttm_{vmap,vunmap}(),
>> two helpers that convert a GEM object into the TTM BO and forward the
>> call
>> to TTM's vmap/vunmap. These helpers can be dropped into the rsp GEM
>> object
>> callbacks.
>>
>> v4:
>> * drop ttm_kmap_obj_to_dma_buf() in favor of vmap helpers (Daniel,
>> Christian)
>
> Bunch of minor comments below, but over all look very solid to me.
>
>>
>> Signed-off-by: Thomas Zimmermann
>> ---
>> drivers/gpu/drm/drm_gem_ttm_helper.c | 38 +++
>> drivers/gpu/drm/ttm/ttm_bo_util.c | 72
>> include/drm/drm_gem_ttm_helper.h | 6 +++
>> include/drm/ttm/ttm_bo_api.h | 28 +++
>> include/linux/dma-buf-map.h | 20
>> 5 files changed, 164 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c
>> b/drivers/gpu/drm/drm_gem_ttm_helper.c
>> index 0e4fb9ba43ad..db4c14d78a30 100644
>> --- a/drivers/gpu/drm/drm_gem_ttm_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
>> @@ -49,6 +49,44 @@ void drm_gem_ttm_print_info(struct drm_printer *p,
>> unsigned int indent,
>> }
>> EXPORT_SYMBOL(drm_gem_ttm_print_info);
>> +/**
>> + * drm_gem_ttm_vmap() - vmap &ttm_buffer_object
>> + * @gem: GEM object.
>> + * @map: [out] returns the dma-buf mapping.
>> + *
>> + * Maps a GEM object with ttm_bo_vmap(). This function can be used as
>> + * &drm_gem_object_funcs.vmap callback.
>> + *
>> + * Returns:
>> + * 0 on success, or a negative errno code otherwise.
>> + */
>> +int drm_gem_ttm_vmap(struct drm_gem_object *gem,
>> + struct dma_buf_map *map)
>> +{
>> + struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
>> +
>> + return ttm_bo_vmap(bo, map);
>> +
>> +}
>> +EXPORT_SYMBOL(drm_gem_ttm_vmap);
>> +
>> +/**
>> + * drm_gem_ttm_vunmap() - vunmap &ttm_buffer_object
>> + * @gem: GEM object.
>> + * @map: dma-buf mapping.
>> + *
>> + * Unmaps a GEM object with ttm_bo_vunmap(). This function can be
>> used as
>> + * &drm_gem_object_funcs.vmap callback.
>> + */
>> +void drm_gem_ttm_vunmap(struct drm_gem_object *gem,
>> + struct dma_buf_map *map)
>> +{
>> + struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
>> +
>> + ttm_bo_vunmap(bo, map);
>> +}
>> +EXPORT_SYMBOL(drm_gem_ttm_vunmap);
>> +
>> /**
>> * drm_gem_ttm_mmap() - mmap &ttm_buffer_object
>> * @gem: GEM object.
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> index bdee4df1f3f2..80c42c774c7d 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> @@ -32,6 +32,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -526,6 +527,77 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
>> }
>> EXPORT_SYMBOL(ttm_bo_kunmap);
>> +int ttm_bo_vmap(struct ttm_buffer_object *bo, struct dma_buf_map *map)
>> +{
>> + struct ttm_resource *mem = &bo->mem;
>> + int ret;
>> +
>> + ret = ttm_mem_io_reserve(bo->bdev, mem);
>> + if (ret)
>> + return ret;
>> +
>> + if (mem->bus.is_iomem) {
>> + void __iomem *vaddr_iomem;
>> + unsigned long size = bo->num_pages << PAGE_SHIFT;
>
> Please use uint64_t here and make sure to cast bo->num_pages before
> shifting.
I thought the rule of thumb is to use u64 in source code. Yet TTM only
uses uint*_t types. Is there anything special about TTM?
>
> We have an unit tests of allocating a 8GB BO and that should work on a
> 32bit machine as well :)
>
>> +
>> + if (mem->bus.addr)
>> + vaddr_iomem = (void *)(((u8 *)mem->bus.addr));
I after reading the patch again, I realized that this is the
'ttm_bo_map_premapped' case and it's missing from _vunmap(). I see two
options here: ignore this case in _vunmap(), or do an ioremap()
unconditionally. Which one is preferable?
Best regards
Thomas
>> + else if (mem->placement & TTM_PL_FLAG_WC)
>
> I've just nuked the TTM_PL_FLAG_WC flag in drm-misc-next. There is a new
> mem->bus.caching enum as replacement.
>
>> + vaddr_iomem = ioremap_wc(mem->bus.offset, size);
>> + else
>> + vaddr_iomem = ioremap(mem->bus.offset, size);
>> +
>> + if (!vaddr_iomem)
>> + return -ENOMEM;
>> +
>> + dma_buf_map_set_vaddr_iomem(map, vaddr_iomem);
>> +
>> + } else {
>> + struct ttm_operation_ctx ctx = {
>> + .interruptible = false,
>> + .no_wait_gpu = false
>> + };
>> + struct ttm_tt *ttm = bo->ttm;
>> +
Re: [Nouveau] [PATCH v4 05/10] drm/ttm: Add vmap/vunmap to TTM and TTM GEM helpers
Hi Thomas,
[SNIP]
+int ttm_bo_vmap(struct ttm_buffer_object *bo, struct dma_buf_map *map)
+{
+ struct ttm_resource *mem = &bo->mem;
+ int ret;
+
+ ret = ttm_mem_io_reserve(bo->bdev, mem);
+ if (ret)
+ return ret;
+
+ if (mem->bus.is_iomem) {
+ void __iomem *vaddr_iomem;
+ unsigned long size = bo->num_pages << PAGE_SHIFT;
Please use uint64_t here and make sure to cast bo->num_pages before
shifting.
I thought the rule of thumb is to use u64 in source code. Yet TTM only
uses uint*_t types. Is there anything special about TTM?
My last status is that you can use both and my personal preference is to
use the uint*_t types because they are part of a higher level standard.
We have an unit tests of allocating a 8GB BO and that should work on a
32bit machine as well :)
+
+ if (mem->bus.addr)
+ vaddr_iomem = (void *)(((u8 *)mem->bus.addr));
I after reading the patch again, I realized that this is the
'ttm_bo_map_premapped' case and it's missing from _vunmap(). I see two
options here: ignore this case in _vunmap(), or do an ioremap()
unconditionally. Which one is preferable?
ioremap would be very very bad, so we should just do nothing.
Thanks,
Christian.
Best regards
Thomas
+ else if (mem->placement & TTM_PL_FLAG_WC)
I've just nuked the TTM_PL_FLAG_WC flag in drm-misc-next. There is a new
mem->bus.caching enum as replacement.
+ vaddr_iomem = ioremap_wc(mem->bus.offset, size);
+ else
+ vaddr_iomem = ioremap(mem->bus.offset, size);
+
+ if (!vaddr_iomem)
+ return -ENOMEM;
+
+ dma_buf_map_set_vaddr_iomem(map, vaddr_iomem);
+
+ } else {
+ struct ttm_operation_ctx ctx = {
+ .interruptible = false,
+ .no_wait_gpu = false
+ };
+ struct ttm_tt *ttm = bo->ttm;
+ pgprot_t prot;
+ void *vaddr;
+
+ BUG_ON(!ttm);
I think we can drop this, populate will just crash badly anyway.
+
+ ret = ttm_tt_populate(bo->bdev, ttm, &ctx);
+ if (ret)
+ return ret;
+
+ /*
+ * We need to use vmap to get the desired page protection
+ * or to make the buffer object look contiguous.
+ */
+ prot = ttm_io_prot(mem->placement, PAGE_KERNEL);
The calling convention has changed on drm-misc-next as well, but should
be trivial to adapt.
Regards,
Christian.
+ vaddr = vmap(ttm->pages, bo->num_pages, 0, prot);
+ if (!vaddr)
+ return -ENOMEM;
+
+ dma_buf_map_set_vaddr(map, vaddr);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(ttm_bo_vmap);
+
+void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct dma_buf_map
*map)
+{
+ if (dma_buf_map_is_null(map))
+ return;
+
+ if (map->is_iomem)
+ iounmap(map->vaddr_iomem);
+ else
+ vunmap(map->vaddr);
+ dma_buf_map_clear(map);
+
+ ttm_mem_io_free(bo->bdev, &bo->mem);
+}
+EXPORT_SYMBOL(ttm_bo_vunmap);
+
static int ttm_bo_wait_free_node(struct ttm_buffer_object *bo,
bool dst_use_tt)
{
diff --git a/include/drm/drm_gem_ttm_helper.h
b/include/drm/drm_gem_ttm_helper.h
index 118cef76f84f..7c6d874910b8 100644
--- a/include/drm/drm_gem_ttm_helper.h
+++ b/include/drm/drm_gem_ttm_helper.h
@@ -10,11 +10,17 @@
#include
#include
+struct dma_buf_map;
+
#define drm_gem_ttm_of_gem(gem_obj) \
container_of(gem_obj, struct ttm_buffer_object, base)
void drm_gem_ttm_print_info(struct drm_printer *p, unsigned int
indent,
const struct drm_gem_object *gem);
+int drm_gem_ttm_vmap(struct drm_gem_object *gem,
+ struct dma_buf_map *map);
+void drm_gem_ttm_vunmap(struct drm_gem_object *gem,
+ struct dma_buf_map *map);
int drm_gem_ttm_mmap(struct drm_gem_object *gem,
struct vm_area_struct *vma);
diff --git a/include/drm/ttm/ttm_bo_api.h
b/include/drm/ttm/ttm_bo_api.h
index 37102e45e496..2c59a785374c 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -48,6 +48,8 @@ struct ttm_bo_global;
struct ttm_bo_device;
+struct dma_buf_map;
+
struct drm_mm_node;
struct ttm_placement;
@@ -494,6 +496,32 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
unsigned long start_page,
*/
void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map);
+/**
+ * ttm_bo_vmap
+ *
+ * @bo: The buffer object.
+ * @map: pointer to a struct dma_buf_map representing the map.
+ *
+ * Sets up a kernel virtual mapping, using ioremap or vmap to the
+ * data in the buffer object. The parameter @map returns the virtual
+ * address as struct dma_buf_map. Unmap the buffer with ttm_bo_vunmap().
+ *
+ * Returns
+ * -ENOMEM: Out of memory.
+ * -EINVAL: Invalid range.
+ */
+int ttm_bo_vmap(struct ttm_buffer_object *bo, struct dma_buf_map *map);
+
+/**
+ * ttm_bo_vunmap
+ *
+ * @bo: The buffer object.
+ * @map: Object describing the map to unmap.
+ *
+ * Unmaps a kernel map set up by ttm_bo_vmap().
+ */
+void tt
[Nouveau] [PATCH] drm: remove unneeded break
From: Tom Rix
A break is not needed if it is preceded by a return or break
Signed-off-by: Tom Rix
---
drivers/gpu/drm/mgag200/mgag200_mode.c | 5 -
drivers/gpu/drm/nouveau/nvkm/subdev/bios/pll.c | 1 -
drivers/gpu/drm/nouveau/nvkm/subdev/clk/mcp77.c | 3 ---
drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramnv50.c | 1 -
drivers/gpu/drm/nouveau/nvkm/subdev/top/gk104.c | 1 -
drivers/gpu/drm/qxl/qxl_ioctl.c | 1 -
6 files changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 38672f9e5c4f..bbe4e60dfd08 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -794,21 +794,16 @@ static int mgag200_crtc_set_plls(struct mga_device *mdev,
long clock)
case G200_SE_A:
case G200_SE_B:
return mga_g200se_set_plls(mdev, clock);
- break;
case G200_WB:
case G200_EW3:
return mga_g200wb_set_plls(mdev, clock);
- break;
case G200_EV:
return mga_g200ev_set_plls(mdev, clock);
- break;
case G200_EH:
case G200_EH3:
return mga_g200eh_set_plls(mdev, clock);
- break;
case G200_ER:
return mga_g200er_set_plls(mdev, clock);
- break;
}
misc = RREG8(MGA_MISC_IN);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/pll.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/pll.c
index 350f10a3de37..2ec84b8a3b3a 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/pll.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/pll.c
@@ -123,7 +123,6 @@ pll_map(struct nvkm_bios *bios)
case NV_20:
case NV_30:
return nv04_pll_mapping;
- break;
case NV_40:
return nv40_pll_mapping;
case NV_50:
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/mcp77.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/mcp77.c
index efa50274df97..4884eb4a9221 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/mcp77.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/mcp77.c
@@ -140,17 +140,14 @@ mcp77_clk_read(struct nvkm_clk *base, enum nv_clk_src src)
break;
case nv_clk_src_mem:
return 0;
- break;
case nv_clk_src_vdec:
P = (read_div(clk) & 0x0700) >> 8;
switch (mast & 0x0040) {
case 0x0040:
return nvkm_clk_read(&clk->base, nv_clk_src_core) >> P;
- break;
default:
return 50 >> P;
- break;
}
break;
default:
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramnv50.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramnv50.c
index 2ccb4b6be153..7b1eb44ff3da 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramnv50.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramnv50.c
@@ -171,7 +171,6 @@ nv50_ram_timing_read(struct nv50_ram *ram, u32 *timing)
break;
default:
return -ENOSYS;
- break;
}
T(WR) = ((timing[1] >> 24) & 0xff) - 1 - T(CWL);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/top/gk104.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/top/gk104.c
index e01746ce9fc4..1156634533f9 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/top/gk104.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/top/gk104.c
@@ -90,7 +90,6 @@ gk104_top_oneinit(struct nvkm_top *top)
case 0x0010: B_(NVDEC ); break;
case 0x0013: B_(CE); break;
case 0x0014: C_(GSP ); break;
- break;
default:
break;
}
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 5cea6eea72ab..2072ddc9549c 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -160,7 +160,6 @@ static int qxl_process_single_command(struct qxl_device
*qdev,
default:
DRM_DEBUG("Only draw commands in execbuffers\n");
return -EINVAL;
- break;
}
if (cmd->command_size > PAGE_SIZE - sizeof(union qxl_release_info))
--
2.18.1
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v4 05/10] drm/ttm: Add vmap/vunmap to TTM and TTM GEM helpers
On Mon, Oct 19, 2020 at 11:45:05AM +0200, Christian König wrote:
> Hi Thomas,
>
> [SNIP]
> > > > +int ttm_bo_vmap(struct ttm_buffer_object *bo, struct dma_buf_map
> > > > *map)
> > > > +{
> > > > + struct ttm_resource *mem = &bo->mem;
> > > > + int ret;
> > > > +
> > > > + ret = ttm_mem_io_reserve(bo->bdev, mem);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + if (mem->bus.is_iomem) {
> > > > + void __iomem *vaddr_iomem;
> > > > + unsigned long size = bo->num_pages << PAGE_SHIFT;
> > > Please use uint64_t here and make sure to cast bo->num_pages before
> > > shifting.
> > I thought the rule of thumb is to use u64 in source code. Yet TTM only
> > uses uint*_t types. Is there anything special about TTM?
>
> My last status is that you can use both and my personal preference is to use
> the uint*_t types because they are part of a higher level standard.
Yeah the only hard rule is that in uapi headers you need to use the __u64
and similar typedefs, to avoid cluttering the namespace for unrelated
stuff in userspace.
In the kernel c99 types are perfectly fine, and I think slowly on the
rise.
-Daniel
>
> > > We have an unit tests of allocating a 8GB BO and that should work on a
> > > 32bit machine as well :)
> > >
> > > > +
> > > > + if (mem->bus.addr)
> > > > + vaddr_iomem = (void *)(((u8 *)mem->bus.addr));
> > I after reading the patch again, I realized that this is the
> > 'ttm_bo_map_premapped' case and it's missing from _vunmap(). I see two
> > options here: ignore this case in _vunmap(), or do an ioremap()
> > unconditionally. Which one is preferable?
>
> ioremap would be very very bad, so we should just do nothing.
>
> Thanks,
> Christian.
>
> >
> > Best regards
> > Thomas
> >
> > > > + else if (mem->placement & TTM_PL_FLAG_WC)
> > > I've just nuked the TTM_PL_FLAG_WC flag in drm-misc-next. There is a new
> > > mem->bus.caching enum as replacement.
> > >
> > > > + vaddr_iomem = ioremap_wc(mem->bus.offset, size);
> > > > + else
> > > > + vaddr_iomem = ioremap(mem->bus.offset, size);
> > > > +
> > > > + if (!vaddr_iomem)
> > > > + return -ENOMEM;
> > > > +
> > > > + dma_buf_map_set_vaddr_iomem(map, vaddr_iomem);
> > > > +
> > > > + } else {
> > > > + struct ttm_operation_ctx ctx = {
> > > > + .interruptible = false,
> > > > + .no_wait_gpu = false
> > > > + };
> > > > + struct ttm_tt *ttm = bo->ttm;
> > > > + pgprot_t prot;
> > > > + void *vaddr;
> > > > +
> > > > + BUG_ON(!ttm);
> > > I think we can drop this, populate will just crash badly anyway.
> > >
> > > > +
> > > > + ret = ttm_tt_populate(bo->bdev, ttm, &ctx);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + /*
> > > > + * We need to use vmap to get the desired page protection
> > > > + * or to make the buffer object look contiguous.
> > > > + */
> > > > + prot = ttm_io_prot(mem->placement, PAGE_KERNEL);
> > > The calling convention has changed on drm-misc-next as well, but should
> > > be trivial to adapt.
> > >
> > > Regards,
> > > Christian.
> > >
> > > > + vaddr = vmap(ttm->pages, bo->num_pages, 0, prot);
> > > > + if (!vaddr)
> > > > + return -ENOMEM;
> > > > +
> > > > + dma_buf_map_set_vaddr(map, vaddr);
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +EXPORT_SYMBOL(ttm_bo_vmap);
> > > > +
> > > > +void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct dma_buf_map
> > > > *map)
> > > > +{
> > > > + if (dma_buf_map_is_null(map))
> > > > + return;
> > > > +
> > > > + if (map->is_iomem)
> > > > + iounmap(map->vaddr_iomem);
> > > > + else
> > > > + vunmap(map->vaddr);
> > > > + dma_buf_map_clear(map);
> > > > +
> > > > + ttm_mem_io_free(bo->bdev, &bo->mem);
> > > > +}
> > > > +EXPORT_SYMBOL(ttm_bo_vunmap);
> > > > +
> > > > static int ttm_bo_wait_free_node(struct ttm_buffer_object *bo,
> > > > bool dst_use_tt)
> > > > {
> > > > diff --git a/include/drm/drm_gem_ttm_helper.h
> > > > b/include/drm/drm_gem_ttm_helper.h
> > > > index 118cef76f84f..7c6d874910b8 100644
> > > > --- a/include/drm/drm_gem_ttm_helper.h
> > > > +++ b/include/drm/drm_gem_ttm_helper.h
> > > > @@ -10,11 +10,17 @@
> > > > #include
> > > > #include
> > > > +struct dma_buf_map;
> > > > +
> > > > #define drm_gem_ttm_of_gem(gem_obj) \
> > > > container_of(gem_obj, struct ttm_buffer_object, base)
> > > > void drm_gem_ttm_print_info(struct drm_printer *p, unsigned int
> > > > indent,
> > > > const struct drm_gem_object *gem);
> > > > +int drm_gem_ttm_vmap(struct drm_gem_object *gem,
> > > > + struct dma_buf_map *map);
> > > > +void drm_gem_ttm_vunmap(struct drm_gem_object *gem,
> > > > + struct dma_buf_map *map);
> > > > i
Re: [Nouveau] [PATCH] drm: remove unneeded break
Hi Tom On Mon, Oct 19, 2020 at 09:31:15AM -0700, t...@redhat.com wrote: > From: Tom Rix > > A break is not needed if it is preceded by a return or break > > Signed-off-by: Tom Rix Looks good and builds with no warnings. One of the diffs made me - "oh this looks wrong". But after I looked again it was right and the resulting code is more readable - so good. Acked-by: Sam Ravnborg Was tempted to just apply to drm-misc-next but will give others the opportunity to chime in. Sam ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] Small patch for nv50_vbo.c
This isn't meant to fix applications, it's meant to limit the range of
access to the buffer, for robustness/etc reasons. One would have to
look very carefully, as all this logic is rather tricky.
On Mon, Oct 19, 2020 at 4:07 AM Andrew Randrianasulu
wrote:
>
> Hi all!
>
> I saw TODO comment in nv50_vbo.c and decided to look at similar file,
> nvc0_vbo.c. I copied those two lines into nv50 with slight name change
> (nvc0->nv50) and apparently it doesn't fix any OpenGL app I have, but does
> not broke them further (I have compute pacthes currently applied, so some
> breakage expected).
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> index 8d1c8c7665b..29b63a525aa 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> @@ -189,8 +189,12 @@ nv50_user_vbuf_range(struct nv50_context *nv50, unsigned
> vbi,
> assert(vbi < PIPE_MAX_ATTRIBS);
> if (unlikely(nv50->vertex->instance_bufs & (1 << vbi))) {
>/* TODO: use min and max instance divisor to get a proper range */
> - *base = 0;
> - *size = nv50->vtxbuf[vbi].buffer.resource->width0;
> + const uint32_t div = nv50->vertex->min_instance_div[vbi];
> + *base = nv50->instance_off * nv50->vtxbuf[vbi].stride;
> + *size = (nv50->instance_max / div) * nv50->vtxbuf[vbi].stride +
> + nv50->vertex->vb_access_size[vbi];
> +// *base = 0;
> +// *size = nv50->vtxbuf[vbi].buffer.resource->width0;
> } else {
>/* NOTE: if there are user buffers, we *must* have index bounds */
>assert(nv50->vb_elt_limit != ~0);
>
> Does this look correct?
>
>
> ___
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] Small patch for nv50_vbo.c
Hi all!
I saw TODO comment in nv50_vbo.c and decided to look at similar file,
nvc0_vbo.c. I copied those two lines into nv50 with slight name change
(nvc0->nv50) and apparently it doesn't fix any OpenGL app I have, but does not
broke them further (I have compute pacthes currently applied, so some breakage
expected).
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
index 8d1c8c7665b..29b63a525aa 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
@@ -189,8 +189,12 @@ nv50_user_vbuf_range(struct nv50_context *nv50, unsigned
vbi,
assert(vbi < PIPE_MAX_ATTRIBS);
if (unlikely(nv50->vertex->instance_bufs & (1 << vbi))) {
/* TODO: use min and max instance divisor to get a proper range */
- *base = 0;
- *size = nv50->vtxbuf[vbi].buffer.resource->width0;
+ const uint32_t div = nv50->vertex->min_instance_div[vbi];
+ *base = nv50->instance_off * nv50->vtxbuf[vbi].stride;
+ *size = (nv50->instance_max / div) * nv50->vtxbuf[vbi].stride +
+ nv50->vertex->vb_access_size[vbi];
+// *base = 0;
+// *size = nv50->vtxbuf[vbi].buffer.resource->width0;
} else {
/* NOTE: if there are user buffers, we *must* have index bounds */
assert(nv50->vb_elt_limit != ~0);
Does this look correct?
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
