dri-devel August 2019

dri-devel@lists.freedesktop.org

318 participants
2338 discussions

[PATCH 0/8] drm/bridge: dw-hdmi: improve i2s support
by Jerome Brunet 07 Aug '19

07 Aug '19

The purpose of this patchset is to improve the support of the i2s interface of the synopsys hdmi controller. Once applied, the interface should support all the usual i2s bus formats, 8 channels playback and properly setup the channel number and allocation in the infoframes. Also, the dw-hdmi i2s interface will now provide the eld to the generic hdmi-codec so it can expose the related controls to user space. This work was inspired by Jonas Karlman's work, available here [0]. This was tested the Amlogic meson-g12a-sei510 platform. For this specific platform, which uses codec2codec links, there is a runtime dependency for patch 8 on this ASoC series [1]. [0]: https://github.com/Kwiboo/linux-rockchip/commits/rockchip-5.2-for-libreelec… [1]: https://lkml.kernel.org/r/20190725165949.29699-1-jbrunet@baylibre.com Jerome Brunet (8): drm/bridge: dw-hdmi-i2s: support more i2s format drm/bridge: dw-hdmi: move audio channel setup out of ahb drm/bridge: dw-hdmi: set channel count in the infoframes drm/bridge: dw-hdmi-i2s: enable lpcm multi channels drm/bridge: dw-hdmi-i2s: set the channel allocation drm/bridge: dw-hdmi-i2s: reset audio fifo before applying new params drm/bridge: dw-hdmi-i2s: enable only the required i2s lanes drm/bridge: dw-hdmi-i2s: add .get_eld support .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 20 ++----- .../gpu/drm/bridge/synopsys/dw-hdmi-audio.h | 1 + .../drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 59 +++++++++++++++++-- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 37 ++++++++++++ drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 13 +++- include/drm/bridge/dw_hdmi.h | 2 + 6 files changed, 107 insertions(+), 25 deletions(-) -- 2.21.0

2 10

Re: [PATCH 1/2] drm: add cache support for arm64
by Rob Clark 07 Aug '19

07 Aug '19

On Tue, Aug 6, 2019 at 11:25 PM Christoph Hellwig <hch(a)lst.de> wrote: > > On Tue, Aug 06, 2019 at 09:23:51AM -0700, Rob Clark wrote: > > On Tue, Aug 6, 2019 at 8:50 AM Christoph Hellwig <hch(a)lst.de> wrote: > > > > > > On Tue, Aug 06, 2019 at 07:11:41AM -0700, Rob Clark wrote: > > > > Agreed that drm_cflush_* isn't a great API. In this particular case > > > > (IIUC), I need wb+inv so that there aren't dirty cache lines that drop > > > > out to memory later, and so that I don't get a cache hit on > > > > uncached/wc mmap'ing. > > > > > > So what is the use case here? Allocate pages using the page allocator > > > (or CMA for that matter), and then mmaping them to userspace and never > > > touching them again from the kernel? > > > > Currently, it is pages coming from tmpfs. Ideally we want pages that > > are swappable when unpinned. > > tmpfs is basically a (complicated) frontend for alloc pages as far > as page allocation is concerned. > > > CPU mappings are *mostly* just mapping to userspace. There are a few > > exceptions that are vmap'd (fbcon, and ringbuffer). > > And those use the same backend? yes > > (Eventually I'd like to support pages passed in from userspace.. but > > that is down the road.) > > Eww. Please talk to the iommu list before starting on that. This is more of a long term goal, we can't do it until we have per-context/process pagetables, ofc. Getting a bit off topic, but I'm curious about what problems you are concerned about. Userspace can shoot it's own foot, but if it is not sharing GPU pagetables with other processes, it can't shoot other's feet. (I'm guessing you are concerned about non-page-aligned mappings?) > > > > Tying it in w/ iommu seems a bit weird to me.. but maybe that is just > > > > me, I'm certainly willing to consider proposals or to try things and > > > > see how they work out. > > > > > > This was just my through as the fit seems easy. But maybe you'll > > > need to explain your use case(s) a bit more so that we can figure out > > > what a good high level API is. > > > > Tying it to iommu_map/unmap would be awkward, as we could need to > > setup cpu mmap before it ends up mapped to iommu. And the plan to > > support per-process pagetables involved creating an iommu_domain per > > userspace gl context.. some buffers would end up mapped into multiple > > contexts/iommu_domains. > > > > If the cache operation was detached from iommu_map/unmap, then it > > would seem weird to be part of the iommu API. > > > > I guess I'm not entirely sure what you had in mind, but this is why > > iommu seemed to me like a bad fit. > > So back to the question, I'd like to understand your use case (and > maybe hear from the other drm folks if that is common): > > - you allocate pages from shmem (why shmem, btw? if this is done by > other drm drivers how do they guarantee addressability without an > iommu?) shmem for swappable pages. I don't unpin and let things get swapped out yet, but I'm told it starts to become important when you have 50 browser tabs open ;-) There are some display-only drm drivers with no IOMMU, which use CMA rather than shmem. Basically every real GPU has some form of MMU or IOMMU for memory protection. (The couple exceptions do expensive kernel side cmdstream validation.) > - then the memory is either mapped to userspace or vmapped (or even > both, althrough the lack of aliasing you mentioned would speak > against it) as writecombine (aka arm v6+ normal uncached). Does > the mapping live on until the memory is freed? (side note, *most* of the drm/msm supported devices are armv8, the exceptions are 8060 and 8064 which are armv7.. I don't think drm/msm will ever have to deal w/ armv6) Userspace keeps the userspace mmap around opportunistically (once it is mmap'd, not all buffers will be accessed from CPU). In fact there is a userspace buffer cache, where we try to re-use buffers that are already allocated and possibly mmap'd, since allocation and setting up mmap is expensive. (There is an MADVISE ioctl so userspace can tell kernel about buffers in the cache, which are available to be purged by shrinker.. if a buffer is purged, it's userspace mmap is torn down... along with it's IOMMU map, ofc) > - as you mention swapping - how do you guarantee there are no > aliases in the kernel direct mapping after the page has been swapped > in? Before unpinning and allowing pages to be swapped out, CPU and IOMMU maps would be torn down. (I don't think we could unpin buffers w/ a vmap, but those are just a drop in the bucket.) Currently, the kernel always knows buffers associated w/ a submit (job) queued to the GPU, so it could bring pages back in and re-store iommu map.. the fault handler can be used to bring things back in for CPU access. (For more free-form HMM style access to userspace memory, we'd need to be able to sleep in IOMMU fault handler before the IOMMU resumes translation.) As far as cache is concerned, it would be basically the same as newly allocated pages, ie. need to wb+inv the new pages. > - then the memory is potentially mapped to the iommu. Is it using > a long-living mapping, or does get unmapped/remapped repeatedly? Similar to CPU maps, we keep the IOMMU map around as long as possible. (Side note, I was thinking batching unmaps could be useful to reduce IOMMU TLB inv overhead.. usually when we are freeing a buffer, we are freeing multiple, so we could unmap them all, then TLB inv, then free pages.) BR, -R

1 0

[PATCH 0/5] drm-misc-next: Revert patches missing reviews
by Sean Paul 07 Aug '19

07 Aug '19

From: Sean Paul <seanpaul(a)chromium.org> Hellooo, This has been covered ad nauseam on the m-l and irc, but for the record: Reviews are a mandatory requirement for patches in drm-misc-next, it's what keeps us all honest in the committer model. The most recent drm-misc-next pull included a handful of patches that were missing reviews. There was absolutely zero nefarious intent, but rules are rules, so we're reverting them. I feel confident re-applying all of these with my SoB, so I plan on doing that in the same push. Thanks to everyone for handling this so well, seriously awesome that we can stay constructive as a community \o/. Lastly, this will be caught by our tooling in the future so this should be the last time this happens (on accident). Sean Sean Paul (5): Revert "Revert "drm/gem: Rename drm_gem_dumb_map_offset() to drm_gem_map_offset()"" Revert "Revert "drm/panfrost: Use drm_gem_map_offset()"" Revert "drm/vgem: drop DRM_AUTH usage from the driver" Revert "drm/msm: drop DRM_AUTH usage from the driver" Revert "drm/nouveau: remove open-coded drm_invalid_op()" drivers/gpu/drm/drm_dumb_buffers.c | 4 ++-- drivers/gpu/drm/drm_gem.c | 10 +++++++--- drivers/gpu/drm/exynos/exynos_drm_gem.c | 3 +-- drivers/gpu/drm/msm/msm_drv.c | 22 +++++++++++----------- drivers/gpu/drm/nouveau/nouveau_abi16.c | 6 ++++++ drivers/gpu/drm/nouveau/nouveau_abi16.h | 1 + drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++-------------- drivers/gpu/drm/vgem/vgem_drv.c | 4 ++-- include/drm/drm_gem.h | 4 ++-- 10 files changed, 35 insertions(+), 37 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS

3 8

Re: drm pull for v5.3-rc1
by Matthew Wilcox 07 Aug '19

07 Aug '19

On Tue, Aug 06, 2019 at 11:40:00PM -0700, Christoph Hellwig wrote: > On Tue, Aug 06, 2019 at 12:09:38PM -0700, Matthew Wilcox wrote: > > Has anyone looked at turning the interface inside-out? ie something like: > > > > struct mm_walk_state state = { .mm = mm, .start = start, .end = end, }; > > > > for_each_page_range(&state, page) { > > ... do something with page ... > > } > > > > with appropriate macrology along the lines of: > > > > #define for_each_page_range(state, page) \ > > while ((page = page_range_walk_next(state))) > > > > Then you don't need to package anything up into structs that are shared > > between the caller and the iterated function. > > I'm not an all that huge fan of super magic macro loops. But in this > case I don't see how it could even work, as we get special callbacks > for huge pages and holes, and people are trying to add a few more ops > as well. We could have bits in the mm_walk_state which indicate what things to return and what things to skip. We could (and probably should) also use different iterator names if people actually want to iterate different things. eg for_each_pte_range(&state, pte) as well as for_each_page_range().

2 4

[PATCH] drm/nouveau/nvif/mmu: Use struct_size() helper
by Gustavo A. R. Silva 07 Aug '19

07 Aug '19

One of the more common cases of allocation size calculations is finding the size of a structure that has a zero-sized array at the end, along with memory for some number of elements for that array. For example: struct nvif_mmu_kind_v0 { ... __u8 data[]; }; Make use of the struct_size() helper instead of an open-coded version in order to avoid any potential type mistakes. So, replace the following form: sizeof(*kind) + sizeof(*kind->data) * mmu->kind_nr with: struct_size(kind, data, mmu->kind_nr) This code was detected with the help of Coccinelle. Signed-off-by: Gustavo A. R. Silva <gustavo(a)embeddedor.com> --- drivers/gpu/drm/nouveau/nvif/mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/nvif/mmu.c b/drivers/gpu/drm/nouveau/nvif/mmu.c index ae08a1ca8044..5641bda2046d 100644 --- a/drivers/gpu/drm/nouveau/nvif/mmu.c +++ b/drivers/gpu/drm/nouveau/nvif/mmu.c @@ -110,7 +110,7 @@ nvif_mmu_init(struct nvif_object *parent, s32 oclass, struct nvif_mmu *mmu) if (mmu->kind_nr) { struct nvif_mmu_kind_v0 *kind; - u32 argc = sizeof(*kind) + sizeof(*kind->data) * mmu->kind_nr; + size_t argc = struct_size(kind, data, mmu->kind_nr); if (ret = -ENOMEM, !(kind = kmalloc(argc, GFP_KERNEL))) goto done; -- 2.22.0

1 0

[PATCH] drm/panfrost: Add "compute shader only" hint
by Alyssa Rosenzweig 07 Aug '19

07 Aug '19

Midgard contains two job slots capable of compute jobs, JS1 and JS2. As an optimization, it is preferable to schedule compute-only workloads to JS2, although compute jobs are functionally able to be scheduled to JS1 (barring an obscure errata). Accordingly, we reserve a compute-only hint in the UABI to allow future compute-equipped userspace and future compute-optimized kernelspace to hint towards JS2. At the moment, the hint is ignored, but this is harmless. I have verified compute jobs can be successfully submitted and executed with an appropriate userspace against a 5.1 kernel without this hint. Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig(a)collabora.com> --- drivers/gpu/drm/panfrost/TODO | 3 --- drivers/gpu/drm/panfrost/panfrost_job.c | 4 +++- include/uapi/drm/panfrost_drm.h | 8 ++++++++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/panfrost/TODO b/drivers/gpu/drm/panfrost/TODO index c2e44add3..8c367a5a6 100644 --- a/drivers/gpu/drm/panfrost/TODO +++ b/drivers/gpu/drm/panfrost/TODO @@ -20,8 +20,5 @@ - Support for madvise and a shrinker. -- Compute job support. So called 'compute only' jobs need to be plumbed up to - userspace. - - Performance counter support. (Boris) diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 9bb9260d9..3e1385a3b 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -108,7 +108,9 @@ static int panfrost_job_get_slot(struct panfrost_job *job) if (job->requirements & PANFROST_JD_REQ_FS) return 0; -/* Not exposed to userspace yet */ + /* Compute jobs can run on JS1, so compute-only jobs can run with this + * simple implementations (useful for backwards compatibility). As an + * optimization, we will eventually want to schedule to JS2. */ #if 0 if (job->requirements & PANFROST_JD_REQ_ONLY_COMPUTE) { if ((job->requirements & PANFROST_JD_REQ_CORE_GRP_MASK) && diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h index b5d370638..25acde34b 100644 --- a/include/uapi/drm/panfrost_drm.h +++ b/include/uapi/drm/panfrost_drm.h @@ -38,6 +38,14 @@ extern "C" { #define DRM_IOCTL_PANFROST_PERFCNT_DUMP DRM_IOW(DRM_COMMAND_BASE + DRM_PANFROST_PERFCNT_DUMP, struct drm_panfrost_perfcnt_dump) #define PANFROST_JD_REQ_FS (1 << 0) + +/* Optional (mandatory for T600 r0p0 15dev0 due to errata #8987) hint to the + * kernel that the commands only contain JOB_TYPE_COMPUTE jobs, without + * vertex/tiler/fragment jobs. If present, the kernel may use this as an + * optimization, but it is not strictly necessary. */ + +#define PANFROST_JD_REQ_ONLY_COMPUTE (1 << 1) + /** * struct drm_panfrost_submit - ioctl argument for submitting commands to the 3D * engine. -- 2.20.1

3 6

[GIT PULL] drm/tegra: Fixes for v5.3-rc4
by Thierry Reding 07 Aug '19

07 Aug '19

Hi Dave, The following changes since commit 5f9e832c137075045d15cd6899ab0505cfb2ca4b: Linus 5.3-rc1 (2019-07-21 14:05:38 -0700) are available in the Git repository at: git://anongit.freedesktop.org/tegra/linux tags/drm/tegra/for-5.3-rc4 for you to fetch changes up to 2a6fc3cb5cb68597f1072bfeef28d2ca02310220: drm/tegra: Fix gpiod_get_from_of_node() regression (2019-07-25 15:23:26 +0200) Thanks, Thierry ---------------------------------------------------------------- drm/tegra: Fixes for v5.3-rc4 This contains a single fix for a regression introduced by a combination of a GPIO and a drm/tegra patch merged in v5.3-rc1. ---------------------------------------------------------------- Dmitry Osipenko (1): drm/tegra: Fix gpiod_get_from_of_node() regression drivers/gpu/drm/tegra/output.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)

1 0

[PATCH 0/2] drm/syncobj: add syncobj sideband payload for threaded submission
by Lionel Landwerlin 07 Aug '19

07 Aug '19

Hi, The goal of this feature is to solve an issue that was seen in our testing. After discussing on [1] I thought we could solve this problem with stalling the application thread after each vkQueueSubmit() call where a binary semaphore is signaled but I don't think it's a good option because of performance impacts on the application. Unfortunately there isn't a good way to reproduce this problem 100% because it essentially exposes a race between the application thread and the submission thread. I've uploaded tests in the Khronos repository to try to expose the issue. Thanks, [1] : https://lists.freedesktop.org/archives/dri-devel/2019-August/229188.html Lionel Landwerlin (2): drm/syncobj: add sideband payload drm/syncobj: add submit point query capability drivers/gpu/drm/drm_syncobj.c | 132 ++++++++++++++++++++++------------ include/drm/drm_syncobj.h | 9 +++ include/uapi/drm/drm.h | 5 +- 3 files changed, 100 insertions(+), 46 deletions(-) -- 2.23.0.rc1

2 4

[PATCH] drm/i915: Fix some NULL vs IS_ERR() conditions
by Dan Carpenter 07 Aug '19

07 Aug '19

There were several places which check for NULL when they should have been checking for IS_ERR(). Fixes: d8af05ff38ae ("drm/i915: Allow sharing the idle-barrier from other kernel requests") Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> --- drivers/gpu/drm/i915/gt/selftest_context.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/selftest_context.c b/drivers/gpu/drm/i915/gt/selftest_context.c index d39b5594cb02..6e7e9a6fd235 100644 --- a/drivers/gpu/drm/i915/gt/selftest_context.c +++ b/drivers/gpu/drm/i915/gt/selftest_context.c @@ -86,8 +86,8 @@ static int __live_active_context(struct intel_engine_cs *engine, } ce = intel_context_create(fixme, engine); - if (!ce) - return -ENOMEM; + if (IS_ERR(ce)) + return PTR_ERR(ce); for (pass = 0; pass <= 2; pass++) { struct i915_request *rq; @@ -161,8 +161,8 @@ static int live_active_context(void *arg) mutex_lock(&gt->i915->drm.struct_mutex); fixme = live_context(gt->i915, file); - if (!fixme) { - err = -ENOMEM; + if (IS_ERR(fixme)) { + err = PTR_ERR(fixme); goto unlock; } @@ -226,12 +226,12 @@ static int __live_remote_context(struct intel_engine_cs *engine, */ remote = intel_context_create(fixme, engine); - if (!remote) - return -ENOMEM; + if (IS_ERR(remote)) + return PTR_ERR(remote); local = intel_context_create(fixme, engine); - if (!local) { - err = -ENOMEM; + if (IS_ERR(local)) { + err = PTR_ERR(local); goto err_remote; } @@ -274,8 +274,8 @@ static int live_remote_context(void *arg) mutex_lock(&gt->i915->drm.struct_mutex); fixme = live_context(gt->i915, file); - if (!fixme) { - err = -ENOMEM; + if (IS_ERR(fixme)) { + err = PTR_ERR(fixme); goto unlock; } -- 2.20.1

2 2

[PATCH 1/8] dma-buf: fix busy wait for new shared fences
by Christian König 07 Aug '19

07 Aug '19

When reservation_object_add_shared_fence is replacing an old fence with a new one we should not drop the old one before the new one is in place. Otherwise other cores can busy wait for the new one to appear. Signed-off-by: Christian König <christian.koenig(a)amd.com> --- drivers/dma-buf/reservation.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index c71b85c8c159..d59207ca72d2 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -196,6 +196,7 @@ void reservation_object_add_shared_fence(struct reservation_object *obj, struct dma_fence *fence) { struct reservation_object_list *fobj; + struct dma_fence *old; unsigned int i, count; dma_fence_get(fence); @@ -209,18 +210,16 @@ void reservation_object_add_shared_fence(struct reservation_object *obj, write_seqcount_begin(&obj->seq); for (i = 0; i < count; ++i) { - struct dma_fence *old_fence; - old_fence = rcu_dereference_protected(fobj->shared[i], - reservation_object_held(obj)); - if (old_fence->context == fence->context || - dma_fence_is_signaled(old_fence)) { - dma_fence_put(old_fence); + old = rcu_dereference_protected(fobj->shared[i], + reservation_object_held(obj)); + if (old->context == fence->context || + dma_fence_is_signaled(old)) goto replace; - } } BUG_ON(fobj->shared_count >= fobj->shared_max); + old = NULL; count++; replace: @@ -230,6 +229,7 @@ void reservation_object_add_shared_fence(struct reservation_object *obj, write_seqcount_end(&obj->seq); preempt_enable(); + dma_fence_put(old); } EXPORT_SYMBOL(reservation_object_add_shared_fence); -- 2.17.1

3 20

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel August 2019