[PATCH v10 6/8] udmabuf: Convert udmabuf driver to use folios (v2)
This is mainly a preparatory patch to use memfd_pin_folios() API for pinning folios. Using folios instead of pages makes sense as the udmabuf driver needs to handle both shmem and hugetlb cases. However, the function vmap_udmabuf() still needs a list of pages; so, we collect all the head pages into a local array in this case. Other changes in this patch include the addition of helpers for checking the memfd seals and exporting dmabuf. Moving code from udmabuf_create() into these helpers improves readability given that udmabuf_create() is a bit long. v2: (Matthew) - Use folio_pfn() on the folio instead of page_to_pfn() on head page - Don't split the arguments to shmem_read_folio() on multiple lines Cc: David Hildenbrand Cc: Matthew Wilcox Cc: Daniel Vetter Cc: Mike Kravetz Cc: Hugh Dickins Cc: Peter Xu Cc: Jason Gunthorpe Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Signed-off-by: Vivek Kasireddy --- drivers/dma-buf/udmabuf.c | 140 ++ 1 file changed, 83 insertions(+), 57 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 274defd3fa3e..a8f3af61f7f2 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -26,7 +26,7 @@ MODULE_PARM_DESC(size_limit_mb, "Max size of a dmabuf, in megabytes. Default is struct udmabuf { pgoff_t pagecount; - struct page **pages; + struct folio **folios; struct sg_table *sg; struct miscdevice *device; pgoff_t *offsets; @@ -42,7 +42,7 @@ static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) if (pgoff >= ubuf->pagecount) return VM_FAULT_SIGBUS; - pfn = page_to_pfn(ubuf->pages[pgoff]); + pfn = folio_pfn(ubuf->folios[pgoff]); pfn += ubuf->offsets[pgoff] >> PAGE_SHIFT; return vmf_insert_pfn(vma, vmf->address, pfn); @@ -68,11 +68,21 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) { struct udmabuf *ubuf = buf->priv; + struct page **pages; void *vaddr; + pgoff_t pg; dma_resv_assert_held(buf->resv); - vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1); + pages = kmalloc_array(ubuf->pagecount, sizeof(*pages), GFP_KERNEL); + if (!pages) + return -ENOMEM; + + for (pg = 0; pg < ubuf->pagecount; pg++) + pages[pg] = >folios[pg]->page; + + vaddr = vm_map_ram(pages, ubuf->pagecount, -1); + kfree(pages); if (!vaddr) return -EINVAL; @@ -107,7 +117,8 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, goto err_alloc; for_each_sg(sg->sgl, sgl, ubuf->pagecount, i) - sg_set_page(sgl, ubuf->pages[i], PAGE_SIZE, ubuf->offsets[i]); + sg_set_folio(sgl, ubuf->folios[i], PAGE_SIZE, +ubuf->offsets[i]); ret = dma_map_sgtable(dev, sg, direction, 0); if (ret < 0) @@ -152,9 +163,9 @@ static void release_udmabuf(struct dma_buf *buf) put_sg_table(dev, ubuf->sg, DMA_BIDIRECTIONAL); for (pg = 0; pg < ubuf->pagecount; pg++) - put_page(ubuf->pages[pg]); + folio_put(ubuf->folios[pg]); kfree(ubuf->offsets); - kfree(ubuf->pages); + kfree(ubuf->folios); kfree(ubuf); } @@ -215,36 +226,33 @@ static int handle_hugetlb_pages(struct udmabuf *ubuf, struct file *memfd, pgoff_t mapidx = offset >> huge_page_shift(hpstate); pgoff_t subpgoff = (offset & ~huge_page_mask(hpstate)) >> PAGE_SHIFT; pgoff_t maxsubpgs = huge_page_size(hpstate) >> PAGE_SHIFT; - struct page *hpage = NULL; - struct folio *folio; + struct folio *folio = NULL; pgoff_t pgidx; mapidx <<= huge_page_order(hpstate); for (pgidx = 0; pgidx < pgcnt; pgidx++) { - if (!hpage) { + if (!folio) { folio = __filemap_get_folio(memfd->f_mapping, mapidx, FGP_ACCESSED, 0); if (IS_ERR(folio)) return PTR_ERR(folio); - - hpage = >page; } - get_page(hpage); - ubuf->pages[*pgbuf] = hpage; + folio_get(folio); + ubuf->folios[*pgbuf] = folio; ubuf->offsets[*pgbuf] = subpgoff << PAGE_SHIFT; (*pgbuf)++; if (++subpgoff == maxsubpgs) { - put_page(hpage); - hpage = NULL; + folio_put(folio); + folio = NULL; subpgoff = 0; mapidx += pages_per_huge_page(hpstate); } } -
[PATCH v10 4/8] mm/gup: Introduce check_and_migrate_movable_folios()
This helper is the folio equivalent of check_and_migrate_movable_pages(). Therefore, all the rules that apply to check_and_migrate_movable_pages() also apply to this one as well. Currently, this helper is only used by memfd_pin_folios(). This patch also includes changes to rename and convert the internal functions collect_longterm_unpinnable_pages() and migrate_longterm_unpinnable_pages() to work on folios. Since they are also used by check_and_migrate_movable_pages(), a temporary array is used to collect and share the folios with these functions. Cc: David Hildenbrand Cc: Matthew Wilcox Cc: Christoph Hellwig Cc: Jason Gunthorpe Cc: Peter Xu Suggested-by: David Hildenbrand Signed-off-by: Vivek Kasireddy --- mm/gup.c | 129 +++ 1 file changed, 92 insertions(+), 37 deletions(-) diff --git a/mm/gup.c b/mm/gup.c index 4d7bc4453819..00b24a429ba8 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -2097,20 +2097,24 @@ struct page *get_dump_page(unsigned long addr) #ifdef CONFIG_MIGRATION /* - * Returns the number of collected pages. Return value is always >= 0. + * Returns the number of collected folios. Return value is always >= 0. */ -static unsigned long collect_longterm_unpinnable_pages( - struct list_head *movable_page_list, - unsigned long nr_pages, +static unsigned long collect_longterm_unpinnable_folios( + struct list_head *movable_folio_list, + unsigned long nr_folios, + struct folio **folios, struct page **pages) { unsigned long i, collected = 0; struct folio *prev_folio = NULL; bool drain_allow = true; + struct folio *folio; - for (i = 0; i < nr_pages; i++) { - struct folio *folio = page_folio(pages[i]); + for (i = 0; i < nr_folios; i++) { + if (pages) + folios[i] = page_folio(pages[i]); + folio = folios[i]; if (folio == prev_folio) continue; prev_folio = folio; @@ -2124,7 +2128,7 @@ static unsigned long collect_longterm_unpinnable_pages( continue; if (folio_test_hugetlb(folio)) { - isolate_hugetlb(folio, movable_page_list); + isolate_hugetlb(folio, movable_folio_list); continue; } @@ -2136,7 +2140,7 @@ static unsigned long collect_longterm_unpinnable_pages( if (!folio_isolate_lru(folio)) continue; - list_add_tail(>lru, movable_page_list); + list_add_tail(>lru, movable_folio_list); node_stat_mod_folio(folio, NR_ISOLATED_ANON + folio_is_file_lru(folio), folio_nr_pages(folio)); @@ -2146,27 +2150,28 @@ static unsigned long collect_longterm_unpinnable_pages( } /* - * Unpins all pages and migrates device coherent pages and movable_page_list. - * Returns -EAGAIN if all pages were successfully migrated or -errno for failure - * (or partial success). + * Unpins all folios and migrates device coherent folios and movable_folio_list. + * Returns -EAGAIN if all folios were successfully migrated or -errno for + * failure (or partial success). */ -static int migrate_longterm_unpinnable_pages( - struct list_head *movable_page_list, - unsigned long nr_pages, - struct page **pages) +static int migrate_longterm_unpinnable_folios( + struct list_head *movable_folio_list, + unsigned long nr_folios, + struct folio **folios) { int ret; unsigned long i; - for (i = 0; i < nr_pages; i++) { - struct folio *folio = page_folio(pages[i]); + for (i = 0; i < nr_folios; i++) { + struct folio *folio = folios[i]; if (folio_is_device_coherent(folio)) { /* -* Migration will fail if the page is pinned, so convert -* the pin on the source page to a normal reference. +* Migration will fail if the folio is pinned, so +* convert the pin on the source folio to a normal +* reference. */ - pages[i] = NULL; + folios[i] = NULL; folio_get(folio); gup_put_folio(folio, 1, FOLL_PIN); @@ -2179,23 +2184,23 @@ static int migrate_longterm_unpinnable_pages(
[PATCH v10 3/8] mm/gup: Introduce unpin_folio/unpin_folios helpers
These helpers are the folio versions of unpin_user_page/unpin_user_pages. They are currently only useful for unpinning folios pinned by memfd_pin_folios() or other associated routines. However, they could find new uses in the future, when more and more folio-only helpers are added to GUP. Cc: David Hildenbrand Cc: Matthew Wilcox Cc: Christoph Hellwig Cc: Jason Gunthorpe Cc: Peter Xu Suggested-by: David Hildenbrand Signed-off-by: Vivek Kasireddy --- include/linux/mm.h | 2 ++ mm/gup.c | 81 -- 2 files changed, 74 insertions(+), 9 deletions(-) diff --git a/include/linux/mm.h b/include/linux/mm.h index 418d26608ece..4dbfdd277014 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1585,11 +1585,13 @@ static inline void put_page(struct page *page) #define GUP_PIN_COUNTING_BIAS (1U << 10) void unpin_user_page(struct page *page); +void unpin_folio(struct folio *folio); void unpin_user_pages_dirty_lock(struct page **pages, unsigned long npages, bool make_dirty); void unpin_user_page_range_dirty_lock(struct page *page, unsigned long npages, bool make_dirty); void unpin_user_pages(struct page **pages, unsigned long npages); +void unpin_folios(struct folio **folios, unsigned long nfolios); static inline bool is_cow_mapping(vm_flags_t flags) { diff --git a/mm/gup.c b/mm/gup.c index 231711efa390..4d7bc4453819 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -30,6 +30,23 @@ struct follow_page_context { unsigned int page_mask; }; +static inline void sanity_check_pinned_folios(struct folio **folios, + unsigned long nfolios) +{ + if (!IS_ENABLED(CONFIG_DEBUG_VM)) + return; + + for (; nfolios; nfolios--, folios++) { + struct folio *folio = *folios; + + if (is_zero_folio(folio) || + !folio_test_anon(folio)) + continue; + + VM_BUG_ON_FOLIO(!PageAnonExclusive(>page), folio); + } +} + static inline void sanity_check_pinned_pages(struct page **pages, unsigned long npages) { @@ -52,15 +69,11 @@ static inline void sanity_check_pinned_pages(struct page **pages, struct page *page = *pages; struct folio *folio = page_folio(page); - if (is_zero_page(page) || - !folio_test_anon(folio)) - continue; - if (!folio_test_large(folio) || folio_test_hugetlb(folio)) - VM_BUG_ON_PAGE(!PageAnonExclusive(>page), page); - else - /* Either a PTE-mapped or a PMD-mapped THP. */ - VM_BUG_ON_PAGE(!PageAnonExclusive(>page) && - !PageAnonExclusive(page), page); + sanity_check_pinned_folios(, 1); + + /* Either a PTE-mapped or a PMD-mapped THP. */ + if (folio_test_large(folio) && !folio_test_hugetlb(folio)) + VM_BUG_ON_PAGE(!PageAnonExclusive(page), page); } } @@ -276,6 +289,21 @@ void unpin_user_page(struct page *page) } EXPORT_SYMBOL(unpin_user_page); +/** + * unpin_folio() - release a dma-pinned folio + * @folio: pointer to folio to be released + * + * Folios that were pinned via memfd_pin_folios() or other similar routines + * must be released either using unpin_folio() or unpin_folios(). This is so + * that such folios can be separately tracked and uniquely handled. + */ +void unpin_folio(struct folio *folio) +{ + sanity_check_pinned_folios(, 1); + gup_put_folio(folio, 1, FOLL_PIN); +} +EXPORT_SYMBOL(unpin_folio); + /** * folio_add_pin - Try to get an additional pin on a pinned folio * @folio: The folio to be pinned @@ -488,6 +516,41 @@ void unpin_user_pages(struct page **pages, unsigned long npages) } EXPORT_SYMBOL(unpin_user_pages); +/** + * unpin_folios() - release an array of gup-pinned folios. + * @folios: array of folios to be marked dirty and released. + * @nfolios: number of folios in the @folios array. + * + * For each folio in the @folios array, release the folio using unpin_folio(). + * + * Please see the unpin_folio() documentation for details. + */ +void unpin_folios(struct folio **folios, unsigned long nfolios) +{ + unsigned long i = 0, j; + + /* +* If this WARN_ON() fires, then the system *might* be leaking folios +* (by leaving them pinned), but probably not. More likely, gup/pup +* returned a hard -ERRNO error to the caller, who erroneously passed +* it here. +*/ + if (WARN_ON(IS_ERR_VALUE(nfolios))) + return; + + sanity_check_pinned_folios(folios, nfolios); + while (i < nfolios) { + for (j = i + 1; j < nfolios; j++) + if (folios[i] !=
[PATCH v10 8/8] selftests/dma-buf/udmabuf: Add tests to verify data after page migration
Since the memfd pages associated with a udmabuf may be migrated as part of udmabuf create, we need to verify the data coherency after successful migration. The new tests added in this patch try to do just that using 4k sized pages and also 2 MB sized huge pages for the memfd. Successful completion of the tests would mean that there is no disconnect between the memfd pages and the ones associated with a udmabuf. And, these tests can also be augmented in the future to test newer udmabuf features (such as handling memfd hole punch). Cc: Shuah Khan Cc: David Hildenbrand Cc: Daniel Vetter Cc: Mike Kravetz Cc: Hugh Dickins Cc: Peter Xu Cc: Jason Gunthorpe Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Based-on-patch-by: Mike Kravetz Signed-off-by: Vivek Kasireddy --- .../selftests/drivers/dma-buf/udmabuf.c | 151 +- 1 file changed, 147 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/drivers/dma-buf/udmabuf.c b/tools/testing/selftests/drivers/dma-buf/udmabuf.c index c812080e304e..d76c813fe652 100644 --- a/tools/testing/selftests/drivers/dma-buf/udmabuf.c +++ b/tools/testing/selftests/drivers/dma-buf/udmabuf.c @@ -9,26 +9,132 @@ #include #include #include +#include #include #include +#include #include #include #define TEST_PREFIX"drivers/dma-buf/udmabuf" #define NUM_PAGES 4 +#define NUM_ENTRIES 4 +#define MEMFD_SIZE 1024 /* in pages */ -static int memfd_create(const char *name, unsigned int flags) +static unsigned int page_size; + +static int create_memfd_with_seals(off64_t size, bool hpage) +{ + int memfd, ret; + unsigned int flags = MFD_ALLOW_SEALING; + + if (hpage) + flags |= MFD_HUGETLB; + + memfd = memfd_create("udmabuf-test", flags); + if (memfd < 0) { + printf("%s: [skip,no-memfd]\n", TEST_PREFIX); + exit(77); + } + + ret = fcntl(memfd, F_ADD_SEALS, F_SEAL_SHRINK); + if (ret < 0) { + printf("%s: [skip,fcntl-add-seals]\n", TEST_PREFIX); + exit(77); + } + + ret = ftruncate(memfd, size); + if (ret == -1) { + printf("%s: [FAIL,memfd-truncate]\n", TEST_PREFIX); + exit(1); + } + + return memfd; +} + +static int create_udmabuf_list(int devfd, int memfd, off64_t memfd_size) +{ + struct udmabuf_create_list *list; + int ubuf_fd, i; + + list = malloc(sizeof(struct udmabuf_create_list) + + sizeof(struct udmabuf_create_item) * NUM_ENTRIES); + if (!list) { + printf("%s: [FAIL, udmabuf-malloc]\n", TEST_PREFIX); + exit(1); + } + + for (i = 0; i < NUM_ENTRIES; i++) { + list->list[i].memfd = memfd; + list->list[i].offset = i * (memfd_size / NUM_ENTRIES); + list->list[i].size = getpagesize() * NUM_PAGES; + } + + list->count = NUM_ENTRIES; + list->flags = UDMABUF_FLAGS_CLOEXEC; + ubuf_fd = ioctl(devfd, UDMABUF_CREATE_LIST, list); + free(list); + if (ubuf_fd < 0) { + printf("%s: [FAIL, udmabuf-create]\n", TEST_PREFIX); + exit(1); + } + + return ubuf_fd; +} + +static void write_to_memfd(void *addr, off64_t size, char chr) +{ + int i; + + for (i = 0; i < size / page_size; i++) { + *((char *)addr + (i * page_size)) = chr; + } +} + +static void *mmap_fd(int fd, off64_t size) { - return syscall(__NR_memfd_create, name, flags); + void *addr; + + addr = mmap(NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); + if (addr == MAP_FAILED) { + printf("%s: ubuf_fd mmap fail\n", TEST_PREFIX); + exit(1); + } + + return addr; +} + +static int compare_chunks(void *addr1, void *addr2, off64_t memfd_size) +{ + off64_t off; + int i = 0, j, k = 0, ret = 0; + char char1, char2; + + while (i < NUM_ENTRIES) { + off = i * (memfd_size / NUM_ENTRIES); + for (j = 0; j < NUM_PAGES; j++, k++) { + char1 = *((char *)addr1 + off + (j * getpagesize())); + char2 = *((char *)addr2 + (k * getpagesize())); + if (char1 != char2) { + ret = -1; + goto err; + } + } + i++; + } +err: + munmap(addr1, memfd_size); + munmap(addr2, NUM_ENTRIES * NUM_PAGES * getpagesize()); + return ret; } int main(int argc, char *argv[]) { struct udmabuf_create create; int devfd, memfd, buf, ret; - off_t size; - void *mem; + off64_t size; + void *addr1, *addr2; devfd = open("/dev/udmabuf", O_RDWR); if (devfd < 0) { @@ -90,6 +196,9 @@ int main(int argc, char *argv[]) } /* should work */ +
[PATCH v10 5/8] mm/gup: Introduce memfd_pin_folios() for pinning memfd folios (v10)
For drivers that would like to longterm-pin the folios associated with a memfd, the memfd_pin_folios() API provides an option to not only pin the folios via FOLL_PIN but also to check and migrate them if they reside in movable zone or CMA block. This API currently works with memfds but it should work with any files that belong to either shmemfs or hugetlbfs. Files belonging to other filesystems are rejected for now. The folios need to be located first before pinning them via FOLL_PIN. If they are found in the page cache, they can be immediately pinned. Otherwise, they need to be allocated using the filesystem specific APIs and then pinned. v2: - Drop gup_flags and improve comments and commit message (David) - Allocate a page if we cannot find in page cache for the hugetlbfs case as well (David) - Don't unpin pages if there is a migration related failure (David) - Drop the unnecessary nr_pages <= 0 check (Jason) - Have the caller of the API pass in file * instead of fd (Jason) v3: (David) - Enclose the huge page allocation code with #ifdef CONFIG_HUGETLB_PAGE (Build error reported by kernel test robot ) - Don't forget memalloc_pin_restore() on non-migration related errors - Improve the readability of the cleanup code associated with non-migration related errors - Augment the comments by describing FOLL_LONGTERM like behavior - Include the R-b tag from Jason v4: - Remove the local variable "page" and instead use 3 return statements in alloc_file_page() (David) - Add the R-b tag from David v5: (David) - For hugetlb case, ensure that we only obtain head pages from the mapping by using __filemap_get_folio() instead of find_get_page_flags() - Handle -EEXIST when two or more potential users try to simultaneously add a huge page to the mapping by forcing them to retry on failure v6: (Christoph) - Rename this API to memfd_pin_user_pages() to make it clear that it is intended for memfds - Move the memfd page allocation helper from gup.c to memfd.c - Fix indentation errors in memfd_pin_user_pages() - For contiguous ranges of folios, use a helper such as filemap_get_folios_contig() to lookup the page cache in batches v7: - Rename this API to memfd_pin_folios() and make it return folios and offsets instead of pages (David) - Don't continue processing the folios in the batch returned by filemap_get_folios_contig() if they do not have correct next_idx - Add the R-b tag from Christoph v8: (David) - Have caller pass [start, end], max_folios instead of start, nr_pages - Replace offsets array with just offset into the first page - Add comments explaning the need for next_idx - Pin (and return) the folio (via FOLL_PIN) only once v9: (Matthew) - Drop the extern while declaring memfd_alloc_folio() - Fix memfd_alloc_folio() declaration to have it return struct folio * instead of struct page * when CONFIG_MEMFD_CREATE is not defined v10: - Use unpin_folios() to unpin folios on error - Use check_and_migrate_movable_folios() instead of check_and_migrate_movable_pages() for checking and migrating folios Cc: David Hildenbrand Cc: Matthew Wilcox (Oracle) Cc: Christoph Hellwig Cc: Daniel Vetter Cc: Mike Kravetz Cc: Hugh Dickins Cc: Peter Xu Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Suggested-by: Jason Gunthorpe Reviewed-by: Jason Gunthorpe (v2) Reviewed-by: David Hildenbrand (v3) Reviewed-by: Christoph Hellwig (v6) Signed-off-by: Vivek Kasireddy --- include/linux/memfd.h | 5 ++ include/linux/mm.h| 3 + mm/gup.c | 136 ++ mm/memfd.c| 34 +++ 4 files changed, 178 insertions(+) diff --git a/include/linux/memfd.h b/include/linux/memfd.h index e7abf6fa4c52..3f2cf339ceaf 100644 --- a/include/linux/memfd.h +++ b/include/linux/memfd.h @@ -6,11 +6,16 @@ #ifdef CONFIG_MEMFD_CREATE extern long memfd_fcntl(struct file *file, unsigned int cmd, unsigned int arg); +struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx); #else static inline long memfd_fcntl(struct file *f, unsigned int c, unsigned int a) { return -EINVAL; } +static inline struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx) +{ + return ERR_PTR(-EINVAL); +} #endif #endif /* __LINUX_MEMFD_H */ diff --git a/include/linux/mm.h b/include/linux/mm.h index 4dbfdd277014..cbc0c6360690 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2474,6 +2474,9 @@ long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages, struct page **pages, unsigned int gup_flags); long pin_user_pages_unlocked(unsigned long start, unsigned long nr_pages, struct page **pages, unsigned int gup_flags); +long memfd_pin_folios(struct file *memfd, loff_t start, loff_t end, + struct folio **folios, unsigned int max_folios, + pgoff_t *offset); int get_user_pages_fast(unsigned long start, int nr_pages,
[PATCH v10 7/8] udmabuf: Pin the pages using memfd_pin_folios() API (v8)
Using memfd_pin_folios() will ensure that the pages are pinned correctly using FOLL_PIN. And, this also ensures that we don't accidentally break features such as memory hotunplug as it would not allow pinning pages in the movable zone. Using this new API also simplifies the code as we no longer have to deal with extracting individual pages from their mappings or handle shmem and hugetlb cases separately. v2: - Adjust to the change in signature of pin_user_pages_fd() by passing in file * instead of fd. v3: - Limit the changes in this patch only to those that are required for using pin_user_pages_fd() - Slightly improve the commit message v4: - Adjust to the change in name of the API (memfd_pin_user_pages) v5: - Adjust to the changes in memfd_pin_folios which now populates a list of folios and offsets v6: - Don't unnecessarily use folio_page() (Matthew) - Pass [start, end] and max_folios to memfd_pin_folios() - Create another temporary array to hold the folios returned by memfd_pin_folios() as we populate ubuf->folios. - Unpin the folios only once as memfd_pin_folios pins them once v7: - Use a list to track the folios that need to be unpinned v8: - Use unpin_folio() instead of unpin_user_page() to unpin folio Cc: David Hildenbrand Cc: Matthew Wilcox Cc: Daniel Vetter Cc: Mike Kravetz Cc: Hugh Dickins Cc: Peter Xu Cc: Jason Gunthorpe Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Signed-off-by: Vivek Kasireddy --- drivers/dma-buf/udmabuf.c | 153 +++--- 1 file changed, 78 insertions(+), 75 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index a8f3af61f7f2..afa8bfd2a2a9 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -30,6 +30,12 @@ struct udmabuf { struct sg_table *sg; struct miscdevice *device; pgoff_t *offsets; + struct list_head unpin_list; +}; + +struct udmabuf_folio { + struct folio *folio; + struct list_head list; }; static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) @@ -153,17 +159,43 @@ static void unmap_udmabuf(struct dma_buf_attachment *at, return put_sg_table(at->dev, sg, direction); } +static void unpin_all_folios(struct list_head *unpin_list) +{ + struct udmabuf_folio *ubuf_folio; + + while (!list_empty(unpin_list)) { + ubuf_folio = list_first_entry(unpin_list, + struct udmabuf_folio, list); + unpin_folio(ubuf_folio->folio); + + list_del(_folio->list); + kfree(ubuf_folio); + } +} + +static int add_to_unpin_list(struct list_head *unpin_list, +struct folio *folio) +{ + struct udmabuf_folio *ubuf_folio; + + ubuf_folio = kzalloc(sizeof(*ubuf_folio), GFP_KERNEL); + if (!ubuf_folio) + return -ENOMEM; + + ubuf_folio->folio = folio; + list_add_tail(_folio->list, unpin_list); + return 0; +} + static void release_udmabuf(struct dma_buf *buf) { struct udmabuf *ubuf = buf->priv; struct device *dev = ubuf->device->this_device; - pgoff_t pg; if (ubuf->sg) put_sg_table(dev, ubuf->sg, DMA_BIDIRECTIONAL); - for (pg = 0; pg < ubuf->pagecount; pg++) - folio_put(ubuf->folios[pg]); + unpin_all_folios(>unpin_list); kfree(ubuf->offsets); kfree(ubuf->folios); kfree(ubuf); @@ -218,64 +250,6 @@ static const struct dma_buf_ops udmabuf_ops = { #define SEALS_WANTED (F_SEAL_SHRINK) #define SEALS_DENIED (F_SEAL_WRITE) -static int handle_hugetlb_pages(struct udmabuf *ubuf, struct file *memfd, - pgoff_t offset, pgoff_t pgcnt, - pgoff_t *pgbuf) -{ - struct hstate *hpstate = hstate_file(memfd); - pgoff_t mapidx = offset >> huge_page_shift(hpstate); - pgoff_t subpgoff = (offset & ~huge_page_mask(hpstate)) >> PAGE_SHIFT; - pgoff_t maxsubpgs = huge_page_size(hpstate) >> PAGE_SHIFT; - struct folio *folio = NULL; - pgoff_t pgidx; - - mapidx <<= huge_page_order(hpstate); - for (pgidx = 0; pgidx < pgcnt; pgidx++) { - if (!folio) { - folio = __filemap_get_folio(memfd->f_mapping, - mapidx, - FGP_ACCESSED, 0); - if (IS_ERR(folio)) - return PTR_ERR(folio); - } - - folio_get(folio); - ubuf->folios[*pgbuf] = folio; - ubuf->offsets[*pgbuf] = subpgoff << PAGE_SHIFT; - (*pgbuf)++; - if (++subpgoff == maxsubpgs) { - folio_put(folio); - folio = NULL; - subpgoff = 0; - mapidx += pages_per_huge_page(hpstate);
[PATCH v10 1/8] udmabuf: Use vmf_insert_pfn and VM_PFNMAP for handling mmap
Add VM_PFNMAP to vm_flags in the mmap handler to ensure that the mappings would be managed without using struct page. And, in the vm_fault handler, use vmf_insert_pfn to share the page's pfn to userspace instead of directly sharing the page (via struct page *). Cc: David Hildenbrand Cc: Daniel Vetter Cc: Mike Kravetz Cc: Hugh Dickins Cc: Peter Xu Cc: Jason Gunthorpe Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Suggested-by: David Hildenbrand Acked-by: David Hildenbrand Signed-off-by: Vivek Kasireddy --- drivers/dma-buf/udmabuf.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index c40645999648..820c993c8659 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -35,12 +35,13 @@ static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) struct vm_area_struct *vma = vmf->vma; struct udmabuf *ubuf = vma->vm_private_data; pgoff_t pgoff = vmf->pgoff; + unsigned long pfn; if (pgoff >= ubuf->pagecount) return VM_FAULT_SIGBUS; - vmf->page = ubuf->pages[pgoff]; - get_page(vmf->page); - return 0; + + pfn = page_to_pfn(ubuf->pages[pgoff]); + return vmf_insert_pfn(vma, vmf->address, pfn); } static const struct vm_operations_struct udmabuf_vm_ops = { @@ -56,6 +57,7 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) vma->vm_ops = _vm_ops; vma->vm_private_data = ubuf; + vm_flags_set(vma, VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP); return 0; } -- 2.39.2
[PATCH v10 0/8] mm/gup: Introduce memfd_pin_folios() for pinning memfd folios (v10)
The first two patches were previously reviewed but not yet merged. These ones need to be merged first as the fourth patch depends on the changes introduced in them and they also fix bugs seen in very specific scenarios (running Qemu with hugetlb=on, blob=true and rebooting guest VM). Patches 3 and 4 add helpers that'd be used in subsequent patches. The 5th patch introduces memfd_pin_folios() API and the 6th one converts udmabuf driver to use folios. The 7th patch shows how the udmabuf driver can make use of the new API to longterm-pin the folios. And, the last patch adds two new udmabuf selftests to verify data coherency after potential page migration. v2: - Updated the first patch to include review feedback from David and Jason. The main change in this series is the allocation of page in the case of hugetlbfs if it is not found in the page cache. v3: - Made changes to include review feedback from David to improve the comments and readability of code - Enclosed the hugepage alloc code with #ifdef CONFIG_HUGETLB_PAGE v4: - Augmented the commit message of the udmabuf patch that uses pin_user_pages_fd() - Added previously reviewed but unmerged udmabuf patches to this series v5: - Updated the patch that adds pin_user_pages_fd() to include feedback from David to handle simultaneous users trying to add a huge page to the mapping - Replaced find_get_page_flags() with __filemap_get_folio() in the second and third patches to ensure that we only obtain head pages from the mapping v6: (Christoph) - Renamed the new API to memfd_pin_user_pages() - Improved the page cache lookup efficiency by using filemap_get_folios_contig() which uses batches v7: - Rename the new API to memfd_pin_folios() and make it return folios and offsets (David) - Added a new preparatory patch to this series to convert udmabuf driver to use folios v8: - Addressed review comments from Matthew in patches 4 and 5 - Included David's suggestions to have the caller of memfd_pin_folios() pass a range [stard, end], max_folios instead of start, nr_pages - Ensured that a folio is pinned and unpinned only once (David) v9: - Drop the extern and fix the return type in the declaration of memfd_alloc_folio() (Matthew) - Use a list to track the folios that need to be unpinned (patch 5) v10: - Introduced and used unpin_folio(), unpin_folios() and check_and_migrate_movable_folios() helpers This series is tested using following methods: - Run the subtests added in the fifth patch - Run Qemu (master) with the following options and a few additional patches to Spice: qemu-system-x86_64 -m 4096m -device virtio-gpu-pci,max_outputs=1,blob=true,xres=1920,yres=1080 -spice port=3001,gl=on,disable-ticketing=on,preferred-codec=gstreamer:h264 -object memory-backend-memfd,hugetlb=on,id=mem1,size=4096M -machine memory-backend=mem1 Cc: David Hildenbrand Cc: Matthew Wilcox (Oracle) Cc: Christoph Hellwig Cc: Daniel Vetter Cc: Mike Kravetz Cc: Hugh Dickins Cc: Peter Xu Cc: Jason Gunthorpe Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Vivek Kasireddy (8): udmabuf: Use vmf_insert_pfn and VM_PFNMAP for handling mmap udmabuf: Add back support for mapping hugetlb pages (v6) mm/gup: Introduce unpin_folio/unpin_folios helpers mm/gup: Introduce check_and_migrate_movable_folios() mm/gup: Introduce memfd_pin_folios() for pinning memfd folios (v10) udmabuf: Convert udmabuf driver to use folios (v2) udmabuf: Pin the pages using memfd_pin_folios() API (v8) selftests/dma-buf/udmabuf: Add tests to verify data after page migration drivers/dma-buf/udmabuf.c | 231 +--- include/linux/memfd.h | 5 + include/linux/mm.h| 5 + mm/gup.c | 346 +++--- mm/memfd.c| 34 ++ .../selftests/drivers/dma-buf/udmabuf.c | 151 +++- 6 files changed, 662 insertions(+), 110 deletions(-) -- 2.39.2
[PATCH v10 2/8] udmabuf: Add back support for mapping hugetlb pages (v6)
A user or admin can configure a VMM (Qemu) Guest's memory to be backed by hugetlb pages for various reasons. However, a Guest OS would still allocate (and pin) buffers that are backed by regular 4k sized pages. In order to map these buffers and create dma-bufs for them on the Host, we first need to find the hugetlb pages where the buffer allocations are located and then determine the offsets of individual chunks (within those pages) and use this information to eventually populate a scatterlist. Testcase: default_hugepagesz=2M hugepagesz=2M hugepages=2500 options were passed to the Host kernel and Qemu was launched with these relevant options: qemu-system-x86_64 -m 4096m -device virtio-gpu-pci,max_outputs=1,blob=true,xres=1920,yres=1080 -display gtk,gl=on -object memory-backend-memfd,hugetlb=on,id=mem1,size=4096M -machine memory-backend=mem1 Replacing -display gtk,gl=on with -display gtk,gl=off above would exercise the mmap handler. v2: Updated get_sg_table() to manually populate the scatterlist for both huge page and non-huge-page cases. v3: s/offsets/subpgoff/g s/hpoff/mapidx/g v4: Replaced find_get_page_flags() with __filemap_get_folio() to ensure that we only obtain head pages from the mapping v5: Fix the calculation of mapidx to ensure that it is a order-n page multiple v6: - Split the processing of hugetlb or shmem pages into helpers to simplify the code in udmabuf_create() (Christoph) - Move the creation of offsets array out of hugetlb context and into common code Cc: David Hildenbrand Cc: Daniel Vetter Cc: Mike Kravetz Cc: Hugh Dickins Cc: Peter Xu Cc: Jason Gunthorpe Cc: Gerd Hoffmann Cc: Dongwon Kim Cc: Junxiao Chang Acked-by: Mike Kravetz (v2) Signed-off-by: Vivek Kasireddy --- drivers/dma-buf/udmabuf.c | 122 +++--- 1 file changed, 101 insertions(+), 21 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 820c993c8659..274defd3fa3e 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -28,6 +29,7 @@ struct udmabuf { struct page **pages; struct sg_table *sg; struct miscdevice *device; + pgoff_t *offsets; }; static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) @@ -41,6 +43,8 @@ static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) return VM_FAULT_SIGBUS; pfn = page_to_pfn(ubuf->pages[pgoff]); + pfn += ubuf->offsets[pgoff] >> PAGE_SHIFT; + return vmf_insert_pfn(vma, vmf->address, pfn); } @@ -90,23 +94,29 @@ static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, { struct udmabuf *ubuf = buf->priv; struct sg_table *sg; + struct scatterlist *sgl; + unsigned int i = 0; int ret; sg = kzalloc(sizeof(*sg), GFP_KERNEL); if (!sg) return ERR_PTR(-ENOMEM); - ret = sg_alloc_table_from_pages(sg, ubuf->pages, ubuf->pagecount, - 0, ubuf->pagecount << PAGE_SHIFT, - GFP_KERNEL); + + ret = sg_alloc_table(sg, ubuf->pagecount, GFP_KERNEL); if (ret < 0) - goto err; + goto err_alloc; + + for_each_sg(sg->sgl, sgl, ubuf->pagecount, i) + sg_set_page(sgl, ubuf->pages[i], PAGE_SIZE, ubuf->offsets[i]); + ret = dma_map_sgtable(dev, sg, direction, 0); if (ret < 0) - goto err; + goto err_map; return sg; -err: +err_map: sg_free_table(sg); +err_alloc: kfree(sg); return ERR_PTR(ret); } @@ -143,6 +153,7 @@ static void release_udmabuf(struct dma_buf *buf) for (pg = 0; pg < ubuf->pagecount; pg++) put_page(ubuf->pages[pg]); + kfree(ubuf->offsets); kfree(ubuf->pages); kfree(ubuf); } @@ -196,17 +207,77 @@ static const struct dma_buf_ops udmabuf_ops = { #define SEALS_WANTED (F_SEAL_SHRINK) #define SEALS_DENIED (F_SEAL_WRITE) +static int handle_hugetlb_pages(struct udmabuf *ubuf, struct file *memfd, + pgoff_t offset, pgoff_t pgcnt, + pgoff_t *pgbuf) +{ + struct hstate *hpstate = hstate_file(memfd); + pgoff_t mapidx = offset >> huge_page_shift(hpstate); + pgoff_t subpgoff = (offset & ~huge_page_mask(hpstate)) >> PAGE_SHIFT; + pgoff_t maxsubpgs = huge_page_size(hpstate) >> PAGE_SHIFT; + struct page *hpage = NULL; + struct folio *folio; + pgoff_t pgidx; + + mapidx <<= huge_page_order(hpstate); + for (pgidx = 0; pgidx < pgcnt; pgidx++) { + if (!hpage) { + folio = __filemap_get_folio(memfd->f_mapping, + mapidx, + FGP_ACCESSED, 0);
Re: [PATCH] drm/mediatek/dp: Add the HDCP feature for DisplayPort
[PATCH v3 8/8] drm/panel: nt35510: support FRIDA FRD400B25025-A-CTK
The initialization commands are taken from the STMicroelectronics driver found at [1]. To ensure backward compatibility, flags have been added to enable gamma correction setting and display control. In other cases, registers have been set to their default values according to the specifications found in the datasheet. [1] https://github.com/STMicroelectronics/STM32CubeF7/blob/master/Drivers/BSP/Components/nt35510/ Signed-off-by: Dario Binacchi --- (no changes since v2) Changes in v2: - Re-write the patch [8/8] "drm/panel: nt35510: support FRIDA FRD400B25025-A-CTK" in the same style as the original driver. drivers/gpu/drm/panel/panel-novatek-nt35510.c | 282 -- 1 file changed, 251 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c b/drivers/gpu/drm/panel/panel-novatek-nt35510.c index ce8969f48286..c85dd0d0829d 100644 --- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c +++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c @@ -36,6 +36,9 @@ #include #include +#define NT35510_CMD_CORRECT_GAMMA BIT(0) +#define NT35510_CMD_CONTROL_DISPLAY BIT(1) + #define MCS_CMD_MAUCCTR0xF0 /* Manufacturer command enable */ #define MCS_CMD_READ_ID1 0xDA #define MCS_CMD_READ_ID2 0xDB @@ -112,18 +115,33 @@ /* AVDD and AVEE setting 3 bytes */ #define NT35510_P1_AVDD_LEN 3 #define NT35510_P1_AVEE_LEN 3 +#define NT35510_P1_VCL_LEN 3 #define NT35510_P1_VGH_LEN 3 #define NT35510_P1_VGL_LEN 3 #define NT35510_P1_VGP_LEN 3 #define NT35510_P1_VGN_LEN 3 +#define NT35510_P1_VCMOFF_LEN 2 /* BT1CTR thru BT5CTR setting 3 bytes */ #define NT35510_P1_BT1CTR_LEN 3 #define NT35510_P1_BT2CTR_LEN 3 +#define NT35510_P1_BT3CTR_LEN 3 #define NT35510_P1_BT4CTR_LEN 3 #define NT35510_P1_BT5CTR_LEN 3 /* 52 gamma parameters times two per color: positive and negative */ #define NT35510_P1_GAMMA_LEN 52 +#define NT35510_WRCTRLD_BCTRL BIT(5) +#define NT35510_WRCTRLD_A BIT(4) +#define NT35510_WRCTRLD_DD BIT(3) +#define NT35510_WRCTRLD_BL BIT(2) +#define NT35510_WRCTRLD_DB BIT(1) +#define NT35510_WRCTRLD_G BIT(0) + +#define NT35510_WRCABC_OFF 0 +#define NT35510_WRCABC_UI_MODE 1 +#define NT35510_WRCABC_STILL_MODE 2 +#define NT35510_WRCABC_MOVING_MODE 3 + /** * struct nt35510_config - the display-specific NT35510 configuration * @@ -175,6 +193,10 @@ struct nt35510_config { * @mode_flags: DSI operation mode related flags */ unsigned long mode_flags; + /** +* @cmds: enable DSI commands +*/ + u32 cmds; /** * @avdd: setting for AVDD ranging from 0x00 = 6.5V to 0x14 = 4.5V * in 0.1V steps the default is 0x05 which means 6.0V @@ -224,6 +246,25 @@ struct nt35510_config { * The defaults are 4 and 3 yielding 0x34 */ u8 bt2ctr[NT35510_P1_BT2CTR_LEN]; + /** +* @vcl: setting for VCL ranging from 0x00 = -2.5V to 0x11 = -4.0V +* in 1V steps, the default is 0x00 which means -2.5V +*/ + u8 vcl[NT35510_P1_VCL_LEN]; + /** +* @bt3ctr: setting for boost power control for the VCL step-up +* circuit (3) +* bits 0..2 in the lower nibble controls CLCK, the booster clock +* frequency, the values are the same as for PCK in @bt1ctr. +* bits 4..5 in the upper nibble controls BTCL, the boosting +* amplification for the step-up circuit. +* 0 = Disable +* 1 = -0.5 x VDDB +* 2 = -1 x VDDB +* 3 = -2 x VDDB +* The defaults are 4 and 2 yielding 0x24 +*/ + u8 bt3ctr[NT35510_P1_BT3CTR_LEN]; /** * @vgh: setting for VGH ranging from 0x00 = 7.0V to 0x0B = 18.0V * in 1V steps, the default is 0x08 which means 15V @@ -277,6 +318,19 @@ struct nt35510_config { * same layout of bytes as @vgp. */ u8 vgn[NT35510_P1_VGN_LEN]; + /** +* @vcmoff: setting the DC VCOM offset voltage +* The first byte contains bit 8 of VCM in bit 0 and VCMOFFSEL in bit 4. +* The second byte contains bits 0..7 of VCM. +* VCMOFFSEL the common voltage offset mode. +* VCMOFFSEL 0x00 = VCOM .. 0x01 Gamma. +* The default is 0x00. +* VCM the VCOM output voltage (VCMOFFSEL = 0) or the internal register +* offset for gamma voltage (VCMOFFSEL = 1). +* VCM 0x00 = 0V/0 .. 0x118 = 3.5V/280 in steps of 12.5mV/1step +* The default is 0x00 = 0V/0. +*/ + u8 vcmoff[NT35510_P1_VCMOFF_LEN]; /** * @dopctr: setting optional control for display * ERR bits 0..1 in the first byte is the ERR pin output signal setting. @@ -441,6 +495,43 @@ struct nt35510_config { * @gamma_corr_neg_b: Blue gamma correction parameters, negative */ u8 gamma_corr_neg_b[NT35510_P1_GAMMA_LEN]; + /** +* @wrdisbv: write display brightness +* 0x00 value means the lowest brightness and 0xff value
[PATCH v3 7/8] drm/panel: nt35510: move hardwired parameters to configuration
This patch, preparatory for future developments, move the hardwired parameters to configuration data to allow the addition of new NT35510-based panels. Signed-off-by: Dario Binacchi --- (no changes since v2) Changes in v2: - Re-write the patch [7/8] "drm/panel: nt35510: refactor panel initialization" in the same style as the original driver in order to maintain the same structure. drivers/gpu/drm/panel/panel-novatek-nt35510.c | 140 ++ 1 file changed, 115 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c b/drivers/gpu/drm/panel/panel-novatek-nt35510.c index d6dceb858008..ce8969f48286 100644 --- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c +++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c @@ -171,6 +171,10 @@ struct nt35510_config { * timing in the display controller. */ const struct drm_display_mode mode; + /** +* @mode_flags: DSI operation mode related flags +*/ + unsigned long mode_flags; /** * @avdd: setting for AVDD ranging from 0x00 = 6.5V to 0x14 = 4.5V * in 0.1V steps the default is 0x05 which means 6.0V @@ -273,6 +277,100 @@ struct nt35510_config { * same layout of bytes as @vgp. */ u8 vgn[NT35510_P1_VGN_LEN]; + /** +* @dopctr: setting optional control for display +* ERR bits 0..1 in the first byte is the ERR pin output signal setting. +* 0 = Disable, ERR pin output low +* 1 = ERR pin output CRC error only +* 2 = ERR pin output ECC error only +* 3 = ERR pin output CRC and ECC error +* The default is 0. +* N565 bit 2 in the first byte is the 16-bit/pixel format selection. +* 0 = R[4:0] + G[5:3] & G[2:0] + B[4:0] +* 1 = G[2:0] + R[4:0] & B[4:0] + G[5:3] +* The default is 0. +* DIS_EoTP_HS bit 3 in the first byte is "DSI protocol violation" error +* reporting. +* 0 = reporting when error +* 1 = not reporting when error +* DSIM bit 4 in the first byte is the video mode data type enable +* 0 = Video mode data type disable +* 1 = Video mode data type enable +* The default is 0. +* DSIG bit 5 int the first byte is the generic r/w data type enable +* 0 = Generic r/w disable +* 1 = Generic r/w enable +* The default is 0. +* DSITE bit 6 in the first byte is TE line enable +* 0 = TE line is disabled +* 1 = TE line is enabled +* The default is 0. +* RAMKP bit 7 in the first byte is the frame memory keep/loss in +* sleep-in mode +* 0 = contents loss in sleep-in +* 1 = contents keep in sleep-in +* The default is 0. +* CRL bit 1 in the second byte is the source driver data shift +* direction selection. This bit is XOR operation with bit RSMX +* of 3600h command. +* 0 (RMSX = 0) = S1 -> S1440 +* 0 (RMSX = 1) = S1440 -> S1 +* 1 (RMSX = 0) = S1440 -> S1 +* 1 (RMSX = 1) = S1 -> S1440 +* The default is 0. +* CTB bit 2 in the second byte is the vertical scanning direction +* selection for gate control signals. This bit is XOR operation +* with bit ML of 3600h command. +* 0 (ML = 0) = Forward (top -> bottom) +* 0 (ML = 1) = Reverse (bottom -> top) +* 1 (ML = 0) = Reverse (bottom -> top) +* 1 (ML = 1) = Forward (top -> bottom) +* The default is 0. +* CRGB bit 3 in the second byte is RGB-BGR order selection. This +* bit is XOR operation with bit RGB of 3600h command. +* 0 (RGB = 0) = RGB/Normal +* 0 (RGB = 1) = BGR/RB swap +* 1 (RGB = 0) = BGR/RB swap +* 1 (RGB = 1) = RGB/Normal +* The default is 0. +* TE_PWR_SEL bit 4 in the second byte is the TE output voltage +* level selection (only valid when DSTB_SEL = 0 or DSTB_SEL = 1, +* VSEL = High and VDDI = 1.665~3.3V). +* 0 = TE output voltage level is VDDI +* 1 = TE output voltage level is VDDA +* The default is 0. +*/ + u8 dopctr[NT35510_P0_DOPCTR_LEN]; + /** +* @madctl: Memory data access control +* RSMY bit 0 is flip vertical. Flips the display image top to down. +* RSMX bit 1 is flip horizontal. Flips the display image left to right. +* MH bit 2 is the horizontal refresh order. +* RGB bit 3 is the RGB-BGR order. +* 0 = RGB color sequence +* 1 = BGR color sequence +* ML bit 4 is the vertical refresh order. +* MV bit 5 is the row/column exchange. +* MX bit 6 is the column address order. +* MY bit 7 is the row address order. +*/ + u8 madctl; + /** +* @sdhdtctr: source output data hold time +* 0x00..0x3F = 0..31.5us in steps of 0.5us +* The default is 0x05 =
[PATCH v3 5/8] dt-bindings: nt35510: add compatible for FRIDA FRD400B25025-A-CTK
The patch adds the FRIDA FRD400B25025-A-CTK panel, which belongs to the Novatek NT35510-based panel family. Signed-off-by: Dario Binacchi --- Changes in v3: - Use "enum" to have less code changed Changes in v2: - Add a dash in front of each "items:" .../devicetree/bindings/display/panel/novatek,nt35510.yaml| 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/panel/novatek,nt35510.yaml b/Documentation/devicetree/bindings/display/panel/novatek,nt35510.yaml index bc92928c805b..43afb316e0e9 100644 --- a/Documentation/devicetree/bindings/display/panel/novatek,nt35510.yaml +++ b/Documentation/devicetree/bindings/display/panel/novatek,nt35510.yaml @@ -15,7 +15,9 @@ allOf: properties: compatible: items: - - const: hydis,hva40wv1 + - enum: + - hydis,hva40wv1 + - frida,frd400b25025 - const: novatek,nt35510 description: This indicates the panel manufacturer of the panel that is in turn using the NT35510 panel driver. The compatible -- 2.43.0
[PATCH v3 0/8] Add display support for stm32f769-disco board
The series adds display support for the stm32f769-disco board. It has been tested on hardware revisions MB1225-B03 and MB1166-A09. This required modifications to the nt35510 driver. As I do not have the Hydis HVA40WV1 display, it would be better if someone tested the driver in that configuration. Changes in v3: - Use "enum" to have less code changed Changes in v2: - Add Acked-by tag of Conor Dooley - Add a dash in front of each "items:" - Change the status of panel_backlight node to "disabled" - Delete backlight property from panel0 node. - Re-write the patch [7/8] "drm/panel: nt35510: refactor panel initialization" in the same style as the original driver in order to maintain the same structure. - Re-write the patch [8/8] "drm/panel: nt35510: support FRIDA FRD400B25025-A-CTK" in the same style as the original driver. Dario Binacchi (8): dt-bindings: mfd: stm32f7: Add binding definition for DSI ARM: dts: stm32: add DSI support on stm32f769 ARM: dts: stm32: rename mmc_vcard to vcc-3v3 on stm32f769-disco ARM: dts: stm32: add display support on stm32f769-disco dt-bindings: nt35510: add compatible for FRIDA FRD400B25025-A-CTK ARM: dts: add stm32f769-disco-mb1225-revb03-mb1166-reva09 drm/panel: nt35510: move hardwired parameters to configuration drm/panel: nt35510: support FRIDA FRD400B25025-A-CTK .../display/panel/novatek,nt35510.yaml| 4 +- arch/arm/boot/dts/st/Makefile | 1 + ...f769-disco-mb1225-revb03-mb1166-reva09.dts | 18 + arch/arm/boot/dts/st/stm32f769-disco.dts | 78 +++- arch/arm/boot/dts/st/stm32f769.dtsi | 21 + drivers/gpu/drm/panel/panel-novatek-nt35510.c | 422 +++--- include/dt-bindings/mfd/stm32f7-rcc.h | 1 + 7 files changed, 484 insertions(+), 61 deletions(-) create mode 100644 arch/arm/boot/dts/st/stm32f769-disco-mb1225-revb03-mb1166-reva09.dts create mode 100644 arch/arm/boot/dts/st/stm32f769.dtsi -- 2.43.0
Re: [PATCH] drm/mediatek/dp: Add the HDCP feature for DisplayPort
Re: [PATCH] drm/mediatek/dp: Add the HDCP feature for DisplayPort
Re: [PATCH] drm/mediatek/dp: Add the HDCP feature for DisplayPort
Re: [PATCH] nightly.conf: Add the xe repo to drm-tip
On Wed, Jan 03, 2024 at 02:50:57PM +0100, Thomas Hellström wrote: On Tue, 2023-12-26 at 13:30 -0500, Rodrigo Vivi wrote: On Fri, Dec 22, 2023 at 12:36:39PM +0100, Thomas Hellström wrote: > Add the xe repo to drm-tip and the dim tools. > For now use the sha1 of the first drm-xe-next pull request for drm- > tip, > since that branch tip is currently adapted for our CI testing. > > Cc: Rodrigo Vivi > Cc: Lucas De Marchi > Cc: Oded Gabbay > Cc: daniel.vet...@ffwll.ch > Cc: Maarten Lankhorst > Cc: dim-to...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Cc: intel-...@lists.freedesktop.org > Signed-off-by: Thomas Hellström > --- > nightly.conf | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/nightly.conf b/nightly.conf > index 24126b61b797..accd3ff2cc39 100644 > --- a/nightly.conf > +++ b/nightly.conf > @@ -24,6 +24,10 @@ git://anongit.freedesktop.org/drm-tip > https://anongit.freedesktop.org/git/drm/drm-tip > https://anongit.freedesktop.org/git/drm/drm-tip.git > " > +drm_tip_repos[drm-xe]=" > +ssh://g...@gitlab.freedesktop.org/drm/xe/kernel.git > +https://gitlab.freedesktop.org/drm/xe/kernel.git > +" > drm_tip_repos[drm-intel]=" > ssh://git.freedesktop.org/git/drm/drm-intel > ssh://git.freedesktop.org/git/drm-intel > @@ -65,14 +69,17 @@ drm_tip_config=( > "drm drm-fixes" > "drm-misc drm-misc-fixes" > "drm-intel drm-intel-fixes" > + "drm-xedrm-xe-fixes" > > "drm drm-next" > "drm-misc drm-misc-next-fixes" > "drm-intel drm-intel-next-fixes" > + "drm-xedrm-xe-next-fixes" > > "drm-misc drm-misc-next" > "drm-intel drm-intel-next" > "drm-intel drm-intel-gt-next" > + "drm-xedrm-xe-next b6e1b7081768" yeap, up to this commit nothing else should change, but then we will need an extra rebase of the rest on top of drm/drm-next. But then we need to decide where these following patches will live: 880277f31cc69 drm/xe/guc: define LNL FW 2cfc5ae1b8267 drm/xe/guc: define PVC FW 52383b58eb8cf mei/hdcp: Also enable for XE bea27d7369855 mei: gsc: add support for auxiliary device created by Xe driver fcb3410197f05 fault-inject: Include linux/types.h by default. 8ebd9cd71f8ac drm/xe: Add PVC's PCI device IDs Will it be the topic/core-for-CI? or topic/xe-extras? or what? This sounds to me like topic/core-for-CI? Or is there any drawback with that? I think some of them are not really a "for CI". It's more like the workflow we are adopting e.g. with guc/huc, not sending it to linux-firmware until we are confident on what version we will start officially supporting. This one can't go to topic/core-for-CI neither: fcb3410197f05 fault-inject: Include linux/types.h by default. what it would do would be that we would not see the build error anymore, but everyone else would (and it's not a CI-only configuration). Unless it's merged to another branch, we shouldn't merge it. "52383b58eb8cf mei/hdcp: Also enable for XE" could be material for topic/core-for-CI and "8ebd9cd71f8ac drm/xe: Add PVC's PCI device IDs" could either be on that branch or another xe-specific one. Anyway, for the inclusion like this, after our CI is ready: Could we merge this patch already at this point, considering it will, at least for now, only update drm-tip with our fixes? ack Lucas De Marchi Thanks, /Thomas Acked-by: Rodrigo Vivi > > "drm-intel topic/core-for-CI" > "drm-misc topic/i915-ttm" > -- > 2.42.0 >
Re: [PATCH v2 34/39] drm: bridge: dw_hdmi: switch to ->edid_read callback
Hi Jani, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.7-rc8 next-20240103] [cannot apply to drm-intel/for-linux-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-bridge-add-edid_read-hook-and-drm_bridge_edid_read/20240103-181513 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/a8f71940221fb085b8767f8123f496c9b36b22cc.1704276309.git.jani.nikula%40intel.com patch subject: [PATCH v2 34/39] drm: bridge: dw_hdmi: switch to ->edid_read callback config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20240104/202401041305.nctmamoj-...@intel.com/config) compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project 7e186d366d6c7def0543acc255931f617e76dff0) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240104/202401041305.nctmamoj-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202401041305.nctmamoj-...@intel.com/ All warnings (new ones prefixed by >>): >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c:2473:3: warning: variable 'edid' >> is uninitialized when used here [-Wuninitialized] 2473 | edid->width_cm, edid->height_cm); | ^~~~ include/linux/dev_printk.h:155:39: note: expanded from macro 'dev_dbg' 155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__) | ^~~ include/linux/dynamic_debug.h:274:19: note: expanded from macro 'dynamic_dev_dbg' 274 |dev, fmt, ##__VA_ARGS__) |^~~ include/linux/dynamic_debug.h:250:59: note: expanded from macro '_dynamic_func_call' 250 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__) | ^~~ include/linux/dynamic_debug.h:248:65: note: expanded from macro '_dynamic_func_call_cls' 248 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__) | ^~~ include/linux/dynamic_debug.h:224:15: note: expanded from macro '__dynamic_func_call_cls' 224 | func(, ##__VA_ARGS__); \ | ^~~ drivers/gpu/drm/bridge/synopsys/dw-hdmi.c:2461:25: note: initialize the variable 'edid' to silence this warning 2461 | const struct edid *edid; |^ | = NULL 1 warning generated. vim +/edid +2473 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 9aaf880ed4ee3c drivers/staging/imx-drm/imx-hdmi.cFabio Estevam 2013-11-29 2456 fcb55de55cf341 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c Jani Nikula 2024-01-03 2457 static const struct drm_edid *dw_hdmi_edid_read(struct dw_hdmi *hdmi, ec971aaa6775cf drivers/gpu/drm/bridge/synopsys/dw-hdmi.c Laurent Pinchart 2020-05-26 2458 struct drm_connector *connector) 9aaf880ed4ee3c drivers/staging/imx-drm/imx-hdmi.cFabio Estevam 2013-11-29 2459 { fcb55de55cf341 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c Jani Nikula 2024-01-03 2460 const struct drm_edid *drm_edid; fcb55de55cf341 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c Jani Nikula 2024-01-03 2461 const struct edid *edid; 9aaf880ed4ee3c drivers/staging/imx-drm/imx-hdmi.cFabio Estevam 2013-11-29 2462 9aaf880ed4ee3c drivers/staging/imx-drm/imx-hdmi.cFabio Estevam 2013-11-29 2463 if (!hdmi->ddc) ec971aaa6775cf drivers/gpu/drm/bridge/synopsys/dw-hdmi.c Laurent Pinchart 2020-05-26 2464 return NULL; 9aaf880ed4ee3c drivers/staging/imx-drm/imx-hdmi.cFabio Estevam 2013-11-29 2465 fcb55de55cf341 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c Jani Nikula 2024-01-03 2466 drm_edid = drm_edid_read_ddc(connector, hdmi->ddc); fcb55de55cf341 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c Jani Nikula 2024-01-03 2467 if (!drm_edid) { ec971aaa6775cf drivers/gpu/drm/bridge/synopsys/dw-hdmi.c Laurent Pinchart 2020-05-26 2468 dev_dbg(hdmi->dev, "failed to get
Re: [PATCH V2] drivers: gpu: drm: vmwgfx: fixed typos
On Fri, Dec 15, 2023 at 1:21 AM Randy Dunlap wrote: > > Hi-- > > On 12/14/23 22:01, Ghanshyam Agrawal wrote: > > Fixed multiple typos in vmwgfx_execbuf.c > > > > Signed-off-by: Ghanshyam Agrawal > > --- > > V2: > > Fixed some more typos suggested by codespell > > and the community. > > > > V1: > > Fixed multiple typos > > > > drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 8 > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c > > b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c > > index 36987ef3fc30..76aa72e8be73 100644 > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c > > @@ -127,7 +127,7 @@ struct vmw_ctx_validation_info { > > * @func: Call-back to handle the command. > > * @user_allow: Whether allowed from the execbuf ioctl. > > * @gb_disable: Whether disabled if guest-backed objects are available. > > - * @gb_enable: Whether enabled iff guest-backed objects are available. > > + * @gb_enable: Whether enabled if guest-backed objects are available. > > "iff" normally means "if and only if" and its use in the kernel sources is > usually not a mistake. However, this one sounds dodgy to me (before your > change), > so it's OK IMO. Also, the line above it uses "if" for a similar comment. > > Maybe someone else knows better. Right, this one was "iff". I submitted a version of this without the iff change to drm-misc-next. z
Re: [PATCH] drm/vmwgfx: fix kernel-doc Excess struct member 'base'
On Fri, Dec 15, 2023 at 6:56 PM Randy Dunlap wrote: > > Fix a new kernel-doc warning reported by kernel test robot: > > vmwgfx_surface.c:55: warning: Excess struct member 'base' description in > 'vmw_user_surface' > > The other warning is not correct: it is confused by "__counted_by". > Kees has made a separate patch for that. > > In -Wall mode, kernel-doc still reports 20 warnings of this nature: > vmwgfx_surface.c:198: warning: No description found for return value of > 'vmw_surface_dma_size' > but I am not addressing those. > > Signed-off-by: Randy Dunlap > Reported-by: kernel test robot > Closes: > https://lore.kernel.org/oe-kbuild-all/202312150701.kni9lum3-...@intel.com/ > Cc: Kees Cook > Cc: Zack Rusin > Cc: VMware Graphics Reviewers > Cc: dri-devel@lists.freedesktop.org > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Thomas Zimmermann > --- > drivers/gpu/drm/vmwgfx/vmwgfx_surface.c |1 - > 1 file changed, 1 deletion(-) > > diff -- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c > @@ -44,7 +44,6 @@ > * struct vmw_user_surface - User-space visible surface resource > * > * @prime: The TTM prime object. > - * @base: The TTM base object handling user-space visibility. > * @srf:The surface metadata. > * @master: Master of the creating client. Used for security check. > */ Thanks, looks great. I went ahead and pushed this one to drm-misc-next. z
Re: [PATCH] [v2] drm/vmwgfx: fix a memleak in vmw_gmrid_man_get_node
On Mon, Dec 4, 2023 at 4:15 AM Zhipeng Lu wrote: > > When ida_alloc_max fails, resources allocated before should be freed, > including *res allocated by kmalloc and ttm_resource_init. > > Fixes: d3bcb4b02fe9 ("drm/vmwgfx: switch the TTM backends to self alloc") > Signed-off-by: Zhipeng Lu > --- > > Changelog: > > v2: Adding {} to correct the if statement > --- > drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > index ceb4d3d3b965..a0b47c9b33f5 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c > @@ -64,8 +64,11 @@ static int vmw_gmrid_man_get_node(struct > ttm_resource_manager *man, > ttm_resource_init(bo, place, *res); > > id = ida_alloc_max(>gmr_ida, gman->max_gmr_ids - 1, GFP_KERNEL); > - if (id < 0) > + if (id < 0) { > + ttm_resource_fini(man, *res); > + kfree(*res); > return id; > + } > > spin_lock(>lock); Thanks, I pushed it to drm-misc-next. z
[git pull] drm fixes for 6.8
Hi Linus, These were from over the holiday period, mainly i915, a couple of qaic, bridge and an mgag200. I have a set of nouveau fixes that I'll send after this, that might be too rich for you at this point. I expect there might also be some more regular fixes before 6.8, but they should be minor. Dave. drm-fixes-2024-01-04: drm fixes for 6.8 qaic: - fix GEM import - add quirk for soc version bridge: - parade-ps8640, ti-sn65dsi86: fix aux reads bounds mgag200: - fix gamma LUT init i915: - Fix bogus DPCD rev usage for DP phy test pattern setup - Fix handling of MMIO triggered reports in the OA buffer The following changes since commit 610a9b8f49fbcf1100716370d3b5f6f884a2835a: Linux 6.7-rc8 (2023-12-31 12:51:25 -0800) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2024-01-04 for you to fetch changes up to faa21f4c20960fee268bdb0fe977ed0edb6685fe: Merge tag 'drm-misc-fixes-2024-01-03' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes (2024-01-04 11:18:32 +1000) drm fixes for 6.8 qaic: - fix GEM import - add quirk for soc version bridge: - parade-ps8640, ti-sn65dsi86: fix aux reads bounds mgag200: - fix gamma LUT init i915: - Fix bogus DPCD rev usage for DP phy test pattern setup - Fix handling of MMIO triggered reports in the OA buffer Dave Airlie (2): Merge tag 'drm-intel-fixes-2023-12-28' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Merge tag 'drm-misc-fixes-2024-01-03' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Douglas Anderson (3): drm/bridge: parade-ps8640: Never store more than msg->size bytes in AUX xfer drm/bridge: ti-sn65dsi86: Never store more than msg->size bytes in AUX xfer drm/bridge: ps8640: Fix size mismatch warning w/ len Jeffrey Hugo (1): accel/qaic: Implement quirk for SOC_HW_VERSION Jocelyn Falempe (1): drm/mgag200: Fix gamma lut not initialized for G200ER, G200EV, G200SE Khaled Almahallawy (1): drm/i915/dp: Fix passing the correct DPCD_REV for drm_dp_set_phy_test_pattern Pranjal Ramajor Asha Kanojiya (1): accel/qaic: Fix GEM import path code Umesh Nerlige Ramappa (1): drm/i915/perf: Update handling of MMIO triggered reports drivers/accel/qaic/mhi_controller.c | 15 +++- drivers/accel/qaic/qaic_data.c | 6 ++--- drivers/gpu/drm/bridge/parade-ps8640.c | 7 +++--- drivers/gpu/drm/bridge/ti-sn65dsi86.c| 4 +++- drivers/gpu/drm/i915/display/intel_dp.c | 2 +- drivers/gpu/drm/i915/i915_perf.c | 39 drivers/gpu/drm/mgag200/mgag200_drv.h| 5 drivers/gpu/drm/mgag200/mgag200_g200er.c | 5 drivers/gpu/drm/mgag200/mgag200_g200ev.c | 5 drivers/gpu/drm/mgag200/mgag200_g200se.c | 5 drivers/gpu/drm/mgag200/mgag200_mode.c | 10 11 files changed, 83 insertions(+), 20 deletions(-)
Re: [PATCH 08/11] nouveau/gsp: don't free ctrl messages on errors
On Thu, 4 Jan 2024 at 00:47, Dan Carpenter wrote: > > Hi Dave, > > kernel test robot noticed the following build warnings: > > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: > https://github.com/intel-lab-lkp/linux/commits/Dave-Airlie/nouveau-gsp-drop-some-acpi-related-debug/20231222-180432 > base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next > patch link: > https://lore.kernel.org/r/20231222043308.3090089-9-airlied%40gmail.com > patch subject: [PATCH 08/11] nouveau/gsp: don't free ctrl messages on errors > config: powerpc-randconfig-r071-20231226 > (https://download.01.org/0day-ci/archive/20231227/202312271917.55xudmdc-...@intel.com/config) > compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project > d3ef86708241a3bee902615c190dead1638c4e09) > > If you fix the issue in a separate patch/commit (i.e. not just a new version > of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot > | Reported-by: Dan Carpenter > | Closes: https://lore.kernel.org/r/202312271917.55xudmdc-...@intel.com/ This is a false positive, I think the code is operating like I'd expect, we maybe could restructure it to avoid this warning? The idea is you send an rpc msg, if there's a reply you get a reply, if no reply you get NULL and if an error you get an error. So in the case you get an error or NULL you just want to return 0 for the NULL as it's successful, and error otherwise. Would using PTR_ERR_OR_ZERO make smatch happy? (even if it's not really what we want). Dave. > > New smatch warnings: > drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:659 > r535_gsp_rpc_rm_ctrl_push() warn: passing zero to 'PTR_ERR' > drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c:1063 r535_dp_aux_xfer() warn: > passing a valid pointer to 'PTR_ERR' > > Old smatch warnings: > drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1887 nvkm_gsp_radix3_sg() > error: uninitialized symbol 'addr'. > > vim +/PTR_ERR +659 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c > > af265ee961627a Dave Airlie 2023-12-22 649 static int > af265ee961627a Dave Airlie 2023-12-22 650 r535_gsp_rpc_rm_ctrl_push(struct > nvkm_gsp_object *object, void **argv, u32 repc) > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 651 { > af265ee961627a Dave Airlie 2023-12-22 652 rpc_gsp_rm_control_v03_00 > *rpc = container_of((*argv), typeof(*rpc), params); > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 653 struct nvkm_gsp *gsp = > object->client->gsp; > af265ee961627a Dave Airlie 2023-12-22 654 int ret = 0; > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 655 > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 656 rpc = nvkm_gsp_rpc_push(gsp, > rpc, true, repc); > af265ee961627a Dave Airlie 2023-12-22 657 if (IS_ERR_OR_NULL(rpc)) { > af265ee961627a Dave Airlie 2023-12-22 658 *argv = NULL; > af265ee961627a Dave Airlie 2023-12-22 @659 return PTR_ERR(rpc); > > If nvkm_gsp_rpc_push() returns NULL (probably a failure) then this > returns PTR_ERR(NULL) which is zero/success. > > af265ee961627a Dave Airlie 2023-12-22 660 } > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 661 > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 662 if (rpc->status) { > af265ee961627a Dave Airlie 2023-12-22 663 ret = > r535_rpc_status_to_errno(rpc->status); > 555bb9c29a45be Dave Airlie 2023-12-22 664 if (ret != -EAGAIN) > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 665 > nvkm_error(>subdev, "cli:0x%08x obj:0x%08x ctrl cmd:0x%08x failed: > 0x%08x\n", > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 666 > object->client->object.handle, object->handle, rpc->cmd, rpc->status); > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 667 } > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 668 > af265ee961627a Dave Airlie 2023-12-22 669 if (repc) > af265ee961627a Dave Airlie 2023-12-22 670 *argv = rpc->params; > af265ee961627a Dave Airlie 2023-12-22 671 else > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 672 > nvkm_gsp_rpc_done(gsp, rpc); > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 673 > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 674 return ret; > 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 675 } > > -- > 0-DAY CI Kernel Test Service > https://github.com/intel/lkp-tests/wiki >
Re: [PATCH 1/1] drm/virtio: Implement RESOURCE_GET_LAYOUT ioctl
On 12/21/23 13:00, Julia Zhang wrote: > From: Daniel Stone > > Add a new ioctl to allow the guest VM to discover how the guest > actually allocated the underlying buffer, which allows buffers to > be used for GL<->Vulkan interop and through standard window systems. > It's also a step towards properly supporting modifiers in the guest. > > Signed-off-by: Daniel Stone > Co-developed-by: Julia Zhang # support query > stride before it's created > Signed-off-by: Julia Zhang > --- > drivers/gpu/drm/virtio/virtgpu_drv.c | 1 + > drivers/gpu/drm/virtio/virtgpu_drv.h | 22 - > drivers/gpu/drm/virtio/virtgpu_ioctl.c | 66 ++ > drivers/gpu/drm/virtio/virtgpu_kms.c | 8 +++- > drivers/gpu/drm/virtio/virtgpu_vq.c| 63 > include/uapi/drm/virtgpu_drm.h | 21 > include/uapi/linux/virtio_gpu.h| 30 > 7 files changed, 208 insertions(+), 3 deletions(-) ... > +static int virtio_gpu_resource_query_layout_ioctl(struct drm_device *dev, > + void *data, > + struct drm_file *file) > +{ > + struct drm_virtgpu_resource_query_layout *args = data; > + struct virtio_gpu_device *vgdev = dev->dev_private; > + struct drm_gem_object *obj = NULL; > + struct virtio_gpu_object *bo = NULL; > + struct virtio_gpu_query_info bo_info = {0}; > + int ret = 0; > + int i; > + > + if (!vgdev->has_resource_query_layout) { > + DRM_ERROR("failing: no RQL on host\n"); Please remove this message > + return -EINVAL; return -ENOSYS > + } > + > + if (args->handle > 0) { > + obj = drm_gem_object_lookup(file, args->handle); > + if (obj == NULL) { > + DRM_ERROR("invalid handle 0x%x\n", args->handle); > + return -ENOENT; > + } > + bo = gem_to_virtio_gpu_obj(obj); > + } > + > + ret = virtio_gpu_cmd_get_resource_layout(vgdev, _info, args->width, > + args->height, args->format, > + args->bind, bo ? > bo->hw_res_handle : 0); What this special hw_res_handle=0 is doing? Why is it needed? > + if (ret) > + goto out; > + > + ret = wait_event_timeout(vgdev->resp_wq, > + atomic_read(_info.is_valid), > + 5 * HZ); > + if (!ret) > + goto out; > + > +valid: > + smp_rmb(); > + WARN_ON(atomic_read(_info.is_valid)); Please remove this WARN_ON and fix the kernelbot report > + args->num_planes = bo_info.num_planes; > + args->modifier = bo_info.modifier; > + for (i = 0; i < args->num_planes; i++) { > + args->planes[i].offset = bo_info.planes[i].offset; > + args->planes[i].stride = bo_info.planes[i].stride; > + } > + for (; i < VIRTIO_GPU_MAX_RESOURCE_PLANES; i++) { > + args->planes[i].offset = 0; > + args->planes[i].stride = 0; > + } > + ret = 0; ret is already 0 here > +out: > + if (obj) > + drm_gem_object_put(obj); > + return ret; > +} ... > diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h > index f556fde07b76..547575232376 100644 > --- a/include/uapi/linux/virtio_gpu.h > +++ b/include/uapi/linux/virtio_gpu.h > @@ -65,6 +65,11 @@ > */ > #define VIRTIO_GPU_F_CONTEXT_INIT4 > > +/* > + * VIRTIO_GPU_CMD_RESOURCE_QUERY_LAYOUT > + */ > +#define VIRTIO_GPU_F_RESOURCE_QUERY_LAYOUT 5 > + > enum virtio_gpu_ctrl_type { > VIRTIO_GPU_UNDEFINED = 0, > > @@ -95,6 +100,7 @@ enum virtio_gpu_ctrl_type { > VIRTIO_GPU_CMD_SUBMIT_3D, > VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB, > VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB, > + VIRTIO_GPU_CMD_RESOURCE_QUERY_LAYOUT, > > /* cursor commands */ > VIRTIO_GPU_CMD_UPDATE_CURSOR = 0x0300, > @@ -108,6 +114,7 @@ enum virtio_gpu_ctrl_type { > VIRTIO_GPU_RESP_OK_EDID, > VIRTIO_GPU_RESP_OK_RESOURCE_UUID, > VIRTIO_GPU_RESP_OK_MAP_INFO, > + VIRTIO_GPU_RESP_OK_RESOURCE_LAYOUT, > > /* error responses */ > VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200, > @@ -453,4 +460,27 @@ struct virtio_gpu_resource_unmap_blob { > __le32 padding; > }; > > +/* VIRTIO_GPU_CMD_RESOURCE_QUERY_LAYOUT */ > +struct virtio_gpu_resource_query_layout { > + struct virtio_gpu_ctrl_hdr hdr; > + __le32 resource_id; > + __le32 width; > + __le32 height; > + __le32 format; > + __le32 bind; 64b pad missing > +}; > + > + > +/* VIRTIO_GPU_RESP_OK_RESOURCE_LAYOUT */ > +#define VIRTIO_GPU_RES_MAX_PLANES 4 > +struct virtio_gpu_resp_resource_layout { > + struct virtio_gpu_ctrl_hdr hdr; > + __le64 modifier; > + __le32 num_planes; > + struct virtio_gpu_resource_plane { > + __le64 offset; > +
Re: [PATCH v1 1/1] drm/virtio: Spelling fixes
On 12/19/23 18:19, Andy Shevchenko wrote: > While making a spelling mistake myself for `git grep kvalloc` > I found that the only file has such a typo. Fix it and update > to the standard de facto of how we refer to the functions. > Also spell usr-out as user-out, it seems this driver uses its > own terminology nobody else can decypher, make it more readable. > > Signed-off-by: Andy Shevchenko > --- > drivers/gpu/drm/virtio/virtgpu_submit.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_submit.c > b/drivers/gpu/drm/virtio/virtgpu_submit.c > index 5c514946bbad..1c7c7f61a222 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_submit.c > +++ b/drivers/gpu/drm/virtio/virtgpu_submit.c > @@ -99,8 +99,8 @@ virtio_gpu_parse_deps(struct virtio_gpu_submit *submit) > return 0; > > /* > - * kvalloc at first tries to allocate memory using kmalloc and > - * falls back to vmalloc only on failure. It also uses __GFP_NOWARN > + * kvmalloc() at first tries to allocate memory using kmalloc() and > + * falls back to vmalloc() only on failure. It also uses __GFP_NOWARN >* internally for allocations larger than a page size, preventing >* storm of KMSG warnings. >*/ > @@ -529,7 +529,7 @@ int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, > void *data, > virtio_gpu_submit(); > > /* > - * Set up usr-out data after submitting the job to optimize > + * Set up user-out data after submitting the job to optimize >* the job submission path. >*/ > virtio_gpu_install_out_fence_fd(); Applied to misc-next, thanks -- Best regards, Dmitry
Re: [PATCH v2 23/39] drm/bridge: nxp-ptn3460: switch to ->edid_read callback
Hi Jani, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.7-rc8 next-20240103] [cannot apply to drm-intel/for-linux-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Jani-Nikula/drm-bridge-add-edid_read-hook-and-drm_bridge_edid_read/20240103-181513 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/87fb7fd52d087dd9a15b7194f3915b6b1c4146d6.1704276309.git.jani.nikula%40intel.com patch subject: [PATCH v2 23/39] drm/bridge: nxp-ptn3460: switch to ->edid_read callback config: arm-randconfig-001-20240103 (https://download.01.org/0day-ci/archive/20240104/202401040455.pphqjivr-...@intel.com/config) compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project 7e186d366d6c7def0543acc255931f617e76dff0) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240104/202401040455.pphqjivr-...@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/202401040455.pphqjivr-...@intel.com/ All warnings (new ones prefixed by >>): >> drivers/gpu/drm/bridge/nxp-ptn3460.c:170:6: warning: variable 'drm_edid' is >> used uninitialized whenever 'if' condition is true >> [-Wsometimes-uninitialized] 170 | if (!edid) { | ^ drivers/gpu/drm/bridge/nxp-ptn3460.c:189:9: note: uninitialized use occurs here 189 | return drm_edid; |^~~~ drivers/gpu/drm/bridge/nxp-ptn3460.c:170:2: note: remove the 'if' if its condition is always false 170 | if (!edid) { | ^~~~ 171 | DRM_ERROR("Failed to allocate EDID\n"); | ~~~ 172 | goto out; | ~ 173 | } | ~ drivers/gpu/drm/bridge/nxp-ptn3460.c:161:33: note: initialize the variable 'drm_edid' to silence this warning 161 | const struct drm_edid *drm_edid; |^ | = NULL 1 warning generated. vim +170 drivers/gpu/drm/bridge/nxp-ptn3460.c a9fe713d7d45c6 drivers/gpu/drm/bridge/ptn3460.c Sean Paul2014-02-24 155 a9fe713d7d45c6 drivers/gpu/drm/bridge/ptn3460.c Sean Paul2014-02-24 156 dd6c2ed9dace84 drivers/gpu/drm/bridge/nxp-ptn3460.c Jani Nikula 2024-01-03 157 static const struct drm_edid *ptn3460_edid_read(struct drm_bridge *bridge, 4151c14cdda689 drivers/gpu/drm/bridge/nxp-ptn3460.c Sam Ravnborg 2020-07-27 158struct drm_connector *connector) a9fe713d7d45c6 drivers/gpu/drm/bridge/ptn3460.c Sean Paul2014-02-24 159 { 4151c14cdda689 drivers/gpu/drm/bridge/nxp-ptn3460.c Sam Ravnborg 2020-07-27 160struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge); dd6c2ed9dace84 drivers/gpu/drm/bridge/nxp-ptn3460.c Jani Nikula 2024-01-03 161const struct drm_edid *drm_edid; a9fe713d7d45c6 drivers/gpu/drm/bridge/ptn3460.c Sean Paul2014-02-24 162bool power_off; 4151c14cdda689 drivers/gpu/drm/bridge/nxp-ptn3460.c Sam Ravnborg 2020-07-27 163u8 *edid; 4151c14cdda689 drivers/gpu/drm/bridge/nxp-ptn3460.c Sam Ravnborg 2020-07-27 164int ret; a9fe713d7d45c6 drivers/gpu/drm/bridge/ptn3460.c Sean Paul2014-02-24 165 a9fe713d7d45c6 drivers/gpu/drm/bridge/ptn3460.c Sean Paul2014-02-24 166power_off = !ptn_bridge->enabled; 94d50d57c4403a drivers/gpu/drm/bridge/ptn3460.c Ajay Kumar 2015-01-20 167ptn3460_pre_enable(_bridge->bridge); a9fe713d7d45c6 drivers/gpu/drm/bridge/ptn3460.c Sean Paul2014-02-24 168 a9fe713d7d45c6 drivers/gpu/drm/bridge/ptn3460.c Sean Paul2014-02-24 169edid = kmalloc(EDID_LENGTH, GFP_KERNEL); a9fe713d7d45c6 drivers/gpu/drm/bridge/ptn3460.c Sean Paul2014-02-24 @170if (!edid) { 94d50d57c4403a drivers/gpu/drm/bridge/ptn3460.c Ajay Kumar 2015-01-20 171DRM_ERROR("Failed to allocate EDID\n"); 4151c14cdda689 drivers/gpu/drm/bridge/nxp-ptn3460.c Sam Ravnborg 2020-07-27 172goto out; a9fe713d7d45c6 drivers/gpu/drm/bridge/ptn3460.c Sean Paul2014-02-24 173} a9fe713d7d45c6 drivers/gpu/drm/bridge/ptn3460
Re: [PATCH] drm/msm/a7xx: Fix LLC typo
On Tue, Jan 2, 2024 at 12:12 PM Konrad Dybcio wrote: > > On 2.01.2024 20:33, Rob Clark wrote: > > From: Rob Clark > > > > We'd miss actually activating LLC. > > > > Signed-off-by: Rob Clark > > --- > > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > index a5660d63535b..54dc5eb37f70 100644 > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > @@ -1646,7 +1646,7 @@ static int a6xx_gmu_pm_resume(struct msm_gpu *gpu) > > > > msm_devfreq_resume(gpu); > > > > - adreno_is_a7xx(adreno_gpu) ? a7xx_llc_activate : > > a6xx_llc_activate(a6xx_gpu); > > + adreno_is_a7xx(adreno_gpu) ? a7xx_llc_activate(a6xx_gpu) : > > a6xx_llc_activate(a6xx_gpu); > > /me cleans glasses > > oh.. > > Reviewed-by: Konrad Dybcio I suppose I should also add, Fixes: af66706accdf ("drm/msm/a6xx: Add skeleton A7xx support") > Konrad
[PATCH] drm/rockchip: analogix_dp: get encoder port ID from DT
The VOP2 driver needs this port ID to properly configure the display data routing. Signed-off-by: Lucas Stach --- drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c index 84aa811ca1e9..cea8d1af0ec7 100644 --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c @@ -344,6 +344,9 @@ static int rockchip_dp_bind(struct device *dev, struct device *master, return ret; } + rockchip_drm_encoder_set_crtc_endpoint_id(>encoder, + dev->of_node, 0, 0); + dp->plat_data.encoder = >encoder.encoder; ret = analogix_dp_bind(dp->adp, drm_dev); -- 2.43.0
Re: [1/3] drm: property: One function call less in drm_property_create() after error detection
>>> Out of curiosity, what exactly did Coccinelle report? >> >> Some SmPL scripts from my own selection tend to point questionable >> implementation details out. > > That doesn't answer my question. It should. > Without seeing the actual Coccinelle report, There is no “official” report according to the discussed patch which is triggered by known advices for the application of labels in goto chains. > I'll assume that it didn't actually call for this change. Software development opinions are evolving accordingly. Regards, Markus
Re: [PATCH linux-next] drm/panel: samsung: Simplify with dev_err_probe()
On 1/3/2024 6:17 AM, chenguanxi11...@163.com wrote: From: Chen Haonan dev_err_probe() can check if the error code is -EPROBE_DEFER and can return the error code, replacing dev_err() with it simplifies the code. Signed-off-by: Chen Haonan Reviewed-by: Jessica Zhang --- drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c index 79f611963c61..f4103e762b53 100644 --- a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c +++ b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c @@ -194,10 +194,8 @@ static int s6d16d0_probe(struct mipi_dsi_device *dsi) s6->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(s6->reset_gpio)) { - ret = PTR_ERR(s6->reset_gpio); - if (ret != -EPROBE_DEFER) - dev_err(dev, "failed to request GPIO (%d)\n", ret); - return ret; + return dev_err_probe(dev, PTR_ERR(s6->reset_gpio), +"failed to request GPIO\n"); } drm_panel_init(>panel, dev, _drm_funcs, -- 2.25.1
Re: [PATCH linux-next v2] drm/panel: Simplify with dev_err_probe()
On 12/25/2023 6:26 AM, chenguanxi11...@163.com wrote: From: Chen Haonan dev_err_probe() can check if the error code is -EPROBE_DEFER and can return the error code, replacing dev_err() with it simplifies the code. Signed-off-by: Chen Haonan Reviewed-by: Jessica Zhang --- drivers/gpu/drm/panel/panel-boe-himax8279d.c | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-boe-himax8279d.c b/drivers/gpu/drm/panel/panel-boe-himax8279d.c index 11b64acbe8a9..e225840b0d67 100644 --- a/drivers/gpu/drm/panel/panel-boe-himax8279d.c +++ b/drivers/gpu/drm/panel/panel-boe-himax8279d.c @@ -854,26 +854,20 @@ static int panel_add(struct panel_info *pinfo) pinfo->pp18_gpio = devm_gpiod_get(dev, "pp18", GPIOD_OUT_HIGH); if (IS_ERR(pinfo->pp18_gpio)) { - ret = PTR_ERR(pinfo->pp18_gpio); - if (ret != -EPROBE_DEFER) - dev_err(dev, "failed to get pp18 gpio: %d\n", ret); - return ret; + return dev_err_probe(dev, PTR_ERR(pinfo->pp18_gpio), +"failed to get pp18 gpio\n"); } pinfo->pp33_gpio = devm_gpiod_get(dev, "pp33", GPIOD_OUT_HIGH); if (IS_ERR(pinfo->pp33_gpio)) { - ret = PTR_ERR(pinfo->pp33_gpio); - if (ret != -EPROBE_DEFER) - dev_err(dev, "failed to get pp33 gpio: %d\n", ret); - return ret; + return dev_err_probe(dev, PTR_ERR(pinfo->pp33_gpio), +"failed to get pp33 gpio\n"); } pinfo->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH); if (IS_ERR(pinfo->enable_gpio)) { - ret = PTR_ERR(pinfo->enable_gpio); - if (ret != -EPROBE_DEFER) - dev_err(dev, "failed to get enable gpio: %d\n", ret); - return ret; + return dev_err_probe(dev, PTR_ERR(pinfo->enable_gpio), +"failed to get enable gpio\n"); } drm_panel_init(>base, dev, _funcs, -- 2.25.1
Re: [PATCH 1/3] drm: property: One function call less in drm_property_create() after error detection
On 2024-01-03 17:24, Markus Elfring wrote: > >> Out of curiosity, what exactly did Coccinelle report? > > Some SmPL scripts from my own selection tend to point questionable > implementation details out. That doesn't answer my question. Without seeing the actual Coccinelle report, I'll assume that it didn't actually call for this change. -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [PATCH v9 09/25] drm/modes: Move named modes parsing to a separate function
On Wed, 3 Jan 2024 at 14:02, Dave Stevenson wrote: > > Hi Maxime > > On Wed, 3 Jan 2024 at 13:33, Maxime Ripard wrote: > > > > Hi Dave, > > > > Happy new year :) > > And to you. > > > On Tue, Jan 02, 2024 at 03:12:26PM +, Dave Stevenson wrote: > > > Hi Maxime > > > > > > On Mon, 14 Nov 2022 at 13:00, Maxime Ripard wrote: > > > > > > > > The current construction of the named mode parsing doesn't allow to > > > > extend > > > > it easily. Let's move it to a separate function so we can add more > > > > parameters and modes. > > > > > > > > In order for the tests to still pass, some extra checks are needed, so > > > > it's not a 1:1 move. > > > > > > > > Reviewed-by: Noralf Trønnes > > > > Tested-by: Mateusz Kwiatkowski > > > > Signed-off-by: Maxime Ripard > > > > > > > > --- > > > > Changes in v7: > > > > - Add Noralf Reviewed-by > > > > > > > > Changes in v6: > > > > - Simplify the test for connection status extras > > > > - Simplify the code path to call drm_mode_parse_cmdline_named_mode > > > > > > > > Changes in v4: > > > > - Fold down all the named mode patches that were split into a single > > > > patch again to maintain bisectability > > > > --- > > > > drivers/gpu/drm/drm_modes.c | 70 > > > > + > > > > 1 file changed, 58 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > > > > index 71c050c3ee6b..37542612912b 100644 > > > > --- a/drivers/gpu/drm/drm_modes.c > > > > +++ b/drivers/gpu/drm/drm_modes.c > > > > @@ -2229,6 +2229,51 @@ static const char * const > > > > drm_named_modes_whitelist[] = { > > > > "PAL", > > > > }; > > > > > > > > +static int drm_mode_parse_cmdline_named_mode(const char *name, > > > > +unsigned int name_end, > > > > +struct drm_cmdline_mode > > > > *cmdline_mode) > > > > +{ > > > > + unsigned int i; > > > > + > > > > + if (!name_end) > > > > + return 0; > > > > + > > > > + /* If the name starts with a digit, it's not a named mode */ > > > > + if (isdigit(name[0])) > > > > + return 0; > > > > + > > > > + /* > > > > +* If there's an equal sign in the name, the command-line > > > > +* contains only an option and no mode. > > > > +*/ > > > > + if (strnchr(name, name_end, '=')) > > > > + return 0; > > > > + > > > > + /* The connection status extras can be set without a mode. */ > > > > + if (name_end == 1 && > > > > + (name[0] == 'd' || name[0] == 'D' || name[0] == 'e')) > > > > + return 0; > > > > + > > > > + /* > > > > +* We're sure we're a named mode at this point, iterate over the > > > > +* list of modes we're aware of. > > > > +*/ > > > > + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { > > > > + int ret; > > > > + > > > > + ret = str_has_prefix(name, > > > > drm_named_modes_whitelist[i]); > > > > + if (ret != name_end) > > > > + continue; > > > > + > > > > + strcpy(cmdline_mode->name, > > > > drm_named_modes_whitelist[i]); > > > > + cmdline_mode->specified = true; > > > > + > > > > + return 1; > > > > + } > > > > + > > > > + return -EINVAL; > > > > +} > > > > + > > > > /** > > > > * drm_mode_parse_command_line_for_connector - parse command line > > > > modeline for connector > > > > * @mode_option: optional per connector mode option > > > > @@ -2265,7 +2310,7 @@ bool > > > > drm_mode_parse_command_line_for_connector(const char *mode_option, > > > > const char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = > > > > NULL; > > > > const char *options_ptr = NULL; > > > > char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL; > > > > - int i, len, ret; > > > > + int len, ret; > > > > > > > > memset(mode, 0, sizeof(*mode)); > > > > mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN; > > > > @@ -2306,18 +2351,19 @@ bool > > > > drm_mode_parse_command_line_for_connector(const char *mode_option, > > > > parse_extras = true; > > > > } > > > > > > > > - /* First check for a named mode */ > > > > - for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { > > > > - ret = str_has_prefix(name, > > > > drm_named_modes_whitelist[i]); > > > > - if (ret == mode_end) { > > > > - if (refresh_ptr) > > > > - return false; /* named + refresh is > > > > invalid */ > > > > + if (!mode_end) > > > > + return false; > > > > > > I'm chasing down a change in behaviour between 6.1 and 6.6, and this > > > patch seems to be at least part of the cause. > > > > > > Since [1]
Re: [PATCH 1/3] drm: property: One function call less in drm_property_create() after error detection
>> The kfree() function was called in one case by the >> drm_property_create() function during error handling >> even if the passed data structure member contained a null pointer. >> This issue was detected by using the Coccinelle software. >> >> Thus use another label. … >> +++ b/drivers/gpu/drm/drm_property.c >> @@ -117,7 +117,7 @@ struct drm_property *drm_property_create(struct >> drm_device *dev, >> property->values = kcalloc(num_values, sizeof(uint64_t), >> GFP_KERNEL); >> if (!property->values) >> -goto fail; >> +goto free_property; >> } >> >> ret = drm_mode_object_add(dev, >base, >> DRM_MODE_OBJECT_PROPERTY); >> @@ -135,6 +135,7 @@ struct drm_property *drm_property_create(struct >> drm_device *dev, >> return property; >> fail: >> kfree(property->values); >> +free_property: >> kfree(property); >> return NULL; >> } … > This change is pointless at best, kfree(NULL) works fine. * Would you interpret such a special function call as redundant? * Do you find advices applicable from another information source also for this function implementation? https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources > Out of curiosity, what exactly did Coccinelle report? Some SmPL scripts from my own selection tend to point questionable implementation details out. Regards, Markus
Re: [PATCH v4 2/6] x86/vmware: Introduce VMware hypercall API
On Thu, Dec 28, 2023 at 11:24:17AM -0800, Alexey Makhalov wrote: > From: Alexey Makhalov > > Introduce vmware_hypercall family of functions. It is a common > implementation to be used by the VMware guest code and virtual > device drivers in architecture independent manner. > > The API consists of vmware_hypercallX and vmware_hypercall_hb_{out,in} > set of functions by analogy with KVM hypercall API. Architecture > specific implementation is hidden inside. > > It will simplify future enhancements in VMware hypercalls such > as SEV-ES and TDX related changes without needs to modify a > caller in device drivers code. > > Current implementation extends an idea from commit bac7b4e84323 > ("x86/vmware: Update platform detection code for VMCALL/VMMCALL > hypercalls") to have a slow, but safe path in VMWARE_HYPERCALL > earlier during the boot when alternatives are not yet applied. > This logic was inherited from VMWARE_CMD from the commit mentioned > above. Default alternative code was optimized by size to reduce > excessive nop alignment once alternatives are applied. Total > default code size is 26 bytes, in worse case (3 bytes alternative) > remaining 23 bytes will be aligned by only 3 long NOP instructions. > > Signed-off-by: Alexey Makhalov > Reviewed-by: Nadav Amit > Reviewed-by: Jeff Sipek Hi Alexey, I'd like to flag that this breaks gcc-13 x86_64 allmodconfig builds of the following files. And although this is resolved by the subsequent 3 patches in this series, it does still break bisection. drivers/gpu/drm/vmwgfx/vmwgfx_msg.c drivers/input/mouse/vmmouse.c drivers/ptp/ptp_vmw.c ...
Re: [PATCH] drivers/accel/habanalabs: Remove unnecessary braces from if statement
On 28/12/2023 23:08, Malkoot Khan wrote: The coding style in the Linux kernel prefers not to use braces for single-statement if conditions. This patch removes the unnecessary braces from an if statement in the file drivers/accel/habanalabs/common/command_submission.c, which also resolves a coding style warning. Signed-off-by: Malkoot Khan --- drivers/accel/habanalabs/common/command_submission.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/accel/habanalabs/common/command_submission.c b/drivers/accel/habanalabs/common/command_submission.c index 3aa6eeef443b..39e23d625a3c 100644 --- a/drivers/accel/habanalabs/common/command_submission.c +++ b/drivers/accel/habanalabs/common/command_submission.c @@ -1360,9 +1360,8 @@ static int hl_cs_sanity_checks(struct hl_fpriv *hpriv, union hl_cs_args *args) return -EINVAL; } - if (!hl_device_operational(hdev, )) { + if (!hl_device_operational(hdev, )) return -EBUSY; - } if ((args->in.cs_flags & HL_CS_FLAGS_STAGED_SUBMISSION) && !hdev->supports_staged_submission) { Thanks for the patch. Reviewed-by: Oded Gabbay Applied to -next. Oded
Re: [RFC PATCH] drm/amd/display: fix bandwidth validation failure on DCN 2.1
On Wed, Jan 03, 2024 at 09:44:18AM -0500, Hamza Mahfooz wrote: > On 12/29/23 11:25, Melissa Wen wrote: > > IGT `amdgpu/amd_color/crtc-lut-accuracy` fails right at the beginning of > > the test execution, during atomic check, because DC rejects the > > bandwidth state for a fb sizing 64x64. The test was previously working > > with the deprecated dc_commit_state(). Now using > > dc_validate_with_context() approach, the atomic check needs to perform a > > full state validation. Therefore, set fast_validation to false in the > > dc_validate_global_state call for atomic check. > > > > Fixes: b8272241ff9d ("drm/amd/display: Drop dc_commit_state in favor of > > dc_commit_streams") > > Signed-off-by: Melissa Wen > > --- > > > > Hi, > > > > It's a long story. I was inspecting this bug report: > > - https://gitlab.freedesktop.org/drm/amd/-/issues/2016 > > and noticed the IGT test `igt@amdgpu/amd_color@crtc-lut-accuracy` > > mentioned there wasn't even being executed on a laptop with DCN 2.1 > > (HP HP ENVY x360 Convertible 13-ay1xxx/8929). The test fails right at > > the beginning due to an atomic check rejection, as below: > > > > Starting subtest: crtc-lut-accuracy > > (amd_color:14772) igt_kms-CRITICAL: Test assertion failure function > > igt_display_commit_atomic, file ../lib/igt_kms.c:4530: > > (amd_color:14772) igt_kms-CRITICAL: Failed assertion: ret == 0 > > (amd_color:14772) igt_kms-CRITICAL: Last errno: 22, Invalid argument > > (amd_color:14772) igt_kms-CRITICAL: error: -22 != 0 > > Stack trace: > >#0 ../lib/igt_core.c:1989 __igt_fail_assert() > >#1 [igt_display_commit_atomic+0x44] > >#2 ../tests/amdgpu/amd_color.c:159 __igt_uniquereal_main395() > >#3 ../tests/amdgpu/amd_color.c:395 main() > >#4 ../sysdeps/nptl/libc_start_call_main.h:74 __libc_start_call_main() > >#5 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34() > >#6 [_start+0x21] > > Subtest crtc-lut-accuracy failed. > > > > Checking dmesg, we can see that a bandwidth validation failure causes > > the atomic check rejection: > > > > [ 711.147663] amdgpu :04:00.0: [drm] Mode Validation Warning: Unknown > > Status failed validation. > > [ 711.147667] [drm:amdgpu_dm_atomic_check [amdgpu]] DC global validation > > failure: Bandwidth validation failure (BW and Watermark) (13) > > [ 711.147772] [drm:amdgpu_irq_dispatch [amdgpu]] Unregistered interrupt > > src_id: 243 of client_id:8 > > [ 711.148033] [drm:amdgpu_dm_atomic_check [amdgpu]] Atomic check failed > > with err: -22 > > > > I also noticed that the atomic check doesn't fail if I change the fb > > width and height used in the test from 64 to 66, and I can get the test > > execution back (and with success). However, I recall that all test cases > > of IGT `amd_color` were working in the past, so I bisected and found the > > first bad commit: > > > > b8272241ff9d drm/amd/display: Drop dc_commit_state in favor of > > dc_commit_streams > > > > Bringing the `dc_commit_state` machinery back also prevents the > > bandwidth validation failure, but the commit above says > > dc_commit_streams validation is more complete than dc_commit_state, so I > > discarded this approach. > > > > After some debugging and code inspection, I found out that avoiding fast > > validation on dc_validate_global_state during atomic check solves the > > issue, but I'm not sure if this change may affect performance. I > > compared exec time of some IGT tests and didn't see any differences, but > > I recognize it doesn't provide enough evidence. > > > > What do you think about this change? Should I examine other things? Do > > you see any potential issue that I should investigate? Could you > > recommend a better approach to assess any side-effect of not enabling > > fast validation in the atomic check? > > > > Please, let me know your thoughts. > > We shouldn't be doing fast updates when lock_and_validation_needed is > true, so this seems to be correct. > > Which is to say, applied, thanks! > > Cc: sta...@vger.kernel.org This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly.
Re: [PATCH 1/3] drm: property: One function call less in drm_property_create() after error detection
On 2023-12-26 10:38, Markus Elfring wrote: > From: Markus Elfring > Date: Tue, 26 Dec 2023 08:44:37 +0100 > > The kfree() function was called in one case by the > drm_property_create() function during error handling > even if the passed data structure member contained a null pointer. > This issue was detected by using the Coccinelle software. > > Thus use another label. > > Signed-off-by: Markus Elfring > --- > drivers/gpu/drm/drm_property.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c > index 596272149a35..3440f4560e6e 100644 > --- a/drivers/gpu/drm/drm_property.c > +++ b/drivers/gpu/drm/drm_property.c > @@ -117,7 +117,7 @@ struct drm_property *drm_property_create(struct > drm_device *dev, > property->values = kcalloc(num_values, sizeof(uint64_t), > GFP_KERNEL); > if (!property->values) > - goto fail; > + goto free_property; > } > > ret = drm_mode_object_add(dev, >base, > DRM_MODE_OBJECT_PROPERTY); > @@ -135,6 +135,7 @@ struct drm_property *drm_property_create(struct > drm_device *dev, > return property; > fail: > kfree(property->values); > +free_property: > kfree(property); > return NULL; > } > -- > 2.43.0 > This change is pointless at best, kfree(NULL) works fine. Out of curiosity, what exactly did Coccinelle report? -- Earthling Michel Dänzer| https://redhat.com Libre software enthusiast | Mesa and Xwayland developer
Re: [syzbot] [dri?] WARNING in drm_prime_destroy_file_private (2)
Am 28.12.23 um 03:57 schrieb Qi Zheng: On 2023/12/28 04:51, syzbot wrote: Hello, syzbot found the following issue on: HEAD commit: 5254c0cbc92d Merge tag 'block-6.7-2023-12-22' of git://git.. git tree: upstream console+strace: https://syzkaller.appspot.com/x/log.txt?x=10cc6995e8 kernel config: https://syzkaller.appspot.com/x/.config?x=314e9ad033a7d3a7 dashboard link: https://syzkaller.appspot.com/bug?extid=59dcc2e7283a6f5f5ba1 compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13e35809e8 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=155d5fd6e8 Downloadable assets: disk image: https://storage.googleapis.com/syzbot-assets/ebe09a5995ee/disk-5254c0cb.raw.xz vmlinux: https://storage.googleapis.com/syzbot-assets/02178d7f5f98/vmlinux-5254c0cb.xz kernel image: https://storage.googleapis.com/syzbot-assets/12307f47d87c/bzImage-5254c0cb.xz The issue was bisected to: commit ea4452de2ae987342fadbdd2c044034e6480daad Author: Qi Zheng Date: Fri Nov 18 10:00:11 2022 + mm: fix unexpected changes to {failslab|fail_page_alloc}.attr bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=13027f76e8 final oops: https://syzkaller.appspot.com/x/report.txt?x=10827f76e8 console output: https://syzkaller.appspot.com/x/log.txt?x=17027f76e8 IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+59dcc2e7283a6f5f5...@syzkaller.appspotmail.com Fixes: ea4452de2ae9 ("mm: fix unexpected changes to {failslab|fail_page_alloc}.attr") R10: R11: 0246 R12: 7efe98069194 R13: 7efe97fd2210 R14: 0002 R15: 6972642f7665642f [ cut here ] WARNING: CPU: 0 PID: 5107 at drivers/gpu/drm/drm_prime.c:227 drm_prime_destroy_file_private+0x43/0x60 drivers/gpu/drm/drm_prime.c:227 The warning is caused by !RB_EMPTY_ROOT(_fpriv->dmabufs): drm_prime_destroy_file_private --> WARN_ON(!RB_EMPTY_ROOT(_fpriv->dmabufs)); It seems irrelevant to the logic of fault injection. So I don't see why the commit ea4452de2ae9 can cause this warning. :( Making an educated guess I strongly think syzbot incorrectly bisected this. What basically happens is that a DRM test case crashes because a file private data structure is destroyed before all DMA-bufs referring to it are destroyed. Looks like a random race condition in a test case to me. Question is really what test is syzbot running and who is maintaining this test case? Regards, Christian. Modules linked in: CPU: 0 PID: 5107 Comm: syz-executor227 Not tainted 6.7.0-rc6-syzkaller-00248-g5254c0cbc92d #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 11/17/2023 RIP: 0010:drm_prime_destroy_file_private+0x43/0x60 drivers/gpu/drm/drm_prime.c:227 Code: 00 00 fc ff df 48 89 fa 48 c1 ea 03 80 3c 02 00 75 21 48 8b 83 90 00 00 00 48 85 c0 75 06 5b e9 13 f1 93 fc e8 0e f1 93 fc 90 <0f> 0b 90 5b e9 04 f1 93 fc e8 3f 9b ea fc eb d8 66 66 2e 0f 1f 84 RSP: 0018:c90003bdf9e0 EFLAGS: 00010293 RAX: RBX: 888019f28378 RCX: c90003bdf9b0 RDX: 888018ff9dc0 RSI: 84f380c2 RDI: 888019f28408 RBP: 888019f28000 R08: 0001 R09: 0001 R10: 8f193a57 R11: R12: 88814829a000 R13: 888019f282a8 R14: 88814829a068 R15: 88814829a0a0 FS: () GS:8880b980() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 7efe98050410 CR3: 6d1ff000 CR4: 00350ef0 Call Trace: drm_file_free.part.0+0x738/0xb90 drivers/gpu/drm/drm_file.c:290 drm_file_free drivers/gpu/drm/drm_file.c:247 [inline] drm_close_helper.isra.0+0x180/0x1f0 drivers/gpu/drm/drm_file.c:307 drm_release+0x22a/0x4f0 drivers/gpu/drm/drm_file.c:494 __fput+0x270/0xb70 fs/file_table.c:394 task_work_run+0x14d/0x240 kernel/task_work.c:180 exit_task_work include/linux/task_work.h:38 [inline] do_exit+0xa8a/0x2ad0 kernel/exit.c:869 do_group_exit+0xd4/0x2a0 kernel/exit.c:1018 get_signal+0x23b5/0x2790 kernel/signal.c:2904 arch_do_signal_or_restart+0x90/0x7f0 arch/x86/kernel/signal.c:309 exit_to_user_mode_loop kernel/entry/common.c:168 [inline] exit_to_user_mode_prepare+0x121/0x240 kernel/entry/common.c:204 __syscall_exit_to_user_mode_work kernel/entry/common.c:285 [inline] syscall_exit_to_user_mode+0x1e/0x60 kernel/entry/common.c:296 do_syscall_64+0x4d/0x110 arch/x86/entry/common.c:89 entry_SYSCALL_64_after_hwframe+0x63/0x6b RIP: 0033:0x7efe98014769 Code: Unable to access opcode bytes at 0x7efe9801473f. RSP: 002b:7efe97fd2208 EFLAGS: 0246 ORIG_RAX: 00ca RAX: fe00 RBX: 7efe9809c408 RCX: 7efe98014769 RDX: RSI: 0080 RDI: 7efe9809c408 RBP: 7efe9809c400 R08: 3131 R09: 3131
Re: [PATCH] drm/amd/display: avoid stringop-overflow warnings for dp_decide_lane_settings()
Applied. Thanks! On Mon, Dec 25, 2023 at 5:30 AM Randy Dunlap wrote: > > > > On 11/22/23 14:13, Arnd Bergmann wrote: > > From: Arnd Bergmann > > > > gcc prints a warning about a possible array overflow for a couple of > > callers of dp_decide_lane_settings() after commit 1b56c90018f0 ("Makefile: > > Enable -Wstringop-overflow globally"): > > > > drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_training_fixed_vs_pe_retimer.c: > > In function 'dp_perform_fixed_vs_pe_training_sequence_legacy': > > drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_training_fixed_vs_pe_retimer.c:426:25: > > error: 'dp_decide_lane_settings' accessing 4 bytes in a region of size 1 > > [-Werror=stringop-overflow=] > > 426 | dp_decide_lane_settings(lt_settings, > > dpcd_lane_adjust, > > | > > ^~ > > 427 | > > lt_settings->hw_lane_settings, lt_settings->dpcd_lane_settings); > > | > > ~~~ > > drivers/gpu/drm/amd/amdgpu/../display/dc/link/protocols/link_dp_training_fixed_vs_pe_retimer.c:426:25: > > note: referencing argument 4 of type 'union dpcd_training_lane[4]' > > > > I'm not entirely sure what caused this, but changing the prototype to expect > > a pointer instead of an array avoids the warnings. > > > > Fixes: 7727e7b60f82 ("drm/amd/display: Improve robustness of FIXED_VS link > > training at DP1 rates") > > Signed-off-by: Arnd Bergmann > > > Acked-by: Randy Dunlap > Tested-by: Randy Dunlap # build-tested > > Thanks. > > > --- > > .../gpu/drm/amd/display/dc/link/protocols/link_dp_training.c| 2 +- > > .../gpu/drm/amd/display/dc/link/protocols/link_dp_training.h| 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git > > a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c > > b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c > > index 90339c2dfd84..5a0b04518956 100644 > > --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c > > +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.c > > @@ -807,7 +807,7 @@ void dp_decide_lane_settings( > > const struct link_training_settings *lt_settings, > > const union lane_adjust ln_adjust[LANE_COUNT_DP_MAX], > > struct dc_lane_settings hw_lane_settings[LANE_COUNT_DP_MAX], > > - union dpcd_training_lane > > dpcd_lane_settings[LANE_COUNT_DP_MAX]) > > + union dpcd_training_lane *dpcd_lane_settings) > > { > > uint32_t lane; > > > > diff --git > > a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.h > > b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.h > > index 7d027bac8255..851bd17317a0 100644 > > --- a/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.h > > +++ b/drivers/gpu/drm/amd/display/dc/link/protocols/link_dp_training.h > > @@ -111,7 +111,7 @@ void dp_decide_lane_settings( > > const struct link_training_settings *lt_settings, > > const union lane_adjust ln_adjust[LANE_COUNT_DP_MAX], > > struct dc_lane_settings hw_lane_settings[LANE_COUNT_DP_MAX], > > - union dpcd_training_lane dpcd_lane_settings[LANE_COUNT_DP_MAX]); > > + union dpcd_training_lane *dpcd_lane_settings); > > > > enum dc_dp_training_pattern decide_cr_training_pattern( > > const struct dc_link_settings *link_settings); > > -- > #Randy > https://people.kernel.org/tglx/notes-about-netiquette > https://subspace.kernel.org/etiquette.html
Re: [PATCH 08/11] nouveau/gsp: don't free ctrl messages on errors
Hi Dave, kernel test robot noticed the following build warnings: https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Dave-Airlie/nouveau-gsp-drop-some-acpi-related-debug/20231222-180432 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20231222043308.3090089-9-airlied%40gmail.com patch subject: [PATCH 08/11] nouveau/gsp: don't free ctrl messages on errors config: powerpc-randconfig-r071-20231226 (https://download.01.org/0day-ci/archive/20231227/202312271917.55xudmdc-...@intel.com/config) compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project d3ef86708241a3bee902615c190dead1638c4e09) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Reported-by: Dan Carpenter | Closes: https://lore.kernel.org/r/202312271917.55xudmdc-...@intel.com/ New smatch warnings: drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:659 r535_gsp_rpc_rm_ctrl_push() warn: passing zero to 'PTR_ERR' drivers/gpu/drm/nouveau/nvkm/engine/disp/r535.c:1063 r535_dp_aux_xfer() warn: passing a valid pointer to 'PTR_ERR' Old smatch warnings: drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c:1887 nvkm_gsp_radix3_sg() error: uninitialized symbol 'addr'. vim +/PTR_ERR +659 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c af265ee961627a Dave Airlie 2023-12-22 649 static int af265ee961627a Dave Airlie 2023-12-22 650 r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object, void **argv, u32 repc) 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 651 { af265ee961627a Dave Airlie 2023-12-22 652 rpc_gsp_rm_control_v03_00 *rpc = container_of((*argv), typeof(*rpc), params); 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 653 struct nvkm_gsp *gsp = object->client->gsp; af265ee961627a Dave Airlie 2023-12-22 654 int ret = 0; 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 655 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 656 rpc = nvkm_gsp_rpc_push(gsp, rpc, true, repc); af265ee961627a Dave Airlie 2023-12-22 657 if (IS_ERR_OR_NULL(rpc)) { af265ee961627a Dave Airlie 2023-12-22 658 *argv = NULL; af265ee961627a Dave Airlie 2023-12-22 @659 return PTR_ERR(rpc); If nvkm_gsp_rpc_push() returns NULL (probably a failure) then this returns PTR_ERR(NULL) which is zero/success. af265ee961627a Dave Airlie 2023-12-22 660 } 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 661 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 662 if (rpc->status) { af265ee961627a Dave Airlie 2023-12-22 663 ret = r535_rpc_status_to_errno(rpc->status); 555bb9c29a45be Dave Airlie 2023-12-22 664 if (ret != -EAGAIN) 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 665 nvkm_error(>subdev, "cli:0x%08x obj:0x%08x ctrl cmd:0x%08x failed: 0x%08x\n", 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 666 object->client->object.handle, object->handle, rpc->cmd, rpc->status); 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 667 } 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 668 af265ee961627a Dave Airlie 2023-12-22 669 if (repc) af265ee961627a Dave Airlie 2023-12-22 670 *argv = rpc->params; af265ee961627a Dave Airlie 2023-12-22 671 else 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 672 nvkm_gsp_rpc_done(gsp, rpc); 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 673 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 674 return ret; 4cf2c83eb3a4c4 Ben Skeggs 2023-09-19 675 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [RFC PATCH] drm/amd/display: fix bandwidth validation failure on DCN 2.1
On 12/29/23 11:25, Melissa Wen wrote: IGT `amdgpu/amd_color/crtc-lut-accuracy` fails right at the beginning of the test execution, during atomic check, because DC rejects the bandwidth state for a fb sizing 64x64. The test was previously working with the deprecated dc_commit_state(). Now using dc_validate_with_context() approach, the atomic check needs to perform a full state validation. Therefore, set fast_validation to false in the dc_validate_global_state call for atomic check. Fixes: b8272241ff9d ("drm/amd/display: Drop dc_commit_state in favor of dc_commit_streams") Signed-off-by: Melissa Wen --- Hi, It's a long story. I was inspecting this bug report: - https://gitlab.freedesktop.org/drm/amd/-/issues/2016 and noticed the IGT test `igt@amdgpu/amd_color@crtc-lut-accuracy` mentioned there wasn't even being executed on a laptop with DCN 2.1 (HP HP ENVY x360 Convertible 13-ay1xxx/8929). The test fails right at the beginning due to an atomic check rejection, as below: Starting subtest: crtc-lut-accuracy (amd_color:14772) igt_kms-CRITICAL: Test assertion failure function igt_display_commit_atomic, file ../lib/igt_kms.c:4530: (amd_color:14772) igt_kms-CRITICAL: Failed assertion: ret == 0 (amd_color:14772) igt_kms-CRITICAL: Last errno: 22, Invalid argument (amd_color:14772) igt_kms-CRITICAL: error: -22 != 0 Stack trace: #0 ../lib/igt_core.c:1989 __igt_fail_assert() #1 [igt_display_commit_atomic+0x44] #2 ../tests/amdgpu/amd_color.c:159 __igt_uniquereal_main395() #3 ../tests/amdgpu/amd_color.c:395 main() #4 ../sysdeps/nptl/libc_start_call_main.h:74 __libc_start_call_main() #5 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34() #6 [_start+0x21] Subtest crtc-lut-accuracy failed. Checking dmesg, we can see that a bandwidth validation failure causes the atomic check rejection: [ 711.147663] amdgpu :04:00.0: [drm] Mode Validation Warning: Unknown Status failed validation. [ 711.147667] [drm:amdgpu_dm_atomic_check [amdgpu]] DC global validation failure: Bandwidth validation failure (BW and Watermark) (13) [ 711.147772] [drm:amdgpu_irq_dispatch [amdgpu]] Unregistered interrupt src_id: 243 of client_id:8 [ 711.148033] [drm:amdgpu_dm_atomic_check [amdgpu]] Atomic check failed with err: -22 I also noticed that the atomic check doesn't fail if I change the fb width and height used in the test from 64 to 66, and I can get the test execution back (and with success). However, I recall that all test cases of IGT `amd_color` were working in the past, so I bisected and found the first bad commit: b8272241ff9d drm/amd/display: Drop dc_commit_state in favor of dc_commit_streams Bringing the `dc_commit_state` machinery back also prevents the bandwidth validation failure, but the commit above says dc_commit_streams validation is more complete than dc_commit_state, so I discarded this approach. After some debugging and code inspection, I found out that avoiding fast validation on dc_validate_global_state during atomic check solves the issue, but I'm not sure if this change may affect performance. I compared exec time of some IGT tests and didn't see any differences, but I recognize it doesn't provide enough evidence. What do you think about this change? Should I examine other things? Do you see any potential issue that I should investigate? Could you recommend a better approach to assess any side-effect of not enabling fast validation in the atomic check? Please, let me know your thoughts. We shouldn't be doing fast updates when lock_and_validation_needed is true, so this seems to be correct. Which is to say, applied, thanks! Cc: sta...@vger.kernel.org Happy New Year! Melissa drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 2845c884398e..4f51a7ad7a3c 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -10745,7 +10745,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev, DRM_DEBUG_DRIVER("drm_dp_mst_atomic_check() failed\n"); goto fail; } - status = dc_validate_global_state(dc, dm_state->context, true); + status = dc_validate_global_state(dc, dm_state->context, false); if (status != DC_OK) { DRM_DEBUG_DRIVER("DC global validation failure: %s (%d)", dc_status_to_str(status), status); -- Hamza
Re: [PATCH] drm/amd/pm/smu7: fix a memleak in smu7_hwmgr_backend_init
Applied. Thanks! On Mon, Dec 25, 2023 at 5:18 AM Zhipeng Lu wrote: > > The hwmgr->backend, (i.e. data) allocated by kzalloc is not freed in > the error-handling paths of smu7_get_evv_voltages and > smu7_update_edc_leakage_table. However, it did be freed in the > error-handling of phm_initializa_dynamic_state_adjustment_rule_settings, > by smu7_hwmgr_backend_fini. So the lack of free in smu7_get_evv_voltages > and smu7_update_edc_leakage_table is considered a memleak in this patch. > > Fixes: 599a7e9fe1b6 ("drm/amd/powerplay: implement smu7 hwmgr to manager > asics with smu ip version 7.") > Fixes: 8f0804c6b7d0 ("drm/amd/pm: add edc leakage controller setting") > Signed-off-by: Zhipeng Lu > --- > drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c > b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c > index 11372fcc59c8..b1a8799e2dee 100644 > --- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c > +++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/smu7_hwmgr.c > @@ -2974,6 +2974,8 @@ static int smu7_hwmgr_backend_init(struct pp_hwmgr > *hwmgr) > result = smu7_get_evv_voltages(hwmgr); > if (result) { > pr_info("Get EVV Voltage Failed. Abort Driver > loading!\n"); > + kfree(hwmgr->backend); > + hwmgr->backend = NULL; > return -EINVAL; > } > } else { > @@ -3019,8 +3021,10 @@ static int smu7_hwmgr_backend_init(struct pp_hwmgr > *hwmgr) > } > > result = smu7_update_edc_leakage_table(hwmgr); > - if (result) > + if (result) { > + smu7_hwmgr_backend_fini(hwmgr); > return result; > + } > > return 0; > } > -- > 2.34.1 >
Re: [PATCH v3 4/4] arm64: dts: rockchip: Add devicetree for Pine64 PineTab2
Hi Manuel, On 2024-01-03 14:40, Manuel Traut wrote: > Hi Jonas and Ondřej, > + { + pinctrl-names = "default"; + pinctrl-0 = <_dual_io_pins>; + status = "okay"; + #address-cells = <1>; + #size-cells = <0>; + + flash@0 { + compatible = "jedec,spi-nor"; + reg = <0>; + spi-max-frequency = <2400>; >>> >>> That's a bit on the low side. The flash chip should work for all commands >>> up to >>> 80MHz https://megous.com/dl/tmp/b428ad9b85ac4633.png and SGM3157YC6 switch >>> for the FSPI-CLK should have high enough bandwidth, too. >> >> I agree that this is a little bit on the low side, it was a safe rate >> that I used for U-Boot. U-Boot required an exact rate of the supported >> sfc clk rates: 24, 50, 75, 100, 125 or 150 MHz. >> >> Please also note that the SPI NOR flash chip used in PineTab2 is not a >> GigaDevice GD25LQ128E, it should be a SiliconKaiser SK25LP128, same as >> found in the Pine64 PinePhone Pro. > > The schematics for v2.0 reference a GD25LQ128EWIGR. I never checked the jedec > id. How did you retrieve this information, or is it maybe a difference in v0.1 > and 2.0? This was when working on mainline U-Boot for the PineTab2 (and other rk356x devices). See [1] for a pending U-Boot patch that is waiting on a proper mainline linux devicetree for the PT2. The JEDEC ID is reported as 0x257018 on my v2.0 production unit, and does not match the JEDEC ID for GD25LQ128E (0xc86018) referenced in the schematics. I found that the JEDEC ID 0x257018 was referenced in prior patches related to the SK25LP128 SPI NOR flash chip used in Pine64 PinePhone Pro. I have only ever tested the 24 MHz rate, but I am expecting that e.g. 100 MHz also should work. Will not be able to test on my PT2 until at earliest next week. [1] https://github.com/Kwiboo/u-boot-rockchip/commit/66562d6eaf2c11a9f97fcdba379d3ceda8aa70ef Regards, Jonas > + spi-rx-bus-width = <2>; >>> >>> GD25LQ128E supports quad I/O. Maybe try 4 if it will work. >> >> The schematic only shows fspi D0 and D1 connected, and use the D2 line >> for eMMC_RSTn, so spi-rx-bus-width = <2> should be correct. > > ack > > Since it is only needed for bootloader updates and environment its maybe > better > to stay on the safe side? > > But I can test faster frequency if you want me to do.. > > Regards > Manuel
Re: [PATCH] drm/amdkfd: fixes for HMM mem allocation
On Sun, Dec 31, 2023 at 9:39 AM Dafna Hirschfeld wrote: > > Few fixes to amdkfd and the doc of > devm_request_free_mem_region. > > Signed-off-by: Dafna Hirschfeld > --- > drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 6 +++--- > kernel/resource.c| 2 +- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c > b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c > index 6c25dab051d5..b8680e0753ca 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c > @@ -1021,7 +1021,7 @@ int kgd2kfd_init_zone_device(struct amdgpu_device *adev) > } else { > res = devm_request_free_mem_region(adev->dev, > _resource, size); > if (IS_ERR(res)) > - return -ENOMEM; > + return PTR_ERR(res); > pgmap->range.start = res->start; > pgmap->range.end = res->end; > pgmap->type = MEMORY_DEVICE_PRIVATE; > @@ -1037,10 +1037,10 @@ int kgd2kfd_init_zone_device(struct amdgpu_device > *adev) > r = devm_memremap_pages(adev->dev, pgmap); > if (IS_ERR(r)) { > pr_err("failed to register HMM device memory\n"); > - /* Disable SVM support capability */ > - pgmap->type = 0; > if (pgmap->type == MEMORY_DEVICE_PRIVATE) > devm_release_mem_region(adev->dev, res->start, > resource_size(res)); > + /* Disable SVM support capability */ > + pgmap->type = 0; > return PTR_ERR(r); > } > > diff --git a/kernel/resource.c b/kernel/resource.c > index 866ef3663a0b..fe890b874606 100644 > --- a/kernel/resource.c > +++ b/kernel/resource.c > @@ -1905,8 +1905,8 @@ get_free_mem_region(struct device *dev, struct resource > *base, > * devm_request_free_mem_region - find free region for device private memory > * > * @dev: device struct to bind the resource to > - * @size: size in bytes of the device memory to add > * @base: resource tree to look in > + * @size: size in bytes of the device memory to add This change should be a separate patch? It looks unrelated. Alex > * > * This function tries to find an empty range of physical address big enough > to > * contain the new resource, so that it can later be hotplugged as > ZONE_DEVICE > -- > 2.34.1 >
[PATCH linux-next] drm/panel: samsung: Simplify with dev_err_probe()
From: Chen Haonan dev_err_probe() can check if the error code is -EPROBE_DEFER and can return the error code, replacing dev_err() with it simplifies the code. Signed-off-by: Chen Haonan --- drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c index 79f611963c61..f4103e762b53 100644 --- a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c +++ b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c @@ -194,10 +194,8 @@ static int s6d16d0_probe(struct mipi_dsi_device *dsi) s6->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(s6->reset_gpio)) { - ret = PTR_ERR(s6->reset_gpio); - if (ret != -EPROBE_DEFER) - dev_err(dev, "failed to request GPIO (%d)\n", ret); - return ret; + return dev_err_probe(dev, PTR_ERR(s6->reset_gpio), +"failed to request GPIO\n"); } drm_panel_init(>panel, dev, _drm_funcs, -- 2.25.1
Re: [PATCH v9 09/25] drm/modes: Move named modes parsing to a separate function
Hi Maxime On Wed, 3 Jan 2024 at 13:33, Maxime Ripard wrote: > > Hi Dave, > > Happy new year :) And to you. > On Tue, Jan 02, 2024 at 03:12:26PM +, Dave Stevenson wrote: > > Hi Maxime > > > > On Mon, 14 Nov 2022 at 13:00, Maxime Ripard wrote: > > > > > > The current construction of the named mode parsing doesn't allow to extend > > > it easily. Let's move it to a separate function so we can add more > > > parameters and modes. > > > > > > In order for the tests to still pass, some extra checks are needed, so > > > it's not a 1:1 move. > > > > > > Reviewed-by: Noralf Trønnes > > > Tested-by: Mateusz Kwiatkowski > > > Signed-off-by: Maxime Ripard > > > > > > --- > > > Changes in v7: > > > - Add Noralf Reviewed-by > > > > > > Changes in v6: > > > - Simplify the test for connection status extras > > > - Simplify the code path to call drm_mode_parse_cmdline_named_mode > > > > > > Changes in v4: > > > - Fold down all the named mode patches that were split into a single > > > patch again to maintain bisectability > > > --- > > > drivers/gpu/drm/drm_modes.c | 70 > > > + > > > 1 file changed, 58 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > > > index 71c050c3ee6b..37542612912b 100644 > > > --- a/drivers/gpu/drm/drm_modes.c > > > +++ b/drivers/gpu/drm/drm_modes.c > > > @@ -2229,6 +2229,51 @@ static const char * const > > > drm_named_modes_whitelist[] = { > > > "PAL", > > > }; > > > > > > +static int drm_mode_parse_cmdline_named_mode(const char *name, > > > +unsigned int name_end, > > > +struct drm_cmdline_mode > > > *cmdline_mode) > > > +{ > > > + unsigned int i; > > > + > > > + if (!name_end) > > > + return 0; > > > + > > > + /* If the name starts with a digit, it's not a named mode */ > > > + if (isdigit(name[0])) > > > + return 0; > > > + > > > + /* > > > +* If there's an equal sign in the name, the command-line > > > +* contains only an option and no mode. > > > +*/ > > > + if (strnchr(name, name_end, '=')) > > > + return 0; > > > + > > > + /* The connection status extras can be set without a mode. */ > > > + if (name_end == 1 && > > > + (name[0] == 'd' || name[0] == 'D' || name[0] == 'e')) > > > + return 0; > > > + > > > + /* > > > +* We're sure we're a named mode at this point, iterate over the > > > +* list of modes we're aware of. > > > +*/ > > > + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { > > > + int ret; > > > + > > > + ret = str_has_prefix(name, drm_named_modes_whitelist[i]); > > > + if (ret != name_end) > > > + continue; > > > + > > > + strcpy(cmdline_mode->name, drm_named_modes_whitelist[i]); > > > + cmdline_mode->specified = true; > > > + > > > + return 1; > > > + } > > > + > > > + return -EINVAL; > > > +} > > > + > > > /** > > > * drm_mode_parse_command_line_for_connector - parse command line > > > modeline for connector > > > * @mode_option: optional per connector mode option > > > @@ -2265,7 +2310,7 @@ bool > > > drm_mode_parse_command_line_for_connector(const char *mode_option, > > > const char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = > > > NULL; > > > const char *options_ptr = NULL; > > > char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL; > > > - int i, len, ret; > > > + int len, ret; > > > > > > memset(mode, 0, sizeof(*mode)); > > > mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN; > > > @@ -2306,18 +2351,19 @@ bool > > > drm_mode_parse_command_line_for_connector(const char *mode_option, > > > parse_extras = true; > > > } > > > > > > - /* First check for a named mode */ > > > - for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { > > > - ret = str_has_prefix(name, drm_named_modes_whitelist[i]); > > > - if (ret == mode_end) { > > > - if (refresh_ptr) > > > - return false; /* named + refresh is > > > invalid */ > > > + if (!mode_end) > > > + return false; > > > > I'm chasing down a change in behaviour between 6.1 and 6.6, and this > > patch seems to be at least part of the cause. > > > > Since [1] we've had the emulated framebuffer on Pi being 16bpp to save > > memory. All good. > > > > It used to be possible to use "video=HDMI-A-1:-32" on the kernel > > command line to set it back to 32bpp. > > > > After this patch that is no longer possible. "mode_end = bpp_off", and > > "bpp_off = bpp_ptr - name", so with bpp_ptr = name we get mode_end > >
Re: [PATCH] nightly.conf: Add the xe repo to drm-tip
On Tue, 2023-12-26 at 13:30 -0500, Rodrigo Vivi wrote: > On Fri, Dec 22, 2023 at 12:36:39PM +0100, Thomas Hellström wrote: > > Add the xe repo to drm-tip and the dim tools. > > For now use the sha1 of the first drm-xe-next pull request for drm- > > tip, > > since that branch tip is currently adapted for our CI testing. > > > > Cc: Rodrigo Vivi > > Cc: Lucas De Marchi > > Cc: Oded Gabbay > > Cc: daniel.vet...@ffwll.ch > > Cc: Maarten Lankhorst > > Cc: dim-to...@lists.freedesktop.org > > Cc: dri-devel@lists.freedesktop.org > > Cc: intel-...@lists.freedesktop.org > > Signed-off-by: Thomas Hellström > > --- > > nightly.conf | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/nightly.conf b/nightly.conf > > index 24126b61b797..accd3ff2cc39 100644 > > --- a/nightly.conf > > +++ b/nightly.conf > > @@ -24,6 +24,10 @@ git://anongit.freedesktop.org/drm-tip > > https://anongit.freedesktop.org/git/drm/drm-tip > > https://anongit.freedesktop.org/git/drm/drm-tip.git > > " > > +drm_tip_repos[drm-xe]=" > > +ssh://g...@gitlab.freedesktop.org/drm/xe/kernel.git > > +https://gitlab.freedesktop.org/drm/xe/kernel.git > > +" > > drm_tip_repos[drm-intel]=" > > ssh://git.freedesktop.org/git/drm/drm-intel > > ssh://git.freedesktop.org/git/drm-intel > > @@ -65,14 +69,17 @@ drm_tip_config=( > > "drmdrm-fixes" > > "drm-misc drm-misc-fixes" > > "drm-intel drm-intel-fixes" > > + "drm-xe drm-xe-fixes" > > > > "drmdrm-next" > > "drm-misc drm-misc-next-fixes" > > "drm-intel drm-intel-next-fixes" > > + "drm-xe drm-xe-next-fixes" > > > > "drm-misc drm-misc-next" > > "drm-intel drm-intel-next" > > "drm-intel drm-intel-gt-next" > > + "drm-xe drm-xe-next b6e1b7081768" > > yeap, up to this commit nothing else should change, but > then we will need an extra rebase of the rest on top of drm/drm-next. > > But then we need to decide where these following patches will live: > 880277f31cc69 drm/xe/guc: define LNL FW > 2cfc5ae1b8267 drm/xe/guc: define PVC FW > 52383b58eb8cf mei/hdcp: Also enable for XE > bea27d7369855 mei: gsc: add support for auxiliary device created by > Xe driver > fcb3410197f05 fault-inject: Include linux/types.h by default. > 8ebd9cd71f8ac drm/xe: Add PVC's PCI device IDs > > > Will it be the topic/core-for-CI? > or topic/xe-extras? > or what? This sounds to me like topic/core-for-CI? Or is there any drawback with that? > > Anyway, for the inclusion like this, after our CI is ready: Could we merge this patch already at this point, considering it will, at least for now, only update drm-tip with our fixes? Thanks, /Thomas > > Acked-by: Rodrigo Vivi > > > > > "drm-intel topic/core-for-CI" > > "drm-misc topic/i915-ttm" > > -- > > 2.42.0 > >
Re: [PATCH v9 09/25] drm/modes: Move named modes parsing to a separate function
Hi Dave, Happy new year :) On Tue, Jan 02, 2024 at 03:12:26PM +, Dave Stevenson wrote: > Hi Maxime > > On Mon, 14 Nov 2022 at 13:00, Maxime Ripard wrote: > > > > The current construction of the named mode parsing doesn't allow to extend > > it easily. Let's move it to a separate function so we can add more > > parameters and modes. > > > > In order for the tests to still pass, some extra checks are needed, so > > it's not a 1:1 move. > > > > Reviewed-by: Noralf Trønnes > > Tested-by: Mateusz Kwiatkowski > > Signed-off-by: Maxime Ripard > > > > --- > > Changes in v7: > > - Add Noralf Reviewed-by > > > > Changes in v6: > > - Simplify the test for connection status extras > > - Simplify the code path to call drm_mode_parse_cmdline_named_mode > > > > Changes in v4: > > - Fold down all the named mode patches that were split into a single > > patch again to maintain bisectability > > --- > > drivers/gpu/drm/drm_modes.c | 70 > > + > > 1 file changed, 58 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > > index 71c050c3ee6b..37542612912b 100644 > > --- a/drivers/gpu/drm/drm_modes.c > > +++ b/drivers/gpu/drm/drm_modes.c > > @@ -2229,6 +2229,51 @@ static const char * const > > drm_named_modes_whitelist[] = { > > "PAL", > > }; > > > > +static int drm_mode_parse_cmdline_named_mode(const char *name, > > +unsigned int name_end, > > +struct drm_cmdline_mode > > *cmdline_mode) > > +{ > > + unsigned int i; > > + > > + if (!name_end) > > + return 0; > > + > > + /* If the name starts with a digit, it's not a named mode */ > > + if (isdigit(name[0])) > > + return 0; > > + > > + /* > > +* If there's an equal sign in the name, the command-line > > +* contains only an option and no mode. > > +*/ > > + if (strnchr(name, name_end, '=')) > > + return 0; > > + > > + /* The connection status extras can be set without a mode. */ > > + if (name_end == 1 && > > + (name[0] == 'd' || name[0] == 'D' || name[0] == 'e')) > > + return 0; > > + > > + /* > > +* We're sure we're a named mode at this point, iterate over the > > +* list of modes we're aware of. > > +*/ > > + for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { > > + int ret; > > + > > + ret = str_has_prefix(name, drm_named_modes_whitelist[i]); > > + if (ret != name_end) > > + continue; > > + > > + strcpy(cmdline_mode->name, drm_named_modes_whitelist[i]); > > + cmdline_mode->specified = true; > > + > > + return 1; > > + } > > + > > + return -EINVAL; > > +} > > + > > /** > > * drm_mode_parse_command_line_for_connector - parse command line modeline > > for connector > > * @mode_option: optional per connector mode option > > @@ -2265,7 +2310,7 @@ bool drm_mode_parse_command_line_for_connector(const > > char *mode_option, > > const char *bpp_ptr = NULL, *refresh_ptr = NULL, *extra_ptr = NULL; > > const char *options_ptr = NULL; > > char *bpp_end_ptr = NULL, *refresh_end_ptr = NULL; > > - int i, len, ret; > > + int len, ret; > > > > memset(mode, 0, sizeof(*mode)); > > mode->panel_orientation = DRM_MODE_PANEL_ORIENTATION_UNKNOWN; > > @@ -2306,18 +2351,19 @@ bool > > drm_mode_parse_command_line_for_connector(const char *mode_option, > > parse_extras = true; > > } > > > > - /* First check for a named mode */ > > - for (i = 0; i < ARRAY_SIZE(drm_named_modes_whitelist); i++) { > > - ret = str_has_prefix(name, drm_named_modes_whitelist[i]); > > - if (ret == mode_end) { > > - if (refresh_ptr) > > - return false; /* named + refresh is invalid > > */ > > + if (!mode_end) > > + return false; > > I'm chasing down a change in behaviour between 6.1 and 6.6, and this > patch seems to be at least part of the cause. > > Since [1] we've had the emulated framebuffer on Pi being 16bpp to save > memory. All good. > > It used to be possible to use "video=HDMI-A-1:-32" on the kernel > command line to set it back to 32bpp. > > After this patch that is no longer possible. "mode_end = bpp_off", and > "bpp_off = bpp_ptr - name", so with bpp_ptr = name we get mode_end > being 0. That fails this conditional. > drm_mode_parse_cmdline_named_mode already aborts early but with no > error if name_end / mode_end is 0, so this "if" clause seems > redundant, and is a change in behaviour. > > We do then get a second parsing failure due to the check if (bpp_ptr > || refresh_ptr) at [2]. > Prior to this patch my
Re: [PATCH v3 4/4] arm64: dts: rockchip: Add devicetree for Pine64 PineTab2
Hi Jonas and Ondřej, > >> + { > >> + pinctrl-names = "default"; > >> + pinctrl-0 = <_dual_io_pins>; > >> + status = "okay"; > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + flash@0 { > >> + compatible = "jedec,spi-nor"; > >> + reg = <0>; > >> + spi-max-frequency = <2400>; > > > > That's a bit on the low side. The flash chip should work for all commands > > up to > > 80MHz https://megous.com/dl/tmp/b428ad9b85ac4633.png and SGM3157YC6 switch > > for the FSPI-CLK should have high enough bandwidth, too. > > I agree that this is a little bit on the low side, it was a safe rate > that I used for U-Boot. U-Boot required an exact rate of the supported > sfc clk rates: 24, 50, 75, 100, 125 or 150 MHz. > > Please also note that the SPI NOR flash chip used in PineTab2 is not a > GigaDevice GD25LQ128E, it should be a SiliconKaiser SK25LP128, same as > found in the Pine64 PinePhone Pro. The schematics for v2.0 reference a GD25LQ128EWIGR. I never checked the jedec id. How did you retrieve this information, or is it maybe a difference in v0.1 and 2.0? > >> + spi-rx-bus-width = <2>; > > > > GD25LQ128E supports quad I/O. Maybe try 4 if it will work. > > The schematic only shows fspi D0 and D1 connected, and use the D2 line > for eMMC_RSTn, so spi-rx-bus-width = <2> should be correct. ack Since it is only needed for bootloader updates and environment its maybe better to stay on the safe side? But I can test faster frequency if you want me to do.. Regards Manuel
AD107M (197), black screen
Hi all, i am testing current mainline (6.7.rc8), on a lenovo legion, with AD107M (chipset 197000a1). Looks like somewhere at driver probe, screen turns black and stays forever. If any hint, welcome. I can help debugging, rebuilding and testing in case. Regards, Angelo Dureghello
[PATCH 2/2] drm/bridge: sii902x: Fix audio codec unregistration
The driver never unregisters the audio codec platform device, which can lead to a crash on module reloading, nor does it handle the return value from sii902x_audio_codec_init(). Signed-off-by: Tomi Valkeinen Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support") Cc: Jyri Sarha --- drivers/gpu/drm/bridge/sii902x.c | 21 + 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 69da73e414a9..4560ae9cbce1 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -1080,7 +1080,9 @@ static int sii902x_init(struct sii902x *sii902x) return ret; } - sii902x_audio_codec_init(sii902x, dev); + ret = sii902x_audio_codec_init(sii902x, dev); + if (ret) + return ret; i2c_set_clientdata(sii902x->i2c, sii902x); @@ -1088,13 +1090,15 @@ static int sii902x_init(struct sii902x *sii902x) 1, 0, I2C_MUX_GATE, sii902x_i2c_bypass_select, sii902x_i2c_bypass_deselect); - if (!sii902x->i2cmux) - return -ENOMEM; + if (!sii902x->i2cmux) { + ret = -ENOMEM; + goto err_unreg_audio; + } sii902x->i2cmux->priv = sii902x; ret = i2c_mux_add_adapter(sii902x->i2cmux, 0, 0, 0); if (ret) - return ret; + goto err_unreg_audio; sii902x->bridge.funcs = _bridge_funcs; sii902x->bridge.of_node = dev->of_node; @@ -1107,6 +,12 @@ static int sii902x_init(struct sii902x *sii902x) drm_bridge_add(>bridge); return 0; + +err_unreg_audio: + if (!PTR_ERR_OR_ZERO(sii902x->audio.pdev)) + platform_device_unregister(sii902x->audio.pdev); + + return ret; } static int sii902x_probe(struct i2c_client *client) @@ -1179,6 +1189,9 @@ static void sii902x_remove(struct i2c_client *client) drm_bridge_remove(>bridge); i2c_mux_del_adapters(sii902x->i2cmux); + + if (!PTR_ERR_OR_ZERO(sii902x->audio.pdev)) + platform_device_unregister(sii902x->audio.pdev); } static const struct of_device_id sii902x_dt_ids[] = { -- 2.34.1
[PATCH 1/2] drm/bridge: sii902x: Fix probing race issue
A null pointer dereference crash has been observed rarely on TI platforms using sii9022 bridge: [ 53.271356] sii902x_get_edid+0x34/0x70 [sii902x] [ 53.276066] sii902x_bridge_get_edid+0x14/0x20 [sii902x] [ 53.281381] drm_bridge_get_edid+0x20/0x34 [drm] [ 53.286305] drm_bridge_connector_get_modes+0x8c/0xcc [drm_kms_helper] [ 53.292955] drm_helper_probe_single_connector_modes+0x190/0x538 [drm_kms_helper] [ 53.300510] drm_client_modeset_probe+0x1f0/0xbd4 [drm] [ 53.305958] __drm_fb_helper_initial_config_and_unlock+0x50/0x510 [drm_kms_helper] [ 53.313611] drm_fb_helper_initial_config+0x48/0x58 [drm_kms_helper] [ 53.320039] drm_fbdev_dma_client_hotplug+0x84/0xd4 [drm_dma_helper] [ 53.326401] drm_client_register+0x5c/0xa0 [drm] [ 53.331216] drm_fbdev_dma_setup+0xc8/0x13c [drm_dma_helper] [ 53.336881] tidss_probe+0x128/0x264 [tidss] [ 53.341174] platform_probe+0x68/0xc4 [ 53.344841] really_probe+0x188/0x3c4 [ 53.348501] __driver_probe_device+0x7c/0x16c [ 53.352854] driver_probe_device+0x3c/0x10c [ 53.357033] __device_attach_driver+0xbc/0x158 [ 53.361472] bus_for_each_drv+0x88/0xe8 [ 53.365303] __device_attach+0xa0/0x1b4 [ 53.369135] device_initial_probe+0x14/0x20 [ 53.373314] bus_probe_device+0xb0/0xb4 [ 53.377145] deferred_probe_work_func+0xcc/0x124 [ 53.381757] process_one_work+0x1f0/0x518 [ 53.385770] worker_thread+0x1e8/0x3dc [ 53.389519] kthread+0x11c/0x120 [ 53.392750] ret_from_fork+0x10/0x20 The issue here is as follows: - tidss probes, but is deferred as sii902x is still missing. - sii902x starts probing and enters sii902x_init(). - sii902x calls drm_bridge_add(). Now the sii902x bridge is ready from DRM's perspective. - sii902x calls sii902x_audio_codec_init() and platform_device_register_data() - The registration of the audio platform device causes probing of the deferred devices. - tidss probes, which eventually causes sii902x_bridge_get_edid() to be called. - sii902x_bridge_get_edid() tries to use the i2c to read the edid. However, the sii902x driver has not set up the i2c part yet, leading to the crash. Fix this by moving the drm_bridge_add() to the end of the sii902x_init(), which is also at the very end of sii902x_probe(). Signed-off-by: Tomi Valkeinen Fixes: 21d808405fe4 ("drm/bridge/sii902x: Fix EDID readback") --- drivers/gpu/drm/bridge/sii902x.c | 29 - 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 2bdc5b439beb..69da73e414a9 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -1080,16 +1080,6 @@ static int sii902x_init(struct sii902x *sii902x) return ret; } - sii902x->bridge.funcs = _bridge_funcs; - sii902x->bridge.of_node = dev->of_node; - sii902x->bridge.timings = _sii902x_timings; - sii902x->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID; - - if (sii902x->i2c->irq > 0) - sii902x->bridge.ops |= DRM_BRIDGE_OP_HPD; - - drm_bridge_add(>bridge); - sii902x_audio_codec_init(sii902x, dev); i2c_set_clientdata(sii902x->i2c, sii902x); @@ -1102,7 +1092,21 @@ static int sii902x_init(struct sii902x *sii902x) return -ENOMEM; sii902x->i2cmux->priv = sii902x; - return i2c_mux_add_adapter(sii902x->i2cmux, 0, 0, 0); + ret = i2c_mux_add_adapter(sii902x->i2cmux, 0, 0, 0); + if (ret) + return ret; + + sii902x->bridge.funcs = _bridge_funcs; + sii902x->bridge.of_node = dev->of_node; + sii902x->bridge.timings = _sii902x_timings; + sii902x->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID; + + if (sii902x->i2c->irq > 0) + sii902x->bridge.ops |= DRM_BRIDGE_OP_HPD; + + drm_bridge_add(>bridge); + + return 0; } static int sii902x_probe(struct i2c_client *client) @@ -1170,12 +1174,11 @@ static int sii902x_probe(struct i2c_client *client) } static void sii902x_remove(struct i2c_client *client) - { struct sii902x *sii902x = i2c_get_clientdata(client); - i2c_mux_del_adapters(sii902x->i2cmux); drm_bridge_remove(>bridge); + i2c_mux_del_adapters(sii902x->i2cmux); } static const struct of_device_id sii902x_dt_ids[] = { -- 2.34.1
[PATCH 0/2] drm/bridge: sii902x: Crash fixes
Two small fixes to sii902x for crashes. Signed-off-by: Tomi Valkeinen --- Tomi Valkeinen (2): drm/bridge: sii902x: Fix probing race issue drm/bridge: sii902x: Fix audio codec unregistration drivers/gpu/drm/bridge/sii902x.c | 42 +++- 1 file changed, 29 insertions(+), 13 deletions(-) --- base-commit: 0c75d52190b8bfa22cdb66e07148aea599c4535d change-id: 20240103-si902x-fixes-468d7153b8ee Best regards, -- Tomi Valkeinen
Re: [PATCH v3 4/4] arm64: dts: rockchip: Add devicetree for Pine64 PineTab2
On Tue, Jan 02, 2024 at 07:07:56PM +0100, Ondřej Jirman wrote: > Hello Manuel, Hello Ondřej, > On Tue, Jan 02, 2024 at 05:15:47PM +0100, Manuel Traut wrote: > > From: Alexander Warnecke > > > > [...] > > > > + > > + backlight: backlight { > > + compatible = "pwm-backlight"; > > + pwms = < 0 25000 0>; > > + brightness-levels = <20 220>; > > + num-interpolated-steps = <200>; > > Does this linear brightness -> PWM duty cycle mapping lead to linear > relationship between brighntess level and subjective brightness on this HW? > > I doubt it a bit... I tested it with the brightness slider in phosh, for me it looks good. > > + > > + hdmi-con { > > hdmi-connector ack, changed for v4 > > + compatible = "hdmi-connector"; > > + type = "d"; > > + > > + port { > > + hdmi_con_in: endpoint { > > + remote-endpoint = <_out_con>; > > + }; > > + }; > > + }; > > + > > + leds { > > + compatible = "gpio-leds"; > > + > > Spurious newline ^ ack, changed for v4 > > + vcc_3v3: vcc-3v3 { > > Regulator node names shoule end with -regulator suffix. The same applies for > all of the below nodes. ack, changed for v4 > > + compatible = "regulator-fixed"; > > + regulator-name = "vcc_3v3"; > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <330>; > > + regulator-max-microvolt = <330>; > > + vin-supply = <_sys>; > > + }; > > + > > + vdd1v2_dvp: vdd1v2-dvp { > > + compatible = "regulator-fixed"; > > + regulator-name = "vdd1v2_dvp"; > > + regulator-min-microvolt = <120>; > > + regulator-max-microvolt = <120>; > > + vin-supply = <_3v3>; > > + /*enable-supply = <_dvp>;*/ > > There's no such property. Delete this commented out line. ack, changed for v4 > > + lcd: panel@0 { > > + compatible = "boe,th101mb31ig002-28a"; > > + reg = <0>; > > + backlight = <>; > > + enable-gpios = < RK_PC7 GPIO_ACTIVE_HIGH>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <_pwren_h _rst_l>; > > Re lcd0_rst_l: > > It's a bit weird conceptually to reference from dtsi something that's only > declared in dts that includes the dtsi. Maybe move pinctrl-* properties > to dts , too... Will be better I guess, changed for v4. > > + vcc5v_midu: BOOST { > > + regulator-always-on; > > + regulator-boot-on; > > + regulator-min-microvolt = <500>; > > + regulator-max-microvolt = <500>; > > + regulator-name = "boost"; > > + regulator-state-mem { > > + regulator-off-in-suspend; > > I guess this prevents remote USB wakeup by USB devices. Like wakeup via USB > keyboard. Probably not a bad thing, though. That is true. After 'echo mem > /sys/power/state' It is not possible to wakeup the device with a USB Keyboard or mouse. However if the surface like keyboard is used that is shipped with the device, wakeup works if the keyboard/tablet gets unfold. For me this behaviour is fine. Other opinions? > > + { > > + pinctrl-names = "default"; > > + pinctrl-0 = <_reset_h>; > > + reset-gpios = < RK_PB2 GPIO_ACTIVE_HIGH>; > > + vpcie3v3-supply = <_minipcie>; > > + status = "okay"; > > +}; > > Does it make sense to enable this HW block by default, when it isn't used on > actual HW? There is a flat ribbon connector, if someone wants to build sth. it might be helpful. However I am also fine with removing it for now. > > + { > > + bt { > > + bt_wake_host_h: bt-wake-host-h { > > + rockchip,pins = <0 RK_PB5 RK_FUNC_GPIO _pull_down>; > > + }; > > + }; > > ^^^ unused I do not bother to removing unused pinctrls, however even if there is no user at the moment, if we look at a dtb as a machine parseable device description it is probably ok, that it is there? > > + > > + camerab { > > + camerab_pdn_l: camerab-pdn-l { > > + rockchip,pins = <4 RK_PB3 RK_FUNC_GPIO _pull_none>; > > + }; > > + > > + camerab_rst_l: camerab-rst-l { > > + rockchip,pins = <4 RK_PB1 RK_FUNC_GPIO _pull_none>; > > + }; > > + }; > > + > > + cameraf { > > + cameraf_pdn_l: cameraf-pdn-l { > > + rockchip,pins = <4 RK_PB2 RK_FUNC_GPIO _pull_none>; > > + }; > > + > > + cameraf_rst_l: cameraf-rst-l { > > + rockchip,pins = <4 RK_PB0 RK_FUNC_GPIO _pull_none>; > > + }; > > + }; > > ^^^ unused > > > + usb { > > + usbcc_int_l: usbcc-int-l { > > + rockchip,pins = <0
Re: [PATCH -next] drm/xe: Remove unneeded semicolon
Hi, Yang, On Wed, 2024-01-03 at 09:15 +0800, Yang Li wrote: > ./drivers/gpu/drm/xe/xe_rtp.c:168:2-3: Unneeded semicolon > Could you please reformulate using imperative "Remove..." rather than citing a style alert? With that, Reviewed-by: Thomas Hellström > Signed-off-by: Yang Li > --- > drivers/gpu/drm/xe/xe_rtp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/xe/xe_rtp.c > b/drivers/gpu/drm/xe/xe_rtp.c > index fb44cc7521d8..316ed2f6d1f0 100644 > --- a/drivers/gpu/drm/xe/xe_rtp.c > +++ b/drivers/gpu/drm/xe/xe_rtp.c > @@ -165,7 +165,7 @@ static void rtp_get_context(struct > xe_rtp_process_ctx *ctx, > *gt = (*hwe)->gt; > *xe = gt_to_xe(*gt); > break; > - }; > + } > } > > /**
AD107M (197), black screen
Hi all, i am testing current mainline (6.7.rc8), on a lenovo legion, with AD107M (chipset 197000a1). Looks like somewhere at driver probe, screen turns black and stays forever. If any hint, welcome. I can help debugging, rebuilding and testing in case. Regards, Angelo Dureghello
Re: [PATCH v4 6/6] x86/vmware: Add TDX hypercall support
On Thu, Dec 28, 2023 at 11:24:21AM -0800, Alexey Makhalov wrote: > From: Alexey Makhalov > > VMware hypercalls use I/O port, VMCALL or VMMCALL instructions. > Add __tdx_hypercall path to support TDX guests. > > No change in high bandwidth hypercalls, as only low bandwidth > ones are supported for TDX guests. > > Co-developed-by: Tim Merrifield > Signed-off-by: Tim Merrifield > Signed-off-by: Alexey Makhalov > Reviewed-by: Nadav Amit Acked-by: Kirill A. Shutemov -- Kiryl Shutsemau / Kirill A. Shutemov
[PATCH] fbdev/sis: Remove dependency on screen_info
When built-in, the sis driver tries to detect the current display mode from the global screen_info state. That state is only for architecture and firmware code. Drivers should not use it directly as it's not guaranteed to contain valid information. Remove the mode-detection code from sis. Drivers that want to detect a pre-set mode on probe should read the hardware registers directly. Signed-off-by: Thomas Zimmermann --- drivers/video/fbdev/sis/sis_main.c | 37 -- 1 file changed, 37 deletions(-) diff --git a/drivers/video/fbdev/sis/sis_main.c b/drivers/video/fbdev/sis/sis_main.c index 6ad47b6b6004..803ccb6aa479 100644 --- a/drivers/video/fbdev/sis/sis_main.c +++ b/drivers/video/fbdev/sis/sis_main.c @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include @@ -257,36 +256,6 @@ static void sisfb_search_mode(char *name, bool quiet) printk(KERN_ERR "sisfb: Invalid mode '%s'\n", nameptr); } -#ifndef MODULE -static void sisfb_get_vga_mode_from_kernel(void) -{ -#ifdef CONFIG_X86 - char mymode[32]; - int mydepth = screen_info.lfb_depth; - - if(screen_info.orig_video_isVGA != VIDEO_TYPE_VLFB) return; - - if( (screen_info.lfb_width >= 320) && (screen_info.lfb_width <= 2048) && - (screen_info.lfb_height >= 200) && (screen_info.lfb_height <= 1536) && - (mydepth >= 8) && (mydepth <= 32) ) { - - if(mydepth == 24) mydepth = 32; - - sprintf(mymode, "%ux%ux%u", screen_info.lfb_width, - screen_info.lfb_height, - mydepth); - - printk(KERN_DEBUG - "sisfb: Using vga mode %s pre-set by kernel as default\n", - mymode); - - sisfb_search_mode(mymode, true); - } -#endif - return; -} -#endif - static void __init sisfb_search_crt2type(const char *name) { @@ -5901,12 +5870,6 @@ static int sisfb_probe(struct pci_dev *pdev, const struct pci_device_id *ent) ivideo->subsysvendor = pdev->subsystem_vendor; ivideo->subsysdevice = pdev->subsystem_device; -#ifndef MODULE - if(sisfb_mode_idx == -1) { - sisfb_get_vga_mode_from_kernel(); - } -#endif - ivideo->chip = chipinfo->chip; ivideo->chip_real_id = chipinfo->chip; ivideo->sisvga_engine = chipinfo->vgaengine; base-commit: 25232eb8a9ac7fa0dac7e846a4bf7fba2b6db39a -- 2.43.0
[PATCH] drm/xe: circumvent bogus stringop-overflow warning
From: Arnd Bergmann gcc-13 warns about an array overflow that it sees but that is prevented by the "asid % NUM_PF_QUEUE" calculation: drivers/gpu/drm/xe/xe_gt_pagefault.c: In function 'xe_guc_pagefault_handler': include/linux/fortify-string.h:57:33: error: writing 16 bytes into a region of size 0 [-Werror=stringop-overflow=] include/linux/fortify-string.h:689:26: note: in expansion of macro '__fortify_memcpy_chk' 689 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \ | ^~~~ drivers/gpu/drm/xe/xe_gt_pagefault.c:341:17: note: in expansion of macro 'memcpy' 341 | memcpy(pf_queue->data + pf_queue->tail, msg, len * sizeof(u32)); | ^~ drivers/gpu/drm/xe/xe_gt_types.h:102:25: note: at offset [1144, 265324] into destination object 'tile' of size 8 I found that rewriting the assignment using pointer addition rather than the equivalent array index calculation prevents the warning, so use that instead. I sent a bug report against gcc for the false positive warning. Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs") Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113214 Signed-off-by: Arnd Bergmann --- drivers/gpu/drm/xe/xe_gt_pagefault.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_gt_pagefault.c b/drivers/gpu/drm/xe/xe_gt_pagefault.c index 59a70d2e0a7a..78dc08cc2bfe 100644 --- a/drivers/gpu/drm/xe/xe_gt_pagefault.c +++ b/drivers/gpu/drm/xe/xe_gt_pagefault.c @@ -332,7 +332,7 @@ int xe_guc_pagefault_handler(struct xe_guc *guc, u32 *msg, u32 len) return -EPROTO; asid = FIELD_GET(PFD_ASID, msg[1]); - pf_queue = >usm.pf_queue[asid % NUM_PF_QUEUE]; + pf_queue = gt->usm.pf_queue + (asid % NUM_PF_QUEUE); spin_lock_irqsave(_queue->lock, flags); full = pf_queue_full(pf_queue); -- 2.39.2
Re: [PATCH v2 02/39] drm/bridge: switch to drm_bridge_read_edid()
On Wed, 03 Jan 2024, Thomas Zimmermann wrote: > Hi Jani > > > drm/bridge: switch to drm_bridge_read_edid() > > Did you mean drm_bridge_edid_read(), here and in the other patches? Ah, yeah, I did. > (Personally, I'd prefer read_edid over edid_read. The former is common > style and easier to read.) The name comes from drm_edid_read() family of functions, which are so named because they reside in drm_edid.[ch]. BR, Jani. > > Best regards > Thomas > > Am 03.01.24 um 11:08 schrieb Jani Nikula: >> Prefer using the struct drm_edid based functions. >> >> Signed-off-by: Jani Nikula >> --- >> drivers/gpu/drm/drm_bridge_connector.c | 16 >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_bridge_connector.c >> b/drivers/gpu/drm/drm_bridge_connector.c >> index 3acd67021ec6..982552c9f92c 100644 >> --- a/drivers/gpu/drm/drm_bridge_connector.c >> +++ b/drivers/gpu/drm/drm_bridge_connector.c >> @@ -239,27 +239,27 @@ static int drm_bridge_connector_get_modes_edid(struct >> drm_connector *connector, >> struct drm_bridge *bridge) >> { >> enum drm_connector_status status; >> -struct edid *edid; >> +const struct drm_edid *drm_edid; >> int n; >> >> status = drm_bridge_connector_detect(connector, false); >> if (status != connector_status_connected) >> goto no_edid; >> >> -edid = drm_bridge_get_edid(bridge, connector); >> -if (!drm_edid_is_valid(edid)) { >> -kfree(edid); >> +drm_edid = drm_bridge_edid_read(bridge, connector); >> +if (!drm_edid_valid(drm_edid)) { >> +drm_edid_free(drm_edid); >> goto no_edid; >> } >> >> -drm_connector_update_edid_property(connector, edid); >> -n = drm_add_edid_modes(connector, edid); >> +drm_edid_connector_update(connector, drm_edid); >> +n = drm_edid_connector_add_modes(connector); >> >> -kfree(edid); >> +drm_edid_free(drm_edid); >> return n; >> >> no_edid: >> -drm_connector_update_edid_property(connector, NULL); >> +drm_edid_connector_update(connector, NULL); >> return 0; >> } >> -- Jani Nikula, Intel
[PULL] drm-misc-fixes
Hi Dave, Daniel, Happy new year! ~Maarten drm-misc-fixes-2024-01-03: drm-misc-fixes for v6.7 final: - 2 small qaic fixes. - Fixes for overflow in aux xfer. - Fix uninitialised gamma lut in gmag200. - Small compiler warning fix for backports of a ps8640 fix. The following changes since commit 6c9dbee84cd005bed5f9d07b3a2797ae6414b435: drm/panel: ltk050h3146w: Set burst mode for ltk050h3148w (2023-12-13 18:33:43 +0100) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2024-01-03 for you to fetch changes up to 11f9eb899ecc8c02b769cf8d2532ba12786a7af7: drm/mgag200: Fix gamma lut not initialized for G200ER, G200EV, G200SE (2023-12-20 13:26:57 +0100) drm-misc-fixes for v6.7 final: - 2 small qaic fixes. - Fixes for overflow in aux xfer. - Fix uninitialised gamma lut in gmag200. - Small compiler warning fix for backports of a ps8640 fix. Douglas Anderson (3): drm/bridge: parade-ps8640: Never store more than msg->size bytes in AUX xfer drm/bridge: ti-sn65dsi86: Never store more than msg->size bytes in AUX xfer drm/bridge: ps8640: Fix size mismatch warning w/ len Jeffrey Hugo (1): accel/qaic: Implement quirk for SOC_HW_VERSION Jocelyn Falempe (1): drm/mgag200: Fix gamma lut not initialized for G200ER, G200EV, G200SE Pranjal Ramajor Asha Kanojiya (1): accel/qaic: Fix GEM import path code drivers/accel/qaic/mhi_controller.c | 15 ++- drivers/accel/qaic/qaic_data.c | 6 ++ drivers/gpu/drm/bridge/parade-ps8640.c | 7 --- drivers/gpu/drm/bridge/ti-sn65dsi86.c| 4 +++- drivers/gpu/drm/mgag200/mgag200_drv.h| 5 + drivers/gpu/drm/mgag200/mgag200_g200er.c | 5 + drivers/gpu/drm/mgag200/mgag200_g200ev.c | 5 + drivers/gpu/drm/mgag200/mgag200_g200se.c | 5 + drivers/gpu/drm/mgag200/mgag200_mode.c | 10 +- 9 files changed, 48 insertions(+), 14 deletions(-)
RE: [PATCH 1/7] drm: Add eDP 1.5 early transport definition
> -Original Message- > From: Intel-gfx On Behalf Of Jouni > Högander > Sent: Monday, December 18, 2023 7:50 PM > To: intel-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Subject: [PATCH 1/7] drm: Add eDP 1.5 early transport definition > > Add DP_PSR_ENABLE_SU_REGION_ET to enable panel early transport. > > Cc: dri-devel@lists.freedesktop.org > Reviewed-by: Mika Kahola > Signed-off-by: Jouni Högander > --- > include/drm/display/drm_dp.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h > index 3731828825bd..281afff6ee4e 100644 > --- a/include/drm/display/drm_dp.h > +++ b/include/drm/display/drm_dp.h > @@ -718,6 +718,7 @@ > # define DP_PSR_SU_REGION_SCANLINE_CAPTURE BIT(4) /* eDP 1.4a */ > # define DP_PSR_IRQ_HPD_WITH_CRC_ERRORS BIT(5) /* eDP 1.4a */ > # define DP_PSR_ENABLE_PSR2 BIT(6) /* eDP 1.4a */ > +# define DP_PSR_ENABLE_SU_REGION_ET BIT(7) /* eDP 1.5 */ > > #define DP_ADAPTER_CTRL 0x1a0 > # define DP_ADAPTER_CTRL_FORCE_LOAD_SENSE (1 << 0) > -- > 2.34.1
Re: [PATCH] drm/vmwgfx: Unmap the surface before resetting it on a plane state
Great catch! That was a nasty one. Reviewed-by: Martin Krastev Regards, Martin On Sun, Dec 24, 2023 at 7:29 AM Zack Rusin wrote: > Switch to a new plane state requires unreferencing of all held surfaces. > In the work required for mob cursors the mapped surfaces started being > cached but the variable indicating whether the surface is currently > mapped was not being reset. This leads to crashes as the duplicated > state, incorrectly, indicates the that surface is mapped even when > no surface is present. That's because after unreferencing the surface > it's perfectly possible for the plane to be backed by a bo instead of a > surface. > > Reset the surface mapped flag when unreferencing the plane state surface > to fix null derefs in cleanup. Fixes crashes in KDE KWin 6.0 on Wayland: > > Oops: [#1] PREEMPT SMP PTI > CPU: 4 PID: 2533 Comm: kwin_wayland Not tainted 6.7.0-rc3-vmwgfx #2 > Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop > Reference Platform, BIOS 6.00 11/12/2020 > RIP: 0010:vmw_du_cursor_plane_cleanup_fb+0x124/0x140 [vmwgfx] > Code: 00 00 00 75 3a 48 83 c4 10 5b 5d c3 cc cc cc cc 48 8b b3 a8 00 00 00 > 48 c7 c7 99 90 43 c0 e8 93 c5 db ca 48 8b 83 a8 00 00 00 <48> 8b 78 28 e8 > e3 f> > RSP: 0018:b6b98216fa80 EFLAGS: 00010246 > RAX: RBX: 969d84cdcb00 RCX: 0027 > RDX: RSI: 0001 RDI: 969e75f21600 > RBP: 969d4143dc50 R08: R09: b6b98216f920 > R10: 0003 R11: 969e7feb3b10 R12: > R13: R14: 027b R15: 969d49c9fc00 > FS: 7f1e8f1b4180() GS:969e75f0() > knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 0028 CR3: 000104006004 CR4: 003706f0 > Call Trace: > > ? __die+0x23/0x70 > ? page_fault_oops+0x171/0x4e0 > ? exc_page_fault+0x7f/0x180 > ? asm_exc_page_fault+0x26/0x30 > ? vmw_du_cursor_plane_cleanup_fb+0x124/0x140 [vmwgfx] > drm_atomic_helper_cleanup_planes+0x9b/0xc0 > commit_tail+0xd1/0x130 > drm_atomic_helper_commit+0x11a/0x140 > drm_atomic_commit+0x97/0xd0 > ? __pfx___drm_printfn_info+0x10/0x10 > drm_atomic_helper_update_plane+0xf5/0x160 > drm_mode_cursor_universal+0x10e/0x270 > drm_mode_cursor_common+0x102/0x230 > ? __pfx_drm_mode_cursor2_ioctl+0x10/0x10 > drm_ioctl_kernel+0xb2/0x110 > drm_ioctl+0x26d/0x4b0 > ? __pfx_drm_mode_cursor2_ioctl+0x10/0x10 > ? __pfx_drm_ioctl+0x10/0x10 > vmw_generic_ioctl+0xa4/0x110 [vmwgfx] > __x64_sys_ioctl+0x94/0xd0 > do_syscall_64+0x61/0xe0 > ? __x64_sys_ioctl+0xaf/0xd0 > ? syscall_exit_to_user_mode+0x2b/0x40 > ? do_syscall_64+0x70/0xe0 > ? __x64_sys_ioctl+0xaf/0xd0 > ? syscall_exit_to_user_mode+0x2b/0x40 > ? do_syscall_64+0x70/0xe0 > ? exc_page_fault+0x7f/0x180 > entry_SYSCALL_64_after_hwframe+0x6e/0x76 > RIP: 0033:0x7f1e93f279ed > Code: 04 25 28 00 00 00 48 89 45 c8 31 c0 48 8d 45 10 c7 45 b0 10 00 00 00 > 48 89 45 b8 48 8d 45 d0 48 89 45 c0 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 > ff f> > RSP: 002b:7ffca0faf600 EFLAGS: 0246 ORIG_RAX: 0010 > RAX: ffda RBX: 55db876ed2c0 RCX: 7f1e93f279ed > RDX: 7ffca0faf6c0 RSI: c02464bb RDI: 0015 > RBP: 7ffca0faf650 R08: 55db87184010 R09: 0007 > R10: 55db886471a0 R11: 0246 R12: 7ffca0faf6c0 > R13: c02464bb R14: 0015 R15: 7ffca0faf790 > > Modules linked in: snd_seq_dummy snd_hrtimer nf_conntrack_netbios_ns > nf_conntrack_broadcast nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib > nft_reject_ine> > CR2: 0028 > ---[ end trace ]--- > RIP: 0010:vmw_du_cursor_plane_cleanup_fb+0x124/0x140 [vmwgfx] > Code: 00 00 00 75 3a 48 83 c4 10 5b 5d c3 cc cc cc cc 48 8b b3 a8 00 00 00 > 48 c7 c7 99 90 43 c0 e8 93 c5 db ca 48 8b 83 a8 00 00 00 <48> 8b 78 28 e8 > e3 f> > RSP: 0018:b6b98216fa80 EFLAGS: 00010246 > RAX: RBX: 969d84cdcb00 RCX: 0027 > RDX: RSI: 0001 RDI: 969e75f21600 > RBP: 969d4143dc50 R08: R09: b6b98216f920 > R10: 0003 R11: 969e7feb3b10 R12: > R13: R14: 027b R15: 969d49c9fc00 > FS: 7f1e8f1b4180() GS:969e75f0() > knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 0028 CR3: 000104006004 CR4: 003706f0 > > Signed-off-by: Zack Rusin > Fixes: 485d98d472d5 ("drm/vmwgfx: Add support for CursorMob and > CursorBypass 4") > Reported-by: Stefan Hoffmeister > Closes: https://gitlab.freedesktop.org/drm/misc/-/issues/34 > Cc: Martin Krastev > Cc: Maaz Mombasawala > Cc: Ian Forbes > Cc: Broadcom internal kernel review list < > bcm-kernel-feedback-l...@broadcom.com> > Cc: dri-devel@lists.freedesktop.org > Cc: # v5.19+ > --- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 4 > 1
Re: [PATCH v2 02/39] drm/bridge: switch to drm_bridge_read_edid()
Hi Jani > drm/bridge: switch to drm_bridge_read_edid() Did you mean drm_bridge_edid_read(), here and in the other patches? (Personally, I'd prefer read_edid over edid_read. The former is common style and easier to read.) Best regards Thomas Am 03.01.24 um 11:08 schrieb Jani Nikula: Prefer using the struct drm_edid based functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/drm_bridge_connector.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c index 3acd67021ec6..982552c9f92c 100644 --- a/drivers/gpu/drm/drm_bridge_connector.c +++ b/drivers/gpu/drm/drm_bridge_connector.c @@ -239,27 +239,27 @@ static int drm_bridge_connector_get_modes_edid(struct drm_connector *connector, struct drm_bridge *bridge) { enum drm_connector_status status; - struct edid *edid; + const struct drm_edid *drm_edid; int n; status = drm_bridge_connector_detect(connector, false); if (status != connector_status_connected) goto no_edid; - edid = drm_bridge_get_edid(bridge, connector); - if (!drm_edid_is_valid(edid)) { - kfree(edid); + drm_edid = drm_bridge_edid_read(bridge, connector); + if (!drm_edid_valid(drm_edid)) { + drm_edid_free(drm_edid); goto no_edid; } - drm_connector_update_edid_property(connector, edid); - n = drm_add_edid_modes(connector, edid); + drm_edid_connector_update(connector, drm_edid); + n = drm_edid_connector_add_modes(connector); - kfree(edid); + drm_edid_free(drm_edid); return n; no_edid: - drm_connector_update_edid_property(connector, NULL); + drm_edid_connector_update(connector, NULL); return 0; } -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg) OpenPGP_signature.asc Description: OpenPGP digital signature
[PATCH 4/4] fbdev/hyperv_fb: Do not clear global screen_info
Do not clear the global instance of screen_info. If necessary, clearing fields in screen_info should be done by architecture or firmware code that maintains the firmware framebuffer. Signed-off-by: Thomas Zimmermann --- drivers/video/fbdev/hyperv_fb.c | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c index 76c956b9a321..7d5717805c0b 100644 --- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c @@ -48,7 +48,6 @@ #include #include #include -#include #include #include #include @@ -1059,14 +1058,8 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info) else aperture_remove_all_conflicting_devices(KBUILD_MODNAME); - if (!gen2vm) { + if (!gen2vm) pci_dev_put(pdev); - } else if (IS_ENABLED(CONFIG_SYSFB)) { - /* framebuffer is reallocated, clear screen_info to avoid misuse from kexec */ - screen_info.lfb_size = 0; - screen_info.lfb_base = 0; - screen_info.orig_video_isVGA = 0; - } return 0; -- 2.43.0
[PATCH 3/4] firmware/sysfb: Clear screen_info state after consuming it
After consuming the global screen_info_state in sysfb_init(), the created platform device maintains the firmware framebuffer. Clear screen_info to avoid conflicting access. Subsequent kexec reboots now ignore the firmware framebuffer. Signed-off-by: Thomas Zimmermann --- drivers/firmware/sysfb.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c index 82fcfd29bc4d..19706bd2642a 100644 --- a/drivers/firmware/sysfb.c +++ b/drivers/firmware/sysfb.c @@ -71,7 +71,7 @@ EXPORT_SYMBOL_GPL(sysfb_disable); static __init int sysfb_init(void) { - struct screen_info *si = _info; + const struct screen_info *si = _info; struct simplefb_platform_data mode; const char *name; bool compatible; @@ -119,6 +119,18 @@ static __init int sysfb_init(void) if (ret) goto err; + /* +* The firmware framebuffer is now maintained by the created +* device. Disable screen_info after we've consumed it. Prevents +* invalid access during kexec reboots. +* +* TODO: Vgacon still relies on the global screen_info. Make +* vgacon work with the platform device, so we can clear +* the screen_info unconditionally. +*/ + if (strcmp(name, "platform-framebuffer")) + screen_info.orig_video_isVGA = 0; + goto unlock_mutex; err: platform_device_put(pd); -- 2.43.0
[PATCH 2/4] fbdev/hyperv_fb: Remove firmware framebuffers with aperture helpers
Replace use of screen_info state with the correct interfaces from the aperture helpers. The state is only for architecture and firmware code. It is not guaranteed to contain valid data. Drivers are thus not allowed to use it. For removing conflicting firmware framebuffers, there are aperture helpers. Hence replace screen_info with the correct functions that will remove conflicting framebuffers for the hypervfb driver. For GEN1 PCI devices, the driver reads the framebuffer base and size from the PCI BAR, and uses the range for removing the firmware framebuffer. For GEN2 VMBUS devices no range can be detected, so the driver clears all firmware framebuffers. Signed-off-by: Thomas Zimmermann --- drivers/video/fbdev/hyperv_fb.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c index a80939fe2ee6..76c956b9a321 100644 --- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c @@ -975,7 +975,8 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info) struct pci_dev *pdev = NULL; void __iomem *fb_virt; int gen2vm = efi_enabled(EFI_BOOT); - resource_size_t base, size; + resource_size_t base = 0; + resource_size_t size = 0; phys_addr_t paddr; int ret; @@ -1010,9 +1011,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info) goto getmem_done; } pr_info("Unable to allocate enough contiguous physical memory on Gen 1 VM. Using MMIO instead.\n"); - } else if (IS_ENABLED(CONFIG_SYSFB)) { - base = screen_info.lfb_base; - size = screen_info.lfb_size; } else { goto err1; } @@ -1056,7 +1054,10 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info) info->screen_size = dio_fb_size; getmem_done: - aperture_remove_conflicting_devices(base, size, KBUILD_MODNAME); + if (base && size) + aperture_remove_conflicting_devices(base, size, KBUILD_MODNAME); + else + aperture_remove_all_conflicting_devices(KBUILD_MODNAME); if (!gen2vm) { pci_dev_put(pdev); -- 2.43.0
[PATCH 0/4] hyperv, sysfb: Do not use screen_info in drivers
The global screen_info state is only meant for architecture and firmware code. Replace its use in hyperv graphics drivers with the correct aperture helpers. Patches 1 and 2 update hyperv-drm and hyperv-fb to use the correct aperture helpers instead of screen_info for removing existing firmware framebuffers. Hyperv-fb also modifies screen_info for better use with kexec. While that update makes sense, it's not supposed to be done by the driver. Patch 3 adds similar code to sysfb and patch 4 removes the code from the driver. An intented side effect of this patchset is that all systems now benefit from better kexec support. After rebooting with kexec, the kernel operated on stale settings on screen_info. Patch 3 fixes this and the kexec kernel will use screen_info in any meaningful way. Thomas Zimmermann (4): drm/hyperv: Remove firmware framebuffers with aperture helper fbdev/hyperv_fb: Remove firmware framebuffers with aperture helpers firmware/sysfb: Clear screen_info state after consuming it fbdev/hyperv_fb: Do not clear global screen_info drivers/firmware/sysfb.c| 14 +- drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 8 ++-- drivers/video/fbdev/hyperv_fb.c | 20 +++- 3 files changed, 22 insertions(+), 20 deletions(-) base-commit: 25232eb8a9ac7fa0dac7e846a4bf7fba2b6db39a -- 2.43.0
[PATCH 1/4] drm/hyperv: Remove firmware framebuffers with aperture helper
Replace use of screen_info state with the correct interface from the aperture helpers. The state is only for architecture and firmware code. It is not guaranteed to contain valid data. Drivers are thus not allowed to use it. For removing conflicting firmware framebuffers, there are aperture helpers. Hence replace screen_info with the correct function that will remove conflicting framebuffers for the hyperv-drm driver. Also move the call to the correct place within the driver. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c index d511d17c5bdf..cff85086f2d6 100644 --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c @@ -7,7 +7,6 @@ #include #include #include -#include #include #include @@ -73,11 +72,6 @@ static int hyperv_setup_vram(struct hyperv_drm_device *hv, struct drm_device *dev = >dev; int ret; - if (IS_ENABLED(CONFIG_SYSFB)) - drm_aperture_remove_conflicting_framebuffers(screen_info.lfb_base, - screen_info.lfb_size, -_driver); - hv->fb_size = (unsigned long)hv->mmio_megabytes * 1024 * 1024; ret = vmbus_allocate_mmio(>mem, hdev, 0, -1, hv->fb_size, 0x10, @@ -130,6 +124,8 @@ static int hyperv_vmbus_probe(struct hv_device *hdev, goto err_hv_set_drv_data; } + drm_aperture_remove_framebuffers(_driver); + ret = hyperv_setup_vram(hv, hdev); if (ret) goto err_vmbus_close; -- 2.43.0
Re: [PATCH 0/2] drm/bridge: start moving towards struct drm_edid
On Fri, 22 Dec 2023, Jani Nikula wrote: > Okay, I rebased and pushed [1]. Probably doesn't make sense to send a > patch bomb like that right now... And here are the patches: https://patchwork.freedesktop.org/series/128149/ BR, Jani. > > BR, > Jani. > > > [1] https://gitlab.freedesktop.org/jani/linux/-/commits/drm-edid-bridge -- Jani Nikula, Intel
[PATCH v2 39/39] drm/bridge: remove ->get_edid callback
There are no more users of the ->get_edid callback left. They've all been converted to ->edid_read. Remove the callback, and the fallback in drm_bridge_edid_read(). Signed-off-by: Jani Nikula --- drivers/gpu/drm/drm_bridge.c | 19 --- include/drm/drm_bridge.h | 30 -- 2 files changed, 49 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index a3065d4aa3d6..521a71c61b16 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -1216,9 +1216,6 @@ EXPORT_SYMBOL_GPL(drm_bridge_get_modes); * DRM_BRIDGE_OP_EDID bridge ops flag, call _bridge_funcs.edid_read to get * the EDID and return it. Otherwise return NULL. * - * If _bridge_funcs.edid_read is not set, fall back to using - * _bridge_funcs.get_edid and wrapping it in struct drm_edid. - * * RETURNS: * The retrieved EDID on success, or NULL otherwise. */ @@ -1228,22 +1225,6 @@ const struct drm_edid *drm_bridge_edid_read(struct drm_bridge *bridge, if (!(bridge->ops & DRM_BRIDGE_OP_EDID)) return NULL; - /* Transitional: Fall back to ->get_edid. */ - if (!bridge->funcs->edid_read) { - const struct drm_edid *drm_edid; - struct edid *edid; - - edid = bridge->funcs->get_edid(bridge, connector); - if (!edid) - return NULL; - - drm_edid = drm_edid_alloc(edid, (edid->extensions + 1) * EDID_LENGTH); - - kfree(edid); - - return drm_edid; - } - return bridge->funcs->edid_read(bridge, connector); } EXPORT_SYMBOL_GPL(drm_bridge_edid_read); diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index ee12f829aaf7..7293c02e17c5 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -588,36 +588,6 @@ struct drm_bridge_funcs { const struct drm_edid *(*edid_read)(struct drm_bridge *bridge, struct drm_connector *connector); - /** -* @get_edid: -* -* Read and parse the EDID data of the connected display. -* -* The @get_edid callback is the preferred way of reporting mode -* information for a display connected to the bridge output. Bridges -* that support reading EDID shall implement this callback and leave -* the @get_modes callback unimplemented. -* -* The caller of this operation shall first verify the output -* connection status and refrain from reading EDID from a disconnected -* output. -* -* This callback is optional. Bridges that implement it shall set the -* DRM_BRIDGE_OP_EDID flag in their _bridge->ops. -* -* The connector parameter shall be used for the sole purpose of EDID -* retrieval and parsing, and shall not be stored internally by bridge -* drivers for future usage. -* -* RETURNS: -* -* An edid structure newly allocated with kmalloc() (or similar) on -* success, or NULL otherwise. The caller is responsible for freeing -* the returned edid structure with kfree(). -*/ - struct edid *(*get_edid)(struct drm_bridge *bridge, -struct drm_connector *connector); - /** * @hpd_notify: * -- 2.39.2
[PATCH v2 38/39] drm/bridge: ti-sn65dsi86: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index 62cc3893dca5..61dc6f063fb4 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c @@ -1207,19 +1207,19 @@ static enum drm_connector_status ti_sn_bridge_detect(struct drm_bridge *bridge) : connector_status_disconnected; } -static struct edid *ti_sn_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *ti_sn_bridge_edid_read(struct drm_bridge *bridge, +struct drm_connector *connector) { struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge); - return drm_get_edid(connector, >aux.ddc); + return drm_edid_read_ddc(connector, >aux.ddc); } static const struct drm_bridge_funcs ti_sn_bridge_funcs = { .attach = ti_sn_bridge_attach, .detach = ti_sn_bridge_detach, .mode_valid = ti_sn_bridge_mode_valid, - .get_edid = ti_sn_bridge_get_edid, + .edid_read = ti_sn_bridge_edid_read, .detect = ti_sn_bridge_detect, .atomic_pre_enable = ti_sn_bridge_atomic_pre_enable, .atomic_enable = ti_sn_bridge_atomic_enable, -- 2.39.2
[PATCH v2 37/39] drm/bridge: tc358767: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/tc358767.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index da2aec5110c2..975cec698452 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -1646,19 +1646,19 @@ static void tc_bridge_mode_set(struct drm_bridge *bridge, drm_mode_copy(>mode, mode); } -static struct edid *tc_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *tc_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct tc_data *tc = bridge_to_tc(bridge); - return drm_get_edid(connector, >aux.ddc); + return drm_edid_read_ddc(connector, >aux.ddc); } static int tc_connector_get_modes(struct drm_connector *connector) { struct tc_data *tc = connector_to_tc(connector); int num_modes; - struct edid *edid; + const struct drm_edid *drm_edid; int ret; ret = tc_get_display_props(tc); @@ -1673,10 +1673,10 @@ static int tc_connector_get_modes(struct drm_connector *connector) return num_modes; } - edid = tc_get_edid(>bridge, connector); - drm_connector_update_edid_property(connector, edid); - num_modes = drm_add_edid_modes(connector, edid); - kfree(edid); + drm_edid = tc_edid_read(>bridge, connector); + drm_edid_connector_update(connector, drm_edid); + num_modes = drm_edid_connector_add_modes(connector); + drm_edid_free(drm_edid); return num_modes; } @@ -1845,7 +1845,7 @@ static const struct drm_bridge_funcs tc_edp_bridge_funcs = { .atomic_enable = tc_edp_bridge_atomic_enable, .atomic_disable = tc_edp_bridge_atomic_disable, .detect = tc_bridge_detect, - .get_edid = tc_get_edid, + .edid_read = tc_edid_read, .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, .atomic_reset = drm_atomic_helper_bridge_reset, -- 2.39.2
[PATCH v2 36/39] drm/bridge: tc358767: update the EDID property
The EDID property should be updated between reading the EDID and adding the modes. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/tc358767.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index eb0d82a91cb9..da2aec5110c2 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -1674,6 +1674,7 @@ static int tc_connector_get_modes(struct drm_connector *connector) } edid = tc_get_edid(>bridge, connector); + drm_connector_update_edid_property(connector, edid); num_modes = drm_add_edid_modes(connector, edid); kfree(edid); -- 2.39.2
[PATCH v2 35/39] drm: bridge: dw_hdmi: clear the EDID property and CEC address on failures
If EDID read fails, clear the EDID property and CEC address. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 8ce85e973b38..654f35ea516d 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2505,8 +2505,6 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector) int ret; drm_edid = dw_hdmi_edid_read(hdmi, connector); - if (!drm_edid) - return 0; drm_edid_connector_update(connector, drm_edid); cec_notifier_set_phys_addr(hdmi->cec_notifier, -- 2.39.2
[PATCH v2 34/39] drm: bridge: dw_hdmi: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 43 ++- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index 52d91a0df85e..8ce85e973b38 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2454,16 +2454,17 @@ static enum drm_connector_status dw_hdmi_detect(struct dw_hdmi *hdmi) return result; } -static struct edid *dw_hdmi_get_edid(struct dw_hdmi *hdmi, -struct drm_connector *connector) +static const struct drm_edid *dw_hdmi_edid_read(struct dw_hdmi *hdmi, + struct drm_connector *connector) { - struct edid *edid; + const struct drm_edid *drm_edid; + const struct edid *edid; if (!hdmi->ddc) return NULL; - edid = drm_get_edid(connector, hdmi->ddc); - if (!edid) { + drm_edid = drm_edid_read_ddc(connector, hdmi->ddc); + if (!drm_edid) { dev_dbg(hdmi->dev, "failed to get edid\n"); return NULL; } @@ -2471,10 +2472,17 @@ static struct edid *dw_hdmi_get_edid(struct dw_hdmi *hdmi, dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n", edid->width_cm, edid->height_cm); + /* +* FIXME: This should use connector->display_info.is_hdmi and +* connector->display_info.has_audio from a path that has read the EDID +* and called drm_edid_connector_update(). +*/ + edid = drm_edid_raw(drm_edid); + hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid); hdmi->sink_has_audio = drm_detect_monitor_audio(edid); - return edid; + return drm_edid; } /* - @@ -2493,17 +2501,18 @@ static int dw_hdmi_connector_get_modes(struct drm_connector *connector) { struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi, connector); - struct edid *edid; + const struct drm_edid *drm_edid; int ret; - edid = dw_hdmi_get_edid(hdmi, connector); - if (!edid) + drm_edid = dw_hdmi_edid_read(hdmi, connector); + if (!drm_edid) return 0; - drm_connector_update_edid_property(connector, edid); - cec_notifier_set_phys_addr_from_edid(hdmi->cec_notifier, edid); - ret = drm_add_edid_modes(connector, edid); - kfree(edid); + drm_edid_connector_update(connector, drm_edid); + cec_notifier_set_phys_addr(hdmi->cec_notifier, + connector->display_info.source_physical_address); + ret = drm_edid_connector_add_modes(connector); + drm_edid_free(drm_edid); return ret; } @@ -2980,12 +2989,12 @@ static enum drm_connector_status dw_hdmi_bridge_detect(struct drm_bridge *bridge return dw_hdmi_detect(hdmi); } -static struct edid *dw_hdmi_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *dw_hdmi_bridge_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct dw_hdmi *hdmi = bridge->driver_private; - return dw_hdmi_get_edid(hdmi, connector); + return dw_hdmi_edid_read(hdmi, connector); } static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = { @@ -3002,7 +3011,7 @@ static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = { .mode_set = dw_hdmi_bridge_mode_set, .mode_valid = dw_hdmi_bridge_mode_valid, .detect = dw_hdmi_bridge_detect, - .get_edid = dw_hdmi_bridge_get_edid, + .edid_read = dw_hdmi_bridge_edid_read, }; /* - -- 2.39.2
[PATCH v2 33/39] drm: adv7511: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 47 +--- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 8be235144f6d..1e40d451ce8c 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -604,10 +604,10 @@ static int adv7511_get_edid_block(void *data, u8 *buf, unsigned int block, * ADV75xx helpers */ -static struct edid *adv7511_get_edid(struct adv7511 *adv7511, -struct drm_connector *connector) +static const struct drm_edid *adv7511_edid_read(struct adv7511 *adv7511, + struct drm_connector *connector) { - struct edid *edid; + const struct drm_edid *drm_edid; /* Reading the EDID only works if the device is powered */ if (!adv7511->powered) { @@ -621,31 +621,44 @@ static struct edid *adv7511_get_edid(struct adv7511 *adv7511, edid_i2c_addr); } - edid = drm_do_get_edid(connector, adv7511_get_edid_block, adv7511); + drm_edid = drm_edid_read_custom(connector, adv7511_get_edid_block, adv7511); if (!adv7511->powered) __adv7511_power_off(adv7511); - adv7511_set_config_csc(adv7511, connector, adv7511->rgb, - drm_detect_hdmi_monitor(edid)); + if (drm_edid) { + /* +* FIXME: The CEC physical address should be set using +* cec_s_phys_addr(adap, +* connector->display_info.source_physical_address, false) from +* a path that has read the EDID and called +* drm_edid_connector_update(). +*/ + const struct edid *edid = drm_edid_raw(drm_edid); + + adv7511_set_config_csc(adv7511, connector, adv7511->rgb, + drm_detect_hdmi_monitor(edid)); - cec_s_phys_addr_from_edid(adv7511->cec_adap, edid); + cec_s_phys_addr_from_edid(adv7511->cec_adap, edid); + } else { + cec_s_phys_addr_from_edid(adv7511->cec_adap, NULL); + } - return edid; + return drm_edid; } static int adv7511_get_modes(struct adv7511 *adv7511, struct drm_connector *connector) { - struct edid *edid; + const struct drm_edid *drm_edid; unsigned int count; - edid = adv7511_get_edid(adv7511, connector); + drm_edid = adv7511_edid_read(adv7511, connector); - drm_connector_update_edid_property(connector, edid); - count = drm_add_edid_modes(connector, edid); + drm_edid_connector_update(connector, drm_edid); + count = drm_edid_connector_add_modes(connector); - kfree(edid); + drm_edid_free(drm_edid); return count; } @@ -953,12 +966,12 @@ static enum drm_connector_status adv7511_bridge_detect(struct drm_bridge *bridge return adv7511_detect(adv, NULL); } -static struct edid *adv7511_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *adv7511_bridge_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct adv7511 *adv = bridge_to_adv7511(bridge); - return adv7511_get_edid(adv, connector); + return adv7511_edid_read(adv, connector); } static void adv7511_bridge_hpd_notify(struct drm_bridge *bridge, @@ -977,7 +990,7 @@ static const struct drm_bridge_funcs adv7511_bridge_funcs = { .mode_valid = adv7511_bridge_mode_valid, .attach = adv7511_bridge_attach, .detect = adv7511_bridge_detect, - .get_edid = adv7511_bridge_get_edid, + .edid_read = adv7511_bridge_edid_read, .hpd_notify = adv7511_bridge_hpd_notify, }; -- 2.39.2
[PATCH v2 32/39] drm: xlnx: zynqmp_dpsub: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/xlnx/zynqmp_dp.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c index a0606fab0e22..24213eaa38d0 100644 --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c @@ -1560,12 +1560,12 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid return connector_status_disconnected; } -static struct edid *zynqmp_dp_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *zynqmp_dp_bridge_edid_read(struct drm_bridge *bridge, +struct drm_connector *connector) { struct zynqmp_dp *dp = bridge_to_dp(bridge); - return drm_get_edid(connector, >aux.ddc); + return drm_edid_read_ddc(connector, >aux.ddc); } static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = { @@ -1579,7 +1579,7 @@ static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = { .atomic_reset = drm_atomic_helper_bridge_reset, .atomic_check = zynqmp_dp_bridge_atomic_check, .detect = zynqmp_dp_bridge_detect, - .get_edid = zynqmp_dp_bridge_get_edid, + .edid_read = zynqmp_dp_bridge_edid_read, }; /* - -- 2.39.2
[PATCH v2 31/39] drm/omap/hdmi5: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/omapdrm/dss/hdmi5.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c index e6611c683857..c7ae2235ae99 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c @@ -425,11 +425,11 @@ static void hdmi5_bridge_disable(struct drm_bridge *bridge, mutex_unlock(>lock); } -static struct edid *hdmi5_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *hdmi5_bridge_edid_read(struct drm_bridge *bridge, +struct drm_connector *connector) { struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge); - struct edid *edid; + const struct drm_edid *drm_edid; bool need_enable; int idlemode; int r; @@ -452,7 +452,7 @@ static struct edid *hdmi5_bridge_get_edid(struct drm_bridge *bridge, hdmi5_core_ddc_init(>core); - edid = drm_do_get_edid(connector, hdmi5_core_ddc_read, >core); + drm_edid = drm_edid_read_custom(connector, hdmi5_core_ddc_read, >core); hdmi5_core_ddc_uninit(>core); @@ -464,7 +464,7 @@ static struct edid *hdmi5_bridge_get_edid(struct drm_bridge *bridge, if (need_enable) hdmi_core_disable(hdmi); - return (struct edid *)edid; + return drm_edid; } static const struct drm_bridge_funcs hdmi5_bridge_funcs = { @@ -475,7 +475,7 @@ static const struct drm_bridge_funcs hdmi5_bridge_funcs = { .atomic_reset = drm_atomic_helper_bridge_reset, .atomic_enable = hdmi5_bridge_enable, .atomic_disable = hdmi5_bridge_disable, - .get_edid = hdmi5_bridge_get_edid, + .edid_read = hdmi5_bridge_edid_read, }; static void hdmi5_bridge_init(struct omap_hdmi *hdmi) -- 2.39.2
[PATCH v2 30/39] drm/omap/hdmi4: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/omapdrm/dss/hdmi4.c | 22 +++--- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c index a26b77d99d52..9b8747d83ee8 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c @@ -436,11 +436,11 @@ static void hdmi4_bridge_hpd_notify(struct drm_bridge *bridge, hdmi4_cec_set_phys_addr(>core, CEC_PHYS_ADDR_INVALID); } -static struct edid *hdmi4_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *hdmi4_bridge_edid_read(struct drm_bridge *bridge, +struct drm_connector *connector) { struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge); - struct edid *edid = NULL; + const struct drm_edid *drm_edid = NULL; unsigned int cec_addr; bool need_enable; int r; @@ -461,13 +461,21 @@ static struct edid *hdmi4_bridge_get_edid(struct drm_bridge *bridge, if (r) goto done; - edid = drm_do_get_edid(connector, hdmi4_core_ddc_read, >core); + drm_edid = drm_edid_read_custom(connector, hdmi4_core_ddc_read, >core); done: hdmi_runtime_put(hdmi); mutex_unlock(>lock); - if (edid && edid->extensions) { + if (drm_edid) { + /* +* FIXME: The CEC physical address should be set using +* hdmi4_cec_set_phys_addr(>core, +* connector->display_info.source_physical_address) from a path +* that has read the EDID and called +* drm_edid_connector_update(). +*/ + const struct edid *edid = drm_edid_raw(drm_edid); unsigned int len = (edid->extensions + 1) * EDID_LENGTH; cec_addr = cec_get_edid_phys_addr((u8 *)edid, len, NULL); @@ -480,7 +488,7 @@ static struct edid *hdmi4_bridge_get_edid(struct drm_bridge *bridge, if (need_enable) hdmi4_core_disable(>core); - return edid; + return drm_edid; } static const struct drm_bridge_funcs hdmi4_bridge_funcs = { @@ -492,7 +500,7 @@ static const struct drm_bridge_funcs hdmi4_bridge_funcs = { .atomic_enable = hdmi4_bridge_enable, .atomic_disable = hdmi4_bridge_disable, .hpd_notify = hdmi4_bridge_hpd_notify, - .get_edid = hdmi4_bridge_get_edid, + .edid_read = hdmi4_bridge_edid_read, }; static void hdmi4_bridge_init(struct omap_hdmi *hdmi) -- 2.39.2
[PATCH v2 29/39] drm/msm/hdmi: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c index f28c61570533..4a5b5112227f 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c @@ -236,24 +236,33 @@ static void msm_hdmi_bridge_mode_set(struct drm_bridge *bridge, msm_hdmi_audio_update(hdmi); } -static struct edid *msm_hdmi_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *msm_hdmi_bridge_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge); struct hdmi *hdmi = hdmi_bridge->hdmi; - struct edid *edid; + const struct drm_edid *drm_edid; uint32_t hdmi_ctrl; hdmi_ctrl = hdmi_read(hdmi, REG_HDMI_CTRL); hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl | HDMI_CTRL_ENABLE); - edid = drm_get_edid(connector, hdmi->i2c); + drm_edid = drm_edid_read_ddc(connector, hdmi->i2c); hdmi_write(hdmi, REG_HDMI_CTRL, hdmi_ctrl); - hdmi->hdmi_mode = drm_detect_hdmi_monitor(edid); + if (drm_edid) { + /* +* FIXME: This should use connector->display_info.is_hdmi from a +* path that has read the EDID and called +* drm_edid_connector_update(). +*/ + const struct edid *edid = drm_edid_raw(drm_edid); - return edid; + hdmi->hdmi_mode = drm_detect_hdmi_monitor(edid); + } + + return drm_edid; } static enum drm_mode_status msm_hdmi_bridge_mode_valid(struct drm_bridge *bridge, @@ -294,7 +303,7 @@ static const struct drm_bridge_funcs msm_hdmi_bridge_funcs = { .post_disable = msm_hdmi_bridge_post_disable, .mode_set = msm_hdmi_bridge_mode_set, .mode_valid = msm_hdmi_bridge_mode_valid, - .get_edid = msm_hdmi_bridge_get_edid, + .edid_read = msm_hdmi_bridge_edid_read, .detect = msm_hdmi_bridge_detect, }; -- 2.39.2
Re: [PATCH v2 00/39] drm/bridge: switch to struct drm_edid
On Wed, 03 Jan 2024, Jani Nikula wrote: > Convert all of drm/bridge to the new struct drm_edid > infrastructure. It's safer than struct edid, because it contains meta > information about the allocated size of the EDID, instead of relying on > the size (number of extensions) originating from outside of the kernel. > > Among all of drm, I think bridge has the most uses of struct edid that > do not originate from the drm_get_edid() family of functions, which > means the validity checks are somewhat inconsistent, and having the meta > information is more crucial. > > Only build tested. I'm sure there should be more Cc's especially in the > patches towards the end of the series, but I just wanted to get the > series out the door now. PS. It's also available at https://gitlab.freedesktop.org/jani/linux/-/commits/drm-edid-bridge -- Jani Nikula, Intel
[PATCH v2 28/39] drm/msm/hdmi: fix indent
Remove the excess leading tabs. Signed-off-by: Jani Nikula --- drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c index f5e01471b0b0..f28c61570533 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c @@ -290,12 +290,12 @@ static enum drm_mode_status msm_hdmi_bridge_mode_valid(struct drm_bridge *bridge } static const struct drm_bridge_funcs msm_hdmi_bridge_funcs = { - .pre_enable = msm_hdmi_bridge_pre_enable, - .post_disable = msm_hdmi_bridge_post_disable, - .mode_set = msm_hdmi_bridge_mode_set, - .mode_valid = msm_hdmi_bridge_mode_valid, - .get_edid = msm_hdmi_bridge_get_edid, - .detect = msm_hdmi_bridge_detect, + .pre_enable = msm_hdmi_bridge_pre_enable, + .post_disable = msm_hdmi_bridge_post_disable, + .mode_set = msm_hdmi_bridge_mode_set, + .mode_valid = msm_hdmi_bridge_mode_valid, + .get_edid = msm_hdmi_bridge_get_edid, + .detect = msm_hdmi_bridge_detect, }; static void -- 2.39.2
[PATCH v2 27/39] drm/mediatek/hdmi: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/mediatek/mtk_hdmi.c | 26 +- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c index 86133bf16326..c6bdc565e4a9 100644 --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c @@ -1265,19 +1265,27 @@ static enum drm_connector_status mtk_hdmi_bridge_detect(struct drm_bridge *bridg return mtk_hdmi_detect(hdmi); } -static struct edid *mtk_hdmi_bridge_get_edid(struct drm_bridge *bridge, -struct drm_connector *connector) +static const struct drm_edid *mtk_hdmi_bridge_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct mtk_hdmi *hdmi = hdmi_ctx_from_bridge(bridge); - struct edid *edid; + const struct drm_edid *drm_edid; if (!hdmi->ddc_adpt) return NULL; - edid = drm_get_edid(connector, hdmi->ddc_adpt); - if (!edid) - return NULL; - hdmi->dvi_mode = !drm_detect_monitor_audio(edid); - return edid; + drm_edid = drm_edid_read_ddc(connector, hdmi->ddc_adpt); + if (drm_edid) { + /* +* FIXME: This should use !connector->display_info.has_audio (or +* !connector->display_info.is_hdmi) from a path that has read +* the EDID and called drm_edid_connector_update(). +*/ + const struct edid *edid = drm_edid_raw(drm_edid); + + hdmi->dvi_mode = !drm_detect_monitor_audio(edid); + } + + return drm_edid; } static int mtk_hdmi_bridge_attach(struct drm_bridge *bridge, @@ -1417,7 +1425,7 @@ static const struct drm_bridge_funcs mtk_hdmi_bridge_funcs = { .atomic_pre_enable = mtk_hdmi_bridge_atomic_pre_enable, .atomic_enable = mtk_hdmi_bridge_atomic_enable, .detect = mtk_hdmi_bridge_detect, - .get_edid = mtk_hdmi_bridge_get_edid, + .edid_read = mtk_hdmi_bridge_edid_read, }; static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi, -- 2.39.2
[PATCH v2 26/39] drm/mediatek/dp: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/mediatek/mtk_dp.c | 31 --- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c index 2136a596efa1..0ba72102636a 100644 --- a/drivers/gpu/drm/mediatek/mtk_dp.c +++ b/drivers/gpu/drm/mediatek/mtk_dp.c @@ -2042,12 +2042,12 @@ static enum drm_connector_status mtk_dp_bdg_detect(struct drm_bridge *bridge) return ret; } -static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *mtk_dp_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge); bool enabled = mtk_dp->enabled; - struct edid *new_edid = NULL; + const struct drm_edid *drm_edid; struct mtk_dp_audio_cfg *audio_caps = _dp->info.audio_cur_cfg; if (!enabled) { @@ -2055,7 +2055,7 @@ static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge, mtk_dp_aux_panel_poweron(mtk_dp, true); } - new_edid = drm_get_edid(connector, _dp->aux.ddc); + drm_edid = drm_edid_read_ddc(connector, _dp->aux.ddc); /* * Parse capability here to let atomic_get_input_bus_fmts and @@ -2063,17 +2063,26 @@ static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge, */ if (mtk_dp_parse_capabilities(mtk_dp)) { drm_err(mtk_dp->drm_dev, "Can't parse capabilities\n"); - kfree(new_edid); - new_edid = NULL; + drm_edid_free(drm_edid); + drm_edid = NULL; } - if (new_edid) { + if (drm_edid) { + /* +* FIXME: get rid of drm_edid_raw() +*/ + const struct edid *edid = drm_edid_raw(drm_edid); struct cea_sad *sads; - audio_caps->sad_count = drm_edid_to_sad(new_edid, ); + audio_caps->sad_count = drm_edid_to_sad(edid, ); kfree(sads); - audio_caps->detect_monitor = drm_detect_monitor_audio(new_edid); + /* +* FIXME: This should use connector->display_info.has_audio from +* a path that has read the EDID and called +* drm_edid_connector_update(). +*/ + audio_caps->detect_monitor = drm_detect_monitor_audio(edid); } if (!enabled) { @@ -2081,7 +2090,7 @@ static struct edid *mtk_dp_get_edid(struct drm_bridge *bridge, drm_atomic_bridge_chain_post_disable(bridge, connector->state->state); } - return new_edid; + return drm_edid; } static ssize_t mtk_dp_aux_transfer(struct drm_dp_aux *mtk_aux, @@ -2433,7 +2442,7 @@ static const struct drm_bridge_funcs mtk_dp_bridge_funcs = { .atomic_enable = mtk_dp_bridge_atomic_enable, .atomic_disable = mtk_dp_bridge_atomic_disable, .mode_valid = mtk_dp_bridge_mode_valid, - .get_edid = mtk_dp_get_edid, + .edid_read = mtk_dp_edid_read, .detect = mtk_dp_bdg_detect, }; -- 2.39.2
[PATCH v2 25/39] drm/bridge: sii902x: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/sii902x.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 2f876b805b83..12346bacecaf 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -278,31 +278,31 @@ static const struct drm_connector_funcs sii902x_connector_funcs = { .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, }; -static struct edid *sii902x_get_edid(struct sii902x *sii902x, -struct drm_connector *connector) +static const struct drm_edid *sii902x_edid_read(struct sii902x *sii902x, + struct drm_connector *connector) { - struct edid *edid; + const struct drm_edid *drm_edid; mutex_lock(>mutex); - edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]); + drm_edid = drm_edid_read_ddc(connector, sii902x->i2cmux->adapter[0]); mutex_unlock(>mutex); - return edid; + return drm_edid; } static int sii902x_get_modes(struct drm_connector *connector) { struct sii902x *sii902x = connector_to_sii902x(connector); - struct edid *edid; + const struct drm_edid *drm_edid; int num = 0; - edid = sii902x_get_edid(sii902x, connector); - drm_connector_update_edid_property(connector, edid); - if (edid) { - num = drm_add_edid_modes(connector, edid); - kfree(edid); + drm_edid = sii902x_edid_read(sii902x, connector); + drm_edid_connector_update(connector, drm_edid); + if (drm_edid) { + num = drm_edid_connector_add_modes(connector); + drm_edid_free(drm_edid); } sii902x->sink_is_hdmi = connector->display_info.is_hdmi; @@ -461,12 +461,12 @@ static enum drm_connector_status sii902x_bridge_detect(struct drm_bridge *bridge return sii902x_detect(sii902x); } -static struct edid *sii902x_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *sii902x_bridge_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct sii902x *sii902x = bridge_to_sii902x(bridge); - return sii902x_get_edid(sii902x, connector); + return sii902x_edid_read(sii902x, connector); } static u32 *sii902x_bridge_atomic_get_input_bus_fmts(struct drm_bridge *bridge, @@ -510,7 +510,7 @@ static const struct drm_bridge_funcs sii902x_bridge_funcs = { .disable = sii902x_bridge_disable, .enable = sii902x_bridge_enable, .detect = sii902x_bridge_detect, - .get_edid = sii902x_bridge_get_edid, + .edid_read = sii902x_bridge_edid_read, .atomic_reset = drm_atomic_helper_bridge_reset, .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, -- 2.39.2
[PATCH v2 24/39] drm/bridge: sii902x: use display info is_hdmi
Use the pre-parsed information instead of parsing EDID again. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/sii902x.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c index 2bdc5b439beb..2f876b805b83 100644 --- a/drivers/gpu/drm/bridge/sii902x.c +++ b/drivers/gpu/drm/bridge/sii902x.c @@ -286,12 +286,6 @@ static struct edid *sii902x_get_edid(struct sii902x *sii902x, mutex_lock(>mutex); edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]); - if (edid) { - if (drm_detect_hdmi_monitor(edid)) - sii902x->sink_is_hdmi = true; - else - sii902x->sink_is_hdmi = false; - } mutex_unlock(>mutex); @@ -311,6 +305,8 @@ static int sii902x_get_modes(struct drm_connector *connector) kfree(edid); } + sii902x->sink_is_hdmi = connector->display_info.is_hdmi; + return num; } -- 2.39.2
[PATCH v2 23/39] drm/bridge: nxp-ptn3460: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/nxp-ptn3460.c | 23 +-- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c b/drivers/gpu/drm/bridge/nxp-ptn3460.c index 7c0076e49953..4af3af5662cc 100644 --- a/drivers/gpu/drm/bridge/nxp-ptn3460.c +++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c @@ -154,10 +154,11 @@ static void ptn3460_disable(struct drm_bridge *bridge) } -static struct edid *ptn3460_get_edid(struct drm_bridge *bridge, -struct drm_connector *connector) +static const struct drm_edid *ptn3460_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge); + const struct drm_edid *drm_edid; bool power_off; u8 *edid; int ret; @@ -175,27 +176,29 @@ static struct edid *ptn3460_get_edid(struct drm_bridge *bridge, EDID_LENGTH); if (ret) { kfree(edid); - edid = NULL; + drm_edid = NULL; goto out; } + drm_edid = drm_edid_alloc(edid, EDID_LENGTH); + out: if (power_off) ptn3460_disable(_bridge->bridge); - return (struct edid *)edid; + return drm_edid; } static int ptn3460_connector_get_modes(struct drm_connector *connector) { struct ptn3460_bridge *ptn_bridge = connector_to_ptn3460(connector); - struct edid *edid; + const struct drm_edid *drm_edid; int num_modes; - edid = ptn3460_get_edid(_bridge->bridge, connector); - drm_connector_update_edid_property(connector, edid); - num_modes = drm_add_edid_modes(connector, edid); - kfree(edid); + drm_edid = ptn3460_edid_read(_bridge->bridge, connector); + drm_edid_connector_update(connector, drm_edid); + num_modes = drm_edid_connector_add_modes(connector); + drm_edid_free(drm_edid); return num_modes; } @@ -254,7 +257,7 @@ static const struct drm_bridge_funcs ptn3460_bridge_funcs = { .pre_enable = ptn3460_pre_enable, .disable = ptn3460_disable, .attach = ptn3460_bridge_attach, - .get_edid = ptn3460_get_edid, + .edid_read = ptn3460_edid_read, }; static int ptn3460_probe(struct i2c_client *client) -- 2.39.2
[PATCH v2 22/39] drm/bridge: megachips: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- .../bridge/megachips-stdp-ge-b850v3-fw.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c index e93083bbec9d..4480523244e4 100644 --- a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c +++ b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c @@ -91,26 +91,26 @@ static int stdp2690_read_block(void *context, u8 *buf, unsigned int block, size_ return 0; } -static struct edid *ge_b850v3_lvds_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *ge_b850v3_lvds_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct i2c_client *client; client = ge_b850v3_lvds_ptr->stdp2690_i2c; - return drm_do_get_edid(connector, stdp2690_read_block, client); + return drm_edid_read_custom(connector, stdp2690_read_block, client); } static int ge_b850v3_lvds_get_modes(struct drm_connector *connector) { - struct edid *edid; + const struct drm_edid *drm_edid; int num_modes; - edid = ge_b850v3_lvds_get_edid(_b850v3_lvds_ptr->bridge, connector); + drm_edid = ge_b850v3_lvds_edid_read(_b850v3_lvds_ptr->bridge, connector); - drm_connector_update_edid_property(connector, edid); - num_modes = drm_add_edid_modes(connector, edid); - kfree(edid); + drm_edid_connector_update(connector, drm_edid); + num_modes = drm_edid_connector_add_modes(connector); + drm_edid_free(drm_edid); return num_modes; } @@ -226,7 +226,7 @@ static int ge_b850v3_lvds_attach(struct drm_bridge *bridge, static const struct drm_bridge_funcs ge_b850v3_lvds_funcs = { .attach = ge_b850v3_lvds_attach, .detect = ge_b850v3_lvds_bridge_detect, - .get_edid = ge_b850v3_lvds_get_edid, + .edid_read = ge_b850v3_lvds_edid_read, }; static int ge_b850v3_lvds_init(struct device *dev) -- 2.39.2
[PATCH v2 21/39] drm/bridge: lt9611uxc: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c index 3d916306a47d..28ceae10fa25 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c @@ -494,8 +494,8 @@ static int lt9611uxc_get_edid_block(void *data, u8 *buf, unsigned int block, siz return 0; }; -static struct edid *lt9611uxc_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *lt9611uxc_bridge_edid_read(struct drm_bridge *bridge, +struct drm_connector *connector) { struct lt9611uxc *lt9611uxc = bridge_to_lt9611uxc(bridge); int ret; @@ -509,7 +509,7 @@ static struct edid *lt9611uxc_bridge_get_edid(struct drm_bridge *bridge, return NULL; } - return drm_do_get_edid(connector, lt9611uxc_get_edid_block, lt9611uxc); + return drm_edid_read_custom(connector, lt9611uxc_get_edid_block, lt9611uxc); } static const struct drm_bridge_funcs lt9611uxc_bridge_funcs = { @@ -517,7 +517,7 @@ static const struct drm_bridge_funcs lt9611uxc_bridge_funcs = { .mode_valid = lt9611uxc_bridge_mode_valid, .mode_set = lt9611uxc_bridge_mode_set, .detect = lt9611uxc_bridge_detect, - .get_edid = lt9611uxc_bridge_get_edid, + .edid_read = lt9611uxc_bridge_edid_read, }; static int lt9611uxc_parse_dt(struct device *dev, -- 2.39.2
[PATCH v2 20/39] drm/bridge: lt9611: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/lontium-lt9611.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c index 9663601ce098..1385e3378985 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c @@ -846,13 +846,13 @@ lt9611_bridge_atomic_post_disable(struct drm_bridge *bridge, lt9611_sleep_setup(lt9611); } -static struct edid *lt9611_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *lt9611_bridge_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct lt9611 *lt9611 = bridge_to_lt9611(bridge); lt9611_power_on(lt9611); - return drm_do_get_edid(connector, lt9611_get_edid_block, lt9611); + return drm_edid_read_custom(connector, lt9611_get_edid_block, lt9611); } static void lt9611_bridge_hpd_enable(struct drm_bridge *bridge) @@ -892,7 +892,7 @@ static const struct drm_bridge_funcs lt9611_bridge_funcs = { .attach = lt9611_bridge_attach, .mode_valid = lt9611_bridge_mode_valid, .detect = lt9611_bridge_detect, - .get_edid = lt9611_bridge_get_edid, + .edid_read = lt9611_bridge_edid_read, .hpd_enable = lt9611_bridge_hpd_enable, .atomic_pre_enable = lt9611_bridge_atomic_pre_enable, -- 2.39.2
[PATCH v2 19/39] drm: bridge: it66121: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/ite-it66121.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c index 1cf3fb1f13dc..1c3433b5e366 100644 --- a/drivers/gpu/drm/bridge/ite-it66121.c +++ b/drivers/gpu/drm/bridge/ite-it66121.c @@ -874,33 +874,33 @@ static void it66121_bridge_hpd_disable(struct drm_bridge *bridge) dev_err(ctx->dev, "failed to disable HPD IRQ\n"); } -static struct edid *it66121_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *it66121_bridge_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct it66121_ctx *ctx = container_of(bridge, struct it66121_ctx, bridge); - struct edid *edid; + const struct drm_edid *drm_edid; int ret; mutex_lock(>lock); ret = it66121_preamble_ddc(ctx); if (ret) { - edid = NULL; + drm_edid = NULL; goto out_unlock; } ret = regmap_write(ctx->regmap, IT66121_DDC_HEADER_REG, IT66121_DDC_HEADER_EDID); if (ret) { - edid = NULL; + drm_edid = NULL; goto out_unlock; } - edid = drm_do_get_edid(connector, it66121_get_edid_block, ctx); + drm_edid = drm_edid_read_custom(connector, it66121_get_edid_block, ctx); out_unlock: mutex_unlock(>lock); - return edid; + return drm_edid; } static const struct drm_bridge_funcs it66121_bridge_funcs = { @@ -916,7 +916,7 @@ static const struct drm_bridge_funcs it66121_bridge_funcs = { .mode_set = it66121_bridge_mode_set, .mode_valid = it66121_bridge_mode_valid, .detect = it66121_bridge_detect, - .get_edid = it66121_bridge_get_edid, + .edid_read = it66121_bridge_edid_read, .hpd_enable = it66121_bridge_hpd_enable, .hpd_disable = it66121_bridge_hpd_disable, }; -- 2.39.2
[PATCH v2 18/39] drm/bridge: it6505: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/ite-it6505.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/bridge/ite-it6505.c b/drivers/gpu/drm/bridge/ite-it6505.c index 2f300f5ca051..914b58ec130d 100644 --- a/drivers/gpu/drm/bridge/ite-it6505.c +++ b/drivers/gpu/drm/bridge/ite-it6505.c @@ -458,7 +458,7 @@ struct it6505 { /* it6505 driver hold option */ bool enable_drv_hold; - struct edid *cached_edid; + const struct drm_edid *cached_edid; }; struct it6505_step_train_para { @@ -2261,7 +2261,7 @@ static void it6505_plugged_status_to_codec(struct it6505 *it6505) static void it6505_remove_edid(struct it6505 *it6505) { - kfree(it6505->cached_edid); + drm_edid_free(it6505->cached_edid); it6505->cached_edid = NULL; } @@ -3032,15 +3032,16 @@ it6505_bridge_detect(struct drm_bridge *bridge) return it6505_detect(it6505); } -static struct edid *it6505_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *it6505_bridge_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct it6505 *it6505 = bridge_to_it6505(bridge); struct device *dev = it6505->dev; if (!it6505->cached_edid) { - it6505->cached_edid = drm_do_get_edid(connector, it6505_get_edid_block, - it6505); + it6505->cached_edid = drm_edid_read_custom(connector, + it6505_get_edid_block, + it6505); if (!it6505->cached_edid) { DRM_DEV_DEBUG_DRIVER(dev, "failed to get edid!"); @@ -3048,7 +3049,7 @@ static struct edid *it6505_bridge_get_edid(struct drm_bridge *bridge, } } - return drm_edid_duplicate(it6505->cached_edid); + return drm_edid_dup(it6505->cached_edid); } static const struct drm_bridge_funcs it6505_bridge_funcs = { @@ -3063,7 +3064,7 @@ static const struct drm_bridge_funcs it6505_bridge_funcs = { .atomic_pre_enable = it6505_bridge_atomic_pre_enable, .atomic_post_disable = it6505_bridge_atomic_post_disable, .detect = it6505_bridge_detect, - .get_edid = it6505_bridge_get_edid, + .edid_read = it6505_bridge_edid_read, }; static __maybe_unused int it6505_bridge_resume(struct device *dev) -- 2.39.2
[PATCH v2 17/39] drm/bridge: display-connector: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/display-connector.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c index 08bd5695ddae..ab8e00baf3f1 100644 --- a/drivers/gpu/drm/bridge/display-connector.c +++ b/drivers/gpu/drm/bridge/display-connector.c @@ -81,12 +81,12 @@ display_connector_detect(struct drm_bridge *bridge) } } -static struct edid *display_connector_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *display_connector_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct display_connector *conn = to_display_connector(bridge); - return drm_get_edid(connector, conn->bridge.ddc); + return drm_edid_read_ddc(connector, conn->bridge.ddc); } /* @@ -172,7 +172,7 @@ static u32 *display_connector_get_input_bus_fmts(struct drm_bridge *bridge, static const struct drm_bridge_funcs display_connector_bridge_funcs = { .attach = display_connector_attach, .detect = display_connector_detect, - .get_edid = display_connector_get_edid, + .edid_read = display_connector_edid_read, .atomic_get_output_bus_fmts = display_connector_get_output_bus_fmts, .atomic_get_input_bus_fmts = display_connector_get_input_bus_fmts, .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, -- 2.39.2
[PATCH v2 16/39] drm/bridge: cdns-mhdp8546: clear the EDID property on failures
If EDID read fails, clear the EDID property. Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index e44cb89c33f0..e226acc5c15e 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -1524,12 +1524,14 @@ static int cdns_mhdp_get_modes(struct drm_connector *connector) return 0; drm_edid = cdns_mhdp_edid_read(mhdp, connector); + + drm_edid_connector_update(connector, drm_edid); + if (!drm_edid) { dev_err(mhdp->dev, "Failed to read EDID\n"); return 0; } - drm_edid_connector_update(connector, drm_edid); num_modes = drm_edid_connector_add_modes(connector); drm_edid_free(drm_edid); -- 2.39.2
[PATCH v2 14/39] drm/bridge: anx7625: switch to ->edid_read callback
Prefer using the struct drm_edid based callback. v2: Fix build (goto out;) Signed-off-by: Jani Nikula --- drivers/gpu/drm/bridge/analogix/anx7625.c | 30 --- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c index ef31033439bc..25f7afa408c2 100644 --- a/drivers/gpu/drm/bridge/analogix/anx7625.c +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c @@ -1782,24 +1782,14 @@ static ssize_t anx7625_aux_transfer(struct drm_dp_aux *aux, return ret; } -static struct edid *anx7625_get_edid(struct anx7625_data *ctx) +static const struct drm_edid *anx7625_edid_read(struct anx7625_data *ctx) { struct device *dev = ctx->dev; struct s_edid_data *p_edid = >slimport_edid_p; int edid_num; - u8 *edid; - edid = kmalloc(FOUR_BLOCK_SIZE, GFP_KERNEL); - if (!edid) { - DRM_DEV_ERROR(dev, "Fail to allocate buffer\n"); - return NULL; - } - - if (ctx->slimport_edid_p.edid_block_num > 0) { - memcpy(edid, ctx->slimport_edid_p.edid_raw_data, - FOUR_BLOCK_SIZE); - return (struct edid *)edid; - } + if (ctx->slimport_edid_p.edid_block_num > 0) + goto out; pm_runtime_get_sync(dev); _anx7625_hpd_polling(ctx, 5000 * 100); @@ -1808,14 +1798,14 @@ static struct edid *anx7625_get_edid(struct anx7625_data *ctx) if (edid_num < 1) { DRM_DEV_ERROR(dev, "Fail to read EDID: %d\n", edid_num); - kfree(edid); return NULL; } p_edid->edid_block_num = edid_num; - memcpy(edid, ctx->slimport_edid_p.edid_raw_data, FOUR_BLOCK_SIZE); - return (struct edid *)edid; +out: + return drm_edid_alloc(ctx->slimport_edid_p.edid_raw_data, + FOUR_BLOCK_SIZE); } static enum drm_connector_status anx7625_sink_detect(struct anx7625_data *ctx) @@ -2488,15 +2478,15 @@ anx7625_bridge_detect(struct drm_bridge *bridge) return anx7625_sink_detect(ctx); } -static struct edid *anx7625_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *anx7625_bridge_edid_read(struct drm_bridge *bridge, + struct drm_connector *connector) { struct anx7625_data *ctx = bridge_to_anx7625(bridge); struct device *dev = ctx->dev; DRM_DEV_DEBUG_DRIVER(dev, "drm bridge get edid\n"); - return anx7625_get_edid(ctx); + return anx7625_edid_read(ctx); } static const struct drm_bridge_funcs anx7625_bridge_funcs = { @@ -2511,7 +2501,7 @@ static const struct drm_bridge_funcs anx7625_bridge_funcs = { .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, .atomic_reset = drm_atomic_helper_bridge_reset, .detect = anx7625_bridge_detect, - .get_edid = anx7625_bridge_get_edid, + .edid_read = anx7625_bridge_edid_read, }; static int anx7625_register_i2c_dummy_clients(struct anx7625_data *ctx, -- 2.39.2
[PATCH v2 15/39] drm/bridge: cdns-mhdp8546: switch to ->edid_read callback
Prefer using the struct drm_edid based callback and functions. Signed-off-by: Jani Nikula --- .../drm/bridge/cadence/cdns-mhdp8546-core.c | 26 +-- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index 7d470527455b..e44cb89c33f0 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -1505,33 +1505,33 @@ static void cdns_mhdp_link_down(struct cdns_mhdp_device *mhdp) mhdp->link_up = false; } -static struct edid *cdns_mhdp_get_edid(struct cdns_mhdp_device *mhdp, - struct drm_connector *connector) +static const struct drm_edid *cdns_mhdp_edid_read(struct cdns_mhdp_device *mhdp, + struct drm_connector *connector) { if (!mhdp->plugged) return NULL; - return drm_do_get_edid(connector, cdns_mhdp_get_edid_block, mhdp); + return drm_edid_read_custom(connector, cdns_mhdp_get_edid_block, mhdp); } static int cdns_mhdp_get_modes(struct drm_connector *connector) { struct cdns_mhdp_device *mhdp = connector_to_mhdp(connector); - struct edid *edid; + const struct drm_edid *drm_edid; int num_modes; if (!mhdp->plugged) return 0; - edid = cdns_mhdp_get_edid(mhdp, connector); - if (!edid) { + drm_edid = cdns_mhdp_edid_read(mhdp, connector); + if (!drm_edid) { dev_err(mhdp->dev, "Failed to read EDID\n"); return 0; } - drm_connector_update_edid_property(connector, edid); - num_modes = drm_add_edid_modes(connector, edid); - kfree(edid); + drm_edid_connector_update(connector, drm_edid); + num_modes = drm_edid_connector_add_modes(connector); + drm_edid_free(drm_edid); /* * HACK: Warn about unsupported display formats until we deal @@ -2220,12 +2220,12 @@ static enum drm_connector_status cdns_mhdp_bridge_detect(struct drm_bridge *brid return cdns_mhdp_detect(mhdp); } -static struct edid *cdns_mhdp_bridge_get_edid(struct drm_bridge *bridge, - struct drm_connector *connector) +static const struct drm_edid *cdns_mhdp_bridge_edid_read(struct drm_bridge *bridge, +struct drm_connector *connector) { struct cdns_mhdp_device *mhdp = bridge_to_mhdp(bridge); - return cdns_mhdp_get_edid(mhdp, connector); + return cdns_mhdp_edid_read(mhdp, connector); } static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = { @@ -2239,7 +2239,7 @@ static const struct drm_bridge_funcs cdns_mhdp_bridge_funcs = { .atomic_reset = cdns_mhdp_bridge_atomic_reset, .atomic_get_input_bus_fmts = cdns_mhdp_get_input_bus_fmts, .detect = cdns_mhdp_bridge_detect, - .get_edid = cdns_mhdp_bridge_get_edid, + .edid_read = cdns_mhdp_bridge_edid_read, .hpd_enable = cdns_mhdp_bridge_hpd_enable, .hpd_disable = cdns_mhdp_bridge_hpd_disable, }; -- 2.39.2
[PATCH v2 13/39] drm/bridge: remove drm_bridge_get_edid() in favour of drm_bridge_edid_read()
All users of drm_bridge_get_edid() have been converted to use drm_bridge_edid_read(). Remove drm_bridge_get_edid(). Signed-off-by: Jani Nikula --- drivers/gpu/drm/drm_bridge.c | 28 ++-- include/drm/drm_bridge.h | 2 -- 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 4f6f8c662d3f..a3065d4aa3d6 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -1217,7 +1217,7 @@ EXPORT_SYMBOL_GPL(drm_bridge_get_modes); * the EDID and return it. Otherwise return NULL. * * If _bridge_funcs.edid_read is not set, fall back to using - * drm_bridge_get_edid() and wrapping it in struct drm_edid. + * _bridge_funcs.get_edid and wrapping it in struct drm_edid. * * RETURNS: * The retrieved EDID on success, or NULL otherwise. @@ -1233,7 +1233,7 @@ const struct drm_edid *drm_bridge_edid_read(struct drm_bridge *bridge, const struct drm_edid *drm_edid; struct edid *edid; - edid = drm_bridge_get_edid(bridge, connector); + edid = bridge->funcs->get_edid(bridge, connector); if (!edid) return NULL; @@ -1248,30 +1248,6 @@ const struct drm_edid *drm_bridge_edid_read(struct drm_bridge *bridge, } EXPORT_SYMBOL_GPL(drm_bridge_edid_read); -/** - * drm_bridge_get_edid - get the EDID data of the connected display - * @bridge: bridge control structure - * @connector: the connector to read EDID for - * - * If the bridge supports output EDID retrieval, as reported by the - * DRM_BRIDGE_OP_EDID bridge ops flag, call _bridge_funcs.get_edid to - * get the EDID and return it. Otherwise return NULL. - * - * Deprecated. Prefer using drm_bridge_edid_read(). - * - * RETURNS: - * The retrieved EDID on success, or NULL otherwise. - */ -struct edid *drm_bridge_get_edid(struct drm_bridge *bridge, -struct drm_connector *connector) -{ - if (!(bridge->ops & DRM_BRIDGE_OP_EDID)) - return NULL; - - return bridge->funcs->get_edid(bridge, connector); -} -EXPORT_SYMBOL_GPL(drm_bridge_get_edid); - /** * drm_bridge_hpd_enable - enable hot plug detection for the bridge * @bridge: bridge control structure diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index b7aed3ead705..ee12f829aaf7 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -921,8 +921,6 @@ int drm_bridge_get_modes(struct drm_bridge *bridge, struct drm_connector *connector); const struct drm_edid *drm_bridge_edid_read(struct drm_bridge *bridge, struct drm_connector *connector); -struct edid *drm_bridge_get_edid(struct drm_bridge *bridge, -struct drm_connector *connector); void drm_bridge_hpd_enable(struct drm_bridge *bridge, void (*cb)(void *data, enum drm_connector_status status), -- 2.39.2