Re: [PATCHv16 3/3] ARM:drm ivip Intel FPGA Video and Image Processing Suite
On 6/7/19 7:30 AM, Hean-Loong, Ong wrote: > diff --git a/drivers/gpu/drm/ivip/Kconfig b/drivers/gpu/drm/ivip/Kconfig > new file mode 100644 > index ..1b2af85fe757 > --- /dev/null > +++ b/drivers/gpu/drm/ivip/Kconfig > @@ -0,0 +1,14 @@ > +config DRM_IVIP > +tristate "Intel FGPA Video and Image Processing" > +depends on DRM && OF > +select DRM_GEM_CMA_HELPER > +select DRM_KMS_HELPER > +select DRM_KMS_FB_HELPER > +select DRM_KMS_CMA_HELPER > +help > + Choose this option if you have an Intel FPGA Arria 10 system > + and above with an Intel Display Port IP. This does not support > + legacy Intel FPGA Cyclone V display port. Currently only single > + frame buffer is supported. Note that ACPI and X_86 architecture > + is not supported for Arria10. If M is selected the module will > be > + called ivip. According to Documentation/process/coding-style.rst, Kconfig help text should be indented with 1 tab + 2 spaces, not 2 tabs. -- ~Randy
Re: [PATCH v16 14/16] tee, arm64: untag user pointers in tee_shm_register
On Mon, Jun 03, 2019 at 06:55:16PM +0200, 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. > > tee_shm_register()->optee_shm_unregister()->check_mem_type() uses provided > user pointers for vma lookups (via __check_mem_type()), which can only by > done with untagged pointers. > > Untag user pointers in this function. > > Signed-off-by: Andrey Konovalov "tee: shm: untag user pointers in tee_shm_register" Reviewed-by: Kees Cook -Kees > --- > drivers/tee/tee_shm.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c > index 49fd7312e2aa..96945f4cefb8 100644 > --- a/drivers/tee/tee_shm.c > +++ b/drivers/tee/tee_shm.c > @@ -263,6 +263,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, > unsigned long addr, > shm->teedev = teedev; > shm->ctx = ctx; > shm->id = -1; > + addr = untagged_addr(addr); > start = rounddown(addr, PAGE_SIZE); > shm->offset = addr - start; > shm->size = length; > -- > 2.22.0.rc1.311.g5d7573a151-goog > -- Kees Cook ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 09/16] fs, arm64: untag user pointers in fs/userfaultfd.c
On Mon, Jun 03, 2019 at 06:55:11PM +0200, 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.
>
> userfaultfd code use provided user pointers for vma lookups, which can
> only by done with untagged pointers.
>
> Untag user pointers in validate_range().
>
> Signed-off-by: Andrey Konovalov
"userfaultfd: untag user pointers"
Reviewed-by: Kees Cook
-Kees
> ---
> fs/userfaultfd.c | 22 --
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index 3b30301c90ec..24d68c3b5ee2 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -1263,21 +1263,23 @@ static __always_inline void wake_userfault(struct
> userfaultfd_ctx *ctx,
> }
>
> static __always_inline int validate_range(struct mm_struct *mm,
> - __u64 start, __u64 len)
> + __u64 *start, __u64 len)
> {
> __u64 task_size = mm->task_size;
>
> - if (start & ~PAGE_MASK)
> + *start = untagged_addr(*start);
> +
> + if (*start & ~PAGE_MASK)
> return -EINVAL;
> if (len & ~PAGE_MASK)
> return -EINVAL;
> if (!len)
> return -EINVAL;
> - if (start < mmap_min_addr)
> + if (*start < mmap_min_addr)
> return -EINVAL;
> - if (start >= task_size)
> + if (*start >= task_size)
> return -EINVAL;
> - if (len > task_size - start)
> + if (len > task_size - *start)
> return -EINVAL;
> return 0;
> }
> @@ -1327,7 +1329,7 @@ static int userfaultfd_register(struct userfaultfd_ctx
> *ctx,
> goto out;
> }
>
> - ret = validate_range(mm, uffdio_register.range.start,
> + ret = validate_range(mm, _register.range.start,
>uffdio_register.range.len);
> if (ret)
> goto out;
> @@ -1516,7 +1518,7 @@ static int userfaultfd_unregister(struct
> userfaultfd_ctx *ctx,
> if (copy_from_user(_unregister, buf, sizeof(uffdio_unregister)))
> goto out;
>
> - ret = validate_range(mm, uffdio_unregister.start,
> + ret = validate_range(mm, _unregister.start,
>uffdio_unregister.len);
> if (ret)
> goto out;
> @@ -1667,7 +1669,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
> if (copy_from_user(_wake, buf, sizeof(uffdio_wake)))
> goto out;
>
> - ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len);
> + ret = validate_range(ctx->mm, _wake.start, uffdio_wake.len);
> if (ret)
> goto out;
>
> @@ -1707,7 +1709,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
> sizeof(uffdio_copy)-sizeof(__s64)))
> goto out;
>
> - ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len);
> + ret = validate_range(ctx->mm, _copy.dst, uffdio_copy.len);
> if (ret)
> goto out;
> /*
> @@ -1763,7 +1765,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx
> *ctx,
> sizeof(uffdio_zeropage)-sizeof(__s64)))
> goto out;
>
> - ret = validate_range(ctx->mm, uffdio_zeropage.range.start,
> + ret = validate_range(ctx->mm, _zeropage.range.start,
>uffdio_zeropage.range.len);
> if (ret)
> goto out;
> --
> 2.22.0.rc1.311.g5d7573a151-goog
>
--
Kees Cook
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 08/16] fs, arm64: untag user pointers in copy_mount_options
On Mon, Jun 03, 2019 at 06:55:10PM +0200, 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: Catalin Marinas > Signed-off-by: Andrey Konovalov One thing I just noticed in the commit titles... "arm64" is in the prefix, but these are arch-indep areas. Should the ", arm64" be left out? I would expect, instead: fs/namespace: untag user pointers in copy_mount_options Reviewed-by: Kees Cook -Kees > --- > 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; > > -- > 2.22.0.rc1.311.g5d7573a151-goog > -- Kees Cook ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 07/16] mm, arm64: untag user pointers in get_vaddr_frames
On Mon, Jun 03, 2019 at 06:55:09PM +0200, 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. > > Signed-off-by: Andrey Konovalov Reviewed-by: Kees Cook -Kees > --- > 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(>mmap_sem); > locked = 1; > vma = find_vma_intersection(mm, start, start + 1); > -- > 2.22.0.rc1.311.g5d7573a151-goog > -- Kees Cook ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 06/16] mm, arm64: untag user pointers in mm/gup.c
On Mon, Jun 03, 2019 at 06:55:08PM +0200, 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: Catalin Marinas > Signed-off-by: Andrey Konovalov Reviewed-by: Kees Cook -Kees > --- > 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; > > -- > 2.22.0.rc1.311.g5d7573a151-goog > -- Kees Cook ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 15/16] vfio/type1, arm64: untag user pointers in vaddr_get_pfn
On Mon, Jun 03, 2019 at 06:55:17PM +0200, 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.
>
> vaddr_get_pfn() uses provided user pointers for vma lookups, which can
> only by done with untagged pointers.
>
> Untag user pointers in this function.
>
> Signed-off-by: Andrey Konovalov
Reviewed-by: Kees Cook
-Kees
> ---
> drivers/vfio/vfio_iommu_type1.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 3ddc375e7063..528e39a1c2dd 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -384,6 +384,8 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned
> long vaddr,
>
> down_read(>mmap_sem);
>
> + vaddr = untagged_addr(vaddr);
> +
> vma = find_vma_intersection(mm, vaddr, vaddr + 1);
>
> if (vma && vma->vm_flags & VM_PFNMAP) {
> --
> 2.22.0.rc1.311.g5d7573a151-goog
>
--
Kees Cook
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 16/16] selftests, arm64: add a selftest for passing tagged pointers to kernel
On Mon, Jun 03, 2019 at 06:55:18PM +0200, 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 adds a simple test, that calls the uname syscall with a
> tagged user pointer as an argument. Without the kernel accepting tagged
> user pointers the test fails with EFAULT.
>
> Signed-off-by: Andrey Konovalov
I'm adding Shuah to CC in case she has some suggestions about the new
selftest.
Reviewed-by: Kees Cook
-Kees
> ---
> tools/testing/selftests/arm64/.gitignore | 1 +
> tools/testing/selftests/arm64/Makefile| 22 ++
> .../testing/selftests/arm64/run_tags_test.sh | 12 ++
> tools/testing/selftests/arm64/tags_lib.c | 42 +++
> tools/testing/selftests/arm64/tags_test.c | 18
> 5 files changed, 95 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/.gitignore
> create mode 100644 tools/testing/selftests/arm64/Makefile
> create mode 100755 tools/testing/selftests/arm64/run_tags_test.sh
> create mode 100644 tools/testing/selftests/arm64/tags_lib.c
> create mode 100644 tools/testing/selftests/arm64/tags_test.c
>
> diff --git a/tools/testing/selftests/arm64/.gitignore
> b/tools/testing/selftests/arm64/.gitignore
> new file mode 100644
> index ..e8fae8d61ed6
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/.gitignore
> @@ -0,0 +1 @@
> +tags_test
> diff --git a/tools/testing/selftests/arm64/Makefile
> b/tools/testing/selftests/arm64/Makefile
> new file mode 100644
> index ..9dee18727923
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/Makefile
> @@ -0,0 +1,22 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +include ../lib.mk
> +
> +# ARCH can be overridden by the user for cross compiling
> +ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> +
> +ifneq (,$(filter $(ARCH),aarch64 arm64))
> +
> +TEST_CUSTOM_PROGS := $(OUTPUT)/tags_test
> +
> +$(OUTPUT)/tags_test: tags_test.c $(OUTPUT)/tags_lib.so
> + $(CC) -o $@ $(CFLAGS) $(LDFLAGS) $<
> +
> +$(OUTPUT)/tags_lib.so: tags_lib.c
> + $(CC) -o $@ -shared $(CFLAGS) $(LDFLAGS) $^
> +
> +TEST_PROGS := run_tags_test.sh
> +
> +all: $(TEST_CUSTOM_PROGS)
> +
> +endif
> diff --git a/tools/testing/selftests/arm64/run_tags_test.sh
> b/tools/testing/selftests/arm64/run_tags_test.sh
> new file mode 100755
> index ..2bbe0cd4220b
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/run_tags_test.sh
> @@ -0,0 +1,12 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +echo ""
> +echo "running tags test"
> +echo ""
> +LD_PRELOAD=./tags_lib.so ./tags_test
> +if [ $? -ne 0 ]; then
> + echo "[FAIL]"
> +else
> + echo "[PASS]"
> +fi
> diff --git a/tools/testing/selftests/arm64/tags_lib.c
> b/tools/testing/selftests/arm64/tags_lib.c
> new file mode 100644
> index ..8a674509216e
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/tags_lib.c
> @@ -0,0 +1,42 @@
> +#include
> +
> +#define TAG_SHIFT(56)
> +#define TAG_MASK (0xffUL << TAG_SHIFT)
> +
> +void *__libc_malloc(size_t size);
> +void __libc_free(void *ptr);
> +void *__libc_realloc(void *ptr, size_t size);
> +void *__libc_calloc(size_t nmemb, size_t size);
> +
> +static void *tag_ptr(void *ptr)
> +{
> + unsigned long tag = rand() & 0xff;
> + if (!ptr)
> + return ptr;
> + return (void *)((unsigned long)ptr | (tag << TAG_SHIFT));
> +}
> +
> +static void *untag_ptr(void *ptr)
> +{
> + return (void *)((unsigned long)ptr & ~TAG_MASK);
> +}
> +
> +void *malloc(size_t size)
> +{
> + return tag_ptr(__libc_malloc(size));
> +}
> +
> +void free(void *ptr)
> +{
> + __libc_free(untag_ptr(ptr));
> +}
> +
> +void *realloc(void *ptr, size_t size)
> +{
> + return tag_ptr(__libc_realloc(untag_ptr(ptr), size));
> +}
> +
> +void *calloc(size_t nmemb, size_t size)
> +{
> + return tag_ptr(__libc_calloc(nmemb, size));
> +}
> diff --git a/tools/testing/selftests/arm64/tags_test.c
> b/tools/testing/selftests/arm64/tags_test.c
> new file mode 100644
> index ..263b302874ed
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/tags_test.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +int main(void)
> +{
> + struct utsname *ptr;
> + int err;
> +
> + ptr = (struct utsname *)malloc(sizeof(*ptr));
> + err = uname(ptr);
> + free(ptr);
> + return err;
> +}
> --
> 2.22.0.rc1.311.g5d7573a151-goog
>
--
Kees Cook
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 13/16] media/v4l2-core, arm64: untag user pointers in videobuf_dma_contig_user_get
On Mon, Jun 03, 2019 at 06:55:15PM +0200, 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.
>
> Acked-by: Mauro Carvalho Chehab
> Signed-off-by: Andrey Konovalov
Reviewed-by: Kees Cook
-Kees
> ---
> drivers/media/v4l2-core/videobuf-dma-contig.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> 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(>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, _pfn);
> --
> 2.22.0.rc1.311.g5d7573a151-goog
>
--
Kees Cook
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 05/16] arm64: untag user pointers passed to memory syscalls
On Mon, Jun 03, 2019 at 06:55:07PM +0200, 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.
>
> Signed-off-by: Andrey Konovalov
Reviewed-by: Kees Cook
-Kees
> ---
> mm/madvise.c | 2 ++
> mm/mempolicy.c | 3 +++
> mm/mincore.c | 2 ++
> mm/mlock.c | 4
> mm/mprotect.c | 2 ++
> mm/mremap.c| 2 ++
> mm/msync.c | 2 ++
> 7 files changed, 17 insertions(+)
>
> 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/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..1d98281f7204 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -606,6 +606,8 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned
> long, old_len,
> LIST_HEAD(uf_unmap_early);
> LIST_HEAD(uf_unmap);
>
> + 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.c
> +++ b/mm/msync.c
> @@ -37,6 +37,8 @@ SYSCALL_DEFINE3(msync, unsigned long, start, size_t, len,
> int, flags)
> int unmapped_error = 0;
> int error = -EINVAL;
>
> + start = untagged_addr(start);
> +
> if (flags & ~(MS_ASYNC | MS_INVALIDATE | MS_SYNC))
> goto out;
> if (offset_in_page(start))
> --
> 2.22.0.rc1.311.g5d7573a151-goog
>
--
Kees Cook
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 02/16] arm64: untag user pointers in access_ok and __uaccess_mask_ptr
On Mon, Jun 03, 2019 at 06:55:04PM +0200, 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. > > copy_from_user (and a few other similar functions) are used to copy data > from user memory into the kernel memory or vice versa. Since a user can > provided a tagged pointer to one of the syscalls that use copy_from_user, > we need to correctly handle such pointers. > > Do this by untagging user pointers in access_ok and in __uaccess_mask_ptr, > before performing access validity checks. > > Note, that this patch only temporarily untags the pointers to perform the > checks, but then passes them as is into the kernel internals. > > Reviewed-by: Catalin Marinas > Signed-off-by: Andrey Konovalov Reviewed-by: Kees Cook -Kees > --- > arch/arm64/include/asm/uaccess.h | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/include/asm/uaccess.h > b/arch/arm64/include/asm/uaccess.h > index e5d5f31c6d36..9164ecb5feca 100644 > --- a/arch/arm64/include/asm/uaccess.h > +++ b/arch/arm64/include/asm/uaccess.h > @@ -94,7 +94,7 @@ static inline unsigned long __range_ok(const void __user > *addr, unsigned long si > return ret; > } > > -#define access_ok(addr, size)__range_ok(addr, size) > +#define access_ok(addr, size)__range_ok(untagged_addr(addr), size) > #define user_addr_maxget_fs > > #define _ASM_EXTABLE(from, to) > \ > @@ -226,7 +226,8 @@ static inline void uaccess_enable_not_uao(void) > > /* > * Sanitise a uaccess pointer such that it becomes NULL if above the > - * current addr_limit. > + * current addr_limit. In case the pointer is tagged (has the top byte set), > + * untag the pointer before checking. > */ > #define uaccess_mask_ptr(ptr) (__typeof__(ptr))__uaccess_mask_ptr(ptr) > static inline void __user *__uaccess_mask_ptr(const void __user *ptr) > @@ -234,10 +235,11 @@ static inline void __user *__uaccess_mask_ptr(const > void __user *ptr) > void __user *safe_ptr; > > asm volatile( > - " bicsxzr, %1, %2\n" > + " bicsxzr, %3, %2\n" > " csel%0, %1, xzr, eq\n" > : "=" (safe_ptr) > - : "r" (ptr), "r" (current_thread_info()->addr_limit) > + : "r" (ptr), "r" (current_thread_info()->addr_limit), > + "r" (untagged_addr(ptr)) > : "cc"); > > csdb(); > -- > 2.22.0.rc1.311.g5d7573a151-goog > -- Kees Cook ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 04/16] mm: untag user pointers in do_pages_move
On Mon, Jun 03, 2019 at 06:55:06PM +0200, 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. > > do_pages_move() is used in the implementation of the move_pages syscall. > > Untag user pointers in this function. > > Reviewed-by: Catalin Marinas > Signed-off-by: Andrey Konovalov Reviewed-by: Kees Cook -Kees > --- > mm/migrate.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mm/migrate.c b/mm/migrate.c > index f2ecc2855a12..3930bb6fa656 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -1617,6 +1617,7 @@ static int do_pages_move(struct mm_struct *mm, > nodemask_t task_nodes, > if (get_user(node, nodes + i)) > goto out_flush; > addr = (unsigned long)p; > + addr = untagged_addr(addr); > > err = -ENODEV; > if (node < 0 || node >= MAX_NUMNODES) > -- > 2.22.0.rc1.311.g5d7573a151-goog > -- Kees Cook ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 03/16] lib, arm64: untag user pointers in strn*_user
On Mon, Jun 03, 2019 at 06:55:05PM +0200, 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.
>
> strncpy_from_user and strnlen_user accept user addresses as arguments, and
> do not go through the same path as copy_from_user and others, so here we
> need to handle the case of tagged user addresses separately.
>
> Untag user pointers passed to these functions.
>
> Note, that this patch only temporarily untags the pointers to perform
> validity checks, but then uses them as is to perform user memory accesses.
>
> Reviewed-by: Catalin Marinas
> Signed-off-by: Andrey Konovalov
Acked-by: Kees Cook
-Kees
> ---
> lib/strncpy_from_user.c | 3 ++-
> lib/strnlen_user.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
> index 023ba9f3b99f..dccb95af6003 100644
> --- a/lib/strncpy_from_user.c
> +++ b/lib/strncpy_from_user.c
> @@ -6,6 +6,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -108,7 +109,7 @@ long strncpy_from_user(char *dst, const char __user *src,
> long count)
> return 0;
>
> max_addr = user_addr_max();
> - src_addr = (unsigned long)src;
> + src_addr = (unsigned long)untagged_addr(src);
> if (likely(src_addr < max_addr)) {
> unsigned long max = max_addr - src_addr;
> long retval;
> diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
> index 7f2db3fe311f..28ff554a1be8 100644
> --- a/lib/strnlen_user.c
> +++ b/lib/strnlen_user.c
> @@ -2,6 +2,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -109,7 +110,7 @@ long strnlen_user(const char __user *str, long count)
> return 0;
>
> max_addr = user_addr_max();
> - src_addr = (unsigned long)str;
> + src_addr = (unsigned long)untagged_addr(str);
> if (likely(src_addr < max_addr)) {
> unsigned long max = max_addr - src_addr;
> long retval;
> --
> 2.22.0.rc1.311.g5d7573a151-goog
>
--
Kees Cook
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #59 from Marek Olšák --- Richard, your change for create_fence is correct. -- 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
[GIT PULL FOR v5.3] R-Car DU changes
Hi Dave, The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9: Linux 5.2-rc1 (2019-05-19 15:47:09 -0700) are available in the Git repository at: git://linuxtv.org/pinchartl/media.git tags/du-next-20190608 for you to fetch changes up to 8e8fddab0d0acdefb1ad76852d954b2bbaa3896d: drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode (2019-06-08 02:36:04 +0300) R-Car DU changes for v5.3: - R8A774A1 SoC support - LVDS dual-link mode support - Misc fixes Biju Das (4): dt-bindings: display: renesas: du: Document the r8a774a1 bindings dt-bindings: display: renesas: lvds: Document r8a774a1 bindings drm: rcar-du: Add R8A774A1 support drm: rcar-du: lvds: Add r8a774a1 support Kieran Bingham (1): drm: rcar-du: writeback: include interface header Laurent Pinchart (7): drm: bridge: Add dual_link field to the drm_bridge_timings structure dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation drm: bridge: thc63: Report input bus mode through bridge timings dt-bindings: display: renesas: lvds: Add renesas,companion property drm: rcar-du: lvds: Remove LVDS double-enable checks drm: rcar-du: lvds: Add support for dual-link mode drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode .../bindings/display/bridge/renesas,lvds.txt | 19 ++- .../bindings/display/bridge/thine,thc63lvd1024.txt | 6 + .../devicetree/bindings/display/renesas,du.txt | 2 + drivers/gpu/drm/bridge/thc63lvd1024.c | 54 +++-- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 30 + drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 12 ++ drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_writeback.c| 1 + drivers/gpu/drm/rcar-du/rcar_lvds.c| 127 +++-- drivers/gpu/drm/rcar-du/rcar_lvds.h| 5 + include/drm/drm_bridge.h | 8 ++ 11 files changed, 218 insertions(+), 48 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 07/10] drm: rcar-du: Skip LVDS1 output on Gen3 when using dual-link LVDS mode
Hi Laurent,
On 28/05/2019 15:12, Laurent Pinchart wrote:
> In dual-link LVDS mode, the LVDS1 encoder is used as a companion for
> LVDS0, and both encoders transmit data from DU0. The LVDS1 output of DU1
> can't be used in that case, don't create an encoder and connector for
> it.
>
> Signed-off-by: Laurent Pinchart
> Reviewed-by: Jacopo Mondi
> Tested-by: Jacopo Mondi
And finally, Last one...
Reviewed-by: Kieran Bingham
> ---
> Changess since v2:
>
> - Remove unneeded bridge NULL check
> ---
> drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 12
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 2 +-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> index 6c91753af7bc..0f00bdfe2366 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> @@ -16,6 +16,7 @@
> #include "rcar_du_drv.h"
> #include "rcar_du_encoder.h"
> #include "rcar_du_kms.h"
> +#include "rcar_lvds.h"
>
> /*
> -
> * Encoder
> @@ -97,6 +98,17 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
> }
> }
>
> + /*
> + * On Gen3 skip the LVDS1 output if the LVDS1 encoder is used as a
> + * companion for LVDS0 in dual-link mode.
> + */
> + if (rcdu->info->gen >= 3 && output == RCAR_DU_OUTPUT_LVDS1) {
> + if (rcar_lvds_dual_link(bridge)) {
> + ret = -ENOLINK;
> + goto done;
> + }
> + }
> +
> ret = drm_encoder_init(rcdu->ddev, encoder, _funcs,
> DRM_MODE_ENCODER_NONE, NULL);
> if (ret < 0)
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index f8f7fff34dff..95c81e59e2f1 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -378,7 +378,7 @@ static int rcar_du_encoders_init_one(struct
> rcar_du_device *rcdu,
> }
>
> ret = rcar_du_encoder_init(rcdu, output, entity);
> - if (ret && ret != -EPROBE_DEFER)
> + if (ret && ret != -EPROBE_DEFER && ret != -ENOLINK)
> dev_warn(rcdu->dev,
>"failed to initialize encoder %pOF on output %u (%d),
> skipping\n",
>entity, output, ret);
>
--
Regards
--
Kieran
Re: [PATCH v3 08/10] arm64: dts: renesas: r8a7799[05]: Point LVDS0 to its companion LVDS1
Hi Laurent,
On 28/05/2019 15:12, Laurent Pinchart wrote:
> Add the new renesas,companion property to the LVDS0 node to point to the
> companion LVDS encoder LVDS1.
>
> Signed-off-by: Laurent Pinchart
> Reviewed-by: Jacopo Mondi
> Tested-by: Jacopo Mondi
Reviewed-by: Kieran Bingham
> ---
> arch/arm64/boot/dts/renesas/r8a77990.dtsi | 2 ++
> arch/arm64/boot/dts/renesas/r8a77995.dtsi | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> index 56cb566ffa09..7cf5963eb3ba 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi
> @@ -1801,6 +1801,8 @@
> resets = < 727>;
> status = "disabled";
>
> + renesas,companion = <>;
> +
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> index 5bf3af246e14..94b5177eb152 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi
> @@ -1038,6 +1038,8 @@
> resets = < 727>;
> status = "disabled";
>
> + renesas,companion = <>;
> +
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
--
Regards
--
Kieran
Re: [PATCH v3 06/10] drm: rcar-du: lvds: Add support for dual-link mode
Hi Laurent,
On 28/05/2019 15:12, Laurent Pinchart wrote:
> In dual-link mode the LVDS0 encoder transmits even-numbered pixels, and
> sends odd-numbered pixels to the LVDS1 encoder for transmission on a
> separate link.
>
> To implement support for this mode of operation, determine if the LVDS
> connection operates in dual-link mode by querying the next device in the
> pipeline, locate the companion encoder, and control it directly through
> its bridge operations.
>
> Signed-off-by: Laurent Pinchart
> Reviewed-by: Jacopo Mondi
> Tested-by: Jacopo Mondi
Looks good to me.
Reviewed-by: Kieran Bingham
> ---
> Changes since v2:
>
> - Fail probe if the companion controller can't be found or is invalid
> ---
> drivers/gpu/drm/rcar-du/rcar_lvds.c | 107
> drivers/gpu/drm/rcar-du/rcar_lvds.h | 5 ++
> 2 files changed, 99 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index a331f0c32187..d090191e858e 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -66,6 +66,9 @@ struct rcar_lvds {
>
> struct drm_display_mode display_mode;
> enum rcar_lvds_mode mode;
> +
> + struct drm_bridge *companion;
> + bool dual_link;
> };
>
> #define bridge_to_rcar_lvds(bridge) \
> @@ -400,11 +403,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> {
> struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> const struct drm_display_mode *mode = >display_mode;
> - /*
> - * FIXME: We should really retrieve the CRTC through the state, but how
> - * do we get a state pointer?
> - */
> - struct drm_crtc *crtc = lvds->bridge.encoder->crtc;
> u32 lvdhcr;
> u32 lvdcr0;
> int ret;
> @@ -413,6 +411,10 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> if (ret < 0)
> return;
>
> + /* Enable the companion LVDS encoder in dual-link mode. */
> + if (lvds->dual_link && lvds->companion)
> + lvds->companion->funcs->enable(lvds->companion);
> +
> /*
>* Hardcode the channels and control signals routing for now.
>*
> @@ -435,17 +437,33 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> rcar_lvds_write(lvds, LVDCHCR, lvdhcr);
>
> if (lvds->info->quirks & RCAR_LVDS_QUIRK_DUAL_LINK) {
> - /* Disable dual-link mode. */
> - rcar_lvds_write(lvds, LVDSTRIPE, 0);
> + /*
> + * Configure vertical stripe based on the mode of operation of
> + * the connected device.
> + */
> + rcar_lvds_write(lvds, LVDSTRIPE,
> + lvds->dual_link ? LVDSTRIPE_ST_ON : 0);
> }
>
> - /* PLL clock configuration. */
> - lvds->info->pll_setup(lvds, mode->clock * 1000);
> + /*
> + * PLL clock configuration on all instances but the companion in
> + * dual-link mode.
> + */
> + if (!lvds->dual_link || lvds->companion)
> + lvds->info->pll_setup(lvds, mode->clock * 1000);
>
> /* Set the LVDS mode and select the input. */
> lvdcr0 = lvds->mode << LVDCR0_LVMD_SHIFT;
> - if (drm_crtc_index(crtc) == 2)
> - lvdcr0 |= LVDCR0_DUSEL;
> +
> + if (lvds->bridge.encoder) {
> + /*
> + * FIXME: We should really retrieve the CRTC through the state,
> + * but how do we get a state pointer?
> + */
> + if (drm_crtc_index(lvds->bridge.encoder->crtc) == 2)
> + lvdcr0 |= LVDCR0_DUSEL;
> + }
> +
> rcar_lvds_write(lvds, LVDCR0, lvdcr0);
>
> /* Turn all the channels on. */
> @@ -512,6 +530,10 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
> rcar_lvds_write(lvds, LVDCR1, 0);
> rcar_lvds_write(lvds, LVDPLLCR, 0);
>
> + /* Disable the companion LVDS encoder in dual-link mode. */
> + if (lvds->dual_link && lvds->companion)
> + lvds->companion->funcs->disable(lvds->companion);
> +
> clk_disable_unprepare(lvds->clocks.mod);
> }
>
> @@ -628,10 +650,57 @@ static const struct drm_bridge_funcs
> rcar_lvds_bridge_ops = {
> .mode_set = rcar_lvds_mode_set,
> };
>
> +bool rcar_lvds_dual_link(struct drm_bridge *bridge)
> +{
> + struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
> +
> + return lvds->dual_link;
> +}
> +EXPORT_SYMBOL_GPL(rcar_lvds_dual_link);
> +
> /*
> -
> * Probe & Remove
> */
>
> +static int rcar_lvds_parse_dt_companion(struct rcar_lvds *lvds)
> +{
> + const struct of_device_id *match;
> + struct device_node *companion;
> + struct device *dev = lvds->dev;
> + int ret = 0;
> +
> + /* Locate the companion LVDS encoder for dual-link operation, if any. */
> + companion =
Re: [git pull] drm fixes for v5.2-rc4 (v2)
On Sat, 8 Jun 2019 at 03:24, Linus Torvalds wrote: > > On Fri, Jun 7, 2019 at 10:20 AM Linus Torvalds > wrote: > > > > The second one has the subject, and mentions nouveau, but doesn't > > actually have the tag name or the expected diffstat and shortlog. > > Hmm. I'm guessing you meant for me to pull the > > 'tags/drm-fixes-2019-06-07-1' > > thing, which looks likely, but I'd like to have confirmation. > > Linus Oh man, sorry have a cold/flu thing, brain isn't running so well. Here's the missing bits. drm-fixes-2019-06-07-1: drm i915, amdgpu, arm display, atomic update fixes + nouveau firmware loading fix The following changes since commit f2c7c76c5d0a443053e94adb9f0918fa2fb85c3a: Linux 5.2-rc3 (2019-06-02 13:55:33 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2019-06-07-1 for you to fetch changes up to 671e2ee5ee2127179ca884b439ab6001a623edd6: Merge branch 'linux-5.2' of git://github.com/skeggsb/linux into drm-fixes (2019-06-07 17:16:00 +1000) drm i915, amdgpu, arm display, atomic update fixes + nouveau firmware loading fix Aleksei Gimbitskii (2): drm/i915/gvt: Check if cur_pt_type is valid drm/i915/gvt: Assign NULL to the pointer after memory free. Ben Skeggs (6): drm/nouveau/core: pass subdev into nvkm_firmware_get, rather than device drm/nouveau/core: support versioned firmware loading drm/nouveau/secboot: pass max supported FW version to LS load funcs drm/nouveau/secboot: split out FW version-specific LS function pointers drm/nouveau/secboot: enable loading of versioned LS PMU/SEC2 ACR msgqueue FW drm/nouveau/secboot/gp10[2467]: support newer FW to fix SEC2 failures on some boards Chengming Gui (1): drm/amd/powerplay: add set_power_profile_mode for raven1_refresh Colin Xu (3): drm/i915/gvt: Update force-to-nonpriv register whitelist drm/i915/gvt: Fix GFX_MODE handling drm/i915/gvt: Fix vGPU CSFE_CHICKEN1_REG mmio handler Dan Carpenter (1): drm/komeda: Potential error pointer dereference Dave Airlie (6): Merge tag 'drm-intel-fixes-2019-06-03' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Merge branch 'drm-fixes-5.2' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Merge tag 'drm-misc-fixes-2019-06-05' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge branch 'malidp-fixes' of git://linux-arm.org/linux-ld into drm-fixes Merge tag 'drm-intel-fixes-2019-06-06' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes Merge branch 'linux-5.2' of git://github.com/skeggsb/linux into drm-fixes Gao, Fred (1): drm/i915/gvt: Fix cmd length of VEB_DI_IECP Helen Koike (5): drm/rockchip: fix fb references in async update drm/amd: fix fb references in async update drm/msm: fix fb references in async update drm/vc4: fix fb references in async update drm: don't block fb changes for async plane updates Joonas Lahtinen (2): Merge tag 'gvt-fixes-2019-05-30' of https://github.com/intel/gvt-linux into drm-intel-fixes Merge tag 'gvt-fixes-2019-06-05' of https://github.com/intel/gvt-linux into drm-intel-fixes Louis Li (1): drm/amdgpu: fix ring test failure issue during s3 in vce 3.0 (V2) Lowry Li (Arm Technology China) (1): drm/komeda: fixing of DMA mapping sg segment warning Lucas Stach (1): udmabuf: actually unmap the scatterlist Prike Liang (1): drm/amd/amdgpu: add RLC firmware to support raven1 refresh Robin Murphy (2): drm/arm/hdlcd: Actually validate CRTC modes drm/arm/hdlcd: Allow a bit of clock tolerance Tina Zhang (1): drm/i915/gvt: Initialize intel_gvt_gtt_entry in stack Tvrtko Ursulin (1): drm/i915/icl: Add WaDisableBankHangMode Weinan Li (1): drm/i915/gvt: add F_CMD_ACCESS flag for wa regs Wen He (1): drm/arm/mali-dp: Add a loop around the second set CVAL and try 5 times Xiaolin Zhang (1): drm/i915/gvt: save RING_HEAD into vreg when vgpu switched out Xiong Zhang (1): drm/i915/gvt: refine ggtt range validation YueHaibing (1): drm/komeda: remove set but not used variable 'kcrtc' james qian wang (Arm Technology China) (1): drm/komeda: Constify the usage of komeda_component/pipeline/dev_funcs drivers/dma-buf/udmabuf.c | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 15 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_pm.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c| 4 +- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 12 - drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +- drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c| 1 +
Re: [PATCH v2 hmm 10/11] mm/hmm: Do not use list*_rcu() for hmm->ranges
On Thu, Jun 06, 2019 at 03:44:37PM -0300, Jason Gunthorpe wrote: > From: Jason Gunthorpe > > This list is always read and written while holding hmm->lock so there is > no need for the confusing _rcu annotations. > > Signed-off-by: Jason Gunthorpe > Reviewed-by: Jérôme Glisse Reviewed-by: Ira Weiny > --- > mm/hmm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/hmm.c b/mm/hmm.c > index c2fecb3ecb11e1..709d138dd49027 100644 > --- a/mm/hmm.c > +++ b/mm/hmm.c > @@ -911,7 +911,7 @@ int hmm_range_register(struct hmm_range *range, > mutex_lock(>lock); > > range->hmm = hmm; > - list_add_rcu(>list, >ranges); > + list_add(>list, >ranges); > > /* >* If there are any concurrent notifiers we have to wait for them for > @@ -941,7 +941,7 @@ void hmm_range_unregister(struct hmm_range *range) > return; > > mutex_lock(>lock); > - list_del_rcu(>list); > + list_del(>list); > mutex_unlock(>lock); > > /* Drop reference taken by hmm_range_register() */ > -- > 2.21.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 09/11] mm/hmm: Poison hmm_range during unregister
On Thu, Jun 06, 2019 at 03:44:36PM -0300, Jason Gunthorpe wrote: > From: Jason Gunthorpe > > Trying to misuse a range outside its lifetime is a kernel bug. Use WARN_ON > and poison bytes to detect this condition. > > Signed-off-by: Jason Gunthorpe > Reviewed-by: Jérôme Glisse > --- > v2 > - Keep range start/end valid after unregistration (Jerome) > --- > mm/hmm.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/mm/hmm.c b/mm/hmm.c > index 6802de7080d172..c2fecb3ecb11e1 100644 > --- a/mm/hmm.c > +++ b/mm/hmm.c > @@ -937,7 +937,7 @@ void hmm_range_unregister(struct hmm_range *range) > struct hmm *hmm = range->hmm; > > /* Sanity check this really should not happen. */ > - if (hmm == NULL || range->end <= range->start) > + if (WARN_ON(range->end <= range->start)) > return; > > mutex_lock(>lock); > @@ -948,7 +948,10 @@ void hmm_range_unregister(struct hmm_range *range) > range->valid = false; > mmput(hmm->mm); > hmm_put(hmm); > - range->hmm = NULL; > + > + /* The range is now invalid, leave it poisoned. */ > + range->valid = false; No need to set valid false again as you just did this 5 lines above. Reviewed-by: Ira Weiny > + memset(>hmm, POISON_INUSE, sizeof(range->hmm)); > } > EXPORT_SYMBOL(hmm_range_unregister); > > -- > 2.21.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
On Fri, Jun 07, 2019 at 10:31:07AM -0300, Jason Gunthorpe wrote:
> The wait_event_timeout macro already tests the condition as its first
> action, so there is no reason to open code another version of this, all
> that does is skip the might_sleep() debugging in common cases, which is
> not helpful.
>
> Further, based on prior patches, we can now simplify the required condition
> test:
> - If range is valid memory then so is range->hmm
> - If hmm_release() has run then range->valid is set to false
>at the same time as dead, so no reason to check both.
> - A valid hmm has a valid hmm->mm.
>
> Allowing the return value of wait_event_timeout() (along with its internal
> barriers) to compute the result of the function.
>
> Signed-off-by: Jason Gunthorpe
Reviewed-by: Ira Weiny
> ---
> v3
> - Simplify the wait_event_timeout to not check valid
> ---
> include/linux/hmm.h | 13 ++---
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 1d97b6d62c5bcf..26e7c477490c4e 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -209,17 +209,8 @@ static inline unsigned long hmm_range_page_size(const
> struct hmm_range *range)
> static inline bool hmm_range_wait_until_valid(struct hmm_range *range,
> unsigned long timeout)
> {
> - /* Check if mm is dead ? */
> - if (range->hmm == NULL || range->hmm->dead || range->hmm->mm == NULL) {
> - range->valid = false;
> - return false;
> - }
> - if (range->valid)
> - return true;
> - wait_event_timeout(range->hmm->wq, range->valid || range->hmm->dead,
> -msecs_to_jiffies(timeout));
> - /* Return current valid status just in case we get lucky */
> - return range->valid;
> + return wait_event_timeout(range->hmm->wq, range->valid,
> + msecs_to_jiffies(timeout)) != 0;
> }
>
> /*
> --
> 2.21.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 03/10] drm: bridge: thc63: Report input bus mode through bridge timings
Hi Laurent,
On 28/05/2019 15:12, Laurent Pinchart wrote:
> Set a drm_bridge_timings in the drm_bridge, and use it to report the
> input bus mode (single-link or dual-link). The other fields of the
> timings structure are kept to 0 as they do not apply to LVDS buses.
>
> Signed-off-by: Laurent Pinchart
> Reviewed-by: Jacopo Mondi
> Tested-by: Jacopo Mondi
Reviewed-by: Kieran Bingham
> ---
> Changes since v1:
>
> - Ignore disabled remote device
> ---
> drivers/gpu/drm/bridge/thc63lvd1024.c | 54 +--
> 1 file changed, 43 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c
> b/drivers/gpu/drm/bridge/thc63lvd1024.c
> index b083a740565c..709dd28b43d6 100644
> --- a/drivers/gpu/drm/bridge/thc63lvd1024.c
> +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
> @@ -31,6 +31,8 @@ struct thc63_dev {
>
> struct drm_bridge bridge;
> struct drm_bridge *next;
> +
> + struct drm_bridge_timings timings;
These are just input timings right?
> };
>
> static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge)
> @@ -48,15 +50,28 @@ static int thc63_attach(struct drm_bridge *bridge)
> static enum drm_mode_status thc63_mode_valid(struct drm_bridge *bridge,
> const struct drm_display_mode *mode)
> {
> + struct thc63_dev *thc63 = to_thc63(bridge);
> + unsigned int min_freq;
> + unsigned int max_freq;
> +
> /*
> - * The THC63LVD1024 clock frequency range is 8 to 135 MHz in single-in
> - * mode. Note that the limits are different in dual-in, single-out mode,
> - * and will need to be adjusted accordingly.
> + * The THC63LVD1024 pixel rate range is 8 to 135 MHz in all modes but
> + * dual-in, single-out where it is 40 to 150 MHz. As dual-in, dual-out
That comma is unfortunate, and makes me read the sentence as "in all
modes but dual-in, ... ... single out where it is 40 to 150 mhz (as if
we should single out the device for it's behaviour between 40 to 150mhz).
Perhaps we could enclose the mode in single quotes to denote that it is
a single description:
... in all modes but 'dual-in, single-out' where it ...
> + * isn't supported by the driver yet, simply derive the limits from the
> + * input mode.
>*/
> - if (mode->clock < 8000)
> + if (thc63->timings.dual_link) {
> + min_freq = 4;
> + max_freq = 15;
> + } else {
> + min_freq = 8000;
> + max_freq = 135000;
> + }
> +
> + if (mode->clock < min_freq)
> return MODE_CLOCK_LOW;
>
> - if (mode->clock > 135000)
> + if (mode->clock > max_freq)
> return MODE_CLOCK_HIGH;
>
> return MODE_OK;
> @@ -101,19 +116,19 @@ static const struct drm_bridge_funcs thc63_bridge_func
> = {
>
> static int thc63_parse_dt(struct thc63_dev *thc63)
> {
> - struct device_node *thc63_out;
> + struct device_node *endpoint;
> struct device_node *remote;
>
> - thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> - THC63_RGB_OUT0, -1);
> - if (!thc63_out) {
> + endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> + THC63_RGB_OUT0, -1);
> + if (!endpoint) {
> dev_err(thc63->dev, "Missing endpoint in port@%u\n",
> THC63_RGB_OUT0);
> return -ENODEV;
> }
>
> - remote = of_graph_get_remote_port_parent(thc63_out);
> - of_node_put(thc63_out);
> + remote = of_graph_get_remote_port_parent(endpoint);
> + of_node_put(endpoint);
> if (!remote) {
> dev_err(thc63->dev, "Endpoint in port@%u unconnected\n",
> THC63_RGB_OUT0);
> @@ -132,6 +147,22 @@ static int thc63_parse_dt(struct thc63_dev *thc63)
> if (!thc63->next)
> return -EPROBE_DEFER;
>
> + endpoint = of_graph_get_endpoint_by_regs(thc63->dev->of_node,
> + THC63_LVDS_IN1, -1);
> + if (endpoint) {
> + remote = of_graph_get_remote_port_parent(endpoint);
> + of_node_put(endpoint);
> +
> + if (remote) {
> + if (of_device_is_available(remote))
> + thc63->timings.dual_link = true;
> + of_node_put(remote);
> + }
> + }
> +
> + dev_dbg(thc63->dev, "operating in %s-link mode\n",
> + thc63->timings.dual_link ? "dual" : "single");
> +
> return 0;
> }
>
> @@ -188,6 +219,7 @@ static int thc63_probe(struct platform_device *pdev)
> thc63->bridge.driver_private = thc63;
> thc63->bridge.of_node = pdev->dev.of_node;
> thc63->bridge.funcs = _bridge_func;
> + thc63->bridge.timings = >timings;
>
> drm_bridge_add(>bridge);
>
>
--
Regards
--
Kieran
Re: [PATCH v2 hmm 04/11] mm/hmm: Simplify hmm_get_or_create and make it reliable
On Thu, Jun 06, 2019 at 03:44:31PM -0300, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> As coded this function can false-fail in various racy situations. Make it
> reliable by running only under the write side of the mmap_sem and avoiding
> the false-failing compare/exchange pattern.
>
> Also make the locking very easy to understand by only ever reading or
> writing mm->hmm while holding the write side of the mmap_sem.
>
> Signed-off-by: Jason Gunthorpe
Reviewed-by: Ira Weiny
> ---
> v2:
> - Fix error unwind of mmgrab (Jerome)
> - Use hmm local instead of 2nd container_of (Jerome)
> ---
> mm/hmm.c | 80
> 1 file changed, 29 insertions(+), 51 deletions(-)
>
> diff --git a/mm/hmm.c b/mm/hmm.c
> index cc7c26fda3300e..dc30edad9a8a02 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -40,16 +40,6 @@
> #if IS_ENABLED(CONFIG_HMM_MIRROR)
> static const struct mmu_notifier_ops hmm_mmu_notifier_ops;
>
> -static inline struct hmm *mm_get_hmm(struct mm_struct *mm)
> -{
> - struct hmm *hmm = READ_ONCE(mm->hmm);
> -
> - if (hmm && kref_get_unless_zero(>kref))
> - return hmm;
> -
> - return NULL;
> -}
> -
> /**
> * hmm_get_or_create - register HMM against an mm (HMM internal)
> *
> @@ -64,11 +54,20 @@ static inline struct hmm *mm_get_hmm(struct mm_struct *mm)
> */
> static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> {
> - struct hmm *hmm = mm_get_hmm(mm);
> - bool cleanup = false;
> + struct hmm *hmm;
>
> - if (hmm)
> - return hmm;
> + lockdep_assert_held_exclusive(>mmap_sem);
> +
> + if (mm->hmm) {
> + if (kref_get_unless_zero(>hmm->kref))
> + return mm->hmm;
> + /*
> + * The hmm is being freed by some other CPU and is pending a
> + * RCU grace period, but this CPU can NULL now it since we
> + * have the mmap_sem.
> + */
> + mm->hmm = NULL;
> + }
>
> hmm = kmalloc(sizeof(*hmm), GFP_KERNEL);
> if (!hmm)
> @@ -83,57 +82,36 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> hmm->notifiers = 0;
> hmm->dead = false;
> hmm->mm = mm;
> - mmgrab(hmm->mm);
> -
> - spin_lock(>page_table_lock);
> - if (!mm->hmm)
> - mm->hmm = hmm;
> - else
> - cleanup = true;
> - spin_unlock(>page_table_lock);
>
> - if (cleanup)
> - goto error;
> -
> - /*
> - * We should only get here if hold the mmap_sem in write mode ie on
> - * registration of first mirror through hmm_mirror_register()
> - */
> hmm->mmu_notifier.ops = _mmu_notifier_ops;
> - if (__mmu_notifier_register(>mmu_notifier, mm))
> - goto error_mm;
> + if (__mmu_notifier_register(>mmu_notifier, mm)) {
> + kfree(hmm);
> + return NULL;
> + }
>
> + mmgrab(hmm->mm);
> + mm->hmm = hmm;
> return hmm;
> -
> -error_mm:
> - spin_lock(>page_table_lock);
> - if (mm->hmm == hmm)
> - mm->hmm = NULL;
> - spin_unlock(>page_table_lock);
> -error:
> - mmdrop(hmm->mm);
> - kfree(hmm);
> - return NULL;
> }
>
> static void hmm_free_rcu(struct rcu_head *rcu)
> {
> - kfree(container_of(rcu, struct hmm, rcu));
> + struct hmm *hmm = container_of(rcu, struct hmm, rcu);
> +
> + down_write(>mm->mmap_sem);
> + if (hmm->mm->hmm == hmm)
> + hmm->mm->hmm = NULL;
> + up_write(>mm->mmap_sem);
> + mmdrop(hmm->mm);
> +
> + kfree(hmm);
> }
>
> static void hmm_free(struct kref *kref)
> {
> struct hmm *hmm = container_of(kref, struct hmm, kref);
> - struct mm_struct *mm = hmm->mm;
> -
> - mmu_notifier_unregister_no_release(>mmu_notifier, mm);
>
> - spin_lock(>page_table_lock);
> - if (mm->hmm == hmm)
> - mm->hmm = NULL;
> - spin_unlock(>page_table_lock);
> -
> - mmdrop(hmm->mm);
> + mmu_notifier_unregister_no_release(>mmu_notifier, hmm->mm);
> mmu_notifier_call_srcu(>rcu, hmm_free_rcu);
> }
>
> --
> 2.21.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 03/11] mm/hmm: Hold a mmgrab from hmm to mm
On Thu, Jun 06, 2019 at 03:44:30PM -0300, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> So long a a struct hmm pointer exists, so should the struct mm it is
> linked too. Hold the mmgrab() as soon as a hmm is created, and mmdrop() it
> once the hmm refcount goes to zero.
>
> Since mmdrop() (ie a 0 kref on struct mm) is now impossible with a !NULL
> mm->hmm delete the hmm_hmm_destroy().
>
> Signed-off-by: Jason Gunthorpe
> Reviewed-by: Jérôme Glisse
Reviewed-by: Ira Weiny
> ---
> v2:
> - Fix error unwind paths in hmm_get_or_create (Jerome/Jason)
> ---
> include/linux/hmm.h | 3 ---
> kernel/fork.c | 1 -
> mm/hmm.c| 22 --
> 3 files changed, 4 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 2d519797cb134a..4ee3acabe5ed22 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -586,14 +586,11 @@ static inline int hmm_vma_fault(struct hmm_mirror
> *mirror,
> }
>
> /* Below are for HMM internal use only! Not to be used by device driver! */
> -void hmm_mm_destroy(struct mm_struct *mm);
> -
> static inline void hmm_mm_init(struct mm_struct *mm)
> {
> mm->hmm = NULL;
> }
> #else /* IS_ENABLED(CONFIG_HMM_MIRROR) */
> -static inline void hmm_mm_destroy(struct mm_struct *mm) {}
> static inline void hmm_mm_init(struct mm_struct *mm) {}
> #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index b2b87d450b80b5..588c768ae72451 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -673,7 +673,6 @@ void __mmdrop(struct mm_struct *mm)
> WARN_ON_ONCE(mm == current->active_mm);
> mm_free_pgd(mm);
> destroy_context(mm);
> - hmm_mm_destroy(mm);
> mmu_notifier_mm_destroy(mm);
> check_mm(mm);
> put_user_ns(mm->user_ns);
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 8796447299023c..cc7c26fda3300e 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -82,6 +83,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> hmm->notifiers = 0;
> hmm->dead = false;
> hmm->mm = mm;
> + mmgrab(hmm->mm);
>
> spin_lock(>page_table_lock);
> if (!mm->hmm)
> @@ -109,6 +111,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> mm->hmm = NULL;
> spin_unlock(>page_table_lock);
> error:
> + mmdrop(hmm->mm);
> kfree(hmm);
> return NULL;
> }
> @@ -130,6 +133,7 @@ static void hmm_free(struct kref *kref)
> mm->hmm = NULL;
> spin_unlock(>page_table_lock);
>
> + mmdrop(hmm->mm);
> mmu_notifier_call_srcu(>rcu, hmm_free_rcu);
> }
>
> @@ -138,24 +142,6 @@ static inline void hmm_put(struct hmm *hmm)
> kref_put(>kref, hmm_free);
> }
>
> -void hmm_mm_destroy(struct mm_struct *mm)
> -{
> - struct hmm *hmm;
> -
> - spin_lock(>page_table_lock);
> - hmm = mm_get_hmm(mm);
> - mm->hmm = NULL;
> - if (hmm) {
> - hmm->mm = NULL;
> - hmm->dead = true;
> - spin_unlock(>page_table_lock);
> - hmm_put(hmm);
> - return;
> - }
> -
> - spin_unlock(>page_table_lock);
> -}
> -
> static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> {
> struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
> --
> 2.21.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 02/10] dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation
Hi Laurent,
On 07/06/2019 23:30, Laurent Pinchart wrote:
> Hi Kieran,
>
> On Fri, Jun 07, 2019 at 11:15:06PM +0100, Kieran Bingham wrote:
>> On 28/05/2019 15:12, Laurent Pinchart wrote:
>>> The THC63LVD1024 LVDS decoder can operate in two modes, single-link or
>>> dual-link. In dual-link mode both input ports are used to carry even-
>>> and odd-numbered pixels separately. Document this in the DT bindings,
>>> along with the related rules governing port and usage.
>>>
>>> Signed-off-by: Laurent Pinchart
>>> Reviewed-by: Jacopo Mondi
>>> Reviewed-by: Rob Herring
>>> Tested-by: Jacopo Mondi
>>> ---
>>> .../bindings/display/bridge/thine,thc63lvd1024.txt | 6 ++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>>> b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>>> index 37f0c04d5a28..d17d1e5820d7 100644
>>> ---
>>> a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>>> +++
>>> b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
>>> @@ -28,6 +28,12 @@ Optional video port nodes:
>>> - port@1: Second LVDS input port
>>> - port@3: Second digital CMOS/TTL parallel output
>>>
>>> +The device can operate in single-link mode or dual-link mode. In
>>> single-link
>>> +mode, all pixels are received on port@0, and port@1 shall not contain any
>>> +endpoint. In dual-link mode, even-numbered pixels are received on port@0
>>> and
>>> +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain
>>> +endpoints.
>>> +
>>
>> Your cover letter details 4 different modes of operation for this part.
>>
>> Do you anticipate the other combinations {Single-in, dual-out; dual-in,
>> dual-out} being supported? Perhaps that would be defined by the relevant
>> endpoints being connected or not ?
>
> I expect that someone might need those modes at some point, but I
> haven't specified them on purpose, as I don't like writing DT bindings
> that can't be tested. I however expoect that those additional modes can
> be derived from the connected endpoints.
>
>> You state that in dual-link mode, both port@0, and port@1 shall contain
>> endpoints, so that implies that you only expect to support dual-in with
>> the 'dual-link' property. If that is correct, should it be stated
>> explicitly?
>
> What do you mean by the 'dual-link' property ? The patch series defines
> no such property.
Aha, my imagination is creating something from all the references to the
word 'dual-link' :-D
Ok - so it is just the existence of the endpoints which will
enable//configure the various modes of operation.
I guess that will become more clear when I get down to the driver patches :)
>
>> Otherwise,
>>
>> Reviewed-by: Kieran Bingham
>>
>>> Example:
>>>
>>>
>
--
Regards
--
Kieran
Re: [PATCH v3 04/10] dt-bindings: display: renesas: lvds: Add renesas,companion property
Hi Laurent, On 28/05/2019 15:12, Laurent Pinchart wrote: > Add a new optional renesas,companion property to point to the companion > LVDS encoder. This is used to support dual-link operation where the main > LVDS encoder splits even-numbered and odd-numbered pixels between the > two LVDS encoders. > > The new property doesn't control the mode of operation, it only > describes the relationship between the master and companion LVDS > encoders. > > Signed-off-by: Laurent Pinchart > Reviewed-by: Jacopo Mondi > Tested-by: Jacopo Mondi > --- > Changes since v2: > > - Clarify when the companion property is required or not allowed > > Changes since v1: > > - Fixed typo > --- > .../devicetree/bindings/display/bridge/renesas,lvds.txt| 7 +++ > 1 file changed, 7 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > index 900a884ad9f5..2d24bd8cbec5 100644 > --- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt > @@ -45,6 +45,13 @@ OF graph bindings specified in > Documentation/devicetree/bindings/graph.txt. > > Each port shall have a single endpoint. > > +Optional properties: > +> +- renesas,companion : phandle to the companion LVDS encoder. This property is > + mandatory for the first LVDS encoder on D3 and E3 SoCs, and shall point to > + the second encoder to be used as a companion in dual-link mode. It shall > not > + be set for any other LVDS encoder. > + I see Sam has already asked for an updated example, so with that: I'm fine with the text above. Reviewed-by: Kieran Bingham > > Example: > > -- Regards -- Kieran
Re: [PATCH v2 hmm 02/11] mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
On Thu, Jun 06, 2019 at 03:44:29PM -0300, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> Ralph observes that hmm_range_register() can only be called by a driver
> while a mirror is registered. Make this clear in the API by passing in the
> mirror structure as a parameter.
>
> This also simplifies understanding the lifetime model for struct hmm, as
> the hmm pointer must be valid as part of a registered mirror so all we
> need in hmm_register_range() is a simple kref_get.
>
> Suggested-by: Ralph Campbell
> Signed-off-by: Jason Gunthorpe
> ---
> v2
> - Include the oneline patch to nouveau_svm.c
> ---
> drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +-
> include/linux/hmm.h | 7 ---
> mm/hmm.c | 15 ++-
> 3 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
> b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index 93ed43c413f0bb..8c92374afcf227 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -649,7 +649,7 @@ nouveau_svm_fault(struct nvif_notify *notify)
> range.values = nouveau_svm_pfn_values;
> range.pfn_shift = NVIF_VMM_PFNMAP_V0_ADDR_SHIFT;
> again:
> - ret = hmm_vma_fault(, true);
> + ret = hmm_vma_fault(>mirror, , true);
> if (ret == 0) {
> mutex_lock(>mutex);
> if (!hmm_vma_range_done()) {
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 688c5ca7068795..2d519797cb134a 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -505,7 +505,7 @@ static inline bool hmm_mirror_mm_is_alive(struct
> hmm_mirror *mirror)
> * Please see Documentation/vm/hmm.rst for how to use the range API.
> */
> int hmm_range_register(struct hmm_range *range,
> -struct mm_struct *mm,
> +struct hmm_mirror *mirror,
> unsigned long start,
> unsigned long end,
> unsigned page_shift);
> @@ -541,7 +541,8 @@ static inline bool hmm_vma_range_done(struct hmm_range
> *range)
> }
>
> /* This is a temporary helper to avoid merge conflict between trees. */
> -static inline int hmm_vma_fault(struct hmm_range *range, bool block)
> +static inline int hmm_vma_fault(struct hmm_mirror *mirror,
> + struct hmm_range *range, bool block)
> {
> long ret;
>
> @@ -554,7 +555,7 @@ static inline int hmm_vma_fault(struct hmm_range *range,
> bool block)
> range->default_flags = 0;
> range->pfn_flags_mask = -1UL;
>
> - ret = hmm_range_register(range, range->vma->vm_mm,
> + ret = hmm_range_register(range, mirror,
>range->start, range->end,
>PAGE_SHIFT);
> if (ret)
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 547002f56a163d..8796447299023c 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -925,13 +925,13 @@ static void hmm_pfns_clear(struct hmm_range *range,
> * Track updates to the CPU page table see include/linux/hmm.h
> */
> int hmm_range_register(struct hmm_range *range,
> -struct mm_struct *mm,
> +struct hmm_mirror *mirror,
> unsigned long start,
> unsigned long end,
> unsigned page_shift)
> {
> unsigned long mask = ((1UL << page_shift) - 1UL);
> - struct hmm *hmm;
> + struct hmm *hmm = mirror->hmm;
>
> range->valid = false;
> range->hmm = NULL;
> @@ -945,15 +945,12 @@ int hmm_range_register(struct hmm_range *range,
> range->start = start;
> range->end = end;
>
> - hmm = hmm_get_or_create(mm);
> - if (!hmm)
> - return -EFAULT;
> -
> /* Check if hmm_mm_destroy() was call. */
> - if (hmm->mm == NULL || hmm->dead) {
> - hmm_put(hmm);
> + if (hmm->mm == NULL || hmm->dead)
> return -EFAULT;
> - }
> +
> + range->hmm = hmm;
I don't think you need this assignment here. In the code below (right after
the mutext_lock()) it is set already. And looks like it remains that way after
the end of the series.
Reviewed-by: Ira Weiny
> + kref_get(>kref);
>
> /* Initialize range to track CPU page table updates. */
> mutex_lock(>lock);
> --
> 2.21.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 02/10] dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation
Hi Kieran,
On Fri, Jun 07, 2019 at 11:15:06PM +0100, Kieran Bingham wrote:
> On 28/05/2019 15:12, Laurent Pinchart wrote:
> > The THC63LVD1024 LVDS decoder can operate in two modes, single-link or
> > dual-link. In dual-link mode both input ports are used to carry even-
> > and odd-numbered pixels separately. Document this in the DT bindings,
> > along with the related rules governing port and usage.
> >
> > Signed-off-by: Laurent Pinchart
> > Reviewed-by: Jacopo Mondi
> > Reviewed-by: Rob Herring
> > Tested-by: Jacopo Mondi
> > ---
> > .../bindings/display/bridge/thine,thc63lvd1024.txt | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > index 37f0c04d5a28..d17d1e5820d7 100644
> > ---
> > a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > +++
> > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > @@ -28,6 +28,12 @@ Optional video port nodes:
> > - port@1: Second LVDS input port
> > - port@3: Second digital CMOS/TTL parallel output
> >
> > +The device can operate in single-link mode or dual-link mode. In
> > single-link
> > +mode, all pixels are received on port@0, and port@1 shall not contain any
> > +endpoint. In dual-link mode, even-numbered pixels are received on port@0
> > and
> > +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain
> > +endpoints.
> > +
>
> Your cover letter details 4 different modes of operation for this part.
>
> Do you anticipate the other combinations {Single-in, dual-out; dual-in,
> dual-out} being supported? Perhaps that would be defined by the relevant
> endpoints being connected or not ?
I expect that someone might need those modes at some point, but I
haven't specified them on purpose, as I don't like writing DT bindings
that can't be tested. I however expoect that those additional modes can
be derived from the connected endpoints.
> You state that in dual-link mode, both port@0, and port@1 shall contain
> endpoints, so that implies that you only expect to support dual-in with
> the 'dual-link' property. If that is correct, should it be stated
> explicitly?
What do you mean by the 'dual-link' property ? The patch series defines
no such property.
> Otherwise,
>
> Reviewed-by: Kieran Bingham
>
> > Example:
> >
> >
--
Regards,
Laurent Pinchart
Re: [PATCH v4 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()
On Fri, Jun 7, 2019 at 12:00 PM Stephen Boyd wrote:
>
> Quoting Iurii Zaikin (2019-06-05 18:29:42)
> > On Fri, May 17, 2019 at 11:22 AM Stephen Boyd wrote:
> > >
> > > Quoting Brendan Higgins (2019-05-14 15:17:10)
> > > > diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
> > > > new file mode 100644
> > > > index 0..fe0f2bae66085
> > > > --- /dev/null
> > > > +++ b/kernel/sysctl-test.c
> > > > +
> > > > +
> > > > +static void sysctl_test_dointvec_happy_single_negative(struct kunit
> > > > *test)
> > > > +{
> > > > + struct ctl_table table = {
> > > > + .procname = "foo",
> > > > + .data = _data.int_0001,
> > > > + .maxlen = sizeof(int),
> > > > + .mode = 0644,
> > > > + .proc_handler = proc_dointvec,
> > > > + .extra1 = _zero,
> > > > + .extra2 = _one_hundred,
> > > > + };
> > > > + char input[] = "-9";
> > > > + size_t len = sizeof(input) - 1;
> > > > + loff_t pos = 0;
> > > > +
> > > > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(, 1, input, ,
> > > > ));
> > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos);
> > > > + KUNIT_EXPECT_EQ(test, -9, *(int *)table.data);
> > >
> > > Is the casting necessary? Or can the macro do a type coercion of the
> > > second parameter based on the first type?
> > Data field is defined as void* so I believe casting is necessary to
> > dereference it as a pointer to an array of ints. I don't think the
> > macro should do any type coercion that == operator wouldn't do.
> > I did change the cast to make it more clear that it's a pointer to an
> > array of ints being dereferenced.
>
> Ok, I still wonder if we should make KUNIT_EXPECT_EQ check the types on
> both sides and cause a build warning/error if the types aren't the same.
> This would be similar to our min/max macros that complain about
> mismatched types in the comparisons. Then if a test developer needs to
> convert one type or the other they could do so with a
> KUNIT_EXPECT_EQ_T() macro that lists the types to coerce both sides to
> explicitly.
Good point. I would definitely like to do this, for me it is only a
question of how difficult it would be to make all that happen.
We will investigate and report back on it.
Thanks for the suggestion! It's a really good idea!
Cheers
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 00/10] R-Car DU: LVDS dual-link mode support
Hi Kieran, On Fri, Jun 07, 2019 at 11:16:37PM +0100, Kieran Bingham wrote: > On 28/05/2019 15:12, Laurent Pinchart wrote: > > Hello everybody, > > > > This patch series implements support for LVDS dual-link mode in the > > R-Car DU and R-Car LVDS encoder drivers, and well as in the thc63lvd1024 > > LVDS decoder driver. > > > > LVDS dual-link is a mode of operation where two individual LVDS links > > are operated together to carry even- and odd-numbered pixels separately. > > This doubles the possible bandwidth of the video transmission. Both the > > transmitter and the receiver need to support this mode of operation. > > > > The R-Car D3 and E3 SoCs include two independent LVDS encoders that can > > be grouped together to operate in dual-link mode. When used separately, > > the LVDS encoders are connected to two different CRTCs and transmit > > independent video streams. When used in dual-link mode, the first LVDS > > encoder is connected to the first CRTC, and split even- and odd-numbered > > pixels. It transmits half of the pixels on its LVDS output, and sends > > the other half to the second LVDS encoder for transmittion over the > > second LVDS link. The second LVDS encoder thus operates under control of > > the first one, and isn't connected directly to a CRTC. > > > > On the receiving side, the THC63LVD1024 LVDS-to-parallel bridge has two > > LVDS inputs and two parallel outputs. It can operate in four different > > modes: > > > > - Single-in, single-out: The first LVDS input receives the video stream, > > and the bridge outputs it on the first parallel output. The second > > LVDS input and the second parallel output are not used. > > > > - Single-in, dual-out: The first LVDS input receives the video stream, > > and the bridge splits even- and odd-numbered pixels and outputs them > > on the first and second parallel outputs. The second LVDS input is not > > used. > > > > - Dual-in, single-out: The two LVDS inputs are used in dual-link mode, > > and the bridge combines the even- and odd-numbered pixels and outputs > > them on the first parallel output. The second parallel output is not > > used. > > > > - Dual-in, dual-out: The two LVDS inputs are used in dual-link mode, > > and the bridge outputs the even- and odd-numbered pixels on the first > > parallel output. > > Clarifying this, having checked with you, Dual-in, dual-out means 'even > pixels are received on the first input, and provided on the first > output, and odd pixels are received on the second input, and provided on > the second output'. Yes, my bad, this is wrong in the cover letter. > > The operating mode is selected by two input pins of the bridge, which > > are connected to DIP switches on the development boards I use. The mode > > is thus fixed from a Linux point of view. > > Would there ever by a scenario where these could be connected to GPIO's > and changed dynamically? I guess that might not make much sense - as the > configuration is more use case dependant. This can't be ruled out, but the use cases would indeed be limited. If the need ever arises, we can just extend the DT bindings to specify those GPIOs, and add a new in-kernel API at the drm_bridge level to configure the mode dynamically. > > Patch 01/10 adds a new dual_link boolen field to the drm_bridge_timings > > structure to let bridges report their LVDS mode of operation. Patch > > 02/10 clarifies the THC63LVD1024 DT bindings to document dual-link > > operation, and patch 03/10 implements dual-link support in the > > thc64lvd1024 bridge driver by setting the drm_bridge_timings dual_link > > field according to the mode selected through DT. > > > > Patch 04/10 extends the R-Car LVDS DT bindings to specify the companion > > LVDS encoder for dual-link operation. Patches 05/10 then performs a > > small cleanup in the LVDS encoder driver. Patch 06/10 implements > > dual-link support in the LVDS encoder driver, which involves retrieving > > the operation mode from the LVDS receiver, locating the companion LVDS > > encoder, and configuring both encoders when dual-link operation is > > desired. The API towards the DU driver is also extended to report the > > mode of operation. > > > > Patch 07/10 implements dual-link mode support in the DU driver. There is > > no specific configuration to be performed there, as dual-link is fully > > implemented in the LVDS encoder driver, but the DU driver has to skip > > creation of the DRM encoder and connector related to the second LVDS > > encoder when dual-link is used, as the second LVDS encoder operates as a > > slave of the first one, transparently from a CRTC (and thus userspace) > > perspective. > > > > Patch 08/10 specifies the companion LVDS encoder in the D3 and E3 DT > > bindings. This by itself doesn't enable dual-link mode, the LVDS0 > > encoder is still connected to the HDMI output through LVDS receiver, and > > the LVDS1 encoder is not used. Patches 09/10 and 10/10, not intended to > >
Re: [PATCH v3 00/10] R-Car DU: LVDS dual-link mode support
Hi Laurent, On 28/05/2019 15:12, Laurent Pinchart wrote: > Hello everybody, > > This patch series implements support for LVDS dual-link mode in the > R-Car DU and R-Car LVDS encoder drivers, and well as in the thc63lvd1024 > LVDS decoder driver. > > LVDS dual-link is a mode of operation where two individual LVDS links > are operated together to carry even- and odd-numbered pixels separately. > This doubles the possible bandwidth of the video transmission. Both the > transmitter and the receiver need to support this mode of operation. > > The R-Car D3 and E3 SoCs include two independent LVDS encoders that can > be grouped together to operate in dual-link mode. When used separately, > the LVDS encoders are connected to two different CRTCs and transmit > independent video streams. When used in dual-link mode, the first LVDS > encoder is connected to the first CRTC, and split even- and odd-numbered > pixels. It transmits half of the pixels on its LVDS output, and sends > the other half to the second LVDS encoder for transmittion over the > second LVDS link. The second LVDS encoder thus operates under control of > the first one, and isn't connected directly to a CRTC. > > On the receiving side, the THC63LVD1024 LVDS-to-parallel bridge has two > LVDS inputs and two parallel outputs. It can operate in four different > modes: > > - Single-in, single-out: The first LVDS input receives the video stream, > and the bridge outputs it on the first parallel output. The second > LVDS input and the second parallel output are not used. > > - Single-in, dual-out: The first LVDS input receives the video stream, > and the bridge splits even- and odd-numbered pixels and outputs them > on the first and second parallel outputs. The second LVDS input is not > used. > > - Dual-in, single-out: The two LVDS inputs are used in dual-link mode, > and the bridge combines the even- and odd-numbered pixels and outputs > them on the first parallel output. The second parallel output is not > used. > > - Dual-in, dual-out: The two LVDS inputs are used in dual-link mode, > and the bridge outputs the even- and odd-numbered pixels on the first > parallel output. Clarifying this, having checked with you, Dual-in, dual-out means 'even pixels are received on the first input, and provided on the first output, and odd pixels are received on the second input, and provided on the second output'. > The operating mode is selected by two input pins of the bridge, which > are connected to DIP switches on the development boards I use. The mode > is thus fixed from a Linux point of view. Would there ever by a scenario where these could be connected to GPIO's and changed dynamically? I guess that might not make much sense - as the configuration is more use case dependant. > Patch 01/10 adds a new dual_link boolen field to the drm_bridge_timings > structure to let bridges report their LVDS mode of operation. Patch > 02/10 clarifies the THC63LVD1024 DT bindings to document dual-link > operation, and patch 03/10 implements dual-link support in the > thc64lvd1024 bridge driver by setting the drm_bridge_timings dual_link > field according to the mode selected through DT. > > Patch 04/10 extends the R-Car LVDS DT bindings to specify the companion > LVDS encoder for dual-link operation. Patches 05/10 then performs a > small cleanup in the LVDS encoder driver. Patch 06/10 implements > dual-link support in the LVDS encoder driver, which involves retrieving > the operation mode from the LVDS receiver, locating the companion LVDS > encoder, and configuring both encoders when dual-link operation is > desired. The API towards the DU driver is also extended to report the > mode of operation. > > Patch 07/10 implements dual-link mode support in the DU driver. There is > no specific configuration to be performed there, as dual-link is fully > implemented in the LVDS encoder driver, but the DU driver has to skip > creation of the DRM encoder and connector related to the second LVDS > encoder when dual-link is used, as the second LVDS encoder operates as a > slave of the first one, transparently from a CRTC (and thus userspace) > perspective. > > Patch 08/10 specifies the companion LVDS encoder in the D3 and E3 DT > bindings. This by itself doesn't enable dual-link mode, the LVDS0 > encoder is still connected to the HDMI output through LVDS receiver, and > the LVDS1 encoder is not used. Patches 09/10 and 10/10, not intended to > be merged, enable dual-link operation for the D3 and E3 boards for > testing and require flipping DIP switches on the boards. > > The patches are based on top of my drm/du/next branch, and are available > for convenience at > > git://linuxtv.org/pinchartl/media.git drm/du/lvds/dual-link > > They have been tested successfully on the D3 Draak board. I expect them > to work on E3 as well, but I don't have access to an Ebisu board to test > this. > > Laurent Pinchart (10): > drm: bridge: Add dual_link
Re: [PATCH v3 02/10] dt-bindings: display: bridge: thc63lvd1024: Document dual-link operation
Hi Laurent,
On 28/05/2019 15:12, Laurent Pinchart wrote:
> The THC63LVD1024 LVDS decoder can operate in two modes, single-link or
> dual-link. In dual-link mode both input ports are used to carry even-
> and odd-numbered pixels separately. Document this in the DT bindings,
> along with the related rules governing port and usage.
>
> Signed-off-by: Laurent Pinchart
> Reviewed-by: Jacopo Mondi
> Reviewed-by: Rob Herring
> Tested-by: Jacopo Mondi
> ---
> .../bindings/display/bridge/thine,thc63lvd1024.txt | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> index 37f0c04d5a28..d17d1e5820d7 100644
> --- a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> @@ -28,6 +28,12 @@ Optional video port nodes:
> - port@1: Second LVDS input port
> - port@3: Second digital CMOS/TTL parallel output
>
> +The device can operate in single-link mode or dual-link mode. In single-link
> +mode, all pixels are received on port@0, and port@1 shall not contain any
> +endpoint. In dual-link mode, even-numbered pixels are received on port@0 and
> +odd-numbered pixels on port@1, and both port@0 and port@1 shall contain
> +endpoints.
> +
Your cover letter details 4 different modes of operation for this part.
Do you anticipate the other combinations {Single-in, dual-out; dual-in,
dual-out} being supported? Perhaps that would be defined by the relevant
endpoints being connected or not ?
You state that in dual-link mode, both port@0, and port@1 shall contain
endpoints, so that implies that you only expect to support dual-in with
the 'dual-link' property. If that is correct, should it be stated
explicitly?
Otherwise,
Reviewed-by: Kieran Bingham
> Example:
>
>
>
--
Regards
--
Kieran
___
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 #6 from Jon Doane --- I'm also having this particular issue with a Vega 64. It appears that setting any non-stock voltage to any of the power states will cause the voltage at any clock to jump to 1.20v. I haven't been able to find a way around it yet. -- 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 05/10] drm: rcar-du: lvds: Remove LVDS double-enable checks
Hi Laurent,
On 28/05/2019 15:12, Laurent Pinchart wrote:
> The DRM core and DU driver guarantee that the LVDS bridge will not be
> double-enabled or double-disabled. Remove the corresponding unnecessary
> checks.
I'm glad to hear it - that's quite a few WARN_ON's removed which
hopefully is a good thing!
>
> Signed-off-by: Laurent Pinchart
> Reviewed-by: Jacopo Mondi > Tested-by: Jacopo Mondi
>
Reviewed-by: Kieran Bingham
> ---
> drivers/gpu/drm/rcar-du/rcar_lvds.c | 19 ---
> 1 file changed, 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 620b51aab291..a331f0c32187 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -63,7 +63,6 @@ struct rcar_lvds {
> struct clk *extal; /* External clock */
> struct clk *dotclkin[2];/* External DU clocks */
> } clocks;
> - bool enabled;
>
> struct drm_display_mode display_mode;
> enum rcar_lvds_mode mode;
> @@ -368,15 +367,12 @@ int rcar_lvds_clk_enable(struct drm_bridge *bridge,
> unsigned long freq)
>
> dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
>
> - WARN_ON(lvds->enabled);
> -
> ret = clk_prepare_enable(lvds->clocks.mod);
> if (ret < 0)
> return ret;
>
> __rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
>
> - lvds->enabled = true;
> return 0;
> }
> EXPORT_SYMBOL_GPL(rcar_lvds_clk_enable);
> @@ -390,13 +386,9 @@ void rcar_lvds_clk_disable(struct drm_bridge *bridge)
>
> dev_dbg(lvds->dev, "disabling LVDS PLL\n");
>
> - WARN_ON(!lvds->enabled);
> -
> rcar_lvds_write(lvds, LVDPLLCR, 0);
>
> clk_disable_unprepare(lvds->clocks.mod);
> -
> - lvds->enabled = false;
> }
> EXPORT_SYMBOL_GPL(rcar_lvds_clk_disable);
>
> @@ -417,8 +409,6 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> u32 lvdcr0;
> int ret;
>
> - WARN_ON(lvds->enabled);
> -
> ret = clk_prepare_enable(lvds->clocks.mod);
> if (ret < 0)
> return;
> @@ -507,16 +497,12 @@ static void rcar_lvds_enable(struct drm_bridge *bridge)
> drm_panel_prepare(lvds->panel);
> drm_panel_enable(lvds->panel);
> }
> -
> - lvds->enabled = true;
> }
>
> static void rcar_lvds_disable(struct drm_bridge *bridge)
> {
> struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
>
> - WARN_ON(!lvds->enabled);
> -
> if (lvds->panel) {
> drm_panel_disable(lvds->panel);
> drm_panel_unprepare(lvds->panel);
> @@ -527,8 +513,6 @@ static void rcar_lvds_disable(struct drm_bridge *bridge)
> rcar_lvds_write(lvds, LVDPLLCR, 0);
>
> clk_disable_unprepare(lvds->clocks.mod);
> -
> - lvds->enabled = false;
> }
>
> static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
> @@ -592,8 +576,6 @@ static void rcar_lvds_mode_set(struct drm_bridge *bridge,
> {
> struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
>
> - WARN_ON(lvds->enabled);
> -
> lvds->display_mode = *adjusted_mode;
>
> rcar_lvds_get_lvds_mode(lvds);
> @@ -793,7 +775,6 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>
> lvds->dev = >dev;
> lvds->info = of_device_get_match_data(>dev);
> - lvds->enabled = false;
>
> ret = rcar_lvds_parse_dt(lvds);
> if (ret < 0)
>
--
Regards
--
Kieran
[PATCH] drm: remove the newline for CRC source name.
userspace may transfer a newline, and this terminating newline is replaced by a '\0' to avoid followup issues. 'n-1' is the index to replace the newline of CRC source name. v2: update patch subject, body and format. (Sam) Cc: Leo Li Cc: Harry Wentland Cc: Sam Ravnborg Signed-off-by: Dingchen Zhang --- drivers/gpu/drm/drm_debugfs_crc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c index 585169f0dcc5..dac267e840af 100644 --- a/drivers/gpu/drm/drm_debugfs_crc.c +++ b/drivers/gpu/drm/drm_debugfs_crc.c @@ -131,8 +131,8 @@ static ssize_t crc_control_write(struct file *file, const char __user *ubuf, if (IS_ERR(source)) return PTR_ERR(source); - if (source[len] == '\n') - source[len] = '\0'; + if (source[len - 1] == '\n') + source[len - 1] = '\0'; ret = crtc->funcs->verify_crc_source(crtc, source, _cnt); if (ret) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #58 from Richard Thier --- The output from gdb glxgears on the segfault: #0 0xb75a8892 in radeon_winsys_bo_create (rws=0x45e100, size=4096, alignment=4096, domain=RADEON_DOMAIN_GTT, flags=(unknown: 0)) at radeon_drm_bo.c:993 #1 0xb75a972b in radeon_cs_create_fence (rcs=rcs@entry=0xb06a4010) at radeon_drm_cs.c:753 #2 0xb75aa327 in radeon_drm_cs_flush (rcs=0xb06a4010, flags=2, pfence=0xb7d8) at radeon_drm_cs.c:593 #3 0xb757845a in r300_flush_and_cleanup (r300=r300@entry=0x446090, flags=flags@entry=2, fence=fence@entry=0xb7d8) at r300_flush.c:56 #4 0xb757860f in r300_flush (pipe=0x446090, flags=2, fence=0xb7d8) at r300_flush.c:82 #5 0xb731508f in st_context_flush (stctxi=0x55ed20, flags=3, fence=0xb7d8) at state_tracker/st_manager.c:635 #6 0xb74511f8 in dri_flush (cPriv=0x445fd0, dPriv=0x5760d0, flags=3, reason=__DRI2_THROTTLE_SWAPBUFFER) at dri_drawable.c:568 #7 0xb7f9dbe8 in dri2Flush (psc=psc@entry=0x4165d0, ctx=, draw=draw@entry=0x5590e0, flags=3, throttle_reason=__DRI2_THROTTLE_SWAPBUFFER) at dri2_glx.c:553 #8 0xb7f9e09d in dri2SwapBuffers (pdraw=0x5590e0, target_msc=0, divisor=0, remainder=0, flush=1) at dri2_glx.c:845 #9 0xb7f73049 in glXSwapBuffers (dpy=0x407160, drawable=18874370) at glxcmds.c:843 #10 0x00401742 in ?? () #11 0xb7b5c669 in __libc_start_main () from /usr/lib/libc.so.6 #12 0x00401cd5 in ?? () I have changed this relevant part with the added flag: /* Create a fence, which is a dummy BO. */ fence = cs->ws->base.buffer_create(>ws->base, 1, 1, RADEON_DOMAIN_GTT, RADEON_FLAG_NO_SUBALLOC | RADEON_FLAG_NO_INTERPROCESS_SHARING); Currently I am testing this, in latest but the build is still going. Will upload patch for it only if it works. I think it does not harm, but this is happening when the bo is a "fence". I have no idea what a fence is in this terminology, but surely it sounds some synchronization primitive so I am unsure that I can add the flag safely, but knowing earlier it was not in the code likely I can just add it. Will see how this shrinks the GEM_CREATE call numbers. -- 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
[RFC PATCH 2/2] soc: ti: Add Support for the TI Page-based Address Translator (PAT)
This patch adds a driver for the Page-based Address Translator (PAT)
present on various TI SoCs. A PAT device performs address translation
using tables stored in an internal SRAM. Each PAT supports a set number
of pages, each occupying a programmable 4KB, 16KB, 64KB, or 1MB of
addresses in a window for which an incoming transaction will be
translated.
Signed-off-by: Andrew F. Davis
---
drivers/soc/ti/Kconfig | 9 +
drivers/soc/ti/Makefile | 1 +
drivers/soc/ti/ti-pat.c | 569
include/uapi/linux/ti-pat.h | 44 +++
4 files changed, 623 insertions(+)
create mode 100644 drivers/soc/ti/ti-pat.c
create mode 100644 include/uapi/linux/ti-pat.h
diff --git a/drivers/soc/ti/Kconfig b/drivers/soc/ti/Kconfig
index f0be35d3dcba..b838ae74d01f 100644
--- a/drivers/soc/ti/Kconfig
+++ b/drivers/soc/ti/Kconfig
@@ -86,4 +86,13 @@ config TI_SCI_INTA_MSI_DOMAIN
help
Driver to enable Interrupt Aggregator specific MSI Domain.
+config TI_PAT
+ tristate "TI PAT DMA-BUF exporter"
+ select REGMAP
+ help
+ Driver for TI Page-based Address Translator (PAT). This driver
+ provides the an API allowing the remapping of a non-contiguous
+ DMA-BUF into a contiguous one that is sutable for devices needing
+ coniguous memory.
+
endif # SOC_TI
diff --git a/drivers/soc/ti/Makefile b/drivers/soc/ti/Makefile
index b3868d392d4f..1369642b40c3 100644
--- a/drivers/soc/ti/Makefile
+++ b/drivers/soc/ti/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_AMX3_PM) += pm33xx.o
obj-$(CONFIG_WKUP_M3_IPC) += wkup_m3_ipc.o
obj-$(CONFIG_TI_SCI_PM_DOMAINS)+= ti_sci_pm_domains.o
obj-$(CONFIG_TI_SCI_INTA_MSI_DOMAIN) += ti_sci_inta_msi.o
+obj-$(CONFIG_TI_PAT) += ti-pat.o
diff --git a/drivers/soc/ti/ti-pat.c b/drivers/soc/ti/ti-pat.c
new file mode 100644
index ..7359ea0f7ccf
--- /dev/null
+++ b/drivers/soc/ti/ti-pat.c
@@ -0,0 +1,569 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TI PAT mapped DMA-BUF memory re-exporter
+ *
+ * Copyright (C) 2018-2019 Texas Instruments Incorporated - http://www.ti.com/
+ * Andrew F. Davis
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#define TI_PAT_DRIVER_NAME "ti-pat"
+
+/* TI PAT MMRS registers */
+#define TI_PAT_MMRS_PID0x0 /* Revision Register */
+#define TI_PAT_MMRS_CONFIG 0x4 /* Config Register */
+#define TI_PAT_MMRS_CONTROL0x10 /* Control Register */
+
+/* TI PAT CONTROL register field values */
+#define TI_PAT_CONTROL_ARB_MODE_UF 0x0 /* Updates first */
+#define TI_PAT_CONTROL_ARB_MODE_RR 0x2 /* Round-robin */
+
+#define TI_PAT_CONTROL_PAGE_SIZE_4KB 0x0
+#define TI_PAT_CONTROL_PAGE_SIZE_16KB 0x1
+#define TI_PAT_CONTROL_PAGE_SIZE_64KB 0x2
+#define TI_PAT_CONTROL_PAGE_SIZE_1MB 0x3
+
+static unsigned int ti_pat_page_sizes[] = {
+ [TI_PAT_CONTROL_PAGE_SIZE_4KB] = 4 * 1024,
+ [TI_PAT_CONTROL_PAGE_SIZE_16KB] = 16 * 1024,
+ [TI_PAT_CONTROL_PAGE_SIZE_64KB] = 64 * 1024,
+ [TI_PAT_CONTROL_PAGE_SIZE_1MB] = 1024 * 1024,
+};
+
+enum ti_pat_mmrs_fields {
+ /* Revision */
+ F_PID_MAJOR,
+ F_PID_MINOR,
+
+ /* Controls */
+ F_CONTROL_ARB_MODE,
+ F_CONTROL_PAGE_SIZE,
+ F_CONTROL_REPLACE_OID_EN,
+ F_CONTROL_EN,
+
+ /* sentinel */
+ F_MAX_FIELDS
+};
+
+static const struct reg_field ti_pat_mmrs_reg_fields[] = {
+ /* Revision */
+ [F_PID_MAJOR] = REG_FIELD(TI_PAT_MMRS_PID, 8, 10),
+ [F_PID_MINOR] = REG_FIELD(TI_PAT_MMRS_PID, 0, 5),
+ /* Controls */
+ [F_CONTROL_ARB_MODE]= REG_FIELD(TI_PAT_MMRS_CONTROL, 6, 7),
+ [F_CONTROL_PAGE_SIZE] = REG_FIELD(TI_PAT_MMRS_CONTROL, 4, 5),
+ [F_CONTROL_REPLACE_OID_EN] = REG_FIELD(TI_PAT_MMRS_CONTROL, 1, 1),
+ [F_CONTROL_EN] = REG_FIELD(TI_PAT_MMRS_CONTROL, 0, 0),
+};
+
+/**
+ * struct ti_pat_data - PAT device instance data
+ * @dev: PAT device structure
+ * @mdev: misc device
+ * @mmrs_map: Register map of MMRS region
+ * @table_base: Base address of TABLE region
+ */
+struct ti_pat_data {
+ struct device *dev;
+ struct miscdevice mdev;
+ struct regmap *mmrs_map;
+ struct regmap_field *mmrs_fields[F_MAX_FIELDS];
+ void __iomem *table_base;
+ unsigned int page_count;
+ unsigned int page_size;
+ phys_addr_t window_base;
+ struct gen_pool *pool;
+};
+
+struct ti_pat_dma_buf_attachment {
+ struct device *dev;
+ struct sg_table *table;
+ struct ti_pat_buffer *buffer;
+ struct list_head list;
+};
+
+struct ti_pat_buffer {
+ struct ti_pat_data *pat;
+ struct dma_buf *i_dma_buf;
+ size_t size;
+ unsigned long offset;
+ struct dma_buf *e_dma_buf;
+
+ struct
[RFC PATCH 0/2] Support for TI Page-based Address Translator
Hello all, So I've got a new IP on our new SoC I'm looking to make use of and would like some help figuring out what framework best matches its function. The IP is called a "Page-based Address Translator" or PAT. A PAT instance (there are 5 of these things on our J721e device[0]) is basically a really simple IOMMU sitting on the interconnect between the device bus and what is effectively our northbridge called MSMC (DRAM/SRAM/L3-Cache/Coherency controller). Simplified it looks about like this: CPUs | DRAM --- MSMC --- SRAM/L3 | NAVSS - (PATs) | --- Device Bus - | | | | Device Device Device etc.. Each PAT has a set a window in high memory (about 0x48__ area) for which any transaction with an address targeting its window will be routed into that PAT. The PAT then does a simple calculation based on the how far into the window the address is and the current page size, does a lookup to an internally held table of translations, then sends the transaction back out on the interconnect with a new address. Usually this address should be towards somewhere in DRAM, but can be some other device or even back into PAT (I'm not sure there is a valid use-case for this but just a point of interest). My gut reaction is that this is an IOMMU which belongs in the IOMMU subsystem. But there are a couple oddities that make me less sure it is really suitable for the IOMMU framework. First it doesn't sit in front of any devices, it sits in front of *all* devices, this means we would have every device claim it as an IOMMU parent, even though many devices also have a traditional IOMMU connected. Second, there is only a limited window of address space per PAT, this means we will get fragmentation and allocation failures on occasion, in this way it looks to me more like AGP GART. Third, the window is in high-memory, so unlike some IOMMU devices which can be used to allow DMA to high-mem from low-mem only devices, PAT can't be used for that. Lastly it doesn't provide any isolation, if the access does not target the PAT window it is not used (that is not to say we don't have isolation, just that it is taken care of by other parts of the interconnect). This means, to me, that PAT has one main purpose: making physically-contiguous views of scattered pages in system memory for DMA. But it does that really well, the whole translation table is held in a PAT-internal SRAM giving 1 bus cycle latency and at full bus bandwidth. So what are my options here, is IOMMU the right way to go or not? Looking around the kernel I also see the char dev ARP/GART interface which looks like a good fit, but also looks quite dated and my guess deprecated at this point. Moving right along.. Another thing I saw is we have the support upstream of the DMM device[1] available in some OMAPx/AM57x SoCs. I'm a little more familiar with this device. The DMM is a bundle of IPs and in fact one of them is called "PAT" and it even does basically the same thing this incarnation of "PAT" does. It's upstream integration design is a bit questionable unfortunately, the DMM support was integrated into the OMAPDRM display driver, which does make some sense then given its support for rotation (using TILER IP contained in DMM). The issue with this was that the DMM/TILER/PAT IP was not part of the our display IP, but instead out at the end of the shared device bus, inside the external memory controller. Like this new PAT this meant that any IP that could make use of it, but only the display framework could actually provide buffers backed by it. This meant, for instance, if we wanted to decode some video buffer using our video decoder we would have to allocate from DRM framework then pass that over to the V4L2 system. This doesn't make much sense and required the user-space to know about this odd situation and allocate from the right spot or else have to use up valuable CMA space or waste memory with dedicated carveouts. Another idea would be to have this as a special central allocator (exposed through DMA-BUF heaps[2] or ION) that would give out normal system memory as a DMA-BUF but remap it with PAT if a device that only supports contiguous memory tries to attach/map that DMA-BUF. One last option would be to allow user-space to choose to make the buffer contiguous when it needs. That's what the driver in this series allows. We expose a remapping device, user-space passes it a non-contiguous DMA-BUF handle and it passes a contiguous one back. Simple as that. So how do we use this, lets take Android for example, we don't know at allocation time if a rendering buffer will end up going back into the GPU for further processing, or if it will be consumed directly by the display. This is a problem for us as our GPU can work with non-contiguous buffers but our display cannot, so any buffers that could possibly go to the display at some point currently needs to be allocated as contiguous
[RFC PATCH 1/2] dt-bindings: soc: ti: Add TI PAT bindings
This patch adds the bindings for the Page-based Address Translator (PAT)
present on various TI SoCs. A Page-based Address Translator (PAT) device
performs address translation using tables stored in an internal SRAM.
Each PAT supports a set number of pages, each occupying a programmable
4KB, 16KB, 64KB, or 1MB of addresses in a window for which an incoming
transaction will be translated.
Signed-off-by: Andrew F. Davis
---
.../devicetree/bindings/misc/ti,pat.txt | 34 +++
1 file changed, 34 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/ti,pat.txt
diff --git a/Documentation/devicetree/bindings/misc/ti,pat.txt
b/Documentation/devicetree/bindings/misc/ti,pat.txt
new file mode 100644
index ..fac20d45ad4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/ti,pat.txt
@@ -0,0 +1,34 @@
+Texas Instruments Page-based Address Translator (PAT) driver binding
+
+
+A Page-based Address Translator (PAT) device performs address translation
+using tables stored in an internal SRAM. Each PAT supports a set number of
+pages, each occupying a programmable 4KB, 16KB, 64KB, or 1MB of addresses
+in a window for which an incoming transaction will be translated.
+
+TI-PAT controller Device Node
+=
+
+The TI-PAT node describes the Texas Instrument's Page-based Address
+Translator (PAT).
+
+Required properties:
+---
+- compatible: should be "ti,j721e-pat"
+- reg-names:
+ mmrs - Memory mapped registers region
+ table - Location of the table of translation pages
+ window - Window of memory addresses translated by this PAT
+- reg: register addresses corresponding to the above
+
+Example:
+
+navss_pat0: pat@3101 {
+ compatible = "ti,j721e-pat";
+ reg = <0x00 0x3101 0x00 0x0100>,
+ <0x00 0x3640 0x00 0x0004>,
+ <0x48 0x 0x00 0x4000>;
+ reg-names = "mmrs",
+ "table",
+ "window";
+};
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781
--- Comment #57 from Richard Thier ---
Hello everyone!
I have found one more place where we can end up calling extra ioctls for
GEM_CREATE. I remember telling you above after the very first proposed patch of
Marek, that "Interesting to me is that the CREATE call number is still higher
in strace output than how it was originally before the first slowdown"
I went on to find the cause because I was using his patch when bisecting the
minor issue and this makes it really hard to spot the minor issue I mentined
earlier.
So I decided to find out what causes the 600-700 CREATE calls despite the
performance seems fast.
This is what I did:
/* Shared resources don't use cached heaps. */
if (use_reusable_pool) {
int heap = radeon_get_heap_index(domain, flags);
assert(heap >= 0 && heap < RADEON_MAX_CACHED_HEAPS);
usage = 1 << heap; /* Only set one usage bit for each heap. */
pb_cache_bucket = radeon_get_pb_cache_bucket_index(heap);
assert(pb_cache_bucket < ARRAY_SIZE(ws->bo_cache.buckets));
bo = radeon_bo(pb_cache_reclaim_buffer(>bo_cache, size, alignment,
usage, pb_cache_bucket));
if (bo)
return >base;
} else {
// first only logs where here
fprintf(stderr, "1 KULAKVA!!!\n");
fprintf(stderr, "2 KULAKVA!!!\n");
fprintf(stderr, "3 KULAKVA!!!\n");
// this segfault got added later
uint kula = 0;
int *kulakva = (int*)kula;
*kulakva=42;
}
I saw that still I can get to the code path where "KULAKVA" gets printed, while
in the very original code of course this was never the case ever as the whole
if and the bool variable if using reusable pools or not was not existing yet.
I am now recompiling with debug symbols so that running glxgears with gdb will
hopefully fail at the last line of the else clause and tell me a backtrace
where I am coming from ;-)
--
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
[RESEND PATCH] gpu/drm_memory: fix a few warnings
The opening comment mark "/**" is reserved for kernel-doc comments, so it will generate a warning with "make W=1". drivers/gpu/drm/drm_memory.c:2: warning: Cannot understand * \file drm_memory.c Also, silence a checkpatch warning by adding a license identfiter where it indicates the MIT license further down in the source file. WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 Signed-off-by: Qian Cai --- drivers/gpu/drm/drm_memory.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c index 132fef8ff1b6..683042c8ee2c 100644 --- a/drivers/gpu/drm/drm_memory.c +++ b/drivers/gpu/drm/drm_memory.c @@ -1,4 +1,5 @@ -/** +// SPDX-License-Identifier: MIT +/* * \file drm_memory.c * Memory management wrappers for DRM * -- 1.8.3.1
Re: [PATCH v4 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()
Quoting Iurii Zaikin (2019-06-05 18:29:42)
> On Fri, May 17, 2019 at 11:22 AM Stephen Boyd wrote:
> >
> > Quoting Brendan Higgins (2019-05-14 15:17:10)
> > > diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
> > > new file mode 100644
> > > index 0..fe0f2bae66085
> > > --- /dev/null
> > > +++ b/kernel/sysctl-test.c
> > > +
> > > +
> > > +static void sysctl_test_dointvec_happy_single_negative(struct kunit
> > > *test)
> > > +{
> > > + struct ctl_table table = {
> > > + .procname = "foo",
> > > + .data = _data.int_0001,
> > > + .maxlen = sizeof(int),
> > > + .mode = 0644,
> > > + .proc_handler = proc_dointvec,
> > > + .extra1 = _zero,
> > > + .extra2 = _one_hundred,
> > > + };
> > > + char input[] = "-9";
> > > + size_t len = sizeof(input) - 1;
> > > + loff_t pos = 0;
> > > +
> > > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(, 1, input, ,
> > > ));
> > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos);
> > > + KUNIT_EXPECT_EQ(test, -9, *(int *)table.data);
> >
> > Is the casting necessary? Or can the macro do a type coercion of the
> > second parameter based on the first type?
> Data field is defined as void* so I believe casting is necessary to
> dereference it as a pointer to an array of ints. I don't think the
> macro should do any type coercion that == operator wouldn't do.
> I did change the cast to make it more clear that it's a pointer to an
> array of ints being dereferenced.
Ok, I still wonder if we should make KUNIT_EXPECT_EQ check the types on
both sides and cause a build warning/error if the types aren't the same.
This would be similar to our min/max macros that complain about
mismatched types in the comparisons. Then if a test developer needs to
convert one type or the other they could do so with a
KUNIT_EXPECT_EQ_T() macro that lists the types to coerce both sides to
explicitly.
[PATCH v3 06/20] docs: mark orphan documents as such
Sphinx doesn't like orphan documents: Documentation/accelerators/ocxl.rst: WARNING: document isn't included in any toctree Documentation/arm/stm32/overview.rst: WARNING: document isn't included in any toctree Documentation/arm/stm32/stm32f429-overview.rst: WARNING: document isn't included in any toctree Documentation/arm/stm32/stm32f746-overview.rst: WARNING: document isn't included in any toctree Documentation/arm/stm32/stm32f769-overview.rst: WARNING: document isn't included in any toctree Documentation/arm/stm32/stm32h743-overview.rst: WARNING: document isn't included in any toctree Documentation/arm/stm32/stm32mp157-overview.rst: WARNING: document isn't included in any toctree Documentation/gpu/msm-crash-dump.rst: WARNING: document isn't included in any toctree Documentation/interconnect/interconnect.rst: WARNING: document isn't included in any toctree Documentation/laptops/lg-laptop.rst: WARNING: document isn't included in any toctree Documentation/powerpc/isa-versions.rst: WARNING: document isn't included in any toctree Documentation/virtual/kvm/amd-memory-encryption.rst: WARNING: document isn't included in any toctree Documentation/virtual/kvm/vcpu-requests.rst: WARNING: document isn't included in any toctree So, while they aren't on any toctree, add :orphan: to them, in order to silent this warning. Signed-off-by: Mauro Carvalho Chehab Acked-by: Andrew Donnellan --- Documentation/accelerators/ocxl.rst | 2 ++ Documentation/arm/stm32/overview.rst| 2 ++ Documentation/arm/stm32/stm32f429-overview.rst | 2 ++ Documentation/arm/stm32/stm32f746-overview.rst | 2 ++ Documentation/arm/stm32/stm32f769-overview.rst | 2 ++ Documentation/arm/stm32/stm32h743-overview.rst | 2 ++ Documentation/arm/stm32/stm32mp157-overview.rst | 2 ++ Documentation/gpu/msm-crash-dump.rst| 2 ++ Documentation/interconnect/interconnect.rst | 2 ++ Documentation/laptops/lg-laptop.rst | 2 ++ Documentation/powerpc/isa-versions.rst | 2 ++ 11 files changed, 22 insertions(+) diff --git a/Documentation/accelerators/ocxl.rst b/Documentation/accelerators/ocxl.rst index 14cefc020e2d..b1cea19a90f5 100644 --- a/Documentation/accelerators/ocxl.rst +++ b/Documentation/accelerators/ocxl.rst @@ -1,3 +1,5 @@ +:orphan: + OpenCAPI (Open Coherent Accelerator Processor Interface) diff --git a/Documentation/arm/stm32/overview.rst b/Documentation/arm/stm32/overview.rst index 85cfc8410798..f7e734153860 100644 --- a/Documentation/arm/stm32/overview.rst +++ b/Documentation/arm/stm32/overview.rst @@ -1,3 +1,5 @@ +:orphan: + STM32 ARM Linux Overview diff --git a/Documentation/arm/stm32/stm32f429-overview.rst b/Documentation/arm/stm32/stm32f429-overview.rst index 18feda97f483..65bbb1c3b423 100644 --- a/Documentation/arm/stm32/stm32f429-overview.rst +++ b/Documentation/arm/stm32/stm32f429-overview.rst @@ -1,3 +1,5 @@ +:orphan: + STM32F429 Overview == diff --git a/Documentation/arm/stm32/stm32f746-overview.rst b/Documentation/arm/stm32/stm32f746-overview.rst index b5f4b6ce7656..42d593085015 100644 --- a/Documentation/arm/stm32/stm32f746-overview.rst +++ b/Documentation/arm/stm32/stm32f746-overview.rst @@ -1,3 +1,5 @@ +:orphan: + STM32F746 Overview == diff --git a/Documentation/arm/stm32/stm32f769-overview.rst b/Documentation/arm/stm32/stm32f769-overview.rst index 228656ced2fe..f6adac862b17 100644 --- a/Documentation/arm/stm32/stm32f769-overview.rst +++ b/Documentation/arm/stm32/stm32f769-overview.rst @@ -1,3 +1,5 @@ +:orphan: + STM32F769 Overview == diff --git a/Documentation/arm/stm32/stm32h743-overview.rst b/Documentation/arm/stm32/stm32h743-overview.rst index 3458dc00095d..c525835e7473 100644 --- a/Documentation/arm/stm32/stm32h743-overview.rst +++ b/Documentation/arm/stm32/stm32h743-overview.rst @@ -1,3 +1,5 @@ +:orphan: + STM32H743 Overview == diff --git a/Documentation/arm/stm32/stm32mp157-overview.rst b/Documentation/arm/stm32/stm32mp157-overview.rst index 62e176d47ca7..2c52cd020601 100644 --- a/Documentation/arm/stm32/stm32mp157-overview.rst +++ b/Documentation/arm/stm32/stm32mp157-overview.rst @@ -1,3 +1,5 @@ +:orphan: + STM32MP157 Overview === diff --git a/Documentation/gpu/msm-crash-dump.rst b/Documentation/gpu/msm-crash-dump.rst index 757cd257e0d8..240ef200f76c 100644 --- a/Documentation/gpu/msm-crash-dump.rst +++ b/Documentation/gpu/msm-crash-dump.rst @@ -1,3 +1,5 @@ +:orphan: + = MSM Crash Dump Format = diff --git a/Documentation/interconnect/interconnect.rst b/Documentation/interconnect/interconnect.rst index c3e004893796..56e331dab70e 100644 --- a/Documentation/interconnect/interconnect.rst
[PATCH] drm/fb-helper: Unexport cmdline helpers
No longer needed since the i915 initial config logic was pulled into
the shared helper by Noralf.
Spotted while reviewing patches from Ville.
Cc: Noralf Trønnes
Cc: Ville Syrjala
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_fb_helper.c | 9 +
include/drm/drm_fb_helper.h | 5 -
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b9b7c06cbc4f..95079d5c07b8 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2138,7 +2138,9 @@ static int drm_fb_helper_probe_connector_modes(struct
drm_fb_helper *fb_helper,
return count;
}
-struct drm_display_mode *drm_has_preferred_mode(struct drm_fb_helper_connector
*fb_connector, int width, int height)
+static struct drm_display_mode *
+drm_has_preferred_mode(struct drm_fb_helper_connector *fb_connector,
+ int width, int height)
{
struct drm_display_mode *mode;
@@ -2151,14 +2153,14 @@ struct drm_display_mode *drm_has_preferred_mode(struct
drm_fb_helper_connector *
}
return NULL;
}
-EXPORT_SYMBOL(drm_has_preferred_mode);
static bool drm_has_cmdline_mode(struct drm_fb_helper_connector *fb_connector)
{
return fb_connector->connector->cmdline_mode.specified;
}
-struct drm_display_mode *drm_pick_cmdline_mode(struct drm_fb_helper_connector
*fb_helper_conn)
+static struct drm_display_mode *
+drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn)
{
struct drm_cmdline_mode *cmdline_mode;
struct drm_display_mode *mode;
@@ -2208,7 +2210,6 @@ struct drm_display_mode *drm_pick_cmdline_mode(struct
drm_fb_helper_connector *f
list_add(>head, _helper_conn->connector->modes);
return mode;
}
-EXPORT_SYMBOL(drm_pick_cmdline_mode);
static bool drm_connector_enabled(struct drm_connector *connector, bool strict)
{
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 6b334f4d8a22..5a7e5d131913 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -289,11 +289,6 @@ int drm_fb_helper_initial_config(struct drm_fb_helper
*fb_helper, int bpp_sel);
int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper);
int drm_fb_helper_debug_enter(struct fb_info *info);
int drm_fb_helper_debug_leave(struct fb_info *info);
-struct drm_display_mode *
-drm_has_preferred_mode(struct drm_fb_helper_connector *fb_connector,
- int width, int height);
-struct drm_display_mode *
-drm_pick_cmdline_mode(struct drm_fb_helper_connector *fb_helper_conn);
int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct
drm_connector *connector);
int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Throw away the BIOS fb if has the wrong depth/bpp
On Fri, Jun 07, 2019 at 07:26:11PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Respect the user's choice of depth/bpp for the fbdev framebuffer
> and throw out the fb we inherited from the BIOS if it doesn't
> match.
>
> Signed-off-by: Ville Syrjälä
I guess we're going boom right now, which is maybe a bit much? i.e.
Cc: sta...@vger.kernel.org
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/intel_fbdev.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
> b/drivers/gpu/drm/i915/intel_fbdev.c
> index 0d3a6fa674e6..1a935dc74d23 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -199,6 +199,17 @@ static int intelfb_create(struct drm_fb_helper *helper,
> drm_framebuffer_put(_fb->base);
> intel_fb = ifbdev->fb = NULL;
> }
> + if (intel_fb &&
> + (sizes->surface_depth != intel_fb->base.format->depth ||
> + sizes->surface_bpp != intel_fb->base.format->cpp[0] * 8)) {
Bikeshed: A little helper that does all these checks with debug output,
and just one "throw bios fb away" path would look a lot neater.
-Daniel
> + DRM_DEBUG_KMS("BIOS fb using wrong depth/bpp (%d/%d), we
> require (%d/%d),"
> + " releasing it\n",
> + intel_fb->base.format->depth,
> + intel_fb->base.format->cpp[0] * 8,
> + sizes->surface_depth, sizes->surface_bpp);
> + drm_framebuffer_put(_fb->base);
> + intel_fb = ifbdev->fb = NULL;
> + }
> if (!intel_fb || WARN_ON(!intel_fb_obj(_fb->base))) {
> DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n");
> ret = intelfb_alloc(helper, sizes);
> --
> 2.21.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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: [Intel-gfx] [PATCH 3/4] drm/fb-helper: Set up gamma_lut during restore_fbdev_mode_atomic()
On Fri, Jun 07, 2019 at 07:26:10PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> i915 will now refuse to enable a C8 plane unless the gamma_lut
> is already enabled (to avoid scanning out whatever garbage got
> left in the hw LUT previously). So in order to make the fbdev
> code work with C8 we need to program the gamma_lut already
> during restore_fbdev_mode_atomic().
>
> To avoid having to update the hw LUT every time
> restore_fbdev_mode_atomic() is called we hang on to the
> current gamma_lut blob. Note that the first crtc to get
> enabled will dictate the size of the gamma_lut, so this
> approach isn't 100% great for hardware with non-uniform
> crtcs. But that problem already exists in setcmap_atomic()
> so we'll just keep ignoring it.
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/drm_fb_helper.c | 165
> include/drm/drm_fb_helper.h | 7 ++
> 2 files changed, 113 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index bdfa14cd7f6d..0ecec763789f 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -436,10 +436,76 @@ static bool drm_fb_helper_panel_rotation(struct
> drm_mode_set *modeset,
> return true;
> }
>
> +static int setcmap_crtc_atomic(struct drm_atomic_state *state,
> +struct drm_crtc *crtc,
> +struct drm_property_blob *gamma_lut)
> +{
> + struct drm_crtc_state *crtc_state;
> + bool replaced;
> +
> + crtc_state = drm_atomic_get_crtc_state(state, crtc);
> + if (IS_ERR(crtc_state))
> + return PTR_ERR(crtc_state);
> +
> + replaced = drm_property_replace_blob(_state->degamma_lut,
> + NULL);
> + replaced |= drm_property_replace_blob(_state->ctm, NULL);
> + replaced |= drm_property_replace_blob(_state->gamma_lut,
> + gamma_lut);
> + crtc_state->color_mgmt_changed |= replaced;
> +
> + return 0;
> +}
> +
> +static struct drm_property_blob *setcmap_new_gamma_lut(struct drm_crtc *crtc,
> +struct fb_cmap *cmap)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_property_blob *gamma_lut;
> + struct drm_color_lut *lut;
> + int size = crtc->gamma_size;
> + int i;
> +
> + if (!size || cmap->start + cmap->len > size)
> + return ERR_PTR(-EINVAL);
> +
> + gamma_lut = drm_property_create_blob(dev, sizeof(*lut) * size, NULL);
> + if (IS_ERR(gamma_lut))
> + return gamma_lut;
> +
> + lut = gamma_lut->data;
> + if (cmap->start || cmap->len != size) {
> + u16 *r = crtc->gamma_store;
> + u16 *g = r + crtc->gamma_size;
> + u16 *b = g + crtc->gamma_size;
> +
> + for (i = 0; i < cmap->start; i++) {
> + lut[i].red = r[i];
> + lut[i].green = g[i];
> + lut[i].blue = b[i];
> + }
> + for (i = cmap->start + cmap->len; i < size; i++) {
> + lut[i].red = r[i];
> + lut[i].green = g[i];
> + lut[i].blue = b[i];
> + }
> + }
> +
> + for (i = 0; i < cmap->len; i++) {
> + lut[cmap->start + i].red = cmap->red[i];
> + lut[cmap->start + i].green = cmap->green[i];
> + lut[cmap->start + i].blue = cmap->blue[i];
> + }
> +
> + return gamma_lut;
> +}
> +
> static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool
> active)
> {
> + struct fb_info *info = fb_helper->fbdev;
> struct drm_client_dev *client = _helper->client;
> struct drm_device *dev = fb_helper->dev;
> + struct drm_property_blob *gamma_lut;
> struct drm_plane_state *plane_state;
> struct drm_plane *plane;
> struct drm_atomic_state *state;
> @@ -455,6 +521,10 @@ static int restore_fbdev_mode_atomic(struct
> drm_fb_helper *fb_helper, bool activ
> goto out_ctx;
> }
>
> + gamma_lut = fb_helper->gamma_lut;
> + if (gamma_lut)
> + drm_property_blob_get(gamma_lut);
Why the get/put stuff here?
> +
> state->acquire_ctx =
> retry:
> drm_for_each_plane(plane, dev) {
> @@ -476,7 +546,8 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper
> *fb_helper, bool activ
> }
>
> drm_client_for_each_modeset(mode_set, client) {
> - struct drm_plane *primary = mode_set->crtc->primary;
> + struct drm_crtc *crtc = mode_set->crtc;
> + struct drm_plane *primary = crtc->primary;
> unsigned int rotation;
>
> if (drm_fb_helper_panel_rotation(mode_set, )) {
> @@ -489,24 +560,46 @@ static int restore_fbdev_mode_atomic(struct
> drm_fb_helper *fb_helper, bool activ
>
Re: [Intel-gfx] [PATCH 2/4] drm: Refuse to create zero width/height cmdline modes
On Fri, Jun 07, 2019 at 07:26:09PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> If the user specifies zero width/height cmdline mode i915 will
> blow up as the fbdev path will bypass the regular fb sanity
> check that would otherwise have refused to create a framebuffer
> with zero width/height.
>
> The reason I thought to try this is so that I can force a specific
> depth for fbdev without actually having to hardcode the mode
> on the kernel cmdline. Eg. if I pass video=0x0-8 I will get an
> 8bpp framebuffer at my monitor's native resolution.
>
> Signed-off-by: Ville Syrjälä
Makes sense. Maybe we should fix the fbdev helper code and stop it from
bypassing normal drm_fb sanity checks too, but that's another story.
Probably would mean rebasing somehow into Noralf's generic fbdev stuff,
which doesn't use magic direct access to the driver anymore.
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_modes.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 5a07a28fec6d..b36248a5d826 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1593,6 +1593,9 @@ drm_mode_create_from_cmdline_mode(struct drm_device
> *dev,
> {
> struct drm_display_mode *mode;
>
> + if (cmd->xres == 0 || cmd->yres == 0)
> + return NULL;
> +
> if (cmd->cvt)
> mode = drm_cvt_mode(dev,
> cmd->xres, cmd->yres,
> --
> 2.21.0
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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: [Intel-gfx] [PATCH 1/4] drm/fb-helper: Do not assume drm_mode_create_from_cmdline_mode() can't fail
On Fri, Jun 07, 2019 at 07:26:08PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä > > drm_mode_create_from_cmdline_mode() can return NULL, so the caller > should check for that. > > Signed-off-by: Ville Syrjälä Reviewed-by: Daniel Vetter Aside I noticed that we can drop a few EXPORT_SYMBOL here, I'll type a patch. -Daniel > --- > drivers/gpu/drm/drm_fb_helper.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index b9b7c06cbc4f..bdfa14cd7f6d 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -2205,7 +2205,9 @@ struct drm_display_mode *drm_pick_cmdline_mode(struct > drm_fb_helper_connector *f > create_mode: > mode = drm_mode_create_from_cmdline_mode(fb_helper_conn->connector->dev, >cmdline_mode); > - list_add(>head, _helper_conn->connector->modes); > + if (mode) > + list_add(>head, _helper_conn->connector->modes); > + > return mode; > } > EXPORT_SYMBOL(drm_pick_cmdline_mode); > -- > 2.21.0 > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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/komeda: Enable/Disable vblank interrupts
On Fri, Jun 07, 2019 at 03:03:39PM +, Ayan Halder wrote: > One needs to set "(struct drm_device *)->irq_enabled = true" to signal drm > core > to enable vblank interrupts after the hardware has been initialized. > Correspondingly, one needs to disable vblank interrupts by setting > "(struct drm_device *)->irq_enabled = false" at the beginning of the > de-initializing routine. Only if you don't use the drm_irq_install helper. Quoting the kerneldoc in full: /** * @irq_enabled: * * Indicates that interrupt handling is enabled, specifically vblank * handling. Drivers which don't use drm_irq_install() need to set this * to true manually. */ bool irq_enabled; Not entirely sure where you've found your quote, but it's not complete. Cheers, Daniel > > Signed-off-by: Ayan Kumar halder > --- > drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > index 7b5cde14e3ba..b4fd8ee0d05f 100644 > --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c > @@ -204,6 +204,8 @@ struct komeda_kms_dev *komeda_kms_attach(struct > komeda_dev *mdev) > if (err) > goto uninstall_irq; > > + drm->irq_enabled = true; > + > err = drm_dev_register(drm, 0); > if (err) > goto uninstall_irq; > @@ -211,6 +213,7 @@ struct komeda_kms_dev *komeda_kms_attach(struct > komeda_dev *mdev) > return kms; > > uninstall_irq: > + drm->irq_enabled = false; > drm_irq_uninstall(drm); > cleanup_mode_config: > drm_mode_config_cleanup(drm); > @@ -225,6 +228,7 @@ void komeda_kms_detach(struct komeda_kms_dev *kms) > struct drm_device *drm = >base; > struct komeda_dev *mdev = drm->dev_private; > > + drm->irq_enabled = false; > mdev->funcs->disable_irq(mdev); > drm_dev_unregister(drm); > drm_irq_uninstall(drm); > -- > 2.21.0 > -- 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 v3 2/2] drm/rockchip: dw_hdmi: Handle suspend/resume
On Thu, Jun 06, 2019 at 03:58:21PM -0700, Doug Anderson wrote:
> Hi,
>
> On Thu, Jun 6, 2019 at 9:42 AM Sean Paul wrote:
> >
> > On Tue, Jun 04, 2019 at 01:42:07PM -0700, Douglas Anderson wrote:
> > > On Rockchip rk3288-based Chromebooks when you do a suspend/resume
> > > cycle:
> > >
> > > 1. You lose the ability to detect an HDMI device being plugged in.
> > >
> > > 2. If you're using the i2c bus built in to dw_hdmi then it stops
> > > working.
> > >
> > > Let's call the core dw-hdmi's suspend/resume functions to restore
> > > things.
> > >
> > > NOTE: in downstream Chrome OS (based on kernel 3.14) we used the
> > > "late/early" versions of suspend/resume because we found that the VOP
> > > was sometimes resuming before dw_hdmi and then calling into us before
> > > we were fully resumed. For now I have gone back to the normal
> > > suspend/resume because I can't reproduce the problems.
> > >
> > > Signed-off-by: Douglas Anderson
> > > ---
> > >
> > > Changes in v3:
> > > - dw_hdmi_resume() is now a void function (Laurent)
> > >
> > > Changes in v2:
> > > - Add forgotten static (Laurent)
> > > - No empty stub for suspend (Laurent)
> > >
> > > drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 14 ++
> > > 1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > index 4cdc9f86c2e5..7bb0f922b303 100644
> > > --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > > @@ -542,11 +542,25 @@ static int dw_hdmi_rockchip_remove(struct
> > > platform_device *pdev)
> > > return 0;
> > > }
> > >
> > > +static int __maybe_unused dw_hdmi_rockchip_resume(struct device *dev)
> > > +{
> > > + struct rockchip_hdmi *hdmi = dev_get_drvdata(dev);
> > > +
> > > + dw_hdmi_resume(hdmi->hdmi);
> >
> > The rockchip driver is already using the atomic suspend/resume helpers (via
> > the
> > modeset helpers). Would you be able to accomplish the same thing by just
> > moving
> > this call into the encoder enable callback?
> >
> > .enable is called on resume via the atomic commit framework, so everything
> > is
> > ordered properly. Of course, this would reset the dw_hdmi bridge on each
> > enable,
> > but I don't think that would be a problem?
>
> I tried and it sorta kinda half worked, but...
>
> 1. One of the problems solved by this patch is making "hot plug
> detect" work after suspend / resume. AKA: if you have nothing plugged
> in to the HDMI port and then suspend/resume you need to be able to
> detect when something is plugged in. When nothing is plugged in then
> the ".enable" isn't called at resume time.
>
Ahh, ok. So we've hit this with other bridges/dongles as well, and yeah the
solution is to keep the bridge powered up enough to detect hotplug, so you would
need to do some work in .resume
Usually there's a second stage of enable where you power things on more fully
and that is done in .enable
> 2. I'm not so convinced about the whole ordering being correct.
> Unfortunately on my system (Chrome OS running the chromeos-4.19
> kernel) we end up getting an i2c transfer before the ".enable" is
> called. I put a dump_stack() in the i2c transfer:
>
> [ 42.212516] CPU: 0 PID: 1479 Comm: DrmThread Tainted: G C
> 4.19.47 #60
> [ 42.221182] Hardware name: Rockchip (Device Tree)
> [ 42.226449] [] (unwind_backtrace) from []
> (show_stack+0x20/0x24)
> [ 42.235114] [] (show_stack) from []
> (dump_stack+0x84/0xa4)
> [ 42.243195] [] (dump_stack) from []
> (dw_hdmi_i2c_wait+0x6c/0xa8)
> [ 42.251858] [] (dw_hdmi_i2c_wait) from []
> (dw_hdmi_i2c_xfer+0x1a8/0x30c)
> [ 42.261298] [] (dw_hdmi_i2c_xfer) from []
> (__i2c_transfer+0x3a8/0x5d8)
> [ 42.270543] [] (__i2c_transfer) from []
> (i2c_transfer+0x94/0xc4)
> [ 42.279204] [] (i2c_transfer) from []
> (drm_do_probe_ddc_edid+0xbc/0x11c)
> [ 42.288642] [] (drm_do_probe_ddc_edid) from []
> (drm_probe_ddc+0x34/0x5c)
> [ 42.298081] [] (drm_probe_ddc) from []
> (drm_get_edid+0x60/0x2e0)
> [ 42.306743] [] (drm_get_edid) from []
> (dw_hdmi_connector_get_modes+0x30/0x78)
> [ 42.316669] [] (dw_hdmi_connector_get_modes) from
> [] (drm_helper_probe_single_connector_modes+0x218/0x5c0)
> [ 42.329413] [] (drm_helper_probe_single_connector_modes)
> from [] (drm_mode_getconnector+0x144/0x418)
> [ 42.341573] [] (drm_mode_getconnector) from []
> (drm_ioctl_kernel+0xa0/0xf0)
> [ 42.351303] [] (drm_ioctl_kernel) from []
> (drm_ioctl+0x32c/0x3c0)
> [ 42.360063] [] (drm_ioctl) from []
> (vfs_ioctl+0x28/0x44)
> [ 42.367946] [] (vfs_ioctl) from []
> (do_vfs_ioctl+0x718/0x8b0)
> [ 42.376315] [] (do_vfs_ioctl) from []
> (ksys_ioctl+0x5c/0x84)
> [ 42.384587] [] (ksys_ioctl) from []
> (sys_ioctl+0x18/0x1c)
> [ 42.392570] [] (sys_ioctl) from []
> (__sys_trace_return+0x0/0x10)
>
> ...I see several transfers fail and then finally a few seconds later
> finally see the .enable
Re: [git pull] drm fixes for v5.2-rc4 (v2)
On Fri, Jun 7, 2019 at 10:20 AM Linus Torvalds wrote: > > The second one has the subject, and mentions nouveau, but doesn't > actually have the tag name or the expected diffstat and shortlog. Hmm. I'm guessing you meant for me to pull the 'tags/drm-fixes-2019-06-07-1' thing, which looks likely, but I'd like to have confirmation. Linus
Re: [git pull] drm fixes for v5.2-rc4 (v2)
On Fri, Jun 7, 2019 at 12:24 AM Dave Airlie wrote: > > I sent this out earlier, but I forgot the subject, and then Ben asked > about some nouveau firmware fixes. Well, the first one at least had the address to pull from, and the diffstat. The second one has the subject, and mentions nouveau, but doesn't actually have the tag name or the expected diffstat and shortlog. Third time is the charm? Linus
[Bug 110856] Freesync causes in-game blackscreens when game has low fps.
https://bugs.freedesktop.org/show_bug.cgi?id=110856 Arek Tumas changed: What|Removed |Added Priority|medium |high -- 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
[Bug 110856] Freesync causes in-game blackscreens when game has low fps.
https://bugs.freedesktop.org/show_bug.cgi?id=110856 Bug ID: 110856 Summary: Freesync causes in-game blackscreens when game has low fps. Product: DRI Version: XOrg git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: critical Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: mokura...@gmail.com When I play games on my Benq EX2303R 144hz monitor, the monitor shows blackscreens/turns of for a few seconds in low fps scenarios like in-game menus or during stutters. This is probably when the monitor exits it's VRR window. Freesync is working normally with the same hardware and software but an Acer XG270HU Freesync 1 monitor. I have The following hardware: an ASUS Strix Vega 64 8GB GPU Benq EX3203r Freesync 2 Monitor Lindy premium displayport cable. Ryzen 7 1800x Stock clocks with SMT Software: MESA - 19.0.4-1 Operating System: Manjaro Linux KDE Plasma Version: 5.15.5 KDE Frameworks Version: 5.58.0 Qt Version: 5.12.3 Kernel Version: 5.2.0-1-MANJARO OS Type: 64-bit Processors: 16 × AMD Ryzen 7 1800X Eight-Core Processor Memory: 15,7 GiB of RAM (Hi, I'm new to filing bugs so I'm not sure if this is the right place to post this. If it's the wrong place please tell me where I can file this in.) -- 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: Linking ALSA playback devices and DRM connectors
On Thu, Jun 06, 2019 at 09:13:51AM +, Ser, Simon wrote: > On Tue, 2019-06-04 at 17:15 +0200, Christian König wrote: > > Am 04.06.19 um 17:05 schrieb Ser, Simon: > > > Hi, > > > > > > I'm trying to link ALSA playback devices and DRM connectors. In other > > > words, I'd like to be able to know which ALSA device I should open to > > > play audio on a given connector. > > > > > > Unfortunately, I haven't found a way to extract this information. I > > > know /proc/asound/cardN/eld* expose the EDID-like data. However by > > > looking at the ALSA API (also: aplay -l and -L) I can't find a way to > > > figure out which PCM device maps to the ELD. > > > > > > Am I missing something? > > > > Is that actually fixed on all hardware? Or do we maybe have some > > hardware with only one audio codec and multiple connectors? > > > > > If not, what would be the best way to expose this? > > > > > > - A symlink to the ALSA audio PCM device in > > >/sys/class/drm/cardN-CONNECTOR? > > > - A symlink to the DRM connector in /sys/class/sound/pcm*? > > > > If it's fixed than those two options sound sane to me. > > > > > - A DRM connector property? > > > > If it's configurable than that sounds like a good option to me. > > I'm still not sure whether this is fixed on all hardware or not. > > Ville, on this old Intel hw, is the single connector that gets the > audio configurable? The force-audio property can be used for that. That is, you can force audio off for all the connector where you don't want audio and then it should get automagically routed to the remaining connector. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 108309] Raven Ridge 2700U system lock-up on multiple games
https://bugs.freedesktop.org/show_bug.cgi?id=108309 --- Comment #10 from Jason Oliveira --- Having the same issue, but it seems to be localized to Vulkan. I've confirmed this on a Ryzen 2300U laptop and a desktop Ryzen 2400G. I have a save file in Pac-Man Champion Edition DX+ that guaranteed will crash the system within 5 seconds of loading a time trial screen on both laptops. Same save file works fine on a proprietary nvidia card, and on (cherry trail) intel drivers, so this looks like it's a Vulkan bug exhibiting itself through DXVK on Raven Ridge. Tested on 19.1.0_rc2 and git current as of this date: both exhibit identical issues in Ubuntu and Gentoo. -- 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
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 Andrew Shark changed: What|Removed |Added Attachment #144478|0 |1 is obsolete|| --- Comment #11 from Andrew Shark --- Created attachment 144479 --> https://bugs.freedesktop.org/attachment.cgi?id=144479=edit Script to modify packages to be able to use with ubuntu 19.04 Added "-f" to rm command to prevent error message in output rm: cannot remove 'amdgpu-core_19.10-785425_all.no_ub_ver_chk.deb': No such file or directory Explicitly specified that problems appears in 19.10-785425 release, because they may be fixed by amd in further releases -- 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
[PATCH 2/4] drm: Refuse to create zero width/height cmdline modes
From: Ville Syrjälä
If the user specifies zero width/height cmdline mode i915 will
blow up as the fbdev path will bypass the regular fb sanity
check that would otherwise have refused to create a framebuffer
with zero width/height.
The reason I thought to try this is so that I can force a specific
depth for fbdev without actually having to hardcode the mode
on the kernel cmdline. Eg. if I pass video=0x0-8 I will get an
8bpp framebuffer at my monitor's native resolution.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_modes.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index 5a07a28fec6d..b36248a5d826 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -1593,6 +1593,9 @@ drm_mode_create_from_cmdline_mode(struct drm_device *dev,
{
struct drm_display_mode *mode;
+ if (cmd->xres == 0 || cmd->yres == 0)
+ return NULL;
+
if (cmd->cvt)
mode = drm_cvt_mode(dev,
cmd->xres, cmd->yres,
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm/i915: Throw away the BIOS fb if has the wrong depth/bpp
From: Ville Syrjälä
Respect the user's choice of depth/bpp for the fbdev framebuffer
and throw out the fb we inherited from the BIOS if it doesn't
match.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_fbdev.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index 0d3a6fa674e6..1a935dc74d23 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -199,6 +199,17 @@ static int intelfb_create(struct drm_fb_helper *helper,
drm_framebuffer_put(_fb->base);
intel_fb = ifbdev->fb = NULL;
}
+ if (intel_fb &&
+ (sizes->surface_depth != intel_fb->base.format->depth ||
+sizes->surface_bpp != intel_fb->base.format->cpp[0] * 8)) {
+ DRM_DEBUG_KMS("BIOS fb using wrong depth/bpp (%d/%d), we
require (%d/%d),"
+ " releasing it\n",
+ intel_fb->base.format->depth,
+ intel_fb->base.format->cpp[0] * 8,
+ sizes->surface_depth, sizes->surface_bpp);
+ drm_framebuffer_put(_fb->base);
+ intel_fb = ifbdev->fb = NULL;
+ }
if (!intel_fb || WARN_ON(!intel_fb_obj(_fb->base))) {
DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n");
ret = intelfb_alloc(helper, sizes);
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm/fb-helper: Do not assume drm_mode_create_from_cmdline_mode() can't fail
From: Ville Syrjälä drm_mode_create_from_cmdline_mode() can return NULL, so the caller should check for that. Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/drm_fb_helper.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index b9b7c06cbc4f..bdfa14cd7f6d 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2205,7 +2205,9 @@ struct drm_display_mode *drm_pick_cmdline_mode(struct drm_fb_helper_connector *f create_mode: mode = drm_mode_create_from_cmdline_mode(fb_helper_conn->connector->dev, cmdline_mode); - list_add(>head, _helper_conn->connector->modes); + if (mode) + list_add(>head, _helper_conn->connector->modes); + return mode; } EXPORT_SYMBOL(drm_pick_cmdline_mode); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/fb-helper: Set up gamma_lut during restore_fbdev_mode_atomic()
From: Ville Syrjälä
i915 will now refuse to enable a C8 plane unless the gamma_lut
is already enabled (to avoid scanning out whatever garbage got
left in the hw LUT previously). So in order to make the fbdev
code work with C8 we need to program the gamma_lut already
during restore_fbdev_mode_atomic().
To avoid having to update the hw LUT every time
restore_fbdev_mode_atomic() is called we hang on to the
current gamma_lut blob. Note that the first crtc to get
enabled will dictate the size of the gamma_lut, so this
approach isn't 100% great for hardware with non-uniform
crtcs. But that problem already exists in setcmap_atomic()
so we'll just keep ignoring it.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_fb_helper.c | 165
include/drm/drm_fb_helper.h | 7 ++
2 files changed, 113 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index bdfa14cd7f6d..0ecec763789f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -436,10 +436,76 @@ static bool drm_fb_helper_panel_rotation(struct
drm_mode_set *modeset,
return true;
}
+static int setcmap_crtc_atomic(struct drm_atomic_state *state,
+ struct drm_crtc *crtc,
+ struct drm_property_blob *gamma_lut)
+{
+ struct drm_crtc_state *crtc_state;
+ bool replaced;
+
+ crtc_state = drm_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
+
+ replaced = drm_property_replace_blob(_state->degamma_lut,
+ NULL);
+ replaced |= drm_property_replace_blob(_state->ctm, NULL);
+ replaced |= drm_property_replace_blob(_state->gamma_lut,
+ gamma_lut);
+ crtc_state->color_mgmt_changed |= replaced;
+
+ return 0;
+}
+
+static struct drm_property_blob *setcmap_new_gamma_lut(struct drm_crtc *crtc,
+ struct fb_cmap *cmap)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_property_blob *gamma_lut;
+ struct drm_color_lut *lut;
+ int size = crtc->gamma_size;
+ int i;
+
+ if (!size || cmap->start + cmap->len > size)
+ return ERR_PTR(-EINVAL);
+
+ gamma_lut = drm_property_create_blob(dev, sizeof(*lut) * size, NULL);
+ if (IS_ERR(gamma_lut))
+ return gamma_lut;
+
+ lut = gamma_lut->data;
+ if (cmap->start || cmap->len != size) {
+ u16 *r = crtc->gamma_store;
+ u16 *g = r + crtc->gamma_size;
+ u16 *b = g + crtc->gamma_size;
+
+ for (i = 0; i < cmap->start; i++) {
+ lut[i].red = r[i];
+ lut[i].green = g[i];
+ lut[i].blue = b[i];
+ }
+ for (i = cmap->start + cmap->len; i < size; i++) {
+ lut[i].red = r[i];
+ lut[i].green = g[i];
+ lut[i].blue = b[i];
+ }
+ }
+
+ for (i = 0; i < cmap->len; i++) {
+ lut[cmap->start + i].red = cmap->red[i];
+ lut[cmap->start + i].green = cmap->green[i];
+ lut[cmap->start + i].blue = cmap->blue[i];
+ }
+
+ return gamma_lut;
+}
+
static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool
active)
{
+ struct fb_info *info = fb_helper->fbdev;
struct drm_client_dev *client = _helper->client;
struct drm_device *dev = fb_helper->dev;
+ struct drm_property_blob *gamma_lut;
struct drm_plane_state *plane_state;
struct drm_plane *plane;
struct drm_atomic_state *state;
@@ -455,6 +521,10 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper
*fb_helper, bool activ
goto out_ctx;
}
+ gamma_lut = fb_helper->gamma_lut;
+ if (gamma_lut)
+ drm_property_blob_get(gamma_lut);
+
state->acquire_ctx =
retry:
drm_for_each_plane(plane, dev) {
@@ -476,7 +546,8 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper
*fb_helper, bool activ
}
drm_client_for_each_modeset(mode_set, client) {
- struct drm_plane *primary = mode_set->crtc->primary;
+ struct drm_crtc *crtc = mode_set->crtc;
+ struct drm_plane *primary = crtc->primary;
unsigned int rotation;
if (drm_fb_helper_panel_rotation(mode_set, )) {
@@ -489,24 +560,46 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper
*fb_helper, bool activ
if (ret != 0)
goto out_state;
+ if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
+ if (!gamma_lut)
+ gamma_lut =
[Bug 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #56 from Rui Salvaterra --- And speaking of performance, I wonder if this will have any effect… I'll try it later. https://git.congatec.com/android/qmx6_kernel/commit/a0a53aa8c7b491a43e2ef66786f9511bae8cbc35 -- 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
[Bug 110677] Can't invert screen colors by adjusting the gamma ramp
https://bugs.freedesktop.org/show_bug.cgi?id=110677 Michel Dänzer changed: What|Removed |Added Attachment #144269|text/x-log |text/plain mime type|| -- 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
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 Andrew Shark changed: What|Removed |Added Attachment #144471|0 |1 is obsolete|| --- Comment #10 from Andrew Shark --- Created attachment 144478 --> https://bugs.freedesktop.org/attachment.cgi?id=144478=edit Script to modify packages to be able to use with ubuntu 19.04 Fixed typo lagacy -> legacy in "--opencl=legacy,pal". -- 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
[Bug 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #55 from Alex Deucher --- Performance will be better because the 3D engine is rendering to a smaller surface (e.g., 640x480) regardless of whether you are fullscreen or windowed. The size of the rendered image is what impact 3D performance. The scaler is fixed function hw so it doesn't impact the 3D engine directly other than using some memory bandwidth to read the source image. -- 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: [PATCHv16 1/3] ARM:dt-bindings:display Intel FPGA Video and Image Processing Suite
Hi Hean-Loong:
Please format your commit message like this:
Reviewed-by: Rob Herring
Signed-off-by: Ong, Hean Loong
---
V15:
v14:
The version history needs go after the ---
Dinh
On 6/7/19 9:30 AM, Hean-Loong, Ong wrote:
> From: "Ong, Hean Loong"
>
> Device tree binding for Intel FPGA Video and Image Processing Suite.
> The bindings would set the max width, max height,
> bits per pixel and memory port width.
> The device tree binding only supports the Intel
> Arria10 devkit and its variants. Vendor name retained as altr.
>
> Reviewed-by: Rob Herring
>
> V15:
> Reviewed
>
> V14:
> No Change
>
> V13:
> No change
>
> V12:
> Wrap comments and fix commit message
>
> V11:
> No change
>
> V10:
> No change
>
> V9:
> Remove Display port node
>
> V8:
> *Add port to Display port decoder
>
> V7:
> *Fix OF graph for better description
> *Add description for encoder
>
> V6:
> *Description have not describe DT device in general
>
> V5:
> *remove bindings for bits per symbol as it has only one value which is 8
>
> V4:
> *fix properties that does not describe the values
>
> V3:
> *OF graph not in accordance to graph.txt
>
> V2:
> *Remove Linux driver description
>
> V1:
> *Missing vendor prefix
>
> Signed-off-by: Ong, Hean Loong
> ---
> .../bindings/display/altr,vip-fb2.txt | 63 +++
> 1 file changed, 63 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/altr,vip-fb2.txt
>
> diff --git a/Documentation/devicetree/bindings/display/altr,vip-fb2.txt
> b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt
> new file mode 100644
> index ..89a3b9e166a8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt
> @@ -0,0 +1,63 @@
> +Intel Video and Image Processing(VIP) Frame Buffer II bindings
> +
> +Supported hardware: Intel FPGA SoC Arria10 and above with display port IP
> +
> +The Video Frame Buffer II in Video Image Processing (VIP) suite is an IP core
> +that interfaces between system memory and Avalon-ST video ports. The IP core
> +can be configured to support the memory reader (from memory to Avalon-ST)
> +and/or memory writer (from Avalon-ST to memory) interfaces.
> +
> +More information the FPGA video IP component can be acquired from
> +https://www.altera.com/content/dam/altera-www/global/en_US/pdfs\
> +/literature/ug/ug_vip.pdf
> +
> +DT-Bindings:
> +=
> +Required properties:
> +
> +- compatible: "altr,vip-frame-buffer-2.0"
> +- reg: Physical base address and length of the framebuffer controller's
> + registers.
> +- altr,max-width: The maximum width of the framebuffer in pixels.
> +- altr,max-height: The maximum height of the framebuffer in pixels.
> +- altr,mem-port-width = the bus width of the avalon master port
> + on the frame reader
> +
> +Optional sub-nodes:
> +- ports: The connection to the encoder
> +
> +Connections between the Frame Buffer II and other video IP cores in the
> system
> +are modelled using the OF graph DT bindings. The Frame Buffer II node has up
> +to two OF graph ports. When the memory writer interface is enabled, port 0
> +maps to the Avalon-ST Input (din) port. When the memory reader interface is
> +enabled, port 1 maps to the Avalon-ST Output (dout) port.
> +
> +The encoder is built into the FPGA HW design and therefore would not
> +be accessible from the DDR.
> +
> + Port 0 Port1
> +-
> +ARRIA10 AVALON_ST (DIN) AVALON_ST (DOUT)
> +
> +Required Properties Example:
> +
> +
> +framebuffer@10280 {
> + compatible = "altr,vip-frame-buffer-2.0";
> + reg = <0x0001 0x0280 0x0040>;
> + altr,max-width = <1280>;
> + altr,max-height = <720>;
> + altr,mem-port-width = <128>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@1 {
> + reg = <1>;
> + fb_output: endpoint {
> + remote-endpoint =
> <_encoder_input>;
> + };
> + };
> + };
> +};
>
[Bug 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #54 from Richard Thier --- > The GPU has a scaler that can scale a smaller image to the native timing. > Many > LCD monitors also have a scaler built in to provide the same > functionality. I see, but still I see a big performance rise when I change the resolution smaller and let this scaler do its work in the last phase. I have no idea if there is a real speed difference between this two: - having 640x480 on a 4:3 aspect screen without a scaler - Having hardware pixels of 1024x768, but using the scaler in the end as 640x480 software. - Z tests run for smaller amount of pixels - Stencil and Z buffers are smaller just like the backbuffer - etc. Memory usage on the GPU should be still smaller, pixel shaders still run rarer times, post processing things still run on smaller amount of pixels etc... The only reason it can be slower if for some reason the scaler circuit would be so slow while doing its job that it cannot keep its FPS, but I think this is a highly specialized hardware not doing anything else so I hoped it is just "fast" and even if there is some loss I guess it is barely measurable - but tell me if I am wrong as that would be new to me if this has a really measurable overhead. In any ways I think it is better to use a same aspect ratio, but smaller resolution than using the biggest possible native size. Actually on my machine I get faster frame rate even if I run a game in 640x480 window on the top left corner the same way as if I turn the screen into 640x480 itself and even then I barely see a difference despite this case I feel there should be some overhead against using directly a video mode for that.. -- 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 2/2] drm: add fallback override/firmware EDID modes workaround
On Fri, Jun 07, 2019 at 02:05:13PM +0300, Jani Nikula wrote:
> We've moved the override and firmware EDID (simply "override EDID" from
> now on) handling to the low level drm_do_get_edid() function in order to
> transparently use the override throughout the stack. The idea is that
> you get the override EDID via the ->get_modes() hook.
>
> Unfortunately, there are scenarios where the DDC probe in drm_get_edid()
> called via ->get_modes() fails, although the preceding ->detect()
> succeeds.
>
> In the case reported by Paul Wise, the ->detect() hook,
> intel_crt_detect(), relies on hotplug detect, bypassing the DDC. In the
> case reported by Ilpo Järvinen, there is no ->detect() hook, which is
> interpreted as connected. The subsequent DDC probe reached via
> ->get_modes() fails, and we don't even look at the override EDID,
> resulting in no modes being added.
>
> Because drm_get_edid() is used via ->detect() all over the place, we
> can't trivially remove the DDC probe, as it leads to override EDID
> effectively meaning connector forcing. The goal is that connector
> forcing and override EDID remain orthogonal.
>
> Generally, the underlying problem here is the conflation of ->detect()
> and ->get_modes() via drm_get_edid(). The former should just detect, and
> the latter should just get the modes, typically via reading the EDID. As
> long as drm_get_edid() is used in ->detect(), it needs to retain the DDC
> probe. Or such users need to have a separate DDC probe step first.
>
> Work around the regression by falling back to a separate attempt at
> getting the override EDID at drm_helper_probe_single_connector_modes()
> level. With a working DDC and override EDID, it'll never be called; the
> override EDID will come via ->get_modes(). There will still be a failing
> DDC probe attempt in the cases that require the fallback.
I think we should also highlight here that EDID caching between ->detect
and ->get_modes is a further complicating concern, which is why making
drm_do_get_edid magically dtrt between ->detect and ->get_modes doesn't
work either.
Aside from that nit I think this covers our lengthy discussion completely.
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107583
> Reported-by: Paul Wise
> Cc: Paul Wise
> References:
> alpine.DEB.2.20.1905262211270.24390@whs-18.cs.helsinki.fi">http://mid.mail-archive.com/alpine.DEB.2.20.1905262211270.24390@whs-18.cs.helsinki.fi
> Reported-by: Ilpo Järvinen
> Cc: Ilpo Järvinen
> References: 15f080f08d48 ("drm/edid: respect connector force for drm_get_edid
> ddc probe")
> Fixes: 53fd40a90f3c ("drm: handle override and firmware EDID at
> drm_do_get_edid() level")
> Cc: # v4.15+
> Cc: Daniel Vetter
> Cc: Ville Syrjälä
> Cc: Harish Chegondi
> Signed-off-by: Jani Nikula
As discussed on irc, we need tested-by here from the reporters since
there's way too many losing and frustrangingly few winning moves here.
With all that, on the series:
Reviewed-by: Daniel Vetter
Thanks a lot for slogging through all this and pondering all the options
and implications!
Cheers, Daniel
> ---
> drivers/gpu/drm/drm_edid.c | 29 +
> drivers/gpu/drm/drm_probe_helper.c | 7 +++
> include/drm/drm_edid.h | 1 +
> 3 files changed, 37 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index c59a1e8c5ada..780146bfc225 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1587,6 +1587,35 @@ static struct edid *drm_get_override_edid(struct
> drm_connector *connector)
> return IS_ERR(override) ? NULL : override;
> }
>
> +/**
> + * drm_add_override_edid_modes - add modes from override/firmware EDID
> + * @connector: connector we're probing
> + *
> + * Add modes from the override/firmware EDID, if available. Only to be used
> from
> + * drm_helper_probe_single_connector_modes() as a fallback for when DDC probe
> + * failed during drm_get_edid() and caused the override/firmware EDID to be
> + * skipped.
> + *
> + * Return: The number of modes added or 0 if we couldn't find any.
> + */
> +int drm_add_override_edid_modes(struct drm_connector *connector)
> +{
> + struct edid *override;
> + int num_modes = 0;
> +
> + override = drm_get_override_edid(connector);
> + if (override) {
> + num_modes = drm_add_edid_modes(connector, override);
> + kfree(override);
> +
> + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] adding %d modes via fallback
> override/firmware EDID\n",
> + connector->base.id, connector->name, num_modes);
> + }
> +
> + return num_modes;
> +}
> +EXPORT_SYMBOL(drm_add_override_edid_modes);
> +
> /**
> * drm_do_get_edid - get EDID data using a custom EDID block read function
> * @connector: connector we're probing
> diff --git a/drivers/gpu/drm/drm_probe_helper.c
> b/drivers/gpu/drm/drm_probe_helper.c
> index 01e243f1ea94..ef2c468205a2 100644
> ---
[Bug 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #53 from Rui Salvaterra --- (In reply to Richard Thier from comment #49) > I don't get it. Is there no supported resolution below that big one? That is > quite hard to believe. Mine is 1024x768 when maxed, but sometimes I reverse > engineer game binaries that do not let me use 640x480 in case they lag haha. 1024x768?! Jesus, that's less than half the number of pixels, no wonder it's faster…! And yes, like Alex Deucher said, I've never seen an LCD with more than one native resolution. At different resolutions, there's always scaling involved (either at the panel or at the GPU). -- 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 1/2] drm/vkms: Use index instead of 0 in possible crtc
On Fri, Jun 07, 2019 at 11:37:55AM -0300, Rodrigo Siqueira wrote:
> On Fri, Jun 7, 2019 at 4:40 AM Daniel Vetter wrote:
> >
> > On Thu, Jun 06, 2019 at 07:40:38PM -0300, Rodrigo Siqueira wrote:
> > > When vkms calls drm_universal_plane_init(), it sets 0 for the
> > > possible_crtcs parameter which works well for a single encoder and
> > > connector; however, this approach is not flexible and does not fit well
> > > for vkms. This commit adds an index parameter for vkms_plane_init()
> > > which makes code flexible and enables vkms to support other DRM features.
> > >
> > > Signed-off-by: Rodrigo Siqueira
> >
> > I think a core patch to WARN_ON if this is NULL would be good. Since
> > that's indeed a bit broken ... We'd need to check all callers to make sure
> > there's not other such bugs anywhere ofc.
> > -Daniel
>
> Do you mean add WARN_ON in `drm_universal_plane_init()` if
> `possible_crtcs` is equal to zero?
Yeah, and same for endcoders I guess too. Altough with encoders I think
there's a ton of broken drivers.
-Daniel
>
> > > ---
> > > drivers/gpu/drm/vkms/vkms_drv.c| 2 +-
> > > drivers/gpu/drm/vkms/vkms_drv.h| 4 ++--
> > > drivers/gpu/drm/vkms/vkms_output.c | 6 +++---
> > > drivers/gpu/drm/vkms/vkms_plane.c | 4 ++--
> > > 4 files changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c
> > > b/drivers/gpu/drm/vkms/vkms_drv.c
> > > index 738dd6206d85..92296bd8f623 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > > @@ -92,7 +92,7 @@ static int vkms_modeset_init(struct vkms_device
> > > *vkmsdev)
> > > dev->mode_config.max_height = YRES_MAX;
> > > dev->mode_config.preferred_depth = 24;
> > >
> > > - return vkms_output_init(vkmsdev);
> > > + return vkms_output_init(vkmsdev, 0);
> > > }
> > >
> > > static int __init vkms_init(void)
> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h
> > > b/drivers/gpu/drm/vkms/vkms_drv.h
> > > index 81f1cfbeb936..e81073dea154 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > > @@ -113,10 +113,10 @@ bool vkms_get_vblank_timestamp(struct drm_device
> > > *dev, unsigned int pipe,
> > > int *max_error, ktime_t *vblank_time,
> > > bool in_vblank_irq);
> > >
> > > -int vkms_output_init(struct vkms_device *vkmsdev);
> > > +int vkms_output_init(struct vkms_device *vkmsdev, int index);
> > >
> > > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> > > - enum drm_plane_type type);
> > > + enum drm_plane_type type, int index);
> > >
> > > /* Gem stuff */
> > > struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> > > diff --git a/drivers/gpu/drm/vkms/vkms_output.c
> > > b/drivers/gpu/drm/vkms/vkms_output.c
> > > index 3b162b25312e..1442b447c707 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_output.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > > @@ -36,7 +36,7 @@ static const struct drm_connector_helper_funcs
> > > vkms_conn_helper_funcs = {
> > > .get_modes= vkms_conn_get_modes,
> > > };
> > >
> > > -int vkms_output_init(struct vkms_device *vkmsdev)
> > > +int vkms_output_init(struct vkms_device *vkmsdev, int index)
> > > {
> > > struct vkms_output *output = >output;
> > > struct drm_device *dev = >drm;
> > > @@ -46,12 +46,12 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> > > struct drm_plane *primary, *cursor = NULL;
> > > int ret;
> > >
> > > - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY);
> > > + primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index);
> > > if (IS_ERR(primary))
> > > return PTR_ERR(primary);
> > >
> > > if (enable_cursor) {
> > > - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR);
> > > + cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR,
> > > index);
> > > if (IS_ERR(cursor)) {
> > > ret = PTR_ERR(cursor);
> > > goto err_cursor;
> > > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c
> > > b/drivers/gpu/drm/vkms/vkms_plane.c
> > > index 0e67d2d42f0c..20ffc52f9194 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_plane.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> > > @@ -168,7 +168,7 @@ static const struct drm_plane_helper_funcs
> > > vkms_primary_helper_funcs = {
> > > };
> > >
> > > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> > > - enum drm_plane_type type)
> > > + enum drm_plane_type type, int index)
> > > {
> > > struct drm_device *dev = >drm;
> > > const struct drm_plane_helper_funcs *funcs;
> > > @@ -190,7 +190,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device
> > > *vkmsdev,
> > > funcs = _primary_helper_funcs;
[PATCH] drm/komeda: Enable/Disable vblank interrupts
One needs to set "(struct drm_device *)->irq_enabled = true" to signal drm core to enable vblank interrupts after the hardware has been initialized. Correspondingly, one needs to disable vblank interrupts by setting "(struct drm_device *)->irq_enabled = false" at the beginning of the de-initializing routine. Signed-off-by: Ayan Kumar halder --- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c index 7b5cde14e3ba..b4fd8ee0d05f 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c @@ -204,6 +204,8 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev) if (err) goto uninstall_irq; + drm->irq_enabled = true; + err = drm_dev_register(drm, 0); if (err) goto uninstall_irq; @@ -211,6 +213,7 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev) return kms; uninstall_irq: + drm->irq_enabled = false; drm_irq_uninstall(drm); cleanup_mode_config: drm_mode_config_cleanup(drm); @@ -225,6 +228,7 @@ void komeda_kms_detach(struct komeda_kms_dev *kms) struct drm_device *drm = >base; struct komeda_dev *mdev = drm->dev_private; + drm->irq_enabled = false; mdev->funcs->disable_irq(mdev); drm_dev_unregister(drm); drm_irq_uninstall(drm); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/vkms: Add support for writeback
On Fri, Jun 7, 2019 at 4:48 AM Daniel Vetter wrote:
>
> On Thu, Jun 06, 2019 at 07:41:01PM -0300, Rodrigo Siqueira wrote:
> > This patch implements the necessary functions to add writeback support
> > for vkms. This feature is useful for testing compositors if you don’t
> > have hardware with writeback support.
> >
> > Signed-off-by: Rodrigo Siqueira
> > ---
> > drivers/gpu/drm/vkms/Makefile | 9 +-
> > drivers/gpu/drm/vkms/vkms_crtc.c | 5 +
> > drivers/gpu/drm/vkms/vkms_drv.c | 10 ++
> > drivers/gpu/drm/vkms/vkms_drv.h | 12 ++
> > drivers/gpu/drm/vkms/vkms_output.c| 6 +
> > drivers/gpu/drm/vkms/vkms_writeback.c | 165 ++
> > 6 files changed, 206 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/gpu/drm/vkms/vkms_writeback.c
> >
> > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> > index 89f09bec7b23..90eb7acd618d 100644
> > --- a/drivers/gpu/drm/vkms/Makefile
> > +++ b/drivers/gpu/drm/vkms/Makefile
> > @@ -1,4 +1,11 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
> > vkms_crc.o
> > +vkms-y := \
> > + vkms_drv.o \
> > + vkms_plane.o \
> > + vkms_output.o \
> > + vkms_crtc.o \
> > + vkms_gem.o \
> > + vkms_crc.o \
> > + vkms_writeback.o
> >
> > obj-$(CONFIG_DRM_VKMS) += vkms.o
> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> > b/drivers/gpu/drm/vkms/vkms_crtc.c
> > index 1bbe099b7db8..ce797e265b1b 100644
> > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -23,6 +23,11 @@ static enum hrtimer_restart vkms_vblank_simulate(struct
> > hrtimer *timer)
> > if (!ret)
> > DRM_ERROR("vkms failure on handling vblank");
> >
> > + if (output->writeback_status == WB_START) {
> > + drm_writeback_signal_completion(>wb_connector, 0);
> > + output->writeback_status = WB_STOP;
> > + }
> > +
> > if (state && output->crc_enabled) {
> > u64 frame = drm_crtc_accurate_vblank_count(crtc);
> >
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c
> > b/drivers/gpu/drm/vkms/vkms_drv.c
> > index 92296bd8f623..d5917d5a45e3 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -29,6 +29,10 @@ bool enable_cursor;
> > module_param_named(enable_cursor, enable_cursor, bool, 0444);
> > MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
> >
> > +int enable_writeback;
> > +module_param_named(enable_writeback, enable_writeback, int, 0444);
> > +MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector");
> > +
> > static const struct file_operations vkms_driver_fops = {
> > .owner = THIS_MODULE,
> > .open = drm_open,
> > @@ -123,6 +127,12 @@ static int __init vkms_init(void)
> > goto out_fini;
> > }
> >
> > + vkms_device->output.writeback_status = WB_DISABLED;
> > + if (enable_writeback) {
> > + vkms_device->output.writeback_status = WB_STOP;
> > + DRM_INFO("Writeback connector enabled");
> > + }
> > +
> > ret = vkms_modeset_init(vkms_device);
> > if (ret)
> > goto out_fini;
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h
> > b/drivers/gpu/drm/vkms/vkms_drv.h
> > index e81073dea154..ca1f9ee63ec8 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -7,6 +7,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #define XRES_MIN20
> > @@ -60,14 +61,22 @@ struct vkms_crtc_state {
> > u64 frame_end;
> > };
> >
> > +enum wb_status {
> > + WB_DISABLED,
> > + WB_START,
> > + WB_STOP,
> > +};
> > +
> > struct vkms_output {
> > struct drm_crtc crtc;
> > struct drm_encoder encoder;
> > struct drm_connector connector;
> > + struct drm_writeback_connector wb_connector;
> > struct hrtimer vblank_hrtimer;
> > ktime_t period_ns;
> > struct drm_pending_vblank_event *event;
> > bool crc_enabled;
> > + enum wb_status writeback_status;
> > /* ordered wq for crc_work */
> > struct workqueue_struct *crc_workq;
> > /* protects concurrent access to crc_data */
> > @@ -141,4 +150,7 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const
> > char *source_name,
> > size_t *values_cnt);
> > void vkms_crc_work_handle(struct work_struct *work);
> >
> > +/* Writeback */
> > +int enable_writeback_connector(struct vkms_device *vkmsdev);
> > +
> > #endif /* _VKMS_DRV_H_ */
> > diff --git a/drivers/gpu/drm/vkms/vkms_output.c
> > b/drivers/gpu/drm/vkms/vkms_output.c
> > index 1442b447c707..1fc1d4e9585c 100644
> > --- a/drivers/gpu/drm/vkms/vkms_output.c
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -91,6 +91,12 @@ int vkms_output_init(struct vkms_device *vkmsdev,
[Bug 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #52 from Alex Deucher --- (In reply to Richard Thier from comment #51) > > I do have an old 1024x768 monitor with VGA input, > > I might give it a spin this weekend. > > But didn't the LCD also supports smaler resolutions natively? Most of them > do even if their best native is more exotic ratios and stuff. Then you just > need to change with xrandr to something smaller. Generally panels only have a single native mode. The GPU has a scaler that can scale a smaller image to the native timing. Many LCD monitors also have a scaler built in to provide the same functionality. -- 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 v2 2/2] drm: panel-orientation-quirks: Add quirk for GPD MicroPC
On Thu, Jun 06, 2019 at 04:03:57PM +0200, Hans de Goede wrote: > Hi, > > On 06-06-19 15:38, Maxime Ripard wrote: > > On Thu, Jun 06, 2019 at 01:13:40PM +0200, Hans de Goede wrote: > > > On 06-06-19 11:14, Maxime Ripard wrote: > > > > On Fri, May 24, 2019 at 02:57:59PM +0200, Hans de Goede wrote: > > > > > GPD has done it again, make a nice device (good), use way too generic > > > > > DMI strings (bad) and use a portrait screen rotated 90 degrees (ugly). > > > > > > > > > > Because of the too generic DMI strings this entry is also doing > > > > > bios-date > > > > > matching, so the gpd_micropc data struct may very well need to be > > > > > updated > > > > > with some extra bios-dates in the future. > > > > > > > > > > Signed-off-by: Hans de Goede > > > > > > > > For both patches, > > > > Acked-by: Maxime Ripard > > > > > > Thank you, I've pushed both to drm-misc-next now. > > > > > > Can you add them to drm-misc-fixes please ? > > > > > > (AFAIK I'm not supposed to do that myself) > > > > You definitely can :) > > > > Now that it's in next though, it's pretty hard to come back in time. I > > guess we could just apply it in fixes and let git figure it out, or > > revert the one in next. I'm not sure which one is preferred > > though. > > I thought that the procedure was to get it in -next and then cherry-pick > into -fixes? If you feel like something should be in fixes, you can definitely apply it there only. > Git should sort this out without issues when Linus merges -next; or > when we back-merge Linus' tree. Yeah, looking at the doc, cherry-picking is the one encouraged, so feel free to cherry-pick them in drm-misc-fixes. 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: [PATCH 2/2] drm/vkms: Add support for writeback
On Fri, Jun 07, 2019 at 02:17:20PM +, Brian Starkey wrote:
> Hi Rodrigo,
>
> On Thu, Jun 06, 2019 at 07:41:01PM -0300, Rodrigo Siqueira wrote:
> > This patch implements the necessary functions to add writeback support
> > for vkms. This feature is useful for testing compositors if you don’t
> > have hardware with writeback support.
> >
> > Signed-off-by: Rodrigo Siqueira
> > ---
> > drivers/gpu/drm/vkms/Makefile | 9 +-
> > drivers/gpu/drm/vkms/vkms_crtc.c | 5 +
> > drivers/gpu/drm/vkms/vkms_drv.c | 10 ++
> > drivers/gpu/drm/vkms/vkms_drv.h | 12 ++
> > drivers/gpu/drm/vkms/vkms_output.c| 6 +
> > drivers/gpu/drm/vkms/vkms_writeback.c | 165 ++
> > 6 files changed, 206 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/gpu/drm/vkms/vkms_writeback.c
> >
> > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> > index 89f09bec7b23..90eb7acd618d 100644
> > --- a/drivers/gpu/drm/vkms/Makefile
> > +++ b/drivers/gpu/drm/vkms/Makefile
> > @@ -1,4 +1,11 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
> > vkms_crc.o
> > +vkms-y := \
> > + vkms_drv.o \
> > + vkms_plane.o \
> > + vkms_output.o \
> > + vkms_crtc.o \
> > + vkms_gem.o \
> > + vkms_crc.o \
> > + vkms_writeback.o
> >
> > obj-$(CONFIG_DRM_VKMS) += vkms.o
> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> > b/drivers/gpu/drm/vkms/vkms_crtc.c
> > index 1bbe099b7db8..ce797e265b1b 100644
> > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -23,6 +23,11 @@ static enum hrtimer_restart vkms_vblank_simulate(struct
> > hrtimer *timer)
> > if (!ret)
> > DRM_ERROR("vkms failure on handling vblank");
> >
> > + if (output->writeback_status == WB_START) {
> > + drm_writeback_signal_completion(>wb_connector, 0);
> > + output->writeback_status = WB_STOP;
> > + }
> > +
> > if (state && output->crc_enabled) {
> > u64 frame = drm_crtc_accurate_vblank_count(crtc);
> >
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c
> > b/drivers/gpu/drm/vkms/vkms_drv.c
> > index 92296bd8f623..d5917d5a45e3 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -29,6 +29,10 @@ bool enable_cursor;
> > module_param_named(enable_cursor, enable_cursor, bool, 0444);
> > MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
> >
> > +int enable_writeback;
> > +module_param_named(enable_writeback, enable_writeback, int, 0444);
> > +MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector");
> > +
> > static const struct file_operations vkms_driver_fops = {
> > .owner = THIS_MODULE,
> > .open = drm_open,
> > @@ -123,6 +127,12 @@ static int __init vkms_init(void)
> > goto out_fini;
> > }
> >
> > + vkms_device->output.writeback_status = WB_DISABLED;
> > + if (enable_writeback) {
> > + vkms_device->output.writeback_status = WB_STOP;
> > + DRM_INFO("Writeback connector enabled");
> > + }
> > +
> > ret = vkms_modeset_init(vkms_device);
> > if (ret)
> > goto out_fini;
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h
> > b/drivers/gpu/drm/vkms/vkms_drv.h
> > index e81073dea154..ca1f9ee63ec8 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -7,6 +7,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #define XRES_MIN20
> > @@ -60,14 +61,22 @@ struct vkms_crtc_state {
> > u64 frame_end;
> > };
> >
> > +enum wb_status {
> > + WB_DISABLED,
> > + WB_START,
> > + WB_STOP,
> > +};
> > +
> > struct vkms_output {
> > struct drm_crtc crtc;
> > struct drm_encoder encoder;
> > struct drm_connector connector;
> > + struct drm_writeback_connector wb_connector;
> > struct hrtimer vblank_hrtimer;
> > ktime_t period_ns;
> > struct drm_pending_vblank_event *event;
> > bool crc_enabled;
> > + enum wb_status writeback_status;
> > /* ordered wq for crc_work */
> > struct workqueue_struct *crc_workq;
> > /* protects concurrent access to crc_data */
> > @@ -141,4 +150,7 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const
> > char *source_name,
> >size_t *values_cnt);
> > void vkms_crc_work_handle(struct work_struct *work);
> >
> > +/* Writeback */
> > +int enable_writeback_connector(struct vkms_device *vkmsdev);
> > +
> > #endif /* _VKMS_DRV_H_ */
> > diff --git a/drivers/gpu/drm/vkms/vkms_output.c
> > b/drivers/gpu/drm/vkms/vkms_output.c
> > index 1442b447c707..1fc1d4e9585c 100644
> > --- a/drivers/gpu/drm/vkms/vkms_output.c
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -91,6 +91,12 @@ int vkms_output_init(struct vkms_device *vkmsdev, int
> > index)
> > goto
Re: [Intel-gfx] [PATCH] drm/crc-debugfs: Also sprinkle irqrestore over early exits
On Fri, Jun 07, 2019 at 01:14:57PM +0200, Benjamin Gaignard wrote:
> Le ven. 7 juin 2019 à 12:07, Emil Velikov a écrit :
> >
> > On Thu, 6 Jun 2019 at 22:15, Daniel Vetter wrote:
> > >
> > > I. was. blind.
> > >
> > > Caught with vkms, which has some really slow crc computation function.
> > >
> > > Fixes: 1882018a70e0 ("drm/crc-debugfs: User irqsafe spinlock in
> > > drm_crtc_add_crc_entry")
> > > Cc: Rodrigo Siqueira
> > > Cc: Tomeu Vizoso
> > > Cc: Emil Velikov
> > > Cc: Benjamin Gaignard
> > > Cc: Ville Syrjälä
> > > Signed-off-by: Daniel Vetter
> >
> > Reviewed-by: Emil Velikov
>
> Reviewed-by: Benjamin Gaignard
Thanks for the reviews, applied to -misc-next.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/ttm: move cpu_writers handling into vmwgfx
This feature is only used by vmwgfx and superflous for everybody else.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c | 27 --
drivers/gpu/drm/ttm/ttm_bo_util.c| 1 -
drivers/gpu/drm/ttm/ttm_execbuf_util.c | 7 +
drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 35
drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 ++
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 8 ++
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 4 +++
include/drm/ttm/ttm_bo_api.h | 31 -
8 files changed, 45 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index c7de667d482a..4ec055ffd6a7 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -153,7 +153,6 @@ static void ttm_bo_release_list(struct kref *list_kref)
BUG_ON(kref_read(>list_kref));
BUG_ON(kref_read(>kref));
- BUG_ON(atomic_read(>cpu_writers));
BUG_ON(bo->mem.mm_node != NULL);
BUG_ON(!list_empty(>lru));
BUG_ON(!list_empty(>ddestroy));
@@ -1308,7 +1307,6 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
kref_init(>kref);
kref_init(>list_kref);
- atomic_set(>cpu_writers, 0);
INIT_LIST_HEAD(>lru);
INIT_LIST_HEAD(>ddestroy);
INIT_LIST_HEAD(>swap);
@@ -1814,31 +1812,6 @@ int ttm_bo_wait(struct ttm_buffer_object *bo,
}
EXPORT_SYMBOL(ttm_bo_wait);
-int ttm_bo_synccpu_write_grab(struct ttm_buffer_object *bo, bool no_wait)
-{
- int ret = 0;
-
- /*
-* Using ttm_bo_reserve makes sure the lru lists are updated.
-*/
-
- ret = ttm_bo_reserve(bo, true, no_wait, NULL);
- if (unlikely(ret != 0))
- return ret;
- ret = ttm_bo_wait(bo, true, no_wait);
- if (likely(ret == 0))
- atomic_inc(>cpu_writers);
- ttm_bo_unreserve(bo);
- return ret;
-}
-EXPORT_SYMBOL(ttm_bo_synccpu_write_grab);
-
-void ttm_bo_synccpu_write_release(struct ttm_buffer_object *bo)
-{
- atomic_dec(>cpu_writers);
-}
-EXPORT_SYMBOL(ttm_bo_synccpu_write_release);
-
/**
* A buffer object shrink method that tries to swap out the first
* buffer object on the bo_global::swap_lru list.
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 895d77d799e4..6f43f1f0de7c 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -511,7 +511,6 @@ static int ttm_buffer_object_transfer(struct
ttm_buffer_object *bo,
mutex_init(>base.wu_mutex);
fbo->base.moving = NULL;
drm_vma_node_reset(>base.vma_node);
- atomic_set(>base.cpu_writers, 0);
kref_init(>base.list_kref);
kref_init(>base.kref);
diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
index 957ec375a4ba..80fa52b36d5c 100644
--- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
+++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
@@ -113,12 +113,7 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
struct ttm_buffer_object *bo = entry->bo;
ret = __ttm_bo_reserve(bo, intr, (ticket == NULL), ticket);
- if (!ret && unlikely(atomic_read(>cpu_writers) > 0)) {
- reservation_object_unlock(bo->resv);
-
- ret = -EBUSY;
-
- } else if (ret == -EALREADY && dups) {
+ if (ret == -EALREADY && dups) {
struct ttm_validate_buffer *safe = entry;
entry = list_prev_entry(entry, head);
list_del(>head);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 5d5c2bce01f3..457861c5047f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -565,7 +565,7 @@ static void vmw_user_bo_ref_obj_release(struct
ttm_base_object *base,
switch (ref_type) {
case TTM_REF_SYNCCPU_WRITE:
- ttm_bo_synccpu_write_release(_bo->vbo.base);
+ atomic_dec(_bo->vbo.cpu_writers);
break;
default:
WARN_ONCE(true, "Undefined buffer object reference release.\n");
@@ -681,12 +681,12 @@ static int vmw_user_bo_synccpu_grab(struct
vmw_user_buffer_object *user_bo,
struct ttm_object_file *tfile,
uint32_t flags)
{
+ bool nonblock = !!(flags & drm_vmw_synccpu_dontblock);
struct ttm_buffer_object *bo = _bo->vbo.base;
bool existed;
int ret;
if (flags & drm_vmw_synccpu_allow_cs) {
- bool nonblock = !!(flags & drm_vmw_synccpu_dontblock);
long lret;
lret = reservation_object_wait_timeout_rcu
@@ -699,15 +699,20 @@ static int vmw_user_bo_synccpu_grab(struct
vmw_user_buffer_object *user_bo,
return
Re: [PATCH 00/33] fbcon notifier begone v3!
On Fri, Jun 7, 2019 at 12:07 PM Bartlomiej Zolnierkiewicz wrote: > > > On 6/6/19 9:38 AM, Daniel Vetter wrote: > > Hi Bart, > > Hi Daniel, > > > On Tue, May 28, 2019 at 11:02:31AM +0200, Daniel Vetter wrote: > >> Hi all, > >> > >> I think we're slowly getting there. Previous cover letters with more > >> context: > >> > >> https://lists.freedesktop.org/archives/dri-devel/2019-May/218362.html > >> > >> tldr; I have a multi-year plan to improve fbcon locking, because the > >> current thing is a bit a mess. > >> > >> Cover letter of this version, where I detail a bit more the details > >> fixed in this one here: > >> > >> https://lists.freedesktop.org/archives/dri-devel/2019-May/218984.html > >> > >> Note that the locking plan in this one is already outdated, I overlooked a > >> few fun issues around any printk() going back to console_lock. > >> > >> I think remaining bits: > >> > >> - Ack from Daniel Thompson for the backlight bits, he wanted to check the > >> big picture. > > > > I think Daniel is still on vacation until next week or so. > > > >> - Hash out actual merge plan. > > > > I'd like to stuff this into drm.git somehow, I guess topic branch works > > too. > > I would like to have topic branch for this patchset. Do you plan to prep that, or should I? Doesn't really matter to me, and assuming Daniel Thompson doesn't have any last minute concerns should still be enough time to get it all sorted and have a few weeks of testing left before the merge window. > > Long term I think we need to reconsider how we handle fbdev, at least the > > core/fbcon pieces. Since a few years all the work in that area has been > > motivated by drm, and pushed by drm contributors. Having that maintained > > in a separate tree that doesn't regularly integrate imo doesn't make much > > sense, and we ended up merging almost everything through some drm tree. > > That one time we didn't (for some panel rotation stuff) it resulted in > > some good suprises. > > > > I think best solution is if we put the core and fbcon bits into drm-misc, > > as group maintained infrastructure piece. All the other gfx infra pieces > > are maintained in there already too. You'd obviously get commit rights. > > I think that would include > > - drivers/video/fbdev > > - drivers/video/*c > > - drivers/video/console > > Sounds fine to me. > > > I don't really care about what happens with the actual fbdev drivers > > (aside from the drm one in drm_fb_helper.c, but that's already maintained > > as part of drm). I guess we could also put those into drm-misc, or as a > > separate tree, depending what you want. > > > > Thoughts? > > I would like to handle fbdev changes for v5.3 merge window using fbdev > tree but after that everything (including changes to fbdev drivers) can go > through drm-misc tree. Fully agreed, no need to rush anything here. For the drm-misc account you need to request an SSH legacy account here by filing a issue: https://gitlab.freedesktop.org/freedesktop/freedesktop/issues/new Select the "New SSH account" template, it has instructions with everything that's needed. For the getting started howto, see https://drm.pages.freedesktop.org/maintainer-tools/getting-started.html If that all looks good and you're set up I think best to test-drive the entire process with the MAINTAINERS patch to change the git repo for 5.4. Cheers, Daniel > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R Institute Poland > Samsung Electronics > > > Cheers, Daniel > > > > > >> > >> I'm also cc'ing the entire pile to a lot more people on request. > >> > >> Thanks, Daniel > >> > >> Daniel Vetter (33): > >> 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
[Bug 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #51 from Richard Thier --- > I do have an old 1024x768 monitor with VGA input, > I might give it a spin this weekend. But didn't the LCD also supports smaler resolutions natively? Most of them do even if their best native is more exotic ratios and stuff. Then you just need to change with xrandr to something smaller. -- 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 1/2] drm/vkms: Use index instead of 0 in possible crtc
On Fri, Jun 7, 2019 at 4:40 AM Daniel Vetter wrote:
>
> On Thu, Jun 06, 2019 at 07:40:38PM -0300, Rodrigo Siqueira wrote:
> > When vkms calls drm_universal_plane_init(), it sets 0 for the
> > possible_crtcs parameter which works well for a single encoder and
> > connector; however, this approach is not flexible and does not fit well
> > for vkms. This commit adds an index parameter for vkms_plane_init()
> > which makes code flexible and enables vkms to support other DRM features.
> >
> > Signed-off-by: Rodrigo Siqueira
>
> I think a core patch to WARN_ON if this is NULL would be good. Since
> that's indeed a bit broken ... We'd need to check all callers to make sure
> there's not other such bugs anywhere ofc.
> -Daniel
Do you mean add WARN_ON in `drm_universal_plane_init()` if
`possible_crtcs` is equal to zero?
> > ---
> > drivers/gpu/drm/vkms/vkms_drv.c| 2 +-
> > drivers/gpu/drm/vkms/vkms_drv.h| 4 ++--
> > drivers/gpu/drm/vkms/vkms_output.c | 6 +++---
> > drivers/gpu/drm/vkms/vkms_plane.c | 4 ++--
> > 4 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c
> > b/drivers/gpu/drm/vkms/vkms_drv.c
> > index 738dd6206d85..92296bd8f623 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -92,7 +92,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
> > dev->mode_config.max_height = YRES_MAX;
> > dev->mode_config.preferred_depth = 24;
> >
> > - return vkms_output_init(vkmsdev);
> > + return vkms_output_init(vkmsdev, 0);
> > }
> >
> > static int __init vkms_init(void)
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h
> > b/drivers/gpu/drm/vkms/vkms_drv.h
> > index 81f1cfbeb936..e81073dea154 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -113,10 +113,10 @@ bool vkms_get_vblank_timestamp(struct drm_device
> > *dev, unsigned int pipe,
> > int *max_error, ktime_t *vblank_time,
> > bool in_vblank_irq);
> >
> > -int vkms_output_init(struct vkms_device *vkmsdev);
> > +int vkms_output_init(struct vkms_device *vkmsdev, int index);
> >
> > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> > - enum drm_plane_type type);
> > + enum drm_plane_type type, int index);
> >
> > /* Gem stuff */
> > struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> > diff --git a/drivers/gpu/drm/vkms/vkms_output.c
> > b/drivers/gpu/drm/vkms/vkms_output.c
> > index 3b162b25312e..1442b447c707 100644
> > --- a/drivers/gpu/drm/vkms/vkms_output.c
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -36,7 +36,7 @@ static const struct drm_connector_helper_funcs
> > vkms_conn_helper_funcs = {
> > .get_modes= vkms_conn_get_modes,
> > };
> >
> > -int vkms_output_init(struct vkms_device *vkmsdev)
> > +int vkms_output_init(struct vkms_device *vkmsdev, int index)
> > {
> > struct vkms_output *output = >output;
> > struct drm_device *dev = >drm;
> > @@ -46,12 +46,12 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> > struct drm_plane *primary, *cursor = NULL;
> > int ret;
> >
> > - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY);
> > + primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index);
> > if (IS_ERR(primary))
> > return PTR_ERR(primary);
> >
> > if (enable_cursor) {
> > - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR);
> > + cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR,
> > index);
> > if (IS_ERR(cursor)) {
> > ret = PTR_ERR(cursor);
> > goto err_cursor;
> > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c
> > b/drivers/gpu/drm/vkms/vkms_plane.c
> > index 0e67d2d42f0c..20ffc52f9194 100644
> > --- a/drivers/gpu/drm/vkms/vkms_plane.c
> > +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> > @@ -168,7 +168,7 @@ static const struct drm_plane_helper_funcs
> > vkms_primary_helper_funcs = {
> > };
> >
> > struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> > - enum drm_plane_type type)
> > + enum drm_plane_type type, int index)
> > {
> > struct drm_device *dev = >drm;
> > const struct drm_plane_helper_funcs *funcs;
> > @@ -190,7 +190,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device
> > *vkmsdev,
> > funcs = _primary_helper_funcs;
> > }
> >
> > - ret = drm_universal_plane_init(dev, plane, 0,
> > + ret = drm_universal_plane_init(dev, plane, 1 << index,
> > _plane_funcs,
> > formats, nformats,
> > NULL, type, NULL);
> > --
> > 2.21.0
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
>
Re: [PATCH 2/2] drm/vkms: Add support for writeback
Hi Rodrigo,
On Thu, Jun 06, 2019 at 07:41:01PM -0300, Rodrigo Siqueira wrote:
> This patch implements the necessary functions to add writeback support
> for vkms. This feature is useful for testing compositors if you don’t
> have hardware with writeback support.
>
> Signed-off-by: Rodrigo Siqueira
> ---
> drivers/gpu/drm/vkms/Makefile | 9 +-
> drivers/gpu/drm/vkms/vkms_crtc.c | 5 +
> drivers/gpu/drm/vkms/vkms_drv.c | 10 ++
> drivers/gpu/drm/vkms/vkms_drv.h | 12 ++
> drivers/gpu/drm/vkms/vkms_output.c| 6 +
> drivers/gpu/drm/vkms/vkms_writeback.c | 165 ++
> 6 files changed, 206 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/vkms/vkms_writeback.c
>
> diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> index 89f09bec7b23..90eb7acd618d 100644
> --- a/drivers/gpu/drm/vkms/Makefile
> +++ b/drivers/gpu/drm/vkms/Makefile
> @@ -1,4 +1,11 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
> vkms_crc.o
> +vkms-y := \
> + vkms_drv.o \
> + vkms_plane.o \
> + vkms_output.o \
> + vkms_crtc.o \
> + vkms_gem.o \
> + vkms_crc.o \
> + vkms_writeback.o
>
> obj-$(CONFIG_DRM_VKMS) += vkms.o
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 1bbe099b7db8..ce797e265b1b 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -23,6 +23,11 @@ static enum hrtimer_restart vkms_vblank_simulate(struct
> hrtimer *timer)
> if (!ret)
> DRM_ERROR("vkms failure on handling vblank");
>
> + if (output->writeback_status == WB_START) {
> + drm_writeback_signal_completion(>wb_connector, 0);
> + output->writeback_status = WB_STOP;
> + }
> +
> if (state && output->crc_enabled) {
> u64 frame = drm_crtc_accurate_vblank_count(crtc);
>
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 92296bd8f623..d5917d5a45e3 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -29,6 +29,10 @@ bool enable_cursor;
> module_param_named(enable_cursor, enable_cursor, bool, 0444);
> MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
>
> +int enable_writeback;
> +module_param_named(enable_writeback, enable_writeback, int, 0444);
> +MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector");
> +
> static const struct file_operations vkms_driver_fops = {
> .owner = THIS_MODULE,
> .open = drm_open,
> @@ -123,6 +127,12 @@ static int __init vkms_init(void)
> goto out_fini;
> }
>
> + vkms_device->output.writeback_status = WB_DISABLED;
> + if (enable_writeback) {
> + vkms_device->output.writeback_status = WB_STOP;
> + DRM_INFO("Writeback connector enabled");
> + }
> +
> ret = vkms_modeset_init(vkms_device);
> if (ret)
> goto out_fini;
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index e81073dea154..ca1f9ee63ec8 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -7,6 +7,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #define XRES_MIN20
> @@ -60,14 +61,22 @@ struct vkms_crtc_state {
> u64 frame_end;
> };
>
> +enum wb_status {
> + WB_DISABLED,
> + WB_START,
> + WB_STOP,
> +};
> +
> struct vkms_output {
> struct drm_crtc crtc;
> struct drm_encoder encoder;
> struct drm_connector connector;
> + struct drm_writeback_connector wb_connector;
> struct hrtimer vblank_hrtimer;
> ktime_t period_ns;
> struct drm_pending_vblank_event *event;
> bool crc_enabled;
> + enum wb_status writeback_status;
> /* ordered wq for crc_work */
> struct workqueue_struct *crc_workq;
> /* protects concurrent access to crc_data */
> @@ -141,4 +150,7 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const
> char *source_name,
> size_t *values_cnt);
> void vkms_crc_work_handle(struct work_struct *work);
>
> +/* Writeback */
> +int enable_writeback_connector(struct vkms_device *vkmsdev);
> +
> #endif /* _VKMS_DRV_H_ */
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c
> b/drivers/gpu/drm/vkms/vkms_output.c
> index 1442b447c707..1fc1d4e9585c 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -91,6 +91,12 @@ int vkms_output_init(struct vkms_device *vkmsdev, int
> index)
> goto err_attach;
> }
>
> + if (vkmsdev->output.writeback_status != WB_DISABLED) {
> + ret = enable_writeback_connector(vkmsdev);
> + if (ret)
> + DRM_ERROR("Failed to init writeback connector\n");
> + }
> +
Re: [PATCH 2/5] drm/bridge: add encoder support to specify bridge input format
On 07/06/2019 15:38, Laurent Pinchart wrote:
> Hi Neil,
>
> Thank you for the patch.
>
> On Mon, May 20, 2019 at 03:37:50PM +0200, Neil Armstrong wrote:
>> This patch adds a new format_set() callback to the bridge ops permitting
>> the encoder to specify the new input format and encoding.
>>
>> This allows supporting the very specific HDMI2.0 YUV420 output mode
>> when the bridge cannot convert from RGB or YUV444 to YUV420.
>>
>> In this case, the encode must downsample before the bridge and must
>> specify the bridge the new input bus format differs.
>>
>> This will also help supporting the YUV420 mode where the bridge cannot
>> downsample, and also support 10bit, 12bit and 16bit output modes
>> when the bridge cannot convert between different bit depths.
>>
>> Signed-off-by: Neil Armstrong
>> ---
>> drivers/gpu/drm/drm_bridge.c | 35 +++
>> include/drm/drm_bridge.h | 19 +++
>> 2 files changed, 54 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
>> index 138b2711d389..33be74a977f7 100644
>> --- a/drivers/gpu/drm/drm_bridge.c
>> +++ b/drivers/gpu/drm/drm_bridge.c
>> @@ -307,6 +307,41 @@ void drm_bridge_mode_set(struct drm_bridge *bridge,
>> }
>> EXPORT_SYMBOL(drm_bridge_mode_set);
>>
>> +/**
>> + * drm_bridge_format_set - setup with proposed input format and encoding for
>> + * all bridges in the encoder chain
>> + * @bridge: bridge control structure
>> + * @input_bus_format: proposed input bus format for the bridge
>> + * @input_encoding: proposed input encoding for this bridge
>> + *
>> + * Calls _bridge_funcs.format_set op for all the bridges in the
>> + * encoder chain, starting from the first bridge to the last.
>> + *
>> + * Note: the bridge passed should be the one closest to the encoder
>> + *
>> + * RETURNS:
>> + * true on success, false if one of the bridge cannot handle the format
>
> I would return an int to propagate the failure reason upstream. It will
> reach the commit tail handler in any case, so will be dropped there, but
> could help debugging issues if we print it in the right place.
Indeed.
>
>> + */
>> +bool drm_bridge_format_set(struct drm_bridge *bridge,
>> + const u32 input_bus_format,
>> + const u32 input_encoding)
>
> You don't need a const here.
Noted
>
>> +{
>> +bool ret = true;
>> +
>> +if (!bridge)
>> +return true;
>> +
>> +if (bridge->funcs->format_set)
>> +ret = bridge->funcs->format_set(bridge, input_bus_format,
>> +input_encoding);
>> +if (!ret)
>> +return ret;
>> +
>> +return drm_bridge_format_set(bridge->next, input_bus_format,
>> + input_encoding);
>
> I don't think this will scale. It's not that uncommon for bridges to
> change the format (most likely converting from YUV to RGB or the other
> way around, or reducing the number of bits per sample) and the encoding.
> We thus can't propagate it from bridge to bridge and expect that to
> work.
>
> At the very least, the bridge should report its output bus format and
> encoding, to be applied to the next bridge, but this won't allow
> checking if the configuration can be applied ahead of time, resulting in
> possible failures of a commit tail handler. I wonder if this wouldn't be
> a good time to introduce bridge states...
Ok for chaining the format_set, I thought about it when writing it.
And what if a added a second call drm_bridge_format_check() to check
if a future format_set chaining would work ? then we could either do the
format_set, try another or fallback to a known default format setup.
Would that be sufficient ?
Bridge states would be interesting, but it's really out of the scope of
our current needs in term of bridge changes.
IMHO we can start with format_check/format_set and switch to bridges states
when we have a sufficient use case needing a finer handling of states.
Neil
>
>> +}
>> +EXPORT_SYMBOL(drm_bridge_format_set);
>> +
>> /**
>> * drm_bridge_pre_enable - prepares for enabling all
>> * bridges in the encoder chain
>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>> index d4428913a4e1..7a79e61b7825 100644
>> --- a/include/drm/drm_bridge.h
>> +++ b/include/drm/drm_bridge.h
>> @@ -198,6 +198,22 @@ struct drm_bridge_funcs {
>> void (*mode_set)(struct drm_bridge *bridge,
>> const struct drm_display_mode *mode,
>> const struct drm_display_mode *adjusted_mode);
>> +
>> +/**
>> + * @format_set:
>> + *
>> + * This callback should configure the bridge for the given input bus
>> + * format and encoding. It is called after the @format_set callback
>> + * for the preceding element in the display pipeline has been called
>> + * already. If the bridge is the first element then this
[Bug 102646] Screen flickering under amdgpu-experimental [buggy auto power profile]
https://bugs.freedesktop.org/show_bug.cgi?id=102646 --- Comment #92 from magist3r --- Just for the reference: screen flickering appears for me only with amdgpu.ppfeaturemask=0x. I think to flash my card with OC'ed BIOS and disable buggy OverDrive completely. -- 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
[Bug 109206] Kernel 4.20 amdgpu fails to load firmware on Ryzen 2500U
https://bugs.freedesktop.org/show_bug.cgi?id=109206 --- Comment #48 from Alex Deucher --- The patch is for the kernel. -- 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 2/5] drm/bridge: add encoder support to specify bridge input format
Hi Neil,
Thank you for the patch.
On Mon, May 20, 2019 at 03:37:50PM +0200, Neil Armstrong wrote:
> This patch adds a new format_set() callback to the bridge ops permitting
> the encoder to specify the new input format and encoding.
>
> This allows supporting the very specific HDMI2.0 YUV420 output mode
> when the bridge cannot convert from RGB or YUV444 to YUV420.
>
> In this case, the encode must downsample before the bridge and must
> specify the bridge the new input bus format differs.
>
> This will also help supporting the YUV420 mode where the bridge cannot
> downsample, and also support 10bit, 12bit and 16bit output modes
> when the bridge cannot convert between different bit depths.
>
> Signed-off-by: Neil Armstrong
> ---
> drivers/gpu/drm/drm_bridge.c | 35 +++
> include/drm/drm_bridge.h | 19 +++
> 2 files changed, 54 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 138b2711d389..33be74a977f7 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -307,6 +307,41 @@ void drm_bridge_mode_set(struct drm_bridge *bridge,
> }
> EXPORT_SYMBOL(drm_bridge_mode_set);
>
> +/**
> + * drm_bridge_format_set - setup with proposed input format and encoding for
> + * all bridges in the encoder chain
> + * @bridge: bridge control structure
> + * @input_bus_format: proposed input bus format for the bridge
> + * @input_encoding: proposed input encoding for this bridge
> + *
> + * Calls _bridge_funcs.format_set op for all the bridges in the
> + * encoder chain, starting from the first bridge to the last.
> + *
> + * Note: the bridge passed should be the one closest to the encoder
> + *
> + * RETURNS:
> + * true on success, false if one of the bridge cannot handle the format
I would return an int to propagate the failure reason upstream. It will
reach the commit tail handler in any case, so will be dropped there, but
could help debugging issues if we print it in the right place.
> + */
> +bool drm_bridge_format_set(struct drm_bridge *bridge,
> +const u32 input_bus_format,
> +const u32 input_encoding)
You don't need a const here.
> +{
> + bool ret = true;
> +
> + if (!bridge)
> + return true;
> +
> + if (bridge->funcs->format_set)
> + ret = bridge->funcs->format_set(bridge, input_bus_format,
> + input_encoding);
> + if (!ret)
> + return ret;
> +
> + return drm_bridge_format_set(bridge->next, input_bus_format,
> + input_encoding);
I don't think this will scale. It's not that uncommon for bridges to
change the format (most likely converting from YUV to RGB or the other
way around, or reducing the number of bits per sample) and the encoding.
We thus can't propagate it from bridge to bridge and expect that to
work.
At the very least, the bridge should report its output bus format and
encoding, to be applied to the next bridge, but this won't allow
checking if the configuration can be applied ahead of time, resulting in
possible failures of a commit tail handler. I wonder if this wouldn't be
a good time to introduce bridge states...
> +}
> +EXPORT_SYMBOL(drm_bridge_format_set);
> +
> /**
> * drm_bridge_pre_enable - prepares for enabling all
> * bridges in the encoder chain
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index d4428913a4e1..7a79e61b7825 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -198,6 +198,22 @@ struct drm_bridge_funcs {
> void (*mode_set)(struct drm_bridge *bridge,
>const struct drm_display_mode *mode,
>const struct drm_display_mode *adjusted_mode);
> +
> + /**
> + * @format_set:
> + *
> + * This callback should configure the bridge for the given input bus
> + * format and encoding. It is called after the @format_set callback
> + * for the preceding element in the display pipeline has been called
> + * already. If the bridge is the first element then this would be
> + * _encoder_helper_funcs.format_set. The display pipe (i.e.
> + * clocks and timing signals) is off when this function is called.
> + *
> + * @returns: true in success, false is a bridge refuses the format
> + */
> + bool (*format_set)(struct drm_bridge *bridge,
> +const u32 input_bus_format,
> +const u32 input_encoding);
> /**
>* @pre_enable:
>*
> @@ -311,6 +327,9 @@ void drm_bridge_post_disable(struct drm_bridge *bridge);
> void drm_bridge_mode_set(struct drm_bridge *bridge,
>const struct drm_display_mode *mode,
>const struct drm_display_mode *adjusted_mode);
> +bool
[Bug 110795] Unable to install on latest Ubuntu (19.04)
https://bugs.freedesktop.org/show_bug.cgi?id=110795 --- Comment #9 from Andrew Shark --- @Rolf, my script is specifically targeted at pro variant. I do not understand your desire to use amd bundled archive for All-Open stack, because All-Open is already provided by Ubuntu repos. However, you can just remove "--pro" from the end of the script in line: ./amdgpu-install --pro --opencl=lagacy,pal --no-dkms -- 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 1/2] drm/bridge: sii902x: re-order conditions to prevent out of bounds read
On 07.06.2019 09:27, Dan Carpenter wrote:
> This should check that "i" is within bounds before checking reading from
> the array.
>
> Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
> Signed-off-by: Dan Carpenter
> ---
> drivers/gpu/drm/bridge/sii902x.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/sii902x.c
> b/drivers/gpu/drm/bridge/sii902x.c
> index d6f98d388ac2..6b03616d6bc3 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -589,8 +589,8 @@ static int sii902x_audio_hw_params(struct device *dev,
> void *data,
> if (ret)
> goto out;
>
> - for (i = 0; sii902x->audio.i2s_fifo_sequence[i] &&
> - i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++)
> + for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) &&
> + sii902x->audio.i2s_fifo_sequence[i]; i++)
> regmap_write(sii902x->regmap,
>SII902X_TPI_I2S_ENABLE_MAPPING_REG,
>sii902x->audio.i2s_fifo_sequence[i]);
Queued to drm-misc-next.
--
Regards
Andrzej
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/bridge: sii902x: fix comparision of u32 with less than zero
On 03.06.2019 16:21, Colin King wrote:
> From: Colin Ian King
>
> The less than check for the variable num_lanes is always going to be
> false because the variable is a u32. Fix this by making num_lanes an
> int and also make loop index i an int too.
>
> Addresses-Coverity: ("Unsigned compared against 0")
> Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
> Signed-off-by: Colin Ian King
> ---
> drivers/gpu/drm/bridge/sii902x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/sii902x.c
> b/drivers/gpu/drm/bridge/sii902x.c
> index d6f98d388ac2..21a947603c88 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -719,7 +719,7 @@ static int sii902x_audio_codec_init(struct sii902x
> *sii902x,
> .max_i2s_channels = 0,
> };
> u8 lanes[4];
> - u32 num_lanes, i;
> + int num_lanes, i;
>
> if (!of_property_read_bool(dev->of_node, "#sound-dai-cells")) {
> dev_dbg(dev, "%s: No \"#sound-dai-cells\", no audio\n",
Queued to drm-misc-next.
--
Regards
Andrzej
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 5/5] kselftests: Add dma-heap test
Hi John,
I haven't looked at any selftests before, but is there any advantage
to using the test harness?
https://www.kernel.org/doc/html/v5.1/dev-tools/kselftest.html#test-harness
Couple of minor things below
On Fri, Jun 07, 2019 at 03:07:19AM +, John Stultz wrote:
> Add very trivial allocation and import test for dma-heaps,
> utilizing the vgem driver as a test importer.
>
> A good chunk of this code taken from:
> tools/testing/selftests/android/ion/ionmap_test.c
> Originally by Laura Abbott
>
> Cc: Benjamin Gaignard
> Cc: Sumit Semwal
> Cc: Liam Mark
> Cc: Pratik Patel
> Cc: Brian Starkey
> Cc: Vincent Donnefort
> Cc: Sudipto Paul
> Cc: Andrew F. Davis
> Cc: Christoph Hellwig
> Cc: Chenbo Feng
> Cc: Alistair Strachan
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Benjamin Gaignard
> Signed-off-by: John Stultz
> Change-Id: Ib98569fdda6378eb086b8092fb5d6bd419b8d431
> ---
> v2:
> * Switched to use reworked dma-heap apis
> v3:
> * Add simple mmap
> * Utilize dma-buf testdev to test importing
> v4:
> * Rework to use vgem
> * Pass in fd_flags to match interface changes
> * Skip . and .. dirs
> ---
> tools/testing/selftests/dmabuf-heaps/Makefile | 11 +
> .../selftests/dmabuf-heaps/dmabuf-heap.c | 232 ++
> 2 files changed, 243 insertions(+)
> create mode 100644 tools/testing/selftests/dmabuf-heaps/Makefile
> create mode 100644 tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
>
> diff --git a/tools/testing/selftests/dmabuf-heaps/Makefile
> b/tools/testing/selftests/dmabuf-heaps/Makefile
> new file mode 100644
> index ..c414ad36b4bf
> --- /dev/null
> +++ b/tools/testing/selftests/dmabuf-heaps/Makefile
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0
> +CFLAGS += -static -O3 -Wl,-no-as-needed -Wall
> +#LDLIBS += -lrt -lpthread -lm
> +
> +# these are all "safe" tests that don't modify
> +# system time or require escalated privileges
> +TEST_GEN_PROGS = dmabuf-heap
> +
newline
> +
> +include ../lib.mk
> +
> diff --git a/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
> b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
> new file mode 100644
> index ..33d4b105c673
> --- /dev/null
> +++ b/tools/testing/selftests/dmabuf-heaps/dmabuf-heap.c
> @@ -0,0 +1,232 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +
> +#include "../../../../include/uapi/linux/dma-heap.h"
> +
> +#define DEVPATH "/dev/dma_heap"
> +
> +int check_vgem(int fd)
Functions can be static
> +{
> + drm_version_t version = { 0 };
> + char name[5];
> + int ret;
> +
> + version.name_len = 4;
> + version.name = name;
> +
> + ret = ioctl(fd, DRM_IOCTL_VERSION, );
> + if (ret)
> + return 1;
> +
> + return strcmp(name, "vgem");
> +}
> +
> +int open_vgem(void)
> +{
> + int i, fd;
> + const char *drmstr = "/dev/dri/card";
> +
> + fd = -1;
> + for (i = 0; i < 16; i++) {
> + char name[80];
> +
> + sprintf(name, "%s%u", drmstr, i);
> +
> + fd = open(name, O_RDWR);
> + if (fd < 0)
> + continue;
> +
> + if (check_vgem(fd)) {
It's a minor thing, but the naming vs the logic reads backwards to me
here. I'd expect check_vgem() to return true for vgem.
> + close(fd);
> + continue;
> + } else {
> + break;
> + }
> +
> + }
> + return fd;
> +}
> +
> +int import_vgem_fd(int vgem_fd, int dma_buf_fd, uint32_t *handle)
> +{
> + struct drm_prime_handle import_handle = { 0 };
> + int ret;
> +
> + import_handle.fd = dma_buf_fd;
> + import_handle.flags = 0;
> + import_handle.handle = 0;
You could just initialise import_handle directly. Same for the other
functions
> +
> + ret = ioctl(vgem_fd, DRM_IOCTL_PRIME_FD_TO_HANDLE, _handle);
> + if (ret == 0)
> + *handle = import_handle.handle;
> + return ret;
> +}
> +
> +void close_handle(int vgem_fd, uint32_t handle)
> +{
> + struct drm_gem_close close = { 0 };
> +
> + close.handle = handle;
> + ioctl(vgem_fd, DRM_IOCTL_GEM_CLOSE, );
> +}
> +
> +
> +int dmabuf_heap_open(char *name)
> +{
> + int ret, fd;
> + char buf[256];
> +
> + ret = sprintf(buf, "%s/%s", DEVPATH, name);
> + if (ret < 0) {
> + printf("sprintf failed!\n");
> + return ret;
> + }
> +
> + fd = open(buf, O_RDWR);
> + if (fd < 0)
> + printf("open %s failed!\n", buf);
> + return fd;
> +}
> +
> +int dmabuf_heap_alloc(int fd, size_t len, unsigned int flags, int *dmabuf_fd)
> +{
> + struct dma_heap_allocation_data data = {
> + .len = len,
> + .fd_flags = O_RDWR | O_CLOEXEC,
> + .heap_flags = flags,
> + };
Like
[PATCH 5/6] drm/amdgpu: add independent DMA-buf export v3
The caching of SGT's is actually quite harmful and should probably removed
altogether when all drivers are audited.
Start by providing a separate DMA-buf export implementation in amdgpu. This is
also a prerequisite of unpinned DMA-buf handling.
v2: fix unintended recursion, remove debugging leftovers
v3: split out from unpinned DMA-buf work
v4: rebase on top of new no_sgt_cache flag
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 204
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 5 +
4 files changed, 133 insertions(+), 78 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 4711cf1b5bd2..68a071060793 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -40,22 +40,6 @@
#include
#include
-/**
- * amdgpu_gem_prime_get_sg_table - _driver.gem_prime_get_sg_table
- * implementation
- * @obj: GEM buffer object (BO)
- *
- * Returns:
- * A scatter/gather table for the pinned pages of the BO's memory.
- */
-struct sg_table *amdgpu_gem_prime_get_sg_table(struct drm_gem_object *obj)
-{
- struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
- int npages = bo->tbo.num_pages;
-
- return drm_prime_pages_to_sg(bo->tbo.ttm->pages, npages);
-}
-
/**
* amdgpu_gem_prime_vmap - _buf_ops.vmap implementation
* @obj: GEM BO
@@ -181,92 +165,158 @@ __reservation_object_make_exclusive(struct
reservation_object *obj)
}
/**
- * amdgpu_dma_buf_map_attach - _buf_ops.attach implementation
- * @dma_buf: Shared DMA buffer
+ * amdgpu_dma_buf_attach - _buf_ops.attach implementation
+ *
+ * @dmabuf: DMA-buf where we attach to
+ * @attach: attachment to add
+ *
+ * Add the attachment as user to the exported DMA-buf.
+ */
+static int amdgpu_dma_buf_attach(struct dma_buf *dmabuf,
+struct dma_buf_attachment *attach)
+{
+ struct drm_gem_object *obj = dmabuf->priv;
+ struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
+ struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+ int r;
+
+ if (attach->dev->driver == adev->dev->driver)
+ return 0;
+
+ /*
+* We only create shared fences for internal use, but importers
+* of the dmabuf rely on exclusive fences for implicitly
+* tracking write hazards. As any of the current fences may
+* correspond to a write, we need to convert all existing
+* fences on the reservation object into a single exclusive
+* fence.
+*/
+ r = __reservation_object_make_exclusive(bo->tbo.resv);
+ if (r)
+ return r;
+
+ bo->prime_shared_count++;
+ return 0;
+}
+
+/**
+ * amdgpu_dma_buf_detach - _buf_ops.detach implementation
+ *
+ * @dmabuf: DMA-buf where we remove the attachment from
+ * @attach: the attachment to remove
+ *
+ * Called when an attachment is removed from the DMA-buf.
+ */
+static void amdgpu_dma_buf_detach(struct dma_buf *dmabuf,
+ struct dma_buf_attachment *attach)
+{
+ struct drm_gem_object *obj = dmabuf->priv;
+ struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
+ struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+
+ if (attach->dev->driver != adev->dev->driver && bo->prime_shared_count)
+ bo->prime_shared_count--;
+}
+
+/**
+ * amdgpu_dma_buf_pin - _buf_ops.pin implementation
+ *
+ * @attach: attachment to pin down
+ *
+ * Pin the BO which is backing the DMA-buf so that it can't move any more.
+ */
+static int amdgpu_dma_buf_pin(struct dma_buf_attachment *attach)
+{
+ struct drm_gem_object *obj = attach->dmabuf->priv;
+ struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
+
+ /* pin buffer into GTT */
+ return amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT);
+}
+
+/**
+ * amdgpu_dma_buf_unpin - _buf_ops.unpin implementation
+ *
+ * @attach: attachment to unpin
+ *
+ * Unpin a previously pinned BO to make it movable again.
+ */
+static void amdgpu_dma_buf_unpin(struct dma_buf_attachment *attach)
+{
+ struct drm_gem_object *obj = attach->dmabuf->priv;
+ struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
+
+ amdgpu_bo_unpin(bo);
+}
+
+/**
+ * amdgpu_dma_buf_map_dma_buf - _buf_ops.map_dma_buf implementation
* @attach: DMA-buf attachment
+ * @dir: DMA direction
*
* Makes sure that the shared DMA buffer can be accessed by the target device.
* For now, simply pins it to the GTT domain, where it should be accessible by
* all DMA devices.
*
* Returns:
- * 0 on success or a negative error code on failure.
+ * sg_table filled with the DMA addresses to use or ERR_PRT with negative error
+ * code.
*/
-static int amdgpu_dma_buf_map_attach(struct dma_buf *dma_buf,
-struct dma_buf_attachment
[PATCH 3/6] drm/ttm: use the parent resv for ghost objects v2
This way we can even pipeline imported BO evictions.
v2: Limit this to only cases when the parent object uses a separate reservation
object as well. This fixes another OOM problem.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 895d77d799e4..95f47d685921 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -517,9 +517,11 @@ static int ttm_buffer_object_transfer(struct
ttm_buffer_object *bo,
kref_init(>base.kref);
fbo->base.destroy = _transfered_destroy;
fbo->base.acc_size = 0;
- fbo->base.resv = >base.ttm_resv;
- reservation_object_init(fbo->base.resv);
- ret = reservation_object_trylock(fbo->base.resv);
+ if (bo->resv == >ttm_resv)
+ fbo->base.resv = >base.ttm_resv;
+
+ reservation_object_init(>base.ttm_resv);
+ ret = reservation_object_trylock(>base.ttm_resv);
WARN_ON(!ret);
*new_obj = >base;
@@ -716,7 +718,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
if (ret)
return ret;
- reservation_object_add_excl_fence(ghost_obj->resv, fence);
+ reservation_object_add_excl_fence(_obj->ttm_resv, fence);
/**
* If we're not moving to fixed memory, the TTM object
@@ -729,7 +731,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
else
bo->ttm = NULL;
- ttm_bo_unreserve(ghost_obj);
+ reservation_object_unlock(_obj->ttm_resv);
ttm_bo_put(ghost_obj);
}
@@ -772,7 +774,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
if (ret)
return ret;
- reservation_object_add_excl_fence(ghost_obj->resv, fence);
+ reservation_object_add_excl_fence(_obj->ttm_resv, fence);
/**
* If we're not moving to fixed memory, the TTM object
@@ -785,7 +787,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
else
bo->ttm = NULL;
- ttm_bo_unreserve(ghost_obj);
+ reservation_object_unlock(_obj->ttm_resv);
ttm_bo_put(ghost_obj);
} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
@@ -841,7 +843,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
if (ret)
return ret;
- ret = reservation_object_copy_fences(ghost->resv, bo->resv);
+ ret = reservation_object_copy_fences(>ttm_resv, bo->resv);
/* Last resort, wait for the BO to be idle when we are OOM */
if (ret)
ttm_bo_wait(bo, false, false);
@@ -850,7 +852,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
bo->mem.mem_type = TTM_PL_SYSTEM;
bo->ttm = NULL;
- ttm_bo_unreserve(ghost);
+ reservation_object_unlock(>ttm_resv);
ttm_bo_put(ghost);
return 0;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/6] drm/ttm: remove the backing store if no placement is given
Pipeline removal of the BOs backing store when no placement is given
during validation.
Signed-off-by: Christian König
---
drivers/gpu/drm/ttm/ttm_bo.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index c7de667d482a..682a1a035b35 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1240,6 +1240,18 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
uint32_t new_flags;
reservation_object_assert_held(bo->resv);
+
+ /*
+* Remove the backing store if no placement is given.
+*/
+ if (!placement->num_placement && !placement->num_busy_placement) {
+ ret = ttm_bo_pipeline_gutting(bo);
+ if (ret)
+ return ret;
+
+ return ttm_tt_create(bo, false);
+ }
+
/*
* Check whether we need to move buffer.
*/
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/6] drm/amdgpu: add independent DMA-buf import v6
Instead of relying on the DRM functions just implement our own import
functions. This prepares support for taking care of unpinned DMA-buf.
v2: enable for all exporters, not just amdgpu, fix invalidation
handling, lock reservation object while setting callback
v3: change to new dma_buf attach interface
v4: split out from unpinned DMA-buf work
v5: rebased and cleanup on new DMA-buf interface
v6: squash with invalidation callback change,
stop using _(map|unmap)_locked
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 65 -
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h | 4 --
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 -
drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 32 --
5 files changed, 83 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 68a071060793..63dedc93c3d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -420,31 +420,28 @@ struct dma_buf *amdgpu_gem_prime_export(struct drm_device
*dev,
}
/**
- * amdgpu_gem_prime_import_sg_table - _driver.gem_prime_import_sg_table
- * implementation
+ * amdgpu_dma_buf_create_obj - create BO for DMA-buf import
+ *
* @dev: DRM device
- * @attach: DMA-buf attachment
- * @sg: Scatter/gather table
+ * @dma_buf: DMA-buf
*
- * Imports shared DMA buffer memory exported by another device.
+ * Creates an empty SG BO for DMA-buf import.
*
* Returns:
* A new GEM BO of the given DRM device, representing the memory
* described by the given DMA-buf attachment and scatter/gather table.
*/
-struct drm_gem_object *
-amdgpu_gem_prime_import_sg_table(struct drm_device *dev,
-struct dma_buf_attachment *attach,
-struct sg_table *sg)
+static struct drm_gem_object *
+amdgpu_dma_buf_create_obj(struct drm_device *dev, struct dma_buf *dma_buf)
{
- struct reservation_object *resv = attach->dmabuf->resv;
+ struct reservation_object *resv = dma_buf->resv;
struct amdgpu_device *adev = dev->dev_private;
struct amdgpu_bo *bo;
struct amdgpu_bo_param bp;
int ret;
memset(, 0, sizeof(bp));
- bp.size = attach->dmabuf->size;
+ bp.size = dma_buf->size;
bp.byte_align = PAGE_SIZE;
bp.domain = AMDGPU_GEM_DOMAIN_CPU;
bp.flags = 0;
@@ -455,11 +452,9 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev,
if (ret)
goto error;
- bo->tbo.sg = sg;
- bo->tbo.ttm->sg = sg;
bo->allowed_domains = AMDGPU_GEM_DOMAIN_GTT;
bo->preferred_domains = AMDGPU_GEM_DOMAIN_GTT;
- if (attach->dmabuf->ops != _dmabuf_ops)
+ if (dma_buf->ops != _dmabuf_ops)
bo->prime_shared_count = 1;
ww_mutex_unlock(>lock);
@@ -470,6 +465,32 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev,
return ERR_PTR(ret);
}
+/**
+ * amdgpu_gem_prime_invalidate_mappings - implementation
+ *
+ * @attach: the DMA-buf attachment
+ *
+ * Invalidate the DMA-buf attachment, making sure that the we re-create the
+ * mapping before the next use.
+ */
+static void
+amdgpu_gem_prime_invalidate_mappings(struct dma_buf_attachment *attach)
+{
+ struct ttm_operation_ctx ctx = { false, false };
+ struct drm_gem_object *obj = attach->importer_priv;
+ struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
+ struct ttm_placement placement = {};
+ int r;
+
+ r = ttm_bo_validate(>tbo, , );
+ if (r)
+ DRM_ERROR("Failed to invalidate DMA-buf import (%d))\n", r);
+}
+
+static const struct dma_buf_attach_ops amdgpu_dma_buf_attach_ops = {
+ .invalidate = amdgpu_gem_prime_invalidate_mappings
+};
+
/**
* amdgpu_gem_prime_import - _driver.gem_prime_import implementation
* @dev: DRM device
@@ -484,6 +505,7 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev,
struct drm_gem_object *amdgpu_gem_prime_import(struct drm_device *dev,
struct dma_buf *dma_buf)
{
+ struct dma_buf_attachment *attach;
struct drm_gem_object *obj;
if (dma_buf->ops == _dmabuf_ops) {
@@ -498,5 +520,18 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct
drm_device *dev,
}
}
- return drm_gem_prime_import(dev, dma_buf);
+ obj = amdgpu_dma_buf_create_obj(dev, dma_buf);
+ if (IS_ERR(obj))
+ return obj;
+
+ attach = dma_buf_dynamic_attach(dma_buf, dev->dev,
+ _dma_buf_attach_ops, obj);
+ if (IS_ERR(attach)) {
+ drm_gem_object_put(obj);
+ return ERR_CAST(attach);
+ }
+
+ get_dma_buf(dma_buf);
+ obj->import_attach = attach;
+ return obj;
}
diff --git
[PATCH 1/6] dma-buf: add dynamic DMA-buf handling v9
On the exporter side we add optional explicit pinning callbacks. If those
callbacks are implemented the framework no longer caches sg tables and the
map/unmap callbacks are always called with the lock of the reservation object
held.
On the importer side we add an optional invalidate callback. This callback is
used by the exporter to inform the importers that their mappings should be
destroyed as soon as possible.
This allows the exporter to provide the mappings without the need to pin
the backing store.
v2: don't try to invalidate mappings when the callback is NULL,
lock the reservation obj while using the attachments,
add helper to set the callback
v3: move flag for invalidation support into the DMA-buf,
use new attach_info structure to set the callback
v4: use importer_priv field instead of mangling exporter priv.
v5: drop invalidation_supported flag
v6: squash together with pin/unpin changes
v7: pin/unpin takes an attachment now
v8: nuke dma_buf_attachment_(map|unmap)_locked,
everything is now handled backward compatible
v9: always cache when export/importer don't agree on dynamic handling
Signed-off-by: Christian König
---
drivers/dma-buf/dma-buf.c | 188 --
include/linux/dma-buf.h | 109 --
2 files changed, 283 insertions(+), 14 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index a2d34c6b80a5..f756870ba509 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -409,6 +409,9 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
return ERR_PTR(-EINVAL);
}
+ if (WARN_ON(exp_info->ops->cache_sgt_mapping && exp_info->ops->pin))
+ return ERR_PTR(-EINVAL);
+
if (!try_module_get(exp_info->owner))
return ERR_PTR(-ENOENT);
@@ -530,10 +533,12 @@ void dma_buf_put(struct dma_buf *dmabuf)
EXPORT_SYMBOL_GPL(dma_buf_put);
/**
- * dma_buf_attach - Add the device to dma_buf's attachments list; optionally,
+ * dma_buf_dynamic_attach - Add the device to dma_buf's attachments list;
optionally,
* calls attach() of dma_buf_ops to allow device-specific attach functionality
- * @dmabuf:[in]buffer to attach device to.
- * @dev: [in]device to be attached.
+ * @dmabuf:[in]buffer to attach device to.
+ * @dev: [in]device to be attached.
+ * @importer_ops [in]importer operations for the attachment
+ * @importer_priv [in]importer private pointer for the attachment
*
* Returns struct dma_buf_attachment pointer for this attachment. Attachments
* must be cleaned up by calling dma_buf_detach().
@@ -547,8 +552,10 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
* accessible to @dev, and cannot be moved to a more suitable place. This is
* indicated with the error code -EBUSY.
*/
-struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
- struct device *dev)
+struct dma_buf_attachment *
+dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
+ const struct dma_buf_attach_ops *importer_ops,
+ void *importer_priv)
{
struct dma_buf_attachment *attach;
int ret;
@@ -562,6 +569,8 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf
*dmabuf,
attach->dev = dev;
attach->dmabuf = dmabuf;
+ attach->importer_ops = importer_ops;
+ attach->importer_priv = importer_priv;
mutex_lock(>lock);
@@ -570,16 +579,72 @@ struct dma_buf_attachment *dma_buf_attach(struct dma_buf
*dmabuf,
if (ret)
goto err_attach;
}
+ reservation_object_lock(dmabuf->resv, NULL);
list_add(>node, >attachments);
+ reservation_object_unlock(dmabuf->resv);
mutex_unlock(>lock);
+ /* When either the importer or the exporter can't handle dynamic
+* mappings we cache the mapping here to avoid issues with the
+* reservation object lock.
+*/
+ if (dma_buf_attachment_is_dynamic(attach) !=
+ dma_buf_is_dynamic(dmabuf)) {
+ struct sg_table *sgt;
+
+ if (dma_buf_is_dynamic(attach->dmabuf)) {
+ reservation_object_lock(attach->dmabuf->resv, NULL);
+ ret = dma_buf_pin(attach);
+ if (ret)
+ goto err_unlock;
+ }
+
+ sgt = dmabuf->ops->map_dma_buf(attach, DMA_BIDIRECTIONAL);
+ if (!sgt)
+ sgt = ERR_PTR(-ENOMEM);
+ if (IS_ERR(sgt)) {
+ ret = PTR_ERR(sgt);
+ goto err_unpin;
+ }
+ if (dma_buf_is_dynamic(attach->dmabuf))
+ reservation_object_unlock(attach->dmabuf->resv);
+ attach->sgt = sgt;
+
[PATCH 4/6] drm/amdgpu: use allowed_domains for exported DMA-bufs
Avoid that we ping/pong the buffers when we stop to pin DMA-buf
exports by using the allowed domains for exported buffers.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 3e2da24cd17a..a59780654335 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -26,6 +26,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -412,7 +413,9 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
/* Don't move this buffer if we have depleted our allowance
* to move it. Don't move anything if the threshold is zero.
*/
- if (p->bytes_moved < p->bytes_moved_threshold) {
+ if (p->bytes_moved < p->bytes_moved_threshold &&
+ (!bo->gem_base.dma_buf ||
+ list_empty(>gem_base.dma_buf->attachments))) {
if (!amdgpu_gmc_vram_full_visible(>gmc) &&
(bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)) {
/* And don't move a CPU_ACCESS_REQUIRED BO to limited
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/stm: ltdc: No message if probe
Le ven. 7 juin 2019 à 10:51, Philippe CORNU a écrit :
>
> Hi Yannick,
>
> Thank you for your patch
>
> Acked-by: Philippe Cornu
Applied on drm-misc-next
Thanks,
Benjamin
> Philippe :-)
>
> On 6/3/19 10:31 AM, Yannick Fertré wrote:
> > Print display controller hardware version in debug mode only.
> >
> > Signed-off-by: Yannick Fertré
> > ---
> > drivers/gpu/drm/stm/ltdc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> > index a40870b..2fe6c4a 100644
> > --- a/drivers/gpu/drm/stm/ltdc.c
> > +++ b/drivers/gpu/drm/stm/ltdc.c
> > @@ -1229,7 +1229,7 @@ int ltdc_load(struct drm_device *ddev)
> > goto err;
> > }
> >
> > - DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version);
> > + DRM_DEBUG_DRIVER("ltdc hw version 0x%08x\n", ldev->caps.hw_version);
> >
> > /* Add endpoints panels or bridges if any */
> > for (i = 0; i < MAX_ENDPOINTS; i++) {
> >
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/stm: support runtime power management
Le ven. 7 juin 2019 à 13:19, Philippe CORNU a écrit :
>
> Hi Yannick,
>
> Thank you for your patch
>
> Acked-by: Philippe Cornu
Applied on drm-misc-next
Thanks,
Benjamin
>
> Philippe :-)
>
> On 6/3/19 10:32 AM, Yannick Fertré wrote:
> > This patch enables runtime power management (runtime PM) support for
> > the display controller. pm_runtime_enable() and pm_runtime_disable()
> > are added during ltdc load and unload respectively.
> > pm_runtime_get_sync() and pm_runtime_put_sync() are added for ltdc
> > register access.
> >
> > Signed-off-by: Yannick Fertré
> > ---
> > drivers/gpu/drm/stm/drv.c | 43 +++--
> > drivers/gpu/drm/stm/ltdc.c | 67
> > +++---
> > 2 files changed, 86 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
> > index 5834ef5..5659572 100644
> > --- a/drivers/gpu/drm/stm/drv.c
> > +++ b/drivers/gpu/drm/stm/drv.c
> > @@ -12,6 +12,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include
> > #include
> > @@ -135,14 +136,15 @@ static __maybe_unused int drv_suspend(struct device
> > *dev)
> > struct ltdc_device *ldev = ddev->dev_private;
> > struct drm_atomic_state *state;
> >
> > - drm_kms_helper_poll_disable(ddev);
> > + if (WARN_ON(!ldev->suspend_state))
> > + return -ENOENT;
> > +
> > state = drm_atomic_helper_suspend(ddev);
> > - if (IS_ERR(state)) {
> > - drm_kms_helper_poll_enable(ddev);
> > + if (IS_ERR(state))
> > return PTR_ERR(state);
> > - }
> > +
> > ldev->suspend_state = state;
> > - ltdc_suspend(ddev);
> > + pm_runtime_force_suspend(dev);
> >
> > return 0;
> > }
> > @@ -151,16 +153,41 @@ static __maybe_unused int drv_resume(struct device
> > *dev)
> > {
> > struct drm_device *ddev = dev_get_drvdata(dev);
> > struct ltdc_device *ldev = ddev->dev_private;
> > + int ret;
> >
> > - ltdc_resume(ddev);
> > - drm_atomic_helper_resume(ddev, ldev->suspend_state);
> > - drm_kms_helper_poll_enable(ddev);
> > + pm_runtime_force_resume(dev);
> > + ret = drm_atomic_helper_resume(ddev, ldev->suspend_state);
> > + if (ret) {
> > + pm_runtime_force_suspend(dev);
> > + ldev->suspend_state = NULL;
> > + return ret;
> > + }
> >
> > return 0;
> > }
> >
> > +static __maybe_unused int drv_runtime_suspend(struct device *dev)
> > +{
> > + struct drm_device *ddev = dev_get_drvdata(dev);
> > +
> > + DRM_DEBUG_DRIVER("\n");
> > + ltdc_suspend(ddev);
> > +
> > + return 0;
> > +}
> > +
> > +static __maybe_unused int drv_runtime_resume(struct device *dev)
> > +{
> > + struct drm_device *ddev = dev_get_drvdata(dev);
> > +
> > + DRM_DEBUG_DRIVER("\n");
> > + return ltdc_resume(ddev);
> > +}
> > +
> > static const struct dev_pm_ops drv_pm_ops = {
> > SET_SYSTEM_SLEEP_PM_OPS(drv_suspend, drv_resume)
> > + SET_RUNTIME_PM_OPS(drv_runtime_suspend,
> > +drv_runtime_resume, NULL)
> > };
> >
> > static int stm_drm_platform_probe(struct platform_device *pdev)
> > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> > index ac29890..a40870b 100644
> > --- a/drivers/gpu/drm/stm/ltdc.c
> > +++ b/drivers/gpu/drm/stm/ltdc.c
> > @@ -16,6 +16,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #include
> > @@ -444,6 +445,7 @@ static void ltdc_crtc_atomic_disable(struct drm_crtc
> > *crtc,
> >struct drm_crtc_state *old_state)
> > {
> > struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> > + struct drm_device *ddev = crtc->dev;
> >
> > DRM_DEBUG_DRIVER("\n");
> >
> > @@ -457,6 +459,8 @@ static void ltdc_crtc_atomic_disable(struct drm_crtc
> > *crtc,
> >
> > /* immediately commit disable of layers before switching off LTDC */
> > reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR);
> > +
> > + pm_runtime_put_sync(ddev->dev);
> > }
> >
> > #define CLK_TOLERANCE_HZ 50
> > @@ -505,17 +509,31 @@ static bool ltdc_crtc_mode_fixup(struct drm_crtc
> > *crtc,
> >struct drm_display_mode *adjusted_mode)
> > {
> > struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> > + struct drm_device *ddev = crtc->dev;
> > int rate = mode->clock * 1000;
> > + bool runtime_active;
> > + int ret;
> > +
> > + runtime_active = pm_runtime_active(ddev->dev);
> > +
> > + if (runtime_active)
> > + pm_runtime_put_sync(ddev->dev);
> >
> > - clk_disable(ldev->pixel_clk);
> > if (clk_set_rate(ldev->pixel_clk, rate) < 0) {
> > DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate);
> > return false;
> > }
> > - clk_enable(ldev->pixel_clk);
> >
> > adjusted_mode->clock = clk_get_rate(ldev->pixel_clk)
Re: [PATCH v5 4/5] dma-buf: heaps: Add CMA heap to dmabuf heaps
Hi,
On Fri, Jun 07, 2019 at 03:07:18AM +, John Stultz wrote:
> This adds a CMA heap, which allows userspace to allocate
> a dma-buf of contiguous memory out of a CMA region.
>
> This code is an evolution of the Android ION implementation, so
> thanks to its original author and maintainters:
> Benjamin Gaignard, Laura Abbott, and others!
>
> Cc: Laura Abbott
> Cc: Benjamin Gaignard
> Cc: Sumit Semwal
> Cc: Liam Mark
> Cc: Pratik Patel
> Cc: Brian Starkey
> Cc: Vincent Donnefort
> Cc: Sudipto Paul
> Cc: Andrew F. Davis
> Cc: Christoph Hellwig
> Cc: Chenbo Feng
> Cc: Alistair Strachan
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Benjamin Gaignard
> Signed-off-by: John Stultz
> Change-Id: Ic2b0c5dfc0dbaff5245bd1c50170c64b06c73051
> ---
> v2:
> * Switch allocate to return dmabuf fd
> * Simplify init code
> * Checkpatch fixups
> v3:
> * Switch to inline function for to_cma_heap()
> * Minor cleanups suggested by Brian
> * Fold in new registration style from Andrew
> * Folded in changes from Andrew to use simplified page list
> from the heap helpers
> v4:
> * Use the fd_flags when creating dmabuf fd (Suggested by
> Benjamin)
> * Use precalculated pagecount (Suggested by Andrew)
> ---
> drivers/dma-buf/heaps/Kconfig| 8 ++
> drivers/dma-buf/heaps/Makefile | 1 +
> drivers/dma-buf/heaps/cma_heap.c | 169 +++
> 3 files changed, 178 insertions(+)
> create mode 100644 drivers/dma-buf/heaps/cma_heap.c
>
> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> index 205052744169..a5eef06c4226 100644
> --- a/drivers/dma-buf/heaps/Kconfig
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -4,3 +4,11 @@ config DMABUF_HEAPS_SYSTEM
> help
> Choose this option to enable the system dmabuf heap. The system heap
> is backed by pages from the buddy allocator. If in doubt, say Y.
> +
> +config DMABUF_HEAPS_CMA
> + bool "DMA-BUF CMA Heap"
> + depends on DMABUF_HEAPS && DMA_CMA
> + help
> + Choose this option to enable dma-buf CMA heap. This heap is backed
> + by the Contiguous Memory Allocator (CMA). If your system has these
> + regions, you should say Y here.
> diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> index d1808eca2581..6e54cdec3da0 100644
> --- a/drivers/dma-buf/heaps/Makefile
> +++ b/drivers/dma-buf/heaps/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-y+= heap-helpers.o
> obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)+= system_heap.o
> +obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o
> diff --git a/drivers/dma-buf/heaps/cma_heap.c
> b/drivers/dma-buf/heaps/cma_heap.c
> new file mode 100644
> index ..3d0ffbbd0a34
> --- /dev/null
> +++ b/drivers/dma-buf/heaps/cma_heap.c
> @@ -0,0 +1,169 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DMABUF CMA heap exporter
> + *
> + * Copyright (C) 2012, 2019 Linaro Ltd.
> + * Author: for ST-Ericsson.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "heap-helpers.h"
> +
> +struct cma_heap {
> + struct dma_heap *heap;
> + struct cma *cma;
> +};
> +
> +static void cma_heap_free(struct heap_helper_buffer *buffer)
> +{
> + struct cma_heap *cma_heap = dma_heap_get_data(buffer->heap_buffer.heap);
> + unsigned long nr_pages = buffer->pagecount;
> + struct page *pages = buffer->priv_virt;
> +
> + /* free page list */
> + kfree(buffer->pages);
> + /* release memory */
> + cma_release(cma_heap->cma, pages, nr_pages);
> + kfree(buffer);
Sorry for not managing to review the past couple of revisions where
the helper buffer pages were added, but:
I'm probably a bit dim; but I got a bit confused about "pages" vs
"buffer->pages" here and in the allocate function.
Could I suggest a different name for the CMA allocation? Even simply
"cma_pages" would be clearer IMO.
Otherwise LGTM:
Reviewed-by: Brian Starkey
> +}
> +
> +/* dmabuf heap CMA operations functions */
> +static int cma_heap_allocate(struct dma_heap *heap,
> + unsigned long len,
> + unsigned long fd_flags,
> + unsigned long heap_flags)
> +{
> + struct cma_heap *cma_heap = dma_heap_get_data(heap);
> + struct heap_helper_buffer *helper_buffer;
> + struct page *pages;
> + size_t size = PAGE_ALIGN(len);
> + unsigned long nr_pages = size >> PAGE_SHIFT;
> + unsigned long align = get_order(size);
> + DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> + struct dma_buf *dmabuf;
> + int ret = -ENOMEM;
> + pgoff_t pg;
> +
> + if (align > CONFIG_CMA_ALIGNMENT)
> + align = CONFIG_CMA_ALIGNMENT;
> +
> + helper_buffer = kzalloc(sizeof(*helper_buffer), GFP_KERNEL);
> + if (!helper_buffer)
> + return -ENOMEM;
> +
> +
Re: [PATCH v3 2/2] video: fbdev: pvr2fb: add COMPILE_TEST support
On 5/24/19 1:58 PM, Bartlomiej Zolnierkiewicz wrote: > Add COMPILE_TEST support to pvr2fb driver for better compile > testing coverage. > > While at it: > > - mark pvr2fb_interrupt() and pvr2fb_common_init() with > __maybe_unused tag (to silence build warnings when > !SH_DREAMCAST) > > - convert mmio_base in struct pvr2fb_par to 'void __iomem *' > from 'unsigned long' (needed to silence build warnings on > ARM). > > - split pvr2_get_param() on pvr2_get_param_name() and > pvr2_get_param_val() (needed to silence build warnings on > x86). > > Signed-off-by: Bartlomiej Zolnierkiewicz I queued the patch for v5.3. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
