Re: [PATCH v17 04/15] mm, arm64: untag user pointers passed to memory syscalls
On 6/19/19 9:55 AM, Khalid Aziz wrote: > On 6/12/19 5:43 AM, Andrey Konovalov wrote: >> This patch is a part of a series that extends arm64 kernel ABI to allow to >> pass tagged user pointers (with the top byte set to something else other >> than 0x00) as syscall arguments. >> >> This patch allows tagged pointers to be passed to the following memory >> syscalls: get_mempolicy, madvise, mbind, mincore, mlock, mlock2, mprotect, >> mremap, msync, munlock, move_pages. >> >> The mmap and mremap syscalls do not currently accept tagged addresses. >> Architectures may interpret the tag as a background colour for the >> corresponding vma. >> >> Reviewed-by: Catalin Marinas >> Reviewed-by: Kees Cook >> Signed-off-by: Andrey Konovalov >> --- > > Reviewed-by: Khalid Aziz > > I would also recommend updating commit log for all the patches in this series that are changing files under mm/ as opposed to arch/arm64 to not reference arm64 kernel ABI since the change applies to every architecture. So something along the lines of "This patch is part of a series that extends kernel ABI to allow..." -- Khalid >> mm/madvise.c | 2 ++ >> mm/mempolicy.c | 3 +++ >> mm/migrate.c | 2 +- >> mm/mincore.c | 2 ++ >> mm/mlock.c | 4 >> mm/mprotect.c | 2 ++ >> mm/mremap.c| 7 +++ >> mm/msync.c | 2 ++ >> 8 files changed, 23 insertions(+), 1 deletion(-) >> >> diff --git a/mm/madvise.c b/mm/madvise.c >> index 628022e674a7..39b82f8a698f 100644 >> --- a/mm/madvise.c >> +++ b/mm/madvise.c >> @@ -810,6 +810,8 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, >> len_in, int, behavior) >> size_t len; >> struct blk_plug plug; >> >> +start = untagged_addr(start); >> + >> if (!madvise_behavior_valid(behavior)) >> return error; >> >> diff --git a/mm/mempolicy.c b/mm/mempolicy.c >> index 01600d80ae01..78e0a88b2680 100644 >> --- a/mm/mempolicy.c >> +++ b/mm/mempolicy.c >> @@ -1360,6 +1360,7 @@ static long kernel_mbind(unsigned long start, unsigned >> long len, >> int err; >> unsigned short mode_flags; >> >> +start = untagged_addr(start); >> mode_flags = mode & MPOL_MODE_FLAGS; >> mode &= ~MPOL_MODE_FLAGS; >> if (mode >= MPOL_MAX) >> @@ -1517,6 +1518,8 @@ static int kernel_get_mempolicy(int __user *policy, >> int uninitialized_var(pval); >> nodemask_t nodes; >> >> +addr = untagged_addr(addr); >> + >> if (nmask != NULL && maxnode < nr_node_ids) >> return -EINVAL; >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> index f2ecc2855a12..d22c45cf36b2 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -1616,7 +1616,7 @@ static int do_pages_move(struct mm_struct *mm, >> nodemask_t task_nodes, >> goto out_flush; >> if (get_user(node, nodes + i)) >> goto out_flush; >> -addr = (unsigned long)p; >> +addr = (unsigned long)untagged_addr(p); >> >> err = -ENODEV; >> if (node < 0 || node >= MAX_NUMNODES) >> diff --git a/mm/mincore.c b/mm/mincore.c >> index c3f058bd0faf..64c322ed845c 100644 >> --- a/mm/mincore.c >> +++ b/mm/mincore.c >> @@ -249,6 +249,8 @@ SYSCALL_DEFINE3(mincore, unsigned long, start, size_t, >> len, >> unsigned long pages; >> unsigned char *tmp; >> >> +start = untagged_addr(start); >> + >> /* Check the start address: needs to be page-aligned.. */ >> if (start & ~PAGE_MASK) >> return -EINVAL;fixup_user_fault >> diff --git a/mm/mlock.c b/mm/mlock.c >> index 080f3b36415b..e82609eaa428 100644 >> --- a/mm/mlock.c >> +++ b/mm/mlock.c >> @@ -674,6 +674,8 @@ static __must_check int do_mlock(unsigned long start, >> size_t len, vm_flags_t fla >> unsigned long lock_limit; >> int error = -ENOMEM; >> >> +start = untagged_addr(start); >> + >> if (!can_do_mlock()) >> return -EPERM; >> >> @@ -735,6 +737,8 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, >> len) >> { >> int ret; >> >> +start = untagged_addr(start); >> + >> len = PAGE_ALIGN(len + (offset_in_page(start))); >> start &= PAGE_MASK; >> >> diff --git a/mm/mprotect.c b/mm/mprotect.c >> index bf38dfbbb4b4..19f981b733bc 100644 >> --- a/mm/mprotect.c >> +++ b/mm/mprotect.c >> @@ -465,6 +465,8 @@ static int do_mprotect_pkey(unsigned long start, size_t >> len, >> const bool rier = (current->personality & READ_IMPLIES_EXEC) && >> (prot & PROT_READ); >> >> +start = untagged_addr(start); >> + >> prot &= ~(PROT_GROWSDOWN|PROT_GROWSUP); >> if (grows == (PROT_GROWSDOWN|PROT_GROWSUP)) /* can't be both */ >> return -EINVAL; >> diff --git a/mm/mremap.c b/mm/mremap.c >> index fc241d23cd97..64c9a3b8be0a 100644 >> --- a/mm/mremap.c >> +++ b/mm/mremap.c >> @@ -606,6 +606,13 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned >> long, old_len, >> LIST_HEAD(uf_unmap_early); >>
Re: [PATCH v17 12/15] media/v4l2-core, arm64: untag user pointers in videobuf_dma_contig_user_get
On 6/12/19 5:43 AM, Andrey Konovalov wrote: > This patch is a part of a series that extends arm64 kernel ABI to allow to > pass tagged user pointers (with the top byte set to something else other > than 0x00) as syscall arguments. > > videobuf_dma_contig_user_get() uses provided user pointers for vma > lookups, which can only by done with untagged pointers. > > Untag the pointers in this function. > > Reviewed-by: Kees Cook > Acked-by: Mauro Carvalho Chehab > Signed-off-by: Andrey Konovalov > --- Patch looks good, but commit log should be updated to not be specific to arm64. Reviewed-by: Khalid Aziz > drivers/media/v4l2-core/videobuf-dma-contig.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > exact_copy_from_user > diff --git a/drivers/media/v4l2-core/videobuf-dma-contig.c > b/drivers/media/v4l2-core/videobuf-dma-contig.c > index e1bf50df4c70..8a1ddd146b17 100644 > --- a/drivers/media/v4l2-core/videobuf-dma-contig.c > +++ b/drivers/media/v4l2-core/videobuf-dma-contig.c > @@ -160,6 +160,7 @@ static void videobuf_dma_contig_user_put(struct > videobuf_dma_contig_memory *mem) > static int videobuf_dma_contig_user_get(struct videobuf_dma_contig_memory > *mem, > struct videobuf_buffer *vb) > { > + unsigned long untagged_baddr = untagged_addr(vb->baddr); > struct mm_struct *mm = current->mm; > struct vm_area_struct *vma; > unsigned long prev_pfn, this_pfn; > @@ -167,22 +168,22 @@ static int videobuf_dma_contig_user_get(struct > videobuf_dma_contig_memory *mem, > unsigned int offset; > int ret; > > - offset = vb->baddr & ~PAGE_MASK; > + offset = untagged_baddr & ~PAGE_MASK; > mem->size = PAGE_ALIGN(vb->size + offset); > ret = -EINVAL; > > down_read(&mm->mmap_sem); > > - vma = find_vma(mm, vb->baddr); > + vma = find_vma(mm, untagged_baddr); > if (!vma) > goto out_up; > > - if ((vb->baddr + mem->size) > vma->vm_end) > + if ((untagged_baddr + mem->size) > vma->vm_end) > goto out_up; > > pages_done = 0; > prev_pfn = 0; /* kill warning */ > - user_address = vb->baddr; > + user_address = untagged_baddr; > > while (pages_done < (mem->size >> PAGE_SHIFT)) { > ret = follow_pfn(vma, user_address, &this_pfn); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: dev_pagemap related cleanups v2
On Wed, Jun 19, 2019 at 11:40:32AM +0200, Christoph Hellwig wrote: > On Tue, Jun 18, 2019 at 12:47:10PM -0700, Dan Williams wrote: > > > Git tree: > > > > > > git://git.infradead.org/users/hch/misc.git hmm-devmem-cleanup.2 > > > > > > Gitweb: > > > > > > > > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/hmm-devmem-cleanup.2 > > > > > Attached is my incremental fixups on top of this series, with those > > integrated you can add: > > I've folded your incremental bits in and pushed out a new > hmm-devmem-cleanup.3 to the repo above. Let me know if I didn't mess > up anything else. I'll wait for a few more comments and Jason's > planned rebase of the hmm branch before reposting. I said I wouldn't rebase the hmm.git (as it needs to go to DRM, AMD and RDMA git trees).. Instead I will merge v5.2-rc5 to the tree before applying this series. I've understood this to be Linus's prefered workflow. So, please send the next iteration of this against either plainv5.2-rc5 or v5.2-rc5 merged with hmm.git and I'll sort it out. Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH net-next] br_netfilter: prevent UAF in brnf_exit_net()
Prevent a UAF in brnf_exit_net(). When unregister_net_sysctl_table() is called the ctl_hdr pointer will obviously be freed and so accessing it righter after is invalid. Fix this by stashing a pointer to the table we want to free before we unregister the sysctl header. Note that syzkaller falsely chased this down to the drm tree so the Fixes tag that syzkaller requested would be wrong. This commit uses a different but the correct Fixes tag. /* Splat */ BUG: KASAN: use-after-free in br_netfilter_sysctl_exit_net net/bridge/br_netfilter_hooks.c:1121 [inline] BUG: KASAN: use-after-free in brnf_exit_net+0x38c/0x3a0 net/bridge/br_netfilter_hooks.c:1141 Read of size 8 at addr 8880a4078d60 by task kworker/u4:4/8749 CPU: 0 PID: 8749 Comm: kworker/u4:4 Not tainted 5.2.0-rc5-next-20190618 #17 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: netns cleanup_net Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x172/0x1f0 lib/dump_stack.c:113 print_address_description.cold+0xd4/0x306 mm/kasan/report.c:351 __kasan_report.cold+0x1b/0x36 mm/kasan/report.c:482 kasan_report+0x12/0x20 mm/kasan/common.c:614 __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:132 br_netfilter_sysctl_exit_net net/bridge/br_netfilter_hooks.c:1121 [inline] brnf_exit_net+0x38c/0x3a0 net/bridge/br_netfilter_hooks.c:1141 ops_exit_list.isra.0+0xaa/0x150 net/core/net_namespace.c:154 cleanup_net+0x3fb/0x960 net/core/net_namespace.c:553 process_one_work+0x989/0x1790 kernel/workqueue.c:2269 worker_thread+0x98/0xe40 kernel/workqueue.c:2415 kthread+0x354/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Allocated by task 11374: save_stack+0x23/0x90 mm/kasan/common.c:71 set_track mm/kasan/common.c:79 [inline] __kasan_kmalloc mm/kasan/common.c:489 [inline] __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:462 kasan_kmalloc+0x9/0x10 mm/kasan/common.c:503 __do_kmalloc mm/slab.c:3645 [inline] __kmalloc+0x15c/0x740 mm/slab.c:3654 kmalloc include/linux/slab.h:552 [inline] kzalloc include/linux/slab.h:743 [inline] __register_sysctl_table+0xc7/0xef0 fs/proc/proc_sysctl.c:1327 register_net_sysctl+0x29/0x30 net/sysctl_net.c:121 br_netfilter_sysctl_init_net net/bridge/br_netfilter_hooks.c:1105 [inline] brnf_init_net+0x379/0x6a0 net/bridge/br_netfilter_hooks.c:1126 ops_init+0xb3/0x410 net/core/net_namespace.c:130 setup_net+0x2d3/0x740 net/core/net_namespace.c:316 copy_net_ns+0x1df/0x340 net/core/net_namespace.c:439 create_new_namespaces+0x400/0x7b0 kernel/nsproxy.c:103 unshare_nsproxy_namespaces+0xc2/0x200 kernel/nsproxy.c:202 ksys_unshare+0x444/0x980 kernel/fork.c:2822 __do_sys_unshare kernel/fork.c:2890 [inline] __se_sys_unshare kernel/fork.c:2888 [inline] __x64_sys_unshare+0x31/0x40 kernel/fork.c:2888 do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301 entry_SYSCALL_64_after_hwframe+0x49/0xbe Freed by task 9: save_stack+0x23/0x90 mm/kasan/common.c:71 set_track mm/kasan/common.c:79 [inline] __kasan_slab_free+0x102/0x150 mm/kasan/common.c:451 kasan_slab_free+0xe/0x10 mm/kasan/common.c:459 __cache_free mm/slab.c:3417 [inline] kfree+0x10a/0x2c0 mm/slab.c:3746 __rcu_reclaim kernel/rcu/rcu.h:215 [inline] rcu_do_batch kernel/rcu/tree.c:2092 [inline] invoke_rcu_callbacks kernel/rcu/tree.c:2310 [inline] rcu_core+0xcc7/0x1500 kernel/rcu/tree.c:2291 __do_softirq+0x25c/0x94c kernel/softirq.c:292 The buggy address belongs to the object at 8880a4078d40 which belongs to the cache kmalloc-512 of size 512 The buggy address is located 32 bytes inside of 512-byte region [8880a4078d40, 8880a4078f40) The buggy address belongs to the page: page:ea0002901e00 refcount:1 mapcount:0 mapping:8880aa400a80 index:0x8880a40785c0 flags: 0x1fffc000200(slab) raw: 01fffc000200 ea0001d636c8 ea0001b07308 8880aa400a80 raw: 8880a40785c0 8880a40780c0 00010004 page dumped because: kasan: bad access detected Memory state around the buggy address: 8880a4078c00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb 8880a4078c80: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc > 8880a4078d00: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb ^ 8880a4078d80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb 8880a4078e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb Reported-by: syzbot+43a3fa52c0d9c5c94...@syzkaller.appspotmail.com Fixes: 22567590b2e6 ("netfilter: bridge: namespace bridge netfilter sysctls") Signed-off-by: Christian Brauner --- net/bridge/br_netfilter_hooks.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c index fd9e991c1189..d3f9592f4ff8 100644 --- a/net/bridge/br_netfilter_hooks.c +++ b/net/bridge/br_netfilter_hooks.c @@ -1116,9 +1116,11 @@ static int br_netfilter_sysctl_init_net(s
Re: dev_pagemap related cleanups v2
On Wed, Jun 19, 2019 at 09:46:23AM -0700, Dan Williams wrote: > On Wed, Jun 19, 2019 at 9:37 AM Jason Gunthorpe wrote: > > > > On Wed, Jun 19, 2019 at 11:40:32AM +0200, Christoph Hellwig wrote: > > > On Tue, Jun 18, 2019 at 12:47:10PM -0700, Dan Williams wrote: > > > > > Git tree: > > > > > > > > > > git://git.infradead.org/users/hch/misc.git hmm-devmem-cleanup.2 > > > > > > > > > > Gitweb: > > > > > > > > > > > > > > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/hmm-devmem-cleanup.2 > > > > > > > > > > > Attached is my incremental fixups on top of this series, with those > > > > integrated you can add: > > > > > > I've folded your incremental bits in and pushed out a new > > > hmm-devmem-cleanup.3 to the repo above. Let me know if I didn't mess > > > up anything else. I'll wait for a few more comments and Jason's > > > planned rebase of the hmm branch before reposting. > > > > I said I wouldn't rebase the hmm.git (as it needs to go to DRM, AMD > > and RDMA git trees).. > > > > Instead I will merge v5.2-rc5 to the tree before applying this series. > > > > I've understood this to be Linus's prefered workflow. > > > > So, please send the next iteration of this against either > > plainv5.2-rc5 or v5.2-rc5 merged with hmm.git and I'll sort it out. > > Just make sure that when you backmerge v5.2-rc5 you have a clear > reason in the merge commit message about why you needed to do it. > While needless rebasing is top of the pet peeve list, second place, as > I found out, is mystery merges without explanations. Yes, I always describe the merge commits. Linus also particular about having *good reasons* for merges. This is why I can't fix the hmm.git to have rc5 until I have patches to apply.. Probbaly I will just put CH's series on rc5 and merge it with the cover letter as the merge message. This avoid both rebasing and gives purposeful merges. Thanks, Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v17 07/15] fs, arm64: untag user pointers in copy_mount_options
On 6/12/19 5:43 AM, Andrey Konovalov wrote: > This patch is a part of a series that extends arm64 kernel ABI to allow to > pass tagged user pointers (with the top byte set to something else other > than 0x00) as syscall arguments. > > In copy_mount_options a user address is being subtracted from TASK_SIZE. > If the address is lower than TASK_SIZE, the size is calculated to not > allow the exact_copy_from_user() call to cross TASK_SIZE boundary. > However if the address is tagged, then the size will be calculated > incorrectly. > > Untag the address before subtracting. > > Reviewed-by: Kees Cook > Reviewed-by: Catalin Marinas > Signed-off-by: Andrey Konovalov > --- Please update commit log to make it not arm64 specific since this change affects other architectures as well. Other than that, Reviewed-by: Khalid Aziz > fs/namespace.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/namespace.c b/fs/namespace.c > index b26778bdc236..2e85712a19ed 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -2993,7 +2993,7 @@ void *copy_mount_options(const void __user * data) >* the remainder of the page. >*/ > /* copy_from_user cannot cross TASK_SIZE ! */ > - size = TASK_SIZE - (unsigned long)data; > + size = TASK_SIZE - (unsigned long)untagged_addr(data); > if (size > PAGE_SIZE) > size = PAGE_SIZE; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/7] Hexdump Enhancements
On Wed, 2019-06-19 at 17:35 -0700, Joe Perches wrote: > On Thu, 2019-06-20 at 09:15 +1000, Alastair D'Silva wrote: > > On Wed, 2019-06-19 at 09:31 -0700, Joe Perches wrote: > > > On Mon, 2019-06-17 at 12:04 +1000, Alastair D'Silva wrote: > > > > From: Alastair D'Silva > > > > > > > > Apologies for the large CC list, it's a heads up for those > > > > responsible > > > > for subsystems where a prototype change in generic code causes > > > > a > > > > change > > > > in those subsystems. > > > > > > > > This series enhances hexdump. > > > > > > Still not a fan of these patches. > > > > I'm afraid there's not too much action I can take on that, I'm > > happy to > > address specific issues though. > > > > > > These improve the readability of the dumped data in certain > > > > situations > > > > (eg. wide terminals are available, many lines of empty bytes > > > > exist, > > > > etc). > > I think it's generally overkill for the desired uses. I understand where you're coming from, however, these patches make it a lot easier to work with large chucks of binary data. I think it makes more sense to have these patches upstream, even though committed code may not necessarily have all the features enabled, as it means that devs won't have to apply out-of-tree patches during development to make larger dumps manageable. > > > > Changing hexdump's last argument from bool to int is odd. > > > > > > > Think of it as replacing a single boolean with many booleans. > > I understand it. It's odd. > > I would rather not have a mixture of true, false, and apparently > random collections of bitfields like 0xd or 0b1011 or their > equivalent or'd defines. > Where's the mixture? What would you propose instead? -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva Twitter: @EvilDeece blog: http://alastair.d-silva.org ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/7] Hexdump Enhancements
On Wed, 2019-06-19 at 09:31 -0700, Joe Perches wrote: > On Mon, 2019-06-17 at 12:04 +1000, Alastair D'Silva wrote: > > From: Alastair D'Silva > > > > Apologies for the large CC list, it's a heads up for those > > responsible > > for subsystems where a prototype change in generic code causes a > > change > > in those subsystems. > > > > This series enhances hexdump. > > Still not a fan of these patches. I'm afraid there's not too much action I can take on that, I'm happy to address specific issues though. > > > These improve the readability of the dumped data in certain > > situations > > (eg. wide terminals are available, many lines of empty bytes exist, > > etc). > > Changing hexdump's last argument from bool to int is odd. > Think of it as replacing a single boolean with many booleans. > Perhaps a new function should be added instead of changing > the existing hexdump. > There's only a handful of consumers, I don't think there is a value-add in creating more wrappers vs updating the existing callers. -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva Twitter: @EvilDeece blog: http://alastair.d-silva.org ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 18/22] mm: mark DEVICE_PUBLIC as broken
On Thu, Jun 13, 2019 at 06:23:04PM -0700, John Hubbard wrote: > On 6/13/19 5:43 PM, Ira Weiny wrote: > > On Thu, Jun 13, 2019 at 07:58:29PM +, Jason Gunthorpe wrote: > >> On Thu, Jun 13, 2019 at 12:53:02PM -0700, Ralph Campbell wrote: > >>> > ... > >> Hum, so the only thing this config does is short circuit here: > >> > >> static inline bool is_device_public_page(const struct page *page) > >> { > >> return IS_ENABLED(CONFIG_DEV_PAGEMAP_OPS) && > >> IS_ENABLED(CONFIG_DEVICE_PUBLIC) && > >> is_zone_device_page(page) && > >> page->pgmap->type == MEMORY_DEVICE_PUBLIC; > >> } > >> > >> Which is called all over the place.. > > > > yes but the earlier patch: > > > > [PATCH 03/22] mm: remove hmm_devmem_add_resource > > > > Removes the only place type is set to MEMORY_DEVICE_PUBLIC. > > > > So I think it is ok. Frankly I was wondering if we should remove the public > > type altogether but conceptually it seems ok. But I don't see any users of > > it > > so... should we get rid of it in the code rather than turning the config > > off? > > > > Ira > > That seems reasonable. I recall that the hope was for those IBM Power 9 > systems to use _PUBLIC, as they have hardware-based coherent device (GPU) > memory, and so the memory really is visible to the CPU. And the IBM team > was thinking of taking advantage of it. But I haven't seen anything on > that front for a while. Does anyone know who those people are and can we encourage them to send some patches? :) Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v17 06/15] mm, arm64: untag user pointers in get_vaddr_frames
On 6/12/19 5:43 AM, Andrey Konovalov wrote: > This patch is a part of a series that extends arm64 kernel ABI to allow to > pass tagged user pointers (with the top byte set to something else other > than 0x00) as syscall arguments. > > get_vaddr_frames uses provided user pointers for vma lookups, which can > only by done with untagged pointers. Instead of locating and changing > all callers of this function, perform untagging in it. > > Acked-by: Catalin Marinas > Reviewed-by: Kees Cook > Signed-off-by: Andrey Konovalov > --- With the suggested change to commit log in my previous email: Reviewed-by: Khalid Aziz > mm/frame_vector.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/mm/frame_vector.c b/mm/frame_vector.c > index c64dca6e27c2..c431ca81dad5 100644 > --- a/mm/frame_vector.c > +++ b/mm/frame_vector.c > @@ -46,6 +46,8 @@ int get_vaddr_frames(unsigned long start, unsigned int > nr_frames, > if (WARN_ON_ONCE(nr_frames > vec->nr_allocated)) > nr_frames = vec->nr_allocated; > > + start = untagged_addr(start); > + > down_read(&mm->mmap_sem); > locked = 1; > vma = find_vma_intersection(mm, start, start + 1); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v17 03/15] arm64: Introduce prctl() options to control the tagged user addresses ABI
On Wed, Jun 19, 2019 at 04:45:02PM +0200, Andrey Konovalov wrote: > On Wed, Jun 12, 2019 at 1:43 PM Andrey Konovalov > wrote: > > From: Catalin Marinas > > > > It is not desirable to relax the ABI to allow tagged user addresses into > > the kernel indiscriminately. This patch introduces a prctl() interface > > for enabling or disabling the tagged ABI with a global sysctl control > > for preventing applications from enabling the relaxed ABI (meant for > > testing user-space prctl() return error checking without reconfiguring > > the kernel). The ABI properties are inherited by threads of the same > > application and fork()'ed children but cleared on execve(). > > > > The PR_SET_TAGGED_ADDR_CTRL will be expanded in the future to handle > > MTE-specific settings like imprecise vs precise exceptions. > > > > Signed-off-by: Catalin Marinas > > Catalin, would you like to do the requested changes to this patch > yourself and send it to me or should I do that? I'll send you an updated version this week. -- Catalin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dsi: Add parentheses to quirks check in dsi_phy_hw_v3_0_lane_settings
Clang warns: drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:80:6: warning: logical not is only applied to the left hand side of this bitwise operator [-Wlogical-not-parentheses] if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) { ^ ~ drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:80:6: note: add parentheses after the '!' to evaluate the bitwise operator first if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) { ^ ( ) drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:80:6: note: add parentheses around left hand side expression to silence this warning if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) { ^ () 1 warning generated. Add parentheses around the bitwise AND so it is evaluated first then negated. Fixes: 3dbbf8f09e83 ("drm/msm/dsi: Add old timings quirk for 10nm phy") Link: https://github.com/ClangBuiltLinux/linux/547 Reported-by: kbuild test robot Reviewed-by: Jeffrey Hugo Signed-off-by: Nathan Chancellor --- drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c index eb28937f4b34..47403d4f2d28 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c @@ -77,7 +77,7 @@ static void dsi_phy_hw_v3_0_lane_settings(struct msm_dsi_phy *phy) tx_dctrl[i]); } - if (!phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK) { + if (!(phy->cfg->quirks & V3_0_0_10NM_OLD_TIMINGS_QUIRK)) { /* Toggle BIT 0 to release freeze I/0 */ dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_TX_DCTRL(3), 0x05); dsi_phy_write(lane_base + REG_DSI_10nm_PHY_LN_TX_DCTRL(3), 0x04); -- 2.22.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
KASAN: use-after-free Read in brnf_exit_net
Hello, syzbot found the following crash on: HEAD commit:1c6b4050 Add linux-next specific files for 20190618 git tree: linux-next console output: https://syzkaller.appspot.com/x/log.txt?x=10126209a0 kernel config: https://syzkaller.appspot.com/x/.config?x=3c614278993de456 dashboard link: https://syzkaller.appspot.com/bug?extid=43a3fa52c0d9c5c94f41 compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16291176a0 The bug was bisected to: commit b38d37a08ec4b19a9b9ec3a1ff5566781fcae1f1 Author: Stephen Rothwell Date: Tue Jun 18 04:19:55 2019 + Merge remote-tracking branch 'drm/drm-next' bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=146f914ea0 final crash:https://syzkaller.appspot.com/x/report.txt?x=166f914ea0 console output: https://syzkaller.appspot.com/x/log.txt?x=126f914ea0 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+43a3fa52c0d9c5c94...@syzkaller.appspotmail.com Fixes: b38d37a08ec4 ("Merge remote-tracking branch 'drm/drm-next'") == BUG: KASAN: use-after-free in br_netfilter_sysctl_exit_net net/bridge/br_netfilter_hooks.c:1121 [inline] BUG: KASAN: use-after-free in brnf_exit_net+0x38c/0x3a0 net/bridge/br_netfilter_hooks.c:1141 Read of size 8 at addr 8880a4078d60 by task kworker/u4:4/8749 CPU: 0 PID: 8749 Comm: kworker/u4:4 Not tainted 5.2.0-rc5-next-20190618 #17 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: netns cleanup_net Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x172/0x1f0 lib/dump_stack.c:113 print_address_description.cold+0xd4/0x306 mm/kasan/report.c:351 __kasan_report.cold+0x1b/0x36 mm/kasan/report.c:482 kasan_report+0x12/0x20 mm/kasan/common.c:614 __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:132 br_netfilter_sysctl_exit_net net/bridge/br_netfilter_hooks.c:1121 [inline] brnf_exit_net+0x38c/0x3a0 net/bridge/br_netfilter_hooks.c:1141 ops_exit_list.isra.0+0xaa/0x150 net/core/net_namespace.c:154 cleanup_net+0x3fb/0x960 net/core/net_namespace.c:553 process_one_work+0x989/0x1790 kernel/workqueue.c:2269 worker_thread+0x98/0xe40 kernel/workqueue.c:2415 kthread+0x354/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Allocated by task 11374: save_stack+0x23/0x90 mm/kasan/common.c:71 set_track mm/kasan/common.c:79 [inline] __kasan_kmalloc mm/kasan/common.c:489 [inline] __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:462 kasan_kmalloc+0x9/0x10 mm/kasan/common.c:503 __do_kmalloc mm/slab.c:3645 [inline] __kmalloc+0x15c/0x740 mm/slab.c:3654 kmalloc include/linux/slab.h:552 [inline] kzalloc include/linux/slab.h:743 [inline] __register_sysctl_table+0xc7/0xef0 fs/proc/proc_sysctl.c:1327 register_net_sysctl+0x29/0x30 net/sysctl_net.c:121 br_netfilter_sysctl_init_net net/bridge/br_netfilter_hooks.c:1105 [inline] brnf_init_net+0x379/0x6a0 net/bridge/br_netfilter_hooks.c:1126 ops_init+0xb3/0x410 net/core/net_namespace.c:130 setup_net+0x2d3/0x740 net/core/net_namespace.c:316 copy_net_ns+0x1df/0x340 net/core/net_namespace.c:439 create_new_namespaces+0x400/0x7b0 kernel/nsproxy.c:103 unshare_nsproxy_namespaces+0xc2/0x200 kernel/nsproxy.c:202 ksys_unshare+0x444/0x980 kernel/fork.c:2822 __do_sys_unshare kernel/fork.c:2890 [inline] __se_sys_unshare kernel/fork.c:2888 [inline] __x64_sys_unshare+0x31/0x40 kernel/fork.c:2888 do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301 entry_SYSCALL_64_after_hwframe+0x49/0xbe Freed by task 9: save_stack+0x23/0x90 mm/kasan/common.c:71 set_track mm/kasan/common.c:79 [inline] __kasan_slab_free+0x102/0x150 mm/kasan/common.c:451 kasan_slab_free+0xe/0x10 mm/kasan/common.c:459 __cache_free mm/slab.c:3417 [inline] kfree+0x10a/0x2c0 mm/slab.c:3746 __rcu_reclaim kernel/rcu/rcu.h:215 [inline] rcu_do_batch kernel/rcu/tree.c:2092 [inline] invoke_rcu_callbacks kernel/rcu/tree.c:2310 [inline] rcu_core+0xcc7/0x1500 kernel/rcu/tree.c:2291 __do_softirq+0x25c/0x94c kernel/softirq.c:292 The buggy address belongs to the object at 8880a4078d40 which belongs to the cache kmalloc-512 of size 512 The buggy address is located 32 bytes inside of 512-byte region [8880a4078d40, 8880a4078f40) The buggy address belongs to the page: page:ea0002901e00 refcount:1 mapcount:0 mapping:8880aa400a80 index:0x8880a40785c0 flags: 0x1fffc000200(slab) raw: 01fffc000200 ea0001d636c8 ea0001b07308 8880aa400a80 raw: 8880a40785c0 8880a40780c0 00010004 page dumped because: kasan: bad access detected Memory state around the buggy address: 8880a4078c00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb 8880a4078c80: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc 8880a4078d00: fc fc fc fc fc fc fc fc fb fb f
[PATCH v1] backlight: gpio_backlight: Enable ACPI enumeration
ACPI allows to enumerate specific devices by using compatible strings. Enable that enumeration for GPIO based backlight devices. Signed-off-by: Andy Shevchenko --- drivers/video/backlight/gpio_backlight.c | 23 +-- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index e470da95d806..05c12df62b27 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -18,6 +18,7 @@ #include #include #include +#include #include struct gpio_backlight { @@ -61,11 +62,10 @@ static int gpio_backlight_probe_dt(struct platform_device *pdev, struct gpio_backlight *gbl) { struct device *dev = &pdev->dev; - struct device_node *np = dev->of_node; enum gpiod_flags flags; int ret; - gbl->def_value = of_property_read_bool(np, "default-on"); + gbl->def_value = device_property_read_bool(dev, "default-on"); flags = gbl->def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW; gbl->gpiod = devm_gpiod_get(dev, NULL, flags); @@ -89,26 +89,19 @@ static int gpio_backlight_probe(struct platform_device *pdev) struct backlight_properties props; struct backlight_device *bl; struct gpio_backlight *gbl; - struct device_node *np = pdev->dev.of_node; int ret; - if (!pdata && !np) { - dev_err(&pdev->dev, - "failed to find platform data or device tree node.\n"); - return -ENODEV; - } - gbl = devm_kzalloc(&pdev->dev, sizeof(*gbl), GFP_KERNEL); if (gbl == NULL) return -ENOMEM; gbl->dev = &pdev->dev; - if (np) { + if (pdev->dev.fwnode) { ret = gpio_backlight_probe_dt(pdev, gbl); if (ret) return ret; - } else { + } else if (pdata) { /* * Legacy platform data GPIO retrieveal. Do not expand * the use of this code path, currently only used by one @@ -129,6 +122,10 @@ static int gpio_backlight_probe(struct platform_device *pdev) gbl->gpiod = gpio_to_desc(pdata->gpio); if (!gbl->gpiod) return -EINVAL; + } else { + dev_err(&pdev->dev, + "failed to find platform data or device tree node.\n"); + return -ENODEV; } memset(&props, 0, sizeof(props)); @@ -149,19 +146,17 @@ static int gpio_backlight_probe(struct platform_device *pdev) return 0; } -#ifdef CONFIG_OF static struct of_device_id gpio_backlight_of_match[] = { { .compatible = "gpio-backlight" }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, gpio_backlight_of_match); -#endif static struct platform_driver gpio_backlight_driver = { .driver = { .name = "gpio-backlight", - .of_match_table = of_match_ptr(gpio_backlight_of_match), + .of_match_table = gpio_backlight_of_match, }, .probe = gpio_backlight_probe, }; -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v17 04/15] mm, arm64: untag user pointers passed to memory syscalls
On 6/12/19 5:43 AM, Andrey Konovalov wrote: > This patch is a part of a series that extends arm64 kernel ABI to allow to > pass tagged user pointers (with the top byte set to something else other > than 0x00) as syscall arguments. > > This patch allows tagged pointers to be passed to the following memory > syscalls: get_mempolicy, madvise, mbind, mincore, mlock, mlock2, mprotect, > mremap, msync, munlock, move_pages. > > The mmap and mremap syscalls do not currently accept tagged addresses. > Architectures may interpret the tag as a background colour for the > corresponding vma. > > Reviewed-by: Catalin Marinas > Reviewed-by: Kees Cook > Signed-off-by: Andrey Konovalov > --- Reviewed-by: Khalid Aziz > mm/madvise.c | 2 ++ > mm/mempolicy.c | 3 +++ > mm/migrate.c | 2 +- > mm/mincore.c | 2 ++ > mm/mlock.c | 4 > mm/mprotect.c | 2 ++ > mm/mremap.c| 7 +++ > mm/msync.c | 2 ++ > 8 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/mm/madvise.c b/mm/madvise.c > index 628022e674a7..39b82f8a698f 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -810,6 +810,8 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, > len_in, int, behavior) > size_t len; > struct blk_plug plug; > > + start = untagged_addr(start); > + > if (!madvise_behavior_valid(behavior)) > return error; > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index 01600d80ae01..78e0a88b2680 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -1360,6 +1360,7 @@ static long kernel_mbind(unsigned long start, unsigned > long len, > int err; > unsigned short mode_flags; > > + start = untagged_addr(start); > mode_flags = mode & MPOL_MODE_FLAGS; > mode &= ~MPOL_MODE_FLAGS; > if (mode >= MPOL_MAX) > @@ -1517,6 +1518,8 @@ static int kernel_get_mempolicy(int __user *policy, > int uninitialized_var(pval); > nodemask_t nodes; > > + addr = untagged_addr(addr); > + > if (nmask != NULL && maxnode < nr_node_ids) > return -EINVAL; > > diff --git a/mm/migrate.c b/mm/migrate.c > index f2ecc2855a12..d22c45cf36b2 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -1616,7 +1616,7 @@ static int do_pages_move(struct mm_struct *mm, > nodemask_t task_nodes, > goto out_flush; > if (get_user(node, nodes + i)) > goto out_flush; > - addr = (unsigned long)p; > + addr = (unsigned long)untagged_addr(p); > > err = -ENODEV; > if (node < 0 || node >= MAX_NUMNODES) > diff --git a/mm/mincore.c b/mm/mincore.c > index c3f058bd0faf..64c322ed845c 100644 > --- a/mm/mincore.c > +++ b/mm/mincore.c > @@ -249,6 +249,8 @@ SYSCALL_DEFINE3(mincore, unsigned long, start, size_t, > len, > unsigned long pages; > unsigned char *tmp; > > + start = untagged_addr(start); > + > /* Check the start address: needs to be page-aligned.. */ > if (start & ~PAGE_MASK) > return -EINVAL; > diff --git a/mm/mlock.c b/mm/mlock.c > index 080f3b36415b..e82609eaa428 100644 > --- a/mm/mlock.c > +++ b/mm/mlock.c > @@ -674,6 +674,8 @@ static __must_check int do_mlock(unsigned long start, > size_t len, vm_flags_t fla > unsigned long lock_limit; > int error = -ENOMEM; > > + start = untagged_addr(start); > + > if (!can_do_mlock()) > return -EPERM; > > @@ -735,6 +737,8 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, > len) > { > int ret; > > + start = untagged_addr(start); > + > len = PAGE_ALIGN(len + (offset_in_page(start))); > start &= PAGE_MASK; > > diff --git a/mm/mprotect.c b/mm/mprotect.c > index bf38dfbbb4b4..19f981b733bc 100644 > --- a/mm/mprotect.c > +++ b/mm/mprotect.c > @@ -465,6 +465,8 @@ static int do_mprotect_pkey(unsigned long start, size_t > len, > const bool rier = (current->personality & READ_IMPLIES_EXEC) && > (prot & PROT_READ); > > + start = untagged_addr(start); > + > prot &= ~(PROT_GROWSDOWN|PROT_GROWSUP); > if (grows == (PROT_GROWSDOWN|PROT_GROWSUP)) /* can't be both */ > return -EINVAL; > diff --git a/mm/mremap.c b/mm/mremap.c > index fc241d23cd97..64c9a3b8be0a 100644 > --- a/mm/mremap.c > +++ b/mm/mremap.c > @@ -606,6 +606,13 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned > long, old_len, > LIST_HEAD(uf_unmap_early); > LIST_HEAD(uf_unmap); > > + /* > + * Architectures may interpret the tag passed to mmap as a background > + * colour for the corresponding vma. For mremap we don't allow tagged > + * new_addr to preserve similar behaviour to mmap. > + */ > + addr = untagged_addr(addr); > + > if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE)) > return ret; > > diff --git a/mm/msync.c b/mm/msync.c > index ef30a429623a..c3bd3e75f687 100644 > --- a/mm/msync.
Re: [PATCH v17 05/15] mm, arm64: untag user pointers in mm/gup.c
On 6/12/19 5:43 AM, Andrey Konovalov wrote: > This patch is a part of a series that extends arm64 kernel ABI to allow to > pass tagged user pointers (with the top byte set to something else other > than 0x00) as syscall arguments. > > mm/gup.c provides a kernel interface that accepts user addresses and > manipulates user pages directly (for example get_user_pages, that is used > by the futex syscall). Since a user can provided tagged addresses, we need > to handle this case. > > Add untagging to gup.c functions that use user addresses for vma lookups. > > Reviewed-by: Kees Cook > Reviewed-by: Catalin Marinas > Signed-off-by: Andrey Konovalov > --- Reviewed-by: Khalid Aziz > mm/gup.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/mm/gup.c b/mm/gup.c > index ddde097cf9e4..c37df3d455a2 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -802,6 +802,8 @@ static long __get_user_pages(struct task_struct *tsk, > struct mm_struct *mm, > if (!nr_pages) > return 0; > > + start = untagged_addr(start); > + > VM_BUG_ON(!!pages != !!(gup_flags & FOLL_GET)); > > /* > @@ -964,6 +966,8 @@ int fixup_user_fault(struct task_struct *tsk, struct > mm_struct *mm, > struct vm_area_struct *vma; > vm_fault_t ret, major = 0; > > + address = untagged_addr(address); > + > if (unlocked) > fault_flags |= FAULT_FLAG_ALLOW_RETRY; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/6] drm/qxl: use embedded gem object
Drop drm_gem_object from qxl_bo, use the ttm_buffer_object.base instead. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_drv.h | 6 +++--- drivers/gpu/drm/qxl/qxl_object.h | 4 ++-- drivers/gpu/drm/qxl/qxl_cmd.c | 4 ++-- drivers/gpu/drm/qxl/qxl_debugfs.c | 2 +- drivers/gpu/drm/qxl/qxl_display.c | 8 drivers/gpu/drm/qxl/qxl_gem.c | 2 +- drivers/gpu/drm/qxl/qxl_object.c | 20 ++-- drivers/gpu/drm/qxl/qxl_release.c | 2 +- drivers/gpu/drm/qxl/qxl_ttm.c | 4 ++-- 9 files changed, 26 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h index 2896bb6fdbf4..b80d4a4361cd 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.h +++ b/drivers/gpu/drm/qxl/qxl_drv.h @@ -72,12 +72,13 @@ extern int qxl_max_ioctls; QXL_INTERRUPT_CLIENT_MONITORS_CONFIG) struct qxl_bo { + struct ttm_buffer_objecttbo; + /* Protected by gem.mutex */ struct list_headlist; /* Protected by tbo.reserved */ struct ttm_placeplacements[3]; struct ttm_placementplacement; - struct ttm_buffer_objecttbo; struct ttm_bo_kmap_obj kmap; unsigned int pin_count; void*kptr; @@ -85,7 +86,6 @@ struct qxl_bo { int type; /* Constant after initialization */ - struct drm_gem_object gem_base; unsigned int is_primary:1; /* is this now a primary surface */ unsigned int is_dumb:1; struct qxl_bo *shadow; @@ -94,7 +94,7 @@ struct qxl_bo { uint32_t surface_id; struct qxl_release *surf_create; }; -#define gem_to_qxl_bo(gobj) container_of((gobj), struct qxl_bo, gem_base) +#define gem_to_qxl_bo(gobj) container_of((gobj), struct qxl_bo, tbo.base) #define to_qxl_bo(tobj) container_of((tobj), struct qxl_bo, tbo) struct qxl_gem { diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h index 255b914e2a7b..b812d4ae9d0d 100644 --- a/drivers/gpu/drm/qxl/qxl_object.h +++ b/drivers/gpu/drm/qxl/qxl_object.h @@ -34,7 +34,7 @@ static inline int qxl_bo_reserve(struct qxl_bo *bo, bool no_wait) r = ttm_bo_reserve(&bo->tbo, true, no_wait, NULL); if (unlikely(r != 0)) { if (r != -ERESTARTSYS) { - struct drm_device *ddev = bo->gem_base.dev; + struct drm_device *ddev = bo->tbo.base.dev; dev_err(ddev->dev, "%p reserve failed\n", bo); } @@ -71,7 +71,7 @@ static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type, r = ttm_bo_reserve(&bo->tbo, true, no_wait, NULL); if (unlikely(r != 0)) { if (r != -ERESTARTSYS) { - struct drm_device *ddev = bo->gem_base.dev; + struct drm_device *ddev = bo->tbo.base.dev; dev_err(ddev->dev, "%p reserve failed for wait\n", bo); diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c index 0a2e51af1230..498000899bfd 100644 --- a/drivers/gpu/drm/qxl/qxl_cmd.c +++ b/drivers/gpu/drm/qxl/qxl_cmd.c @@ -375,7 +375,7 @@ void qxl_io_destroy_primary(struct qxl_device *qdev) { wait_for_io_cmd(qdev, 0, QXL_IO_DESTROY_PRIMARY_ASYNC); qdev->primary_bo->is_primary = false; - drm_gem_object_put_unlocked(&qdev->primary_bo->gem_base); + drm_gem_object_put_unlocked(&qdev->primary_bo->tbo.base); qdev->primary_bo = NULL; } @@ -402,7 +402,7 @@ void qxl_io_create_primary(struct qxl_device *qdev, struct qxl_bo *bo) wait_for_io_cmd(qdev, 0, QXL_IO_CREATE_PRIMARY_ASYNC); qdev->primary_bo = bo; qdev->primary_bo->is_primary = true; - drm_gem_object_get(&qdev->primary_bo->gem_base); + drm_gem_object_get(&qdev->primary_bo->tbo.base); } void qxl_io_memslot_add(struct qxl_device *qdev, uint8_t id) diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c b/drivers/gpu/drm/qxl/qxl_debugfs.c index 118422549828..013b938986c7 100644 --- a/drivers/gpu/drm/qxl/qxl_debugfs.c +++ b/drivers/gpu/drm/qxl/qxl_debugfs.c @@ -66,7 +66,7 @@ qxl_debugfs_buffers_info(struct seq_file *m, void *data) rcu_read_unlock(); seq_printf(m, "size %ld, pc %d, num releases %d\n", - (unsigned long)bo->gem_base.size, + (unsigned long)bo->tbo.base.size, bo->pin_count, rel); } return 0; diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 8b319ebbb0fb..93e31d062854 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -794,7 +794,7 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane, qdev->dumb_shadow_bo->surf.height != surf.height) {
[PATCH 2/6] drm/vram: use embedded gem object
Drop drm_gem_object from drm_gem_vram_object, use the ttm_buffer_object.base instead. Signed-off-by: Gerd Hoffmann --- include/drm/drm_gem_vram_helper.h | 3 +-- drivers/gpu/drm/drm_gem_vram_helper.c | 16 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h index 9581ea0a4f7e..7b9f50ba3fce 100644 --- a/include/drm/drm_gem_vram_helper.h +++ b/include/drm/drm_gem_vram_helper.h @@ -36,7 +36,6 @@ struct vm_area_struct; * video memory becomes scarce. */ struct drm_gem_vram_object { - struct drm_gem_object gem; struct ttm_buffer_object bo; struct ttm_bo_kmap_obj kmap; @@ -68,7 +67,7 @@ static inline struct drm_gem_vram_object *drm_gem_vram_of_bo( static inline struct drm_gem_vram_object *drm_gem_vram_of_gem( struct drm_gem_object *gem) { - return container_of(gem, struct drm_gem_vram_object, gem); + return container_of(gem, struct drm_gem_vram_object, bo.base); } struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 4de782ca26b2..61d9520cc15f 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -24,7 +24,7 @@ static void drm_gem_vram_cleanup(struct drm_gem_vram_object *gbo) * TTM buffer object in 'bo' has already been cleaned * up; only release the GEM object. */ - drm_gem_object_release(&gbo->gem); + drm_gem_object_release(&gbo->bo.base); } static void drm_gem_vram_destroy(struct drm_gem_vram_object *gbo) @@ -80,7 +80,7 @@ static int drm_gem_vram_init(struct drm_device *dev, int ret; size_t acc_size; - ret = drm_gem_object_init(dev, &gbo->gem, size); + ret = drm_gem_object_init(dev, &gbo->bo.base, size); if (ret) return ret; @@ -98,7 +98,7 @@ static int drm_gem_vram_init(struct drm_device *dev, return 0; err_drm_gem_object_release: - drm_gem_object_release(&gbo->gem); + drm_gem_object_release(&gbo->bo.base); return ret; } @@ -378,11 +378,11 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file, if (IS_ERR(gbo)) return PTR_ERR(gbo); - ret = drm_gem_handle_create(file, &gbo->gem, &handle); + ret = drm_gem_handle_create(file, &gbo->bo.base, &handle); if (ret) goto err_drm_gem_object_put_unlocked; - drm_gem_object_put_unlocked(&gbo->gem); + drm_gem_object_put_unlocked(&gbo->bo.base); args->pitch = pitch; args->size = size; @@ -391,7 +391,7 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file, return 0; err_drm_gem_object_put_unlocked: - drm_gem_object_put_unlocked(&gbo->gem); + drm_gem_object_put_unlocked(&gbo->bo.base); return ret; } EXPORT_SYMBOL(drm_gem_vram_fill_create_dumb); @@ -441,7 +441,7 @@ int drm_gem_vram_bo_driver_verify_access(struct ttm_buffer_object *bo, { struct drm_gem_vram_object *gbo = drm_gem_vram_of_bo(bo); - return drm_vma_node_verify_access(&gbo->gem.vma_node, + return drm_vma_node_verify_access(&gbo->bo.base.vma_node, filp->private_data); } EXPORT_SYMBOL(drm_gem_vram_bo_driver_verify_access); @@ -635,7 +635,7 @@ int drm_gem_vram_driver_gem_prime_mmap(struct drm_gem_object *gem, { struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem); - gbo->gem.vma_node.vm_node.start = gbo->bo.vma_node.vm_node.start; + gbo->bo.base.vma_node.vm_node.start = gbo->bo.vma_node.vm_node.start; return drm_gem_prime_mmap(gem, vma); } EXPORT_SYMBOL(drm_gem_vram_driver_gem_prime_mmap); -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/6] drm/ttm: add gem base object
Add drm_gem_object struct to ttm_buffer_object, so ttm objects are a gdm object superclass. Add a function to check whenever a given bo actually uses the embedded drm_gem_object, for the transition period. Signed-off-by: Gerd Hoffmann --- include/drm/ttm/ttm_bo_api.h | 18 ++ 1 file changed, 18 insertions(+) diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 49d9cdfc58f2..72f6aeda619c 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -31,6 +31,7 @@ #ifndef _TTM_BO_API_H_ #define _TTM_BO_API_H_ +#include #include #include #include @@ -127,6 +128,7 @@ struct ttm_tt; /** * struct ttm_buffer_object * + * @base: drm_gem_object superclass data. * @bdev: Pointer to the buffer object device structure. * @type: The bo type. * @destroy: Destruction function. If NULL, kfree is used. @@ -169,6 +171,8 @@ struct ttm_tt; */ struct ttm_buffer_object { + struct drm_gem_object base; + /** * Members constant at init. */ @@ -768,4 +772,18 @@ int ttm_bo_swapout(struct ttm_bo_global *glob, struct ttm_operation_ctx *ctx); void ttm_bo_swapout_all(struct ttm_bo_device *bdev); int ttm_bo_wait_unreserved(struct ttm_buffer_object *bo); + +/** + * ttm_bo_uses_embedded_gem_object - check if the given bo uses the + * embedded drm_gem_object. + * + * Helper for the transition period, once all drivers are switched to + * use the embedded drm_gem_object this can go away. + * + * @bo: The bo to check. + */ +static inline bool ttm_bo_uses_embedded_gem_object(struct ttm_buffer_object *bo) +{ + return bo->base.dev != NULL; +} #endif -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/6] drm/vram: drop drm_gem_vram_driver_gem_prime_mmap
The wrapper doesn't do anything any more, drop it. Signed-off-by: Gerd Hoffmann --- include/drm/drm_gem_vram_helper.h | 4 +--- drivers/gpu/drm/drm_gem_vram_helper.c | 17 - 2 files changed, 1 insertion(+), 20 deletions(-) diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h index 7b9f50ba3fce..2ada671a2a6b 100644 --- a/include/drm/drm_gem_vram_helper.h +++ b/include/drm/drm_gem_vram_helper.h @@ -137,8 +137,6 @@ void drm_gem_vram_driver_gem_prime_unpin(struct drm_gem_object *obj); void *drm_gem_vram_driver_gem_prime_vmap(struct drm_gem_object *obj); void drm_gem_vram_driver_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr); -int drm_gem_vram_driver_gem_prime_mmap(struct drm_gem_object *obj, - struct vm_area_struct *vma); #define DRM_GEM_VRAM_DRIVER_PRIME \ .gem_prime_export = drm_gem_prime_export, \ @@ -147,6 +145,6 @@ int drm_gem_vram_driver_gem_prime_mmap(struct drm_gem_object *obj, .gem_prime_unpin = drm_gem_vram_driver_gem_prime_unpin, \ .gem_prime_vmap = drm_gem_vram_driver_gem_prime_vmap, \ .gem_prime_vunmap = drm_gem_vram_driver_gem_prime_vunmap, \ - .gem_prime_mmap = drm_gem_vram_driver_gem_prime_mmap + .gem_prime_mmap = drm_gem_prime_mmap #endif diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 2e474dee30df..d78761802374 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -619,20 +619,3 @@ void drm_gem_vram_driver_gem_prime_vunmap(struct drm_gem_object *gem, drm_gem_vram_unpin(gbo); } EXPORT_SYMBOL(drm_gem_vram_driver_gem_prime_vunmap); - -/** - * drm_gem_vram_driver_gem_prime_mmap() - \ - Implements &struct drm_driver.gem_prime_mmap - * @gem: The GEM object to map - * @vma: The VMA describing the mapping - * - * Returns: - * 0 on success, or - * a negative errno code otherwise. - */ -int drm_gem_vram_driver_gem_prime_mmap(struct drm_gem_object *gem, - struct vm_area_struct *vma) -{ - return drm_gem_prime_mmap(gem, vma); -} -EXPORT_SYMBOL(drm_gem_vram_driver_gem_prime_mmap); -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/6] drm/ttm: make ttm bo a gem bo subclass
Gerd Hoffmann (6): drm/ttm: add gem base object drm/vram: use embedded gem object drm/qxl: use embedded gem object drm/ttm: use gem reservation object drm/ttm: use gem vma_node drm/vram: drop drm_gem_vram_driver_gem_prime_mmap drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +- drivers/gpu/drm/qxl/qxl_drv.h | 6 +-- drivers/gpu/drm/qxl/qxl_object.h | 6 +-- drivers/gpu/drm/radeon/radeon_object.h | 2 +- drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +- include/drm/drm_gem_vram_helper.h | 7 +--- include/drm/ttm/ttm_bo_api.h | 24 --- drivers/gpu/drm/drm_gem_vram_helper.c | 36 drivers/gpu/drm/qxl/qxl_cmd.c | 4 +- drivers/gpu/drm/qxl/qxl_debugfs.c | 2 +- drivers/gpu/drm/qxl/qxl_display.c | 8 ++-- drivers/gpu/drm/qxl/qxl_gem.c | 2 +- drivers/gpu/drm/qxl/qxl_object.c | 20 - drivers/gpu/drm/qxl/qxl_release.c | 2 +- drivers/gpu/drm/qxl/qxl_ttm.c | 4 +- drivers/gpu/drm/ttm/ttm_bo.c | 48 +- drivers/gpu/drm/ttm/ttm_bo_util.c | 4 +- drivers/gpu/drm/ttm/ttm_bo_vm.c| 9 ++-- drivers/gpu/drm/virtio/virtgpu_prime.c | 3 -- 19 files changed, 94 insertions(+), 97 deletions(-) -- 2.18.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/6] drm/ttm: use gem reservation object
Drop ttm_resv from ttm_buffer_object, use the gem reservation object (base._resv) instead. Signed-off-by: Gerd Hoffmann --- include/drm/ttm/ttm_bo_api.h | 1 - drivers/gpu/drm/ttm/ttm_bo.c | 40 ++- drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 72f6aeda619c..88aa7bf1b18a 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -235,7 +235,6 @@ struct ttm_buffer_object { struct sg_table *sg; struct reservation_object *resv; - struct reservation_object ttm_resv; struct mutex wu_mutex; }; diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index c7de667d482a..516eef3b76d5 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -160,7 +160,8 @@ static void ttm_bo_release_list(struct kref *list_kref) ttm_tt_destroy(bo->ttm); atomic_dec(&bo->bdev->glob->bo_count); dma_fence_put(bo->moving); - reservation_object_fini(&bo->ttm_resv); + if (!ttm_bo_uses_embedded_gem_object(bo)) + reservation_object_fini(&bo->base._resv); mutex_destroy(&bo->wu_mutex); bo->destroy(bo); ttm_mem_global_free(bdev->glob->mem_glob, acc_size); @@ -438,14 +439,14 @@ static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo) { int r; - if (bo->resv == &bo->ttm_resv) + if (bo->resv == &bo->base._resv) return 0; - BUG_ON(!reservation_object_trylock(&bo->ttm_resv)); + BUG_ON(!reservation_object_trylock(&bo->base._resv)); - r = reservation_object_copy_fences(&bo->ttm_resv, bo->resv); + r = reservation_object_copy_fences(&bo->base._resv, bo->resv); if (r) - reservation_object_unlock(&bo->ttm_resv); + reservation_object_unlock(&bo->base._resv); return r; } @@ -456,8 +457,8 @@ static void ttm_bo_flush_all_fences(struct ttm_buffer_object *bo) struct dma_fence *fence; int i; - fobj = reservation_object_get_list(&bo->ttm_resv); - fence = reservation_object_get_excl(&bo->ttm_resv); + fobj = reservation_object_get_list(&bo->base._resv); + fence = reservation_object_get_excl(&bo->base._resv); if (fence && !fence->ops->signaled) dma_fence_enable_sw_signaling(fence); @@ -490,11 +491,11 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo) spin_lock(&glob->lru_lock); ret = reservation_object_trylock(bo->resv) ? 0 : -EBUSY; if (!ret) { - if (reservation_object_test_signaled_rcu(&bo->ttm_resv, true)) { + if (reservation_object_test_signaled_rcu(&bo->base._resv, true)) { ttm_bo_del_from_lru(bo); spin_unlock(&glob->lru_lock); - if (bo->resv != &bo->ttm_resv) - reservation_object_unlock(&bo->ttm_resv); + if (bo->resv != &bo->base._resv) + reservation_object_unlock(&bo->base._resv); ttm_bo_cleanup_memtype_use(bo); reservation_object_unlock(bo->resv); @@ -515,8 +516,8 @@ static void ttm_bo_cleanup_refs_or_queue(struct ttm_buffer_object *bo) reservation_object_unlock(bo->resv); } - if (bo->resv != &bo->ttm_resv) - reservation_object_unlock(&bo->ttm_resv); + if (bo->resv != &bo->base._resv) + reservation_object_unlock(&bo->base._resv); error: kref_get(&bo->list_kref); @@ -551,7 +552,7 @@ static int ttm_bo_cleanup_refs(struct ttm_buffer_object *bo, if (unlikely(list_empty(&bo->ddestroy))) resv = bo->resv; else - resv = &bo->ttm_resv; + resv = &bo->base._resv; if (reservation_object_test_signaled_rcu(resv, true)) ret = 0; @@ -631,7 +632,7 @@ static bool ttm_bo_delayed_delete(struct ttm_bo_device *bdev, bool remove_all) kref_get(&bo->list_kref); list_move_tail(&bo->ddestroy, &removed); - if (remove_all || bo->resv != &bo->ttm_resv) { + if (remove_all || bo->resv != &bo->base._resv) { spin_unlock(&glob->lru_lock); reservation_object_lock(bo->resv, NULL); @@ -1332,9 +1333,16 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev, bo->resv = resv; reservation_object_assert_held(bo->resv); } else { - bo->resv = &bo->ttm_resv; + bo->resv = &bo->base._resv; + } + if (!ttm_bo_uses_embedded_gem_object(bo)) { + /* +* bo.gem is not initialized, so we have to setup the +* struct elemen
[PATCH 5/6] drm/ttm: use gem vma_node
Drop vma_node from ttm_buffer_object, use the gem struct (base.vma_node) instead. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 2 +- drivers/gpu/drm/qxl/qxl_object.h | 2 +- drivers/gpu/drm/radeon/radeon_object.h | 2 +- drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +- include/drm/ttm/ttm_bo_api.h | 5 + drivers/gpu/drm/drm_gem_vram_helper.c | 5 + drivers/gpu/drm/ttm/ttm_bo.c | 8 drivers/gpu/drm/ttm/ttm_bo_util.c | 2 +- drivers/gpu/drm/ttm/ttm_bo_vm.c| 9 + drivers/gpu/drm/virtio/virtgpu_prime.c | 3 --- 10 files changed, 16 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h index c430e8259038..002fd1450ec7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h @@ -192,7 +192,7 @@ static inline unsigned amdgpu_bo_gpu_page_alignment(struct amdgpu_bo *bo) */ static inline u64 amdgpu_bo_mmap_offset(struct amdgpu_bo *bo) { - return drm_vma_node_offset_addr(&bo->tbo.vma_node); + return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); } /** diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h index b812d4ae9d0d..8ae54ba7857c 100644 --- a/drivers/gpu/drm/qxl/qxl_object.h +++ b/drivers/gpu/drm/qxl/qxl_object.h @@ -60,7 +60,7 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo) static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo) { - return drm_vma_node_offset_addr(&bo->tbo.vma_node); + return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); } static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type, diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h index 9ffd8215d38a..e5554bf9140e 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -116,7 +116,7 @@ static inline unsigned radeon_bo_gpu_page_alignment(struct radeon_bo *bo) */ static inline u64 radeon_bo_mmap_offset(struct radeon_bo *bo) { - return drm_vma_node_offset_addr(&bo->tbo.vma_node); + return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); } extern int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type, diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 9e2d3062b01d..7146ba00fd5b 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -396,7 +396,7 @@ static inline void virtio_gpu_object_unref(struct virtio_gpu_object **bo) static inline u64 virtio_gpu_object_mmap_offset(struct virtio_gpu_object *bo) { - return drm_vma_node_offset_addr(&bo->tbo.vma_node); + return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); } static inline int virtio_gpu_object_reserve(struct virtio_gpu_object *bo, diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 88aa7bf1b18a..77bd420a147a 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -152,7 +152,6 @@ struct ttm_tt; * @ddestroy: List head for the delayed destroy list. * @swap: List head for swap LRU list. * @moving: Fence set when BO is moving - * @vma_node: Address space manager node. * @offset: The current GPU offset, which can have different meanings * depending on the memory type. For SYSTEM type memory, it should be 0. * @cur_placement: Hint of current placement. @@ -219,9 +218,7 @@ struct ttm_buffer_object { */ struct dma_fence *moving; - - struct drm_vma_offset_node vma_node; - + /* base.vma_node */ unsigned priority; /** diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index 61d9520cc15f..2e474dee30df 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -163,7 +163,7 @@ EXPORT_SYMBOL(drm_gem_vram_put); */ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo) { - return drm_vma_node_offset_addr(&gbo->bo.vma_node); + return drm_vma_node_offset_addr(&gbo->bo.base.vma_node); } EXPORT_SYMBOL(drm_gem_vram_mmap_offset); @@ -633,9 +633,6 @@ EXPORT_SYMBOL(drm_gem_vram_driver_gem_prime_vunmap); int drm_gem_vram_driver_gem_prime_mmap(struct drm_gem_object *gem, struct vm_area_struct *vma) { - struct drm_gem_vram_object *gbo = drm_gem_vram_of_gem(gem); - - gbo->bo.base.vma_node.vm_node.start = gbo->bo.vma_node.vm_node.start; return drm_gem_prime_mmap(gem, vma); } EXPORT_SYMBOL(drm_gem_vram_driver_gem_prime_mmap); diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 516eef3b76d5..1d91da85f399 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -672,7 +672,7 @@ static void ttm_bo_release(struct kref *kref) struct
Re: [RFC PATCH 1/4] dt-bindings: display: Convert common panel bindings to DT schema
On Wed, Jun 19, 2019 at 03:51:53PM -0600, Rob Herring wrote: > Convert the common panel bindings to DT schema consolidating scattered > definitions to a single schema file. > > The 'simple-panel' binding just a collection of properties and not a > complete binding itself. All of the 'simple-panel' properties are > covered by the panel-common.txt binding with the exception of the > 'no-hpd' property, so add that to the schema. > > As there are lots of references to simple-panel.txt, just keep the file > with a reference to panel-common.yaml for now until all the bindings are > converted. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [EXT] Re: [PATCH v2 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel
On Mi, 2019-06-19 at 10:21 -0300, Fabio Estevam wrote: > Caution: EXT Email > > Hi Robert, > > On Tue, Jun 18, 2019 at 10:33 AM Robert Chiras > wrote: > > > > > +Optional properties: > > +- reset-gpios: a GPIO spec for the RST_B GPIO pin > > +- pinctrl-0phandle to the pin settings for the reset > > pin > > +- width-mm:physical panel width [mm] > > +- height-mm: physical panel height [mm] > > +- display-timings: timings for the connected panel according > > to [1] > Still not convinced we need the 'display-timings' property, even as > an > optional property. My understanding is that passing display timings > in > the devicetree is not encouraged. > > Last time you said you need to pass ''display-timings' to workaround > the problem of connecting this panel to mx8m DCSS or eLCDIF. > > The panel timings come from the LCD manufacturer and it is agnostic > to > what display controller interface it is connected to. > > So I suggest making sure the timings passed in the driver are correct > as per the vendor datasheet. If they are correct and one specific > interface is not able to drive it, then probably it is a bug in this > specific display controller interface or in the SoC clock driver. I understand. I will remove the display-timings from dt and also from driver. Currently, this panel works in this form with both LCDIF and DCSS on our 8M SoC so, as you said, probably the issue we were seeing in our tree was coming from elsewhere.
Re: [RFC PATCH 2/4] dt-bindings: display: Convert ampire,am-480272h3tmqw-t01h panel to DT schema
On Wed, Jun 19, 2019 at 03:51:54PM -0600, Rob Herring wrote: > Convert the ampire,am-480272h3tmqw-t01h panel binding to DT schema. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 3/4] dt-bindings: display: Convert panel-lvds to DT schema
On Wed, Jun 19, 2019 at 03:51:55PM -0600, Rob Herring wrote: > Convert the panel-lvds binding to use DT schema. The panel-lvds schema > inherits from the panel-common.yaml schema and specific LVDS panel > bindings should inherit from this schema. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature
Re: [RFC PATCH 4/4] dt-bindings: display: Convert innolux, ee101ia-01 panel to DT schema
On Wed, Jun 19, 2019 at 03:51:56PM -0600, Rob Herring wrote: > Convert the innolux,ee101ia-01 LVDS panel binding to DT schema. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [EXT] Re: [PATCH v2 2/2] drm/panel: Add support for Raydium RM67191 panel driver
Hi Sam, On Mi, 2019-06-19 at 15:25 +0200, Sam Ravnborg wrote: > On Tue, Jun 18, 2019 at 04:30:46PM +0300, Robert Chiras wrote: > > > > This patch adds Raydium RM67191 TFT LCD panel driver (MIPI-DSI > > protocol). > > > > Signed-off-by: Robert Chiras > Please include in the changelog a list of what was updated - like > this: > > v2: > - added kconif symbol sorted (sam) > - another nitpick (foo) > - etc > > In general try to namme who gave feedback to give them some credit. Thanks. I will keep this in mind, next time > > Who is maintainer for this onwards? I can offer myself to maintain this. > > > > > --- > > drivers/gpu/drm/panel/Kconfig | 9 + > > drivers/gpu/drm/panel/Makefile| 1 + > > drivers/gpu/drm/panel/panel-raydium-rm67191.c | 709 > > ++ > > 3 files changed, 719 insertions(+) > > create mode 100644 drivers/gpu/drm/panel/panel-raydium-rm67191.c > > > > +static int rad_panel_prepare(struct drm_panel *panel) > > +{ > > + struct rad_panel *rad = to_rad_panel(panel); > > + > > + if (rad->prepared) > > + return 0; > > + > > + if (rad->reset) { > > + gpiod_set_value_cansleep(rad->reset, 1); > > + usleep_range(3000, 5000); > > + gpiod_set_value_cansleep(rad->reset, 0); > So writing a 0 will release reset. > > > > + usleep_range(18000, 2); > > + } > > + > > + rad->prepared = true; > > + > > + return 0; > > +} > > + > > +static int rad_panel_unprepare(struct drm_panel *panel) > > +{ > > + struct rad_panel *rad = to_rad_panel(panel); > > + > > + if (!rad->prepared) > > + return 0; > > + > > + if (rad->reset) { > > + gpiod_set_value_cansleep(rad->reset, 1); > > + usleep_range(15000, 17000); > > + gpiod_set_value_cansleep(rad->reset, 0); > Looks strange that reset is released in unprepare. > I would expect it to stay reset to minimize power etc. Yes, it looks strange indeed. The reason here is coming from the fact that this panel also has touch-screen that shares this reset pin with the OLED display. While the display may be turned off, the touch driver may need to keep the connection active with the touch controller from this panel. This is the reason for releasing the reset pin in unprepare. I will add this in a comment in the code, to eliminate the confusion. > > > > > + > > + ret = drm_display_info_set_bus_formats(&connector- > > >display_info, > > +rad_bus_formats, > > +ARRAY_SIZE(rad_bus_for > > mats)); > Other drivers has this as the last stement in their enable function. > I did not check why, but maybe something to invest a few minutes > into. > Be different only if there is a reason to do so. Ok, I will move this as the last statement. I also noticed that the other panel drivers do not check the return of this call, like I do here, so I will also remove this too. > > > > > + if (ret) > > + return ret; > > + > > + drm_mode_probed_add(panel->connector, mode); > > + > > + return 1; > > +} > > + > > + > > +#ifdef CONFIG_PM > > +static int rad_panel_suspend(struct device *dev) > > +{ > > + struct rad_panel *rad = dev_get_drvdata(dev); > > + > > + if (!rad->reset) > > + return 0; > > + > > + devm_gpiod_put(dev, rad->reset); > > + rad->reset = NULL; > > + > > + return 0; > > +} > > + > > +static int rad_panel_resume(struct device *dev) > > +{ > > + struct rad_panel *rad = dev_get_drvdata(dev); > > + > > + if (rad->reset) > > + return 0; > > + > > + rad->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); > > + if (IS_ERR(rad->reset)) > > + rad->reset = NULL; > > + > > + return PTR_ERR_OR_ZERO(rad->reset); > > +} > > + > > +#endif > Use __maybe_unused for the tw functions above. > And loose the ifdef... Thanks. Will apply. > > > > > + > > +static const struct dev_pm_ops rad_pm_ops = { > > + SET_RUNTIME_PM_OPS(rad_panel_suspend, rad_panel_resume, NULL) > > + SET_SYSTEM_SLEEP_PM_OPS(rad_panel_suspend, rad_panel_resume) > > +}; > > + > > +static const struct of_device_id rad_of_match[] = { > > + { .compatible = "raydium,rm67191", }, > > + { } > We often, but not always, write this as { /* sentinal */ }, Thanks. Will apply. > > > > > +}; > > +MODULE_DEVICE_TABLE(of, rad_of_match); > > + > > +static struct mipi_dsi_driver rad_panel_driver = { > > + .driver = { > > + .name = "panel-raydium-rm67191", > > + .of_match_table = rad_of_match, > > + .pm = &rad_pm_ops, > > + }, > > + .probe = rad_panel_probe, > > + .remove = rad_panel_remove, > > + .shutdown = rad_panel_shutdown, > > +}; > > +module_mipi_dsi_driver(rad_panel_driver); > > + > > +MODULE_AUTHOR("Robert Chiras "); > > +MODULE_DESCRIPTION("DRM Driver for Raydium RM67191
Re: [EXT] Re: [PATCH v2 2/2] drm/panel: Add support for Raydium RM67191 panel driver
On Mi, 2019-06-19 at 10:28 -0300, Fabio Estevam wrote: > Caution: EXT Email > > Hi Robert, > > On Tue, Jun 18, 2019 at 10:31 AM Robert Chiras > wrote: > > > > > +static const struct display_timing rad_default_timing = { > > + .pixelclock = { 6600, 13200, 13200 }, > > + .hactive = { 1080, 1080, 1080 }, > > + .hfront_porch = { 20, 20, 20 }, > > + .hsync_len = { 2, 2, 2 }, > > + .hback_porch = { 34, 34, 34 }, > > + .vactive = { 1920, 1920, 1920 }, > > + .vfront_porch = { 10, 10, 10 }, > > + .vsync_len = { 2, 2, 2 }, > > + .vback_porch = { 4, 4, 4 }, > Are you sure that the sync_len and porch parameters are the same for > both 66MHz and 132MHz? Probably they are not, since I didn't get this panel to work well with 66Hz. So I will just keep 132MHz, since these are the only timinings we received from vendor.
Re: [PATCH] drm/imx: correct order of crtc disable
On Wed, Jun 19, 2019 at 11:40 AM Philipp Zabel wrote: > > Hi Robert, > > thank you for the patch. > > On Tue, 2019-06-18 at 16:50 +0100, Robert Beckett wrote: > > Notify drm core before sending pending events during crtc disable. > > This fixes the first event after disable having an old stale timestamp > > by having drm_crtc_vblank_off update the timestamp to now. > > > > This was seen while debugging weston log message: > > Warning: computed repaint delay is insane: -8212 msec > > > > Would you say this > Fixes: a474478642d5 ("drm/imx: fix crtc vblank state regression") > ? > > > Signed-off-by: Robert Beckett > > --- > > drivers/gpu/drm/imx/ipuv3-crtc.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c > > b/drivers/gpu/drm/imx/ipuv3-crtc.c > > index 9cc1d678674f..c436a28d50e4 100644 > > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c > > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c > > @@ -91,14 +91,14 @@ static void ipu_crtc_atomic_disable(struct drm_crtc > > *crtc, > > ipu_dc_disable(ipu); > > ipu_prg_disable(ipu); > > > > + drm_crtc_vblank_off(crtc); > > + > > This is explained in the commit message and aligns with the > drm_crtc_state @event documentation. This part here looks fishy. The drm_vblank.c code is supposed to do the right thing, no matter where or when you ask it to generate an event. It definitely shouldn't generate a timestamp that's a few seconds too early. Bunch of options: - there's a bug in drm_vblank.c and it's mixing up something and generating a totally bogus value. - there's a lie in your imx vblank code, which trips the drm_vblank.c counter interpolation and results in a totally bogus value. drm_vblank.c assumes that if you do claim to have a hw counter and generate timestamps, that those are perfectly accurate. It only falls back to guestimating using the system timer if that's not present. Either way, this very much smells like papering over a bug if this change indeed fixes your wrong vblank timestamps. > > spin_lock_irq(&crtc->dev->event_lock); > > - if (crtc->state->event) { > > + if (crtc->state->event && !crtc->state->active) { > > This is not mentioned though. > > If the pending event is not sent here, I assume it will be picked up by > .atomic_flush and will then be sent after the first EOF interrupt after > the modeset is complete. Can you explain this in the commit message? Yeah looks correct (you only want to generate the event here when the crtc stays off), if it gets re-enabled the event should only be generated later on once that's all finished. But separate bugfix. -Daniel > > With that, > Reviewed-by: Philipp Zabel > > > drm_crtc_send_vblank_event(crtc, crtc->state->event); > > crtc->state->event = NULL; > > } > > spin_unlock_irq(&crtc->dev->event_lock); > > - > > - drm_crtc_vblank_off(crtc); > > } > > > > static void imx_drm_crtc_reset(struct drm_crtc *crtc) > > regards > Philipp > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/4] dt-bindings: display: Convert common panel bindings to DT schema
On Wed, Jun 19, 2019 at 03:51:53PM -0600, Rob Herring wrote: > Convert the common panel bindings to DT schema consolidating scattered > definitions to a single schema file. > > The 'simple-panel' binding just a collection of properties and not a > complete binding itself. All of the 'simple-panel' properties are > covered by the panel-common.txt binding with the exception of the > 'no-hpd' property, so add that to the schema. > > As there are lots of references to simple-panel.txt, just keep the file > with a reference to panel-common.yaml for now until all the bindings are > converted. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring > --- > Note there's still some references to panel-common.txt that I need to > update or just go ahead and convert to schema. > > .../bindings/display/panel/panel-common.txt | 101 - > .../bindings/display/panel/panel-common.yaml | 143 ++ > .../bindings/display/panel/panel.txt | 4 - > .../bindings/display/panel/simple-panel.txt | 29 +--- > 4 files changed, 144 insertions(+), 133 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/panel/panel-common.txt > create mode 100644 > Documentation/devicetree/bindings/display/panel/panel-common.yaml I know it was this way before, but perhaps remove the redundant panel- prefix while at it? > delete mode 100644 Documentation/devicetree/bindings/display/panel/panel.txt > > diff --git a/Documentation/devicetree/bindings/display/panel/panel-common.txt > b/Documentation/devicetree/bindings/display/panel/panel-common.txt > deleted file mode 100644 > index 5d2519af4bb5.. > --- a/Documentation/devicetree/bindings/display/panel/panel-common.txt > +++ /dev/null > @@ -1,101 +0,0 @@ > -Common Properties for Display Panel > -=== > - > -This document defines device tree properties common to several classes of > -display panels. It doesn't constitue a device tree binding specification by > -itself but is meant to be referenced by device tree bindings. > - > -When referenced from panel device tree bindings the properties defined in > this > -document are defined as follows. The panel device tree bindings are > -responsible for defining whether each property is required or optional. > - > - > -Descriptive Properties > --- > - > -- width-mm, > -- height-mm: The width-mm and height-mm specify the width and height of the > - physical area where images are displayed. These properties are expressed in > - millimeters and rounded to the closest unit. > - > -- label: The label property specifies a symbolic name for the panel as a > - string suitable for use by humans. It typically contains a name inscribed > on > - the system (e.g. as an affixed label) or specified in the system's > - documentation (e.g. in the user's manual). > - > - If no such name exists, and unless the property is mandatory according to > - device tree bindings, it shall rather be omitted than constructed of > - non-descriptive information. For instance an LCD panel in a system that > - contains a single panel shall not be labelled "LCD" if that name is not > - inscribed on the system or used in a descriptive fashion in system > - documentation. > - > - > -Display Timings > > - > -- panel-timing: Most display panels are restricted to a single resolution and > - require specific display timings. The panel-timing subnode expresses those > - timings as specified in the timing subnode section of the display timing > - bindings defined in > - Documentation/devicetree/bindings/display/panel/display-timing.txt. > - > - > -Connectivity > - > - > -- ports: Panels receive video data through one or multiple connections. While > - the nature of those connections is specific to the panel type, the > - connectivity is expressed in a standard fashion using ports as specified in > - the device graph bindings defined in > - Documentation/devicetree/bindings/graph.txt. > - > -- ddc-i2c-bus: Some panels expose EDID information through an I2C-compatible > - bus such as DDC2 or E-DDC. For such panels the ddc-i2c-bus contains a > - phandle to the system I2C controller connected to that bus. > - > - > -Control I/Os > - > - > -Many display panels can be controlled through pins driven by GPIOs. The > nature > -and timing of those control signals are device-specific and left for panel > -device tree bindings to specify. The following GPIO specifiers can however be > -used for panels that implement compatible control signals. > - > -- enable-gpios: Specifier for a GPIO connected to the panel enable control > - signal. The enable signal is active high and enables operation of the > panel. > - This property can also be used for panels implementing an active low power > - down signal, which is
Re: [RFC PATCH 2/4] dt-bindings: display: Convert ampire,am-480272h3tmqw-t01h panel to DT schema
On Wed, Jun 19, 2019 at 03:51:54PM -0600, Rob Herring wrote: > Convert the ampire,am-480272h3tmqw-t01h panel binding to DT schema. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring > --- > .../panel/ampire,am-480272h3tmqw-t01h.txt | 26 > .../panel/ampire,am-480272h3tmqw-t01h.yaml| 41 +++ > 2 files changed, 41 insertions(+), 26 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/panel/ampire,am-480272h3tmqw-t01h.txt > create mode 100644 > Documentation/devicetree/bindings/display/panel/ampire,am-480272h3tmqw-t01h.yaml Acked-by: Thierry Reding signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 3/4] dt-bindings: display: Convert panel-lvds to DT schema
On Wed, Jun 19, 2019 at 03:51:55PM -0600, Rob Herring wrote: > Convert the panel-lvds binding to use DT schema. The panel-lvds schema > inherits from the panel-common.yaml schema and specific LVDS panel > bindings should inherit from this schema. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring > --- > .../bindings/display/panel/panel-lvds.txt | 121 -- > .../bindings/display/panel/panel-lvds.yaml| 107 > 2 files changed, 107 insertions(+), 121 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/panel/panel-lvds.txt > create mode 100644 > Documentation/devicetree/bindings/display/panel/panel-lvds.yaml Similar to my comments on panel-common.yaml, perhaps make this just lvds.yaml? Otherwise: Acked-by: Thierry Reding signature.asc Description: PGP signature
Re: [RFC PATCH 4/4] dt-bindings: display: Convert innolux,ee101ia-01 panel to DT schema
On Wed, Jun 19, 2019 at 03:51:56PM -0600, Rob Herring wrote: > Convert the innolux,ee101ia-01 LVDS panel binding to DT schema. > > Cc: Thierry Reding > Cc: Sam Ravnborg > Cc: Maxime Ripard > Cc: Laurent Pinchart > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Rob Herring > --- > .../display/panel/innolux,ee101ia-01d.txt | 7 --- > .../display/panel/innolux,ee101ia-01d.yaml| 21 +++ > 2 files changed, 21 insertions(+), 7 deletions(-) > delete mode 100644 > Documentation/devicetree/bindings/display/panel/innolux,ee101ia-01d.txt > create mode 100644 > Documentation/devicetree/bindings/display/panel/innolux,ee101ia-01d.yaml Acked-by: Thierry Reding signature.asc Description: PGP signature
Re: [PATCH 1/7] video: add HDMI state notifier support
On Wed, Jun 19, 2019 at 07:48:11PM +0800, Cheng-yi Chiang wrote: > On Tue, Jun 18, 2019 at 8:12 PM Daniel Vetter wrote: > > > > On Tue, Jun 18, 2019 at 07:48:06PM +0800, Cheng-yi Chiang wrote: > > > On Tue, Jun 11, 2019 at 8:35 PM Daniel Vetter wrote: > > > > > > > > On Tue, Jun 11, 2019 at 08:10:38PM +0800, Cheng-yi Chiang wrote: > > > > > On Tue, Jun 4, 2019 at 3:24 PM Daniel Vetter wrote: > > > > > > > > > > > > On Tue, Jun 04, 2019 at 10:32:50AM +0800, Cheng-yi Chiang wrote: > > > > > > > On Mon, Jun 3, 2019 at 4:09 PM Daniel Vetter > > > > > > > wrote: > > > > > > > > > > > > > > > > On Mon, Jun 03, 2019 at 09:45:49AM +0200, Hans Verkuil wrote: > > > > > > > > > On 6/3/19 6:32 AM, Cheng-Yi Chiang wrote: > > > > > > > > > > From: Hans Verkuil > > > > > > > > > > > > > > > > > > > > Add support for HDMI hotplug and EDID notifiers, which is > > > > > > > > > > used to convey > > > > > > > > > > information from HDMI drivers to their CEC and audio > > > > > > > > > > counterparts. > > > > > > > > > > > > > > > > > > > > Based on an earlier version from Russell King: > > > > > > > > > > > > > > > > > > > > https://patchwork.kernel.org/patch/9277043/ > > > > > > > > > > > > > > > > > > > > The hdmi_notifier is a reference counted object containing > > > > > > > > > > the HDMI state > > > > > > > > > > of an HDMI device. > > > > > > > > > > > > > > > > > > > > When a new notifier is registered the current state will be > > > > > > > > > > reported to > > > > > > > > > > that notifier at registration time. > > > > > > > > > > > > > > > > > > > > Based on Hans Verkuil's patch: > > > > > > > > > > > > > > > > > > > > https://patchwork.kernel.org/patch/9472521/ > > > > > > > > > > > > > > > > > > Erm, you are aware that this patch morphed into a > > > > > > > > > CEC-specific notifier > > > > > > > > > found in drivers/media/cec/cec-notifier.c? > > > > > > > > > > > > > > > > > > I don't think it makes sense to have two notifier > > > > > > > > > implementations in the kernel. > > > > > > > > > The original intention was to have the notifier deal with > > > > > > > > > both CEC and ASoC > > > > > > > > > notifications, but there was not enough interest for the ASoC > > > > > > > > > bits at the time > > > > > > > > > and it was dropped. > > > > > > > > > > > > > > > > > > I am planning changes to the cec-notifier API, I hope to work > > > > > > > > > on that this > > > > > > > > > week. I'll CC you when I post those. Those might be a good > > > > > > > > > starting point > > > > > > > > > to convert the cec-notifier to an hdmi-notifier as was > > > > > > > > > originally intended. > > > > > > > > > > > > > > > > > > I've added your colleague Dariusz Marcinkiewicz to the CC > > > > > > > > > list since he's been > > > > > > > > > working on some nice cec-notifier improvements as well. > > > > > > > > > > > > > > > > We also have some interfaces for drm/alsa interactions around > > > > > > > > hdmi > > > > > > > > already in drm/drm_audio_component.h, but it's not used by > > > > > > > > anything > > > > > > > > outside of i915. Imo we should extend that, not reinvent a new > > > > > > > > wheel. > > > > > > > > > > > > > > > Hi Daniel, > > > > > > > Thank you for the pointer. Looking at the ops, it seems that it is > > > > > > > specific to HDA. > > > > > > > I am not familiar with drm and HDA. I am not sure how applicable > > > > > > > it > > > > > > > would be to report jack status to ASoC. > > > > > > > There is a use case in sound/soc/codecs/hdac_hdmi.c though so it > > > > > > > should be possible. > > > > > > > > > > > > Currently hda is the only user, but the idea was to make it more > > > > > > generic. > > > > > > Jack status in alsa is what drm calls connector status btw. > > > > > > > > > > > > So if we can take that as a baseline and extend it (probably needs > > > > > > some > > > > > > registration boilerplate and helpers to look up the right endpoint > > > > > > using > > > > > > of/dt for soc systems, we use component.c in i915/hda for this), > > > > > > that > > > > > > would be great I think. > > > > > > > > > > > > > > Another note: notifiers considered evil, imo. Gets the job done > > > > > > > > for one > > > > > > > > case, as soon as you have multiple devices and need to make > > > > > > > > sure you get > > > > > > > > the update for the right one it all comes crashing down. Please > > > > > > > > create an > > > > > > > > api which registers for updates from a specific device only, > > > > > > > > plus > > > > > > > > something that has real callbacks (like the > > > > > > > > drm_audio_component.h thing we > > > > > > > > started already). > > > > > > > > > > > > > > To clarify a bit, this hdmi-notifier indeed supports updating > > > > > > > from a > > > > > > > specific device only. > > > > > > > hdmi_notifier_get takes a device and return the notifier. > > > > > > > > > > > > Hm I missed that, I thought it's global, so one of my usual notifier > > > > >
[GIT PULL] mali-dp and komeda patches for drm-next
Hi DRM maintainers, Picking up pace on the upstreaming of Komeda driver, with quite a lot of new features added this time. On top of that we have the small cleanups and improved usage of the debugfs functions. Please pull! Best regards, Liviu The following changes since commit 52d2d44eee8091e740d0d275df1311fb8373c9a9: Merge v5.2-rc5 into drm-next (2019-06-19 12:07:29 +0200) are available in the Git repository at: git://linux-arm.org/linux-ld.git for-upstream/mali-dp for you to fetch changes up to 344f00e4d7d6538c1862505b25b662b47c9e0bb0: drm/komeda: Make Komeda interrupts shareable (2019-06-19 17:04:21 +0100) Arnd Bergmann (1): drm/komeda: fix 32-bit komeda_crtc_update_clock_ratio Ayan Halder (1): drm/komeda: Make Komeda interrupts shareable Greg Kroah-Hartman (2): komeda: no need to check return value of debugfs_create functions malidp: no need to check return value of debugfs_create functions Liviu Dudau (1): arm/komeda: Convert dp_wait_cond() to return an error code. Lowry Li (Arm Technology China) (10): drm/komeda: Creates plane alpha and blend mode properties drm/komeda: Clear enable bit in CU_INPUTx_CONTROL drm/komeda: Add rotation support on Komeda driver drm/komeda: Adds limitation check for AFBC wide block not support Rot90 drm/komeda: Update HW up-sampling on D71 drm/komeda: Enable color-encoding (YUV format) support drm/komeda: Adds SMMU support dt/bindings: drm/komeda: Adds SMMU support for D71 devicetree drm/komeda: Adds zorder support drm/komeda: Add slave pipeline support james qian wang (Arm Technology China) (21): drm/komeda: Add writeback support drm/komeda: Added AFBC support for komeda driver drm/komeda: Attach scaler to drm as private object drm/komeda: Add the initial scaler support for CORE drm/komeda: Implement D71 scaler support drm/komeda: Add writeback scaling support drm/komeda: Add engine clock requirement check for the downscaling drm/komeda: Add image enhancement support drm/komeda: Add komeda_fb_check_src_coords drm/komeda: Add format support for Y0L2, P010, YUV420_8/10BIT drm/komeda: Unify mclk/pclk/pipeline->aclk to one MCLK drm/komeda: Rename main engine clk name "mclk" to "aclk" dt/bindings: drm/komeda: Unify mclk/pclk/pipeline->aclk to one ACLK drm/komeda: Add component komeda_merger drm/komeda: Add split support for scaler drm/komeda: Add layer split support drm/komeda: Refine function to_d71_input_id drm/komeda: Accept null writeback configurations for writeback drm/komeda: Add new component komeda_splitter drm/komeda: Enable writeback split support drm/komeda: Correct printk format specifier for "size_t" .../devicetree/bindings/display/arm,komeda.txt | 23 +- drivers/gpu/drm/arm/display/include/malidp_io.h| 7 + drivers/gpu/drm/arm/display/include/malidp_utils.h | 5 +- drivers/gpu/drm/arm/display/komeda/Makefile| 2 + .../gpu/drm/arm/display/komeda/d71/d71_component.c | 582 +- drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c | 142 +++-- drivers/gpu/drm/arm/display/komeda/d71/d71_dev.h | 2 + .../gpu/drm/arm/display/komeda/komeda_color_mgmt.c | 67 ++ .../gpu/drm/arm/display/komeda/komeda_color_mgmt.h | 17 + drivers/gpu/drm/arm/display/komeda/komeda_crtc.c | 154 - drivers/gpu/drm/arm/display/komeda/komeda_dev.c| 59 +- drivers/gpu/drm/arm/display/komeda/komeda_dev.h| 13 +- .../drm/arm/display/komeda/komeda_format_caps.c| 58 ++ .../drm/arm/display/komeda/komeda_format_caps.h| 24 +- .../drm/arm/display/komeda/komeda_framebuffer.c| 175 +- .../drm/arm/display/komeda/komeda_framebuffer.h| 13 +- drivers/gpu/drm/arm/display/komeda/komeda_kms.c| 130 +++- drivers/gpu/drm/arm/display/komeda/komeda_kms.h| 71 ++- .../gpu/drm/arm/display/komeda/komeda_pipeline.c | 66 +- .../gpu/drm/arm/display/komeda/komeda_pipeline.h | 111 +++- .../drm/arm/display/komeda/komeda_pipeline_state.c | 679 - drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 191 +- .../drm/arm/display/komeda/komeda_private_obj.c| 154 + .../drm/arm/display/komeda/komeda_wb_connector.c | 199 ++ drivers/gpu/drm/arm/malidp_drv.c | 11 +- 25 files changed, 2728 insertions(+), 227 deletions(-) create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h create mode 100644 drivers/gpu/drm/arm/display/komeda/komeda_wb_connector.c -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ ___ dri-devel mailing list dri-devel@li
[Bug 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #23 from tempel.jul...@gmail.com --- Any news on this? I'd really like to have this sorted out before I wholeheartedly recommended Navi for Linux gaming. I can imagine that Navi causes a ton of work, but still this issue is painful. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/7] Hexdump Enhancements
On Wed, 19 Jun 2019, Joe Perches wrote: > On Thu, 2019-06-20 at 11:14 +1000, Alastair D'Silva wrote: >> On Wed, 2019-06-19 at 17:35 -0700, Joe Perches wrote: >> > On Thu, 2019-06-20 at 09:15 +1000, Alastair D'Silva wrote: >> > > On Wed, 2019-06-19 at 09:31 -0700, Joe Perches wrote: >> > > > On Mon, 2019-06-17 at 12:04 +1000, Alastair D'Silva wrote: >> > > > > From: Alastair D'Silva >> > > > > >> > > > > Apologies for the large CC list, it's a heads up for those >> > > > > responsible >> > > > > for subsystems where a prototype change in generic code causes >> > > > > a >> > > > > change >> > > > > in those subsystems. >> > > > > >> > > > > This series enhances hexdump. >> > > > >> > > > Still not a fan of these patches. >> > > >> > > I'm afraid there's not too much action I can take on that, I'm >> > > happy to >> > > address specific issues though. >> > > >> > > > > These improve the readability of the dumped data in certain >> > > > > situations >> > > > > (eg. wide terminals are available, many lines of empty bytes >> > > > > exist, >> > > > > etc). >> > >> > I think it's generally overkill for the desired uses. >> >> I understand where you're coming from, however, these patches make it a >> lot easier to work with large chucks of binary data. I think it makes >> more sense to have these patches upstream, even though committed code >> may not necessarily have all the features enabled, as it means that >> devs won't have to apply out-of-tree patches during development to make >> larger dumps manageable. >> >> > > > Changing hexdump's last argument from bool to int is odd. >> > > > >> > > >> > > Think of it as replacing a single boolean with many booleans. >> > >> > I understand it. It's odd. >> > >> > I would rather not have a mixture of true, false, and apparently >> > random collections of bitfields like 0xd or 0b1011 or their >> > equivalent or'd defines. >> > >> >> Where's the mixture? What would you propose instead? > > create a hex_dump_to_buffer_ext with a new argument > and a new static inline for the old hex_dump_to_buffer > without modifying the argument list that calls > hex_dump_to_buffer with whatever added argument content > you need. > > Something like: > > static inline > int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, > int groupsize, char *linebuf, size_t linebuflen, > bool ascii) > { > return hex_dump_to_buffer_ext(buf, len, rowsize, groupsize, > linebuf, linebuflen, ascii, 0); > } > > and remove EXPORT_SYMBOL(hex_dump_to_buffer) If you decide to do something like this, I'd actually suggest you drop the bool ascii parameter from hex_dump_to_buffer() altogether, and replace the callers that do require ascii with hex_dump_to_buffer_ext(..., HEXDUMP_ASCII). Even if that also requires touching all callers. But no strong opinions, really. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110635] briefly flashing corruption when playing various OGL games
https://bugs.freedesktop.org/show_bug.cgi?id=110635 --- Comment #9 from tempel.jul...@gmail.com --- *bump* Situation unchanged with recent llvm-git and mesa-git. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: use exact allocation for dma coherent memory
On Wed, Jun 19, 2019 at 01:29:03PM -0300, Jason Gunthorpe wrote: > > Yes. This will blow up badly on many platforms, as sq->queue > > might be vmapped, ioremapped, come from a pool without page backing. > > Gah, this addr gets fed into io_remap_pfn_range/remap_pfn_range too.. > > Potnuri, you should fix this.. > > You probably need to use dma_mmap_from_dev_coherent() in the mmap ? The function to use is dma_mmap_coherent, dma_mmap_from_dev_coherent is just an internal helper. That beiŋ said the drivers/infiniband code has a lot of *remap_pfn_range, and a lot of them look like they might be for DMA memory.
Re: [PATCH] drm/imx: correct order of crtc disable
On Thu, 2019-06-20 at 10:50 +0200, Daniel Vetter wrote: > On Wed, Jun 19, 2019 at 11:40 AM Philipp Zabel < > p.za...@pengutronix.de> wrote: > > > > Hi Robert, > > > > thank you for the patch. > > > > On Tue, 2019-06-18 at 16:50 +0100, Robert Beckett wrote: > > > Notify drm core before sending pending events during crtc > > > disable. > > > This fixes the first event after disable having an old stale > > > timestamp > > > by having drm_crtc_vblank_off update the timestamp to now. > > > > > > This was seen while debugging weston log message: > > > Warning: computed repaint delay is insane: -8212 msec > > > > > > > Would you say this > > Fixes: a474478642d5 ("drm/imx: fix crtc vblank state regression") > > ? > > > > > Signed-off-by: Robert Beckett > > > --- > > > drivers/gpu/drm/imx/ipuv3-crtc.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c > > > b/drivers/gpu/drm/imx/ipuv3-crtc.c > > > index 9cc1d678674f..c436a28d50e4 100644 > > > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c > > > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c > > > @@ -91,14 +91,14 @@ static void ipu_crtc_atomic_disable(struct > > > drm_crtc *crtc, > > > ipu_dc_disable(ipu); > > > ipu_prg_disable(ipu); > > > > > > + drm_crtc_vblank_off(crtc); > > > + > > > > This is explained in the commit message and aligns with the > > drm_crtc_state @event documentation. > > This part here looks fishy. The drm_vblank.c code is supposed to do > the right thing, no matter where or when you ask it to generate an > event. It definitely shouldn't generate a timestamp that's a few > seconds too early. Bunch of options: > - there's a bug in drm_vblank.c and it's mixing up something and > generating a totally bogus value. > - there's a lie in your imx vblank code, which trips the drm_vblank.c > counter interpolation and results in a totally bogus value. > > drm_vblank.c assumes that if you do claim to have a hw counter and > generate timestamps, that those are perfectly accurate. It only falls > back to guestimating using the system timer if that's not present. > > Either way, this very much smells like papering over a bug if this > change indeed fixes your wrong vblank timestamps. > A quick explaination of where the dodgy timestamp came from: 1. driver starts up 2. fbcon comes along and restores fbdev, enabling vblank 3. vblank_disable_fn fires via timer disabling vblank, keeping vblank seq number and time set at current value (some time later) 4. weston starts and does a modeset 5. atomic commit disables crtc while it does the modeset 6. ipu_crtc_atomic_disable sends vblank with old seq number and time It turns out the actual fix for the old vblank is the next change, which stops it being sent at all during the crtc disable as it is is still active, it would then go through drm_crtc_vblank_off, reseting the timestamp, and get delivered during the vblank enable as part of the atomic commit. So, in theory, we could just have the following change to fix the specific issue of a stale timestamp. However, given the documentation for the event in include/drm/drm_crtc.h: * - The event is for a CRTC which is being disabled through this *atomic commit. In that case the event can be send out any time *after the hardware has stopped scanning out the current *framebuffers. It should contain the timestamp and counter for the *last vblank before the display pipeline was shut off. The simplest *way to achieve that is calling drm_crtc_send_vblank_event() *somewhen after drm_crtc_vblank_off() has been called. This still seems like a sensible change for when the crtc is being disabled. > > > spin_lock_irq(&crtc->dev->event_lock); > > > - if (crtc->state->event) > > > { > > > + if (crtc->state->event && !crtc->state->active) { > > > > This is not mentioned though. > > > > If the pending event is not sent here, I assume it will be picked > > up by > > .atomic_flush and will then be sent after the first EOF interrupt > > after > > the modeset is complete. Can you explain this in the commit > > message? > > Yeah looks correct (you only want to generate the event here when the > crtc stays off), if it gets re-enabled the event should only be > generated later on once that's all finished. But separate bugfix. > -Daniel > It looks like this is actually the fix needed to avoid the bogus timestamp. I can split this patch up in to 2 commits if desired? > > > > With that, > > Reviewed-by: Philipp Zabel > > > > > drm_crtc_send_vblank_event(crtc, crtc->state- > > > >event); > > > crtc->state->event = NULL; > > > } > > > spin_unlock_irq(&crtc->dev->event_lock); > > > - > > > - drm_crtc_vblank_off(crtc); > > > } > > > > > > static void imx_drm_crtc_reset(struct drm_crtc *crtc) > > > > regards > > Philipp > > _
Re: [PATCH] drm/self_refresh: Fix possible NULL deref in failure path
On Wed, Jun 19, 2019 at 02:19:47PM -0400, Sean Paul wrote: > From: Sean Paul > > If state allocation fails, we still try to give back the reference on > it. Also initialize ret in case the crtc is not enabled and we hit the > eject button. > > Fixes: 1452c25b0e60 ("drm: Add helpers to kick off self refresh mode in > drivers") > Cc: Daniel Vetter > Cc: Jose Souza > Cc: Zain Wang > Cc: Tomasz Figa > Cc: Ville Syrjälä > Cc: Sam Ravnborg > Cc: Sean Paul > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: dri-devel@lists.freedesktop.org > Reported-by: Dan Carpenter > Signed-off-by: Sean Paul Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/drm_self_refresh_helper.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_self_refresh_helper.c > b/drivers/gpu/drm/drm_self_refresh_helper.c > index e0d2ad1f070cb..4b9424a8f1f1c 100644 > --- a/drivers/gpu/drm/drm_self_refresh_helper.c > +++ b/drivers/gpu/drm/drm_self_refresh_helper.c > @@ -69,14 +69,14 @@ static void drm_self_refresh_helper_entry_work(struct > work_struct *work) > struct drm_connector *conn; > struct drm_connector_state *conn_state; > struct drm_crtc_state *crtc_state; > - int i, ret; > + int i, ret = 0; > > drm_modeset_acquire_init(&ctx, 0); > > state = drm_atomic_state_alloc(dev); > if (!state) { > ret = -ENOMEM; > - goto out; > + goto out_drop_locks; > } > > retry: > @@ -116,6 +116,8 @@ static void drm_self_refresh_helper_entry_work(struct > work_struct *work) > } > > drm_atomic_state_put(state); > + > +out_drop_locks: > drm_modeset_drop_locks(&ctx); > drm_modeset_acquire_fini(&ctx); > } > -- > Sean Paul, Software Engineer, Google / Chromium OS > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/imx: correct order of crtc disable
Hi Robert, Daniel, On Thu, 2019-06-20 at 12:12 +0100, Robert Beckett wrote: > On Thu, 2019-06-20 at 10:50 +0200, Daniel Vetter wrote: > > On Wed, Jun 19, 2019 at 11:40 AM Philipp Zabel < > > p.za...@pengutronix.de> wrote: > > > > > > Hi Robert, > > > > > > thank you for the patch. > > > > > > On Tue, 2019-06-18 at 16:50 +0100, Robert Beckett wrote: > > > > Notify drm core before sending pending events during crtc > > > > disable. > > > > This fixes the first event after disable having an old stale > > > > timestamp > > > > by having drm_crtc_vblank_off update the timestamp to now. > > > > > > > > This was seen while debugging weston log message: > > > > Warning: computed repaint delay is insane: -8212 msec > > > > > > > > > > Would you say this > > > Fixes: a474478642d5 ("drm/imx: fix crtc vblank state regression") > > > ? > > > > > > > Signed-off-by: Robert Beckett > > > > --- > > > > drivers/gpu/drm/imx/ipuv3-crtc.c | 6 +++--- > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > b/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > index 9cc1d678674f..c436a28d50e4 100644 > > > > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > @@ -91,14 +91,14 @@ static void ipu_crtc_atomic_disable(struct > > > > drm_crtc *crtc, > > > > ipu_dc_disable(ipu); > > > > ipu_prg_disable(ipu); > > > > > > > > + drm_crtc_vblank_off(crtc); > > > > + > > > > > > This is explained in the commit message and aligns with the > > > drm_crtc_state @event documentation. > > > > This part here looks fishy. The drm_vblank.c code is supposed to do > > the right thing, no matter where or when you ask it to generate an > > event. It definitely shouldn't generate a timestamp that's a few > > seconds too early. Bunch of options: > > - there's a bug in drm_vblank.c and it's mixing up something and > > generating a totally bogus value. > > - there's a lie in your imx vblank code, which trips the drm_vblank.c > > counter interpolation and results in a totally bogus value. > > > > drm_vblank.c assumes that if you do claim to have a hw counter and > > generate timestamps, that those are perfectly accurate. It only falls > > back to guestimating using the system timer if that's not present. > > > > Either way, this very much smells like papering over a bug if this > > change indeed fixes your wrong vblank timestamps. Thank you for chiming in, I can confirm that just moving the drm_crtc_vblank_off around does not change anything. I'll drop this patch and wait for v3. > A quick explaination of where the dodgy timestamp came from: > 1. driver starts up > 2. fbcon comes along and restores fbdev, enabling vblank > 3. vblank_disable_fn fires via timer disabling vblank, keeping vblank > seq number and time set at current value > (some time later) > 4. weston starts and does a modeset > 5. atomic commit disables crtc while it does the modeset > 6. ipu_crtc_atomic_disable sends vblank with old seq number and time It would be great to have this in the commit message for context. > It turns out the actual fix for the old vblank is the next change, > which stops it being sent at all during the crtc disable as it is is > still active, it would then go through drm_crtc_vblank_off, reseting > the timestamp, and get delivered during the vblank enable as part of > the atomic commit. Which means this patch isn't "Fixes: a474478642d5" after all. The offending code has been there since commit 5f2f911578fb ("drm/imx: atomic phase 3 step 1: Use atomic configuration"). > So, in theory, we could just have the following change to fix the > specific issue of a stale timestamp. > > However, given the documentation for the event in > include/drm/drm_crtc.h: > > * - The event is for a CRTC which is being disabled through this > *atomic commit. In that case the event can be send out any time > *after the hardware has stopped scanning out the current > *framebuffers. It should contain the timestamp and counter for > the > *last vblank before the display pipeline was shut off. The > simplest > *way to achieve that is calling drm_crtc_send_vblank_event() > *somewhen after drm_crtc_vblank_off() has been called. > > This still seems like a sensible change for when the crtc is being > disabled. This is what had me confused as well. It doesn't really say, but it seems to imply that drm_crtc_send_vblank_event must only be sent after drm_crtc_vblank_off. If this is not a hard requirement, maybe this could be mentioned here? > > > > spin_lock_irq(&crtc->dev->event_lock); > > > > - if (crtc->state->event) > > > > { > > > > + if (crtc->state->event && !crtc->state->active) { > > > > > > This is not mentioned though. > > > > > > If the pending event is not sent here, I assume it will be picked > > > up by > > > .atomic_flush and will then b
[PULL] drm-intel-fixes
Hi Dave & Daniel - drm-intel-fixes-2019-06-20: drm/i915 fixes for v5.2-rc6: - GVT: Fix reserved PVINFO register write (Weinan) - Avoid clobbering M/N values in fastset fuzzy checks (Ville) BR, Jani. The following changes since commit 9e0babf2c06c73cda2c0cd37a1653d823adb40ec: Linux 5.2-rc5 (2019-06-16 08:49:45 -1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2019-06-20 for you to fetch changes up to 475df5d0f3eb2d031e4505f84d8fba75baaf2e80: drm/i915: Don't clobber M/N values during fastset check (2019-06-19 15:57:09 +0300) drm/i915 fixes for v5.2-rc6: - GVT: Fix reserved PVINFO register write (Weinan) - Avoid clobbering M/N values in fastset fuzzy checks (Ville) Jani Nikula (1): Merge tag 'gvt-fixes-2019-06-19' of https://github.com/intel/gvt-linux into drm-intel-fixes Ville Syrjälä (1): drm/i915: Don't clobber M/N values during fastset check Weinan Li (1): drm/i915/gvt: ignore unexpected pvinfo write drivers/gpu/drm/i915/gvt/handlers.c | 15 -- drivers/gpu/drm/i915/intel_display.c | 38 +++- 2 files changed, 38 insertions(+), 15 deletions(-) -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110949] Continuious warnings from agd5f 5.3-wip branch
https://bugs.freedesktop.org/show_bug.cgi?id=110949 --- Comment #3 from Nicholas Kazlauskas --- Seems like there's still issues with dropping the check depending on the ASIC revision and probably userspace that's being used. I can revert this for now while investigating the issue. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/imx: correct order of crtc disable
On Thu, Jun 20, 2019 at 12:12:13PM +0100, Robert Beckett wrote: > On Thu, 2019-06-20 at 10:50 +0200, Daniel Vetter wrote: > > On Wed, Jun 19, 2019 at 11:40 AM Philipp Zabel < > > p.za...@pengutronix.de> wrote: > > > > > > Hi Robert, > > > > > > thank you for the patch. > > > > > > On Tue, 2019-06-18 at 16:50 +0100, Robert Beckett wrote: > > > > Notify drm core before sending pending events during crtc > > > > disable. > > > > This fixes the first event after disable having an old stale > > > > timestamp > > > > by having drm_crtc_vblank_off update the timestamp to now. > > > > > > > > This was seen while debugging weston log message: > > > > Warning: computed repaint delay is insane: -8212 msec > > > > > > > > > > Would you say this > > > Fixes: a474478642d5 ("drm/imx: fix crtc vblank state regression") > > > ? > > > > > > > Signed-off-by: Robert Beckett > > > > --- > > > > drivers/gpu/drm/imx/ipuv3-crtc.c | 6 +++--- > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > b/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > index 9cc1d678674f..c436a28d50e4 100644 > > > > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > @@ -91,14 +91,14 @@ static void ipu_crtc_atomic_disable(struct > > > > drm_crtc *crtc, > > > > ipu_dc_disable(ipu); > > > > ipu_prg_disable(ipu); > > > > > > > > + drm_crtc_vblank_off(crtc); > > > > + > > > > > > This is explained in the commit message and aligns with the > > > drm_crtc_state @event documentation. > > > > This part here looks fishy. The drm_vblank.c code is supposed to do > > the right thing, no matter where or when you ask it to generate an > > event. It definitely shouldn't generate a timestamp that's a few > > seconds too early. Bunch of options: > > - there's a bug in drm_vblank.c and it's mixing up something and > > generating a totally bogus value. > > - there's a lie in your imx vblank code, which trips the drm_vblank.c > > counter interpolation and results in a totally bogus value. > > > > drm_vblank.c assumes that if you do claim to have a hw counter and > > generate timestamps, that those are perfectly accurate. It only falls > > back to guestimating using the system timer if that's not present. > > > > Either way, this very much smells like papering over a bug if this > > change indeed fixes your wrong vblank timestamps. > > > > A quick explaination of where the dodgy timestamp came from: > 1. driver starts up > 2. fbcon comes along and restores fbdev, enabling vblank > 3. vblank_disable_fn fires via timer disabling vblank, keeping vblank > seq number and time set at current value > (some time later) > 4. weston starts and does a modeset > 5. atomic commit disables crtc while it does the modeset > 6. ipu_crtc_atomic_disable sends vblank with old seq number and time > > It turns out the actual fix for the old vblank is the next change, > which stops it being sent at all during the crtc disable as it is is > still active, it would then go through drm_crtc_vblank_off, reseting > the timestamp, and get delivered during the vblank enable as part of > the atomic commit. This shouldn't fix your vblank timestamp troubles either. It might mean that the timestamp is slightly too early (because you take it while shutting down the crtc, not while re-enabling), but not by seconds. Quick experiment: Disable vblank disabling with drm.vblankoffdelay = 0. If that also fixes the timestamps, then I'm pretty sure you have a driver bug somewhere and lie to the vblank core code about something. -Daniel > > So, in theory, we could just have the following change to fix the > specific issue of a stale timestamp. > > However, given the documentation for the event in > include/drm/drm_crtc.h: > > * - The event is for a CRTC which is being disabled through > this > *atomic commit. In that case the event can be send out any > time > *after the hardware has stopped scanning out the current > *framebuffers. It should contain the timestamp and counter > for the > *last vblank before the display pipeline was shut off. The > simplest > *way to achieve that is calling > drm_crtc_send_vblank_event() > *somewhen after drm_crtc_vblank_off() has been called. > > This still seems like a sensible change for when the crtc is being > disabled. > > > > > > > spin_lock_irq(&crtc->dev->event_lock); > > > > - if (crtc->state->event) > > > > { > > > > + if (crtc->state->event && !crtc->state->active) { > > > > > > This is not mentioned though. > > > > > > If the pending event is not sent here, I assume it will be picked > > > up by > > > .atomic_flush and will then be sent after the first EOF interrupt > > > after > > > the modeset is complete. Can you explain this in the commit > > > message? > > > > Yeah looks correct (you only want to generate the event here
Re: [PATCH 2/2] drm/prime: Update docs
On Wed, Jun 19, 2019 at 02:43:20PM +0200, Noralf Trønnes wrote: > > > Den 18.06.2019 11.20, skrev Daniel Vetter: > > Yes this is a bit a big patch, but since it's essentially a complete > > rewrite of all the prime docs I didn't see how to better split it up. > > > > Changes: > > - Consistently point to drm_gem_object_funcs as the preferred hooks, > > where applicable. > > > > - Document all the hooks in &drm_driver that lacked kerneldoc. > > > > - Completely new overview section, which now also includes the cleaned > > up lifetime/reference counting subchapter. I also mentioned the weak > > references in there due to the lookup caches. > > > > - Completely rewritten helper intro section, highlight the > > import/export related functionality. > > > > - Polish for all the functions and more cross references. > > > > I also sprinkled a bunch of todos all over. > > > > Most important: 0 code changes in here. The cleanup motivated by > > reading and improving all this will follow later on. > > > > v2: Actually update the prime helper docs. Plus add a few FIXMEs that > > I won't address right away in subsequent cleanup patches. > > > > v3: > > - Split out the function moving. This patch is now exclusively > > documentation changes. > > - Typos and nits (Sam). > > > > Cc: Sam Ravnborg > > Cc: Eric Anholt > > Cc: Emil Velikov > > Signed-off-by: Daniel Vetter > > --- > > Documentation/gpu/drm-mm.rst | 40 +-- > > drivers/gpu/drm/drm_prime.c | 197 +-- > > include/drm/drm_drv.h| 104 +++--- > > include/drm/drm_gem.h| 18 ++-- > > 4 files changed, 226 insertions(+), 133 deletions(-) > > > > > > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > > index 68b4de85370c..2234206288fa 100644 > > --- a/drivers/gpu/drm/drm_prime.c > > +++ b/drivers/gpu/drm/drm_prime.c > > @@ -38,47 +38,53 @@ > > > > #include "drm_internal.h" > > > > -/* > > - * DMA-BUF/GEM Object references and lifetime overview: > > - * > > - * On the export the dma_buf holds a reference to the exporting GEM > > - * object. It takes this reference in handle_to_fd_ioctl, when it > > - * first calls .prime_export and stores the exporting GEM object in > > - * the dma_buf priv. This reference needs to be released when the > > - * final reference to the &dma_buf itself is dropped and its > > - * &dma_buf_ops.release function is called. For GEM-based drivers, > > - * the dma_buf should be exported using drm_gem_dmabuf_export() and > > - * then released by drm_gem_dmabuf_release(). > > - * > > - * On the import the importing GEM object holds a reference to the > > - * dma_buf (which in turn holds a ref to the exporting GEM object). > > - * It takes that reference in the fd_to_handle ioctl. > > - * It calls dma_buf_get, creates an attachment to it and stores the > > - * attachment in the GEM object. When this attachment is destroyed > > - * when the imported object is destroyed, we remove the attachment > > - * and drop the reference to the dma_buf. > > - * > > - * When all the references to the &dma_buf are dropped, i.e. when > > - * userspace has closed both handles to the imported GEM object (through > > the > > - * FD_TO_HANDLE IOCTL) and closed the file descriptor of the exported > > - * (through the HANDLE_TO_FD IOCTL) dma_buf, and all kernel-internal > > references > > - * are also gone, then the dma_buf gets destroyed. This can also happen > > as a > > - * part of the clean up procedure in the drm_release() function if > > userspace > > - * fails to properly clean up. Note that both the kernel and userspace (by > > - * keeeping the PRIME file descriptors open) can hold references onto a > > - * &dma_buf. > > - * > > - * Thus the chain of references always flows in one direction > > - * (avoiding loops): importing_gem -> dmabuf -> exporting_gem > > - * > > - * Self-importing: if userspace is using PRIME as a replacement for flink > > - * then it will get a fd->handle request for a GEM object that it created. > > - * Drivers should detect this situation and return back the gem object > > - * from the dma-buf private. Prime will do this automatically for drivers > > that > > - * use the drm_gem_prime_{import,export} helpers. > > - * > > - * GEM struct &dma_buf_ops symbols are now exported. They can be resued by > > - * drivers which implement GEM interface. > > +/** > > + * DOC: overview and lifetime rules > > + * > > + * Similar to GEM global names, PRIME file descriptors are also used to > > share > > + * buffer objects across processes. They offer additional security: as file > > + * descriptors must be explicitly sent over UNIX domain sockets to be > > shared > > + * between applications, they can't be guessed like the globally unique GEM > > + * names. > > + * > > + * Drivers that support the PRIME API must set the DRIVER_PRIME bit in the > > + * &drm_driver.driver_features field, and implement the > > + * &drm_driver.
[PATCH] drm/prime: Update docs
Yes this is a bit a big patch, but since it's essentially a complete rewrite of all the prime docs I didn't see how to better split it up. Changes: - Consistently point to drm_gem_object_funcs as the preferred hooks, where applicable. - Document all the hooks in &drm_driver that lacked kerneldoc. - Completely new overview section, which now also includes the cleaned up lifetime/reference counting subchapter. I also mentioned the weak references in there due to the lookup caches. - Completely rewritten helper intro section, highlight the import/export related functionality. - Polish for all the functions and more cross references. I also sprinkled a bunch of todos all over. Most important: 0 code changes in here. The cleanup motivated by reading and improving all this will follow later on. v2: Actually update the prime helper docs. Plus add a few FIXMEs that I won't address right away in subsequent cleanup patches. v3: - Split out the function moving. This patch is now exclusively documentation changes. - Typos and nits (Sam). v4: Polish suggestions from Noralf. Acked-by: Gerd Hoffmann Acked-by: Emil Velikov Acked-by: Noralf Trønnes Cc: Thomas Zimmermann Cc: Gerd Hoffmann Cc: Noralf Trønnes Cc: Sam Ravnborg Cc: Eric Anholt Cc: Emil Velikov Signed-off-by: Daniel Vetter --- Documentation/gpu/drm-mm.rst | 40 +-- drivers/gpu/drm/drm_prime.c | 201 +-- include/drm/drm_drv.h| 104 +++--- include/drm/drm_gem.h| 18 ++-- 4 files changed, 229 insertions(+), 134 deletions(-) diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index c8ebd4f66a6a..b664f054c259 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -433,43 +433,11 @@ PRIME is the cross device buffer sharing framework in drm, originally created for the OPTIMUS range of multi-gpu platforms. To userspace PRIME buffers are dma-buf based file descriptors. -Overview and Driver Interface -- +Overview and Lifetime Rules +--- -Similar to GEM global names, PRIME file descriptors are also used to -share buffer objects across processes. They offer additional security: -as file descriptors must be explicitly sent over UNIX domain sockets to -be shared between applications, they can't be guessed like the globally -unique GEM names. - -Drivers that support the PRIME API must set the DRIVER_PRIME bit in the -struct :c:type:`struct drm_driver ` -driver_features field, and implement the prime_handle_to_fd and -prime_fd_to_handle operations. - -int (\*prime_handle_to_fd)(struct drm_device \*dev, struct drm_file -\*file_priv, uint32_t handle, uint32_t flags, int \*prime_fd); int -(\*prime_fd_to_handle)(struct drm_device \*dev, struct drm_file -\*file_priv, int prime_fd, uint32_t \*handle); Those two operations -convert a handle to a PRIME file descriptor and vice versa. Drivers must -use the kernel dma-buf buffer sharing framework to manage the PRIME file -descriptors. Similar to the mode setting API PRIME is agnostic to the -underlying buffer object manager, as long as handles are 32bit unsigned -integers. - -While non-GEM drivers must implement the operations themselves, GEM -drivers must use the :c:func:`drm_gem_prime_handle_to_fd()` and -:c:func:`drm_gem_prime_fd_to_handle()` helper functions. Those -helpers rely on the driver gem_prime_export and gem_prime_import -operations to create a dma-buf instance from a GEM object (dma-buf -exporter role) and to create a GEM object from a dma-buf instance -(dma-buf importer role). - -struct dma_buf \* (\*gem_prime_export)(struct drm_device \*dev, -struct drm_gem_object \*obj, int flags); struct drm_gem_object \* -(\*gem_prime_import)(struct drm_device \*dev, struct dma_buf -\*dma_buf); These two operations are mandatory for GEM drivers that -support PRIME. +.. kernel-doc:: drivers/gpu/drm/drm_prime.c + :doc: overview and lifetime rules PRIME Helper Functions -- diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 68b4de85370c..c269bc03c42a 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -38,47 +38,53 @@ #include "drm_internal.h" -/* - * DMA-BUF/GEM Object references and lifetime overview: - * - * On the export the dma_buf holds a reference to the exporting GEM - * object. It takes this reference in handle_to_fd_ioctl, when it - * first calls .prime_export and stores the exporting GEM object in - * the dma_buf priv. This reference needs to be released when the - * final reference to the &dma_buf itself is dropped and its - * &dma_buf_ops.release function is called. For GEM-based drivers, - * the dma_buf should be exported using drm_gem_dmabuf_export() and - * then released by drm_gem_dmabuf_release(). - * - * On the import the importing GEM object holds a reference to the - * dma_buf (which in turn holds a ref to the exporting GEM object). - *
Re: [PATCH] drm/todo: Update drm_gem_object_funcs todo even more
On Tue, Jun 18, 2019 at 07:25:08PM +0100, Eric Engestrom wrote: > On Tuesday, 2019-06-18 16:02:41 +0200, Daniel Vetter wrote: > > I rushed merging this a bit too much, and Noralf pointed out that > > we're a lot better already and have made great progress. > > > > Let's try again. > > > > Fixes: 42dbbb4b54a3 ("drm/todo: Add new debugfs todo") > > Cc: Greg Kroah-Hartman > > Cc: Daniel Vetter > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Maarten Lankhorst > > Cc: Maxime Ripard > > Cc: Sean Paul > > Cc: Thomas Zimmermann > > Cc: Gerd Hoffmann > > Cc: Rob Herring > > Cc: Noralf Trønnes > > Cc: Eric Anholt > > Cc: Gerd Hoffmann > > Signed-off-by: Daniel Vetter > > --- > > Documentation/gpu/todo.rst | 8 +--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > index 25878dd048fd..66c123737c3d 100644 > > --- a/Documentation/gpu/todo.rst > > +++ b/Documentation/gpu/todo.rst > > @@ -212,9 +212,11 @@ struct drm_gem_object_funcs > > GEM objects can now have a function table instead of having the callbacks > > on the > > DRM driver struct. This is now the preferred way and drivers can be moved > > over. > > > > -Unfortunately some of the recently added GEM helpers are going in the wrong > > -direction by adding OPS macros that use the old, deprecated hooks. See > > -DRM_GEM_CMA_VMAP_DRIVER_OPS, DRM_GEM_SHMEM_DRIVER_OPS, and > > DRM_GEM_VRAM_DRIVER_PRIME. > > +DRM_GEM_CMA_VMAP_DRIVER_OPS, DRM_GEM_SHMEM_DRIVER_OPS already support > > this, but > > +DRM_GEM_VRAM_DRIVER_PRIME does not yet and needs to be aligend with the > > previous > > s/aligend/aligned/ Fixed while applying. -Daniel > > > +two. We also need a 2nd version of the CMA define that doesn't require the > > +vmapping to be present (different hook for prime importing). Plus this > > needs to > > +be rolled out to all drivers using their own implementations, too. > > > > Use DRM_MODESET_LOCK_ALL_* helpers instead of boilerplate > > - > > -- > > 2.20.1 > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] video: add HDMI state notifier support
On Thu, Jun 20, 2019 at 5:25 PM Daniel Vetter wrote: > > On Wed, Jun 19, 2019 at 07:48:11PM +0800, Cheng-yi Chiang wrote: > > On Tue, Jun 18, 2019 at 8:12 PM Daniel Vetter wrote: > > > > > > On Tue, Jun 18, 2019 at 07:48:06PM +0800, Cheng-yi Chiang wrote: > > > > On Tue, Jun 11, 2019 at 8:35 PM Daniel Vetter wrote: > > > > > > > > > > On Tue, Jun 11, 2019 at 08:10:38PM +0800, Cheng-yi Chiang wrote: > > > > > > On Tue, Jun 4, 2019 at 3:24 PM Daniel Vetter > > > > > > wrote: > > > > > > > > > > > > > > On Tue, Jun 04, 2019 at 10:32:50AM +0800, Cheng-yi Chiang wrote: > > > > > > > > On Mon, Jun 3, 2019 at 4:09 PM Daniel Vetter > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Mon, Jun 03, 2019 at 09:45:49AM +0200, Hans Verkuil wrote: > > > > > > > > > > On 6/3/19 6:32 AM, Cheng-Yi Chiang wrote: > > > > > > > > > > > From: Hans Verkuil > > > > > > > > > > > > > > > > > > > > > > Add support for HDMI hotplug and EDID notifiers, which is > > > > > > > > > > > used to convey > > > > > > > > > > > information from HDMI drivers to their CEC and audio > > > > > > > > > > > counterparts. > > > > > > > > > > > > > > > > > > > > > > Based on an earlier version from Russell King: > > > > > > > > > > > > > > > > > > > > > > https://patchwork.kernel.org/patch/9277043/ > > > > > > > > > > > > > > > > > > > > > > The hdmi_notifier is a reference counted object > > > > > > > > > > > containing the HDMI state > > > > > > > > > > > of an HDMI device. > > > > > > > > > > > > > > > > > > > > > > When a new notifier is registered the current state will > > > > > > > > > > > be reported to > > > > > > > > > > > that notifier at registration time. > > > > > > > > > > > > > > > > > > > > > > Based on Hans Verkuil's patch: > > > > > > > > > > > > > > > > > > > > > > https://patchwork.kernel.org/patch/9472521/ > > > > > > > > > > > > > > > > > > > > Erm, you are aware that this patch morphed into a > > > > > > > > > > CEC-specific notifier > > > > > > > > > > found in drivers/media/cec/cec-notifier.c? > > > > > > > > > > > > > > > > > > > > I don't think it makes sense to have two notifier > > > > > > > > > > implementations in the kernel. > > > > > > > > > > The original intention was to have the notifier deal with > > > > > > > > > > both CEC and ASoC > > > > > > > > > > notifications, but there was not enough interest for the > > > > > > > > > > ASoC bits at the time > > > > > > > > > > and it was dropped. > > > > > > > > > > > > > > > > > > > > I am planning changes to the cec-notifier API, I hope to > > > > > > > > > > work on that this > > > > > > > > > > week. I'll CC you when I post those. Those might be a good > > > > > > > > > > starting point > > > > > > > > > > to convert the cec-notifier to an hdmi-notifier as was > > > > > > > > > > originally intended. > > > > > > > > > > > > > > > > > > > > I've added your colleague Dariusz Marcinkiewicz to the CC > > > > > > > > > > list since he's been > > > > > > > > > > working on some nice cec-notifier improvements as well. > > > > > > > > > > > > > > > > > > We also have some interfaces for drm/alsa interactions around > > > > > > > > > hdmi > > > > > > > > > already in drm/drm_audio_component.h, but it's not used by > > > > > > > > > anything > > > > > > > > > outside of i915. Imo we should extend that, not reinvent a > > > > > > > > > new wheel. > > > > > > > > > > > > > > > > > Hi Daniel, > > > > > > > > Thank you for the pointer. Looking at the ops, it seems that it > > > > > > > > is > > > > > > > > specific to HDA. > > > > > > > > I am not familiar with drm and HDA. I am not sure how > > > > > > > > applicable it > > > > > > > > would be to report jack status to ASoC. > > > > > > > > There is a use case in sound/soc/codecs/hdac_hdmi.c though so it > > > > > > > > should be possible. > > > > > > > > > > > > > > Currently hda is the only user, but the idea was to make it more > > > > > > > generic. > > > > > > > Jack status in alsa is what drm calls connector status btw. > > > > > > > > > > > > > > So if we can take that as a baseline and extend it (probably > > > > > > > needs some > > > > > > > registration boilerplate and helpers to look up the right > > > > > > > endpoint using > > > > > > > of/dt for soc systems, we use component.c in i915/hda for this), > > > > > > > that > > > > > > > would be great I think. > > > > > > > > > > > > > > > > Another note: notifiers considered evil, imo. Gets the job > > > > > > > > > done for one > > > > > > > > > case, as soon as you have multiple devices and need to make > > > > > > > > > sure you get > > > > > > > > > the update for the right one it all comes crashing down. > > > > > > > > > Please create an > > > > > > > > > api which registers for updates from a specific device only, > > > > > > > > > plus > > > > > > > > > something that has real callbacks (like the > > > > > > > > > drm_audio_component.h thing we > > > > > > > > > started already). > > > > > > > > > > > > > > > > To cl
[PATCH v3 1/2] dt-bindings: display: panel: Add support for Raydium RM67191 panel
Add dt-bindings documentation for Raydium RM67191 DSI panel. Signed-off-by: Robert Chiras Reviewed-by: Sam Ravnborg --- .../bindings/display/panel/raydium,rm67191.txt | 39 ++ 1 file changed, 39 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt diff --git a/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt new file mode 100644 index 000..52af272 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt @@ -0,0 +1,39 @@ +Raydium RM67171 OLED LCD panel with MIPI-DSI protocol + +Required properties: +- compatible: "raydium,rm67191" +- reg: virtual channel for MIPI-DSI protocol + must be <0> +- dsi-lanes: number of DSI lanes to be used + must be <3> or <4> +- port:input port node with endpoint definition as + defined in Documentation/devicetree/bindings/graph.txt; + the input port should be connected to a MIPI-DSI device + driver + +Optional properties: +- reset-gpios: a GPIO spec for the RST_B GPIO pin +- width-mm:see panel-common.txt +- height-mm: see panel-common.txt +- video-mode: 0 - burst-mode + 1 - non-burst with sync event + 2 - non-burst with sync pulse + +Example: + + panel@0 { + compatible = "raydium,rm67191"; + reg = <0>; + pinctrl-0 = <&pinctrl_mipi_dsi_0_1_en>; + pinctrl-names = "default"; + reset-gpios = <&gpio1 7 GPIO_ACTIVE_LOW>; + dsi-lanes = <4>; + width-mm = <68>; + height-mm = <121>; + + port { + panel_in: endpoint { + remote-endpoint = <&mipi_out>; + }; + }; + }; -- 2.7.4
Re: [PATCH] drm/imx: correct order of crtc disable
On Thu, 2019-06-20 at 14:32 +0200, Daniel Vetter wrote: > On Thu, Jun 20, 2019 at 12:12:13PM +0100, Robert Beckett wrote: > > On Thu, 2019-06-20 at 10:50 +0200, Daniel Vetter wrote: > > > On Wed, Jun 19, 2019 at 11:40 AM Philipp Zabel < > > > p.za...@pengutronix.de> wrote: > > > > > > > > Hi Robert, > > > > > > > > thank you for the patch. > > > > > > > > On Tue, 2019-06-18 at 16:50 +0100, Robert Beckett wrote: > > > > > Notify drm core before sending pending events during crtc > > > > > disable. > > > > > This fixes the first event after disable having an old stale > > > > > timestamp > > > > > by having drm_crtc_vblank_off update the timestamp to now. > > > > > > > > > > This was seen while debugging weston log message: > > > > > Warning: computed repaint delay is insane: -8212 msec > > > > > > > > > > > > > Would you say this > > > > Fixes: a474478642d5 ("drm/imx: fix crtc vblank state > > > > regression") > > > > ? > > > > > > > > > Signed-off-by: Robert Beckett > > > > > --- > > > > > drivers/gpu/drm/imx/ipuv3-crtc.c | 6 +++--- > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > > b/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > > index 9cc1d678674f..c436a28d50e4 100644 > > > > > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > > @@ -91,14 +91,14 @@ static void > > > > > ipu_crtc_atomic_disable(struct > > > > > drm_crtc *crtc, > > > > > ipu_dc_disable(ipu); > > > > > ipu_prg_disable(ipu); > > > > > > > > > > + drm_crtc_vblank_off(crtc); > > > > > + > > > > > > > > This is explained in the commit message and aligns with the > > > > drm_crtc_state @event documentation. > > > > > > This part here looks fishy. The drm_vblank.c code is supposed to > > > do > > > the right thing, no matter where or when you ask it to generate > > > an > > > event. It definitely shouldn't generate a timestamp that's a few > > > seconds too early. Bunch of options: > > > - there's a bug in drm_vblank.c and it's mixing up something and > > > generating a totally bogus value. > > > - there's a lie in your imx vblank code, which trips the > > > drm_vblank.c > > > counter interpolation and results in a totally bogus value. > > > > > > drm_vblank.c assumes that if you do claim to have a hw counter > > > and > > > generate timestamps, that those are perfectly accurate. It only > > > falls > > > back to guestimating using the system timer if that's not > > > present. > > > > > > Either way, this very much smells like papering over a bug if > > > this > > > change indeed fixes your wrong vblank timestamps. > > > > > > > A quick explaination of where the dodgy timestamp came from: > > 1. driver starts up > > 2. fbcon comes along and restores fbdev, enabling vblank > > 3. vblank_disable_fn fires via timer disabling vblank, keeping > > vblank > > seq number and time set at current value > > (some time later) > > 4. weston starts and does a modeset > > 5. atomic commit disables crtc while it does the modeset > > 6. ipu_crtc_atomic_disable sends vblank with old seq number and > > time > > > > It turns out the actual fix for the old vblank is the next change, > > which stops it being sent at all during the crtc disable as it is > > is > > still active, it would then go through drm_crtc_vblank_off, > > reseting > > the timestamp, and get delivered during the vblank enable as part > > of > > the atomic commit. > > This shouldn't fix your vblank timestamp troubles either. It might > mean > that the timestamp is slightly too early (because you take it while > shutting down the crtc, not while re-enabling), but not by seconds. > > Quick experiment: Disable vblank disabling with drm.vblankoffdelay = > 0. If > that also fixes the timestamps, then I'm pretty sure you have a > driver bug > somewhere and lie to the vblank core code about something. > -Daniel > Experiment done. The timestamp is fine, it is the timestamp of the previous vblank update. But, this would fix it because the vblank interrupt was never disabled. The original issue was that the event got sent after vblank was disabled and before it got re-enabled during the modeset, so nothing had happened to update the timestamp and seq number. What are you expecting to update the timestamp and seq number while the interrupt is disabled after vblank_disable_fn? Im struggling to see what this experiment was meant to test/prove. > > > > So, in theory, we could just have the following change to fix the > > specific issue of a stale timestamp. > > > > However, given the documentation for the event in > > include/drm/drm_crtc.h: > > > > * - The event is for a CRTC which is being disabled > > through > > this > > *atomic commit. In that case the event can be send out > > any > > time > > *after the hardware has stopped scanning out the > > current > > *framebuffers. It should contain t
[PATCH v3 2/2] drm/panel: Add support for Raydium RM67191 panel driver
This patch adds Raydium RM67191 TFT LCD panel driver (MIPI-DSI protocol). Signed-off-by: Robert Chiras Reviewed-by: Sam Ravnborg --- MAINTAINERS | 6 + drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile| 1 + drivers/gpu/drm/panel/panel-raydium-rm67191.c | 690 ++ 4 files changed, 706 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-raydium-rm67191.c diff --git a/MAINTAINERS b/MAINTAINERS index 7a2f487..cd93030e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5089,6 +5089,12 @@ S: Maintained F: drivers/gpu/drm/qxl/ F: include/uapi/drm/qxl_drm.h +DRM DRIVER FOR RAYDIUM RM67191 PANELS +M: Robert Chiras +S: Maintained +F: drivers/gpu/drm/panel/panel-raydium-rm67191.c +F: Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt + DRM DRIVER FOR RAGE 128 VIDEO CARDS S: Orphan / Obsolete F: drivers/gpu/drm/r128/ diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index d9d931a..8be1ac1 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -159,6 +159,15 @@ config DRM_PANEL_RASPBERRYPI_TOUCHSCREEN Pi 7" Touchscreen. To compile this driver as a module, choose M here. +config DRM_PANEL_RAYDIUM_RM67191 + tristate "Raydium RM67191 FHD 1080x1920 DSI video mode panel" + depends on OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y here if you want to enable support for Raydium RM67191 FHD + (1080x1920) DSI panel. + config DRM_PANEL_RAYDIUM_RM68200 tristate "Raydium RM68200 720x1280 DSI video mode panel" depends on OF diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile index fb0cb3a..1fc0f68 100644 --- a/drivers/gpu/drm/panel/Makefile +++ b/drivers/gpu/drm/panel/Makefile @@ -14,6 +14,7 @@ obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += panel-orisetech-otm8009a.o obj-$(CONFIG_DRM_PANEL_OSD_OSD101T2587_53TS) += panel-osd-osd101t2587-53ts.o obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += panel-panasonic-vvx10f034n00.o obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += panel-raspberrypi-touchscreen.o +obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM67191) += panel-raydium-rm67191.o obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o obj-$(CONFIG_DRM_PANEL_ROCKTECH_JH057N00900) += panel-rocktech-jh057n00900.o obj-$(CONFIG_DRM_PANEL_RONBO_RB070D30) += panel-ronbo-rb070d30.o diff --git a/drivers/gpu/drm/panel/panel-raydium-rm67191.c b/drivers/gpu/drm/panel/panel-raydium-rm67191.c new file mode 100644 index 000..c4d7dfd --- /dev/null +++ b/drivers/gpu/drm/panel/panel-raydium-rm67191.c @@ -0,0 +1,690 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Raydium RM67191 MIPI-DSI panel driver + * + * Copyright 2019 NXP + */ + +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#include +#include +#include +#include + +/* Panel specific color-format bits */ +#define COL_FMT_16BPP 0x55 +#define COL_FMT_18BPP 0x66 +#define COL_FMT_24BPP 0x77 + +/* Write Manufacture Command Set Control */ +#define WRMAUCCTR 0xFE + +/* Manufacturer Command Set pages (CMD2) */ +struct cmd_set_entry { + u8 cmd; + u8 param; +}; + +/* + * There is no description in the Reference Manual about these commands. + * We received them from vendor, so just use them as is. + */ +static const struct cmd_set_entry manufacturer_cmd_set[] = { + {0xFE, 0x0B}, + {0x28, 0x40}, + {0x29, 0x4F}, + {0xFE, 0x0E}, + {0x4B, 0x00}, + {0x4C, 0x0F}, + {0x4D, 0x20}, + {0x4E, 0x40}, + {0x4F, 0x60}, + {0x50, 0xA0}, + {0x51, 0xC0}, + {0x52, 0xE0}, + {0x53, 0xFF}, + {0xFE, 0x0D}, + {0x18, 0x08}, + {0x42, 0x00}, + {0x08, 0x41}, + {0x46, 0x02}, + {0x72, 0x09}, + {0xFE, 0x0A}, + {0x24, 0x17}, + {0x04, 0x07}, + {0x1A, 0x0C}, + {0x0F, 0x44}, + {0xFE, 0x04}, + {0x00, 0x0C}, + {0x05, 0x08}, + {0x06, 0x08}, + {0x08, 0x08}, + {0x09, 0x08}, + {0x0A, 0xE6}, + {0x0B, 0x8C}, + {0x1A, 0x12}, + {0x1E, 0xE0}, + {0x29, 0x93}, + {0x2A, 0x93}, + {0x2F, 0x02}, + {0x31, 0x02}, + {0x33, 0x05}, + {0x37, 0x2D}, + {0x38, 0x2D}, + {0x3A, 0x1E}, + {0x3B, 0x1E}, + {0x3D, 0x27}, + {0x3F, 0x80}, + {0x40, 0x40}, + {0x41, 0xE0}, + {0x4F, 0x2F}, + {0x50, 0x1E}, + {0xFE, 0x06}, + {0x00, 0xCC}, + {0x05, 0x05}, + {0x07, 0xA2}, + {0x08, 0xCC}, + {0x0D, 0x03}, + {0x0F, 0xA2}, + {0x32, 0xCC}, + {0x37, 0x05}, + {0x39, 0x83}, + {0x3A, 0xCC}, + {0x41, 0x04}, + {0x43, 0x83}, + {0x44, 0xCC}, + {0x49, 0x05},
[PATCH v3 0/2] Add DSI panel driver for Raydium RM67191
This patch-set contains the DRM panel driver and dt-bindings documentation for the DSI driven panel: Raydium RM67191. v3: - Added myself to MAINTAINERS for this driver (sam) - Removed display-timings property (fabio) - Fixed dt description (sam) - Re-arranged calls inside get_modes function (sam) - Changed ifdefs with _maybe_unused for suspend/resume functions (sam) - Collected Reviewed-by from Sam v2: - Fixed 'reset-gpio' to 'reset-gpios' property naming (fabio) - Changed the state of the reset gpio to active low and fixed how it is handled in driver (fabio) - Fixed copyright statement (daniel) - Reordered includes (sam) - Added defines for panel specific color formats (fabio) - Removed unnecessary tests in enable and unprepare (sam) - Removed the unnecessary backlight write in enable (sam) Robert Chiras (2): dt-bindings: display: panel: Add support for Raydium RM67191 panel drm/panel: Add support for Raydium RM67191 panel driver .../bindings/display/panel/raydium,rm67191.txt | 39 ++ MAINTAINERS| 6 + drivers/gpu/drm/panel/Kconfig | 9 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-raydium-rm67191.c | 690 + 5 files changed, 745 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/raydium,rm67191.txt create mode 100644 drivers/gpu/drm/panel/panel-raydium-rm67191.c -- 2.7.4
Re: [PATCH v1] backlight: Don't build support by default
On 12/06/2019 14:27, Marc Gonzalez wrote: b20c5249aa6a ("backlight: Fix compile error if CONFIG_FB is unset") added 'default m' for BACKLIGHT_CLASS_DEVICE and LCD_CLASS_DEVICE. It took me some little while until I realized this patch is from 2005 which explains why I couldn't find it in the modern git repo! Let's go back to not building support by default. At first glance disabling this by default looks like it would cause some existing defconfig files to disable useful drivers. For backlight I think this isn't true (because both DRM and FB_BACKLIGHT have a "select" on BACKLIGHT_CLASS_DEVICE). However for LCD it is not nearly as clear cut. Commit message needs to explain why this won't cause unacceptable problems for existinng defconfig files. Daniel. Signed-off-by: Marc Gonzalez --- drivers/video/backlight/Kconfig | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig index 8b081d61773e..40676be2e46a 100644 --- a/drivers/video/backlight/Kconfig +++ b/drivers/video/backlight/Kconfig @@ -10,7 +10,6 @@ menu "Backlight & LCD device support" # config LCD_CLASS_DEVICE tristate "Lowlevel LCD controls" - default m help This framework adds support for low-level control of LCD. Some framebuffer devices connect to platform-specific LCD modules @@ -143,7 +142,6 @@ endif # LCD_CLASS_DEVICE # config BACKLIGHT_CLASS_DEVICE tristate "Lowlevel Backlight controls" - default m help This framework adds support for low-level control of the LCD backlight. This includes support for brightness and power. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 109887] vega56 undervolting/overclocking voltage issues
https://bugs.freedesktop.org/show_bug.cgi?id=109887 --- Comment #7 from hagar-dunor --- Met the same annoyance, and found a rather convoluted way to get around it. It would be better overclocking/undervolting work by setting pp_od_clk_voltage only. https://forum.level1techs.com/t/how-to-overclock-vega-on-linux/132771/65 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [v7 00/16] Add Plane Color Properties
>-Original Message- >From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of >Ville >Syrjälä >Sent: Wednesday, June 19, 2019 10:00 PM >To: Ezequiel Garcia >Cc: Syrjala, Ville ; intel-...@lists.freedesktop.org; >Emil >Velikov ; dri-devel >; >Andrzej Pietrasiewicz ; Shankar, Uma >; Sean Paul ; Ezequiel Garcia >; Boris Brezillon >; >Lankhorst, Maarten >Subject: Re: [v7 00/16] Add Plane Color Properties > >On Wed, Jun 19, 2019 at 12:33:33PM -0300, Ezequiel Garcia wrote: >> On Wed, 2019-06-19 at 18:03 +0300, Ville Syrjälä wrote: >> > On Wed, Jun 19, 2019 at 10:18:18AM -0300, Ezequiel Garcia wrote: >> > > On Wed, 2019-06-19 at 06:20 +, Shankar, Uma wrote: >> > > > > -Original Message- >> > > > > From: dri-devel >> > > > > [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of >> > > > > Ezequiel Garcia >> > > > > Sent: Friday, June 14, 2019 9:48 PM >> > > > > To: Shankar, Uma >> > > > > Cc: Emil Velikov ; >> > > > > intel-...@lists.freedesktop.org; Syrjala, Ville >> > > > > ; Lankhorst, Maarten >> > > > > ; dri-devel >> > > > > >> > > > > Subject: Re: [v7 00/16] Add Plane Color Properties >> > > > > >> > > > > On Thu, 28 Mar 2019 at 16:50, Uma Shankar >wrote: >> > > > > > This is how a typical display color hardware pipeline looks like: >> > > > > > +---+ >> > > > > > |RAM| >> > > > > > | +--++-++-+ | >> > > > > > | | FB 1 || FB 2 || FB N| | >> > > > > > | +--++-++-+ | >> > > > > > +---+ >> > > > > >| Plane Color Hardware Block | >> > > > > > ++ >> > > > > > | +---v-+ +---v---+ +---v--+ | >> > > > > > | | Plane A | | Plane B | | Plane N | | >> > > > > > | | DeGamma | | Degamma | | Degamma | | >> > > > > > | +---+-+ +---+---+ +---+--+ | >> > > > > > | | | || >> > > > > > | +---v-+ +---v---+ +---v--+ | >> > > > > > | |Plane A | | Plane B | | Plane N | | >> > > > > > | |CSC/CTM | | CSC/CTM | | CSC/CTM | | >> > > > > > | +---+-+ ++--+ ++-+ | >> > > > > > | | | | | >> > > > > > | +---v-+ +v--+ +v-+ | >> > > > > > | | Plane A | | Plane B | | Plane N | | >> > > > > > | | Gamma | | Gamma | | Gamma| | >> > > > > > | +---+-+ ++--+ ++-+ | >> > > > > > | | | | | >> > > > > > ++ >> > > > > > +--v--v---v---| >> > > > > > > > || >> > > > > > > > Pipe Blender|| >> > > > > > +++ >> > > > > > >|| >> > > > > > >+---v--+ | >> > > > > > >| Pipe DeGamma| | >> > > > > > >| | | >> > > > > > >+---+--+ | >> > > > > > >|Pipe Color | >> > > > > > >+---v--+ Hardware| >> > > > > > >| Pipe CSC/CTM| | >> > > > > > >| | | >> > > > > > >+---+--+ | >> > > > > > >|| >> > > > > > >+---v--+ | >> > > > > > >| Pipe Gamma | | >> > > > > > >| | | >> > > > > > >+---+--+ | >> > > > > > >|| >> > > > > > +-+ >> > > > > > | >> > > > > > v >> > > > > >Pipe Output >> > > > > > >> > > > > > This patch series adds properties for plane color features. >> > > > > > It adds properties for degamma used to linearize data, CSC >> > > > > > used for gamut conversion, and gamma used to again >> > > > > > non-linearize data as per panel supported color space. These >> > > > > > can be utilize by user space to convert planes from one format to >> > > > > > another, >one color space to another etc. >> > > > > > >> > > > > > Usersapce can take smart blending decisions and utilize >> > > > > > these hardware supported plane color features to get >> > > > > > accurate color profile. The same can help in consistent >> > > > > > color quality from source to panel taking advantage of advanced >> > > > > > color >features in hardware. >> > > > > > >> > > > > > These patches just add the property interfaces and
[PATCH v7 1/6] net: stmmac: sun8i: add support for Allwinner H6 EMAC
From: Icenowy Zheng The EMAC on Allwinner H6 is just like the one on A64. The "internal PHY" on H6 is on a co-packaged AC200 chip, and it's not really internal (it's connected via RMII at PA GPIO bank). Add support for the Allwinner H6 EMAC in the dwmac-sun8i driver. Signed-off-by: Icenowy Zheng Signed-off-by: Ondrej Jirman --- .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c| 16 1 file changed, 16 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index b15c6d5dbd38..c3e94104474f 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -138,6 +138,20 @@ static const struct emac_variant emac_variant_a64 = { .tx_delay_max = 7, }; +static const struct emac_variant emac_variant_h6 = { + .default_syscon_value = 0x5, + .syscon_field = &sun8i_syscon_reg_field, + /* The "Internal PHY" of H6 is not on the die. It's on the +* co-packaged AC200 chip instead. +*/ + .soc_has_internal_phy = false, + .support_mii = true, + .support_rmii = true, + .support_rgmii = true, + .rx_delay_max = 31, + .tx_delay_max = 7, +}; + #define EMAC_BASIC_CTL0 0x00 #define EMAC_BASIC_CTL1 0x04 #define EMAC_INT_STA0x08 @@ -1216,6 +1230,8 @@ static const struct of_device_id sun8i_dwmac_match[] = { .data = &emac_variant_r40 }, { .compatible = "allwinner,sun50i-a64-emac", .data = &emac_variant_a64 }, + { .compatible = "allwinner,sun50i-h6-emac", + .data = &emac_variant_h6 }, { } }; MODULE_DEVICE_TABLE(of, sun8i_dwmac_match); -- 2.22.0
[PATCH v7 5/6] drm: sun4i: Add support for enabling DDC I2C bus to sun8i_dw_hdmi glue
From: Ondrej Jirman Orange Pi 3 board requires enabling a voltage shifting circuit via GPIO for the DDC bus to be usable. Add support for hdmi-connector node's optional ddc-en-gpios property to support this use case. Signed-off-by: Ondrej Jirman --- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 54 +-- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 2 + 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c index 39d8509d96a0..6733bfc9c2d6 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c @@ -98,10 +98,34 @@ static u32 sun8i_dw_hdmi_find_possible_crtcs(struct drm_device *drm, return crtcs; } +static int sun8i_dw_hdmi_find_connector_pdev(struct device *dev, +struct platform_device **pdev_out) +{ + struct platform_device *pdev; + struct device_node *remote; + + remote = of_graph_get_remote_node(dev->of_node, 1, -1); + if (!remote) + return -ENODEV; + + if (!of_device_is_compatible(remote, "hdmi-connector")) { + of_node_put(remote); + return -ENODEV; + } + + pdev = of_find_device_by_node(remote); + of_node_put(remote); + if (!pdev) + return -ENODEV; + + *pdev_out = pdev; + return 0; +} + static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, void *data) { - struct platform_device *pdev = to_platform_device(dev); + struct platform_device *pdev = to_platform_device(dev), *connector_pdev; struct dw_hdmi_plat_data *plat_data; struct drm_device *drm = data; struct device_node *phy_node; @@ -151,16 +175,30 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, return PTR_ERR(hdmi->regulator); } + ret = sun8i_dw_hdmi_find_connector_pdev(dev, &connector_pdev); + if (!ret) { + hdmi->ddc_en = gpiod_get_optional(&connector_pdev->dev, + "ddc-en", GPIOD_OUT_HIGH); + platform_device_put(connector_pdev); + + if (IS_ERR(hdmi->ddc_en)) { + dev_err(dev, "Couldn't get ddc-en gpio\n"); + return PTR_ERR(hdmi->ddc_en); + } + } + ret = regulator_enable(hdmi->regulator); if (ret) { dev_err(dev, "Failed to enable regulator\n"); - return ret; + goto err_unref_ddc_en; } + gpiod_set_value(hdmi->ddc_en, 1); + ret = reset_control_deassert(hdmi->rst_ctrl); if (ret) { dev_err(dev, "Could not deassert ctrl reset control\n"); - goto err_disable_regulator; + goto err_disable_ddc_en; } ret = clk_prepare_enable(hdmi->clk_tmds); @@ -213,8 +251,12 @@ static int sun8i_dw_hdmi_bind(struct device *dev, struct device *master, clk_disable_unprepare(hdmi->clk_tmds); err_assert_ctrl_reset: reset_control_assert(hdmi->rst_ctrl); -err_disable_regulator: +err_disable_ddc_en: + gpiod_set_value(hdmi->ddc_en, 0); regulator_disable(hdmi->regulator); +err_unref_ddc_en: + if (hdmi->ddc_en) + gpiod_put(hdmi->ddc_en); return ret; } @@ -228,7 +270,11 @@ static void sun8i_dw_hdmi_unbind(struct device *dev, struct device *master, sun8i_hdmi_phy_remove(hdmi); clk_disable_unprepare(hdmi->clk_tmds); reset_control_assert(hdmi->rst_ctrl); + gpiod_set_value(hdmi->ddc_en, 0); regulator_disable(hdmi->regulator); + + if (hdmi->ddc_en) + gpiod_put(hdmi->ddc_en); } static const struct component_ops sun8i_dw_hdmi_ops = { diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 720c5aa8adc1..d707c9171824 100644 --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -190,6 +191,7 @@ struct sun8i_dw_hdmi { struct regulator*regulator; const struct sun8i_dw_hdmi_quirks *quirks; struct reset_control*rst_ctrl; + struct gpio_desc*ddc_en; }; static inline struct sun8i_dw_hdmi * -- 2.22.0
[PATCH v7 6/6] arm64: dts: allwinner: orange-pi-3: Enable HDMI output
From: Ondrej Jirman Orange Pi 3 has a DDC_CEC_EN signal connected to PH2, that enables the DDC I2C bus voltage shifter. Before EDID can be read, we need to pull PH2 high. This is realized by the ddc-en-gpios property. Signed-off-by: Ondrej Jirman --- .../dts/allwinner/sun50i-h6-orangepi-3.dts| 26 +++ 1 file changed, 26 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts index 2c6807b74ff6..01bb1bafe284 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts @@ -22,6 +22,18 @@ stdout-path = "serial0:115200n8"; }; + connector { + compatible = "hdmi-connector"; + ddc-en-gpios = <&pio 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */ + type = "a"; + + port { + hdmi_con_in: endpoint { + remote-endpoint = <&hdmi_out_con>; + }; + }; + }; + leds { compatible = "gpio-leds"; @@ -72,6 +84,10 @@ cpu-supply = <®_dcdca>; }; +&de { + status = "okay"; +}; + &ehci0 { status = "okay"; }; @@ -91,6 +107,16 @@ status = "okay"; }; +&hdmi { + status = "okay"; +}; + +&hdmi_out { + hdmi_out_con: endpoint { + remote-endpoint = <&hdmi_con_in>; + }; +}; + &mdio { ext_rgmii_phy: ethernet-phy@1 { compatible = "ethernet-phy-ieee802.3-c22"; -- 2.22.0
[PATCH v7 0/6] Add support for Orange Pi 3
From: Ondrej Jirman This series implements support for Xunlong Orange Pi 3 board. - ethernet support (patches 1-3) - HDMI support (patches 4-6) For some people, ethernet doesn't work after reboot (but works on cold boot), when the stmmac driver is built into the kernel. It works when the driver is built as a module. It's either some timing issue, or power supply issue or a combination of both. Module build induces a power cycling of the phy. I encourage people with this issue, to build the driver into the kernel, and try to alter the reset timings for the phy in DTS or startup-delay-us and report the findings. Please take a look. thank you and regards, Ondrej Jirman Changes in v7: - dropped stored reference to connector_pdev as suggested by Jernej - added forgotten dt-bindings reviewed-by tag Changes in v6: - added dt-bindings reviewed-by tag - fix wording in stmmac commit (as suggested by Sergei) Changes in v5: - dropped already applied patches (pinctrl patches, mmc1 pinconf patch) - rename GMAC-3V3 -> GMAC-3V to match the schematic (Jagan) - changed hdmi-connector's ddc-supply property to ddc-en-gpios (Rob Herring) Changes in v4: - fix checkpatch warnings/style issues - use enum in struct sunxi_desc_function for io_bias_cfg_variant - collected acked-by's - fix compile error in drivers/pinctrl/sunxi/pinctrl-sun9i-a80-r.c:156 caused by missing conversion from has_io_bias_cfg struct member (I've kept the acked-by, because it's a trivial change, but feel free to object.) (reported by Martin A. on github) I did not have A80 pinctrl enabled for some reason, so I did not catch this sooner. - dropped brcm firmware patch (was already applied) - dropped the wifi dts patch (will re-send after H6 RTC gets merged, along with bluetooth support, in a separate series) Changes in v3: - dropped already applied patches - changed pinctrl I/O bias selection constants to enum and renamed - added /omit-if-no-ref/ to mmc1_pins - made mmc1_pins default pinconf for mmc1 in H6 dtsi - move ddc-supply to HDMI connector node, updated patch descriptions, changed dt-bindings docs Changes in v2: - added dt-bindings documentation for the board's compatible string (suggested by Clement) - addressed checkpatch warnings and code formatting issues (on Maxime's suggestions) - stmmac: dropped useless parenthesis, reworded description of the patch (suggested by Sergei) - drop useles dev_info() about the selected io bias voltage - docummented io voltage bias selection variant macros - wifi: marked WiFi DTS patch and realted mmc1_pins as "DO NOT MERGE", because wifi depends on H6 RTC support that's not merged yet (suggested by Clement) - added missing signed-of-bys - changed &usb2otg dr_mode to otg, and added a note about VBUS - improved wording of HDMI driver's DDC power supply patch Icenowy Zheng (2): net: stmmac: sun8i: add support for Allwinner H6 EMAC net: stmmac: sun8i: force select external PHY when no internal one Ondrej Jirman (4): arm64: dts: allwinner: orange-pi-3: Enable ethernet dt-bindings: display: hdmi-connector: Support DDC bus enable drm: sun4i: Add support for enabling DDC I2C bus to sun8i_dw_hdmi glue arm64: dts: allwinner: orange-pi-3: Enable HDMI output .../display/connector/hdmi-connector.txt | 1 + .../dts/allwinner/sun50i-h6-orangepi-3.dts| 70 +++ drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 54 -- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 2 + .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 21 ++ 5 files changed, 144 insertions(+), 4 deletions(-) -- 2.22.0
[PATCH v7 2/6] net: stmmac: sun8i: force select external PHY when no internal one
From: Icenowy Zheng The PHY selection bit also exists on SoCs without an internal PHY; if it's set to 1 (internal PHY, default value) then the MAC will not make use of any PHY on such SoCs. This problem appears when adapting for H6, which has no real internal PHY (the "internal PHY" on H6 is not on-die, but on a co-packaged AC200 chip, connected via RMII interface at GPIO bank A). Force the PHY selection bit to 0 when the SOC doesn't have an internal PHY, to address the problem of a wrong default value. Signed-off-by: Icenowy Zheng Signed-off-by: Ondrej Jirman --- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index c3e94104474f..6d5cba4075eb 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -898,6 +898,11 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv) * address. No need to mask it again. */ reg |= 1 << H3_EPHY_ADDR_SHIFT; + } else { + /* For SoCs without internal PHY the PHY selection bit should be +* set to 0 (external PHY). +*/ + reg &= ~H3_EPHY_SELECT; } if (!of_property_read_u32(node, "allwinner,tx-delay-ps", &val)) { -- 2.22.0
[PATCH v7 3/6] arm64: dts: allwinner: orange-pi-3: Enable ethernet
From: Ondrej Jirman Orange Pi 3 has two regulators that power the Realtek RTL8211E. According to the phy datasheet, both regulators need to be enabled at the same time, but we can only specify a single phy-supply in the DT. This can be achieved by making one regulator depedning on the other via vin-supply. While it's not a technically correct description of the hardware, it achieves the purpose. All values of RX/TX delay were tested exhaustively and a middle one of the working values was chosen. Signed-off-by: Ondrej Jirman --- .../dts/allwinner/sun50i-h6-orangepi-3.dts| 44 +++ 1 file changed, 44 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts index 17d496990108..2c6807b74ff6 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts @@ -15,6 +15,7 @@ aliases { serial0 = &uart0; + ethernet0 = &emac; }; chosen { @@ -44,6 +45,27 @@ regulator-max-microvolt = <500>; regulator-always-on; }; + + /* +* The board uses 2.5V RGMII signalling. Power sequence to enable +* the phy is to enable GMAC-2V5 and GMAC-3V (aldo2) power rails +* at the same time and to wait 100ms. +*/ + reg_gmac_2v5: gmac-2v5 { + compatible = "regulator-fixed"; + regulator-name = "gmac-2v5"; + regulator-min-microvolt = <250>; + regulator-max-microvolt = <250>; + startup-delay-us = <10>; + enable-active-high; + gpio = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */ + + /* The real parent of gmac-2v5 is reg_vcc5v, but we need to +* enable two regulators to power the phy. This is one way +* to achieve that. +*/ + vin-supply = <®_aldo2>; /* GMAC-3V */ + }; }; &cpu0 { @@ -58,6 +80,28 @@ status = "okay"; }; +&emac { + pinctrl-names = "default"; + pinctrl-0 = <&ext_rgmii_pins>; + phy-mode = "rgmii"; + phy-handle = <&ext_rgmii_phy>; + phy-supply = <®_gmac_2v5>; + allwinner,rx-delay-ps = <1500>; + allwinner,tx-delay-ps = <700>; + status = "okay"; +}; + +&mdio { + ext_rgmii_phy: ethernet-phy@1 { + compatible = "ethernet-phy-ieee802.3-c22"; + reg = <1>; + + reset-gpios = <&pio 3 14 GPIO_ACTIVE_LOW>; /* PD14 */ + reset-assert-us = <15000>; + reset-deassert-us = <4>; + }; +}; + &mmc0 { vmmc-supply = <®_cldo1>; cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */ -- 2.22.0
[PATCH v7 4/6] dt-bindings: display: hdmi-connector: Support DDC bus enable
From: Ondrej Jirman Some Allwinner SoC using boards (Orange Pi 3 for example) need to enable on-board voltage shifting logic for the DDC bus using a gpio to be able to access DDC bus. Use ddc-en-gpios property on the hdmi-connector to model this. Add binding documentation for optional ddc-en-gpios property. Signed-off-by: Ondrej Jirman Reviewed-by: Rob Herring --- .../devicetree/bindings/display/connector/hdmi-connector.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/display/connector/hdmi-connector.txt b/Documentation/devicetree/bindings/display/connector/hdmi-connector.txt index 508aee461e0d..aeb07c4bd703 100644 --- a/Documentation/devicetree/bindings/display/connector/hdmi-connector.txt +++ b/Documentation/devicetree/bindings/display/connector/hdmi-connector.txt @@ -9,6 +9,7 @@ Optional properties: - label: a symbolic name for the connector - hpd-gpios: HPD GPIO number - ddc-i2c-bus: phandle link to the I2C controller used for DDC EDID probing +- ddc-en-gpios: signal to enable DDC bus Required nodes: - Video port for HDMI input -- 2.22.0
Re: Re: [PATCH] backlight: gpio-backlight: Set power state instead of brightness at probe
On 18/06/2019 13:58, Paul Kocialkowski wrote: Hi, On Fri, 2019-05-17 at 17:05 +0200, Paul Kocialkowski wrote: On a trivial gpio-backlight setup with a panel using the backlight but no boot software to enable it beforehand, we fall in a case where the backlight is disabled (not just blanked) and thus remains disabled when the panel gets enabled. Setting gbl->def_value via the device-tree prop allows enabling the backlight in this situation, but it will be unblanked straight away, in compliance with the binding. This does not work well when there was no boot software to display something before, since we really need to unblank by the time the panel is enabled, not before. Resolve the situation by setting the brightness to 1 at probe and managing the power state accordingly, a bit like it's done in pwm-backlight. Any feedback on this? I was under the impression that it could be quite controversial, as it implies that the backlight can no longer be enabled without a bound panel (which IMO makes good sense but could be a matter to debate). My apologies. This patch brought on such severe deja-vu I got rather confused. Then when I went digging I've also dropped the ball on the same feature previously. Peter Ujfalusi provided a similar patch to yours but with a slightly different implementation: https://lore.kernel.org/patchwork/patch/1002359/ On the whole I think it is important to read the GPIO pin since otherwise we swap problems when there bootloader does setup the backlight for problems where it does not. The thing I don't get is why both patches try to avoid setting the backlight brightness from def_value. Simple displays, especially monochrome ones are perfectly readable with the backlight off... zero brightness is not a "bad" value. Not sure if Peter is still willing to rev his version of this code (given how badly we neglected him previously) or whether you want to try and combine both ideas. Daniel. Cheers, Paul Fixes: 8b770e3c9824 ("backlight: Add GPIO-based backlight driver") Signed-off-by: Paul Kocialkowski --- drivers/video/backlight/gpio_backlight.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index e470da95d806..c9cb97fa13d0 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -57,6 +57,21 @@ static const struct backlight_ops gpio_backlight_ops = { .check_fb = gpio_backlight_check_fb, }; +static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl) +{ + struct device_node *node = gbl->dev->of_node; + + /* If we absolutely want the backlight enabled at boot. */ + if (gbl->def_value) + return FB_BLANK_UNBLANK; + + /* If there's no panel to unblank the backlight later. */ + if (!node || !node->phandle) + return FB_BLANK_UNBLANK; + + return FB_BLANK_POWERDOWN; +} + static int gpio_backlight_probe_dt(struct platform_device *pdev, struct gpio_backlight *gbl) { @@ -142,7 +157,9 @@ static int gpio_backlight_probe(struct platform_device *pdev) return PTR_ERR(bl); } - bl->props.brightness = gbl->def_value; + bl->props.brightness = 1; + bl->props.power = gpio_backlight_initial_power_state(gbl); + backlight_update_status(bl); platform_set_drvdata(pdev, bl);
Re: [PATCH v1] backlight: gpio_backlight: Enable ACPI enumeration
On 19/06/2019 16:21, Andy Shevchenko wrote: ACPI allows to enumerate specific devices by using compatible strings. Enable that enumeration for GPIO based backlight devices. Signed-off-by: Andy Shevchenko --- drivers/video/backlight/gpio_backlight.c | 23 +-- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c index e470da95d806..05c12df62b27 100644 --- a/drivers/video/backlight/gpio_backlight.c +++ b/drivers/video/backlight/gpio_backlight.c @@ -18,6 +18,7 @@ #include #include #include +#include #include struct gpio_backlight { @@ -61,11 +62,10 @@ static int gpio_backlight_probe_dt(struct platform_device *pdev, struct gpio_backlight *gbl) { struct device *dev = &pdev->dev; - struct device_node *np = dev->of_node; enum gpiod_flags flags; int ret; - gbl->def_value = of_property_read_bool(np, "default-on"); + gbl->def_value = device_property_read_bool(dev, "default-on"); flags = gbl->def_value ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW; gbl->gpiod = devm_gpiod_get(dev, NULL, flags); @@ -89,26 +89,19 @@ static int gpio_backlight_probe(struct platform_device *pdev) struct backlight_properties props; struct backlight_device *bl; struct gpio_backlight *gbl; - struct device_node *np = pdev->dev.of_node; int ret; - if (!pdata && !np) { - dev_err(&pdev->dev, - "failed to find platform data or device tree node.\n"); - return -ENODEV; - } - gbl = devm_kzalloc(&pdev->dev, sizeof(*gbl), GFP_KERNEL); if (gbl == NULL) return -ENOMEM; gbl->dev = &pdev->dev; - if (np) { + if (pdev->dev.fwnode) { ret = gpio_backlight_probe_dt(pdev, gbl); if (ret) return ret; - } else { + } else if (pdata) { /* * Legacy platform data GPIO retrieveal. Do not expand * the use of this code path, currently only used by one @@ -129,6 +122,10 @@ static int gpio_backlight_probe(struct platform_device *pdev) gbl->gpiod = gpio_to_desc(pdata->gpio); if (!gbl->gpiod) return -EINVAL; + } else { + dev_err(&pdev->dev, + "failed to find platform data or device tree node.\n"); Should the string also be updated? If what is updated to acknoledge option to use ACPI then: Acked-by: Daniel Thompson + return -ENODEV; } memset(&props, 0, sizeof(props)); @@ -149,19 +146,17 @@ static int gpio_backlight_probe(struct platform_device *pdev) return 0; } -#ifdef CONFIG_OF static struct of_device_id gpio_backlight_of_match[] = { { .compatible = "gpio-backlight" }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, gpio_backlight_of_match); -#endif static struct platform_driver gpio_backlight_driver = { .driver = { .name = "gpio-backlight", - .of_match_table = of_match_ptr(gpio_backlight_of_match), + .of_match_table = gpio_backlight_of_match, }, .probe = gpio_backlight_probe, }; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v1] backlight: pwm_bl: convert to use SPDX identifier
On 19/06/2019 14:59, Andy Shevchenko wrote: Reduce size of duplicated comments by switching to use SPDX identifier. No functional change. While here, correct MODULE_LICENSE() string to be aligned with license text. Signed-off-by: Andy Shevchenko Acked-by: Daniel Thompson --- drivers/video/backlight/pwm_bl.c | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index fb45f866b923..1f7f8d5c0bf1 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -1,13 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 /* - * linux/drivers/video/backlight/pwm_bl.c - * - * simple PWM based backlight control, board code has to setup + * Simple PWM based backlight control, board code has to setup * 1) pin configuration so PWM waveforms can output * 2) platform_data being correctly configured - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. */ #include @@ -708,5 +703,5 @@ static struct platform_driver pwm_backlight_driver = { module_platform_driver(pwm_backlight_driver); MODULE_DESCRIPTION("PWM based Backlight Driver"); -MODULE_LICENSE("GPL"); +MODULE_LICENSE("GPL v2"); MODULE_ALIAS("platform:pwm-backlight"); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/2] DRM: Add KMS driver for the Ingenic JZ47xx SoCs
Le mer. 19 juin 2019 à 14:26, Sam Ravnborg a écrit : Hi Paul. On Mon, Jun 03, 2019 at 05:23:31PM +0200, Paul Cercueil wrote: Add a KMS driver for the Ingenic JZ47xx family of SoCs. This driver is meant to replace the aging jz4740-fb driver. This driver does not make use of the simple pipe helper, for the reason that it will soon be updated to support more advanced features like multiple planes, IPU integration for colorspace conversion and up/down scaling, support for DSI displays, and TV-out and HDMI outputs. Signed-off-by: Paul Cercueil Tested-by: Artur Rojek --- Notes: v2: - Remove custom handling of panel. The panel is now discovered using the standard API. - Lots of small tweaks suggested by upstream v3: - Use devm_drm_dev_init() - Update compatible strings to -lcd instead of -drm - Add destroy() callbacks to plane and crtc - The ingenic,lcd-mode is now read from the bridge's DT node v4: Remove ingenic,lcd-mode property completely. The various modes are now deduced from the connector type, the pixel format or the bus flags. v5: - Fix framebuffer size incorrectly calculated for 24bpp framebuffers - Use 32bpp framebuffer instead of 16bpp, as it'll work with both 16-bit and 24-bit panel - Get rid of drm_format_plane_cpp() which has been dropped upstream - Avoid using drm_format_info->depth, which is deprecated. In the drm world we include the revision notes in the changelog. So I did this when I applied it to drm-misc-next. Fixed a few trivial checkpatch warnings about indent too. There was a few too-long-lines warnings that I ignored. Fixing them would have hurt readability. Thanks. I assume you will maintain this driver onwards from now. Please request drm-misc commit rights (see https://www.freedesktop.org/wiki/AccountRequests/) You will need a legacy SSH account. I requested an account here: https://gitlab.freedesktop.org/freedesktop/freedesktop/issues/162 And you should familiarize yourself with the maintainer-tools: https://drm.pages.freedesktop.org/maintainer-tools/index.html For my use I use "dim update-branches; dim apply; dim push So only a small subset i needed for simple use. Sam
[PATCH 0/5] drm: Aspect ratio fixes
From: Ville Syrjälä Ilia pointed out some oddball crap in the i915 aspect ratio handling. While looking at that I noticed a bunch of fail in the core as well. This series aims to fix it all. Cc: Ilia Mirkin Ville Syrjälä (5): drm: Do not use bitwise OR to set picure_aspect_ratio drm: Do not accept garbage mode aspect ratio flags drm: WARN on illegal aspect ratio when converting a mode to umode drm/i915: Do not override mode's aspect ratio with the prop value NONE drm/i915: Drop redundant aspec ratio prop value initialization drivers/gpu/drm/drm_modes.c | 17 +++-- drivers/gpu/drm/i915/display/intel_hdmi.c | 5 ++--- drivers/gpu/drm/i915/display/intel_sdvo.c | 4 +--- 3 files changed, 14 insertions(+), 12 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm: Do not use bitwise OR to set picure_aspect_ratio
From: Ville Syrjälä enum hdmi_picture_aspect is not a bitmask, so don't use bitwise OR to populate it. Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_modes.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 57e6408288c8..53acc6756ee0 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1966,16 +1966,16 @@ int drm_mode_convert_umode(struct drm_device *dev, switch (in->flags & DRM_MODE_FLAG_PIC_AR_MASK) { case DRM_MODE_FLAG_PIC_AR_4_3: - out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_4_3; + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3; break; case DRM_MODE_FLAG_PIC_AR_16_9: - out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9; + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9; break; case DRM_MODE_FLAG_PIC_AR_64_27: - out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_64_27; + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_64_27; break; case DRM_MODE_FLAG_PIC_AR_256_135: - out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_256_135; + out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135; break; default: out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[GIT PULL] drm/imx: ipu-v3 image converter fixes
Hi Dave, Daniel, please consider merging these fixes for v5.2. regards Philipp The following changes since commit d1fdb6d8f6a4109a4263176c84b899076a5f8008: Linux 5.2-rc4 (2019-06-08 20:24:46 -0700) are available in the Git repository at: git://git.pengutronix.de/git/pza/linux.git tags/imx-drm-fixes-2019-06-20 for you to fetch changes up to 912bbf7e9ca422099935dd69d3ff0fd62db24882: gpu: ipu-v3: image-convert: Fix image downsize coefficients (2019-06-14 14:06:16 +0200) drm/imx: ipu-v3 image converter fixes This series fixes input buffer alignment and downsizer configuration to adhere to IPU mem2mem CSC/scaler hardware restrictions in certain downscaling ratios. Steve Longerbeam (3): gpu: ipu-v3: image-convert: Fix input bytesperline width/height align gpu: ipu-v3: image-convert: Fix input bytesperline for packed formats gpu: ipu-v3: image-convert: Fix image downsize coefficients drivers/gpu/ipu-v3/ipu-image-convert.c | 40 +++--- 1 file changed, 27 insertions(+), 13 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] drm: Do not accept garbage mode aspect ratio flags
From: Ville Syrjälä Don't let userspace feed us any old garbage in the mode aspect ratio flags. Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_modes.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 53acc6756ee0..847048dee048 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1977,9 +1977,11 @@ int drm_mode_convert_umode(struct drm_device *dev, case DRM_MODE_FLAG_PIC_AR_256_135: out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_256_135; break; - default: + case DRM_MODE_FLAG_PIC_AR_NONE: out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; break; + default: + return -EINVAL; } out->status = drm_mode_validate_driver(dev, out); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/5] drm: WARN on illegal aspect ratio when converting a mode to umode
From: Ville Syrjälä WARN if the incoming drm_display_mode has an illegal aspect ratio when converting it to a user mode. This should never happen unless the driver made a mistake and put an invalid value into the aspect ratio. Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_modes.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 847048dee048..be2ccd8eccfd 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1906,8 +1906,11 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, case HDMI_PICTURE_ASPECT_256_135: out->flags |= DRM_MODE_FLAG_PIC_AR_256_135; break; - case HDMI_PICTURE_ASPECT_RESERVED: default: + WARN(1, "Invalid aspect ratio (0%x) on mode\n", +in->picture_aspect_ratio); + /* fall through */ + case HDMI_PICTURE_ASPECT_NONE: out->flags |= DRM_MODE_FLAG_PIC_AR_NONE; break; } -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] drm/i915: Drop redundant aspec ratio prop value initialization
From: Ville Syrjälä HDMI_PICTURE_ASPECT_NONE is zero and the connector state is kzalloc()'d so no need to initialize conn_state->picture_aspect_ratio with it. Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_hdmi.c | 1 - drivers/gpu/drm/i915/display/intel_sdvo.c | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 6a4650b44ac6..f95f3db5ecb8 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2809,7 +2809,6 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c intel_attach_colorspace_property(connector); drm_connector_attach_content_type_property(connector); - connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) drm_object_attach_property(&connector->base, diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index 5cb619613157..c471fcce59f8 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -2624,7 +2624,6 @@ intel_sdvo_add_hdmi_properties(struct intel_sdvo *intel_sdvo, intel_attach_broadcast_rgb_property(&connector->base.base); } intel_attach_aspect_ratio_property(&connector->base.base); - connector->base.base.state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; } static struct intel_sdvo_connector *intel_sdvo_connector_alloc(void) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/i915: Do not override mode's aspect ratio with the prop value NONE
From: Ville Syrjälä HDMI_PICTURE_ASPECT_NONE means "Automatic" so when the user has that selected we should keep whatever aspect ratio the mode already has. Also no point in checking for connector->is_hdmi in the SDVO code since we only attach the property to HDMI connectors. Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/i915/display/intel_hdmi.c | 5 +++-- drivers/gpu/drm/i915/display/intel_sdvo.c | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 0ebec69bbbfc..6a4650b44ac6 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2394,8 +2394,9 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder, return -EINVAL; } - /* Set user selected PAR to incoming mode's member */ - adjusted_mode->picture_aspect_ratio = conn_state->picture_aspect_ratio; + if (conn_state->picture_aspect_ratio) + adjusted_mode->picture_aspect_ratio = + conn_state->picture_aspect_ratio; pipe_config->lane_count = 4; diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c index ceda03e5a3d4..5cb619613157 100644 --- a/drivers/gpu/drm/i915/display/intel_sdvo.c +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c @@ -1321,9 +1321,9 @@ static int intel_sdvo_compute_config(struct intel_encoder *encoder, if (IS_TV(intel_sdvo_connector)) i9xx_adjust_sdvo_tv_clock(pipe_config); - /* Set user selected PAR to incoming mode's member */ - if (intel_sdvo_connector->is_hdmi) - adjusted_mode->picture_aspect_ratio = conn_state->picture_aspect_ratio; + if (conn_state->picture_aspect_ratio) + adjusted_mode->picture_aspect_ratio = + conn_state->picture_aspect_ratio; if (!intel_sdvo_compute_avi_infoframe(intel_sdvo, pipe_config, conn_state)) { -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: dw-hdmi: Use automatic CTS generation mode when using non-AHB audio
Hi Andrzej, Gentle ping, do you think this could go in drm-misc-next for 5.3 ? Thanks, Neil On 12/06/2019 10:51, Neil Armstrong wrote: > When using an I2S source using a different clock source (usually the I2S > audio HW uses dedicated PLLs, different from the HDMI PHY PLL), fixed > CTS values will cause some frequent audio drop-out and glitches as > reported on Amlogic, Allwinner and Rockchip SoCs setups. > > Setting the CTS in automatic mode will let the HDMI controller generate > automatically the CTS value to match the input audio clock. > > The DesignWare DW-HDMI User Guide explains: > For Automatic CTS generation > Write "0" on the bit field "CTS_manual", Register 0x3205: AUD_CTS3 > > The DesignWare DW-HDMI Databook explains : > If "CTS_manual" bit equals 0b this registers contains "audCTS[19:0]" > generated by the Cycle time counter according to specified timing. > > Cc: Jernej Skrabec > Cc: Maxime Ripard > Cc: Jonas Karlman > Cc: Heiko Stuebner > Cc: Jerome Brunet > Signed-off-by: Neil Armstrong > --- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 44 +++ > 1 file changed, 29 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index c68b6ed1bb35..6458c3a31d23 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -437,8 +437,14 @@ static void hdmi_set_cts_n(struct dw_hdmi *hdmi, > unsigned int cts, > /* nshift factor = 0 */ > hdmi_modb(hdmi, 0, HDMI_AUD_CTS3_N_SHIFT_MASK, HDMI_AUD_CTS3); > > - hdmi_writeb(hdmi, ((cts >> 16) & HDMI_AUD_CTS3_AUDCTS19_16_MASK) | > - HDMI_AUD_CTS3_CTS_MANUAL, HDMI_AUD_CTS3); > + /* Use automatic CTS generation mode when CTS is not set */ > + if (cts) > + hdmi_writeb(hdmi, ((cts >> 16) & > +HDMI_AUD_CTS3_AUDCTS19_16_MASK) | > + HDMI_AUD_CTS3_CTS_MANUAL, > + HDMI_AUD_CTS3); > + else > + hdmi_writeb(hdmi, 0, HDMI_AUD_CTS3); > hdmi_writeb(hdmi, (cts >> 8) & 0xff, HDMI_AUD_CTS2); > hdmi_writeb(hdmi, cts & 0xff, HDMI_AUD_CTS1); > > @@ -508,24 +514,32 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi > *hdmi, > { > unsigned long ftdms = pixel_clk; > unsigned int n, cts; > + u8 config3; > u64 tmp; > > n = hdmi_compute_n(sample_rate, pixel_clk); > > - /* > - * Compute the CTS value from the N value. Note that CTS and N > - * can be up to 20 bits in total, so we need 64-bit math. Also > - * note that our TDMS clock is not fully accurate; it is accurate > - * to kHz. This can introduce an unnecessary remainder in the > - * calculation below, so we don't try to warn about that. > - */ > - tmp = (u64)ftdms * n; > - do_div(tmp, 128 * sample_rate); > - cts = tmp; > + config3 = hdmi_readb(hdmi, HDMI_CONFIG3_ID); > > - dev_dbg(hdmi->dev, "%s: fs=%uHz ftdms=%lu.%03luMHz N=%d cts=%d\n", > - __func__, sample_rate, ftdms / 100, (ftdms / 1000) % 1000, > - n, cts); > + /* Only compute CTS when using internal AHB audio */ > + if (config3 & HDMI_CONFIG3_AHBAUDDMA) { > + /* > + * Compute the CTS value from the N value. Note that CTS and N > + * can be up to 20 bits in total, so we need 64-bit math. Also > + * note that our TDMS clock is not fully accurate; it is > + * accurate to kHz. This can introduce an unnecessary remainder > + * in the calculation below, so we don't try to warn about that. > + */ > + tmp = (u64)ftdms * n; > + do_div(tmp, 128 * sample_rate); > + cts = tmp; > + > + dev_dbg(hdmi->dev, "%s: fs=%uHz ftdms=%lu.%03luMHz N=%d > cts=%d\n", > + __func__, sample_rate, > + ftdms / 100, (ftdms / 1000) % 1000, > + n, cts); > + } else > + cts = 0; > > spin_lock_irq(&hdmi->audio_lock); > hdmi->audio_n = n; >
Re: [PATCH 0/4] drm/bridge: dw-hdmi: Add support for HDR metadata
Hi Andrzej, Gentle ping, could you review the dw-hdmi changes here ? Thanks, Neil On 26/05/2019 23:18, Jonas Karlman wrote: > Add support for HDR metadata using the hdr_output_metadata connector property, > configure Dynamic Range and Mastering InfoFrame accordingly. > > A drm_infoframe flag is added to dw_hdmi_plat_data that platform drivers > can use to signal when Dynamic Range and Mastering infoframes is supported. > This flag is needed because Amlogic GXBB and GXL report same DW-HDMI version, > and only GXL support DRM InfoFrame. > > The first patch add functionality to configure DRM InfoFrame based on the > hdr_output_metadata connector property. > > The remaining patches sets the drm_infoframe flag on some SoCs supporting > Dynamic Range and Mastering InfoFrame. > > Note that this was based on top of drm-misc-next and Neil Armstrong's > "drm/meson: Add support for HDMI2.0 YUV420 4k60" series at [1] > > [1] https://patchwork.freedesktop.org/series/58725/#rev2 > > Jonas Karlman (4): > drm/bridge: dw-hdmi: Add Dynamic Range and Mastering InfoFrame support > drm/rockchip: Enable DRM InfoFrame support on RK3328 and RK3399 > drm/meson: Enable DRM InfoFrame support on GXL, GXM and G12A > drm/sun4i: Enable DRM InfoFrame support on H6 > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 109 > drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 37 +++ > drivers/gpu/drm/meson/meson_dw_hdmi.c | 5 + > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 2 + > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 2 + > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 1 + > include/drm/bridge/dw_hdmi.h| 1 + > 7 files changed, 157 insertions(+) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] backlight: gpio-backlight: Set power state instead of brightness at probe
Daniel, On 20/06/2019 16.56, Daniel Thompson wrote: > On 18/06/2019 13:58, Paul Kocialkowski wrote: >> Hi, >> >> On Fri, 2019-05-17 at 17:05 +0200, Paul Kocialkowski wrote: >>> On a trivial gpio-backlight setup with a panel using the backlight but >>> no boot software to enable it beforehand, we fall in a case where the >>> backlight is disabled (not just blanked) and thus remains disabled when >>> the panel gets enabled. >>> >>> Setting gbl->def_value via the device-tree prop allows enabling the >>> backlight in this situation, but it will be unblanked straight away, >>> in compliance with the binding. This does not work well when there >>> was no >>> boot software to display something before, since we really need to >>> unblank >>> by the time the panel is enabled, not before. >>> >>> Resolve the situation by setting the brightness to 1 at probe and >>> managing the power state accordingly, a bit like it's done in >>> pwm-backlight. >> >> Any feedback on this? I was under the impression that it could be quite >> controversial, as it implies that the backlight can no longer be >> enabled without a bound panel (which IMO makes good sense but could be >> a matter to debate). > > My apologies. This patch brought on such severe deja-vu I got rather > confused. Then when I went digging I've also dropped the ball on the > same feature previously. > > Peter Ujfalusi provided a similar patch to yours but with a slightly > different implementation: > https://lore.kernel.org/patchwork/patch/1002359/ > > On the whole I think it is important to read the GPIO pin since > otherwise we swap problems when there bootloader does setup the > backlight for problems where it does not. > > The thing I don't get is why both patches try to avoid setting the > backlight brightness from def_value. Simple displays, especially > monochrome ones are perfectly readable with the backlight off... zero > brightness is not a "bad" value. Because we might have non monochrome displays where the display is not readable when the backlight is off and for the sake of to be consistent? Flickering backlight is not really a nice thing during boot. > Not sure if Peter is still willing to rev his version of this code > (given how badly we neglected him previously) or whether you want to try > and combine both ideas. Nothing special, things sometimes got overlooked. I should have been nagging you about it ;) I think the v2 patch from me should apply just fine and it has the gpio read as well, if not, then I might not be able to resend as I'm out of office for a while - Péter > > > Daniel. > > >> >> Cheers, >> >> Paul >> >>> Fixes: 8b770e3c9824 ("backlight: Add GPIO-based backlight driver") >>> Signed-off-by: Paul Kocialkowski >>> --- >>> drivers/video/backlight/gpio_backlight.c | 19 ++- >>> 1 file changed, 18 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/video/backlight/gpio_backlight.c >>> b/drivers/video/backlight/gpio_backlight.c >>> index e470da95d806..c9cb97fa13d0 100644 >>> --- a/drivers/video/backlight/gpio_backlight.c >>> +++ b/drivers/video/backlight/gpio_backlight.c >>> @@ -57,6 +57,21 @@ static const struct backlight_ops >>> gpio_backlight_ops = { >>> .check_fb = gpio_backlight_check_fb, >>> }; >>> +static int gpio_backlight_initial_power_state(struct >>> gpio_backlight *gbl) >>> +{ >>> + struct device_node *node = gbl->dev->of_node; >>> + >>> + /* If we absolutely want the backlight enabled at boot. */ >>> + if (gbl->def_value) >>> + return FB_BLANK_UNBLANK; >>> + >>> + /* If there's no panel to unblank the backlight later. */ >>> + if (!node || !node->phandle) >>> + return FB_BLANK_UNBLANK; >>> + >>> + return FB_BLANK_POWERDOWN; >>> +} >>> + >>> static int gpio_backlight_probe_dt(struct platform_device *pdev, >>> struct gpio_backlight *gbl) >>> { >>> @@ -142,7 +157,9 @@ static int gpio_backlight_probe(struct >>> platform_device *pdev) >>> return PTR_ERR(bl); >>> } >>> - bl->props.brightness = gbl->def_value; >>> + bl->props.brightness = 1; >>> + bl->props.power = gpio_backlight_initial_power_state(gbl); >>> + >>> backlight_update_status(bl); >>> platform_set_drvdata(pdev, bl); > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/self_refresh: Fix possible NULL deref in failure path
On Thu, Jun 20, 2019 at 01:28:55PM +0200, Daniel Vetter wrote: > On Wed, Jun 19, 2019 at 02:19:47PM -0400, Sean Paul wrote: > > From: Sean Paul > > > > If state allocation fails, we still try to give back the reference on > > it. Also initialize ret in case the crtc is not enabled and we hit the > > eject button. > > > > Fixes: 1452c25b0e60 ("drm: Add helpers to kick off self refresh mode in > > drivers") > > Cc: Daniel Vetter > > Cc: Jose Souza > > Cc: Zain Wang > > Cc: Tomasz Figa > > Cc: Ville Syrjälä > > Cc: Sam Ravnborg > > Cc: Sean Paul > > Cc: Maarten Lankhorst > > Cc: Maxime Ripard > > Cc: Sean Paul > > Cc: David Airlie > > Cc: dri-devel@lists.freedesktop.org > > Reported-by: Dan Carpenter > > Signed-off-by: Sean Paul > > Reviewed-by: Daniel Vetter > Applied to -misc-next, thanks! Sean > > --- > > drivers/gpu/drm/drm_self_refresh_helper.c | 6 -- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_self_refresh_helper.c > > b/drivers/gpu/drm/drm_self_refresh_helper.c > > index e0d2ad1f070cb..4b9424a8f1f1c 100644 > > --- a/drivers/gpu/drm/drm_self_refresh_helper.c > > +++ b/drivers/gpu/drm/drm_self_refresh_helper.c > > @@ -69,14 +69,14 @@ static void drm_self_refresh_helper_entry_work(struct > > work_struct *work) > > struct drm_connector *conn; > > struct drm_connector_state *conn_state; > > struct drm_crtc_state *crtc_state; > > - int i, ret; > > + int i, ret = 0; > > > > drm_modeset_acquire_init(&ctx, 0); > > > > state = drm_atomic_state_alloc(dev); > > if (!state) { > > ret = -ENOMEM; > > - goto out; > > + goto out_drop_locks; > > } > > > > retry: > > @@ -116,6 +116,8 @@ static void drm_self_refresh_helper_entry_work(struct > > work_struct *work) > > } > > > > drm_atomic_state_put(state); > > + > > +out_drop_locks: > > drm_modeset_drop_locks(&ctx); > > drm_modeset_acquire_fini(&ctx); > > } > > -- > > Sean Paul, Software Engineer, Google / Chromium OS > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 5/5 v3] drm/msm/mdp5: Use the interconnect API
On Tue, Jun 18, 2019 at 4:10 PM Rob Clark wrote: > > From: Georgi Djakov > > The interconnect API provides an interface for consumer drivers to > express their bandwidth needs in the SoC. This data is aggregated > and the on-chip interconnect hardware is configured to the most > appropriate power/performance profile. > > Use the API to configure the interconnects and request bandwidth > between DDR and the display hardware (MDP port(s) and rotator > downscaler). > > v2: update the path names to be consistent with dpu, handle the NULL > path case, updated commit msg from Georgi. > v3: split out icc setup into it's own function, and rework logic > slightly so no interconnect paths is not fatal. > > Signed-off-by: Georgi Djakov > Signed-off-by: Rob Clark Looks good to me. Reviewed-By: Jeffrey Hugo
Re: drm connectors, tegra, and the web they weave (was Re: [PATCH 58/59] drm/todo: Add new debugfs todo)
On Tue, Jun 18, 2019 at 11:46:56PM +0200, Daniel Vetter wrote: > On Tue, Jun 18, 2019 at 08:01:13PM +0200, Greg Kroah-Hartman wrote: > > On Tue, Jun 18, 2019 at 07:32:20PM +0200, Daniel Vetter wrote: > > > On Tue, Jun 18, 2019 at 5:25 PM Greg Kroah-Hartman > > > wrote: > > > > On Tue, Jun 18, 2019 at 05:19:38PM +0200, Greg Kroah-Hartman wrote: > > > > > I could just "open code" a bunch of calls to debugfs_create_file() for > > > > > these drivers, which would solve this issue, but in a more "non-drm" > > > > > way. Is it worth to just do that instead of overthinking the whole > > > > > thing and trying to squish it into the drm "model" of drm debugfs > > > > > calls? > > > > > > > > An example of "open coding" this is the patch below for the sor.c > > > > driver. > > > > > > I think open-coding is the way to go here. One of the todos is to > > > extend debugfs support for crtc/connectors, but looking at the > > > open-coded version we really don't need a drm-flavoured midlayer here. > > > > There already is debugfs support in the code for crtc/connectors, these > > files are "hanging" off of those locations already. I'll keep that, but > > indent it one more directory so that there's no namespace collisions. > > The todo was to have some drm wrappers here for the boilerplate, but after > looking at your version that's not a good idea. So not just making sure > crtcs/connectors have a debugfs directory made for them, but more. > > Wrt adding a new directory: debugfs isnt uapi, but there's usually a > massive pile of script relying on them, so it's not nice to shuffle paths > around. Plus the lifetimes match anyway (at least if you don't hotplug > connectors, which tegra doesn't do). So, I think you two already covered everything. From a Tegra perspective there's not really a need to care about the exact structure of debugfs because there are only a handful of scripts that use this and they are not exactly widely distributed. At the same time there's really no need to add another level of directories, since the connector really is the SOR, so an sor directory in the connector's directory is just redundant. Cleaning up and lifetime management aren't issues, really, so there are no good arguments for adding it, in my opinion. Historically, the sor.c driver is interesting because it used to be just plain debugfs calls. Only when I added a second debugfs file I decided to go with the built-in DRM infrastructure for this and then later went and converted the first file to it as well, for consistency. I do remember though that I was very unhappy about the fact that I had to do all this kmemdup()'ing just to make debugfs files per-instance (the DRM infrastructure doesn't allow that by default). In retrospect that was a poor decision and I should've just stuck with debugfs and perhaps just spin a couple of helpers around that instead. So I'm happy to see this effort. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 1/4] dt-bindings: display: Convert common panel bindings to DT schema
On Thu, Jun 20, 2019 at 3:01 AM Thierry Reding wrote: > > On Wed, Jun 19, 2019 at 03:51:53PM -0600, Rob Herring wrote: > > Convert the common panel bindings to DT schema consolidating scattered > > definitions to a single schema file. > > > > The 'simple-panel' binding just a collection of properties and not a > > complete binding itself. All of the 'simple-panel' properties are > > covered by the panel-common.txt binding with the exception of the > > 'no-hpd' property, so add that to the schema. > > > > As there are lots of references to simple-panel.txt, just keep the file > > with a reference to panel-common.yaml for now until all the bindings are > > converted. > > > > Cc: Thierry Reding > > Cc: Sam Ravnborg > > Cc: Maxime Ripard > > Cc: Laurent Pinchart > > Cc: dri-devel@lists.freedesktop.org > > Signed-off-by: Rob Herring > > --- > > Note there's still some references to panel-common.txt that I need to > > update or just go ahead and convert to schema. > > > > .../bindings/display/panel/panel-common.txt | 101 - > > .../bindings/display/panel/panel-common.yaml | 143 ++ > > .../bindings/display/panel/panel.txt | 4 - > > .../bindings/display/panel/simple-panel.txt | 29 +--- > > 4 files changed, 144 insertions(+), 133 deletions(-) > > delete mode 100644 > > Documentation/devicetree/bindings/display/panel/panel-common.txt > > create mode 100644 > > Documentation/devicetree/bindings/display/panel/panel-common.yaml > > I know it was this way before, but perhaps remove the redundant panel- > prefix while at it? Sure. > > diff --git > > a/Documentation/devicetree/bindings/display/panel/panel-common.yaml > > b/Documentation/devicetree/bindings/display/panel/panel-common.yaml > > new file mode 100644 > > index ..6fe87254edad > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/panel/panel-common.yaml > > @@ -0,0 +1,143 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/display/panel/panel-common.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Common Properties for Display Panels > > + > > +maintainers: > > + - Thierry Reding > > + - Laurent Pinchart > > + > > +description: | > > + This document defines device tree properties common to several classes of > > + display panels. It doesn't constitue a device tree binding specification > > by > > + itself but is meant to be referenced by device tree bindings. > > + > > + When referenced from panel device tree bindings the properties defined > > in this > > + document are defined as follows. The panel device tree bindings are > > + responsible for defining whether each property is required or optional. > > + > > + > > Are the two blank lines here on purpose? No. > The original document had two > blank lines here, but that was mostly for readability I would guess. The > YAML format doesn't really need additional formatting for readability, > so perhaps just remove the extra blank line? > > > +properties: > > + # Descriptive Properties > > + width-mm: > > +description: The width-mm and height-mm specify the width and height > > of the > > + physical area where images are displayed. These properties are > > expressed > > + in millimeters and rounded to the closest unit. > > + > > + height-mm: > > +description: The width-mm and height-mm specify the width and height > > of the > > + physical area where images are displayed. These properties are > > expressed > > + in millimeters and rounded to the closest unit. > > I suppose there's no way in YAML to share the description between both > the width-mm and height-mm properties? It's a little unfortunate that we > have to copy, but if there's no better way, guess we'll have to live > with it. I could make it a comment instead, but then we loose being able to parse it. I should probably just reword them to be separate: "Specifies the height of the physical area where images are displayed. The property is expressed in millimeters and rounded to the closest unit." Also, just realized I need to make these 2 dependencies on either other (i.e. not valid to only have one). > > + label: > > +description: | > > + The label property specifies a symbolic name for the panel as a > > + string suitable for use by humans. It typically contains a name > > inscribed > > + on the system (e.g. as an affixed label) or specified in the system's > > + documentation (e.g. in the user's manual). > > + > > + If no such name exists, and unless the property is mandatory > > according to > > + device tree bindings, it shall rather be omitted than constructed of > > + non-descriptive information. For instance an LCD panel in a system > > that > > + contains a single panel shall not be labelled "LCD" if that name is > > not > > + inscribed on the system or used in a
Re: drm connectors, tegra, and the web they weave (was Re: [PATCH 58/59] drm/todo: Add new debugfs todo)
On Tue, Jun 18, 2019 at 05:19:38PM +0200, Greg Kroah-Hartman wrote: > On Fri, Jun 14, 2019 at 10:36:14PM +0200, Daniel Vetter wrote: > > Greg is busy already, but maybe he won't do everything ... > > > > Cc: Greg Kroah-Hartman > > Signed-off-by: Daniel Vetter > > --- > > Documentation/gpu/todo.rst | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > index 9717540ee28f..026e55c517e1 100644 > > --- a/Documentation/gpu/todo.rst > > +++ b/Documentation/gpu/todo.rst > > @@ -375,6 +375,9 @@ There's a bunch of issues with it: > >this (together with the drm_minor->drm_device move) would allow us to > > remove > >debugfs_init. > > > > +- Drop the return code and error checking from all debugfs functions. Greg > > KH is > > + working on this already. > > > Part of this work was to try to delete drm_debugfs_remove_files(). > > There are only 4 files that currently still call this function: > drivers/gpu/drm/tegra/dc.c > drivers/gpu/drm/tegra/dsi.c > drivers/gpu/drm/tegra/hdmi.c > drivers/gpu/drm/tegra/sor.c > > For dc.c, the driver wants to add debugfs files to the struct drm_crtc > debugfs directory. Which is fine, but it has to do some special memory > allocation to get the debugfs callback to point not to the struct > drm_minor pointer, but rather the drm_crtc structure. Actually the reason why the memory allocation is done is because there can be multiple instances of the display controller. In fact, there's always at least two (and up to four in later Tegra generations). The DRM debugfs infrastructure, however, doesn't automatically duplicate the data for each drm_debugfs_create_files() call and at the same time it does not allow you to specify driver-private data other than by embedding it in the drm_info_list structure. So rather than manually create the drm_info_list for each display controller instance, the code creates a template that is then duplicated and for which the driver- private is then set. That way multiple invocations end up with different data. This is because of the extra indirection that the DRM debugfs infrastructure introduces. It's in fact much easier to do this with just plain debugfs function calls. The only downside is the boilerplate required to make that happen. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] drm/stm: drv: fix suspend/resume
Le mar. 18 juin 2019 à 11:57, Philippe CORNU a écrit : > > Hi Yannick, > > Thank you for your patch. > > Acked-by: Philippe Cornu I have corrected Fixes sha1 (should be 12 digits) Applied on drm-misc-next. Benjamin > > Philippe :-) > > On 6/17/19 9:18 AM, Yannick Fertré wrote: > > Without this fix, the system can not go in "suspend" mode > > due to an error in drv_suspend function. > > > > Fixes: 35ab6cf ("drm/stm: support runtime power management") > > > > Signed-off-by: Yannick Fertré > > --- > > drivers/gpu/drm/stm/drv.c | 15 --- > > 1 file changed, 8 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c > > index 5659572..9dee4e4 100644 > > --- a/drivers/gpu/drm/stm/drv.c > > +++ b/drivers/gpu/drm/stm/drv.c > > @@ -136,8 +136,7 @@ static __maybe_unused int drv_suspend(struct device > > *dev) > > struct ltdc_device *ldev = ddev->dev_private; > > struct drm_atomic_state *state; > > > > - if (WARN_ON(!ldev->suspend_state)) > > - return -ENOENT; > > + WARN_ON(ldev->suspend_state); > > > > state = drm_atomic_helper_suspend(ddev); > > if (IS_ERR(state)) > > @@ -155,15 +154,17 @@ static __maybe_unused int drv_resume(struct device > > *dev) > > struct ltdc_device *ldev = ddev->dev_private; > > int ret; > > > > + if (WARN_ON(!ldev->suspend_state)) > > + return -ENOENT; > > + > > pm_runtime_force_resume(dev); > > ret = drm_atomic_helper_resume(ddev, ldev->suspend_state); > > - if (ret) { > > + if (ret) > > pm_runtime_force_suspend(dev); > > - ldev->suspend_state = NULL; > > - return ret; > > - } > > > > - return 0; > > + ldev->suspend_state = NULL; > > + > > + return ret; > > } > > > > static __maybe_unused int drv_runtime_suspend(struct device *dev) > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110949] Continuious warnings from agd5f 5.3-wip branch
https://bugs.freedesktop.org/show_bug.cgi?id=110949 --- Comment #4 from Mike Lothian --- If there's anything you'd like me to test for you, please do shout -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 29/59] drm/sti: Drop drm_gem_prime_export/import
Le ven. 14 juin 2019 à 22:36, Daniel Vetter a écrit : > > They're the default. > > Aside: Would be really nice to switch the others over to > drm_gem_object_funcs. > > Signed-off-by: Daniel Vetter > Cc: Benjamin Gaignard > Cc: Vincent Abriou Thanks, Reviewed-by: Benjamin Gaignard > --- > drivers/gpu/drm/sti/sti_drv.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c > index d9f63c9f287b..faea4dcb21b1 100644 > --- a/drivers/gpu/drm/sti/sti_drv.c > +++ b/drivers/gpu/drm/sti/sti_drv.c > @@ -152,8 +152,6 @@ static struct drm_driver sti_driver = { > > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > - .gem_prime_export = drm_gem_prime_export, > - .gem_prime_import = drm_gem_prime_import, > .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, > .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, > .gem_prime_vmap = drm_gem_cma_prime_vmap, > -- > 2.20.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm connectors, tegra, and the web they weave (was Re: [PATCH 58/59] drm/todo: Add new debugfs todo)
On Tue, Jun 18, 2019 at 07:32:20PM +0200, Daniel Vetter wrote: > On Tue, Jun 18, 2019 at 5:25 PM Greg Kroah-Hartman > wrote: > > On Tue, Jun 18, 2019 at 05:19:38PM +0200, Greg Kroah-Hartman wrote: > > > On Fri, Jun 14, 2019 at 10:36:14PM +0200, Daniel Vetter wrote: > > > > Greg is busy already, but maybe he won't do everything ... > > > > > > > > Cc: Greg Kroah-Hartman > > > > Signed-off-by: Daniel Vetter > > > > --- > > > > Documentation/gpu/todo.rst | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > > > > index 9717540ee28f..026e55c517e1 100644 > > > > --- a/Documentation/gpu/todo.rst > > > > +++ b/Documentation/gpu/todo.rst > > > > @@ -375,6 +375,9 @@ There's a bunch of issues with it: > > > >this (together with the drm_minor->drm_device move) would allow us > > > > to remove > > > >debugfs_init. > > > > > > > > +- Drop the return code and error checking from all debugfs functions. > > > > Greg KH is > > > > + working on this already. > > > > > > > > > Part of this work was to try to delete drm_debugfs_remove_files(). > > > > > > There are only 4 files that currently still call this function: > > > drivers/gpu/drm/tegra/dc.c > > > drivers/gpu/drm/tegra/dsi.c > > > drivers/gpu/drm/tegra/hdmi.c > > > drivers/gpu/drm/tegra/sor.c > > > > > > For dc.c, the driver wants to add debugfs files to the struct drm_crtc > > > debugfs directory. Which is fine, but it has to do some special memory > > > allocation to get the debugfs callback to point not to the struct > > > drm_minor pointer, but rather the drm_crtc structure. > > There's already a todo to switch the drm_minor debugfs stuff over to > drm_device. drm_minor is essentially different uapi flavours (/dev/ > minor nodes, hence the name) sitting on top of the same drm_device. > Last time I checked all the debugfs files want the drm_device, not the > minor. I think we even discussed to only register the debugfs files > for the first minor, and create the other ones as symlinks to the > first one. But haven't yet gotten around to typing that. > > drm_crtc/connector are parts of drm_device with modesetting support, > so the drm_minor is even worse choice really. For the connector drivers we already sit on top of the per-connector debugfs directories. I think the only reason why we don't do that for the display controller is because drm_crtc didn't have built-in debugfs support like the connectors have. It looks like that's no longer true, though it's been there for a while. I think it'd be good to just move those over as well. As for passing struct drm_minor, I think that's mostly unnecessary. As far as I can tell, we only use drm_minor to get at drm_device, which in turn we only use to check some features flags, and drm_minor itself is only used to track the list of files that are being added so that they can later be removed again. Given that we can just tear down everything debugfs recursively, I don't think we need any of that. > > Not exactly sure why we went with this, but probably dates back to the > *bsd compat layer and a lot of these files hanging out in procfs too > (we've fixed those mistakes a few years ago, yay!). > > > > So, to remove this call, I need to remove this special memory allocation > > > and to do that, I need to somehow be able to cast from drm_minor back to > > > the drm_crtc structure being used in this driver. And I can't figure > > > how they are related at all. > > > > > > Any pointers here (pun intended) would be appreciated. > > > > > > For the other 3 files, the situation is much the same, but I need to get > > > from a 'struct drm_minor' pointer to a 'struct drm_connector' pointer. > > Ditch the drm_minor, there's no no way to get from that to something > like drm_connector/crtc, since it's a n:m relationship. Yeah. That too. > > > I could just "open code" a bunch of calls to debugfs_create_file() for > > > these drivers, which would solve this issue, but in a more "non-drm" > > > way. Is it worth to just do that instead of overthinking the whole > > > thing and trying to squish it into the drm "model" of drm debugfs calls? > > > > An example of "open coding" this is the patch below for the sor.c > > driver. > > I think open-coding is the way to go here. One of the todos is to > extend debugfs support for crtc/connectors, but looking at the > open-coded version we really don't need a drm-flavoured midlayer here. Exactly my thoughts. It'd be nice to have some sort of macro to help bring the boilerplate down a bit. Thierry > > Totally untested, not even built, but you should get the idea here. > > > > thanks, > > > > greg k-h > > > > --- > > > > diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c > > index 5be5a0817dfe..3216221c77c4 100644 > > --- a/drivers/gpu/drm/tegra/sor.c > > +++ b/drivers/gpu/drm/tegra/sor.c > > @@ -414,7 +414,8 @@ struct tegra_sor { >
Re: [RFC PATCH 1/4] dt-bindings: display: Convert common panel bindings to DT schema
On Thu, Jun 20, 2019 at 12:55 AM Sam Ravnborg wrote: > > Hi Rob. > > Thanks for starting the conversion of panel bindings to yaml. > > On Wed, Jun 19, 2019 at 03:51:53PM -0600, Rob Herring wrote: > > Convert the common panel bindings to DT schema consolidating scattered > > definitions to a single schema file. > > > > The 'simple-panel' binding just a collection of properties and not a > > complete binding itself. All of the 'simple-panel' properties are > > covered by the panel-common.txt binding with the exception of the > > 'no-hpd' property, so add that to the schema. > > > > As there are lots of references to simple-panel.txt, just keep the file > > with a reference to panel-common.yaml for now until all the bindings are > > converted. > Good idea. > > > > > Cc: Thierry Reding > > Cc: Sam Ravnborg > > Cc: Maxime Ripard > > Cc: Laurent Pinchart > > Cc: dri-devel@lists.freedesktop.org > > Signed-off-by: Rob Herring > > --- > > Note there's still some references to panel-common.txt that I need to > > update or just go ahead and convert to schema. > Better let it point to the .yaml variant, so this patchset does not > depend on too much other bindings to be converted. There's only 8 files referencing panel-common.txt which was why I was debating just converting all of them. > Then we can start the conversion of the remaining panel bindings. > Any tooling that helps the conversions? I have a doc2yaml script that helps with some of the boilerplate. It's in my yaml-bindings-v2 branch[1]. > When this hits upstream I assume all future panel bindings shall be yaml > based - so we have a few pending contributions that need to do something. That would be ideal, but not strictly required. For pending things, no reason to make folks redo things. Requiring schema really depends on whomever is applying things to run at least 'make dt_binding_check' before accepting. > > For the actual conversion below: > Acked-by: Sam Ravnborg Thanks. Rob [1] https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/log/?h=yaml-bindings-v2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm connectors, tegra, and the web they weave (was Re: [PATCH 58/59] drm/todo: Add new debugfs todo)
On Tue, Jun 18, 2019 at 04:37:16PM +0100, Jon Hunter wrote: > > On 18/06/2019 16:19, Greg Kroah-Hartman wrote: > > On Fri, Jun 14, 2019 at 10:36:14PM +0200, Daniel Vetter wrote: > >> Greg is busy already, but maybe he won't do everything ... > >> > >> Cc: Greg Kroah-Hartman > >> Signed-off-by: Daniel Vetter > >> --- > >> Documentation/gpu/todo.rst | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > >> index 9717540ee28f..026e55c517e1 100644 > >> --- a/Documentation/gpu/todo.rst > >> +++ b/Documentation/gpu/todo.rst > >> @@ -375,6 +375,9 @@ There's a bunch of issues with it: > >>this (together with the drm_minor->drm_device move) would allow us to > >> remove > >>debugfs_init. > >> > >> +- Drop the return code and error checking from all debugfs functions. > >> Greg KH is > >> + working on this already. > > > > > > Part of this work was to try to delete drm_debugfs_remove_files(). > > > > There are only 4 files that currently still call this function: > > drivers/gpu/drm/tegra/dc.c > > drivers/gpu/drm/tegra/dsi.c > > drivers/gpu/drm/tegra/hdmi.c > > drivers/gpu/drm/tegra/sor.c > > > > For dc.c, the driver wants to add debugfs files to the struct drm_crtc > > debugfs directory. Which is fine, but it has to do some special memory > > allocation to get the debugfs callback to point not to the struct > > drm_minor pointer, but rather the drm_crtc structure. > > > > So, to remove this call, I need to remove this special memory allocation > > and to do that, I need to somehow be able to cast from drm_minor back to > > the drm_crtc structure being used in this driver. And I can't figure > > how they are related at all. > > > > Any pointers here (pun intended) would be appreciated. > > > > For the other 3 files, the situation is much the same, but I need to get > > from a 'struct drm_minor' pointer to a 'struct drm_connector' pointer. > > > > I could just "open code" a bunch of calls to debugfs_create_file() for > > these drivers, which would solve this issue, but in a more "non-drm" > > way. Is it worth to just do that instead of overthinking the whole > > thing and trying to squish it into the drm "model" of drm debugfs calls? > > > > Either way, who can test these changes? I can't even build the tegra > > driver without digging up an arm64 cross-compiler, and can't test it as > > I have no hardware at all. > > We can definitely compile and boot test these no problem. In fact > anything that lands in -next we will boot test. However, I can do some > quick sanity if you have something to test. > > Thierry may have more specific Tegra DRM tests. We don't have any automated tests for this yet, unfortunately. Let me work on something. In the meantime I can manually test any of the patches that Greg sends out. These should be fairly trivial to test. It's difficult to check for success/failure on something like the register dump or the CRC, but I think for now we don't really need much more than just validating that things don't crash when we read one of these debugfs files. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next
Hi Daniel, Dave, Final pull request for drm-misc-next! Biggest changes are the remove-fbcon-notifiers branch and modeline cmdline parser rework, and the addition of a new KMS driver for ingenic. drm-misc-next-2019-06-20: drm-misc-next for v5.3: UAPI Changes: - Give each dma-buf their own inode, add DMA_BUF_SET_NAME ioctl and a show_fdinfo handler. Cross-subsystem Changes: - Pull in the topic/remove-fbcon-notifiers branch: * remove fbdev notifier usage for fbcon, as prep work to clean up the fbcon locking * assorted locking checks in vt/console code * assorted notifier and cleanups in fbdev and backlight code Core Changes: - Make drm_debugfs_create_files() never fail. - add debug print to update_vblank_count. - Add DP_DPCD_QUIRK_NO_SINK_COUNT quirk. - Add todo item for drm_gem_objects. - Unexport drm_gem_(un)pin/v(un)map. - Document struct drm_cmdline_mode. - Rewrite the command handler for mode names, and add support to specify rotation, reflection and overscan. With a new selftest! :) - Fixes to drm/client for improving rotation support, and fixing variable scope. - Small fixes to self refresh helper. Driver Changes: - Add rockchip RK3328 support. - Assorted driver fixes to rockchip, vc4, rcar-du, vkms. - Expose panfrost performance counters through unstable ioctl's, hidden behind a module parameter. - Enumerate CRC sources list in vkms. - Add a basic kms driver for the Ingenic JZ47xx SoC, which will be expanded soon with more advanced features. - Suspend/resume fix for stm. The following changes since commit 52d2d44eee8091e740d0d275df1311fb8373c9a9: Merge v5.2-rc5 into drm-next (2019-06-19 12:07:29 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2019-06-20 for you to fetch changes up to 836334fd747595331dcdc7709b447ad8134db693: drm/todo: Update drm_gem_object_funcs todo even more (2019-06-20 17:11:53 +0200) Boris Brezillon (4): drm/panfrost: Move gpu_{write, read}() macros to panfrost_regs.h drm/panfrost: Add a module parameter to expose unstable ioctls drm/panfrost: Add an helper to check the GPU generation drm/panfrost: Expose performance counters through unstable ioctls Dan Carpenter (1): drm: self_refresh: Fix a reversed condition in drm_self_refresh_helper_cleanup() Daniel Vetter (38): dummycon: Sprinkle locking checks fbdev: locking check for fb_set_suspend vt: might_sleep() annotation for do_blank_screen vt: More locking checks fbdev/sa1100fb: Remove dead code fbdev/cyber2000: Remove struct display fbdev/aty128fb: Remove dead code fbcon: s/struct display/struct fbcon_display/ fbcon: Remove fbcon_has_exited fbcon: call fbcon_fb_(un)registered directly fbdev/sh_mobile: remove sh_mobile_lcdc_display_notify fbdev/omap: sysfs files can't disappear before the device is gone fbdev: sysfs files can't disappear before the device is gone staging/olpc: lock_fb_info can't fail fbdev/atyfb: lock_fb_info can't fail fbdev: lock_fb_info cannot fail fbcon: call fbcon_fb_bind directly fbdev: make unregister/unlink functions not fail fbdev: unify unlink_framebuffer paths fbdev/sh_mob: Remove fb notifier callback fbdev: directly call fbcon_suspended/resumed fbcon: Call fbcon_mode_deleted/new_modelist directly fbdev: Call fbcon_get_requirement directly Revert "backlight/fbcon: Add FB_EVENT_CONBLANK" fbmem: pull fbcon_fb_blanked out of fb_blank fbdev: remove FBINFO_MISC_USEREVENT around fb_blank fb: Flatten control flow in fb_set_var fbcon: replace FB_EVENT_MODE_CHANGE/_ALL with direct calls vgaswitcheroo: call fbcon_remap_all directly fbcon: Call con2fb_map functions directly fbcon: Document what I learned about fbcon locking staging/olpc_dcon: Add drm conversion to TODO backlight: simplify lcd notifier drm/todo: Improve drm_gem_object funcs todo drm/gem: Unexport drm_gem_(un)pin/v(un)map drm/vkms: Move format arrays to vkms_plane.c fbcon: Export fbcon_update_vcs drm/todo: Update drm_gem_object_funcs todo even more Douglas Anderson (2): drm/rockchip: Properly adjust to a true clock in adjusted_mode drm/rockchip: Base adjustments of the mode based on prev adjustments Greg Hackmann (3): dma-buf: give each buffer a full-fledged inode dma-buf: add DMA_BUF_SET_NAME ioctls dma-buf: add show_fdinfo handler Greg Kroah-Hartman (2): drm: debugfs: make drm_debugfs_create_files() never fail drm/vc4: no need to check return value of debugfs_create functions Justin Swartz (1): drm/rockchip: dw_hdmi: add basic rk3228 support Maarten Lankhorst (3): Merge remote-tracking branch 'drm/drm-next' into drm-misc-next Merge remote-tracking branch 'drm/drm-next'
[PATCH] drm: Dump mode picture aspect ratio
From: Ville Syrjälä Currently the logs show nothing about the mode's aspect ratio. Include that information in the normal mode dump. Cc: Ilia Mirkin Signed-off-by: Ville Syrjälä --- drivers/video/hdmi.c| 3 ++- include/drm/drm_modes.h | 4 ++-- include/linux/hdmi.h| 3 +++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index b939bc28d886..bc593fe1c268 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -1057,7 +1057,7 @@ static const char *hdmi_colorimetry_get_name(enum hdmi_colorimetry colorimetry) return "Invalid"; } -static const char * +const char * hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect) { switch (picture_aspect) { @@ -1076,6 +1076,7 @@ hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect) } return "Invalid"; } +EXPORT_SYMBOL(hdmi_picture_aspect_get_name); static const char * hdmi_active_aspect_get_name(enum hdmi_active_aspect active_aspect) diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 083f16747369..2b1809c74fbe 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -431,7 +431,7 @@ struct drm_display_mode { /** * DRM_MODE_FMT - printf string for &struct drm_display_mode */ -#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x" +#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x %s" /** * DRM_MODE_ARG - printf arguments for &struct drm_display_mode @@ -441,7 +441,7 @@ struct drm_display_mode { (m)->name, (m)->vrefresh, (m)->clock, \ (m)->hdisplay, (m)->hsync_start, (m)->hsync_end, (m)->htotal, \ (m)->vdisplay, (m)->vsync_start, (m)->vsync_end, (m)->vtotal, \ - (m)->type, (m)->flags + (m)->type, (m)->flags, hdmi_picture_aspect_get_name((m)->picture_aspect_ratio) #define obj_to_mode(x) container_of(x, struct drm_display_mode, base) diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index 9918a6c910c5..de7cbe385dba 100644 --- a/include/linux/hdmi.h +++ b/include/linux/hdmi.h @@ -434,4 +434,7 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame, void hdmi_infoframe_log(const char *level, struct device *dev, const union hdmi_infoframe *frame); +const char * +hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect); + #endif /* _DRM_HDMI_H */ -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v7 5/6] drm: sun4i: Add support for enabling DDC I2C bus to sun8i_dw_hdmi glue
Dne četrtek, 20. junij 2019 ob 15:47:47 CEST je megous via linux-sunxi napisal(a): > From: Ondrej Jirman > > Orange Pi 3 board requires enabling a voltage shifting circuit via GPIO > for the DDC bus to be usable. > > Add support for hdmi-connector node's optional ddc-en-gpios property to > support this use case. > > Signed-off-by: Ondrej Jirman Reviewed-by: Jernej Skrabec Best regards, Jernej
Re: [linux-sunxi] [PATCH v7 0/6] Add support for Orange Pi 3
Hi! Dne četrtek, 20. junij 2019 ob 15:47:42 CEST je megous via linux-sunxi napisal(a): > From: Ondrej Jirman > > This series implements support for Xunlong Orange Pi 3 board. > > - ethernet support (patches 1-3) Correct me if I'm wrong, but patches 1-2 aren't strictly necessary for OrangePi 3, right? H6 DTSI already has emac node with dual compatible (H6 and A64) and since OrangePi 3 uses gigabit ethernet, quirk introduced by patches 1-2 are not needed. However, it is nice to have this 100 Mbit fix, because most STB DTS will need it. Best regards, Jernej > - HDMI support (patches 4-6) > > For some people, ethernet doesn't work after reboot (but works on cold > boot), when the stmmac driver is built into the kernel. It works when > the driver is built as a module. It's either some timing issue, or power > supply issue or a combination of both. Module build induces a power > cycling of the phy. > > I encourage people with this issue, to build the driver into the kernel, > and try to alter the reset timings for the phy in DTS or > startup-delay-us and report the findings. > > > Please take a look. > > thank you and regards, > Ondrej Jirman > > > Changes in v7: > - dropped stored reference to connector_pdev as suggested by Jernej > - added forgotten dt-bindings reviewed-by tag > > Changes in v6: > - added dt-bindings reviewed-by tag > - fix wording in stmmac commit (as suggested by Sergei) > > Changes in v5: > - dropped already applied patches (pinctrl patches, mmc1 pinconf patch) > - rename GMAC-3V3 -> GMAC-3V to match the schematic (Jagan) > - changed hdmi-connector's ddc-supply property to ddc-en-gpios > (Rob Herring) > > Changes in v4: > - fix checkpatch warnings/style issues > - use enum in struct sunxi_desc_function for io_bias_cfg_variant > - collected acked-by's > - fix compile error in drivers/pinctrl/sunxi/pinctrl-sun9i-a80-r.c:156 > caused by missing conversion from has_io_bias_cfg struct member > (I've kept the acked-by, because it's a trivial change, but feel free > to object.) (reported by Martin A. on github) > I did not have A80 pinctrl enabled for some reason, so I did not catch > this sooner. > - dropped brcm firmware patch (was already applied) > - dropped the wifi dts patch (will re-send after H6 RTC gets merged, > along with bluetooth support, in a separate series) > > Changes in v3: > - dropped already applied patches > - changed pinctrl I/O bias selection constants to enum and renamed > - added /omit-if-no-ref/ to mmc1_pins > - made mmc1_pins default pinconf for mmc1 in H6 dtsi > - move ddc-supply to HDMI connector node, updated patch descriptions, > changed dt-bindings docs > > Changes in v2: > - added dt-bindings documentation for the board's compatible string > (suggested by Clement) > - addressed checkpatch warnings and code formatting issues (on Maxime's > suggestions) > - stmmac: dropped useless parenthesis, reworded description of the patch > (suggested by Sergei) > - drop useles dev_info() about the selected io bias voltage > - docummented io voltage bias selection variant macros > - wifi: marked WiFi DTS patch and realted mmc1_pins as "DO NOT MERGE", > because wifi depends on H6 RTC support that's not merged yet (suggested > by Clement) > - added missing signed-of-bys > - changed &usb2otg dr_mode to otg, and added a note about VBUS > - improved wording of HDMI driver's DDC power supply patch > > Icenowy Zheng (2): > net: stmmac: sun8i: add support for Allwinner H6 EMAC > net: stmmac: sun8i: force select external PHY when no internal one > > Ondrej Jirman (4): > arm64: dts: allwinner: orange-pi-3: Enable ethernet > dt-bindings: display: hdmi-connector: Support DDC bus enable > drm: sun4i: Add support for enabling DDC I2C bus to sun8i_dw_hdmi glue > arm64: dts: allwinner: orange-pi-3: Enable HDMI output > > .../display/connector/hdmi-connector.txt | 1 + > .../dts/allwinner/sun50i-h6-orangepi-3.dts| 70 +++ > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 54 -- > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 2 + > .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 21 ++ > 5 files changed, 144 insertions(+), 4 deletions(-)
Re: [PATCH] drm: Dump mode picture aspect ratio
On Thu, Jun 20, 2019 at 11:46 AM Ville Syrjala wrote: > > From: Ville Syrjälä > > Currently the logs show nothing about the mode's aspect ratio. > Include that information in the normal mode dump. > > Cc: Ilia Mirkin > Signed-off-by: Ville Syrjälä > --- > drivers/video/hdmi.c| 3 ++- > include/drm/drm_modes.h | 4 ++-- > include/linux/hdmi.h| 3 +++ > 3 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c > index b939bc28d886..bc593fe1c268 100644 > --- a/drivers/video/hdmi.c > +++ b/drivers/video/hdmi.c > @@ -1057,7 +1057,7 @@ static const char *hdmi_colorimetry_get_name(enum > hdmi_colorimetry colorimetry) > return "Invalid"; > } > > -static const char * > +const char * > hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect) > { > switch (picture_aspect) { > @@ -1076,6 +1076,7 @@ hdmi_picture_aspect_get_name(enum hdmi_picture_aspect > picture_aspect) > } > return "Invalid"; > } > +EXPORT_SYMBOL(hdmi_picture_aspect_get_name); So this will return "No Data" most of the time (since the DRM_CAP won't be enabled)? This will look awkward, esp since the person seeing this print will have no idea what "No Data" is referring to. > > static const char * > hdmi_active_aspect_get_name(enum hdmi_active_aspect active_aspect) > diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h > index 083f16747369..2b1809c74fbe 100644 > --- a/include/drm/drm_modes.h > +++ b/include/drm/drm_modes.h > @@ -431,7 +431,7 @@ struct drm_display_mode { > /** > * DRM_MODE_FMT - printf string for &struct drm_display_mode > */ > -#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x" > +#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x %s" > > /** > * DRM_MODE_ARG - printf arguments for &struct drm_display_mode > @@ -441,7 +441,7 @@ struct drm_display_mode { > (m)->name, (m)->vrefresh, (m)->clock, \ > (m)->hdisplay, (m)->hsync_start, (m)->hsync_end, (m)->htotal, \ > (m)->vdisplay, (m)->vsync_start, (m)->vsync_end, (m)->vtotal, \ > - (m)->type, (m)->flags > + (m)->type, (m)->flags, > hdmi_picture_aspect_get_name((m)->picture_aspect_ratio) Flags are printed as a literal integer value. Given that AR stuff is fairly esoteric, I might just print an integer here too. (Why was picture_aspect_ratio not stuffed into ->flags in the first place?) > > #define obj_to_mode(x) container_of(x, struct drm_display_mode, base) > > diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h > index 9918a6c910c5..de7cbe385dba 100644 > --- a/include/linux/hdmi.h > +++ b/include/linux/hdmi.h > @@ -434,4 +434,7 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame, > void hdmi_infoframe_log(const char *level, struct device *dev, > const union hdmi_infoframe *frame); > > +const char * > +hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect); > + > #endif /* _DRM_HDMI_H */ > -- > 2.21.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/rockchip: Add optional support for CRTC gamma LUT
On Wed, 2019-06-19 at 00:18 +0200, Heiko Stübner wrote: > Am Mittwoch, 19. Juni 2019, 00:09:57 CEST schrieb Ezequiel Garcia: > > On Tue, 2019-06-18 at 17:47 -0400, Ilia Mirkin wrote: > > > On Tue, Jun 18, 2019 at 5:43 PM Ezequiel Garcia > > > wrote: > > > > Add an optional CRTC gamma LUT support, and enable it on RK3288. > > > > This is currently enabled via a separate address resource, > > > > which needs to be specified in the devicetree. > > > > > > > > The address resource is required because on some SoCs, such as > > > > RK3288, the LUT address is after the MMU address, and the latter > > > > is supported by a different driver. This prevents the DRM driver > > > > from requesting an entire register space. > > > > > > > > The current implementation works for RGB 10-bit tables, as that > > > > is what seems to work on RK3288. > > > > > > > > Signed-off-by: Ezequiel Garcia > > > > --- > > > > Changes from RFC: > > > > * Request (an optional) address resource for the LUT. > > > > * Drop support for RK3399, which doesn't seem to work > > > > out of the box and needs more research. > > > > * Support pass-thru setting when GAMMA_LUT is NULL. > > > > * Add a check for the gamma size, as suggested by Ilia. > > > > * Move gamma setting to atomic_commit_tail, as pointed > > > > out by Jacopo/Laurent, is the correct way. > > > > --- > > > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > > > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > > > index 12ed5265a90b..5b6edbe2673f 100644 > > > > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > > > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > > > > +static void vop_crtc_gamma_set(struct vop *vop, struct drm_crtc *crtc, > > > > + struct drm_crtc_state *old_state) > > > > +{ > > > > + int idle, ret, i; > > > > + > > > > + spin_lock(&vop->reg_lock); > > > > + VOP_REG_SET(vop, common, dsp_lut_en, 0); > > > > + vop_cfg_done(vop); > > > > + spin_unlock(&vop->reg_lock); > > > > + > > > > + ret = readx_poll_timeout(vop_dsp_lut_is_enable, vop, > > > > + idle, !idle, 5, 30 * 1000); > > > > + if (ret) > > > > + return; > > > > + > > > > + spin_lock(&vop->reg_lock); > > > > + > > > > + if (crtc->state->gamma_lut) { > > > > + if (!old_state->gamma_lut || > > > > (crtc->state->gamma_lut->base.id != > > > > + > > > > old_state->gamma_lut->base.id)) > > > > + vop_crtc_write_gamma_lut(vop, crtc); > > > > + } else { > > > > + for (i = 0; i < crtc->gamma_size; i++) { > > > > + u32 word; > > > > + > > > > + word = (i << 20) | (i << 10) | i; > > > > + writel(word, vop->lut_regs + i * 4); > > > > + } > > > > > > Note - I'm not in any way familiar with the hardware, so take with a > > > grain of salt -- > > > > > > Could you just leave dsp_lut_en turned off in this case? > > > > > > > That was the first thing I tried :-) > > > > It seems dsp_lut_en is not to enable the CRTC gamma LUT stage, > > but to enable writing the gamma LUT to the internal RAM. > > I guess that warants a code comment stating this, so we don't end > up with well-meant cleanup patches in the future :-) . > Sure, makes sense. Any other feedback aside from this? Thanks, Ezequiel
Re: [PATCH] drm: Dump mode picture aspect ratio
On Thu, Jun 20, 2019 at 11:59:37AM -0400, Ilia Mirkin wrote: > On Thu, Jun 20, 2019 at 11:46 AM Ville Syrjala > wrote: > > > > From: Ville Syrjälä > > > > Currently the logs show nothing about the mode's aspect ratio. > > Include that information in the normal mode dump. > > > > Cc: Ilia Mirkin > > Signed-off-by: Ville Syrjälä > > --- > > drivers/video/hdmi.c| 3 ++- > > include/drm/drm_modes.h | 4 ++-- > > include/linux/hdmi.h| 3 +++ > > 3 files changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c > > index b939bc28d886..bc593fe1c268 100644 > > --- a/drivers/video/hdmi.c > > +++ b/drivers/video/hdmi.c > > @@ -1057,7 +1057,7 @@ static const char *hdmi_colorimetry_get_name(enum > > hdmi_colorimetry colorimetry) > > return "Invalid"; > > } > > > > -static const char * > > +const char * > > hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect) > > { > > switch (picture_aspect) { > > @@ -1076,6 +1076,7 @@ hdmi_picture_aspect_get_name(enum hdmi_picture_aspect > > picture_aspect) > > } > > return "Invalid"; > > } > > +EXPORT_SYMBOL(hdmi_picture_aspect_get_name); > > So this will return "No Data" most of the time (since the DRM_CAP > won't be enabled)? This will look awkward, esp since the person seeing > this print will have no idea what "No Data" is referring to. I suppose we could do picture_aspet_ratio ? hdmi_picture_aspect_get_name(picture_aspet_ratio) : "" > > > > > static const char * > > hdmi_active_aspect_get_name(enum hdmi_active_aspect active_aspect) > > diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h > > index 083f16747369..2b1809c74fbe 100644 > > --- a/include/drm/drm_modes.h > > +++ b/include/drm/drm_modes.h > > @@ -431,7 +431,7 @@ struct drm_display_mode { > > /** > > * DRM_MODE_FMT - printf string for &struct drm_display_mode > > */ > > -#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x" > > +#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x > > %s" > > > > /** > > * DRM_MODE_ARG - printf arguments for &struct drm_display_mode > > @@ -441,7 +441,7 @@ struct drm_display_mode { > > (m)->name, (m)->vrefresh, (m)->clock, \ > > (m)->hdisplay, (m)->hsync_start, (m)->hsync_end, (m)->htotal, \ > > (m)->vdisplay, (m)->vsync_start, (m)->vsync_end, (m)->vtotal, \ > > - (m)->type, (m)->flags > > + (m)->type, (m)->flags, > > hdmi_picture_aspect_get_name((m)->picture_aspect_ratio) > > Flags are printed as a literal integer value. Given that AR stuff is > fairly esoteric, I might just print an integer here too. I hate those thing. I can't even remember which is one the type (absolutely useless) and which on is the flags. And I always end up going through the headers to figure out which bit is what sync polarity. So I'd rather like to decode those too, just been too lazy to do it. > (Why was > picture_aspect_ratio not stuffed into ->flags in the first place?) It's also in there. I think the reason for having this odd duplication was that we didn't have the flags initially, and just had the prop. I've not looked how hard it would be to get rid of that. > > > > > #define obj_to_mode(x) container_of(x, struct drm_display_mode, base) > > > > diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h > > index 9918a6c910c5..de7cbe385dba 100644 > > --- a/include/linux/hdmi.h > > +++ b/include/linux/hdmi.h > > @@ -434,4 +434,7 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame, > > void hdmi_infoframe_log(const char *level, struct device *dev, > > const union hdmi_infoframe *frame); > > > > +const char * > > +hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect); > > + > > #endif /* _DRM_HDMI_H */ > > -- > > 2.21.0 > > -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v2 5/9] drm/sun4i: tcon_top: Register clock gates in probe
On Tue, Jun 18, 2019 at 4:24 PM Chen-Yu Tsai wrote: > > On Tue, Jun 18, 2019 at 6:34 PM Jagan Teki wrote: > > > > On Tue, Jun 18, 2019 at 1:23 PM Chen-Yu Tsai wrote: > > > > > > On Tue, Jun 18, 2019 at 3:45 PM Jagan Teki > > > wrote: > > > > > > > > On Tue, Jun 18, 2019 at 12:49 PM Chen-Yu Tsai wrote: > > > > > > > > > > On Mon, Jun 17, 2019 at 6:30 PM Jagan Teki > > > > > wrote: > > > > > > > > > > > > On Sun, Jun 16, 2019 at 11:01 AM Chen-Yu Tsai wrote: > > > > > > > > > > > > > > On Sat, Jun 15, 2019 at 12:44 AM Jagan Teki > > > > > > > wrote: > > > > > > > > > > > > > > > > TCON TOP have clock gates for TV0, TV1, dsi and right > > > > > > > > now these are register during bind call. > > > > > > > > > > > > > > > > Of which, dsi clock gate would required during DPHY probe > > > > > > > > but same can miss to get since tcon top is not bound at > > > > > > > > that time. > > > > > > > > > > > > > > > > To solve, this circular dependency move the clock gate > > > > > > > > registration from bind to probe so-that DPHY can get the > > > > > > > > dsi gate clock on time. > > > > > > > > > > > > > > > > Signed-off-by: Jagan Teki > > > > > > > > --- > > > > > > > > drivers/gpu/drm/sun4i/sun8i_tcon_top.c | 94 > > > > > > > > ++ > > > > > > > > 1 file changed, 49 insertions(+), 45 deletions(-) > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c > > > > > > > > b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c > > > > > > > > index 465e9b0cdfee..a8978b3fe851 100644 > > > > > > > > --- a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c > > > > > > > > +++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c > > > > > > > > @@ -124,7 +124,53 @@ static struct clk_hw > > > > > > > > *sun8i_tcon_top_register_gate(struct device *dev, > > > > > > > > static int sun8i_tcon_top_bind(struct device *dev, struct > > > > > > > > device *master, > > > > > > > >void *data) > > > > > > > > { > > > > > > > > - struct platform_device *pdev = to_platform_device(dev); > > > > > > > > + struct sun8i_tcon_top *tcon_top = dev_get_drvdata(dev); > > > > > > > > + int ret; > > > > > > > > + > > > > > > > > + ret = reset_control_deassert(tcon_top->rst); > > > > > > > > + if (ret) { > > > > > > > > + dev_err(dev, "Could not deassert ctrl reset > > > > > > > > control\n"); > > > > > > > > + return ret; > > > > > > > > + } > > > > > > > > + > > > > > > > > + ret = clk_prepare_enable(tcon_top->bus); > > > > > > > > + if (ret) { > > > > > > > > + dev_err(dev, "Could not enable bus clock\n"); > > > > > > > > + goto err_assert_reset; > > > > > > > > + } > > > > > > > > > > > > > > You have to de-assert the reset control and enable the clock > > > > > > > before the > > > > > > > clocks it provides are registered. Otherwise a consumer may come > > > > > > > in and > > > > > > > ask for the provided clock to be enabled, but since the TCON > > > > > > > TOP's own > > > > > > > reset and clock are still disabled, you can't actually access the > > > > > > > registers > > > > > > > that controls the provided clock. > > > > > > > > > > > > These rst and bus are common reset and bus clocks not tcon top > > > > > > clocks > > > > > > that are trying to register here. ie reason I have not moved it in > > > > > > top. > > > > > > > > > > And you're sure that toggling bits in the TCON TOP block doesn't > > > > > require > > > > > the reset to be de-asserted and the bus clock enabled? > > > > > > > > > > Somehow I doubt that. > > > > > > > > > > Once the driver register the clocks it provides, they absolutely must > > > > > work. > > > > > They can't only work after the bind phase when the reset gets > > > > > de-asserted > > > > > and the bus clock enabled. Or you should provide proper error > > > > > reporting > > > > > in the clock ops. I doubt you want to go that way either. > > > > > > > > Why would they won't work after bind phase? unlike tcon top gates, > > > > these reset, and bus are common like what we have in other DE block > > > > so enable them in bind won't be an issue as per as I understand. let > > > > me know if you want me to check in other directions. > > > > > > You misunderstood. When you moved the clock registering parts to the probe > > > phase, but didn't move the clock enable and reset de-assert parts to go > > > with, > > > the clock ops will not work as expected between probe and bind time. > > > > If I understand correctly, I have moved tcon clock gates, not the bus > > clock or the reset. Both have independent enablement phase, the bus > > clock is enable in tcon top bind and the clock gate ("dsi") enable in > > init call of phy_ops. is both bus clock and clock gates are same and > > related that is what you are saying? > > I am saying that you may need the tcon top bus gates and resets properly > configured to be able to read/write the
Re: [PATCH] drm/imx: correct order of crtc disable
On Thu, Jun 20, 2019 at 3:30 PM Robert Beckett wrote: > On Thu, 2019-06-20 at 14:32 +0200, Daniel Vetter wrote: > > On Thu, Jun 20, 2019 at 12:12:13PM +0100, Robert Beckett wrote: > > > On Thu, 2019-06-20 at 10:50 +0200, Daniel Vetter wrote: > > > > On Wed, Jun 19, 2019 at 11:40 AM Philipp Zabel < > > > > p.za...@pengutronix.de> wrote: > > > > > > > > > > Hi Robert, > > > > > > > > > > thank you for the patch. > > > > > > > > > > On Tue, 2019-06-18 at 16:50 +0100, Robert Beckett wrote: > > > > > > Notify drm core before sending pending events during crtc > > > > > > disable. > > > > > > This fixes the first event after disable having an old stale > > > > > > timestamp > > > > > > by having drm_crtc_vblank_off update the timestamp to now. > > > > > > > > > > > > This was seen while debugging weston log message: > > > > > > Warning: computed repaint delay is insane: -8212 msec > > > > > > > > > > > > > > > > Would you say this > > > > > Fixes: a474478642d5 ("drm/imx: fix crtc vblank state > > > > > regression") > > > > > ? > > > > > > > > > > > Signed-off-by: Robert Beckett > > > > > > --- > > > > > > drivers/gpu/drm/imx/ipuv3-crtc.c | 6 +++--- > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > > > b/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > > > index 9cc1d678674f..c436a28d50e4 100644 > > > > > > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > > > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c > > > > > > @@ -91,14 +91,14 @@ static void > > > > > > ipu_crtc_atomic_disable(struct > > > > > > drm_crtc *crtc, > > > > > > ipu_dc_disable(ipu); > > > > > > ipu_prg_disable(ipu); > > > > > > > > > > > > + drm_crtc_vblank_off(crtc); > > > > > > + > > > > > > > > > > This is explained in the commit message and aligns with the > > > > > drm_crtc_state @event documentation. > > > > > > > > This part here looks fishy. The drm_vblank.c code is supposed to > > > > do > > > > the right thing, no matter where or when you ask it to generate > > > > an > > > > event. It definitely shouldn't generate a timestamp that's a few > > > > seconds too early. Bunch of options: > > > > - there's a bug in drm_vblank.c and it's mixing up something and > > > > generating a totally bogus value. > > > > - there's a lie in your imx vblank code, which trips the > > > > drm_vblank.c > > > > counter interpolation and results in a totally bogus value. > > > > > > > > drm_vblank.c assumes that if you do claim to have a hw counter > > > > and > > > > generate timestamps, that those are perfectly accurate. It only > > > > falls > > > > back to guestimating using the system timer if that's not > > > > present. > > > > > > > > Either way, this very much smells like papering over a bug if > > > > this > > > > change indeed fixes your wrong vblank timestamps. > > > > > > > > > > A quick explaination of where the dodgy timestamp came from: > > > 1. driver starts up > > > 2. fbcon comes along and restores fbdev, enabling vblank > > > 3. vblank_disable_fn fires via timer disabling vblank, keeping > > > vblank > > > seq number and time set at current value > > > (some time later) > > > 4. weston starts and does a modeset > > > 5. atomic commit disables crtc while it does the modeset > > > 6. ipu_crtc_atomic_disable sends vblank with old seq number and > > > time > > > > > > It turns out the actual fix for the old vblank is the next change, > > > which stops it being sent at all during the crtc disable as it is > > > is > > > still active, it would then go through drm_crtc_vblank_off, > > > reseting > > > the timestamp, and get delivered during the vblank enable as part > > > of > > > the atomic commit. > > > > This shouldn't fix your vblank timestamp troubles either. It might > > mean > > that the timestamp is slightly too early (because you take it while > > shutting down the crtc, not while re-enabling), but not by seconds. > > > > Quick experiment: Disable vblank disabling with drm.vblankoffdelay = > > 0. If > > that also fixes the timestamps, then I'm pretty sure you have a > > driver bug > > somewhere and lie to the vblank core code about something. > > -Daniel > > > > Experiment done. The timestamp is fine, it is the timestamp of the > previous vblank update. But, this would fix it because the vblank > interrupt was never disabled. > > The original issue was that the event got sent after vblank was > disabled and before it got re-enabled during the modeset, so nothing > had happened to update the timestamp and seq number. > > What are you expecting to update the timestamp and seq number while the > interrupt is disabled after vblank_disable_fn? Hm maybe this indeed needs to be shuffled around a bit, since currently it's indeed not not allowed to call drm_crtc_send_vblank_event if: - your driver has vblank support (i.e. dev->num_crtcs > 0) - the vblank irq is off (i.e. no one called drm_crtc_vblank_get) - from the vblank code's pov the pipe is still runnin
Re: [PATCH 01/13] drm/amdgpu: introduce and honour DRM_FORCE_AUTH workaround
On 2019/06/14, Koenig, Christian wrote: > Am 14.06.19 um 17:53 schrieb Emil Velikov: > > On 2019/06/14, Koenig, Christian wrote: > >> Am 14.06.19 um 14:09 schrieb Emil Velikov: > >>> On 2019/05/27, Emil Velikov wrote: > >>> [SNIP] > >>> Hi Christian, > >>> > >>> > >>> In the following, I would like to summarise and emphasize the need for > >>> DRM_AUTH removal. I would kindly ask you to spend a couple of minutes > >>> extra reading it. > >>> > >>> > >>> Today DRM drivers* do not make any distinction between primary and > >>> render node clients. > >> That is actually not 100% correct. We have a special case where a DRM > >> master is allowed to change the priority of render node clients. > >> > > Can you provide a link? I cannot find that code. > > See amdgpu_sched_ioctl(). > > >>> Thus for a render capable driver, any premise of > >>> separation, security or otherwise imposed via DRM_AUTH is a fallacy. > >> Yeah, that's what I agree on. I just don't think that removing DRM_AUTH > >> now is the right direction to take. > >> > > Could have been clearer - I'm talking about DRM_AUTH | DRM_RENDER_ALLOW > > ioctls. > > > > That aside, can you propose an alternative solution that addresses this > > and the second point just below? > > Give me a few days to work on this, it's already Friday 6pm here. > Hi Christian, Any progress? As mentioned earlier, I'm OK with writing the patches although I would love to hear your plan. Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel