[git pull] drm amdgpu + vmwgfx fixes for 5.9-rc8
Hi Linus, Just dequeuing these a bit early as the AMD ones are bit larger than I'd prefer, but Alex missed last week so it's a double set of fixes. The larger ones are just register header fixes for the new chips that were just introduced in rc1 along with some new PCI IDs for new hw. Otherwise it is usual fixes. The vmwgfx fix was due to some testing I was doing and found we weren't booting properly, vmware had the fix internally so hurried it out to me. I'm off tomorrow and Monday but I'll be around in case there are any major issues with this, or if I get set of intel or misc fixes come in. Dave. drm-fixes-2020-10-01-1: drm amd/vmwgfx fixes for 5.9-rc8 vmwgfx: - fix a regression due to TTM refactor amdgpu: - Fix potential double free in userptr handling - Sienna Cichlid and Navy Flounder updates - Add Sienna Cichlid PCI IDs - Drop experimental flag for navi12 - Raven fixes - Renoir fixes - HDCP fix - DCN3 fix for clang and older versions of gcc - Fix a runtime pm refcount issue The following changes since commit a1b8638ba1320e6684aa98233c15255eb803fac7: Linux 5.9-rc7 (2020-09-27 14:38:10 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2020-10-01-1 for you to fetch changes up to 132d7c8abeaa6b10ed5f47330b0f06c6dd220a43: Merge tag 'amd-drm-fixes-5.9-2020-09-30' of git://people.freedesktop.org/~agd5f/linux into drm-fixes (2020-10-01 15:25:33 +1000) drm amd/vmwgfx fixes for 5.9-rc8 vmwgfx: - fix a regression due to TTM refactor amdgpu: - Fix potential double free in userptr handling - Sienna Cichlid and Navy Flounder udpates - Add Sienna Cichlid PCI IDs - Drop experimental flag for navi12 - Raven fixes - Renoir fixes - HDCP fix - DCN3 fix for clang and older versions of gcc - Fix a runtime pm refcount issue Alex Deucher (6): drm/amdgpu: add the GC 10.3 VRS registers drm/amdgpu: add VCN 3.0 AV1 registers drm/amdgpu: use the AV1 defines for VCN 3.0 drm/amdgpu: remove experimental flag from navi12 drm/amdgpu/display: fix CFLAGS setup for DCN30 drm/amdgpu/swsmu/smu12: fix force clock handling for mclk Dave Airlie (2): Merge branch 'vmwgfx-fixes-5.9' of git://people.freedesktop.org/~sroland/linux into drm-fixes Merge tag 'amd-drm-fixes-5.9-2020-09-30' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Dirk Gouders (1): drm/amd/display: remove duplicate call to rn_vbios_smu_get_smu_version() Evan Quan (1): drm/amd/pm: setup APU dpm clock table in SMU HW initialization Flora Cui (1): drm/amd/display: fix return value check for hdcp_work Jean Delvare (1): drm/amdgpu: restore proper ref count in amdgpu_display_crtc_set_config Jiansong Chen (2): drm/amdgpu: remove gpu_info fw support for sienna_cichlid etc. drm/amdgpu: disable gfxoff temporarily for navy_flounder Likun Gao (1): drm/amdgpu: add device ID for sienna_cichlid (v2) Philip Yang (1): drm/amdgpu: prevent double kfree ttm->sg Sudheesh Mavila (1): drm/amd/pm: Removed fixed clock in auto mode DPM Zack Rusin (1): drm/vmwgfx: Fix error handling in get_node drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 + drivers/gpu/drm/amd/amdgpu/amdgpu_display.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 12 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 1 + drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 3 ++ drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 16 +++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c | 2 +- .../drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c | 1 - drivers/gpu/drm/amd/display/dc/dcn30/Makefile | 18 +++- .../amd/include/asic_reg/gc/gc_10_3_0_default.h| 2 + .../drm/amd/include/asic_reg/gc/gc_10_3_0_offset.h | 4 ++ .../amd/include/asic_reg/gc/gc_10_3_0_sh_mask.h| 50 ++ .../amd/include/asic_reg/vcn/vcn_3_0_0_sh_mask.h | 34 +++ drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 22 +- drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 10 +++-- drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 8 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_thp.c| 2 +- 18 files changed, 156 insertions(+), 43 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: fix regression in ttm moves
Tracked it down to my init mem type changes, patch is on the list. Dave. On Wed, 30 Sep 2020 at 18:28, Christian König wrote: > > That sounds like the same problem I've got when drm-next was merged into > drm-misc-next. > > I've fixed it in this commit: > > commit 0b06286579b81449b1e8f14f88d3a8db091fd443 > Author: Christian König > Date: Wed Aug 19 15:27:48 2020 +0200 > > drm/ttm: fix broken merge between drm-next and drm-misc-next > > drm-next reverted the changes to ttm_tt_create() to do the > NULL check inside the function, but drm-misc-next adds new > users of this approach. > > Re-apply the NULL check change inside the function to fix this. > > Signed-off-by: Christian König > Reviewed-by: Dave Airlie > Link: https://patchwork.freedesktop.org/patch/386628/ > > > Not sure why it should cause problems with drm-fixes and drm-next as well. > > Regards, > Christian. > > Am 30.09.20 um 09:09 schrieb Dave Airlie: > > just FYI I'm seeing a regression on vmwgfx with drm-fixes and drm-next > > merged into it. > > > > I'm going take some time to dig through and work out where, the > > regression is a command failure and a ioremap failure. > > > > Dave. > > > > On Wed, 30 Sep 2020 at 16:26, Dave Airlie wrote: > >> Uggh this is part of the mess with the revert, I'm not sure how best > >> to dig out of this one yet. > >> > >> Dave. > >> > >> On Wed, 30 Sep 2020 at 15:55, Dave Airlie wrote: > >>> From: Dave Airlie > >>> > >>> This fixes a bug introduced in be1213a341a289afc51f89181c310e368fba0b66 > >>> drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2 > >>> > >>> On vmwgfx this causes a Command buffer error WARN to trigger. > >>> > >>> This is because the old code used to check if bo->ttm was true, > >>> and the new code doesn't, fix it code to add back the check resolves > >>> the issue. > >>> > >>> Fixes: be1213a341a2 ("drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2") > >>> Signed-off-by: Dave Airlie > >>> --- > >>> drivers/gpu/drm/ttm/ttm_bo.c | 8 +--- > >>> 1 file changed, 5 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > >>> index 70b3bee27850..e8aa2fe8e9d1 100644 > >>> --- a/drivers/gpu/drm/ttm/ttm_bo.c > >>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c > >>> @@ -251,9 +251,11 @@ static int ttm_bo_handle_move_mem(struct > >>> ttm_buffer_object *bo, > >>> /* Zero init the new TTM structure if the old location > >>> should > >>> * have used one as well. > >>> */ > >>> - ret = ttm_tt_create(bo, old_man->use_tt); > >>> - if (ret) > >>> - goto out_err; > >>> + if (!bo->ttm) { > >>> + ret = ttm_tt_create(bo, old_man->use_tt); > >>> + if (ret) > >>> + goto out_err; > >>> + } > >>> > >>> ret = ttm_tt_set_placement_caching(bo->ttm, > >>> mem->placement); > >>> if (ret) > >>> -- > >>> 2.20.1 > >>> > >>> ___ > >>> dri-devel mailing list > >>> dri-devel@lists.freedesktop.org > >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7Ca8e51dce1b1346015c1e08d8650fdc59%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637370466085507013&sdata=QrtSgfkmSpNcNfdJ71YNATS0URyEcMNLeMVmOenRpak%3D&reserved=0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vmwgfx: fix regression in thp code due to ttm init refactor.
From: Dave Airlie When I refactored this code with the new init paths, I failed to set the funcs back up properly, this caused a failure to bringup gdm properly. Fixes: 252f8d7b9174 ("drm/vmwgfx/ttm: convert vram mm init to new code paths") Signed-off-by: Dave Airlie --- drivers/gpu/drm/vmwgfx/vmwgfx_thp.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c index 63fe7da4cbf4..c158e672b762 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_thp.c @@ -26,6 +26,8 @@ static struct vmw_thp_manager *to_thp_manager(struct ttm_resource_manager *man) return container_of(man, struct vmw_thp_manager, manager); } +static const struct ttm_resource_manager_func vmw_thp_func; + static int vmw_thp_insert_aligned(struct drm_mm *mm, struct drm_mm_node *node, unsigned long align_pages, const struct ttm_place *place, @@ -132,6 +134,7 @@ int vmw_thp_init(struct vmw_private *dev_priv) ttm_resource_manager_init(&rman->manager, dev_priv->vram_size >> PAGE_SHIFT); + rman->manager.func = &vmw_thp_func; drm_mm_init(&rman->mm, 0, rman->manager.size); spin_lock_init(&rman->lock); @@ -171,7 +174,7 @@ static void vmw_thp_debug(struct ttm_resource_manager *man, spin_unlock(&rman->lock); } -const struct ttm_resource_manager_func vmw_thp_func = { +static const struct ttm_resource_manager_func vmw_thp_func = { .alloc = vmw_thp_get_node, .free = vmw_thp_put_node, .debug = vmw_thp_debug -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 6/6] dma-buf: heaps: Skip sync if not mapped
This patch is basically a port of Ørjan Eide's similar patch for ION https://lore.kernel.org/lkml/20200414134629.54567-1-orjan.e...@arm.com/ Only sync the sg-list of dma-buf heap attachment when the attachment is actually mapped on the device. dma-bufs may be synced at any time. It can be reached from user space via DMA_BUF_IOCTL_SYNC, so there are no guarantees from callers on when syncs may be attempted, and dma_buf_end_cpu_access() and dma_buf_begin_cpu_access() may not be paired. Since the sg_list's dma_address isn't set up until the buffer is used on the device, and dma_map_sg() is called on it, the dma_address will be NULL if sync is attempted on the dma-buf before it's mapped on a device. Before v5.0 (commit 55897af63091 ("dma-direct: merge swiotlb_dma_ops into the dma_direct code")) this was a problem as the dma-api (at least the swiotlb_dma_ops on arm64) would use the potentially invalid dma_address. How that failed depended on how the device handled physical address 0. If 0 was a valid address to physical ram, that page would get flushed a lot, while the actual pages in the buffer would not get synced correctly. While if 0 is an invalid physical address it may cause a fault and trigger a crash. In v5.0 this was incidentally fixed by commit 55897af63091 ("dma-direct: merge swiotlb_dma_ops into the dma_direct code"), as this moved the dma-api to use the page pointer in the sg_list, and (for Ion buffers at least) this will always be valid if the sg_list exists at all. But, this issue is re-introduced in v5.3 with commit 449fa54d6815 ("dma-direct: correct the physical addr in dma_direct_sync_sg_for_cpu/device") moves the dma-api back to the old behaviour and picks the dma_address that may be invalid. dma-buf core doesn't ensure that the buffer is mapped on the device, and thus have a valid sg_list, before calling the exporter's begin_cpu_access. Logic and commit message originally by: Ørjan Eide Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz --- drivers/dma-buf/heaps/cma_heap.c| 10 ++ drivers/dma-buf/heaps/system_heap.c | 10 ++ 2 files changed, 20 insertions(+) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 4f20f07872e5..e19320f52063 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -44,6 +44,7 @@ struct dma_heap_attachment { struct device *dev; struct sg_table table; struct list_head list; + bool mapped; }; static int cma_heap_attach(struct dma_buf *dmabuf, @@ -68,6 +69,7 @@ static int cma_heap_attach(struct dma_buf *dmabuf, a->dev = attachment->dev; INIT_LIST_HEAD(&a->list); + a->mapped = false; attachment->priv = a; @@ -101,6 +103,7 @@ static struct sg_table *cma_heap_map_dma_buf(struct dma_buf_attachment *attachme if (!dma_map_sg(attachment->dev, table->sgl, table->nents, direction)) table = ERR_PTR(-ENOMEM); + a->mapped = true; return table; } @@ -108,6 +111,9 @@ static void cma_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, struct sg_table *table, enum dma_data_direction direction) { + struct dma_heap_attachment *a = attachment->priv; + + a->mapped = false; dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction); } @@ -122,6 +128,8 @@ static int cma_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, mutex_lock(&buffer->lock); list_for_each_entry(a, &buffer->attachments, list) { + if (!a->mapped) + continue; dma_sync_sg_for_cpu(a->dev, a->table.sgl, a->table.nents, direction); } @@ -141,6 +149,8 @@ static int cma_heap_dma_buf_end_cpu_access(struct dma_buf *dmabuf, mutex_lock(&buffer->lock); list_for_each_entry(a, &buffer->attachments, list) { + if (!a->mapped) + continue; dma_sync_sg_for_device(a->dev, a->table.sgl, a->table.nents, direction); } diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index f30904345cce..c0d051203300 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -38,6 +38,7 @@ struct dma_heap_attachment { struct device *dev; struct sg_table *table; struct list_head list; + bool mapped; }; #define HIGH_ORDER_GFP (((GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN \ @@ -94,6 +95,7 @@ static int system_heap_attach(struct
[PATCH v2 5/6] dma-buf: system_heap: Add pagepool support to system heap
Reuse/abuse the pagepool code from the network code to speed up allocation performance. This is similar to the ION pagepool usage, but tries to utilize generic code instead of a custom implementation. Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz --- drivers/dma-buf/heaps/Kconfig | 1 + drivers/dma-buf/heaps/system_heap.c | 32 + 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig index a5eef06c4226..f13cde4321b1 100644 --- a/drivers/dma-buf/heaps/Kconfig +++ b/drivers/dma-buf/heaps/Kconfig @@ -1,6 +1,7 @@ config DMABUF_HEAPS_SYSTEM bool "DMA-BUF System Heap" depends on DMABUF_HEAPS + select PAGE_POOL help Choose this option to enable the system dmabuf heap. The system heap is backed by pages from the buddy allocator. If in doubt, say Y. diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index d18937fa5b18..f30904345cce 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -20,6 +20,7 @@ #include #include #include +#include struct dma_heap *sys_heap; @@ -46,6 +47,7 @@ struct dma_heap_attachment { static gfp_t order_flags[] = {HIGH_ORDER_GFP, LOW_ORDER_GFP, LOW_ORDER_GFP}; static const unsigned int orders[] = {8, 4, 0}; #define NUM_ORDERS ARRAY_SIZE(orders) +struct page_pool *pools[NUM_ORDERS]; static struct sg_table *dup_sg_table(struct sg_table *table) { @@ -265,13 +267,17 @@ static void system_heap_dma_buf_release(struct dma_buf *dmabuf) struct system_heap_buffer *buffer = dmabuf->priv; struct sg_table *table; struct scatterlist *sg; - int i; + int i, j; table = &buffer->sg_table; for_each_sg(table->sgl, sg, table->nents, i) { struct page *page = sg_page(sg); - __free_pages(page, compound_order(page)); + for (j = 0; j < NUM_ORDERS; j++) { + if (compound_order(page) == orders[j]) + break; + } + page_pool_put_full_page(pools[j], page, false); } sg_free_table(table); kfree(buffer); @@ -301,8 +307,7 @@ static struct page *alloc_largest_available(unsigned long size, continue; if (max_order < orders[i]) continue; - - page = alloc_pages(order_flags[i], orders[i]); + page = page_pool_alloc_pages(pools[i], order_flags[i]); if (!page) continue; return page; @@ -407,6 +412,25 @@ static const struct dma_heap_ops system_heap_ops = { static int system_heap_create(void) { struct dma_heap_export_info exp_info; + int i; + + for (i = 0; i < NUM_ORDERS; i++) { + struct page_pool_params pp; + + memset(&pp, 0, sizeof(pp)); + pp.order = orders[i]; + pp.dma_dir = DMA_BIDIRECTIONAL; + pools[i] = page_pool_create(&pp); + + if (IS_ERR(pools[i])) { + int j; + + pr_err("%s: page pool creation failed!\n", __func__); + for (j = 0; j < i; j++) + page_pool_destroy(pools[j]); + return PTR_ERR(pools[i]); + } + } exp_info.name = "system"; exp_info.ops = &system_heap_ops; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 4/6] dma-buf: system_heap: Allocate higher order pages if available
While the system heap can return non-contiguous pages, try to allocate larger order pages if possible. This will allow slight performance gains and make implementing page pooling easier. Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz --- drivers/dma-buf/heaps/system_heap.c | 85 ++--- 1 file changed, 66 insertions(+), 19 deletions(-) diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index 7ec58f4e2bd3..d18937fa5b18 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -39,6 +39,14 @@ struct dma_heap_attachment { struct list_head list; }; +#define HIGH_ORDER_GFP (((GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN \ + | __GFP_NORETRY) & ~__GFP_RECLAIM) \ + | __GFP_COMP) +#define LOW_ORDER_GFP (GFP_HIGHUSER | __GFP_ZERO | __GFP_COMP) +static gfp_t order_flags[] = {HIGH_ORDER_GFP, LOW_ORDER_GFP, LOW_ORDER_GFP}; +static const unsigned int orders[] = {8, 4, 0}; +#define NUM_ORDERS ARRAY_SIZE(orders) + static struct sg_table *dup_sg_table(struct sg_table *table) { struct sg_table *new_table; @@ -260,8 +268,11 @@ static void system_heap_dma_buf_release(struct dma_buf *dmabuf) int i; table = &buffer->sg_table; - for_each_sgtable_sg(table, sg, i) - __free_page(sg_page(sg)); + for_each_sg(table->sgl, sg, table->nents, i) { + struct page *page = sg_page(sg); + + __free_pages(page, compound_order(page)); + } sg_free_table(table); kfree(buffer); } @@ -279,6 +290,26 @@ static const struct dma_buf_ops system_heap_buf_ops = { .release = system_heap_dma_buf_release, }; +static struct page *alloc_largest_available(unsigned long size, + unsigned int max_order) +{ + struct page *page; + int i; + + for (i = 0; i < NUM_ORDERS; i++) { + if (size < (PAGE_SIZE << orders[i])) + continue; + if (max_order < orders[i]) + continue; + + page = alloc_pages(order_flags[i], orders[i]); + if (!page) + continue; + return page; + } + return NULL; +} + static int system_heap_allocate(struct dma_heap *heap, unsigned long len, unsigned long fd_flags, @@ -286,11 +317,13 @@ static int system_heap_allocate(struct dma_heap *heap, { struct system_heap_buffer *buffer; DEFINE_DMA_BUF_EXPORT_INFO(exp_info); + unsigned long size_remaining = len; + unsigned int max_order = orders[0]; struct dma_buf *dmabuf; struct sg_table *table; struct scatterlist *sg; - pgoff_t pagecount; - pgoff_t pg; + struct list_head pages; + struct page *page, *tmp_page; int i, ret = -ENOMEM; buffer = kzalloc(sizeof(*buffer), GFP_KERNEL); @@ -302,25 +335,35 @@ static int system_heap_allocate(struct dma_heap *heap, buffer->heap = heap; buffer->len = len; - table = &buffer->sg_table; - pagecount = len / PAGE_SIZE; - if (sg_alloc_table(table, pagecount, GFP_KERNEL)) - goto free_buffer; - - sg = table->sgl; - for (pg = 0; pg < pagecount; pg++) { - struct page *page; + INIT_LIST_HEAD(&pages); + i = 0; + while (size_remaining > 0) { /* * Avoid trying to allocate memory if the process * has been killed by SIGKILL */ if (fatal_signal_pending(current)) - goto free_pages; - page = alloc_page(GFP_KERNEL | __GFP_ZERO); + goto free_buffer; + + page = alloc_largest_available(size_remaining, max_order); if (!page) - goto free_pages; - sg_set_page(sg, page, page_size(page), 0); + goto free_buffer; + + list_add_tail(&page->lru, &pages); + size_remaining -= PAGE_SIZE << compound_order(page); + max_order = compound_order(page); + i++; + } + + table = &buffer->sg_table; + if (sg_alloc_table(table, i, GFP_KERNEL)) + goto free_buffer; + + sg = table->sgl; + list_for_each_entry_safe(page, tmp_page, &pages, lru) { + sg_set_page(sg, page, PAGE_SIZE << compound_order(page), 0); sg = sg_next(sg); + list_del(&page->lru); } /
[PATCH v2 1/6] dma-buf: system_heap: Rework system heap to use sgtables instead of pagelists
In preparation for some patches to optmize the system heap code, rework the dmabuf exporter to utilize sgtables rather then pageslists for tracking the associated pages. This will allow for large order page allocations, as well as more efficient page pooling. In doing so, the system heap stops using the heap-helpers logic which sadly is not quite as generic as I was hoping it to be, so this patch adds heap specific implementations of the dma_buf_ops function handlers. Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz --- v2: * Fix locking issue and an unused return value Reported-by: kernel test robot Julia Lawall * Make system_heap_buf_ops static Reported-by: kernel test robot --- drivers/dma-buf/heaps/system_heap.c | 344 1 file changed, 298 insertions(+), 46 deletions(-) diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index 0bf688e3c023..7ec58f4e2bd3 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -3,7 +3,11 @@ * DMABUF System heap exporter * * Copyright (C) 2011 Google, Inc. - * Copyright (C) 2019 Linaro Ltd. + * Copyright (C) 2019, 2020 Linaro Ltd. + * + * Portions based off of Andrew Davis' SRAM heap: + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ + * Andrew F. Davis */ #include @@ -15,72 +19,321 @@ #include #include #include -#include -#include - -#include "heap-helpers.h" +#include struct dma_heap *sys_heap; -static void system_heap_free(struct heap_helper_buffer *buffer) +struct system_heap_buffer { + struct dma_heap *heap; + struct list_head attachments; + struct mutex lock; + unsigned long len; + struct sg_table sg_table; + int vmap_cnt; + void *vaddr; +}; + +struct dma_heap_attachment { + struct device *dev; + struct sg_table *table; + struct list_head list; +}; + +static struct sg_table *dup_sg_table(struct sg_table *table) { - pgoff_t pg; + struct sg_table *new_table; + int ret, i; + struct scatterlist *sg, *new_sg; + + new_table = kzalloc(sizeof(*new_table), GFP_KERNEL); + if (!new_table) + return ERR_PTR(-ENOMEM); + + ret = sg_alloc_table(new_table, table->nents, GFP_KERNEL); + if (ret) { + kfree(new_table); + return ERR_PTR(-ENOMEM); + } + + new_sg = new_table->sgl; + for_each_sgtable_sg(table, sg, i) { + sg_set_page(new_sg, sg_page(sg), sg->length, sg->offset); + new_sg = sg_next(new_sg); + } + + return new_table; +} + +static int system_heap_attach(struct dma_buf *dmabuf, + struct dma_buf_attachment *attachment) +{ + struct system_heap_buffer *buffer = dmabuf->priv; + struct dma_heap_attachment *a; + struct sg_table *table; + + a = kzalloc(sizeof(*a), GFP_KERNEL); + if (!a) + return -ENOMEM; + + table = dup_sg_table(&buffer->sg_table); + if (IS_ERR(table)) { + kfree(a); + return -ENOMEM; + } + + a->table = table; + a->dev = attachment->dev; + INIT_LIST_HEAD(&a->list); + + attachment->priv = a; + + mutex_lock(&buffer->lock); + list_add(&a->list, &buffer->attachments); + mutex_unlock(&buffer->lock); + + return 0; +} + +static void system_heap_detatch(struct dma_buf *dmabuf, + struct dma_buf_attachment *attachment) +{ + struct system_heap_buffer *buffer = dmabuf->priv; + struct dma_heap_attachment *a = attachment->priv; + + mutex_lock(&buffer->lock); + list_del(&a->list); + mutex_unlock(&buffer->lock); + + sg_free_table(a->table); + kfree(a->table); + kfree(a); +} + +static struct sg_table *system_heap_map_dma_buf(struct dma_buf_attachment *attachment, + enum dma_data_direction direction) +{ + struct dma_heap_attachment *a = attachment->priv; + struct sg_table *table = a->table; + + if (!dma_map_sg(attachment->dev, table->sgl, table->nents, direction)) + return ERR_PTR(-ENOMEM); + + return table; +} + +static void system_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, + struct sg_table *table, + enum dma_data_direction direction) +{ + dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction); +} + +static int system_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, +
[PATCH v2 2/6] dma-buf: heaps: Move heap-helper logic into the cma_heap implementation
Since the heap-helpers logic ended up not being as generic as hoped, move the heap-helpers dma_buf_ops implementations into the cma_heap directly. This will allow us to remove the heap_helpers code in a following patch. Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz --- v2: * Fix unused return value and locking issue Reported-by: kernel test robot Julia Lawall * Make cma_heap_buf_ops static suggested by kernel test robot * Fix uninitialized return in cma Reported-by: kernel test robot * Minor cleanups --- drivers/dma-buf/heaps/cma_heap.c | 318 ++- 1 file changed, 268 insertions(+), 50 deletions(-) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 626cf7fd033a..4f20f07872e5 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -2,76 +2,292 @@ /* * DMABUF CMA heap exporter * - * Copyright (C) 2012, 2019 Linaro Ltd. + * Copyright (C) 2012, 2019, 2020 Linaro Ltd. * Author: for ST-Ericsson. + * + * Also utilizing parts of Andrew Davis' SRAM heap: + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ + * Andrew F. Davis */ - #include -#include #include -#include #include +#include +#include #include -#include #include +#include +#include #include -#include #include -#include +#include -#include "heap-helpers.h" struct cma_heap { struct dma_heap *heap; struct cma *cma; }; -static void cma_heap_free(struct heap_helper_buffer *buffer) +struct cma_heap_buffer { + struct cma_heap *heap; + struct list_head attachments; + struct mutex lock; + unsigned long len; + struct page *cma_pages; + struct page **pages; + pgoff_t pagecount; + int vmap_cnt; + void *vaddr; +}; + +struct dma_heap_attachment { + struct device *dev; + struct sg_table table; + struct list_head list; +}; + +static int cma_heap_attach(struct dma_buf *dmabuf, + struct dma_buf_attachment *attachment) { - struct cma_heap *cma_heap = dma_heap_get_drvdata(buffer->heap); - unsigned long nr_pages = buffer->pagecount; - struct page *cma_pages = buffer->priv_virt; + struct cma_heap_buffer *buffer = dmabuf->priv; + struct dma_heap_attachment *a; + int ret; - /* free page list */ - kfree(buffer->pages); - /* release memory */ - cma_release(cma_heap->cma, cma_pages, nr_pages); + a = kzalloc(sizeof(*a), GFP_KERNEL); + if (!a) + return -ENOMEM; + + ret = sg_alloc_table_from_pages(&a->table, buffer->pages, + buffer->pagecount, 0, + buffer->pagecount << PAGE_SHIFT, + GFP_KERNEL); + if (ret) { + kfree(a); + return ret; + } + + a->dev = attachment->dev; + INIT_LIST_HEAD(&a->list); + + attachment->priv = a; + + mutex_lock(&buffer->lock); + list_add(&a->list, &buffer->attachments); + mutex_unlock(&buffer->lock); + + return 0; +} + +static void cma_heap_detatch(struct dma_buf *dmabuf, +struct dma_buf_attachment *attachment) +{ + struct cma_heap_buffer *buffer = dmabuf->priv; + struct dma_heap_attachment *a = attachment->priv; + + mutex_lock(&buffer->lock); + list_del(&a->list); + mutex_unlock(&buffer->lock); + + sg_free_table(&a->table); + kfree(a); +} + +static struct sg_table *cma_heap_map_dma_buf(struct dma_buf_attachment *attachment, +enum dma_data_direction direction) +{ + struct dma_heap_attachment *a = attachment->priv; + struct sg_table *table = &a->table; + + if (!dma_map_sg(attachment->dev, table->sgl, table->nents, + direction)) + table = ERR_PTR(-ENOMEM); + return table; +} + +static void cma_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, + struct sg_table *table, + enum dma_data_direction direction) +{ + dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction); +} + +static int cma_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, +enum dma_data_direction direction) +{ + struct cma_heap_buffer *buffer = dmabuf->priv; + struct dma_heap_attachment *a; + + if (buffer->vmap_cnt) + invalidate_kernel_vmap_range(buffer->vaddr, buffer->len); + + mutex_lock(&buffer->lock);
[PATCH v2 0/6] dma-buf: Performance improvements for system heap
Hey All, So this patch series contains a series of performance optimizations to the dma-buf system heap. Unfortunately, in working these up, I realized the heap-helpers infrastructure we tried to add to miniimize code duplication is not as generic as we intended. For some heaps it makes sense to deal with page lists, for other heaps it makes more sense to track things with sgtables. So this series reworks the system heap to use sgtables, and then consolidates the pagelist method from the heap-helpers into the CMA heap. After which the heap-helpers logic is removed (as it is unused). I'd still like to find a better way to avoid some of the logic duplication in implementing the entire dma_buf_ops handlers per heap. But unfortunately that code is tied somewhat to how the buffer's memory is tracked. After this, the series introduces two optimizations to the the system heap, utilizing large order pages, and adding a page-pool (maybe abusing the pagepool logic from the network code, but it seems silly to reimplement it). I implemented a simple allocation microbenchmark to compare dmabuf heaps vs ion: https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/dma-buf-heap-perf&id=24c723fb41c7a9cb5cf9b2412722866cba3a1417 With these changes, the allocation path is *much* improved, performing better then ION (though to be fair, the repeated allocating and freeing of the same size buffer is the ideal case for the pagepool logic, so don't read too much into it). I charted some datapoints from the microbenchmark with each of the patches should folks be interested. https://docs.google.com/spreadsheets/d/1-1C8ZQpmkl_0DISkI6z4xelE08MlNAN7oEu34AnO4Ao/edit#gid=0 Finally, a port of a patch that Ørjan Eide implemented for ION that avoids calling sync on attachments that don't have a mapping. Feedback on these would be great! thanks -john New in v2: * Fix unused return value and locking issue Reported-by: kernel test robot Julia Lawall * Make system_heap_buf_ops static Reported-by: kernel test robot * Make cma_heap_buf_ops static suggested by kernel test robot * Fix uninitialized return in cma Reported-by: kernel test robot * Minor cleanups Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org John Stultz (6): dma-buf: system_heap: Rework system heap to use sgtables instead of pagelists dma-buf: heaps: Move heap-helper logic into the cma_heap implementation dma-buf: heaps: Remove heap-helpers code dma-buf: system_heap: Allocate higher order pages if available dma-buf: system_heap: Add pagepool support to system heap dma-buf: heaps: Skip sync if not mapped drivers/dma-buf/heaps/Kconfig| 1 + drivers/dma-buf/heaps/Makefile | 1 - drivers/dma-buf/heaps/cma_heap.c | 328 drivers/dma-buf/heaps/heap-helpers.c | 271 - drivers/dma-buf/heaps/heap-helpers.h | 53 drivers/dma-buf/heaps/system_heap.c | 427 --- 6 files changed, 659 insertions(+), 422 deletions(-) delete mode 100644 drivers/dma-buf/heaps/heap-helpers.c delete mode 100644 drivers/dma-buf/heaps/heap-helpers.h -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/6] dma-buf: heaps: Remove heap-helpers code
The heap-helpers code was not as generic as initially hoped and it is now not being used, so remove it from the tree. Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz --- drivers/dma-buf/heaps/Makefile | 1 - drivers/dma-buf/heaps/heap-helpers.c | 271 --- drivers/dma-buf/heaps/heap-helpers.h | 53 -- 3 files changed, 325 deletions(-) delete mode 100644 drivers/dma-buf/heaps/heap-helpers.c delete mode 100644 drivers/dma-buf/heaps/heap-helpers.h diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile index 6e54cdec3da0..974467791032 100644 --- a/drivers/dma-buf/heaps/Makefile +++ b/drivers/dma-buf/heaps/Makefile @@ -1,4 +1,3 @@ # SPDX-License-Identifier: GPL-2.0 -obj-y += heap-helpers.o obj-$(CONFIG_DMABUF_HEAPS_SYSTEM) += system_heap.o obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o diff --git a/drivers/dma-buf/heaps/heap-helpers.c b/drivers/dma-buf/heaps/heap-helpers.c deleted file mode 100644 index 9f964ca3f59c.. --- a/drivers/dma-buf/heaps/heap-helpers.c +++ /dev/null @@ -1,271 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include "heap-helpers.h" - -void init_heap_helper_buffer(struct heap_helper_buffer *buffer, -void (*free)(struct heap_helper_buffer *)) -{ - buffer->priv_virt = NULL; - mutex_init(&buffer->lock); - buffer->vmap_cnt = 0; - buffer->vaddr = NULL; - buffer->pagecount = 0; - buffer->pages = NULL; - INIT_LIST_HEAD(&buffer->attachments); - buffer->free = free; -} - -struct dma_buf *heap_helper_export_dmabuf(struct heap_helper_buffer *buffer, - int fd_flags) -{ - DEFINE_DMA_BUF_EXPORT_INFO(exp_info); - - exp_info.ops = &heap_helper_ops; - exp_info.size = buffer->size; - exp_info.flags = fd_flags; - exp_info.priv = buffer; - - return dma_buf_export(&exp_info); -} - -static void *dma_heap_map_kernel(struct heap_helper_buffer *buffer) -{ - void *vaddr; - - vaddr = vmap(buffer->pages, buffer->pagecount, VM_MAP, PAGE_KERNEL); - if (!vaddr) - return ERR_PTR(-ENOMEM); - - return vaddr; -} - -static void dma_heap_buffer_destroy(struct heap_helper_buffer *buffer) -{ - if (buffer->vmap_cnt > 0) { - WARN(1, "%s: buffer still mapped in the kernel\n", __func__); - vunmap(buffer->vaddr); - } - - buffer->free(buffer); -} - -static void *dma_heap_buffer_vmap_get(struct heap_helper_buffer *buffer) -{ - void *vaddr; - - if (buffer->vmap_cnt) { - buffer->vmap_cnt++; - return buffer->vaddr; - } - vaddr = dma_heap_map_kernel(buffer); - if (IS_ERR(vaddr)) - return vaddr; - buffer->vaddr = vaddr; - buffer->vmap_cnt++; - return vaddr; -} - -static void dma_heap_buffer_vmap_put(struct heap_helper_buffer *buffer) -{ - if (!--buffer->vmap_cnt) { - vunmap(buffer->vaddr); - buffer->vaddr = NULL; - } -} - -struct dma_heaps_attachment { - struct device *dev; - struct sg_table table; - struct list_head list; -}; - -static int dma_heap_attach(struct dma_buf *dmabuf, - struct dma_buf_attachment *attachment) -{ - struct dma_heaps_attachment *a; - struct heap_helper_buffer *buffer = dmabuf->priv; - int ret; - - a = kzalloc(sizeof(*a), GFP_KERNEL); - if (!a) - return -ENOMEM; - - ret = sg_alloc_table_from_pages(&a->table, buffer->pages, - buffer->pagecount, 0, - buffer->pagecount << PAGE_SHIFT, - GFP_KERNEL); - if (ret) { - kfree(a); - return ret; - } - - a->dev = attachment->dev; - INIT_LIST_HEAD(&a->list); - - attachment->priv = a; - - mutex_lock(&buffer->lock); - list_add(&a->list, &buffer->attachments); - mutex_unlock(&buffer->lock); - - return 0; -} - -static void dma_heap_detach(struct dma_buf *dmabuf, - struct dma_buf_attachment *attachment) -{ - struct dma_heaps_attachment *a = attachment->priv; - struct heap_helper_buffer *buffer = dmabuf->priv; - - mutex_lock(&buffer->lock); - list_del(&a->list); - mutex_unlock(&buffer->lock); - - sg_free_table(&a->table); - kfree(a); -} - -static -struct sg_table *dma_heap_map_dma_
Re: [Nouveau] [PATCH] drm/nouveau: Drop mutex_lock_nested for atomic
On Wed, 30 Sep 2020 at 19:37, Daniel Vetter wrote: > > On Wed, Sep 30, 2020 at 10:45:05AM +1000, Ben Skeggs wrote: > > On Wed, 30 Sep 2020 at 00:52, Daniel Vetter wrote: > > > > > > On Thu, Sep 17, 2020 at 3:15 PM Daniel Vetter > > > wrote: > > > > > > > > Ben, did you have a chance to look at this? > > > > > > Ping > > > -Daniel > > > > > > > On Mon, Aug 3, 2020 at 1:22 PM Maarten Lankhorst > > > > wrote: > > > > > > > > > > Op 02-08-2020 om 20:18 schreef Daniel Vetter: > > > > > > Purely conjecture, but I think the original locking inversion with > > > > > > the > > > > > > legacy page flip code between flipping and ttm's bo move function > > > > > > shoudn't exist anymore with atomic: With atomic the bo pinning and > > > > > > actual modeset commit is completely separated in the code patsh. > > > > > > > > > > > > This annotation was originally added in > > > > > > > > > > > > commit 060810d7abaabcab282e062c595871d661561400 > > > > > > Author: Ben Skeggs > > > > > > Date: Mon Jul 8 14:15:51 2013 +1000 > > > > > > > > > > > > drm/nouveau: fix locking issues in page flipping paths > > > > > > > > > > > > due to > > > > > > > > > > > > commit b580c9e2b7ba5030a795aa2fb73b796523d65a78 > > > > > > Author: Maarten Lankhorst > > > > > > Date: Thu Jun 27 13:48:18 2013 +0200 > > > > > > > > > > > > drm/nouveau: make flipping lockdep safe > > > > > > > > > > > > Signed-off-by: Daniel Vetter > > > > > > Cc: Maarten Lankhorst > > > > > > Cc: Ben Skeggs > > > > > > Cc: Dave Airlie > > > > > > Cc: nouv...@lists.freedesktop.org > > > > > > --- > > > > > > I might be totally wrong, so this definitely needs testing :-) > > > > > > > > > > > > Cheers, Daniel > > > > > > --- > > > > > > drivers/gpu/drm/nouveau/nouveau_bo.c | 6 +- > > > > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c > > > > > > b/drivers/gpu/drm/nouveau/nouveau_bo.c > > > > > > index 7806278dce57..a7b2a9bb0ffe 100644 > > > > > > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c > > > > > > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c > > > > > > @@ -776,7 +776,11 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object > > > > > > *bo, int evict, bool intr, > > > > > > return ret; > > > > > > } > > > > > > > > > > > > - mutex_lock_nested(&cli->mutex, SINGLE_DEPTH_NESTING); > > > > > > + if (drm_drv_uses_atomic_modeset(drm->dev)) > > > > > > + mutex_lock(&cli->mutex); > > > > > > + else > > > > > > + mutex_lock_nested(&cli->mutex, SINGLE_DEPTH_NESTING); > > > > > > + > > > > > > ret = nouveau_fence_sync(nouveau_bo(bo), chan, true, intr); > > > > > > if (ret == 0) { > > > > > > ret = drm->ttm.move(chan, bo, &bo->mem, new_reg); > > > > > > > > > > Well if you're certain it works now. :) > > > > > > > > > > Reviewed-by: Maarten Lankhorst > > Acked-by: Ben Skeggs > > Can you pull this in through your tree and maybe give it a spin just to > make sure? I don't really have nouveau hardware here. Yeah, I can do that easily enough. Ben. > > Also it's entirely stand-alone, I was simply reviewing all the > mutex_lock_nested we have in drm, and this one stuck out as probably not > necessary anymore, at least with atomic. > > I guess I can also just stuff it into drm-misc-next and if it blows up, > figure out what to do then :-) > -Daniel > > > > > > > > > > > > > > > > > > > > -- > > > > Daniel Vetter > > > > Software Engineer, Intel Corporation > > > > http://blog.ffwll.ch > > > > > > > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > > ___ > > > Nouveau mailing list > > > nouv...@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/nouveau > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RFC 1/1] drm-ttm: Allocate transparent huge pages without clearing __GFP_COMP
TTM currently supports allocating pages with GFP_TRANSHUGE_LIGHT, but with the __GFP_COMP flag cleared. Instead of being normal transparent huge pages, these are multiorder non-compound pages that have the same order as THPs. This interferes with drivers that import DMA-BUFs / SGTs backed by pages from TTM, as they don't have the TTM-specific context to know how the pages were allocated. Change the TTM allocator so that it no longer clears the __GFP_COMP flag when allocating THPs. Signed-off-by: Alex Goins --- drivers/gpu/drm/ttm/ttm_page_alloc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c index 5c8883d7f74a..e72789184398 100644 --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c @@ -920,7 +920,6 @@ static int ttm_get_pages(struct page **pages, unsigned npages, int flags, huge_flags |= GFP_TRANSHUGE_LIGHT | __GFP_NORETRY | __GFP_KSWAPD_RECLAIM; huge_flags &= ~__GFP_MOVABLE; - huge_flags &= ~__GFP_COMP; p = alloc_pages(huge_flags, HPAGE_PMD_ORDER); if (!p) break; @@ -1057,13 +1056,13 @@ int ttm_page_alloc_init(struct ttm_mem_global *glob, unsigned max_pages) ttm_page_pool_init_locked(&_manager->wc_pool_huge, (GFP_TRANSHUGE_LIGHT | __GFP_NORETRY | __GFP_KSWAPD_RECLAIM) & - ~(__GFP_MOVABLE | __GFP_COMP), + ~(__GFP_MOVABLE), "wc huge", order); ttm_page_pool_init_locked(&_manager->uc_pool_huge, (GFP_TRANSHUGE_LIGHT | __GFP_NORETRY | __GFP_KSWAPD_RECLAIM) & - ~(__GFP_MOVABLE | __GFP_COMP) + ~(__GFP_MOVABLE) , "uc huge", order); _manager->options.max_size = max_pages; -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RFC 0/1] drm/ttm: Allocate transparent huge pages without clearing __GFP_COMP
Hi Christian, I've been looking into the DMA-BUFs exported from AMDGPU / TTM. Would you mind giving some input on this? I noticed that your changes implementing transparent huge page support in TTM are allocating them as non-compound. I understand that using multiorder non-compound pages is common in device drivers, but I think this can cause a problem when these pages are exported to other drivers. It's possible for other drivers to access the DMA-BUF's pages via gem_prime_import_sg_table(), but without context from TTM, it's impossible for the importing driver to make sense of them; they simply appear as individual pages, with only the first page having a non-zero refcount. Making TTM's THP allocations compound puts them more in line with the standard definition of a THP, and allows DMA-BUF-importing drivers to make sense of the pages within. I would like to propose making these allocations compound, but based on patch history, it looks like the decision to make them non-compound was intentional, as there were difficulties figuring out how to map them into CPU page tables. I did some cursory testing with compound THPs, and nothing seems obviously broken. I was also able to map compound THP DMA-BUFs into userspace without issue, and access their contents. Are you aware of any other potential consequences? Commit 5c42c64f7d54 ("drm/ttm: fix the fix for huge compound pages") should probably also be reverted if this is applied. Thanks, Alex Alex Goins (1): drm-ttm: Allocate compound transparent huge pages drivers/gpu/drm/ttm/ttm_page_alloc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 3/3] drm: Expose CRTC's kworker task id
From: Rob Clark This will allow userspace to control the scheduling policy and priority. In particular if the userspace half of the display pipeline is SCHED_FIFO then it will want to use the same scheduling policy and an appropriate priority to ensure that it is not preempting commit_work. Signed-off-by: Rob Clark --- drivers/gpu/drm/drm_crtc.c| 3 +++ drivers/gpu/drm/drm_mode_config.c | 14 ++ drivers/gpu/drm/drm_mode_object.c | 4 include/drm/drm_mode_config.h | 9 + include/drm/drm_property.h| 9 + 5 files changed, 39 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 4f7c0bfce0a3..1828853542dc 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -334,6 +334,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, crtc->worker = NULL; return ret; } + + drm_object_attach_property(&crtc->base, + config->kwork_tid_property, 0); } return 0; diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index f1affc1bb679..b11a1fc8ed0d 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -215,6 +215,13 @@ static const struct drm_prop_enum_list drm_plane_type_enum_list[] = { { DRM_PLANE_TYPE_CURSOR, "Cursor" }, }; +static int get_kwork_tid(struct drm_mode_object *obj, uint64_t *val) +{ + struct drm_crtc *crtc = obj_to_crtc(obj); + *val = task_pid_vnr(crtc->worker->task); + return 0; +} + static int drm_mode_create_standard_properties(struct drm_device *dev) { struct drm_property *prop; @@ -371,6 +378,13 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.modifiers_property = prop; + prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC, + "KWORK_TID", 0, UINT_MAX); + if (!prop) + return -ENOMEM; + prop->get_value = get_kwork_tid; + dev->mode_config.kwork_tid_property = prop; + return 0; } diff --git a/drivers/gpu/drm/drm_mode_object.c b/drivers/gpu/drm/drm_mode_object.c index db05f386a709..1a4df65baf0f 100644 --- a/drivers/gpu/drm/drm_mode_object.c +++ b/drivers/gpu/drm/drm_mode_object.c @@ -285,6 +285,7 @@ int drm_object_property_set_value(struct drm_mode_object *obj, WARN_ON(drm_drv_uses_atomic_modeset(property->dev) && !(property->flags & DRM_MODE_PROP_IMMUTABLE)); + WARN_ON(property->get_value); for (i = 0; i < obj->properties->count; i++) { if (obj->properties->properties[i] == property) { @@ -303,6 +304,9 @@ static int __drm_object_property_get_value(struct drm_mode_object *obj, { int i; + if (property->get_value) + return property->get_value(obj, val); + /* read-only properties bypass atomic mechanism and still store * their value in obj->properties->values[].. mostly to avoid * having to deal w/ EDID and similar props in atomic paths: diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index c2d3d71d133c..7244df926a6d 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -926,6 +926,15 @@ struct drm_mode_config { */ struct drm_property *modifiers_property; + /** +* @kwork_tid_property: CRTC property to expose the task-id of the per- +* CRTC kthread-worker, used for non-block atomic commit. This is exposed +* to userspace, to allow userspace to control the scheduling policy and +* priority, as this is a decision that depends on how userspace structures +* it's rendering pipeline. +*/ + struct drm_property *kwork_tid_property; + /* cursor size */ uint32_t cursor_width, cursor_height; diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h index 4a0a80d658c7..6843be6aa3ec 100644 --- a/include/drm/drm_property.h +++ b/include/drm/drm_property.h @@ -188,6 +188,15 @@ struct drm_property { * enum and bitmask values. */ struct list_head enum_list; + + /** +* @get_value: accessor to get current value for "virtual" properties +* +* For properties with dynamic values, where it is for whatever reason +* not feasible to keep updated with drm_object_property_set_value(), +* this callback can be used to retrieve the current value on demand. +*/ + int (*get_value)(struct drm_mode_object *obj, uint64_t *val); }; /** -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 0/3] drm: commit_work scheduling
From: Rob Clark The android userspace treats the display pipeline as a realtime problem. And arguably, if your goal is to not miss frame deadlines (ie. vblank), it is. (See https://lwn.net/Articles/809545/ for the best explaination that I found.) But this presents a problem with using workqueues for non-blocking atomic commit_work(), because the SCHED_FIFO userspace thread(s) can preempt the worker. Which is not really the outcome you want.. once the required fences are scheduled, you want to push the atomic commit down to hw ASAP. But the decision of whether commit_work should be RT or not really depends on what userspace is doing. For a pure CFS userspace display pipeline, commit_work() should remain SCHED_NORMAL. To handle this, convert non-blocking commit_work() to use per-CRTC kthread workers, instead of system_unbound_wq. Per-CRTC workers are used to avoid serializing commits when userspace is using a per-CRTC update loop. And the last patch exposes the task id to userspace as a CRTC property, so that userspace can adjust the priority and sched policy to fit it's needs. v2: Drop client cap and in-kernel setting of priority/policy in favor of exposing the kworker tid to userspace so that user- space can set priority/policy. Rob Clark (3): drm/crtc: Introduce per-crtc kworker drm/atomic: Use kthread worker for nonblocking commits drm: Expose CRTC's kworker task id drivers/gpu/drm/drm_atomic_helper.c | 13 drivers/gpu/drm/drm_crtc.c | 14 + drivers/gpu/drm/drm_mode_config.c | 14 + drivers/gpu/drm/drm_mode_object.c | 4 include/drm/drm_atomic.h| 31 + include/drm/drm_crtc.h | 8 include/drm/drm_mode_config.h | 9 + include/drm/drm_property.h | 9 + 8 files changed, 98 insertions(+), 4 deletions(-) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 2/3] drm/atomic: Use kthread worker for nonblocking commits
From: Rob Clark This will allow us to more easily switch scheduling rules based on what userspace wants. Signed-off-by: Rob Clark --- drivers/gpu/drm/drm_atomic_helper.c | 13 include/drm/drm_atomic.h| 31 + 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 9e1ad493e689..75eeec5e7b10 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1659,11 +1659,11 @@ static void commit_tail(struct drm_atomic_state *old_state) drm_atomic_state_put(old_state); } -static void commit_work(struct work_struct *work) +static void commit_work(struct kthread_work *work) { struct drm_atomic_state *state = container_of(work, struct drm_atomic_state, - commit_work); + commit_kwork); commit_tail(state); } @@ -1797,6 +1797,7 @@ int drm_atomic_helper_commit(struct drm_device *dev, struct drm_atomic_state *state, bool nonblock) { + struct kthread_worker *worker = NULL; int ret; if (state->async_update) { @@ -1814,7 +1815,7 @@ int drm_atomic_helper_commit(struct drm_device *dev, if (ret) return ret; - INIT_WORK(&state->commit_work, commit_work); + kthread_init_work(&state->commit_kwork, commit_work); ret = drm_atomic_helper_prepare_planes(dev, state); if (ret) @@ -1857,8 +1858,12 @@ int drm_atomic_helper_commit(struct drm_device *dev, */ drm_atomic_state_get(state); + if (nonblock) - queue_work(system_unbound_wq, &state->commit_work); + worker = drm_atomic_pick_worker(state); + + if (worker) + kthread_queue_work(worker, &state->commit_kwork); else commit_tail(state); diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index d07c851d255b..8d0ee19953df 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -373,8 +373,18 @@ struct drm_atomic_state { * * Work item which can be used by the driver or helpers to execute the * commit without blocking. +* +* This is deprecated, use commit_kwork. */ struct work_struct commit_work; + + /** +* @commit_kwork: +* +* Work item which can be used by the driver or helpers to execute the +* commit without blocking. +*/ + struct kthread_work commit_kwork; }; void __drm_crtc_commit_free(struct kref *kref); @@ -954,6 +964,27 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (new_obj_state) = (__state)->private_objs[__i].new_state, 1); \ (__i)++) +/** + * drm_atomic_pick_worker - helper to get kworker to use for nonblocking commit + * @state: the &drm_atomic_state for the commit + * + * Pick an appropriate worker for a given atomic update. The first CRTC + * invovled in the atomic update is used to pick the worker, to prevent + * serializing multiple pageflips / atomic-updates on indenpendent CRTCs. + */ +static inline struct kthread_worker * +drm_atomic_pick_worker(const struct drm_atomic_state *state) +{ + struct drm_crtc_state *crtc_state; + struct drm_crtc *crtc; + unsigned i; + + for_each_new_crtc_in_state(state, crtc, crtc_state, i) + return crtc->worker; + + return NULL; +} + /** * drm_atomic_crtc_needs_modeset - compute combined modeset need * @state: &drm_crtc_state for the CRTC -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 1/3] drm/crtc: Introduce per-crtc kworker
From: Rob Clark This will be used for non-block atomic commits. Signed-off-by: Rob Clark --- drivers/gpu/drm/drm_crtc.c | 11 +++ include/drm/drm_crtc.h | 8 2 files changed, 19 insertions(+) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index aecdd7ea26dc..4f7c0bfce0a3 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -326,6 +326,14 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, config->prop_out_fence_ptr, 0); drm_object_attach_property(&crtc->base, config->prop_vrr_enabled, 0); + + crtc->worker = kthread_create_worker(0, "%s-worker", crtc->name); + if (IS_ERR(crtc->worker)) { + drm_mode_object_unregister(dev, &crtc->base); + ret = PTR_ERR(crtc->worker); + crtc->worker = NULL; + return ret; + } } return 0; @@ -366,6 +374,9 @@ void drm_crtc_cleanup(struct drm_crtc *crtc) kfree(crtc->name); + if (crtc->worker) + kthread_destroy_worker(crtc->worker); + memset(crtc, 0, sizeof(*crtc)); } EXPORT_SYMBOL(drm_crtc_cleanup); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 59b51a09cae6..dfdb04619b0d 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -1172,6 +1173,13 @@ struct drm_crtc { * Initialized via drm_self_refresh_helper_init(). */ struct drm_self_refresh_data *self_refresh_data; + + /** +* @worker: +* +* Per-CRTC worker for nonblock atomic commits. +*/ + struct kthread_worker *worker; }; /** -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] i915: Introduce quirk for shifting eDP brightness.
On Wed, 2020-09-30 at 16:25 -0400, Lyude Paul wrote: > On Tue, 2020-09-29 at 13:32 -0600, Kevin Chowski wrote: > > Thank you for the reply. And in regards to digging into it further, > > thanks for requesting that I do some more due diligence here :) > > > > Also if you did get around to it, thanks for double-checking with > > Bill! Let me know if you'd like me to reach out instead, or if > > anything else needs to be done in this regard. > > > > So to clarify the plan: if we do actually move forwards with leaving > > the current functionality as the default, do we need to have the > > complete list of devices which need the quirk applied when the patch > > first goes in? From my perspective, we definitely have one device > > which needs the quirk (and preferably, it'd be good to do it sooner > > than later so that we can get this bugfix out to our users) and an > > unknowable number of others. Would it be OK to introduce the quirk for > > just Pixelbook and to follow up to add others once we have that list? > > Totally-I've got no problem with this. oh-regarding Bill, yeah it might help a bit if you guys messaged them since I'm not in the vesa WG, I just have access to some specs from there > > > It may take a good amount of time for me to herd the cats inside > > Google, especially given there's a chance that there are affected > > laptops and that no one has noticed (e.g., I almost didn't notice with > > the Pixelbook). Given Lyude's analysis it seems like Chrome OS devices > > may be the largest affected group here, so I am incentivized to not > > drop the ball after fixing my immediate Pixelbook problem :) > > > > On Fri, Sep 25, 2020 at 10:53 AM Lyude Paul wrote: > > > On Thu, 2020-09-24 at 17:46 -0600, Kevin Chowski wrote: > > > > cc back a few others who were unintentionally dropped from the thread > > > > earlier. > > > > > > > > Someone (at Google) helped me dig into this a little more and they > > > > found a document titled "eDP Backlight Brightness bit alignment" sent > > > > out for review in January 2017. I registered for a new account (google > > > > is a member) and have access to the document; here is the URL for > > > > those who also have access: > > > > https://groups.vesa.org/wg/AllMem/document/7786. For what it's worth, > > > > it seems like Lyude's contact Bill Lempesis uploaded this > > > > change-request document, so I think we can reach out to him if we have > > > > further questions. It's actually unclear to me what the status of this > > > > change request is, and whether it's been officially accepted. But I > > > > think it can be seen as some official advice on how we can move > > > > forward here. > > > > > > > > Basically, this is a change request to the spec which acknowledges > > > > that, despite the original spec implying that the > > > > most-significant-bits were relevant here, many implementations used > > > > the least-significant-bits. In defense of most-significant it laid out > > > > some similar arguments to what Ville was saying. But it ends up > > > > saying: > > > > > > > > > Unfortunately, the most common interpretation that we have > > > > > encountered is case 1 in the example above. TCON vendors > > > > > tend to align the valid bits to the LSBs, not the MSBs. > > > > > > > > Instead of changing the default defined functionality (as some earlier > > > > version of this doc apparently suggested), this doc prefers to > > > > allocate two extra bits in EDP_GENERAL_CAPABILITY_2 so that future > > > > backlight devices can specify to the Source how to program them: > > > > > > > > 00: the current functionality, i,e. no defined interpretation > > > > 01: aligned to most-significant bits > > > > 10: aligned to least-significant bits > > > > 11: reserved > > > > > > > > It also says "[Sources] should only need panel-specific workarounds > > > > for the currently available panels." > > > > > > > > So I believe this document is an acknowledgement of many > > > > implementations having their alignment to the least-significant bits, > > > > and (to my eyes) clearly validates that the spec "should" be the > > > > opposite. If we believe the doc's claim that "the most common > > > > interpretation" is least-significant, it seems to me that it would > > > > require more quirks if we made most-significant the default > > > > implementation. > > > > > > > > Ville mentioned at some point earlier that we should try to match the > > > > spec, whereas Lyude mentioned we should prefer to do what the majority > > > > of machines do. What do you both think of this new development? > > > > > > That's how displayport happens to be sometimes :). Definitely isn't the > > > first > > > time something in the spec like this got implemented incorrectly so many > > > times > > > by different vendors that they had to update the spec in response (same > > > thing > > > happened with MST and interleaved sideband messages as of DP 2.0), so I'm > > > really glad we went and ac
Re: [Intel-gfx] [PATCH] i915: Introduce quirk for shifting eDP brightness.
On Tue, 2020-09-29 at 13:32 -0600, Kevin Chowski wrote: > Thank you for the reply. And in regards to digging into it further, > thanks for requesting that I do some more due diligence here :) > > Also if you did get around to it, thanks for double-checking with > Bill! Let me know if you'd like me to reach out instead, or if > anything else needs to be done in this regard. > > So to clarify the plan: if we do actually move forwards with leaving > the current functionality as the default, do we need to have the > complete list of devices which need the quirk applied when the patch > first goes in? From my perspective, we definitely have one device > which needs the quirk (and preferably, it'd be good to do it sooner > than later so that we can get this bugfix out to our users) and an > unknowable number of others. Would it be OK to introduce the quirk for > just Pixelbook and to follow up to add others once we have that list? Totally-I've got no problem with this. > It may take a good amount of time for me to herd the cats inside > Google, especially given there's a chance that there are affected > laptops and that no one has noticed (e.g., I almost didn't notice with > the Pixelbook). Given Lyude's analysis it seems like Chrome OS devices > may be the largest affected group here, so I am incentivized to not > drop the ball after fixing my immediate Pixelbook problem :) > > On Fri, Sep 25, 2020 at 10:53 AM Lyude Paul wrote: > > On Thu, 2020-09-24 at 17:46 -0600, Kevin Chowski wrote: > > > cc back a few others who were unintentionally dropped from the thread > > > earlier. > > > > > > Someone (at Google) helped me dig into this a little more and they > > > found a document titled "eDP Backlight Brightness bit alignment" sent > > > out for review in January 2017. I registered for a new account (google > > > is a member) and have access to the document; here is the URL for > > > those who also have access: > > > https://groups.vesa.org/wg/AllMem/document/7786. For what it's worth, > > > it seems like Lyude's contact Bill Lempesis uploaded this > > > change-request document, so I think we can reach out to him if we have > > > further questions. It's actually unclear to me what the status of this > > > change request is, and whether it's been officially accepted. But I > > > think it can be seen as some official advice on how we can move > > > forward here. > > > > > > Basically, this is a change request to the spec which acknowledges > > > that, despite the original spec implying that the > > > most-significant-bits were relevant here, many implementations used > > > the least-significant-bits. In defense of most-significant it laid out > > > some similar arguments to what Ville was saying. But it ends up > > > saying: > > > > > > > Unfortunately, the most common interpretation that we have > > > > encountered is case 1 in the example above. TCON vendors > > > > tend to align the valid bits to the LSBs, not the MSBs. > > > > > > Instead of changing the default defined functionality (as some earlier > > > version of this doc apparently suggested), this doc prefers to > > > allocate two extra bits in EDP_GENERAL_CAPABILITY_2 so that future > > > backlight devices can specify to the Source how to program them: > > > > > > 00: the current functionality, i,e. no defined interpretation > > > 01: aligned to most-significant bits > > > 10: aligned to least-significant bits > > > 11: reserved > > > > > > It also says "[Sources] should only need panel-specific workarounds > > > for the currently available panels." > > > > > > So I believe this document is an acknowledgement of many > > > implementations having their alignment to the least-significant bits, > > > and (to my eyes) clearly validates that the spec "should" be the > > > opposite. If we believe the doc's claim that "the most common > > > interpretation" is least-significant, it seems to me that it would > > > require more quirks if we made most-significant the default > > > implementation. > > > > > > Ville mentioned at some point earlier that we should try to match the > > > spec, whereas Lyude mentioned we should prefer to do what the majority > > > of machines do. What do you both think of this new development? > > > > That's how displayport happens to be sometimes :). Definitely isn't the > > first > > time something in the spec like this got implemented incorrectly so many > > times > > by different vendors that they had to update the spec in response (same > > thing > > happened with MST and interleaved sideband messages as of DP 2.0), so I'm > > really glad we went and actually investigated this. > > > > So yes - I think a quirk for this would definitely be a good idea, and IMO > > we > > should always lean on the side that more panels implement even if it's not > > according to spec - so defaulting to the behavior we currently have in the > > kernel, and adding quirks for panels that were smart enough not to fall for > > this would prob
[pull] amdgpu, amdkfd, radeon drm-next-5.10
Hi Dave, Daniel, Fixes for 5.10. The following changes since commit 911d5bd5e7b8531b39301c2c27e5b90d7bd71b88: drm/amd/pm: Skip smu_post_init in SRIOV (2020-09-18 16:14:56 -0400) are available in the Git repository at: git://people.freedesktop.org/~agd5f/linux tags/amd-drm-next-5.10-2020-09-30 for you to fetch changes up to f2fa07b39fafb2a5f49c71a504862c5efa57d03e: drm/amd/amdkfd: Surface files in Sysfs to allow users to get number of compute units that are in use. (2020-09-30 15:26:27 -0400) amd-drm-next-5.10-2020-09-30: amdgpu: - KFD init failure handling fixes - Display debugfs fixes - Virtual display vblank deadlock fix - Backlight fixes - eDP fixes - Misc display fixes - Misc code cleanups - Mclk handling fixes for navi1x - SR-IOV fixes - Documentation updates - RAS fixes - Power fixes for Raven and Renoir - EEPROM fixes amdkfd: - CPU CRAT fixes - Doorbell allocation cleanup - CU occupancy statistics radeon: - Spelling fixes Alex Deucher (5): drm/amdgpu/display: fix CFLAGS setup for DCN30 drm/amdgpu: store noretry parameter per driver instance drm/amdgpu: add an auto setting to the noretry parameter drm/amdgpu: fix a warning in amdgpu_ras.c (v2) drm/amdgpu/swsmu/smu12: fix force clock handling for mclk Alvin Lee (1): drm/amd/display: Update NV1x SR latency values Anthony Koo (3): drm/amd/display: [FW Promotion] Release 0.0.33 drm/amd/display: [FW Promotion] Release 0.0.34 drm/amd/display: [FW Promotion] Release 0.0.35 Aric Cyr (3): drm/amd/display: 3.2.103 drm/amd/display: 3.2.104 drm/amd/display: Revert check for flip pending before locking pipes Bernard Zhao (3): drm/radeon: fix typoes in comments drm/amd: fix typoes in comments drm/amd/display: optimize code runtime a bit Bokun Zhang (2): drm/amdgpu: Update VF2PF interface drm/amdgpu: Implement new guest side VF2PF message transaction (v2) Chiawen Huang (2): drm/amd/display: disable stream if pixel clock changed with link active drm/amd/display: disable stream if pixel clock changed with link active Chris Park (1): drm/amd/display: TMDS Fallback transition David Galiffi (1): drm/amd/display: Fix incorrect backlight register offset for DCN Dirk Gouders (1): drm/amd/display: remove duplicate call to rn_vbios_smu_get_smu_version() Dmytro Laktyushkin (2): amd/drm/display: avoid dcn3 on flip opp change for slave pipes drm/amd/display: add pipe reassignment prevention code to dcn3 Emily.Deng (2): drm/amdgpu: Fix dead lock issue for vblank drm/amdgpu: Remove some useless code Eric Bernstein (1): drm/amd/display: Add dp_set_dsc_pps_info_packet to virtual stream encoder Evan Quan (5): drm/amd/powerplay: optimize the mclk dpm policy settings drm/amd/pm: correct the pmfw version check for Navi14 drm/amd/pm: decouple the watermark table setting from socclk/uclk dpms drm/amd/pm: drop redundant watermarks bitmap setting drm/amd/pm: fix screen flicker seen on Navi14 with 2*4K monitors Felix Kuehling (1): drm/amdgpu: Fix handling of KFD initialization failures Flora Cui (1): drm/amd/display: fix return value check for hdcp_work Gary Li (1): drm/amd/display: Enable DP YCbCr420 mode support for DCN10 Guchun Chen (3): drm/amdgpu: clean up ras sysfs creation (v2) drm/amdgpu: fix incorrect comment drm/amdgpu: drop duplicated ecc check for vega10 (v5) Jason Yan (2): drm/amd/display: make get_color_space_type() static drm/amd/display: make two symbols static Jean Delvare (1): drm/amdgpu: restore proper ref count in amdgpu_display_crtc_set_config Jiansong Chen (2): drm/amdgpu: remove gpu_info fw support for sienna_cichlid etc. drm/amdgpu: disable gfxoff temporarily for navy_flounder Jingwen Chen (2): drm/amd/pm: Skip use smc fw data in SRIOV drm/amd: Skip not used microcode loading in SRIOV John Clements (1): drm/amdgpu: disable sienna chichlid UMC RAS Joshua Aberback (1): drm/amd/display: Calc DLG from dummy p-state if full p-state unsupported Kent Russell (3): drm/amdkfd: Calculate CPU VCRAT size dynamically (v2) drm/amdkfd: Use kvmalloc instead of kmalloc for VCRAT drm/amdgpu: Use SKU instead of DID for FRU check v2 Lewis Huang (1): drm/amd/display: [FIX] update clock under two conditions Likun Gao (1): drm/amd/pm: update driver if file for sienna cichlid Liu Shixin (2): drm/amd/pm: simplify the return expression of smu_hw_fini drm/amdgpu/gmc9: simplify the return expression of gmc_v9_0_suspend Mukul Joshi (1): drm/amdkfd: Move process doorbell allocation into kfd device Oak Zeng (1): drm/amdgpu: use function pointer for gfxhub functions Peikang Zhang (2): dr
[Bug 204241] amdgpu fails to resume from suspend
https://bugzilla.kernel.org/show_bug.cgi?id=204241 --- Comment #70 from Lahfa Samy (s...@lahfa.xyz) --- I've opened a new bug report as the issue is clearly related to networking and the iwlwifi driver and not to the AMDGPU driver in my case. Here is the link to the bug report : https://bugzilla.kernel.org/show_bug.cgi?id=209435 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204241] amdgpu fails to resume from suspend
https://bugzilla.kernel.org/show_bug.cgi?id=204241 --- Comment #69 from Lahfa Samy (s...@lahfa.xyz) --- I've got news of a current workaround for my T495 with a Ryzen 7 3700U and a Vega RX 10 on kernel 5.8.12arch, I have disabled the Network card (which means no more WiFi at all) in the BIOS and this has solved the problem of the resuming freeze. This is most likely due to a bug in the driver iwlwifi used by the Intel Wireless AC-9260 network card, I can also confirm that the same bug affects the package linux-lts for ArchLinux 5.4.68-1-lts. The logs show a watchdog :soft-lockup on CPU#0 stuck for 22s! [irq/87-iwlwifi::979]. Later in the log there is this line : RIP : 0010:iwl_trans_pcie_read32+0x10/0x20 [iwlwifi] A few more information probably that would help someone make a patch maybe. And finally a call trace. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: remove alloc_vm_area v2
On Wed, Sep 30, 2020 at 4:48 PM Christoph Hellwig wrote: > > On Tue, Sep 29, 2020 at 03:43:30PM +0300, Joonas Lahtinen wrote: > > Hmm, those are both committed after our last -next pull request, so they > > would normally only target next merge window. drm-next closes the merge > > window around -rc5 already. > > > > But, in this specific case those are both Fixes: patches with Cc: stable, > > so they should be pulled into drm-intel-next-fixes PR. > > > > Rodrigo, can you cherry-pick those patches to -next-fixes that you send > > to Dave? > > They still haven't made it to linux-next. I think for now I'll just > rebase without them again and then you can handle the conflicts for > 5.11. Yeah after -rc6 drm is frozen for features, so anything that's stuck in subordinate trees rolls over to the next merge cycle. To avoid upsetting sfr from linux-next we keep those -next branches out of linux-next until after -rc1 again. iow, rebasing onto linux-next and smashing this into 5.10 sounds like the right approach (since everyone else freezes a bunch later afaik). Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Wed, Sep 30, 2020 at 03:57:58PM +0300, Jani Nikula wrote: > On Wed, 30 Sep 2020, Ville Syrjälä wrote: > > Now we have an actual difference between EHL and JSL so we > > should split. Granted it's a bit annoying to have to do it > > just for some vswing tables. Ideally that stuff would be > > specified in a sane way by the VBT. But since VBT is generally > > useless we need to deal with this on a platform level. > > Just to recap, we have three basic approaches for differentiating > platforms based on PCI ID: > > - Separate platforms, each with their own device info and enum > intel_platform, using IS_() for checks. > > - Same platform, with subplatforms, using IS_SUBPLATFORM() for > checks. Generally only used for the ULT/ULX checks, but there's also > the CNL/ICL port F case which is perhaps comparable. > > - Same platform, each with their own device info, and a feature flag. > > (In this case, checking the PCH is a proxy; there is no actual > difference in the PCHs to account for the different values to be > used. Mixing PCHs with the platforms would lead to problems.) > > We've been told JSL and EHL are the same, which would argue for keeping > them INTEL_ELKHARTLAKE. We've done this with other platforms that are > identical. However, now it looks like they're not the same... why not if > they're supposed to be identical? What else is there? My understanding is that they are identical, but the design guidelines for the *motherboards* that they will plug into are different, which necessitates different electrical tuning values to guarantee clean display signals. Ville's right that it would be nice if this kind of stuff was just available from something like the VBT instead of being hardcoded into the driver, but sadly that's just not the case today. So yes, none of this is related to the South Display which is the only place we usually care about the PCH in the graphics driver. But PCH is correlated with board type, which is why I suggested matching on the PCH in the first place. If we really want to split these two platforms then I'd suggest we add a new macro like #define IS_EHL_JSL(i915) ( \ IS_PLATFORM(dev_priv, INTEL_ELKHARTLAKE) || \ IS_PLATFORM(dev_priv, INTEL_JASPERLAKE)) and use that everywhere else in the driver. For the vswing code itself we'd just do a direct IS_PLATFORM() check with just one platform or the other provided; no need to add IS_ELKHARTLAKE/IS_JASPERLAKE macros in that case since it would be a bug to differentiate between the two anywhere else in the driver. Matt > > BR, > Jani. > > > -- > Jani Nikula, Intel Open Source Graphics Center -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 80/80] ARM: dts: bcm2711: Enable the display pipeline
Hi, Am 30.09.20 um 18:38 schrieb Nathan Chancellor: > On Wed, Sep 30, 2020 at 04:07:58PM +0200, Maxime Ripard wrote: >> Hi Nathan, >> >> On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: >>> On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: Now that all the drivers have been adjusted for it, let's bring in the necessary device tree changes. The VEC and PV3 are left out for now, since it will require a more specific clock setup. Reviewed-by: Dave Stevenson Tested-by: Chanwoo Choi Tested-by: Hoegeun Kwon Tested-by: Stefan Wahren Signed-off-by: Maxime Ripard >>> Apologies if this has already been reported or have a solution but this >>> patch (and presumably series) breaks output to the serial console after >>> a certain point during init. On Raspbian, I see systemd startup messages >>> then the output just turns into complete garbage. It looks like this >>> patch is merged first in linux-next, which is why my bisect fell on the >>> DRM merge. I am happy to provide whatever information could be helpful >>> for debugging this. I am on the latest version of the firmware >>> (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). >> Unfortunately, the miniUART is in the same clock tree than the core >> clock and will thus have those kind of issues when the core clock is >> changed (which is also something that one should expect when using the >> DRM or other drivers). >> >> The only real workaround there would be to switch to one of the PL011 >> UARTs. I guess we can also somehow make the UART react to the core clock >> frequency changes, but that's going to require some effort >> >> Maxime > Ack, thank you for the reply! There does not really seem to be a whole > ton of documentation around using one of the other PL011 UARTs so for > now, I will just revert this commit locally. there was a patch series & discussion about this topic, but we finally didn't find a rock solid solution. You can have a look at "[RFC 5/5] serial: 8250: bcm2835aux: add notifier to follow clock changes" from 3.4.2019 on linux-rpi-kernel. Best regards > > Cheers, > Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH rdma-next v4 4/4] RDMA/umem: Move to allocate SG table from pages
On Wed, Sep 30, 2020 at 12:14:06PM -0300, Jason Gunthorpe wrote: > On Wed, Sep 30, 2020 at 06:05:15PM +0300, Maor Gottlieb wrote: > > This is right only for the last iteration. E.g. in the first iteration in > > case that there are more pages (left_pages), then we allocate > > SG_MAX_SINGLE_ALLOC. We don't know how many pages from the second iteration > > will be squashed to the SGE from the first iteration. > > Well, it is 0 or 1 SGE's. Check if the first page is mergable and > subtract one from the required length? > > I dislike this sg_mark_end() it is something that should be internal, > IMHO. I don't think so, but Maor provided possible solution. Can you take the patches? Thanks > > Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204241] amdgpu fails to resume from suspend
https://bugzilla.kernel.org/show_bug.cgi?id=204241 --- Comment #68 from Robert M. Muncrief (rmuncr...@humanavance.com) --- Created attachment 292729 --> https://bugzilla.kernel.org/attachment.cgi?id=292729&action=edit Resume fail with RX 580 GPU I've been having random resume problems form around kernel 5.5, and it persists even up to 5.9-rc6. When this occurs I can still login to SSH and give a reboot command, but though SSH disconnects my computer doesn't reboot and I have to press the reset button. I have an ASUS Gaming TUF X570 motherboard, R7 3700X CPU, RX 580 GPU, and 16GB of RAM. The primary error recorded in dmesg is: [x.xx] amdgpu: last message was failed ret is 65535 I've included the part of dmesg beginning with suspend event through the resume failure. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm/dp: add voltage corners voting support base on dp link rate
On 9/30/2020 1:54 PM, Stephen Boyd wrote: Quoting Kuogee Hsieh (2020-09-29 10:10:26) Set link rate by using OPP set rate api so that CX level will be set accordingly base on the link rate. s/base/based/ Signed-off-by: Kuogee Hsieh --- diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 2e3e1917351f..e1595d829e04 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1849,6 +1853,21 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, return ERR_PTR(-ENOMEM); } + ctrl->opp_table = dev_pm_opp_set_clkname(dev, "ctrl_link"); I see that downstream has multiple DP clocks which end up voting on CX, we don't have a way of associating multiple OPP tables with a device upstream, so whats usually done is (assuming all the clocks get scaled in lock step, which I assume is the case here) we pick the clock with the 'highest' CX requirement and associate that with the OPP table. I haven't looked but I am hoping thats how we have decided to associate "ctrl_link" clock here? + + if (IS_ERR(ctrl->opp_table)) { + dev_err(dev, "invalid DP OPP table in device tree\n"); + ctrl->opp_table = NULL; + } else { + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (ret && ret != -ENODEV) { + dev_err(dev, "add DP OPP table\n"); This is debug noise right? + dev_pm_opp_put_clkname(ctrl->opp_table); + ctrl->opp_table = NULL; + } + } + init_completion(&ctrl->idle_comp); init_completion(&ctrl->video_comp); @@ -1864,6 +1883,18 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, return &ctrl->dp_ctrl; } -void dp_ctrl_put(struct dp_ctrl *dp_ctrl) +void dp_ctrl_put(struct device *dev, struct dp_ctrl *dp_ctrl) { + struct dp_ctrl_private *ctrl; + + if (!dp_ctrl) Can this happen? + return; + + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); + + if (ctrl->opp_table != NULL) { This is usually written as if (ctrl->opp_table) + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(ctrl->opp_table); + ctrl->opp_table = NULL; + } } diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h index f60ba93c8678..19b412a93e02 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h @@ -31,6 +31,6 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, struct dp_panel *panel, struct drm_dp_aux *aux, struct dp_power *power, struct dp_catalog *catalog, struct dp_parser *parser); -void dp_ctrl_put(struct dp_ctrl *dp_ctrl); +void dp_ctrl_put(struct device *dev, struct dp_ctrl *dp_ctrl); Is 'dev' not inside 'dp_ctrl'? #endif /* _DP_CTRL_H_ */ diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c index 17c1fc6a2d44..3d75bf09e38f 100644 --- a/drivers/gpu/drm/msm/dp/dp_power.c +++ b/drivers/gpu/drm/msm/dp/dp_power.c @@ -8,12 +8,14 @@ #include #include #include +#include #include "dp_power.h" #include "msm_drv.h" struct dp_power_private { struct dp_parser *parser; struct platform_device *pdev; + struct device *dev; struct clk *link_clk_src; struct clk *pixel_provider; struct clk *link_provider; @@ -148,18 +150,49 @@ static int dp_power_clk_deinit(struct dp_power_private *power) return 0; } +static int dp_power_clk_set_link_rate(struct dp_power_private *power, + struct dss_clk *clk_arry, int num_clk, int enable) +{ + u32 rate; + int i, rc = 0; + + for (i = 0; i < num_clk; i++) { + if (clk_arry[i].clk) { + if (clk_arry[i].type == DSS_CLK_PCLK) { + if (enable) + rate = clk_arry[i].rate; + else + rate = 0; + + rc = dev_pm_opp_set_rate(power->dev, rate); Why do we keep going if rc is non-zero? + } + + } + } + return rc; +} + static int dp_power_clk_set_rate(struct dp_power_private *power, enum dp_pm_type module, bool enable) { int rc = 0; struct dss_module_power *mp = &power->parser->mp[module]; - if (enable) { - rc = msm_dss_clk_set_rate(mp->clk_config, mp->num_clk); + if (module == DP_CTRL_PM) { + rc = dp_power_clk_set_link_rate(power, mp->clk_config, mp->num_clk, enable); if (rc) { -
[pull] amdgpu drm-fixes-5.9
Hi Dave, Daniel, A bit bigger than usual since I missed last week. Mostly updates for new asics and a few of misc bug fixes. The following changes since commit 1f08fde70075784d28d1687d0e75871e81cc1173: Merge tag 'mediatek-drm-fixes-5.9' of https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux into drm-fixes (2020-09-18 08:52:06 +1000) are available in the Git repository at: git://people.freedesktop.org/~agd5f/linux tags/amd-drm-fixes-5.9-2020-09-30 for you to fetch changes up to 95433a1305a000aa91f558b062ce614a3bb8ceb5: drm/amdgpu: disable gfxoff temporarily for navy_flounder (2020-09-30 09:47:43 -0400) amd-drm-fixes-5.9-2020-09-30: amdgpu: - Fix potential double free in userptr handling - Sienna Cichlid and Navy Flounder udpates - Add Sienna Cichlid PCI IDs - Drop experimental flag for navi12 - Raven fixes - Renoir fixes - HDCP fix - DCN3 fix for clang and older versions of gcc - Fix a runtime pm refcount issue Alex Deucher (6): drm/amdgpu: add the GC 10.3 VRS registers drm/amdgpu: add VCN 3.0 AV1 registers drm/amdgpu: use the AV1 defines for VCN 3.0 drm/amdgpu: remove experimental flag from navi12 drm/amdgpu/display: fix CFLAGS setup for DCN30 drm/amdgpu/swsmu/smu12: fix force clock handling for mclk Dirk Gouders (1): drm/amd/display: remove duplicate call to rn_vbios_smu_get_smu_version() Evan Quan (1): drm/amd/pm: setup APU dpm clock table in SMU HW initialization Flora Cui (1): drm/amd/display: fix return value check for hdcp_work Jean Delvare (1): drm/amdgpu: restore proper ref count in amdgpu_display_crtc_set_config Jiansong Chen (2): drm/amdgpu: remove gpu_info fw support for sienna_cichlid etc. drm/amdgpu: disable gfxoff temporarily for navy_flounder Likun Gao (1): drm/amdgpu: add device ID for sienna_cichlid (v2) Philip Yang (1): drm/amdgpu: prevent double kfree ttm->sg Sudheesh Mavila (1): drm/amd/pm: Removed fixed clock in auto mode DPM drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 + drivers/gpu/drm/amd/amdgpu/amdgpu_display.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c| 12 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 1 + drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 3 ++ drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 16 +++ .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c | 2 +- .../drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c | 1 - drivers/gpu/drm/amd/display/dc/dcn30/Makefile | 18 +++- .../amd/include/asic_reg/gc/gc_10_3_0_default.h| 2 + .../drm/amd/include/asic_reg/gc/gc_10_3_0_offset.h | 4 ++ .../amd/include/asic_reg/gc/gc_10_3_0_sh_mask.h| 50 ++ .../amd/include/asic_reg/vcn/vcn_3_0_0_sh_mask.h | 34 +++ drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 22 +- drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 10 +++-- drivers/gpu/drm/amd/powerplay/renoir_ppt.c | 8 ++-- 16 files changed, 154 insertions(+), 41 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 20/52] drm: drm_dsc.h: fix a kernel-doc markup
On Wed, Sep 30, 2020 at 03:24:43PM +0200, Mauro Carvalho Chehab wrote: > As warned by Sphinx: > > ./Documentation/gpu/drm-kms-helpers:305: ./include/drm/drm_dsc.h:587: > WARNING: Unparseable C cross-reference: 'struct' > Invalid C declaration: Expected identifier in nested name, got keyword: > struct [error at 6] > struct > --^ > > The markup for one struct is wrong, as struct is used twice. > > Signed-off-by: Mauro Carvalho Chehab Applied to drm-misc-fixes, thanks for your patch. -Daniel > --- > include/drm/drm_dsc.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h > index 887954cbfc60..732f32740c86 100644 > --- a/include/drm/drm_dsc.h > +++ b/include/drm/drm_dsc.h > @@ -588,7 +588,7 @@ struct drm_dsc_picture_parameter_set { > * This structure represents the DSC PPS infoframe required to send the > Picture > * Parameter Set metadata required before enabling VESA Display Stream > * Compression. This is based on the DP Secondary Data Packet structure and > - * comprises of SDP Header as defined &struct struct dp_sdp_header in > drm_dp_helper.h > + * comprises of SDP Header as defined &struct dp_sdp_header in > drm_dp_helper.h > * and PPS payload defined in &struct drm_dsc_picture_parameter_set. > * > * @pps_header: Header for PPS as per DP SDP header format of type > -- > 2.26.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] Partially revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 12:53:44PM -0700, Peter Collingbourne wrote: > Also partially revert the follow-up change "drm: pl111: Absorb the > external register header". > > This reverts the parts of commits > 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 and > 0fb8125635e8eb5483fb095f98dcf0651206a7b8 that touch paths outside > of drivers/gpu/drm/pl111. > > The fbdev driver is used by Android's FVP configuration. Using the > DRM driver together with DRM's fbdev emulation results in a failure > to boot Android. The root cause is that Android's generic fbdev > userspace driver relies on the ability to set the pixel format via > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > There have been other less critical behavioral differences identified > between the fbdev driver and the DRM driver with fbdev emulation. The > DRM driver exposes different values for the panel's width, height and > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > with yres_virtual greater than the maximum supported value instead > of letting the syscall succeed and setting yres_virtual based on yres. > > Signed-off-by: Peter Collingbourne Applied to drm-misc-fixes, should make it into Linus tree this week. -Daniel > --- > View this change in Gerrit: > https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > MAINTAINERS | 5 + > drivers/video/fbdev/Kconfig | 20 + > drivers/video/fbdev/Makefile| 1 + > drivers/video/fbdev/amba-clcd.c | 986 > include/linux/amba/clcd-regs.h | 87 +++ > include/linux/amba/clcd.h | 290 ++ > 6 files changed, 1389 insertions(+) > create mode 100644 drivers/video/fbdev/amba-clcd.c > create mode 100644 include/linux/amba/clcd-regs.h > create mode 100644 include/linux/amba/clcd.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 190c7fa2ea01..671c1fa79e64 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1460,6 +1460,11 @@ S: Odd Fixes > F: drivers/amba/ > F: include/linux/amba/bus.h > > +ARM PRIMECELL CLCD PL110 DRIVER > +M: Russell King > +S: Odd Fixes > +F: drivers/video/fbdev/amba-clcd.* > + > ARM PRIMECELL KMI PL050 DRIVER > M: Russell King > S: Odd Fixes > diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig > index b2c9dd4f0cb5..402e85450bb5 100644 > --- a/drivers/video/fbdev/Kconfig > +++ b/drivers/video/fbdev/Kconfig > @@ -272,6 +272,26 @@ config FB_PM2_FIFO_DISCONNECT > help > Support the Permedia2 FIFO disconnect feature. > > +config FB_ARMCLCD > + tristate "ARM PrimeCell PL110 support" > + depends on ARM || ARM64 || COMPILE_TEST > + depends on FB && ARM_AMBA && HAS_IOMEM > + select FB_CFB_FILLRECT > + select FB_CFB_COPYAREA > + select FB_CFB_IMAGEBLIT > + select FB_MODE_HELPERS if OF > + select VIDEOMODE_HELPERS if OF > + select BACKLIGHT_CLASS_DEVICE if OF > + help > + This framebuffer device driver is for the ARM PrimeCell PL110 > + Colour LCD controller. ARM PrimeCells provide the building > + blocks for System on a Chip devices. > + > + If you want to compile this as a module (=code which can be > + inserted into and removed from the running kernel), say M > + here and read . The module > + will be called amba-clcd. > + > config FB_ACORN > bool "Acorn VIDC support" > depends on (FB = y) && ARM && ARCH_ACORN > diff --git a/drivers/video/fbdev/Makefile b/drivers/video/fbdev/Makefile > index cad4fb64442a..a0705b99e643 100644 > --- a/drivers/video/fbdev/Makefile > +++ b/drivers/video/fbdev/Makefile > @@ -75,6 +75,7 @@ obj-$(CONFIG_FB_HIT) += hitfb.o > obj-$(CONFIG_FB_ATMEL) += atmel_lcdfb.o > obj-$(CONFIG_FB_PVR2) += pvr2fb.o > obj-$(CONFIG_FB_VOODOO1) += sstfb.o > +obj-$(CONFIG_FB_ARMCLCD) += amba-clcd.o > obj-$(CONFIG_FB_GOLDFISH) += goldfishfb.o > obj-$(CONFIG_FB_68328)+= 68328fb.o > obj-$(CONFIG_FB_GBE) += gbefb.o > diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c > new file mode 100644 > index ..b7682de412d8 > --- /dev/null > +++ b/drivers/video/fbdev/amba-clcd.c > @@ -0,0 +1,986 @@ > +/* > + * linux/drivers/video/amba-clcd.c > + * > + * Copyright (C) 2001 ARM Limited, by David A Rusling > + * Updated to 2.5, Deep Blue Solutions Ltd. > + * > + * This file is subject to the terms and conditions of the GNU General Public > + * License. See the file COPYING in the main directory of this archive > + * for more details. > + * > + * ARM PrimeCell PL110 Color LCD Controller > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define to_clcd(inf
Re: [PATCH v2] Partially revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 11:16 PM Peter Collingbourne wrote: > On Tue, Sep 29, 2020 at 1:33 PM Linus Walleij > wrote: > > Can you also share the kernel config used for this build so it is > > easy to rebuild a similar kernel? > > There are instructions here for how to build Android targeting FVP: > https://cs.android.com/android/platform/superproject/+/master:device/generic/goldfish/fvpbase/README.md > > It also includes instructions for building the kernel (which is the > Android common kernel so it does have some patches on top, but it does > closely track mainline) so you should be able to make your changes on > top of the common kernel, rebuild and test them that way. OK I'll try it! It seems to assume the user is already familiar with the Android build process in some sense, and I haven't built Android since 2012 or so, so I need some struggling to get up to speed. Android being as it is I suppose it as usual also require quite a lot of harddrive space to build, so I need to get hold of a computer with enough space on it. > Because of how Android boot images work I don't think it would be easy > to provide binaries where you can replace the kernel image. Let me > know if you have any trouble following the instructions. > > The configuration is basically a combination of these two configs: > https://android.googlesource.com/kernel/common/+/refs/heads/android-mainline/arch/arm64/configs/gki_defconfig > https://android-review.googlesource.com/c/kernel/common/+/1145352/11/fvp.fragment > > Those configs enable the fbdev driver. You can apply the patch I > posted earlier to fvp.fragment to switch to the DRM driver. Excellent thanks. Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3 v3] backlight: Add Kinetic KTD253 backlight driver
On Fri, Aug 28, 2020 at 12:47 PM Lee Jones wrote: > > create mode 100644 drivers/video/backlight/ktd253-backlight.c > > Applied, thanks. Not to unnecessarily nag but I can't see this in linux-next and since we are at -rc7 it makes me a bit nervous, is it still gonna be in v5.10? Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 44/52] docs: gpu: i915.rst: Fix several C duplication warnings
As reported by Sphinx: ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:1147: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_oa_wait_unlocked'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:1169: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_oa_poll_wait'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:1189: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_oa_read'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:2669: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_oa_stream_enable'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:2734: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_oa_stream_disable'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:2820: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_oa_stream_init'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3010: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_read'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3098: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_poll_locked'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3129: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_poll'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3152: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_enable_locked'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3181: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_disable_locked'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3273: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_ioctl'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3296: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_destroy_locked'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3321: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_release'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3379: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_open_ioctl_locked'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3534: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'read_properties_unlocked'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3717: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_open_ioctl'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3760: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_register'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:3789: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_unregister'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:4009: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_add_config_ioctl'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:4162: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_remove_config_ioctl'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:4260: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_init'. ./Documentation/gpu/i915:646: ./drivers/gpu/drm/i915/i915_perf.c:4423: WARNING: Duplicate C declaration, also defined in 'gpu/i915'. Declaration is 'i915_perf_fini'. With Sphinx 3, C declarations can't be duplicated anymore, so let's exclude those from the other internals found on i915_perf.c file. Signed-off-by: Mauro Carvalho Chehab --- Documentation/gpu/i915.rst | 29 + 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst index 33cc6ddf8f64..cff1f154b473 100644 --- a/Documentation/gpu/i915.rst +++ b/Documentation/gpu/i915.rst @@ -636,15 +636,36 @@ i915 Perf Observation Architecture Stream .. kernel-doc:: drivers/gpu/drm/i915/i915_perf.c :functions: i915_oa_poll_wait -All i915 Perf
[PATCH v4 20/52] drm: drm_dsc.h: fix a kernel-doc markup
As warned by Sphinx: ./Documentation/gpu/drm-kms-helpers:305: ./include/drm/drm_dsc.h:587: WARNING: Unparseable C cross-reference: 'struct' Invalid C declaration: Expected identifier in nested name, got keyword: struct [error at 6] struct --^ The markup for one struct is wrong, as struct is used twice. Signed-off-by: Mauro Carvalho Chehab --- include/drm/drm_dsc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_dsc.h b/include/drm/drm_dsc.h index 887954cbfc60..732f32740c86 100644 --- a/include/drm/drm_dsc.h +++ b/include/drm/drm_dsc.h @@ -588,7 +588,7 @@ struct drm_dsc_picture_parameter_set { * This structure represents the DSC PPS infoframe required to send the Picture * Parameter Set metadata required before enabling VESA Display Stream * Compression. This is based on the DP Secondary Data Packet structure and - * comprises of SDP Header as defined &struct struct dp_sdp_header in drm_dp_helper.h + * comprises of SDP Header as defined &struct dp_sdp_header in drm_dp_helper.h * and PPS payload defined in &struct drm_dsc_picture_parameter_set. * * @pps_header: Header for PPS as per DP SDP header format of type -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Wed, 30 Sep 2020, Ville Syrjälä wrote: > Now we have an actual difference between EHL and JSL so we > should split. Granted it's a bit annoying to have to do it > just for some vswing tables. Ideally that stuff would be > specified in a sane way by the VBT. But since VBT is generally > useless we need to deal with this on a platform level. Just to recap, we have three basic approaches for differentiating platforms based on PCI ID: - Separate platforms, each with their own device info and enum intel_platform, using IS_() for checks. - Same platform, with subplatforms, using IS_SUBPLATFORM() for checks. Generally only used for the ULT/ULX checks, but there's also the CNL/ICL port F case which is perhaps comparable. - Same platform, each with their own device info, and a feature flag. (In this case, checking the PCH is a proxy; there is no actual difference in the PCHs to account for the different values to be used. Mixing PCHs with the platforms would lead to problems.) We've been told JSL and EHL are the same, which would argue for keeping them INTEL_ELKHARTLAKE. We've done this with other platforms that are identical. However, now it looks like they're not the same... why not if they're supposed to be identical? What else is there? BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
On Wed, Sep 30, 2020 at 2:34 PM Christian König wrote: > > Am 30.09.20 um 11:47 schrieb Daniel Vetter: > > On Wed, Sep 30, 2020 at 10:34:31AM +0200, Christian König wrote: > >> Am 30.09.20 um 10:19 schrieb Thomas Zimmermann: > >>> Hi > >>> > >>> Am 30.09.20 um 10:05 schrieb Christian König: > Am 29.09.20 um 19:49 schrieb Thomas Zimmermann: > > Hi Christian > > > > Am 29.09.20 um 17:35 schrieb Christian König: > >> Am 29.09.20 um 17:14 schrieb Thomas Zimmermann: > >>> The new helper ttm_kmap_obj_to_dma_buf() extracts address and location > >>> from and instance of TTM's kmap_obj and initializes struct dma_buf_map > >>> with these values. Helpful for TTM-based drivers. > >> We could completely drop that if we use the same structure inside TTM > >> as > >> well. > >> > >> Additional to that which driver is going to use this? > > As Daniel mentioned, it's in patch 3. The TTM-based drivers will > > retrieve the pointer via this function. > > > > I do want to see all that being more tightly integrated into TTM, but > > not in this series. This one is about fixing the bochs-on-sparc64 > > problem for good. Patch 7 adds an update to TTM to the DRM TODO list. > I should have asked which driver you try to fix here :) > > In this case just keep the function inside bochs and only fix it there. > > All other drivers can be fixed when we generally pump this through TTM. > >>> Did you take a look at patch 3? This function will be used by VRAM > >>> helpers, nouveau, radeon, amdgpu and qxl. If we don't put it here, we > >>> have to duplicate the functionality in each if these drivers. Bochs > >>> itself uses VRAM helpers and doesn't touch the function directly. > >> Ah, ok can we have that then only in the VRAM helpers? > >> > >> Alternative you could go ahead and use dma_buf_map in ttm_bo_kmap_obj > >> directly and drop the hack with the TTM_BO_MAP_IOMEM_MASK. > >> > >> What I want to avoid is to have another conversion function in TTM because > >> what happens here is that we already convert from ttm_bus_placement to > >> ttm_bo_kmap_obj and then to dma_buf_map. > > Hm I'm not really seeing how that helps with a gradual conversion of > > everything over to dma_buf_map and assorted helpers for access? There's > > too many places in ttm drivers where is_iomem and related stuff is used to > > be able to convert it all in one go. An intermediate state with a bunch of > > conversions seems fairly unavoidable to me. > > Fair enough. I would just have started bottom up and not top down. > > Anyway feel free to go ahead with this approach as long as we can remove > the new function again when we clean that stuff up for good. Yeah I guess bottom up would make more sense as a refactoring. But the main motivation to land this here is to fix the __mmio vs normal memory confusion in the fbdev emulation helpers for sparc (and anything else that needs this). Hence the top down approach for rolling this out. -Daniel > > Christian. > > > -Daniel > > > >> Thanks, > >> Christian. > >> > >>> Best regards > >>> Thomas > >>> > Regards, > Christian. > > > Best regards > > Thomas > > > >> Regards, > >> Christian. > >> > >>> Signed-off-by: Thomas Zimmermann > >>> --- > >>> include/drm/ttm/ttm_bo_api.h | 24 > >>> include/linux/dma-buf-map.h | 20 > >>> 2 files changed, 44 insertions(+) > >>> > >>> diff --git a/include/drm/ttm/ttm_bo_api.h > >>> b/include/drm/ttm/ttm_bo_api.h > >>> index c96a25d571c8..62d89f05a801 100644 > >>> --- a/include/drm/ttm/ttm_bo_api.h > >>> +++ b/include/drm/ttm/ttm_bo_api.h > >>> @@ -34,6 +34,7 @@ > >>> #include > >>> #include > >>> #include > >>> +#include > >>> #include > >>> #include > >>> #include > >>> @@ -486,6 +487,29 @@ static inline void *ttm_kmap_obj_virtual(struct > >>> ttm_bo_kmap_obj *map, > >>> return map->virtual; > >>> } > >>> +/** > >>> + * ttm_kmap_obj_to_dma_buf_map > >>> + * > >>> + * @kmap: A struct ttm_bo_kmap_obj returned from ttm_bo_kmap. > >>> + * @map: Returns the mapping as struct dma_buf_map > >>> + * > >>> + * Converts struct ttm_bo_kmap_obj to struct dma_buf_map. If the > >>> memory > >>> + * is not mapped, the returned mapping is initialized to NULL. > >>> + */ > >>> +static inline void ttm_kmap_obj_to_dma_buf_map(struct ttm_bo_kmap_obj > >>> *kmap, > >>> + struct dma_buf_map *map) > >>> +{ > >>> +bool is_iomem; > >>> +void *vaddr = ttm_kmap_obj_virtual(kmap, &is_iomem); > >>> + > >>> +if (!vaddr) > >>> +dma_buf_map_clear(map); > >>> +else if (is_iomem) > >>> +dma_buf_map_set_vaddr_iomem(map, (void __force __io
Re: [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
Am 30.09.20 um 11:47 schrieb Daniel Vetter: On Wed, Sep 30, 2020 at 10:34:31AM +0200, Christian König wrote: Am 30.09.20 um 10:19 schrieb Thomas Zimmermann: Hi Am 30.09.20 um 10:05 schrieb Christian König: Am 29.09.20 um 19:49 schrieb Thomas Zimmermann: Hi Christian Am 29.09.20 um 17:35 schrieb Christian König: Am 29.09.20 um 17:14 schrieb Thomas Zimmermann: The new helper ttm_kmap_obj_to_dma_buf() extracts address and location from and instance of TTM's kmap_obj and initializes struct dma_buf_map with these values. Helpful for TTM-based drivers. We could completely drop that if we use the same structure inside TTM as well. Additional to that which driver is going to use this? As Daniel mentioned, it's in patch 3. The TTM-based drivers will retrieve the pointer via this function. I do want to see all that being more tightly integrated into TTM, but not in this series. This one is about fixing the bochs-on-sparc64 problem for good. Patch 7 adds an update to TTM to the DRM TODO list. I should have asked which driver you try to fix here :) In this case just keep the function inside bochs and only fix it there. All other drivers can be fixed when we generally pump this through TTM. Did you take a look at patch 3? This function will be used by VRAM helpers, nouveau, radeon, amdgpu and qxl. If we don't put it here, we have to duplicate the functionality in each if these drivers. Bochs itself uses VRAM helpers and doesn't touch the function directly. Ah, ok can we have that then only in the VRAM helpers? Alternative you could go ahead and use dma_buf_map in ttm_bo_kmap_obj directly and drop the hack with the TTM_BO_MAP_IOMEM_MASK. What I want to avoid is to have another conversion function in TTM because what happens here is that we already convert from ttm_bus_placement to ttm_bo_kmap_obj and then to dma_buf_map. Hm I'm not really seeing how that helps with a gradual conversion of everything over to dma_buf_map and assorted helpers for access? There's too many places in ttm drivers where is_iomem and related stuff is used to be able to convert it all in one go. An intermediate state with a bunch of conversions seems fairly unavoidable to me. Fair enough. I would just have started bottom up and not top down. Anyway feel free to go ahead with this approach as long as we can remove the new function again when we clean that stuff up for good. Christian. -Daniel Thanks, Christian. Best regards Thomas Regards, Christian. Best regards Thomas Regards, Christian. Signed-off-by: Thomas Zimmermann --- include/drm/ttm/ttm_bo_api.h | 24 include/linux/dma-buf-map.h | 20 2 files changed, 44 insertions(+) diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index c96a25d571c8..62d89f05a801 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -486,6 +487,29 @@ static inline void *ttm_kmap_obj_virtual(struct ttm_bo_kmap_obj *map, return map->virtual; } +/** + * ttm_kmap_obj_to_dma_buf_map + * + * @kmap: A struct ttm_bo_kmap_obj returned from ttm_bo_kmap. + * @map: Returns the mapping as struct dma_buf_map + * + * Converts struct ttm_bo_kmap_obj to struct dma_buf_map. If the memory + * is not mapped, the returned mapping is initialized to NULL. + */ +static inline void ttm_kmap_obj_to_dma_buf_map(struct ttm_bo_kmap_obj *kmap, + struct dma_buf_map *map) +{ + bool is_iomem; + void *vaddr = ttm_kmap_obj_virtual(kmap, &is_iomem); + + if (!vaddr) + dma_buf_map_clear(map); + else if (is_iomem) + dma_buf_map_set_vaddr_iomem(map, (void __force __iomem *)vaddr); + else + dma_buf_map_set_vaddr(map, vaddr); +} + /** * ttm_bo_kmap * diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h index fd1aba545fdf..2e8bbecb5091 100644 --- a/include/linux/dma-buf-map.h +++ b/include/linux/dma-buf-map.h @@ -45,6 +45,12 @@ * * dma_buf_map_set_vaddr(&map. 0xdeadbeaf); * + * To set an address in I/O memory, use dma_buf_map_set_vaddr_iomem(). + * + * .. code-block:: c + * + * dma_buf_map_set_vaddr_iomem(&map. 0xdeadbeaf); + * * Test if a mapping is valid with either dma_buf_map_is_set() or * dma_buf_map_is_null(). * @@ -118,6 +124,20 @@ static inline void dma_buf_map_set_vaddr(struct dma_buf_map *map, void *vaddr) map->is_iomem = false; } +/** + * dma_buf_map_set_vaddr_iomem - Sets a dma-buf mapping structure to an address in I/O memory + * @map: The dma-buf mapping structure + * @vaddr_iomem: An I/O-memory address + * + * Sets the address and the I/O-memory flag. + */ +static inline void dma_buf_map_set_vaddr_iomem(struct dma_buf_map *map, + void __iomem *vaddr_iomem) +{ + map->vaddr_iomem = vaddr_iomem
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, 29 Sep 2020, "Souza, Jose" wrote: > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote: >> If the thing has nothing to do PCH then it should not use the PCH type >> for the the check. Instead we should just do the EHL/JSL split. > > In the first version Matt Roper suggested to use PCH to differentiate > between EHL and JSL, Jani also agreed with this solution.This 2 PCHs > can only be associate with EHL and JSL respectively, so no downsides > here. FWIW I said, "If the difference is in the PCH", without pondering further. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 9:52 AM Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > > > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > > > Hi, > > > > > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > > > > register header". > > > > > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > > > > DRM driver together with DRM's fbdev emulation results in a failure > > > > > > to boot Android. The root cause is that Android's generic fbdev > > > > > > userspace driver relies on the ability to set the pixel format via > > > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > > > > > Not without kernel changes. See e.g. > > > https://www.spinics.net/lists/dri-devel/msg255883.html > > > > That discussion seems to have died down with no further action. > > > > I also was kinda under the assumption that with Android these buffers > > would be allocated directly from dma-buf heaps/ion, so this all seems not > > that well baked out. The disagreement about whether these allocations should be made via the render node or via dma-buf/ion is one reason why it was hard to make progress on this, unfortunately. > > > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > > > emulation, then let's do that. Not keep fbdev drivers on life support > > > > for > > > > longer than necessary. > > > > > > That should have been done *before* removing the old driver, which was > > > a userspace break that was introduced in 5.9rc1. We shouldn't leave > > > userspace broken for the period of time that it would take to develop > > > that change. Even if such a change were developed before 5.9 is > > > released, it probably wouldn't be considered a bug fix that would be > > > eligible for 5.9. > > > > > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > > > > There have been other less critical behavioral differences > > > > > > identified > > > > > > between the fbdev driver and the DRM driver with fbdev emulation. > > > > > > The > > > > > > DRM driver exposes different values for the panel's width, height > > > > > > and > > > > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > > > > > > with yres_virtual greater than the maximum supported value instead > > > > > > of letting the syscall succeed and setting yres_virtual based on > > > > > > yres. > > > > > > > > Also something that should be fixed I think in upstream, in the drm > > > > fbdev > > > > emulation. At least doesn't sound like it's something unfixable. > > > > > > Again, it should have been fixed before removing the old driver. > > > > Yeah, but we also don't have a whole lot people who seem to push for this. > > So the only way to find the people who still care is to remove the fbdev > > drivers. > > > > fbdev is dead. I'm totally fine reverting the driver to shut up the > > regression report, but it's not fixing anything long term. There's not > > going to be new drivers, or new hw support in existing drivers in fbdev. > > > > Also note that there's not spec nor test suite for fbdev, so "you guys > > should have known before removing drivers" doesn't work. > > btw revert itself is stuck somewhere, so I can't find it. And for the You should be able to download the commit like this (the commit message will need amending to remove most of the trailers at the bottom, though): git fetch https://linux.googlesource.com/linux/kernel/git/torvalds/linux refs/changes/52/2952/1 > revert can you pls just resurrect the files in drivers/video, without > touching anything in drivers/gpu? Tying down drm drivers with stuff in > fbdev doesn't make much sense to me. The touching of code in drm was a consequence of needing to revert a follow-up commit which moved code previously shared between fbdev and drm into the drm driver. Or do you think we should "manually" revert the first commit and as a consequence end up with the two copies of the code? Peter > > Thanks, Daniel > > > -Daniel > > > > > > > > Peter > > > > > > > -Daniel > > > > > > > > > > > > > > > > Signed-off-by: Peter Collingbourne > > > > > > --- > > > > > > View this change in Gerrit: > > > > > > https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > > > > > > > > > > > MAINTAINERS | 5 + > > > > > > drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + > > > > > > drivers/gpu/drm/pl111/pl111_display.c | 1 + > > > > > > drivers/gpu/drm/pl111/pl111_dr
Re: [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers
On Wed, Sep 30, 2020 at 01:25:14PM +0200, Daniel Vetter wrote: > On Wed, Sep 30, 2020 at 12:56 PM Peilin Ye wrote: > > > > On Wed, Sep 30, 2020 at 11:53:17AM +0200, Daniel Vetter wrote: > > > On Wed, Sep 30, 2020 at 03:11:51AM -0400, Peilin Ye wrote: > > > > On Tue, Sep 29, 2020 at 04:38:49PM +0200, Daniel Vetter wrote: > > > > > On Tue, Sep 29, 2020 at 2:34 PM Peilin Ye > > > > > wrote: > > > > > > Ah, and speaking of built-in fonts, see fbcon_startup(): > > > > > > > > > > > > /* Setup default font */ > > > > > > [...] > > > > > > vc->vc_font.charcount = 256; /* FIXME Need to > > > > > > support more fonts */ > > > > > > ^^^ > > > > > > > > > > > > This is because find_font() and get_default_font() return a `struct > > > > > > font_desc *`, but `struct font_desc` doesn't contain `charcount`. I > > > > > > think we also need to add a `charcount` field to `struct font_desc`. > > > > > > > > > > Hm yeah ... I guess maybe struct font_desc should be the starting > > > > > point for the kernel internal font structure. It's at least there > > > > > already ... > > > > > > > > I see, that will also make handling built-in fonts much easier! > > > > > > I think the only downside with starting with font_desc as the internal > > > font represenation is that there's a few fields we don't need/have for > > > userspace fonts (like the id/name stuff). So any helpers to e.g. print out > > > font information need to make sure they don't trip over that > > > > > > But otherwise I don't see a problem with this, I think. > > > > Yes, and built-in fonts don't use refcount. Or maybe we can let > > find_font() and get_default_font() kmalloc() a copy of built-in font > > data, then keep track of refcount for both user and built-in fonts, but > > that will waste a few K of memory for each built-in font we use... > > A possible trick for this would be to make sure built-in fonts start > out with a refcount of 1. So never get freed. Plus maybe a check that > if the name is set, then it's a built-in font and if we ever underflow > the refcount we just WARN, but don't free anything. > > Another trick would be kern_font_get/put wrappers (we'd want those > anyway if the userspace fonts are refcounted) and if kern_font->name > != NULL (i.e. built-in font with name) then we simply don't call > kref_get/put. Ick, don't do that, the first trick of having them start out with an increased reference count is the best way here. Makes the code simpler and no special cases for the tear-down path. thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 11:44 AM Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: > > > > On Tue, Sep 29, 2020 at 9:52 AM Daniel Vetter wrote: > > > > > > On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: > > > > On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > > > > > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > > > > > > > > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > > > > > Hi, > > > > > > > > > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > > > > > Also revert the follow-up change "drm: pl111: Absorb the > > > > > > > > external > > > > > > > > register header". > > > > > > > > > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > > > > > > > > > The fbdev driver is used by Android's FVP configuration. Using > > > > > > > > the > > > > > > > > DRM driver together with DRM's fbdev emulation results in a > > > > > > > > failure > > > > > > > > to boot Android. The root cause is that Android's generic fbdev > > > > > > > > userspace driver relies on the ability to set the pixel format > > > > > > > > via > > > > > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > > > > > > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > > > > > > > > > Not without kernel changes. See e.g. > > > > > https://www.spinics.net/lists/dri-devel/msg255883.html > > > > > > > > That discussion seems to have died down with no further action. > > > > > > > > I also was kinda under the assumption that with Android these buffers > > > > would be allocated directly from dma-buf heaps/ion, so this all seems > > > > not > > > > that well baked out. > > > > The disagreement about whether these allocations should be made via > > the render node or via dma-buf/ion is one reason why it was hard to > > make progress on this, unfortunately. > > Yeah, using dumb buffer create to allocate random buffers for shared > software rendering isn't super popular move. > > But aside from all this, why is this blocking the migration from fbdev > to drm? With fbdev you don't have buffer allocations, nor dma-buf > support, and somehow android can boot. But on drm you can't boot if > these things are not available. That sounds like the bar for drm is > maybe a tad too high for simple dumb kms drivers like pl which are > just there to get a picture on the screen/panel. I would not intend to use dumb buffer create to allocate non-framebuffer-destined buffers for software rendering. With the generic fbdev driver any allocations not destined for the framebuffer use ashmem, which is how the driver avoids making huge allocations in fbdev space. I would intend to do something similar for DRM where framebuffer-destined allocations use dumb buffer create and non-framebuffer-destined allocations use dma-buf. At the time I prototyped this approach on the Android side [1] and together with my render-nodes-everywhere patch and some other hopefully-uncontroversial changes it worked and let me boot to the home screen. I would be very happy if this approach were allowed on render nodes so that Android FVP would be unblocked from moving off fbdev, but at the point where we left the thread off last time you weren't sure whether it would be acceptable [2]. One thing that I might not have mentioned on the thread last time was that render nodes have the advantage that unlike primary nodes, opening them does not take master if it is not already taken. This means that on Android, using primary nodes instead of render nodes in the process responsible for creating frame buffers creates a startup race condition for master between the hwcomposer process (which normally opens the primary node and is responsible for flipping frames) and the surfaceflinger process (which normally opens the render node and is responsible for drawing onto the framebuffer and exporting its frames to the composer). Peter [1] https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/2438955 [2] https://www.spinics.net/lists/dri-devel/msg256559.html > -Daniel > > > > > > > > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > > > > > emulation, then let's do that. Not keep fbdev drivers on life > > > > > > support for > > > > > > longer than necessary. > > > > > > > > > > That should have been done *before* removing the old driver, which was > > > > > a userspace break that was introduced in 5.9rc1. We shouldn't leave > > > > > userspace broken for the period of time that it would take to develop > > > > > that change. Even if such a change were developed before 5.9 is > > > > > released, it probably wouldn't be considered a bug fix that would be > > > > > eligible for 5.9. > > > > > > > > > > > > > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > > > > > > > > > > There have
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > Hi, > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > Also revert the follow-up change "drm: pl111: Absorb the external > > > register header". > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > The fbdev driver is used by Android's FVP configuration. Using the > > > DRM driver together with DRM's fbdev emulation results in a failure > > > to boot Android. The root cause is that Android's generic fbdev > > > userspace driver relies on the ability to set the pixel format via > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev emulation. > > > > Can't Android FVP use drm-hwcomposer instead ? Not without kernel changes. See e.g. https://www.spinics.net/lists/dri-devel/msg255883.html > Also, if we need to add more random fbdev ioctls to the drm fbdev > emulation, then let's do that. Not keep fbdev drivers on life support for > longer than necessary. That should have been done *before* removing the old driver, which was a userspace break that was introduced in 5.9rc1. We shouldn't leave userspace broken for the period of time that it would take to develop that change. Even if such a change were developed before 5.9 is released, it probably wouldn't be considered a bug fix that would be eligible for 5.9. > > > > > Neil > > > > > > > > There have been other less critical behavioral differences identified > > > between the fbdev driver and the DRM driver with fbdev emulation. The > > > DRM driver exposes different values for the panel's width, height and > > > refresh rate, and the DRM driver fails a FBIOPUT_VSCREENINFO syscall > > > with yres_virtual greater than the maximum supported value instead > > > of letting the syscall succeed and setting yres_virtual based on yres. > > Also something that should be fixed I think in upstream, in the drm fbdev > emulation. At least doesn't sound like it's something unfixable. Again, it should have been fixed before removing the old driver. Peter > -Daniel > > > > > > > Signed-off-by: Peter Collingbourne > > > --- > > > View this change in Gerrit: > > > https://linux-review.googlesource.com/q/I2d7e59b0e693d9fec206d40df190c5aa02844b56 > > > > > > MAINTAINERS | 5 + > > > drivers/gpu/drm/pl111/pl111_debugfs.c | 1 + > > > drivers/gpu/drm/pl111/pl111_display.c | 1 + > > > drivers/gpu/drm/pl111/pl111_drm.h | 73 -- > > > drivers/gpu/drm/pl111/pl111_drv.c | 1 + > > > drivers/gpu/drm/pl111/pl111_versatile.c | 1 + > > > drivers/video/fbdev/Kconfig | 20 + > > > drivers/video/fbdev/Makefile| 1 + > > > drivers/video/fbdev/amba-clcd.c | 986 > > > include/linux/amba/clcd-regs.h | 87 +++ > > > include/linux/amba/clcd.h | 290 +++ > > > 11 files changed, 1393 insertions(+), 73 deletions(-) > > > create mode 100644 drivers/video/fbdev/amba-clcd.c > > > create mode 100644 include/linux/amba/clcd-regs.h > > > create mode 100644 include/linux/amba/clcd.h > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 190c7fa2ea01..671c1fa79e64 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -1460,6 +1460,11 @@ S: Odd Fixes > > > F: drivers/amba/ > > > F: include/linux/amba/bus.h > > > > > > +ARM PRIMECELL CLCD PL110 DRIVER > > > +M: Russell King > > > +S: Odd Fixes > > > +F: drivers/video/fbdev/amba-clcd.* > > > + > > > ARM PRIMECELL KMI PL050 DRIVER > > > M: Russell King > > > S: Odd Fixes > > > diff --git a/drivers/gpu/drm/pl111/pl111_debugfs.c > > > b/drivers/gpu/drm/pl111/pl111_debugfs.c > > > index 317f68abf18b..26ca8cdf3e60 100644 > > > --- a/drivers/gpu/drm/pl111/pl111_debugfs.c > > > +++ b/drivers/gpu/drm/pl111/pl111_debugfs.c > > > @@ -3,6 +3,7 @@ > > > * Copyright © 2017 Broadcom > > > */ > > > > > > +#include > > > #include > > > > > > #include > > > diff --git a/drivers/gpu/drm/pl111/pl111_display.c > > > b/drivers/gpu/drm/pl111/pl111_display.c > > > index b3e8697cafcf..703ddc803c55 100644 > > > --- a/drivers/gpu/drm/pl111/pl111_display.c > > > +++ b/drivers/gpu/drm/pl111/pl111_display.c > > > @@ -9,6 +9,7 @@ > > > * Copyright (C) 2011 Texas Instruments > > > */ > > > > > > +#include > > > #include > > > #include > > > #include > > > diff --git a/drivers/gpu/drm/pl111/pl111_drm.h > > > b/drivers/gpu/drm/pl111/pl111_drm.h > > > index 2a46b5bd8576..ba399bcb792f 100644 > > > --- a/drivers/gpu/drm/pl111/pl111_drm.h > > > +++ b/drivers/gpu/drm/pl111/pl111_drm.h > > > @@ -23,79 +23,6 @@ > > > #include > > > #include > > > > > > -/* > > > - * CLCD Controller Internal Register addresses > > > - */ > > > -#define CLCD_TIM0 0x > > > -#define CLCD_TIM1 0x0004 > > > -#define CLCD_TIM2 0x0008 > > > -#define CLCD
[PATCH RESEND] drm/bridge: tc358764: restore connector support
This patch restores DRM connector registration in the TC358764 bridge driver and restores usage of the old drm_panel_* API, thus allows dynamic panel registration. This fixes panel operation on Exynos5250-based Arndale board. This is equivalent to the revert of the following commits: 1644127f83bc "drm/bridge: tc358764: add drm_panel_bridge support" 385ca38da29c "drm/bridge: tc358764: drop drm_connector_(un)register" and removal of the calls to drm_panel_attach()/drm_panel_detach(), which were no-ops and has been removed in meanwhile. Signed-off-by: Marek Szyprowski Reviewed-by: Andrzej Hajda --- As I've reported and Andrzej Hajda pointed, the reverted patches break operation of the panel on the Arndale board. Noone suggested how to fix the regression, I've decided to send a revert until a new solution is found. The issues with tc358764 might be automatically resolved once the Exynos DSI itself is converted to DRM bridge: https://patchwork.kernel.org/cover/11770683/ but that approach has also its own issues so far. Resend reason: added Sam Ravnborg to CC: Best regards, Marek Szyprowski --- drivers/gpu/drm/bridge/tc358764.c | 107 +- 1 file changed, 92 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c index d89394bc5aa4..c1e35bdf9232 100644 --- a/drivers/gpu/drm/bridge/tc358764.c +++ b/drivers/gpu/drm/bridge/tc358764.c @@ -153,9 +153,10 @@ static const char * const tc358764_supplies[] = { struct tc358764 { struct device *dev; struct drm_bridge bridge; + struct drm_connector connector; struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)]; struct gpio_desc *gpio_reset; - struct drm_bridge *panel_bridge; + struct drm_panel *panel; int error; }; @@ -209,6 +210,12 @@ static inline struct tc358764 *bridge_to_tc358764(struct drm_bridge *bridge) return container_of(bridge, struct tc358764, bridge); } +static inline +struct tc358764 *connector_to_tc358764(struct drm_connector *connector) +{ + return container_of(connector, struct tc358764, connector); +} + static int tc358764_init(struct tc358764 *ctx) { u32 v = 0; @@ -271,11 +278,43 @@ static void tc358764_reset(struct tc358764 *ctx) usleep_range(1000, 2000); } +static int tc358764_get_modes(struct drm_connector *connector) +{ + struct tc358764 *ctx = connector_to_tc358764(connector); + + return drm_panel_get_modes(ctx->panel, connector); +} + +static const +struct drm_connector_helper_funcs tc358764_connector_helper_funcs = { + .get_modes = tc358764_get_modes, +}; + +static const struct drm_connector_funcs tc358764_connector_funcs = { + .fill_modes = drm_helper_probe_single_connector_modes, + .destroy = drm_connector_cleanup, + .reset = drm_atomic_helper_connector_reset, + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, +}; + +static void tc358764_disable(struct drm_bridge *bridge) +{ + struct tc358764 *ctx = bridge_to_tc358764(bridge); + int ret = drm_panel_disable(bridge_to_tc358764(bridge)->panel); + + if (ret < 0) + dev_err(ctx->dev, "error disabling panel (%d)\n", ret); +} + static void tc358764_post_disable(struct drm_bridge *bridge) { struct tc358764 *ctx = bridge_to_tc358764(bridge); int ret; + ret = drm_panel_unprepare(ctx->panel); + if (ret < 0) + dev_err(ctx->dev, "error unpreparing panel (%d)\n", ret); tc358764_reset(ctx); usleep_range(1, 15000); ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); @@ -296,28 +335,71 @@ static void tc358764_pre_enable(struct drm_bridge *bridge) ret = tc358764_init(ctx); if (ret < 0) dev_err(ctx->dev, "error initializing bridge (%d)\n", ret); + ret = drm_panel_prepare(ctx->panel); + if (ret < 0) + dev_err(ctx->dev, "error preparing panel (%d)\n", ret); +} + +static void tc358764_enable(struct drm_bridge *bridge) +{ + struct tc358764 *ctx = bridge_to_tc358764(bridge); + int ret = drm_panel_enable(ctx->panel); + + if (ret < 0) + dev_err(ctx->dev, "error enabling panel (%d)\n", ret); } static int tc358764_attach(struct drm_bridge *bridge, enum drm_bridge_attach_flags flags) +{ + struct tc358764 *ctx = bridge_to_tc358764(bridge); + struct drm_device *drm = bridge->dev; + int ret; + + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { + DRM_ERROR("Fix bridge driver to make connector optional!"); + return -EINVAL; + } + + ctx->connector.polled = DRM_CONNECTOR_POLL_HPD; + ret = drm_connector_init(drm, &ctx->connector, +&tc358764_connector_fu
Re: [Intel-gfx] [PATCH][next] drm/i915: Fix inconsistent IS_ERR and PTR_ERR
Quoting Gustavo A. R. Silva (2020-09-25 16:35:12) > Hi all, > > Friendly ping: who can take this? We had picked up the same patch from Dan Carpenter, thanks. commit 68ba71e3ae6dd86a23486655e33c5f8c9bd90777 Author: Dan Carpenter Date: Fri Sep 11 10:52:43 2020 +0300 drm/i915: Fix an error code i915_gem_object_copy_blt() -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers
On Wed, Sep 30, 2020 at 12:56 PM Peilin Ye wrote: > > On Wed, Sep 30, 2020 at 11:53:17AM +0200, Daniel Vetter wrote: > > On Wed, Sep 30, 2020 at 03:11:51AM -0400, Peilin Ye wrote: > > > On Tue, Sep 29, 2020 at 04:38:49PM +0200, Daniel Vetter wrote: > > > > On Tue, Sep 29, 2020 at 2:34 PM Peilin Ye wrote: > > > > > Ah, and speaking of built-in fonts, see fbcon_startup(): > > > > > > > > > > /* Setup default font */ > > > > > [...] > > > > > vc->vc_font.charcount = 256; /* FIXME Need to > > > > > support more fonts */ > > > > > ^^^ > > > > > > > > > > This is because find_font() and get_default_font() return a `struct > > > > > font_desc *`, but `struct font_desc` doesn't contain `charcount`. I > > > > > think we also need to add a `charcount` field to `struct font_desc`. > > > > > > > > Hm yeah ... I guess maybe struct font_desc should be the starting > > > > point for the kernel internal font structure. It's at least there > > > > already ... > > > > > > I see, that will also make handling built-in fonts much easier! > > > > I think the only downside with starting with font_desc as the internal > > font represenation is that there's a few fields we don't need/have for > > userspace fonts (like the id/name stuff). So any helpers to e.g. print out > > font information need to make sure they don't trip over that > > > > But otherwise I don't see a problem with this, I think. > > Yes, and built-in fonts don't use refcount. Or maybe we can let > find_font() and get_default_font() kmalloc() a copy of built-in font > data, then keep track of refcount for both user and built-in fonts, but > that will waste a few K of memory for each built-in font we use... A possible trick for this would be to make sure built-in fonts start out with a refcount of 1. So never get freed. Plus maybe a check that if the name is set, then it's a built-in font and if we ever underflow the refcount we just WARN, but don't free anything. Another trick would be kern_font_get/put wrappers (we'd want those anyway if the userspace fonts are refcounted) and if kern_font->name != NULL (i.e. built-in font with name) then we simply don't call kref_get/put. -Daniel > > > > I think for vc_date->vc_font we might need a multi-step approach: > > > > - first add a new helper function which sets the font for a vc using > > > > an uapi console_font struct (and probably hard-coded assumes cnt == > > > > 256. > > > > > > But user fonts may have a charcount different to 256... But yes I'll try > > > to figure out how. > > > > Hm yeah, maybe we need a helper to give us the charcount then, which by > > default is using the magic negative offset. > > Ah, I see! :) > > > Then once we've converted everything over to explicitly passing charcount > > around, we can switch that helper. So something like > > > > int kern_font_charcount(struct kern_font *font); > > > > Feel free to bikeshed the struct name however you see fit :-) > > I think both `kern_font` and `font_desc` makes sense, naming is so > hard... > > > > > For first steps I'd start with demidlayering some of the internal > > > > users of uapi structs, like the console_font_op really shouldn't be > > > > used anywhere in any function, except in the ioctl handler that > > > > converts it into the right function call. You'll probably discover a > > > > few other places like this on the go. > > > > > > Sure, I'll start from this, then cleaning up these dummy functions, then > > > `vc_data`. Thank you for the insights! > > > > Please don't take this rough plan as fixed, it's just where I'd start from > > browsing the code and your analysis a bit. We'll probably have to adapt as > > we go and more nasty things turn up ... > > Sure, I'll first give it a try and see. Thank you! > > Peilin Ye > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: tc358764: restore connector support
On Wed, Sep 30, 2020 at 12:31 PM Daniel Vetter wrote: > > On Wed, Sep 30, 2020 at 12:13 PM Andrzej Hajda wrote: > > > > > > W dniu 24.09.2020 o 10:31, Marek Szyprowski pisze: > > > This patch restores DRM connector registration in the TC358764 bridge > > > driver and restores usage of the old drm_panel_* API, thus allows dynamic > > > panel registration. This fixes panel operation on Exynos5250-based > > > Arndale board. > > > > > > This is equivalent to the revert of the following commits: > > > 1644127f83bc "drm/bridge: tc358764: add drm_panel_bridge support" > > > 385ca38da29c "drm/bridge: tc358764: drop drm_connector_(un)register" > > > and removal of the calls to drm_panel_attach()/drm_panel_detach(), which > > > were no-ops and has been removed in meanwhile. > > > > > > Signed-off-by: Marek Szyprowski > > Reviewed-by: Andrzej Hajda > > > > Regards > > Andrzej > > > --- > > > As I've reported and Andrzej Hajda pointed, the reverted patches break > > > operation of the panel on the Arndale board. Noone suggested how to fix > > > the regression, I've decided to send a revert until a new solution is > > > found. > > > > > > The issues with tc358764 might be automatically resolved once the Exynos > > > DSI itself is converted to DRM bridge: > > > https://patchwork.kernel.org/cover/11770683/ > > > but that approach has also its own issues so far. > > I'm ok with the revert to fix the regression, but I'd kinda like to > see a bit more than "maybe we fix this in the future". Otherwise this > nice idea of having a common drm_bridge abstraction is just leading > towards a complete disaster where every combination of bridge/driver > works slightly differently. And we're half-way there in that mess > already I think. I think minimally it would be good to at least cc tha author of the commit you're reverting, and getting their ack. Adding Sam. -Daniel > > Cheers, Daniel > > > > > > > Best regards, > > > Marek Szyprowski > > > --- > > > drivers/gpu/drm/bridge/tc358764.c | 107 +- > > > 1 file changed, 92 insertions(+), 15 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/bridge/tc358764.c > > > b/drivers/gpu/drm/bridge/tc358764.c > > > index d89394bc5aa4..c1e35bdf9232 100644 > > > --- a/drivers/gpu/drm/bridge/tc358764.c > > > +++ b/drivers/gpu/drm/bridge/tc358764.c > > > @@ -153,9 +153,10 @@ static const char * const tc358764_supplies[] = { > > > struct tc358764 { > > > struct device *dev; > > > struct drm_bridge bridge; > > > + struct drm_connector connector; > > > struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)]; > > > struct gpio_desc *gpio_reset; > > > - struct drm_bridge *panel_bridge; > > > + struct drm_panel *panel; > > > int error; > > > }; > > > > > > @@ -209,6 +210,12 @@ static inline struct tc358764 > > > *bridge_to_tc358764(struct drm_bridge *bridge) > > > return container_of(bridge, struct tc358764, bridge); > > > } > > > > > > +static inline > > > +struct tc358764 *connector_to_tc358764(struct drm_connector *connector) > > > +{ > > > + return container_of(connector, struct tc358764, connector); > > > +} > > > + > > > static int tc358764_init(struct tc358764 *ctx) > > > { > > > u32 v = 0; > > > @@ -271,11 +278,43 @@ static void tc358764_reset(struct tc358764 *ctx) > > > usleep_range(1000, 2000); > > > } > > > > > > +static int tc358764_get_modes(struct drm_connector *connector) > > > +{ > > > + struct tc358764 *ctx = connector_to_tc358764(connector); > > > + > > > + return drm_panel_get_modes(ctx->panel, connector); > > > +} > > > + > > > +static const > > > +struct drm_connector_helper_funcs tc358764_connector_helper_funcs = { > > > + .get_modes = tc358764_get_modes, > > > +}; > > > + > > > +static const struct drm_connector_funcs tc358764_connector_funcs = { > > > + .fill_modes = drm_helper_probe_single_connector_modes, > > > + .destroy = drm_connector_cleanup, > > > + .reset = drm_atomic_helper_connector_reset, > > > + .atomic_duplicate_state = > > > drm_atomic_helper_connector_duplicate_state, > > > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > > > +}; > > > + > > > +static void tc358764_disable(struct drm_bridge *bridge) > > > +{ > > > + struct tc358764 *ctx = bridge_to_tc358764(bridge); > > > + int ret = drm_panel_disable(bridge_to_tc358764(bridge)->panel); > > > + > > > + if (ret < 0) > > > + dev_err(ctx->dev, "error disabling panel (%d)\n", ret); > > > +} > > > + > > > static void tc358764_post_disable(struct drm_bridge *bridge) > > > { > > > struct tc358764 *ctx = bridge_to_tc358764(bridge); > > > int ret; > > > > > > + ret = drm_panel_unprepare(ctx->panel); > > > + if (ret < 0) > > > + dev_err(ctx->dev, "error unpreparing panel (%d)\n", ret); > > > tc358764_reset(ctx); > > > usleep_range(1, 15000); > > > ret = regulato
[radeon-alex:drm-next 182/207] drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:558:6: warning: no previous prototype for 'amdgpu_virt_update_vf2pf_work_item'
tree: git://people.freedesktop.org/~agd5f/linux.git drm-next head: 1ebf4588bcd8d58d1f3c3d38e360ad068b791634 commit: 519b8b76f0b62a0be0d9fcee39819d2461fc3cb0 [182/207] drm/amdgpu: Implement new guest side VF2PF message transaction (v2) config: microblaze-randconfig-r034-20200930 (attached as .config) compiler: microblaze-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git remote add radeon-alex git://people.freedesktop.org/~agd5f/linux.git git fetch --no-tags radeon-alex drm-next git checkout 519b8b76f0b62a0be0d9fcee39819d2461fc3cb0 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=microblaze If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): >> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:558:6: warning: no previous >> prototype for 'amdgpu_virt_update_vf2pf_work_item' [-Wmissing-prototypes] 558 | void amdgpu_virt_update_vf2pf_work_item(struct work_struct *work) | ^~ In file included from drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:28: drivers/gpu/drm/amd/amdgpu/amdgpu.h:197:19: warning: 'no_system_mem_limit' defined but not used [-Wunused-const-variable=] 197 | static const bool no_system_mem_limit; | ^~~ drivers/gpu/drm/amd/amdgpu/amdgpu.h:196:19: warning: 'debug_evictions' defined but not used [-Wunused-const-variable=] 196 | static const bool debug_evictions; /* = false */ | ^~~ drivers/gpu/drm/amd/amdgpu/amdgpu.h:195:18: warning: 'sched_policy' defined but not used [-Wunused-const-variable=] 195 | static const int sched_policy = KFD_SCHED_POLICY_HWS; | ^~~~ In file included from drivers/gpu/drm/amd/amdgpu/../display/dc/dc_types.h:33, from drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:30, from drivers/gpu/drm/amd/amdgpu/../include/dm_pp_interface.h:26, from drivers/gpu/drm/amd/amdgpu/amdgpu.h:67, from drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:28: drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:76:32: warning: 'dc_fixpt_ln2_div_2' defined but not used [-Wunused-const-variable=] 76 | static const struct fixed31_32 dc_fixpt_ln2_div_2 = { 1488522236LL }; |^~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:75:32: warning: 'dc_fixpt_ln2' defined but not used [-Wunused-const-variable=] 75 | static const struct fixed31_32 dc_fixpt_ln2 = { 2977044471LL }; |^~~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:74:32: warning: 'dc_fixpt_e' defined but not used [-Wunused-const-variable=] 74 | static const struct fixed31_32 dc_fixpt_e = { 11674931555LL }; |^~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:73:32: warning: 'dc_fixpt_two_pi' defined but not used [-Wunused-const-variable=] 73 | static const struct fixed31_32 dc_fixpt_two_pi = { 26986075409LL }; |^~~ drivers/gpu/drm/amd/amdgpu/../display/include/fixed31_32.h:72:32: warning: 'dc_fixpt_pi' defined but not used [-Wunused-const-variable=] 72 | static const struct fixed31_32 dc_fixpt_pi = { 13493037705LL }; |^~~ vim +/amdgpu_virt_update_vf2pf_work_item +558 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 557 > 558 void amdgpu_virt_update_vf2pf_work_item(struct work_struct *work) 559 { 560 struct amdgpu_device *adev = container_of(work, struct amdgpu_device, virt.vf2pf_work.work); 561 562 amdgpu_virt_read_pf2vf_data(adev); 563 amdgpu_virt_write_vf2pf_data(adev); 564 565 schedule_delayed_work(&(adev->virt.vf2pf_work), adev->virt.vf2pf_update_interval_ms); 566 } 567 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915/edp/jsl: Update vswing table for HBR and HBR2
On Tue, Sep 29, 2020 at 04:38:22PM -0700, Matt Roper wrote: > On Wed, Sep 30, 2020 at 12:59:58AM +0300, Ville Syrjälä wrote: > > On Wed, Sep 30, 2020 at 12:11:48AM +0300, Ville Syrjälä wrote: > > > On Tue, Sep 29, 2020 at 02:01:44PM -0700, Matt Roper wrote: > > > > On Tue, Sep 29, 2020 at 11:30:22PM +0300, Ville Syrjälä wrote: > > > > > On Tue, Sep 29, 2020 at 08:20:22PM +, Souza, Jose wrote: > > > > > > On Tue, 2020-09-29 at 23:02 +0300, Ville Syrjälä wrote: > > > > > > > On Tue, Sep 29, 2020 at 07:33:45PM +, Souza, Jose wrote: > > > > > > > > On Tue, 2020-09-29 at 17:41 +0530, Tejas Upadhyay wrote: > > > > > > > > > JSL has update in vswing table for eDP > > > > > > > > > > > > > > > > Would be nice to mention in the commit description why PCH is > > > > > > > > being used, that would avoid Ville's question. > > > > > > > > > > > > > > If the thing has nothing to do PCH then it should not use the PCH > > > > > > > type > > > > > > > for the the check. Instead we should just do the EHL/JSL split. > > > > > > > > > > > > In the first version Matt Roper suggested to use PCH to > > > > > > differentiate between EHL and JSL, Jani also agreed with this > > > > > > solution.This 2 PCHs can only be > > > > > > associate with EHL and JSL respectively, so no downsides here. > > > > > > > > > > The downside is that the code makes no sense on the first glance. > > > > > It's going to generate a "wtf?" exception in the brain and require > > > > > me to take a second look to figure what is going on. Exception > > > > > handling is expensive and shouldn't be needed in cases where it's > > > > > trivial to make the code 100% obvious. > > > > > > > > The bspec documents EHL and JSL as being the same platform and identical > > > > in all programming since they are literally the same display IP; this > > > > vswing table is the one and only place where the two are treated in a > > > > distinct manner for reasons that lie outside the display controller. If > > > > you had to stop and take a closer look at the code here, that's a > > > > probably a good thing since in general there should generally never be a > > > > difference in the behavior between the two. Adding an additional > > > > clarifying comment is probably in order too since this is a very > > > > exceptional special case. > > > > > > > > If we deviate from the bspec's guidance and try to split IS_ELKHARTLAKE > > > > and IS_JASPERLAKE across the whole driver, that's going to be a lot more > > > > pain to maintain down the road since we'll almost certainly have cases > > > > where someone silently leaves one or the other off a condition and gets > > > > unexepcted behavior. I could see arguments for using a SUBPLATFORM here > > > > like we do for TGL_U vs TGL_Y, but even that seems like overkill if we > > > > already have a clear way to distinguish the two cases (PCH pairing) and > > > > can just leave a clarifying comment. > > > > > > That fixed PCH pairing is totally undocumented AFAICS. And vswing has > > > nothing to do with the south display, so the wtf will still happen. > > > Comment or no comment. > > > > Oh and JSP does not show up in bspec even once. MCC appears exactly once > > where it talks about the differences between MCC and ICL-N PCH (which I > > guess is the same as JSP?). > > No, ICL-N PCH is something different. :-( There were some early test > chips created that paired the EHL/JSL graphics/media/display IP with an > ICL PCH just for early debug/test purposes, but that pairing isn't > something that actually exists as a real platform. > > I think the confusion here arises because most driver developers only > look at (or have access to) the bspec, which does not aim to document > end-user platforms, but rather IP families that the > graphics/media/display hardware IP teams design. The bspec is not an > authoritative document for anything that lies outside the GMD IP itself. > The GMD architects do sometimes try to pull in additional information > from external teams/sources (such as PCH pairing or the electrical > details like the vswing programming here) to make life easier for the > software teams like us that only really deal with the bspec, but that > information comes from external sources, so it's a bit inconsistent in > terms of how much detail there is (or even whether it's described at > all). We could probably file bspec defects to get them to include the > PCH pairing details for EHL/JSL in the bspec, but ultimately EHL="EHL > G/M/D + MCC PCH" and JSL="EHL G/M/D + JSP PCH;" this has already been > confirmed in an offline email thread with the hardware teams. > > Elkhart Lake and Jasper Lake are two separate end-user platforms, that > both incorporated the same G/M/D IP design. The name "Jasper Lake" > existed as a codename first, so that's the name that shows up in the > bspec; this wound up being a bit confusing when Elkhart Lake was > actually the first of the two to be released and thus wound up bei
Re: [PATCH] drm/bridge: tc358764: restore connector support
On Wed, Sep 30, 2020 at 12:13 PM Andrzej Hajda wrote: > > > W dniu 24.09.2020 o 10:31, Marek Szyprowski pisze: > > This patch restores DRM connector registration in the TC358764 bridge > > driver and restores usage of the old drm_panel_* API, thus allows dynamic > > panel registration. This fixes panel operation on Exynos5250-based > > Arndale board. > > > > This is equivalent to the revert of the following commits: > > 1644127f83bc "drm/bridge: tc358764: add drm_panel_bridge support" > > 385ca38da29c "drm/bridge: tc358764: drop drm_connector_(un)register" > > and removal of the calls to drm_panel_attach()/drm_panel_detach(), which > > were no-ops and has been removed in meanwhile. > > > > Signed-off-by: Marek Szyprowski > Reviewed-by: Andrzej Hajda > > Regards > Andrzej > > --- > > As I've reported and Andrzej Hajda pointed, the reverted patches break > > operation of the panel on the Arndale board. Noone suggested how to fix > > the regression, I've decided to send a revert until a new solution is > > found. > > > > The issues with tc358764 might be automatically resolved once the Exynos > > DSI itself is converted to DRM bridge: > > https://patchwork.kernel.org/cover/11770683/ > > but that approach has also its own issues so far. I'm ok with the revert to fix the regression, but I'd kinda like to see a bit more than "maybe we fix this in the future". Otherwise this nice idea of having a common drm_bridge abstraction is just leading towards a complete disaster where every combination of bridge/driver works slightly differently. And we're half-way there in that mess already I think. Cheers, Daniel > > > > Best regards, > > Marek Szyprowski > > --- > > drivers/gpu/drm/bridge/tc358764.c | 107 +- > > 1 file changed, 92 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/tc358764.c > > b/drivers/gpu/drm/bridge/tc358764.c > > index d89394bc5aa4..c1e35bdf9232 100644 > > --- a/drivers/gpu/drm/bridge/tc358764.c > > +++ b/drivers/gpu/drm/bridge/tc358764.c > > @@ -153,9 +153,10 @@ static const char * const tc358764_supplies[] = { > > struct tc358764 { > > struct device *dev; > > struct drm_bridge bridge; > > + struct drm_connector connector; > > struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)]; > > struct gpio_desc *gpio_reset; > > - struct drm_bridge *panel_bridge; > > + struct drm_panel *panel; > > int error; > > }; > > > > @@ -209,6 +210,12 @@ static inline struct tc358764 > > *bridge_to_tc358764(struct drm_bridge *bridge) > > return container_of(bridge, struct tc358764, bridge); > > } > > > > +static inline > > +struct tc358764 *connector_to_tc358764(struct drm_connector *connector) > > +{ > > + return container_of(connector, struct tc358764, connector); > > +} > > + > > static int tc358764_init(struct tc358764 *ctx) > > { > > u32 v = 0; > > @@ -271,11 +278,43 @@ static void tc358764_reset(struct tc358764 *ctx) > > usleep_range(1000, 2000); > > } > > > > +static int tc358764_get_modes(struct drm_connector *connector) > > +{ > > + struct tc358764 *ctx = connector_to_tc358764(connector); > > + > > + return drm_panel_get_modes(ctx->panel, connector); > > +} > > + > > +static const > > +struct drm_connector_helper_funcs tc358764_connector_helper_funcs = { > > + .get_modes = tc358764_get_modes, > > +}; > > + > > +static const struct drm_connector_funcs tc358764_connector_funcs = { > > + .fill_modes = drm_helper_probe_single_connector_modes, > > + .destroy = drm_connector_cleanup, > > + .reset = drm_atomic_helper_connector_reset, > > + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > > +}; > > + > > +static void tc358764_disable(struct drm_bridge *bridge) > > +{ > > + struct tc358764 *ctx = bridge_to_tc358764(bridge); > > + int ret = drm_panel_disable(bridge_to_tc358764(bridge)->panel); > > + > > + if (ret < 0) > > + dev_err(ctx->dev, "error disabling panel (%d)\n", ret); > > +} > > + > > static void tc358764_post_disable(struct drm_bridge *bridge) > > { > > struct tc358764 *ctx = bridge_to_tc358764(bridge); > > int ret; > > > > + ret = drm_panel_unprepare(ctx->panel); > > + if (ret < 0) > > + dev_err(ctx->dev, "error unpreparing panel (%d)\n", ret); > > tc358764_reset(ctx); > > usleep_range(1, 15000); > > ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), > > ctx->supplies); > > @@ -296,28 +335,71 @@ static void tc358764_pre_enable(struct drm_bridge > > *bridge) > > ret = tc358764_init(ctx); > > if (ret < 0) > > dev_err(ctx->dev, "error initializing bridge (%d)\n", ret); > > + ret = drm_panel_prepare(ctx->panel); > > + if (ret < 0) > > + dev_err(ctx->dev, "error preparing panel (%d)\n", ret); > > +} > >
Re: [PATCH] drm/bridge: tc358764: restore connector support
W dniu 24.09.2020 o 10:31, Marek Szyprowski pisze: > This patch restores DRM connector registration in the TC358764 bridge > driver and restores usage of the old drm_panel_* API, thus allows dynamic > panel registration. This fixes panel operation on Exynos5250-based > Arndale board. > > This is equivalent to the revert of the following commits: > 1644127f83bc "drm/bridge: tc358764: add drm_panel_bridge support" > 385ca38da29c "drm/bridge: tc358764: drop drm_connector_(un)register" > and removal of the calls to drm_panel_attach()/drm_panel_detach(), which > were no-ops and has been removed in meanwhile. > > Signed-off-by: Marek Szyprowski Reviewed-by: Andrzej Hajda Regards Andrzej > --- > As I've reported and Andrzej Hajda pointed, the reverted patches break > operation of the panel on the Arndale board. Noone suggested how to fix > the regression, I've decided to send a revert until a new solution is > found. > > The issues with tc358764 might be automatically resolved once the Exynos > DSI itself is converted to DRM bridge: > https://patchwork.kernel.org/cover/11770683/ > but that approach has also its own issues so far. > > Best regards, > Marek Szyprowski > --- > drivers/gpu/drm/bridge/tc358764.c | 107 +- > 1 file changed, 92 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358764.c > b/drivers/gpu/drm/bridge/tc358764.c > index d89394bc5aa4..c1e35bdf9232 100644 > --- a/drivers/gpu/drm/bridge/tc358764.c > +++ b/drivers/gpu/drm/bridge/tc358764.c > @@ -153,9 +153,10 @@ static const char * const tc358764_supplies[] = { > struct tc358764 { > struct device *dev; > struct drm_bridge bridge; > + struct drm_connector connector; > struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)]; > struct gpio_desc *gpio_reset; > - struct drm_bridge *panel_bridge; > + struct drm_panel *panel; > int error; > }; > > @@ -209,6 +210,12 @@ static inline struct tc358764 *bridge_to_tc358764(struct > drm_bridge *bridge) > return container_of(bridge, struct tc358764, bridge); > } > > +static inline > +struct tc358764 *connector_to_tc358764(struct drm_connector *connector) > +{ > + return container_of(connector, struct tc358764, connector); > +} > + > static int tc358764_init(struct tc358764 *ctx) > { > u32 v = 0; > @@ -271,11 +278,43 @@ static void tc358764_reset(struct tc358764 *ctx) > usleep_range(1000, 2000); > } > > +static int tc358764_get_modes(struct drm_connector *connector) > +{ > + struct tc358764 *ctx = connector_to_tc358764(connector); > + > + return drm_panel_get_modes(ctx->panel, connector); > +} > + > +static const > +struct drm_connector_helper_funcs tc358764_connector_helper_funcs = { > + .get_modes = tc358764_get_modes, > +}; > + > +static const struct drm_connector_funcs tc358764_connector_funcs = { > + .fill_modes = drm_helper_probe_single_connector_modes, > + .destroy = drm_connector_cleanup, > + .reset = drm_atomic_helper_connector_reset, > + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > +}; > + > +static void tc358764_disable(struct drm_bridge *bridge) > +{ > + struct tc358764 *ctx = bridge_to_tc358764(bridge); > + int ret = drm_panel_disable(bridge_to_tc358764(bridge)->panel); > + > + if (ret < 0) > + dev_err(ctx->dev, "error disabling panel (%d)\n", ret); > +} > + > static void tc358764_post_disable(struct drm_bridge *bridge) > { > struct tc358764 *ctx = bridge_to_tc358764(bridge); > int ret; > > + ret = drm_panel_unprepare(ctx->panel); > + if (ret < 0) > + dev_err(ctx->dev, "error unpreparing panel (%d)\n", ret); > tc358764_reset(ctx); > usleep_range(1, 15000); > ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); > @@ -296,28 +335,71 @@ static void tc358764_pre_enable(struct drm_bridge > *bridge) > ret = tc358764_init(ctx); > if (ret < 0) > dev_err(ctx->dev, "error initializing bridge (%d)\n", ret); > + ret = drm_panel_prepare(ctx->panel); > + if (ret < 0) > + dev_err(ctx->dev, "error preparing panel (%d)\n", ret); > +} > + > +static void tc358764_enable(struct drm_bridge *bridge) > +{ > + struct tc358764 *ctx = bridge_to_tc358764(bridge); > + int ret = drm_panel_enable(ctx->panel); > + > + if (ret < 0) > + dev_err(ctx->dev, "error enabling panel (%d)\n", ret); > } > > static int tc358764_attach(struct drm_bridge *bridge, > enum drm_bridge_attach_flags flags) > +{ > + struct tc358764 *ctx = bridge_to_tc358764(bridge); > + struct drm_device *drm = bridge->dev; > + int ret; > + > + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { > + DRM_ERROR("Fix bridge driver to make connector optional!"); >
Re: [PATCH 3/3] RFC: dma-buf: Add an API for importing and exporting sync files (v5)
On Wed, Sep 30, 2020 at 11:39:06AM +0200, Michel Dänzer wrote: > On 2020-03-17 10:21 p.m., Jason Ekstrand wrote: > > Explicit synchronization is the future. At least, that seems to be what > > most userspace APIs are agreeing on at this point. However, most of our > > Linux APIs (both userspace and kernel UAPI) are currently built around > > implicit synchronization with dma-buf. While work is ongoing to change > > many of the userspace APIs and protocols to an explicit synchronization > > model, switching over piecemeal is difficult due to the number of > > potential components involved. On the kernel side, many drivers use > > dma-buf including GPU (3D/compute), display, v4l, and others. In > > userspace, we have X11, several Wayland compositors, 3D drivers, compute > > drivers (OpenCL etc.), media encode/decode, and the list goes on. > > > > This patch provides a path forward by allowing userspace to manually > > manage the fences attached to a dma-buf. Alternatively, one can think > > of this as making dma-buf's implicit synchronization simply a carrier > > for an explicit fence. This is accomplished by adding two IOCTLs to > > dma-buf for importing and exporting a sync file to/from the dma-buf. > > This way a userspace component which is uses explicit synchronization, > > such as a Vulkan driver, can manually set the write fence on a buffer > > before handing it off to an implicitly synchronized component such as a > > Wayland compositor or video encoder. In this way, each of the different > > components can be upgraded to an explicit synchronization model one at a > > time as long as the userspace pieces connecting them are aware of it and > > import/export fences at the right times. > > > > There is a potential race condition with this API if userspace is not > > careful. A typical use case for implicit synchronization is to wait for > > the dma-buf to be ready, use it, and then signal it for some other > > component. Because a sync_file cannot be created until it is guaranteed > > to complete in finite time, userspace can only signal the dma-buf after > > it has already submitted the work which uses it to the kernel and has > > received a sync_file back. There is no way to atomically submit a > > wait-use-signal operation. This is not, however, really a problem with > > this API so much as it is a problem with explicit synchronization > > itself. The way this is typically handled is to have very explicit > > ownership transfer points in the API or protocol which ensure that only > > one component is using it at any given time. Both X11 (via the PRESENT > > extension) and Wayland provide such ownership transfer points via > > explicit present and idle messages. > > > > The decision was intentionally made in this patch to make the import and > > export operations IOCTLs on the dma-buf itself rather than as a DRM > > IOCTL. This makes it the import/export operation universal across all > > components which use dma-buf including GPU, display, v4l, and others. > > It also means that a userspace component can do the import/export > > without access to the DRM fd which may be tricky to get in cases where > > the client communicates with DRM via a userspace API such as OpenGL or > > Vulkan. At a future date we may choose to add direct import/export APIs > > to components such as drm_syncobj to avoid allocating a file descriptor > > and going through two ioctls. However, that seems to be something of a > > micro-optimization as import/export operations are likely to happen at a > > rate of a few per frame of rendered or decoded video. > > > > v2 (Jason Ekstrand): > > - Use a wrapper dma_fence_array of all fences including the new one > > when importing an exclusive fence. > > > > v3 (Jason Ekstrand): > > - Lock around setting shared fences as well as exclusive > > - Mark SIGNAL_SYNC_FILE as a read-write ioctl. > > - Initialize ret to 0 in dma_buf_wait_sync_file > > > > v4 (Jason Ekstrand): > > - Use the new dma_resv_get_singleton helper > > > > v5 (Jason Ekstrand): > > - Rename the IOCTLs to import/export rather than wait/signal > > - Drop the WRITE flag and always get/set the exclusive fence > > > > Signed-off-by: Jason Ekstrand > > What's the status of this? DMA_BUF_IOCTL_EXPORT_SYNC_FILE would be useful > for Wayland compositors to wait for client buffers to become ready without > being prone to getting delayed by later HW access to them, so it would be > nice to merge that at least (if DMA_BUF_IOCTL_IMPORT_SYNC_FILE is still > controversial). I think the missing bits are just the usual stuff - igt testcases - userspace using the new ioctls - review of the entire pile I don't think there's any fundamental objections aside from "no one ever pushed this over the finish line". Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesk
Re: [PATCH rdma-next v4 4/4] RDMA/umem: Move to allocate SG table from pages
On Tue, Sep 29, 2020 at 04:59:29PM -0300, Jason Gunthorpe wrote: > On Sun, Sep 27, 2020 at 09:46:47AM +0300, Leon Romanovsky wrote: > > @@ -296,11 +223,17 @@ static struct ib_umem *__ib_umem_get(struct ib_device > > *device, > > goto umem_release; > > > > cur_base += ret * PAGE_SIZE; > > - npages -= ret; > > - > > - sg = ib_umem_add_sg_table(sg, page_list, ret, > > - dma_get_max_seg_size(device->dma_device), > > - &umem->sg_nents); > > + npages -= ret; > > + sg = __sg_alloc_table_from_pages( > > + &umem->sg_head, page_list, ret, 0, ret << PAGE_SHIFT, > > + dma_get_max_seg_size(device->dma_device), sg, npages, > > + GFP_KERNEL); > > + umem->sg_nents = umem->sg_head.nents; > > + if (IS_ERR(sg)) { > > + unpin_user_pages_dirty_lock(page_list, ret, 0); > > + ret = PTR_ERR(sg); > > + goto umem_release; > > + } > > } > > > > sg_mark_end(sg); > > Does it still need the sg_mark_end? It is preserved here for correctness, the release logic doesn't rely on this marker, but it is better to leave it. Thanks > > Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers
On Wed, Sep 30, 2020 at 03:11:51AM -0400, Peilin Ye wrote: > On Tue, Sep 29, 2020 at 04:38:49PM +0200, Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 2:34 PM Peilin Ye wrote: > > > It seems that users don't use `console_font` directly, they use > > > `console_font_op`. Then, in TTY: > > > > Wow, this is a maze :-/ > > > > > (drivers/tty/vt/vt.c) > > > int con_font_op(struct vc_data *vc, struct console_font_op *op) > > > { > > > switch (op->op) { > > > case KD_FONT_OP_SET: > > > return con_font_set(vc, op); > > > case KD_FONT_OP_GET: > > > return con_font_get(vc, op); > > > case KD_FONT_OP_SET_DEFAULT: > > > return con_font_default(vc, op); > > > case KD_FONT_OP_COPY: > > > return con_font_copy(vc, op); > > > } > > > return -ENOSYS; > > > } > > > > So my gut feeling is that this is just a bit of overenthusiastic > > common code sharing, and all it results is confuse everyone. I think > > if we change the conf_font_get/set/default/copy functions to not take > > the *op struct (which is take pretty arbitrarily from one of the > > ioctl), but the parameters each needs directly, that would clean up > > the code a _lot_. Since most callers would then directly call the > > right operation, instead of this detour through console_font_op. > > struct console_font_op is an uapi struct, so really shouldn't be used > > for internal abstractions - we can't change uapi, hence this makes it > > impossible to refactor anything from the get-go. > > > > I also think that trying to get rid of con_font_op callers as much as > > possible (everywhere where the op struct is constructed in the kernel > > and doesn't come from userspace essentially) should be doable as a > > stand-alone patch series. > > I see, I'll do some code searching and try to clean them up. > > > > These 4 functions allocate `console_font`. We can replace them with our > > > `kernel_console_font`. So, ... > > > > > > $ vgrep "\.con_font_set" > > > > An aside: git grep is awesome, and really fast. > > Ah, yes, by default vgrep uses git-grep. I use vgrep when I need to see > something colorful :) > > > > $ vgrep "\.con_font_get" > > > Index FileLine Content > > > 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1295 .con_font_get = > > > sisusbcon_font_get, > > > 1 drivers/video/console/vgacon.c 1227 .con_font_get = > > > vgacon_font_get, > > > 2 drivers/video/fbdev/core/fbcon.c3121 .con_font_get > > > = fbcon_get_font, > > > $ > > > $ vgrep "\.con_font_default" > > > Index FileLine Content > > > 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1379 .con_font_default = > > > sisusbdummycon_font_default, > > > 1 drivers/video/console/dummycon.c 163 .con_font_default = > > > dummycon_font_default, > > > > The above two return 0 but do nothing, which means width/height are > > now bogus (or well the same as what userspace set). I don't think that > > works correctly ... > > > > > 2 drivers/video/console/newport_con.c 694 .con_font_default = > > > newport_font_default, > > > > This just seems to release the userspace font. This is already done in > > other places where it makes a lot more sense to clean up. > > > > > 3 drivers/video/fbdev/core/fbcon.c3122 .con_font_default= > > > fbcon_set_def_font, > > > > This actually does something. tbh I would not be surprises if the > > fb_set utility is the only thing that uses this - with a bit of code > > search we could perhaps confirm this, and delete all the other > > implementations. > > > > > $ vgrep "\.con_font_copy" > > > Index FileLine Content > > > 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1380 .con_font_copy = > > > sisusbdummycon_font_copy, > > > 1 drivers/video/console/dummycon.c 164 .con_font_copy = > > > dummycon_font_copy, > > > > Above two do nothing, but return 0. Again this wont work I think. > > > > > 2 drivers/video/fbdev/core/fbcon.c3123 .con_font_copy > > > = fbcon_copy_font, > > > > Smells again like something that's only used by fb_set, and we could > > probably delete the other dummy implementations. Also I'm not even > > really clear on what this does ... > > > > Removing these dummy functions means that for a dummy console these > > ioctls would start failing, but then I don't think anyone boots up > > into a dummy console and expects font changes to work. So again I > > think we could split this cleanup as prep work. > > Sure, for step two, I'll read, confirm and try to remove these dummy > functions. > > > > ... are these all the callbacks we need to take care of? What about > > > other console drivers that don't register these callbacks? ... > > > > > > ... for example, mdacon.c? What font does mdacon.
Re: [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
On Wed, Sep 30, 2020 at 10:34:31AM +0200, Christian König wrote: > Am 30.09.20 um 10:19 schrieb Thomas Zimmermann: > > Hi > > > > Am 30.09.20 um 10:05 schrieb Christian König: > > > Am 29.09.20 um 19:49 schrieb Thomas Zimmermann: > > > > Hi Christian > > > > > > > > Am 29.09.20 um 17:35 schrieb Christian König: > > > > > Am 29.09.20 um 17:14 schrieb Thomas Zimmermann: > > > > > > The new helper ttm_kmap_obj_to_dma_buf() extracts address and > > > > > > location > > > > > > from and instance of TTM's kmap_obj and initializes struct > > > > > > dma_buf_map > > > > > > with these values. Helpful for TTM-based drivers. > > > > > We could completely drop that if we use the same structure inside TTM > > > > > as > > > > > well. > > > > > > > > > > Additional to that which driver is going to use this? > > > > As Daniel mentioned, it's in patch 3. The TTM-based drivers will > > > > retrieve the pointer via this function. > > > > > > > > I do want to see all that being more tightly integrated into TTM, but > > > > not in this series. This one is about fixing the bochs-on-sparc64 > > > > problem for good. Patch 7 adds an update to TTM to the DRM TODO list. > > > I should have asked which driver you try to fix here :) > > > > > > In this case just keep the function inside bochs and only fix it there. > > > > > > All other drivers can be fixed when we generally pump this through TTM. > > Did you take a look at patch 3? This function will be used by VRAM > > helpers, nouveau, radeon, amdgpu and qxl. If we don't put it here, we > > have to duplicate the functionality in each if these drivers. Bochs > > itself uses VRAM helpers and doesn't touch the function directly. > > Ah, ok can we have that then only in the VRAM helpers? > > Alternative you could go ahead and use dma_buf_map in ttm_bo_kmap_obj > directly and drop the hack with the TTM_BO_MAP_IOMEM_MASK. > > What I want to avoid is to have another conversion function in TTM because > what happens here is that we already convert from ttm_bus_placement to > ttm_bo_kmap_obj and then to dma_buf_map. Hm I'm not really seeing how that helps with a gradual conversion of everything over to dma_buf_map and assorted helpers for access? There's too many places in ttm drivers where is_iomem and related stuff is used to be able to convert it all in one go. An intermediate state with a bunch of conversions seems fairly unavoidable to me. -Daniel > > Thanks, > Christian. > > > > > Best regards > > Thomas > > > > > Regards, > > > Christian. > > > > > > > Best regards > > > > Thomas > > > > > > > > > Regards, > > > > > Christian. > > > > > > > > > > > Signed-off-by: Thomas Zimmermann > > > > > > --- > > > > > > include/drm/ttm/ttm_bo_api.h | 24 > > > > > > include/linux/dma-buf-map.h | 20 > > > > > > 2 files changed, 44 insertions(+) > > > > > > > > > > > > diff --git a/include/drm/ttm/ttm_bo_api.h > > > > > > b/include/drm/ttm/ttm_bo_api.h > > > > > > index c96a25d571c8..62d89f05a801 100644 > > > > > > --- a/include/drm/ttm/ttm_bo_api.h > > > > > > +++ b/include/drm/ttm/ttm_bo_api.h > > > > > > @@ -34,6 +34,7 @@ > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > +#include > > > > > > #include > > > > > > #include > > > > > > #include > > > > > > @@ -486,6 +487,29 @@ static inline void *ttm_kmap_obj_virtual(struct > > > > > > ttm_bo_kmap_obj *map, > > > > > > return map->virtual; > > > > > > } > > > > > > +/** > > > > > > + * ttm_kmap_obj_to_dma_buf_map > > > > > > + * > > > > > > + * @kmap: A struct ttm_bo_kmap_obj returned from ttm_bo_kmap. > > > > > > + * @map: Returns the mapping as struct dma_buf_map > > > > > > + * > > > > > > + * Converts struct ttm_bo_kmap_obj to struct dma_buf_map. If the > > > > > > memory > > > > > > + * is not mapped, the returned mapping is initialized to NULL. > > > > > > + */ > > > > > > +static inline void ttm_kmap_obj_to_dma_buf_map(struct > > > > > > ttm_bo_kmap_obj > > > > > > *kmap, > > > > > > + struct dma_buf_map *map) > > > > > > +{ > > > > > > + bool is_iomem; > > > > > > + void *vaddr = ttm_kmap_obj_virtual(kmap, &is_iomem); > > > > > > + > > > > > > + if (!vaddr) > > > > > > + dma_buf_map_clear(map); > > > > > > + else if (is_iomem) > > > > > > + dma_buf_map_set_vaddr_iomem(map, (void __force __iomem > > > > > > *)vaddr); > > > > > > + else > > > > > > + dma_buf_map_set_vaddr(map, vaddr); > > > > > > +} > > > > > > + > > > > > > /** > > > > > > * ttm_bo_kmap > > > > > > * > > > > > > diff --git a/include/linux/dma-buf-map.h > > > > > > b/include/linux/dma-buf-map.h > > > > > > index fd1aba545fdf..2e8bbecb5091 100644 > > > > > > --- a/include/linux/dma-buf-map.h > > > > > > +++ b/include/linux/dma-buf-map.h > > > > > > @@ -45,6 +45,12 @@ > > > > > > * > > > > > > * dma_buf_map_set_vaddr(&map. 0xdeadbeaf); > > > > > >
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 01:51:36PM -0700, Peter Collingbourne wrote: > On Tue, Sep 29, 2020 at 11:44 AM Daniel Vetter wrote: > > > > On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: > > > > > > On Tue, Sep 29, 2020 at 9:52 AM Daniel Vetter wrote: > > > > > > > > On Tue, Sep 29, 2020 at 06:48:28PM +0200, Daniel Vetter wrote: > > > > > On Tue, Sep 29, 2020 at 09:30:08AM -0700, Peter Collingbourne wrote: > > > > > > On Tue, Sep 29, 2020 at 2:32 AM Daniel Vetter > > > > > > wrote: > > > > > > > > > > > > > > On Tue, Sep 29, 2020 at 09:28:56AM +0200, Neil Armstrong wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > On 28/09/2020 22:08, Peter Collingbourne wrote: > > > > > > > > > Also revert the follow-up change "drm: pl111: Absorb the > > > > > > > > > external > > > > > > > > > register header". > > > > > > > > > > > > > > > > > > This reverts commits 7e4e589db76a3cf4c1f534eb5a09cc6422766b93 > > > > > > > > > and 0fb8125635e8eb5483fb095f98dcf0651206a7b8. > > > > > > > > > > > > > > > > > > The fbdev driver is used by Android's FVP configuration. > > > > > > > > > Using the > > > > > > > > > DRM driver together with DRM's fbdev emulation results in a > > > > > > > > > failure > > > > > > > > > to boot Android. The root cause is that Android's generic > > > > > > > > > fbdev > > > > > > > > > userspace driver relies on the ability to set the pixel > > > > > > > > > format via > > > > > > > > > FBIOPUT_VSCREENINFO, which is not supported by fbdev > > > > > > > > > emulation. > > > > > > > > > > > > > > > > Can't Android FVP use drm-hwcomposer instead ? > > > > > > > > > > > > Not without kernel changes. See e.g. > > > > > > https://www.spinics.net/lists/dri-devel/msg255883.html > > > > > > > > > > That discussion seems to have died down with no further action. > > > > > > > > > > I also was kinda under the assumption that with Android these buffers > > > > > would be allocated directly from dma-buf heaps/ion, so this all seems > > > > > not > > > > > that well baked out. > > > > > > The disagreement about whether these allocations should be made via > > > the render node or via dma-buf/ion is one reason why it was hard to > > > make progress on this, unfortunately. > > > > Yeah, using dumb buffer create to allocate random buffers for shared > > software rendering isn't super popular move. > > > > But aside from all this, why is this blocking the migration from fbdev > > to drm? With fbdev you don't have buffer allocations, nor dma-buf > > support, and somehow android can boot. But on drm you can't boot if > > these things are not available. That sounds like the bar for drm is > > maybe a tad too high for simple dumb kms drivers like pl which are > > just there to get a picture on the screen/panel. > > I would not intend to use dumb buffer create to allocate > non-framebuffer-destined buffers for software rendering. > > With the generic fbdev driver any allocations not destined for the > framebuffer use ashmem, which is how the driver avoids making huge > allocations in fbdev space. I would intend to do something similar for > DRM where framebuffer-destined allocations use dumb buffer create and > non-framebuffer-destined allocations use dma-buf. At the time I > prototyped this approach on the Android side [1] and together with my > render-nodes-everywhere patch and some other hopefully-uncontroversial > changes it worked and let me boot to the home screen. I would be very > happy if this approach were allowed on render nodes so that Android > FVP would be unblocked from moving off fbdev, but at the point where > we left the thread off last time you weren't sure whether it would be > acceptable [2]. > > One thing that I might not have mentioned on the thread last time was > that render nodes have the advantage that unlike primary nodes, > opening them does not take master if it is not already taken. This > means that on Android, using primary nodes instead of render nodes in > the process responsible for creating frame buffers creates a startup > race condition for master between the hwcomposer process (which > normally opens the primary node and is responsible for flipping > frames) and the surfaceflinger process (which normally opens the > render node and is responsible for drawing onto the framebuffer and > exporting its frames to the composer). I still don't get how this works with fbdev, how do applications render directly into the fbdev memory (when that's the target)? How is that fbdev memory shared with them? Since if the memory shared there isn't really a functional regression, even if we haven't yet solved the "how can clients allocate scanout-capable memory for simple displays" issue. -Daniel > > Peter > > [1] > https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/2438955 > [2] https://www.spinics.net/lists/dri-devel/msg256559.html > > > -Daniel > > > > > > > > > > > > Also, if we need to add more random fbdev ioctls to the drm fbdev > > >
Re: [PATCH] Revert "video: fbdev: amba-clcd: Retire elder CLCD driver"
On Tue, Sep 29, 2020 at 10:29:22PM +0200, Linus Walleij wrote: > On Tue, Sep 29, 2020 at 8:44 PM Daniel Vetter wrote: > > On Tue, Sep 29, 2020 at 7:49 PM Peter Collingbourne wrote: > > > But aside from all this, why is this blocking the migration from fbdev > > to drm? With fbdev you don't have buffer allocations, nor dma-buf > > support, and somehow android can boot. > > I do not know how Android does things these days but back in the > days it would request a virtual framebuffer twice the height of the > physical framebuffer and then pan that up/down between composing > frames, thus achieving a type of double-buffering from userspace. > > Given the type of bugs Peter is seeing this seems to still be the > case, Peter can you confirm? We have the overallocate option for the fbdev emulation to support this use case of flipping buffers. So this part should work I think, but maybe some other parts of our check_var/set_par implementation don't work in a way to make surface flinger happy. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] RFC: dma-buf: Add an API for importing and exporting sync files (v5)
On 2020-03-17 10:21 p.m., Jason Ekstrand wrote: Explicit synchronization is the future. At least, that seems to be what most userspace APIs are agreeing on at this point. However, most of our Linux APIs (both userspace and kernel UAPI) are currently built around implicit synchronization with dma-buf. While work is ongoing to change many of the userspace APIs and protocols to an explicit synchronization model, switching over piecemeal is difficult due to the number of potential components involved. On the kernel side, many drivers use dma-buf including GPU (3D/compute), display, v4l, and others. In userspace, we have X11, several Wayland compositors, 3D drivers, compute drivers (OpenCL etc.), media encode/decode, and the list goes on. This patch provides a path forward by allowing userspace to manually manage the fences attached to a dma-buf. Alternatively, one can think of this as making dma-buf's implicit synchronization simply a carrier for an explicit fence. This is accomplished by adding two IOCTLs to dma-buf for importing and exporting a sync file to/from the dma-buf. This way a userspace component which is uses explicit synchronization, such as a Vulkan driver, can manually set the write fence on a buffer before handing it off to an implicitly synchronized component such as a Wayland compositor or video encoder. In this way, each of the different components can be upgraded to an explicit synchronization model one at a time as long as the userspace pieces connecting them are aware of it and import/export fences at the right times. There is a potential race condition with this API if userspace is not careful. A typical use case for implicit synchronization is to wait for the dma-buf to be ready, use it, and then signal it for some other component. Because a sync_file cannot be created until it is guaranteed to complete in finite time, userspace can only signal the dma-buf after it has already submitted the work which uses it to the kernel and has received a sync_file back. There is no way to atomically submit a wait-use-signal operation. This is not, however, really a problem with this API so much as it is a problem with explicit synchronization itself. The way this is typically handled is to have very explicit ownership transfer points in the API or protocol which ensure that only one component is using it at any given time. Both X11 (via the PRESENT extension) and Wayland provide such ownership transfer points via explicit present and idle messages. The decision was intentionally made in this patch to make the import and export operations IOCTLs on the dma-buf itself rather than as a DRM IOCTL. This makes it the import/export operation universal across all components which use dma-buf including GPU, display, v4l, and others. It also means that a userspace component can do the import/export without access to the DRM fd which may be tricky to get in cases where the client communicates with DRM via a userspace API such as OpenGL or Vulkan. At a future date we may choose to add direct import/export APIs to components such as drm_syncobj to avoid allocating a file descriptor and going through two ioctls. However, that seems to be something of a micro-optimization as import/export operations are likely to happen at a rate of a few per frame of rendered or decoded video. v2 (Jason Ekstrand): - Use a wrapper dma_fence_array of all fences including the new one when importing an exclusive fence. v3 (Jason Ekstrand): - Lock around setting shared fences as well as exclusive - Mark SIGNAL_SYNC_FILE as a read-write ioctl. - Initialize ret to 0 in dma_buf_wait_sync_file v4 (Jason Ekstrand): - Use the new dma_resv_get_singleton helper v5 (Jason Ekstrand): - Rename the IOCTLs to import/export rather than wait/signal - Drop the WRITE flag and always get/set the exclusive fence Signed-off-by: Jason Ekstrand What's the status of this? DMA_BUF_IOCTL_EXPORT_SYNC_FILE would be useful for Wayland compositors to wait for client buffers to become ready without being prone to getting delayed by later HW access to them, so it would be nice to merge that at least (if DMA_BUF_IOCTL_IMPORT_SYNC_FILE is still controversial). -- Earthling Michel Dänzer | https://redhat.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] drm/nouveau: Drop mutex_lock_nested for atomic
On Wed, Sep 30, 2020 at 10:45:05AM +1000, Ben Skeggs wrote: > On Wed, 30 Sep 2020 at 00:52, Daniel Vetter wrote: > > > > On Thu, Sep 17, 2020 at 3:15 PM Daniel Vetter > > wrote: > > > > > > Ben, did you have a chance to look at this? > > > > Ping > > -Daniel > > > > > On Mon, Aug 3, 2020 at 1:22 PM Maarten Lankhorst > > > wrote: > > > > > > > > Op 02-08-2020 om 20:18 schreef Daniel Vetter: > > > > > Purely conjecture, but I think the original locking inversion with the > > > > > legacy page flip code between flipping and ttm's bo move function > > > > > shoudn't exist anymore with atomic: With atomic the bo pinning and > > > > > actual modeset commit is completely separated in the code patsh. > > > > > > > > > > This annotation was originally added in > > > > > > > > > > commit 060810d7abaabcab282e062c595871d661561400 > > > > > Author: Ben Skeggs > > > > > Date: Mon Jul 8 14:15:51 2013 +1000 > > > > > > > > > > drm/nouveau: fix locking issues in page flipping paths > > > > > > > > > > due to > > > > > > > > > > commit b580c9e2b7ba5030a795aa2fb73b796523d65a78 > > > > > Author: Maarten Lankhorst > > > > > Date: Thu Jun 27 13:48:18 2013 +0200 > > > > > > > > > > drm/nouveau: make flipping lockdep safe > > > > > > > > > > Signed-off-by: Daniel Vetter > > > > > Cc: Maarten Lankhorst > > > > > Cc: Ben Skeggs > > > > > Cc: Dave Airlie > > > > > Cc: nouv...@lists.freedesktop.org > > > > > --- > > > > > I might be totally wrong, so this definitely needs testing :-) > > > > > > > > > > Cheers, Daniel > > > > > --- > > > > > drivers/gpu/drm/nouveau/nouveau_bo.c | 6 +- > > > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c > > > > > b/drivers/gpu/drm/nouveau/nouveau_bo.c > > > > > index 7806278dce57..a7b2a9bb0ffe 100644 > > > > > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c > > > > > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c > > > > > @@ -776,7 +776,11 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object > > > > > *bo, int evict, bool intr, > > > > > return ret; > > > > > } > > > > > > > > > > - mutex_lock_nested(&cli->mutex, SINGLE_DEPTH_NESTING); > > > > > + if (drm_drv_uses_atomic_modeset(drm->dev)) > > > > > + mutex_lock(&cli->mutex); > > > > > + else > > > > > + mutex_lock_nested(&cli->mutex, SINGLE_DEPTH_NESTING); > > > > > + > > > > > ret = nouveau_fence_sync(nouveau_bo(bo), chan, true, intr); > > > > > if (ret == 0) { > > > > > ret = drm->ttm.move(chan, bo, &bo->mem, new_reg); > > > > > > > > Well if you're certain it works now. :) > > > > > > > > Reviewed-by: Maarten Lankhorst > Acked-by: Ben Skeggs Can you pull this in through your tree and maybe give it a spin just to make sure? I don't really have nouveau hardware here. Also it's entirely stand-alone, I was simply reviewing all the mutex_lock_nested we have in drm, and this one stuck out as probably not necessary anymore, at least with atomic. I guess I can also just stuff it into drm-misc-next and if it blows up, figure out what to do then :-) -Daniel > > > > > > > > > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > ___ > > Nouveau mailing list > > nouv...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/nouveau -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 204241] amdgpu fails to resume from suspend
https://bugzilla.kernel.org/show_bug.cgi?id=204241 Lahfa Samy (s...@lahfa.xyz) changed: What|Removed |Added CC||s...@lahfa.xyz --- Comment #67 from Lahfa Samy (s...@lahfa.xyz) --- I'm having currently this issue on a T495 with a Ryzen 3700U with integrated graphics Vega RX 10 on ArchLinux with ZFS. Before 5.8.12-arch1-1, I can suspend however right when I resume the system freezes. I have to hard reset by rebooting using the power button, nothing is present in the journalctl besides systemd saying it did suspend, it's not mentioning something that fails about AMDGPU. However have seen a call trace in dmesg about the wifi driver (RIP: 0010:iwl_pcie_rx_handle+0x9c7/0xbb0 [iwlwifi]) but this is happening during boot and thus maybe not affecting the suspend process. The thing is this issue started when I upgraded the kernel from 5.8.11-arch to 5.8.12 but I have also installed AMDGPU (bad timing) and Mesa-git thus I'm not being too sure if the latter is maybe part of the issue or is the very problem of this bug. I have removed the git packages and installed their stable counterparts also removed the kernel parameters amdgpu.cik_support=1 amdgpu.sk_support=1 radeon.sk_support=0 radeon.cik_support=0 and I'll be doing some tests and reporting if I find a way to mitigate the issue. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
Am 30.09.20 um 10:19 schrieb Thomas Zimmermann: Hi Am 30.09.20 um 10:05 schrieb Christian König: Am 29.09.20 um 19:49 schrieb Thomas Zimmermann: Hi Christian Am 29.09.20 um 17:35 schrieb Christian König: Am 29.09.20 um 17:14 schrieb Thomas Zimmermann: The new helper ttm_kmap_obj_to_dma_buf() extracts address and location from and instance of TTM's kmap_obj and initializes struct dma_buf_map with these values. Helpful for TTM-based drivers. We could completely drop that if we use the same structure inside TTM as well. Additional to that which driver is going to use this? As Daniel mentioned, it's in patch 3. The TTM-based drivers will retrieve the pointer via this function. I do want to see all that being more tightly integrated into TTM, but not in this series. This one is about fixing the bochs-on-sparc64 problem for good. Patch 7 adds an update to TTM to the DRM TODO list. I should have asked which driver you try to fix here :) In this case just keep the function inside bochs and only fix it there. All other drivers can be fixed when we generally pump this through TTM. Did you take a look at patch 3? This function will be used by VRAM helpers, nouveau, radeon, amdgpu and qxl. If we don't put it here, we have to duplicate the functionality in each if these drivers. Bochs itself uses VRAM helpers and doesn't touch the function directly. Ah, ok can we have that then only in the VRAM helpers? Alternative you could go ahead and use dma_buf_map in ttm_bo_kmap_obj directly and drop the hack with the TTM_BO_MAP_IOMEM_MASK. What I want to avoid is to have another conversion function in TTM because what happens here is that we already convert from ttm_bus_placement to ttm_bo_kmap_obj and then to dma_buf_map. Thanks, Christian. Best regards Thomas Regards, Christian. Best regards Thomas Regards, Christian. Signed-off-by: Thomas Zimmermann --- include/drm/ttm/ttm_bo_api.h | 24 include/linux/dma-buf-map.h | 20 2 files changed, 44 insertions(+) diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index c96a25d571c8..62d89f05a801 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -486,6 +487,29 @@ static inline void *ttm_kmap_obj_virtual(struct ttm_bo_kmap_obj *map, return map->virtual; } +/** + * ttm_kmap_obj_to_dma_buf_map + * + * @kmap: A struct ttm_bo_kmap_obj returned from ttm_bo_kmap. + * @map: Returns the mapping as struct dma_buf_map + * + * Converts struct ttm_bo_kmap_obj to struct dma_buf_map. If the memory + * is not mapped, the returned mapping is initialized to NULL. + */ +static inline void ttm_kmap_obj_to_dma_buf_map(struct ttm_bo_kmap_obj *kmap, + struct dma_buf_map *map) +{ + bool is_iomem; + void *vaddr = ttm_kmap_obj_virtual(kmap, &is_iomem); + + if (!vaddr) + dma_buf_map_clear(map); + else if (is_iomem) + dma_buf_map_set_vaddr_iomem(map, (void __force __iomem *)vaddr); + else + dma_buf_map_set_vaddr(map, vaddr); +} + /** * ttm_bo_kmap * diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h index fd1aba545fdf..2e8bbecb5091 100644 --- a/include/linux/dma-buf-map.h +++ b/include/linux/dma-buf-map.h @@ -45,6 +45,12 @@ * * dma_buf_map_set_vaddr(&map. 0xdeadbeaf); * + * To set an address in I/O memory, use dma_buf_map_set_vaddr_iomem(). + * + * .. code-block:: c + * + * dma_buf_map_set_vaddr_iomem(&map. 0xdeadbeaf); + * * Test if a mapping is valid with either dma_buf_map_is_set() or * dma_buf_map_is_null(). * @@ -118,6 +124,20 @@ static inline void dma_buf_map_set_vaddr(struct dma_buf_map *map, void *vaddr) map->is_iomem = false; } +/** + * dma_buf_map_set_vaddr_iomem - Sets a dma-buf mapping structure to an address in I/O memory + * @map: The dma-buf mapping structure + * @vaddr_iomem: An I/O-memory address + * + * Sets the address and the I/O-memory flag. + */ +static inline void dma_buf_map_set_vaddr_iomem(struct dma_buf_map *map, + void __iomem *vaddr_iomem) +{ + map->vaddr_iomem = vaddr_iomem; + map->is_iomem = true; +} + /** * dma_buf_map_is_equal - Compares two dma-buf mapping structures for equality * @lhs: The dma-buf mapping structure ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: fix regression in ttm moves
That sounds like the same problem I've got when drm-next was merged into drm-misc-next. I've fixed it in this commit: commit 0b06286579b81449b1e8f14f88d3a8db091fd443 Author: Christian König Date: Wed Aug 19 15:27:48 2020 +0200 drm/ttm: fix broken merge between drm-next and drm-misc-next drm-next reverted the changes to ttm_tt_create() to do the NULL check inside the function, but drm-misc-next adds new users of this approach. Re-apply the NULL check change inside the function to fix this. Signed-off-by: Christian König Reviewed-by: Dave Airlie Link: https://patchwork.freedesktop.org/patch/386628/ Not sure why it should cause problems with drm-fixes and drm-next as well. Regards, Christian. Am 30.09.20 um 09:09 schrieb Dave Airlie: just FYI I'm seeing a regression on vmwgfx with drm-fixes and drm-next merged into it. I'm going take some time to dig through and work out where, the regression is a command failure and a ioremap failure. Dave. On Wed, 30 Sep 2020 at 16:26, Dave Airlie wrote: Uggh this is part of the mess with the revert, I'm not sure how best to dig out of this one yet. Dave. On Wed, 30 Sep 2020 at 15:55, Dave Airlie wrote: From: Dave Airlie This fixes a bug introduced in be1213a341a289afc51f89181c310e368fba0b66 drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2 On vmwgfx this causes a Command buffer error WARN to trigger. This is because the old code used to check if bo->ttm was true, and the new code doesn't, fix it code to add back the check resolves the issue. Fixes: be1213a341a2 ("drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2") Signed-off-by: Dave Airlie --- drivers/gpu/drm/ttm/ttm_bo.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 70b3bee27850..e8aa2fe8e9d1 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -251,9 +251,11 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, /* Zero init the new TTM structure if the old location should * have used one as well. */ - ret = ttm_tt_create(bo, old_man->use_tt); - if (ret) - goto out_err; + if (!bo->ttm) { + ret = ttm_tt_create(bo, old_man->use_tt); + if (ret) + goto out_err; + } ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement); if (ret) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7Ca8e51dce1b1346015c1e08d8650fdc59%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637370466085507013&sdata=QrtSgfkmSpNcNfdJ71YNATS0URyEcMNLeMVmOenRpak%3D&reserved=0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
Hi Am 30.09.20 um 10:05 schrieb Christian König: > Am 29.09.20 um 19:49 schrieb Thomas Zimmermann: >> Hi Christian >> >> Am 29.09.20 um 17:35 schrieb Christian König: >>> Am 29.09.20 um 17:14 schrieb Thomas Zimmermann: The new helper ttm_kmap_obj_to_dma_buf() extracts address and location from and instance of TTM's kmap_obj and initializes struct dma_buf_map with these values. Helpful for TTM-based drivers. >>> We could completely drop that if we use the same structure inside TTM as >>> well. >>> >>> Additional to that which driver is going to use this? >> As Daniel mentioned, it's in patch 3. The TTM-based drivers will >> retrieve the pointer via this function. >> >> I do want to see all that being more tightly integrated into TTM, but >> not in this series. This one is about fixing the bochs-on-sparc64 >> problem for good. Patch 7 adds an update to TTM to the DRM TODO list. > > I should have asked which driver you try to fix here :) > > In this case just keep the function inside bochs and only fix it there. > > All other drivers can be fixed when we generally pump this through TTM. Did you take a look at patch 3? This function will be used by VRAM helpers, nouveau, radeon, amdgpu and qxl. If we don't put it here, we have to duplicate the functionality in each if these drivers. Bochs itself uses VRAM helpers and doesn't touch the function directly. Best regards Thomas > > Regards, > Christian. > >> Best regards >> Thomas >> >>> Regards, >>> Christian. >>> Signed-off-by: Thomas Zimmermann --- include/drm/ttm/ttm_bo_api.h | 24 include/linux/dma-buf-map.h | 20 2 files changed, 44 insertions(+) diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index c96a25d571c8..62d89f05a801 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -486,6 +487,29 @@ static inline void *ttm_kmap_obj_virtual(struct ttm_bo_kmap_obj *map, return map->virtual; } +/** + * ttm_kmap_obj_to_dma_buf_map + * + * @kmap: A struct ttm_bo_kmap_obj returned from ttm_bo_kmap. + * @map: Returns the mapping as struct dma_buf_map + * + * Converts struct ttm_bo_kmap_obj to struct dma_buf_map. If the memory + * is not mapped, the returned mapping is initialized to NULL. + */ +static inline void ttm_kmap_obj_to_dma_buf_map(struct ttm_bo_kmap_obj *kmap, + struct dma_buf_map *map) +{ + bool is_iomem; + void *vaddr = ttm_kmap_obj_virtual(kmap, &is_iomem); + + if (!vaddr) + dma_buf_map_clear(map); + else if (is_iomem) + dma_buf_map_set_vaddr_iomem(map, (void __force __iomem *)vaddr); + else + dma_buf_map_set_vaddr(map, vaddr); +} + /** * ttm_bo_kmap * diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h index fd1aba545fdf..2e8bbecb5091 100644 --- a/include/linux/dma-buf-map.h +++ b/include/linux/dma-buf-map.h @@ -45,6 +45,12 @@ * * dma_buf_map_set_vaddr(&map. 0xdeadbeaf); * + * To set an address in I/O memory, use dma_buf_map_set_vaddr_iomem(). + * + * .. code-block:: c + * + * dma_buf_map_set_vaddr_iomem(&map. 0xdeadbeaf); + * * Test if a mapping is valid with either dma_buf_map_is_set() or * dma_buf_map_is_null(). * @@ -118,6 +124,20 @@ static inline void dma_buf_map_set_vaddr(struct dma_buf_map *map, void *vaddr) map->is_iomem = false; } +/** + * dma_buf_map_set_vaddr_iomem - Sets a dma-buf mapping structure to an address in I/O memory + * @map: The dma-buf mapping structure + * @vaddr_iomem: An I/O-memory address + * + * Sets the address and the I/O-memory flag. + */ +static inline void dma_buf_map_set_vaddr_iomem(struct dma_buf_map *map, + void __iomem *vaddr_iomem) +{ + map->vaddr_iomem = vaddr_iomem; + map->is_iomem = true; +} + /** * dma_buf_map_is_equal - Compares two dma-buf mapping structures for equality * @lhs: The dma-buf mapping structure >>> ___ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel >> >> ___ >> amd-gfx mailing list >> amd-...@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https:/
Re: [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
Am 29.09.20 um 19:49 schrieb Thomas Zimmermann: Hi Christian Am 29.09.20 um 17:35 schrieb Christian König: Am 29.09.20 um 17:14 schrieb Thomas Zimmermann: The new helper ttm_kmap_obj_to_dma_buf() extracts address and location from and instance of TTM's kmap_obj and initializes struct dma_buf_map with these values. Helpful for TTM-based drivers. We could completely drop that if we use the same structure inside TTM as well. Additional to that which driver is going to use this? As Daniel mentioned, it's in patch 3. The TTM-based drivers will retrieve the pointer via this function. I do want to see all that being more tightly integrated into TTM, but not in this series. This one is about fixing the bochs-on-sparc64 problem for good. Patch 7 adds an update to TTM to the DRM TODO list. I should have asked which driver you try to fix here :) In this case just keep the function inside bochs and only fix it there. All other drivers can be fixed when we generally pump this through TTM. Regards, Christian. Best regards Thomas Regards, Christian. Signed-off-by: Thomas Zimmermann --- include/drm/ttm/ttm_bo_api.h | 24 include/linux/dma-buf-map.h | 20 2 files changed, 44 insertions(+) diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index c96a25d571c8..62d89f05a801 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -486,6 +487,29 @@ static inline void *ttm_kmap_obj_virtual(struct ttm_bo_kmap_obj *map, return map->virtual; } +/** + * ttm_kmap_obj_to_dma_buf_map + * + * @kmap: A struct ttm_bo_kmap_obj returned from ttm_bo_kmap. + * @map: Returns the mapping as struct dma_buf_map + * + * Converts struct ttm_bo_kmap_obj to struct dma_buf_map. If the memory + * is not mapped, the returned mapping is initialized to NULL. + */ +static inline void ttm_kmap_obj_to_dma_buf_map(struct ttm_bo_kmap_obj *kmap, + struct dma_buf_map *map) +{ + bool is_iomem; + void *vaddr = ttm_kmap_obj_virtual(kmap, &is_iomem); + + if (!vaddr) + dma_buf_map_clear(map); + else if (is_iomem) + dma_buf_map_set_vaddr_iomem(map, (void __force __iomem *)vaddr); + else + dma_buf_map_set_vaddr(map, vaddr); +} + /** * ttm_bo_kmap * diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h index fd1aba545fdf..2e8bbecb5091 100644 --- a/include/linux/dma-buf-map.h +++ b/include/linux/dma-buf-map.h @@ -45,6 +45,12 @@ * * dma_buf_map_set_vaddr(&map. 0xdeadbeaf); * + * To set an address in I/O memory, use dma_buf_map_set_vaddr_iomem(). + * + * .. code-block:: c + * + * dma_buf_map_set_vaddr_iomem(&map. 0xdeadbeaf); + * * Test if a mapping is valid with either dma_buf_map_is_set() or * dma_buf_map_is_null(). * @@ -118,6 +124,20 @@ static inline void dma_buf_map_set_vaddr(struct dma_buf_map *map, void *vaddr) map->is_iomem = false; } +/** + * dma_buf_map_set_vaddr_iomem - Sets a dma-buf mapping structure to an address in I/O memory + * @map: The dma-buf mapping structure + * @vaddr_iomem: An I/O-memory address + * + * Sets the address and the I/O-memory flag. + */ +static inline void dma_buf_map_set_vaddr_iomem(struct dma_buf_map *map, + void __iomem *vaddr_iomem) +{ + map->vaddr_iomem = vaddr_iomem; + map->is_iomem = true; +} + /** * dma_buf_map_is_equal - Compares two dma-buf mapping structures for equality * @lhs: The dma-buf mapping structure ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dsi: Replace spin_lock_irqsave by spin_lock in hard IRQ
It is redundant to do irqsave and irqrestore in hardIRQ context. Signed-off-by: Tian Tao --- drivers/gpu/drm/msm/dsi/dsi_host.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index b17ac6c..b2fb5c3 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -1555,15 +1555,14 @@ static irqreturn_t dsi_host_irq(int irq, void *ptr) { struct msm_dsi_host *msm_host = ptr; u32 isr; - unsigned long flags; if (!msm_host->ctrl_base) return IRQ_HANDLED; - spin_lock_irqsave(&msm_host->intr_lock, flags); + spin_lock(&msm_host->intr_lock); isr = dsi_read(msm_host, REG_DSI_INTR_CTRL); dsi_write(msm_host, REG_DSI_INTR_CTRL, isr); - spin_unlock_irqrestore(&msm_host->intr_lock, flags); + spin_unlock(&msm_host->intr_lock); DBG("isr=0x%x, id=%d", isr, msm_host->id); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/hisilicon: Delete the unused macro
在 2020/9/29 15:24, Thomas Zimmermann 写道: Am 29.09.20 um 02:45 schrieb Tian Tao: The macro PADDING is no longer used. Delete it. Signed-off-by: Tian Tao Reviewed-by: Thomas Zimmermann Thanks a lot for the timely help with the review code! --- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c index 4d57ec6..b3a81da 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c @@ -52,8 +52,6 @@ static const struct hibmc_dislay_pll_config hibmc_pll_table[] = { {1920, 1200, CRT_PLL1_HS_193MHZ, CRT_PLL2_HS_193MHZ}, }; -#define PADDING(align, data) (((data) + (align) - 1) & (~((align) - 1))) - static int hibmc_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) { ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] vt_ioctl: make VT_RESIZEX behave like VT_RESIZE
On Tue, Sep 29, 2020 at 10:12:46AM +0900, Tetsuo Handa wrote: > On 2020/09/29 2:59, Martin Hostettler wrote: > > On Sun, Sep 27, 2020 at 08:46:30PM +0900, Tetsuo Handa wrote: > >> VT_RESIZEX was introduced in Linux 1.3.3, but it is unclear that what > >> comes to the "+ more" part, and I couldn't find a user of VT_RESIZEX. > >> > > > > It seems this is/was used by "svgatextmode" which seems to be at > > http://www.ibiblio.org/pub/Linux/utils/console/ > > > > Not sure if that kind of software still has a chance to work nowadays. > > > > Thanks for the information. > > It seems that v.v_vlin = curr_textmode->VDisplay / > (MOFLG_ISSET(curr_textmode, ATTR_DOUBLESCAN) ? 2 : 1) > and v.v_clin = curr_textmode->FontHeight . Thus, v.v_clin is font's height > and seems to be non-zero. > But according to https://bugs.gentoo.org/19485 , people are using kernel > framebuffer instead. > Yes, this seems to be from pre framebuffer times. Back in the days "svga" was the wording you got for "pokes svga card hardware registers from userspace drivers". And textmode means font rendering is done via (fixed function in those times) hardware scanout engine. Of course having only to update 2 bytes per character was a huge saving early on. Likely this is also before vesa VBE was reliable. So i guess the point where this all starts going wrong allowing the X parts of the api to be combined with FB based rendering at all? Sounds the only user didn't use that combination and so it was never tested? Then again, this all relates to hardware from 20 years ago... - Martin Hostettler ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dt-bindings: Fix 'reg' size issues in zynqmp examples
On 29. 09. 20 16:55, Rob Herring wrote: > On Tue, Sep 29, 2020 at 1:55 AM Michal Simek wrote: >> >> Hi Rob, >> >> On 28. 09. 20 17:59, Rob Herring wrote: >>> The default sizes in examples for 'reg' are 1 cell each. Fix the >>> incorrect sizes in zynqmp examples: >>> >>> Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.example.dt.yaml: >>> example-0: dma-controller@fd4c:reg:0: [0, 4249616384, 0, 4096] is too >>> long >>> From schema: >>> /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml >>> Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: >>> example-0: display@fd4a:reg:0: [0, 4249485312, 0, 4096] is too long >>> From schema: >>> /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml >>> Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: >>> example-0: display@fd4a:reg:1: [0, 4249526272, 0, 4096] is too long >>> From schema: >>> /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml >>> Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: >>> example-0: display@fd4a:reg:2: [0, 4249530368, 0, 4096] is too long >>> From schema: >>> /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml >>> Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.example.dt.yaml: >>> example-0: display@fd4a:reg:3: [0, 4249534464, 0, 4096] is too long >>> From schema: >>> /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml >>> >>> Cc: Hyun Kwon >>> Cc: Laurent Pinchart >>> Cc: Michal Simek >>> Cc: Vinod Koul >>> Cc: dri-devel@lists.freedesktop.org >>> Cc: dmaeng...@vger.kernel.org >>> Signed-off-by: Rob Herring >>> --- >>> .../bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml | 8 >>> .../devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml | 2 +- >>> 2 files changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml >>> b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml >>> index 52a939cade3b..7b9d468c3e52 100644 >>> --- a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml >>> +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml >>> @@ -145,10 +145,10 @@ examples: >>> >>> display@fd4a { >>> compatible = "xlnx,zynqmp-dpsub-1.7"; >>> -reg = <0x0 0xfd4a 0x0 0x1000>, >>> - <0x0 0xfd4aa000 0x0 0x1000>, >>> - <0x0 0xfd4ab000 0x0 0x1000>, >>> - <0x0 0xfd4ac000 0x0 0x1000>; >>> +reg = <0xfd4a 0x1000>, >>> + <0xfd4aa000 0x1000>, >>> + <0xfd4ab000 0x1000>, >>> + <0xfd4ac000 0x1000>; >>> reg-names = "dp", "blend", "av_buf", "aud"; >>> interrupts = <0 119 4>; >>> interrupt-parent = <&gic>; >>> diff --git >>> a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml >>> b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml >>> index 5de510f8c88c..2a595b18ff6c 100644 >>> --- a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml >>> +++ b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dpdma.yaml >>> @@ -57,7 +57,7 @@ examples: >>> >>> dma: dma-controller@fd4c { >>>compatible = "xlnx,zynqmp-dpdma"; >>> - reg = <0x0 0xfd4c 0x0 0x1000>; >>> + reg = <0xfd4c 0x1000>; >>>interrupts = ; >>>interrupt-parent = <&gic>; >>>clocks = <&dpdma_clk>; >>> >> >> I would prefer to keep 64bit version. >> I use this style. > > I prefer to keep the examples simple. The address size is outside the > scope of the binding. ok. Acked-by: Michal Simek Thanks, Michal ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers
On Tue, Sep 29, 2020 at 11:09:45AM +0200, Daniel Vetter wrote: > If you want to follow along a bit I think would be good to subscribe to > the dri-devel mailing list. At least for all the fbcon/fbdev/gpu stuff. > > I don't think there's a dedicated list for vt/console stuff, aside from > Greg's inbox :-) Ah, I've been checking lore.kernel.org/dri-devel/ once a while. Sure! I'll subscribe right now :) Peilin Ye ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 6/7] drm: Validate encoder->possible_crtcs
On 10.09.20 20:18, Deucher, Alexander wrote: > [AMD Public Use] > > > >> -Original Message- >> From: amd-gfx On Behalf Of >> Daniel Vetter >> Sent: Monday, September 7, 2020 3:15 AM >> To: Jan Kiszka ; amd-gfx list > g...@lists.freedesktop.org>; Wentland, Harry ; >> Kazlauskas, Nicholas >> Cc: dri-devel ; intel-gfx > g...@lists.freedesktop.org>; Thomas Zimmermann >> ; Ville Syrjala >> Subject: Re: [PATCH v3 6/7] drm: Validate encoder->possible_crtcs >> >> On Sun, Sep 6, 2020 at 1:19 PM Jan Kiszka wrote: >>> >>> On 11.02.20 18:04, Daniel Vetter wrote: On Tue, Feb 11, 2020 at 06:22:07PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä > > WARN if the encoder possible_crtcs is effectively empty or contains > bits for non-existing crtcs. > > v2: Move to drm_mode_config_validate() (Daniel) > Make the docs say we WARN when this is wrong (Daniel) > Extract full_crtc_mask() > > Cc: Thomas Zimmermann > Cc: Daniel Vetter > Signed-off-by: Ville Syrjälä When pushing the fixup needs to be applied before the validation patch here, because we don't want to anger the bisect gods. Reviewed-by: Daniel Vetter I think with the fixup we should be good enough with the existing nonsense in drivers. Fingers crossed. -Daniel > --- > drivers/gpu/drm/drm_mode_config.c | 27 >> ++- > include/drm/drm_encoder.h | 2 +- > 2 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mode_config.c > b/drivers/gpu/drm/drm_mode_config.c > index afc91447293a..4c1b350ddb95 100644 > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -581,6 +581,29 @@ static void >> validate_encoder_possible_clones(struct drm_encoder *encoder) > encoder->possible_clones, encoder_mask); } > > +static u32 full_crtc_mask(struct drm_device *dev) { > +struct drm_crtc *crtc; > +u32 crtc_mask = 0; > + > +drm_for_each_crtc(crtc, dev) > +crtc_mask |= drm_crtc_mask(crtc); > + > +return crtc_mask; > +} > + > +static void validate_encoder_possible_crtcs(struct drm_encoder > +*encoder) { > +u32 crtc_mask = full_crtc_mask(encoder->dev); > + > +WARN((encoder->possible_crtcs & crtc_mask) == 0 || > + (encoder->possible_crtcs & ~crtc_mask) != 0, > + "Bogus possible_crtcs: " > + "[ENCODER:%d:%s] possible_crtcs=0x%x (full crtc mask=0x%x)\n", > + encoder->base.id, encoder->name, > + encoder->possible_crtcs, crtc_mask); } > + > void drm_mode_config_validate(struct drm_device *dev) { > struct drm_encoder *encoder; > @@ -588,6 +611,8 @@ void drm_mode_config_validate(struct >> drm_device *dev) > drm_for_each_encoder(encoder, dev) > fixup_encoder_possible_clones(encoder); > > -drm_for_each_encoder(encoder, dev) > +drm_for_each_encoder(encoder, dev) { > validate_encoder_possible_clones(encoder); > +validate_encoder_possible_crtcs(encoder); > +} > } > diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h > index 3741963b9587..b236269f41ac 100644 > --- a/include/drm/drm_encoder.h > +++ b/include/drm/drm_encoder.h > @@ -142,7 +142,7 @@ struct drm_encoder { > * the bits for all &drm_crtc objects this encoder can be connected > to > * before calling drm_dev_register(). > * > - * In reality almost every driver gets this wrong. > + * You will get a WARN if you get this wrong in the driver. > * > * Note that since CRTC objects can't be hotplugged the assigned >> indices > * are stable and hence known before registering all objects. > -- > 2.24.1 > >>> >>> Triggers on an Advantech AIMB-228 (R1505G, 3 DP outputs): >> >> Adding amdgpu display folks. > > I took a quick look at this and it looks like we limit the number of crtcs > later in the mode init process if the number of physical displays can't > actually use more crtcs. E.g., the physical board configuration would only > allow for 3 active displays even if the hardware technically supports 4 > crtcs. I presume that way we can just leave the additional hardware power > gated all the time. > So, will this be fixed any time soon? I don't feel qualified writing such a patch but I would obviously be happy to test one. Jan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH drm/hisilicon 2/2] drm/hisilicon: Use the same style of variable type in hibmc_drm_drv
Consistently Use the same style of variable type in hibmc_drm_de.c and hibmc_drm_de.h. Signed-off-by: Tian Tao --- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 13 ++--- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 8 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index 5632bce..0c1b40d 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -121,12 +121,11 @@ static void hibmc_kms_fini(struct hibmc_drm_private *priv) /* * It can operate in one of three modes: 0, 1 or Sleep. */ -void hibmc_set_power_mode(struct hibmc_drm_private *priv, - unsigned int power_mode) +void hibmc_set_power_mode(struct hibmc_drm_private *priv, u32 power_mode) { - unsigned int control_value = 0; + u32 control_value = 0; void __iomem *mmio = priv->mmio; - unsigned int input = 1; + u32 input = 1; if (power_mode > HIBMC_PW_MODE_CTL_MODE_SLEEP) return; @@ -144,8 +143,8 @@ void hibmc_set_power_mode(struct hibmc_drm_private *priv, void hibmc_set_current_gate(struct hibmc_drm_private *priv, unsigned int gate) { - unsigned int gate_reg; - unsigned int mode; + u32 gate_reg; + u32 mode; void __iomem *mmio = priv->mmio; /* Get current power mode. */ @@ -170,7 +169,7 @@ void hibmc_set_current_gate(struct hibmc_drm_private *priv, unsigned int gate) static void hibmc_hw_config(struct hibmc_drm_private *priv) { - unsigned int reg; + u32 reg; /* On hardware reset, power mode 0 is default. */ hibmc_set_power_mode(priv, HIBMC_PW_MODE_CTL_MODE_MODE0); diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h index 6a63502..5c4030d 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h @@ -33,8 +33,8 @@ struct hibmc_drm_private { /* hw */ void __iomem *mmio; void __iomem *fb_map; - unsigned long fb_base; - unsigned long fb_size; + u64 fb_base; + u64 fb_size; /* drm */ struct drm_device *dev; @@ -56,9 +56,9 @@ static inline struct hibmc_drm_private *to_hibmc_drm_private(struct drm_device * } void hibmc_set_power_mode(struct hibmc_drm_private *priv, - unsigned int power_mode); + u32 power_mode); void hibmc_set_current_gate(struct hibmc_drm_private *priv, - unsigned int gate); + u32 gate); int hibmc_de_init(struct hibmc_drm_private *priv); int hibmc_vdac_init(struct hibmc_drm_private *priv); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers
On Tue, Sep 29, 2020 at 04:38:49PM +0200, Daniel Vetter wrote: > On Tue, Sep 29, 2020 at 2:34 PM Peilin Ye wrote: > > It seems that users don't use `console_font` directly, they use > > `console_font_op`. Then, in TTY: > > Wow, this is a maze :-/ > > > (drivers/tty/vt/vt.c) > > int con_font_op(struct vc_data *vc, struct console_font_op *op) > > { > > switch (op->op) { > > case KD_FONT_OP_SET: > > return con_font_set(vc, op); > > case KD_FONT_OP_GET: > > return con_font_get(vc, op); > > case KD_FONT_OP_SET_DEFAULT: > > return con_font_default(vc, op); > > case KD_FONT_OP_COPY: > > return con_font_copy(vc, op); > > } > > return -ENOSYS; > > } > > So my gut feeling is that this is just a bit of overenthusiastic > common code sharing, and all it results is confuse everyone. I think > if we change the conf_font_get/set/default/copy functions to not take > the *op struct (which is take pretty arbitrarily from one of the > ioctl), but the parameters each needs directly, that would clean up > the code a _lot_. Since most callers would then directly call the > right operation, instead of this detour through console_font_op. > struct console_font_op is an uapi struct, so really shouldn't be used > for internal abstractions - we can't change uapi, hence this makes it > impossible to refactor anything from the get-go. > > I also think that trying to get rid of con_font_op callers as much as > possible (everywhere where the op struct is constructed in the kernel > and doesn't come from userspace essentially) should be doable as a > stand-alone patch series. I see, I'll do some code searching and try to clean them up. > > These 4 functions allocate `console_font`. We can replace them with our > > `kernel_console_font`. So, ... > > > > $ vgrep "\.con_font_set" > > An aside: git grep is awesome, and really fast. Ah, yes, by default vgrep uses git-grep. I use vgrep when I need to see something colorful :) > > $ vgrep "\.con_font_get" > > Index FileLine Content > > 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1295 .con_font_get = > > sisusbcon_font_get, > > 1 drivers/video/console/vgacon.c 1227 .con_font_get = > > vgacon_font_get, > > 2 drivers/video/fbdev/core/fbcon.c3121 .con_font_get > > = fbcon_get_font, > > $ > > $ vgrep "\.con_font_default" > > Index FileLine Content > > 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1379 .con_font_default = > > sisusbdummycon_font_default, > > 1 drivers/video/console/dummycon.c 163 .con_font_default = > > dummycon_font_default, > > The above two return 0 but do nothing, which means width/height are > now bogus (or well the same as what userspace set). I don't think that > works correctly ... > > > 2 drivers/video/console/newport_con.c 694 .con_font_default = > > newport_font_default, > > This just seems to release the userspace font. This is already done in > other places where it makes a lot more sense to clean up. > > > 3 drivers/video/fbdev/core/fbcon.c3122 .con_font_default= > > fbcon_set_def_font, > > This actually does something. tbh I would not be surprises if the > fb_set utility is the only thing that uses this - with a bit of code > search we could perhaps confirm this, and delete all the other > implementations. > > > $ vgrep "\.con_font_copy" > > Index FileLine Content > > 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1380 .con_font_copy = > > sisusbdummycon_font_copy, > > 1 drivers/video/console/dummycon.c 164 .con_font_copy = > > dummycon_font_copy, > > Above two do nothing, but return 0. Again this wont work I think. > > > 2 drivers/video/fbdev/core/fbcon.c3123 .con_font_copy > > = fbcon_copy_font, > > Smells again like something that's only used by fb_set, and we could > probably delete the other dummy implementations. Also I'm not even > really clear on what this does ... > > Removing these dummy functions means that for a dummy console these > ioctls would start failing, but then I don't think anyone boots up > into a dummy console and expects font changes to work. So again I > think we could split this cleanup as prep work. Sure, for step two, I'll read, confirm and try to remove these dummy functions. > > ... are these all the callbacks we need to take care of? What about > > other console drivers that don't register these callbacks? ... > > > > ... for example, mdacon.c? What font does mdacon.c use? I know that > > /lib/fonts/ exports two functions, find_font() and get_default_font(), > > but I don't see them being used in mdacon.c. > > I think all other consoles either don't have fonts at all, or only > support built-in fonts. Ah, I see. I'll search for find_font() and
Re: [External] Re: [PATCH 2/2] sched: mark PRINTK_DEFERRED_CONTEXT_MASK in __schedule()
On Mon 2020-09-28 12:25:59, Peter Zijlstra wrote: > On Mon, Sep 28, 2020 at 06:04:23PM +0800, Chengming Zhou wrote: > > > Well, you are lucky. So it's a problem in our printk implementation. > > Not lucky; I just kicked it in the groin really hard: > > git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git > debug/experimental > > > The deadlock path is: > > > > printk > > vprintk_emit > > console_unlock > > vt_console_print > > hide_cursor > > bit_cursor > > soft_cursor > > queue_work_on > > __queue_work > > try_to_wake_up > > _raw_spin_lock > > native_queued_spin_lock_slowpath > > > > Looks like it's introduced by this commit: > > > > eaa434defaca1781fb2932c685289b610aeb8b4b > > > > "drm/fb-helper: Add fb_deferred_io support" > > Oh gawd, yeah, all the !serial consoles are utter batshit. > > Please look at John's last printk rewrite, IIRC it farms all that off to > a kernel thread instead of doing it from the printk() caller's context. > > I'm not sure where he hides his latests patches, but I'm sure he'll be > more than happy to tell you. AFAIK, John is just working on updating the patchset so that it will be based on the lockless ringbuffer that is finally in the queue for-5.10. Upstreaming the console handling will be the next big step. I am sure that there will be long discussion about it. But there might be few things that would help removing printk_deferred(). 1. Messages will be printed on consoles by dedicated kthreads. It will be safe context. No deadlocks. 2. The registration and unregistration of consoles should not longer be handled by console_lock (semaphore). It should be possible to call most consoles without a sleeping lock. It should remove all these deadlocks between printk() and scheduler(). There might be problems with some consoles. For example, tty would most likely still need a sleeping lock because it is using the console semaphore also internally. 3. We will try harder to get the messages out immediately during panic(). It would take some time until the above reaches upstream. But it seems to be the right way to go. About printk_deferred(): It is a whack a mole game. It is easy to miss printk() that might eventually cause the deadlock. printk deferred context is more safe. But it is still a what a mole game. The kthreads will do the same job for sure. Finally, the deadlock happens "only" when someone is waiting on console_lock() in parallel. Otherwise, the waitqueue for the semaphore is empty and scheduler is not called. It means that there is quite a big change to see the WARN(). It might be even bigger than with printk_deferred() because WARN() in scheduler means that the scheduler is big troubles. Nobody guarantees that the deferred messages will get handled later. Best Regards, Petr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH drm/hisilicon 1/2] drm/hisilicon: Use the same style of variable type in hibmc_drm_de
Consistently Use the same style of variable type in hibmc_drm_de.c. Signed-off-by: Tian Tao --- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 59 +- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c index a3a9e0a..c54f93d 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c @@ -23,15 +23,15 @@ #include "hibmc_drm_regs.h" struct hibmc_display_panel_pll { - unsigned long M; - unsigned long N; - unsigned long OD; - unsigned long POD; + u64 M; + u64 N; + u64 OD; + u64 POD; }; struct hibmc_dislay_pll_config { - unsigned long hdisplay; - unsigned long vdisplay; + u64 hdisplay; + u64 vdisplay; u32 pll1_config_value; u32 pll2_config_value; }; @@ -102,7 +102,7 @@ static void hibmc_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *state = plane->state; u32 reg; s64 gpu_addr = 0; - unsigned int line_l; + u32 line_l; struct hibmc_drm_private *priv = to_hibmc_drm_private(plane->dev); struct drm_gem_vram_object *gbo; @@ -155,10 +155,10 @@ static const struct drm_plane_helper_funcs hibmc_plane_helper_funcs = { .atomic_update = hibmc_plane_atomic_update, }; -static void hibmc_crtc_dpms(struct drm_crtc *crtc, int dpms) +static void hibmc_crtc_dpms(struct drm_crtc *crtc, u32 dpms) { struct hibmc_drm_private *priv = to_hibmc_drm_private(crtc->dev); - unsigned int reg; + u32 reg; reg = readl(priv->mmio + HIBMC_CRT_DISP_CTL); reg &= ~HIBMC_CRT_DISP_CTL_DPMS_MASK; @@ -172,7 +172,7 @@ static void hibmc_crtc_dpms(struct drm_crtc *crtc, int dpms) static void hibmc_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { - unsigned int reg; + u32 reg; struct hibmc_drm_private *priv = to_hibmc_drm_private(crtc->dev); hibmc_set_power_mode(priv, HIBMC_PW_MODE_CTL_MODE_MODE0); @@ -191,7 +191,7 @@ static void hibmc_crtc_atomic_enable(struct drm_crtc *crtc, static void hibmc_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { - unsigned int reg; + u32 reg; struct hibmc_drm_private *priv = to_hibmc_drm_private(crtc->dev); hibmc_crtc_dpms(crtc, HIBMC_CRT_DPMS_OFF); @@ -212,7 +212,7 @@ static enum drm_mode_status hibmc_crtc_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode *mode) { - int i = 0; + u32 i = 0; int vrefresh = drm_mode_vrefresh(mode); if (vrefresh < 59 || vrefresh > 61) @@ -227,9 +227,9 @@ hibmc_crtc_mode_valid(struct drm_crtc *crtc, return MODE_BAD; } -static unsigned int format_pll_reg(void) +static u32 format_pll_reg(void) { - unsigned int pllreg = 0; + u32 pllreg = 0; struct hibmc_display_panel_pll pll = {0}; /* @@ -249,7 +249,7 @@ static unsigned int format_pll_reg(void) return pllreg; } -static void set_vclock_hisilicon(struct drm_device *dev, unsigned long pll) +static void set_vclock_hisilicon(struct drm_device *dev, u64 pll) { u32 val; struct hibmc_drm_private *priv = to_hibmc_drm_private(dev); @@ -279,11 +279,10 @@ static void set_vclock_hisilicon(struct drm_device *dev, unsigned long pll) writel(val, priv->mmio + CRT_PLL1_HS); } -static void get_pll_config(unsigned long x, unsigned long y, - u32 *pll1, u32 *pll2) +static void get_pll_config(u64 x, u64 y, u32 *pll1, u32 *pll2) { - int i; - int count = ARRAY_SIZE(hibmc_pll_table); + u32 i; + u32 count = ARRAY_SIZE(hibmc_pll_table); for (i = 0; i < count; i++) { if (hibmc_pll_table[i].hdisplay == x && @@ -306,11 +305,11 @@ static void get_pll_config(unsigned long x, unsigned long y, * FPGA only supports 7 predefined pixel clocks, and clock select is * in bit 4:0 of new register 0x802a8. */ -static unsigned int display_ctrl_adjust(struct drm_device *dev, - struct drm_display_mode *mode, - unsigned int ctrl) +static u32 display_ctrl_adjust(struct drm_device *dev, + struct drm_display_mode *mode, + u32 ctrl) { - unsigned long x, y; + u64 x, y; u32 pll1; /* bit[31:0] of PLL */ u32 pll2; /* bit[63:32] of PLL */ struct hibmc_drm_private *priv = to_hibmc_drm_private(dev); @@ -358,12 +357,12 @@ static unsigned int display_ctrl_adjust(struct drm_device *dev, static void hibmc_crtc_mode_set_nofb(struct drm_crtc *crtc) { - unsigned int val; + u32 val; struct dr
Re: [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers
On Fri, Sep 25, 2020 at 03:25:51PM +0200, Daniel Vetter wrote: > I think the only way to make this work is that we have one place which > takes in the userspace uapi struct, and then converts it once into a > kernel_console_font. With all the error checking. Hi Daniel, It seems that users don't use `console_font` directly, they use `console_font_op`. Then, in TTY: (drivers/tty/vt/vt.c) int con_font_op(struct vc_data *vc, struct console_font_op *op) { switch (op->op) { case KD_FONT_OP_SET: return con_font_set(vc, op); case KD_FONT_OP_GET: return con_font_get(vc, op); case KD_FONT_OP_SET_DEFAULT: return con_font_default(vc, op); case KD_FONT_OP_COPY: return con_font_copy(vc, op); } return -ENOSYS; } These 4 functions allocate `console_font`. We can replace them with our `kernel_console_font`. So, ... $ vgrep "\.con_font_set" Index FileLine Content 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1294 .con_font_set = sisusbcon_font_set, 1 drivers/usb/misc/sisusbvga/sisusb_con.c 1378 .con_font_set = sisusbdummycon_font_set, 2 drivers/video/console/dummycon.c 162 .con_font_set = dummycon_font_set, 3 drivers/video/console/newport_con.c 693 .con_font_set = newport_font_set, 4 drivers/video/console/vgacon.c 1226 .con_font_set = vgacon_font_set, 5 drivers/video/fbdev/core/fbcon.c3120 .con_font_set = fbcon_set_font, $ $ vgrep "\.con_font_get" Index FileLine Content 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1295 .con_font_get = sisusbcon_font_get, 1 drivers/video/console/vgacon.c 1227 .con_font_get = vgacon_font_get, 2 drivers/video/fbdev/core/fbcon.c3121 .con_font_get = fbcon_get_font, $ $ vgrep "\.con_font_default" Index FileLine Content 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1379 .con_font_default = sisusbdummycon_font_default, 1 drivers/video/console/dummycon.c 163 .con_font_default = dummycon_font_default, 2 drivers/video/console/newport_con.c 694 .con_font_default = newport_font_default, 3 drivers/video/fbdev/core/fbcon.c3122 .con_font_default= fbcon_set_def_font, $ $ vgrep "\.con_font_copy" Index FileLine Content 0 drivers/usb/misc/sisusbvga/sisusb_con.c 1380 .con_font_copy = sisusbdummycon_font_copy, 1 drivers/video/console/dummycon.c 164 .con_font_copy = dummycon_font_copy, 2 drivers/video/fbdev/core/fbcon.c3123 .con_font_copy = fbcon_copy_font, $ _ ... are these all the callbacks we need to take care of? What about other console drivers that don't register these callbacks? ... $ vgrep "\.con_init" Index FileLine Content [...] 3 drivers/usb/misc/sisusbvga/sisusb_con.c 1285 .con_init = sisusbcon_init, 4 drivers/usb/misc/sisusbvga/sisusb_con.c 1369 .con_init = sisusbdummycon_init, 5 drivers/video/console/dummycon.c 153 .con_init = dummycon_init, 6 drivers/video/console/mdacon.c 544 .con_init = mdacon_init, 7 drivers/video/console/newport_con.c 684 .con_init = newport_init, 8 drivers/video/console/sticon.c 328 .con_init= sticon_init, 9 drivers/video/console/vgacon.c 1217 .con_init = vgacon_init, 10 drivers/video/fbdev/core/fbcon.c3111 .con_init = fbcon_init, [...] ... for example, mdacon.c? What font does mdacon.c use? I know that /lib/fonts/ exports two functions, find_font() and get_default_font(), but I don't see them being used in mdacon.c. Ah, and speaking of built-in fonts, see fbcon_startup(): /* Setup default font */ [...] vc->vc_font.charcount = 256; /* FIXME Need to support more fonts */ ^^^ This is because find_font() and get_default_font() return a `struct font_desc *`, but `struct font_desc` doesn't contain `charcount`. I think we also need to add a `charcount` field to `struct font_desc`. > Then all internal code deals in terms of kernel_console_font, with > properly typed and named struct members and helper functions and > everything. And we might need a gradual conversion for this, so that first > we can convert over invidual console drivers, then subsystems, until at > the end we've pushed the conversion from uapi array to kernel_console_font > all the way to the ioctl entry points. Currently `struct vc_data` contains a `struct console_font vc_font`, and I think this is making gradual conversion very hard. As an example, in fbcon_do_set_font(), we update `vc->vc_font`. We lose all the extra
Re: [PATCH 0/3] Prevent out-of-bounds access for built-in font data buffers
On Wed, Sep 30, 2020 at 07:26:52AM +0200, Jiri Slaby wrote: > On 29. 09. 20, 14:34, Peilin Ye wrote: > > the work in general? I couldn't think of how do we clean up subsystems > > one by one, while keeping a `console_font` in `struct vc_data`. > > Hi, > > feel free to change struct vc_data's content as you need, of course. > Only the UAPI _definitions_ have to be preserved (like struct console_font). Ah, I see. Thank you for the reminder! Peilin Ye ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch 00/13] preempt: Make preempt count unconditional
On Wed 16-09-20 23:43:02, Daniel Vetter wrote: > I can > then figure out whether it's better to risk not spotting issues with > call_rcu vs slapping a memalloc_noio_save/restore around all these > critical section which force-degrades any allocation to GFP_ATOMIC at did you mean memalloc_noreclaim_* here? > most, but has the risk that we run into code that assumes "GFP_KERNEL > never fails for small stuff" and has a decidedly less tested fallback > path than rcu code. Even if the above then please note that memalloc_noreclaim_* or PF_MEMALLOC should be used with an extreme care. Essentially only for internal memory reclaimers. It grants access to _all_ the available memory so any abuse can be detrimental to the overall system operation. Allocation failure in this mode means that we are out of memory and any code relying on such an allocation has to carefuly consider failure. This is not a random allocation mode. -- Michal Hocko SUSE Labs ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 80/80] ARM: dts: bcm2711: Enable the display pipeline
On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: > Now that all the drivers have been adjusted for it, let's bring in the > necessary device tree changes. > > The VEC and PV3 are left out for now, since it will require a more specific > clock setup. > > Reviewed-by: Dave Stevenson > Tested-by: Chanwoo Choi > Tested-by: Hoegeun Kwon > Tested-by: Stefan Wahren > Signed-off-by: Maxime Ripard Apologies if this has already been reported or have a solution but this patch (and presumably series) breaks output to the serial console after a certain point during init. On Raspbian, I see systemd startup messages then the output just turns into complete garbage. It looks like this patch is merged first in linux-next, which is why my bisect fell on the DRM merge. I am happy to provide whatever information could be helpful for debugging this. I am on the latest version of the firmware (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). $ git bisect log # bad: [49e7e3e905e437a02782019570f70997e2da9101] Add linux-next specific files for 20200929 # good: [fb0155a09b0224a7147cb07a4ce6034c8d29667f] Merge tag 'nfs-for-5.9-3' of git://git.linux-nfs.org/projects/trondmy/linux-nfs git bisect start '49e7e3e905e437a02782019570f70997e2da9101' 'fb0155a09b0224a7147cb07a4ce6034c8d29667f' # good: [a07bf9042465c0e4ab28947daf70517f99ef021f] Merge remote-tracking branch 'bluetooth/master' into master git bisect good a07bf9042465c0e4ab28947daf70517f99ef021f # bad: [546d06883722dfc5823d53042b924f4b4f76a459] Merge remote-tracking branch 'spi/for-next' into master git bisect bad 546d06883722dfc5823d53042b924f4b4f76a459 # good: [06c14f5c2d311100a447caf60ecbcf558e4e60fe] Merge tag 'mediatek-drm-next-5.10' of https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux into drm-next git bisect good 06c14f5c2d311100a447caf60ecbcf558e4e60fe # bad: [606865c11f2ed6429c7eddcbc59d3295771d41a4] Merge remote-tracking branch 'sound-asoc/for-next' into master git bisect bad 606865c11f2ed6429c7eddcbc59d3295771d41a4 # bad: [7492f2f52acc72a1d08ad1f1d722237ba66189b9] Merge remote-tracking branch 'regmap/for-next' into master git bisect bad 7492f2f52acc72a1d08ad1f1d722237ba66189b9 # good: [0b7e44d39c8aa7536352b57af2265e92fc253e4f] integrity: Asymmetric digsig supports SM2-with-SM3 algorithm git bisect good 0b7e44d39c8aa7536352b57af2265e92fc253e4f # good: [2ce595ba1cd7a2bc053fcc937b7bbbf743c21818] Merge remote-tracking branch 'nand/nand/next' into master git bisect good 2ce595ba1cd7a2bc053fcc937b7bbbf743c21818 # good: [10e07ca312548f90d5e0fc1d862209285c9a858c] gpu/drm/radeon: fix spelling typo in comments git bisect good 10e07ca312548f90d5e0fc1d862209285c9a858c # bad: [be877462417f05af69729a9eeda1332b15e81de8] Merge remote-tracking branch 'imx-drm/imx-drm/next' into master git bisect bad be877462417f05af69729a9eeda1332b15e81de8 # bad: [7a80fa2ada067aa633a91bce67f6c3e39aad7504] Merge remote-tracking branch 'drm-intel/for-linux-next' into master git bisect bad 7a80fa2ada067aa633a91bce67f6c3e39aad7504 # good: [f4a336053725b689a65021edca26f8d058c0d6b4] drm/amdgpu/display: fix CFLAGS setup for DCN30 git bisect good f4a336053725b689a65021edca26f8d058c0d6b4 # bad: [64e05a0ebd7e6047c9f67c685fe8d4ec79a397ba] Merge remote-tracking branch 'drm/drm-next' into master git bisect bad 64e05a0ebd7e6047c9f67c685fe8d4ec79a397ba # good: [a4b0c1f80de8ec3f473b02918556650b683f044d] Merge remote-tracking branch 'crypto/master' into master git bisect good a4b0c1f80de8ec3f473b02918556650b683f044d # first bad commit: [64e05a0ebd7e6047c9f67c685fe8d4ec79a397ba] Merge remote-tracking branch 'drm/drm-next' into master Cheers, Nathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH rdma-next v4 4/4] RDMA/umem: Move to allocate SG table from pages
On Sun, Sep 27, 2020 at 09:46:47AM +0300, Leon Romanovsky wrote: > @@ -296,11 +223,17 @@ static struct ib_umem *__ib_umem_get(struct ib_device > *device, > goto umem_release; > > cur_base += ret * PAGE_SIZE; > - npages -= ret; > - > - sg = ib_umem_add_sg_table(sg, page_list, ret, > - dma_get_max_seg_size(device->dma_device), > - &umem->sg_nents); > + npages -= ret; > + sg = __sg_alloc_table_from_pages( > + &umem->sg_head, page_list, ret, 0, ret << PAGE_SHIFT, > + dma_get_max_seg_size(device->dma_device), sg, npages, > + GFP_KERNEL); > + umem->sg_nents = umem->sg_head.nents; > + if (IS_ERR(sg)) { > + unpin_user_pages_dirty_lock(page_list, ret, 0); > + ret = PTR_ERR(sg); > + goto umem_release; > + } > } > > sg_mark_end(sg); Does it still need the sg_mark_end? Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dp: add voltage corners voting support base on dp link rate
Set link rate by using OPP set rate api so that CX level will be set accordingly base on the link rate. Signed-off-by: Kuogee Hsieh --- drivers/gpu/drm/msm/dp/dp_ctrl.c| 33 ++- drivers/gpu/drm/msm/dp/dp_ctrl.h| 2 +- drivers/gpu/drm/msm/dp/dp_display.c | 8 +++--- drivers/gpu/drm/msm/dp/dp_power.c | 42 ++--- drivers/gpu/drm/msm/dp/dp_power.h | 2 +- 5 files changed, 77 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index 2e3e1917351f..e1595d829e04 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -76,6 +77,8 @@ struct dp_ctrl_private { struct dp_parser *parser; struct dp_catalog *catalog; + struct opp_table *opp_table; + struct completion idle_comp; struct completion video_comp; }; @@ -1836,6 +1839,7 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, struct dp_parser *parser) { struct dp_ctrl_private *ctrl; + int ret; if (!dev || !panel || !aux || !link || !catalog) { @@ -1849,6 +1853,21 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, return ERR_PTR(-ENOMEM); } + ctrl->opp_table = dev_pm_opp_set_clkname(dev, "ctrl_link"); + + if (IS_ERR(ctrl->opp_table)) { + dev_err(dev, "invalid DP OPP table in device tree\n"); + ctrl->opp_table = NULL; + } else { + /* OPP table is optional */ + ret = dev_pm_opp_of_add_table(dev); + if (ret && ret != -ENODEV) { + dev_err(dev, "add DP OPP table\n"); + dev_pm_opp_put_clkname(ctrl->opp_table); + ctrl->opp_table = NULL; + } + } + init_completion(&ctrl->idle_comp); init_completion(&ctrl->video_comp); @@ -1864,6 +1883,18 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, return &ctrl->dp_ctrl; } -void dp_ctrl_put(struct dp_ctrl *dp_ctrl) +void dp_ctrl_put(struct device *dev, struct dp_ctrl *dp_ctrl) { + struct dp_ctrl_private *ctrl; + + if (!dp_ctrl) + return; + + ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); + + if (ctrl->opp_table != NULL) { + dev_pm_opp_of_remove_table(dev); + dev_pm_opp_put_clkname(ctrl->opp_table); + ctrl->opp_table = NULL; + } } diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h index f60ba93c8678..19b412a93e02 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h @@ -31,6 +31,6 @@ struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, struct dp_panel *panel, struct drm_dp_aux *aux, struct dp_power *power, struct dp_catalog *catalog, struct dp_parser *parser); -void dp_ctrl_put(struct dp_ctrl *dp_ctrl); +void dp_ctrl_put(struct device *dev, struct dp_ctrl *dp_ctrl); #endif /* _DP_CTRL_H_ */ diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 431dff9de797..be941eedf4c6 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -648,8 +648,10 @@ static int dp_irq_hpd_handle(struct dp_display_private *dp, u32 data) static void dp_display_deinit_sub_modules(struct dp_display_private *dp) { + struct device *dev = &dp->pdev->dev; + dp_debug_put(dp->debug); - dp_ctrl_put(dp->ctrl); + dp_ctrl_put(dev, dp->ctrl); dp_panel_put(dp->panel); dp_aux_put(dp->aux); dp_audio_put(dp->audio); @@ -693,7 +695,7 @@ static int dp_init_sub_modules(struct dp_display_private *dp) goto error; } - dp->power = dp_power_get(dp->parser); + dp->power = dp_power_get(dev, dp->parser); if (IS_ERR(dp->power)) { rc = PTR_ERR(dp->power); DRM_ERROR("failed to initialize power, rc = %d\n", rc); @@ -749,7 +751,7 @@ static int dp_init_sub_modules(struct dp_display_private *dp) return rc; error_audio: - dp_ctrl_put(dp->ctrl); + dp_ctrl_put(dev, dp->ctrl); error_ctrl: dp_panel_put(dp->panel); error_link: diff --git a/drivers/gpu/drm/msm/dp/dp_power.c b/drivers/gpu/drm/msm/dp/dp_power.c index 17c1fc6a2d44..3d75bf09e38f 100644 --- a/drivers/gpu/drm/msm/dp/dp_power.c +++ b/drivers/gpu/drm/msm/dp/dp_power.c @@ -8,12 +8,14 @@ #include #include #include +#include #include "dp_power.h" #include "msm_drv.h" struct dp_power_private { struct dp_parser *parser; struct platform_device *pdev; + struct device *dev; struct cl
[PATCH drm/hisilicon 0/2] Use the same style of variable type
patch #1 and #2 Use the same style of variable type in hisilicon drm driver and both are clean up, no actual functional changes. Tian Tao (2): drm/hisilicon: Use the same style of variable type in hibmc_drm_de drm/hisilicon: Use the same style of variable type in hibmc_drm_drv drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 59 - drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 13 +++--- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 8 ++-- 3 files changed, 39 insertions(+), 41 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] i915: Introduce quirk for shifting eDP brightness.
Thank you for the reply. And in regards to digging into it further, thanks for requesting that I do some more due diligence here :) Also if you did get around to it, thanks for double-checking with Bill! Let me know if you'd like me to reach out instead, or if anything else needs to be done in this regard. So to clarify the plan: if we do actually move forwards with leaving the current functionality as the default, do we need to have the complete list of devices which need the quirk applied when the patch first goes in? From my perspective, we definitely have one device which needs the quirk (and preferably, it'd be good to do it sooner than later so that we can get this bugfix out to our users) and an unknowable number of others. Would it be OK to introduce the quirk for just Pixelbook and to follow up to add others once we have that list? It may take a good amount of time for me to herd the cats inside Google, especially given there's a chance that there are affected laptops and that no one has noticed (e.g., I almost didn't notice with the Pixelbook). Given Lyude's analysis it seems like Chrome OS devices may be the largest affected group here, so I am incentivized to not drop the ball after fixing my immediate Pixelbook problem :) On Fri, Sep 25, 2020 at 10:53 AM Lyude Paul wrote: > > On Thu, 2020-09-24 at 17:46 -0600, Kevin Chowski wrote: > > cc back a few others who were unintentionally dropped from the thread > > earlier. > > > > Someone (at Google) helped me dig into this a little more and they > > found a document titled "eDP Backlight Brightness bit alignment" sent > > out for review in January 2017. I registered for a new account (google > > is a member) and have access to the document; here is the URL for > > those who also have access: > > https://groups.vesa.org/wg/AllMem/document/7786. For what it's worth, > > it seems like Lyude's contact Bill Lempesis uploaded this > > change-request document, so I think we can reach out to him if we have > > further questions. It's actually unclear to me what the status of this > > change request is, and whether it's been officially accepted. But I > > think it can be seen as some official advice on how we can move > > forward here. > > > > Basically, this is a change request to the spec which acknowledges > > that, despite the original spec implying that the > > most-significant-bits were relevant here, many implementations used > > the least-significant-bits. In defense of most-significant it laid out > > some similar arguments to what Ville was saying. But it ends up > > saying: > > > > > Unfortunately, the most common interpretation that we have > > > encountered is case 1 in the example above. TCON vendors > > > tend to align the valid bits to the LSBs, not the MSBs. > > > > Instead of changing the default defined functionality (as some earlier > > version of this doc apparently suggested), this doc prefers to > > allocate two extra bits in EDP_GENERAL_CAPABILITY_2 so that future > > backlight devices can specify to the Source how to program them: > > > > 00: the current functionality, i,e. no defined interpretation > > 01: aligned to most-significant bits > > 10: aligned to least-significant bits > > 11: reserved > > > > It also says "[Sources] should only need panel-specific workarounds > > for the currently available panels." > > > > So I believe this document is an acknowledgement of many > > implementations having their alignment to the least-significant bits, > > and (to my eyes) clearly validates that the spec "should" be the > > opposite. If we believe the doc's claim that "the most common > > interpretation" is least-significant, it seems to me that it would > > require more quirks if we made most-significant the default > > implementation. > > > > Ville mentioned at some point earlier that we should try to match the > > spec, whereas Lyude mentioned we should prefer to do what the majority > > of machines do. What do you both think of this new development? > > That's how displayport happens to be sometimes :). Definitely isn't the first > time something in the spec like this got implemented incorrectly so many times > by different vendors that they had to update the spec in response (same thing > happened with MST and interleaved sideband messages as of DP 2.0), so I'm > really glad we went and actually investigated this. > > So yes - I think a quirk for this would definitely be a good idea, and IMO we > should always lean on the side that more panels implement even if it's not > according to spec - so defaulting to the behavior we currently have in the > kernel, and adding quirks for panels that were smart enough not to fall for > this would probably be the best way to go. That just leaves the challenge of > "how do we figure out which panels actually need this and which ones don't?" > > This might end up being a bit of a challenge, but I've got some ideas on how > we might be able to tackle it to the best of our ability based off my >
Re: [RFC][PATCH 5/6] dma-buf: system_heap: Add pagepool support to system heap
On 2020-09-25 21:24, John Stultz wrote: Reuse/abuse the pagepool code from the network code to speed up allocation performance. This is similar to the ION pagepool usage, but tries to utilize generic code instead of a custom implementation. Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz --- drivers/dma-buf/heaps/Kconfig | 1 + drivers/dma-buf/heaps/system_heap.c | 32 + 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig index a5eef06c4226..f13cde4321b1 100644 --- a/drivers/dma-buf/heaps/Kconfig +++ b/drivers/dma-buf/heaps/Kconfig @@ -1,6 +1,7 @@ config DMABUF_HEAPS_SYSTEM bool "DMA-BUF System Heap" depends on DMABUF_HEAPS + select PAGE_POOL help Choose this option to enable the system dmabuf heap. The system heap is backed by pages from the buddy allocator. If in doubt, say Y. diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index 882a632e9bb7..9f57b4c8ae69 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -20,6 +20,7 @@ #include #include #include +#include struct dma_heap *sys_heap; @@ -46,6 +47,7 @@ struct dma_heap_attachment { static gfp_t order_flags[] = {HIGH_ORDER_GFP, LOW_ORDER_GFP, LOW_ORDER_GFP}; static const unsigned int orders[] = {8, 4, 0}; #define NUM_ORDERS ARRAY_SIZE(orders) +struct page_pool *pools[NUM_ORDERS]; static struct sg_table *dup_sg_table(struct sg_table *table) { @@ -264,13 +266,17 @@ static void system_heap_dma_buf_release(struct dma_buf *dmabuf) struct system_heap_buffer *buffer = dmabuf->priv; struct sg_table *table; struct scatterlist *sg; - int i; + int i, j; table = &buffer->sg_table; for_each_sg(table->sgl, sg, table->nents, i) { struct page *page = sg_page(sg); - __free_pages(page, compound_order(page)); + for (j = 0; j < NUM_ORDERS; j++) { + if (compound_order(page) == orders[j]) + break; + } + page_pool_put_full_page(pools[j], page, false); } sg_free_table(table); kfree(buffer); @@ -300,8 +306,7 @@ static struct page *alloc_largest_available(unsigned long size, continue; if (max_order < orders[i]) continue; - - page = alloc_pages(order_flags[i], orders[i]); + page = page_pool_alloc_pages(pools[i], order_flags[i]); if (!page) continue; return page; @@ -406,6 +411,25 @@ static const struct dma_heap_ops system_heap_ops = { static int system_heap_create(void) { struct dma_heap_export_info exp_info; + int i; + + for (i = 0; i < NUM_ORDERS; i++) { + struct page_pool_params pp; + + memset(&pp, 0, sizeof(pp)); + pp.order = orders[i]; + pp.dma_dir = DMA_BIDIRECTIONAL; + pools[i] = page_pool_create(&pp); + + if (IS_ERR(pools[i])) { + int j; + + pr_err("%s: page pool creation failed!\n", __func__); + for (j = 0; j < i; j++) + page_pool_destroy(pools[j]); + return PTR_ERR(pools[i]); + } + } exp_info.name = "system"; exp_info.ops = &system_heap_ops; This is cool, I didn't know about this pooling code under /net/core. Nice and compact. -- The Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [patch 00/13] preempt: Make preempt count unconditional
On Tue 29-09-20 11:00:03, Daniel Vetter wrote: > On Tue, Sep 29, 2020 at 10:19:38AM +0200, Michal Hocko wrote: > > On Wed 16-09-20 23:43:02, Daniel Vetter wrote: > > > I can > > > then figure out whether it's better to risk not spotting issues with > > > call_rcu vs slapping a memalloc_noio_save/restore around all these > > > critical section which force-degrades any allocation to GFP_ATOMIC at > > > > did you mean memalloc_noreclaim_* here? > > Yeah I picked the wrong one of that family of functions. > > > > most, but has the risk that we run into code that assumes "GFP_KERNEL > > > never fails for small stuff" and has a decidedly less tested fallback > > > path than rcu code. > > > > Even if the above then please note that memalloc_noreclaim_* or > > PF_MEMALLOC should be used with an extreme care. Essentially only for > > internal memory reclaimers. It grants access to _all_ the available > > memory so any abuse can be detrimental to the overall system operation. > > Allocation failure in this mode means that we are out of memory and any > > code relying on such an allocation has to carefuly consider failure. > > This is not a random allocation mode. > > Agreed, that's why I don't like having these kind of automagic critical > sections. It's a bit a shotgun approach. Paul said that the code would > handle failures, but the problem is that it applies everywhere. Ohh, in the ideal world we wouldn't need anything like that. But then the reality fires: * PF_MEMALLOC (resp memalloc_noreclaim_* for that matter) is primarily used to make sure that allocations from inside the memory reclaim - yeah that happens - will not recurse. * PF_MEMALLOC_NO{FS,IO} (resp memalloc_no{fs,io}*) are used to mark no fs/io reclaim recursion critical sections because controling that for each allocation inside fs transaction (or other sensitive) or IO contexts turned out to be unmaintainable and people simply fallen into using NOFS/NOIO unconditionally which is causing reclaim imbalance problems. * PF_MEMALLOC_NOCMA (resp memalloc_nocma*) is used for long term pinning when CMA pages cannot be pinned because that would break the CMA guarantees. Communicating this to all potential allocations during pinning is simply unfeasible. So you are absolutely right that these critical sections with side effects on all allocations are far from ideal from the API point of view but they are mostly mirroring a demand for functionality which is _practically_ impossible to achieve with our current code base. Not that we couldn't get back to drawing board and come up with a saner thing and rework the world... -- Michal Hocko SUSE Labs ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v7 1/6] dt-bindings: display: add Unisoc's drm master bindings
On Tue, 29 Sep 2020 at 14:35, Kevin Tang wrote: > > Hi Rob, > Component framework include master and component, here is master subnode. > It seems that everyone else does it, why not me? > > Your comments on v6: > "We generally try to avoid this virtual node as it doesn't represent > any h/w. Can't you bind the driver to the DPU directly?" > > I'm sorry, maybe is my careless, I still don't understand why and how to do it Devicetree is used to describe hardware rather than virtual things like "sprd,display-subsystem" which is not a real device. That's what I understand for Rob's comments here. Chunyan > > Rob Herring 于2020年9月29日周二 上午12:28写道: > > > > > On Mon, Sep 28, 2020 at 3:17 AM Maxime Ripard wrote: > > > > > > Hi! > > > > > > On Mon, Sep 28, 2020 at 02:27:35PM +0800, Kevin Tang wrote: > > > > From: Kevin Tang > > > > > > > > The Unisoc DRM master device is a virtual device needed to list all > > > > DPU devices or other display interface nodes that comprise the > > > > graphics subsystem > > > > > > > > RFC v7: > > > > - Fix DTC unit name warnings > > > > - Fix the problem of maintainers > > > > > > > > Cc: Orson Zhai > > > > Cc: Chunyan Zhang > > > > Signed-off-by: Kevin Tang > > > > --- > > > > .../display/sprd/sprd,display-subsystem.yaml | 39 > > > > ++ > > > > 1 file changed, 39 insertions(+) > > > > create mode 100644 > > > > Documentation/devicetree/bindings/display/sprd/sprd,display-subsystem.yaml > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/display/sprd/sprd,display-subsystem.yaml > > > > > > > > b/Documentation/devicetree/bindings/display/sprd/sprd,display-subsystem.yaml > > > > new file mode 100644 > > > > index 000..9487a39 > > > > --- /dev/null > > > > +++ > > > > b/Documentation/devicetree/bindings/display/sprd/sprd,display-subsystem.yaml > > > > @@ -0,0 +1,39 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: > > > > http://devicetree.org/schemas/display/sprd/sprd,display-subsystem.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Unisoc DRM master device > > > > + > > > > +maintainers: > > > > + - Kevin Tang > > > > + > > > > +description: | > > > > + The Unisoc DRM master device is a virtual device needed to list all > > > > + DPU devices or other display interface nodes that comprise the > > > > + graphics subsystem. > > > > + > > > > +properties: > > > > + compatible: > > > > +const: sprd,display-subsystem > > > > + > > > > + ports: > > > > +$ref: /schemas/types.yaml#/definitions/phandle-array > > > > +description: > > > > + Should contain a list of phandles pointing to display interface > > > > port > > > > + of DPU devices. > > > > > > Generally speaking, driver-specific properties should be prefixed by the > > > vendor name to avoid any conflict with generic properties (like the > > > OF-Graph ports subnode in this case) > > > > We try to avoid this virtual node altogether which I commented about > > on v6 which was ignored. > > > > Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Linaro-mm-sig] [PATCH v5 05/38] drm: prime: use sgtable iterators in drm_prime_sg_to_page_addr_arrays()
Hi All, On 25.09.2020 23:23, Alex Deucher wrote: > On Tue, Sep 22, 2020 at 2:28 AM Marek Szyprowski > wrote: >> On 22.09.2020 01:15, Alex Goins wrote: >>> Tested-by: Alex Goins >>> >>> This change fixes a regression with drm_prime_sg_to_page_addr_arrays() and >>> AMDGPU in v5.9. >> Thanks for testing! >> >>> Commit 39913934 similarly revamped AMDGPU to use sgtable helper functions. >>> When >>> it changed from dma_map_sg_attrs() to dma_map_sgtable(), as a side effect it >>> started correctly updating sgt->nents to the return value of >>> dma_map_sg_attrs(). >>> However, drm_prime_sg_to_page_addr_arrays() incorrectly uses sgt->nents to >>> iterate over pages, rather than sgt->orig_nents, resulting in it now >>> returning >>> the incorrect number of pages on AMDGPU. >>> >>> I had written a patch that changes drm_prime_sg_to_page_addr_arrays() to use >>> for_each_sgtable_sg() instead of for_each_sg(), iterating using >>> sgt->orig_nents: >>> >>> - for_each_sg(sgt->sgl, sg, sgt->nents, count) { >>> + for_each_sgtable_sg(sgt, sg, count) { >>> >>> This patch takes it further, but still has the effect of fixing the number >>> of >>> pages that drm_prime_sg_to_page_addr_arrays() returns. Something like this >>> should be included in v5.9 to prevent a regression with AMDGPU. >> Probably the easiest way to handle a fix for v5.9 would be to simply >> merge the latest version of this patch also to v5.9-rcX: >> https://lore.kernel.org/dri-devel/20200904131711.12950-3-m.szyprow...@samsung.com/ >> >> >> This way we would get it fixed and avoid possible conflict in the -next. >> Do you have any AMDGPU fixes for v5.9 in the queue? Maybe you can add >> that patch to the queue? Dave: would it be okay that way? > I think this should go into drm-misc for 5.9 since it's an update to > drm_prime.c. Is that patch ready to merge? > Acked-by: Alex Deucher Maarten, Maxime or Thomas: could you take this one: https://lore.kernel.org/dri-devel/20200904131711.12950-3-m.szyprow...@samsung.com/ also to drm-misc-fixes for v5.9-rc? Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: fix regression in ttm moves
just FYI I'm seeing a regression on vmwgfx with drm-fixes and drm-next merged into it. I'm going take some time to dig through and work out where, the regression is a command failure and a ioremap failure. Dave. On Wed, 30 Sep 2020 at 16:26, Dave Airlie wrote: > > Uggh this is part of the mess with the revert, I'm not sure how best > to dig out of this one yet. > > Dave. > > On Wed, 30 Sep 2020 at 15:55, Dave Airlie wrote: > > > > From: Dave Airlie > > > > This fixes a bug introduced in be1213a341a289afc51f89181c310e368fba0b66 > > drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2 > > > > On vmwgfx this causes a Command buffer error WARN to trigger. > > > > This is because the old code used to check if bo->ttm was true, > > and the new code doesn't, fix it code to add back the check resolves > > the issue. > > > > Fixes: be1213a341a2 ("drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED v2") > > Signed-off-by: Dave Airlie > > --- > > drivers/gpu/drm/ttm/ttm_bo.c | 8 +--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > > index 70b3bee27850..e8aa2fe8e9d1 100644 > > --- a/drivers/gpu/drm/ttm/ttm_bo.c > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > > @@ -251,9 +251,11 @@ static int ttm_bo_handle_move_mem(struct > > ttm_buffer_object *bo, > > /* Zero init the new TTM structure if the old location > > should > > * have used one as well. > > */ > > - ret = ttm_tt_create(bo, old_man->use_tt); > > - if (ret) > > - goto out_err; > > + if (!bo->ttm) { > > + ret = ttm_tt_create(bo, old_man->use_tt); > > + if (ret) > > + goto out_err; > > + } > > > > ret = ttm_tt_set_placement_caching(bo->ttm, mem->placement); > > if (ret) > > -- > > 2.20.1 > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel