dri-devel August 2019

dri-devel@lists.freedesktop.org

318 participants
2338 discussions

Re: drm pull for v5.3-rc1
by Linus Torvalds 07 Aug '19

07 Aug '19

On Tue, Aug 6, 2019 at 11:40 PM Christoph Hellwig <hch(a)infradead.org> wrote: > > 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. Yeah, in this case we definitely don't want to make some magic loop walker. Loops are certainly simpler than callbacks for most cases (and often faster because you don't … [View More]

1 0

[PATCH] drm/msm: Make DRM_MSM default to 'm'
by Jordan Crouse 07 Aug '19

07 Aug '19

Most use cases for DRM_MSM will prefer to build both DRM and MSM_DRM as modules but there are some cases where DRM might be built in for whatever reason and in those situations it is preferable to still keep MSM as a module by default and let the user decide if they _really_ want to build it in. Additionally select QCOM_COMMAND_DB for ARCH_QCOM targets to make sure it doesn't get missed when we need it for a6xx tarets. Signed-off-by: Jordan Crouse <jcrouse(a)codeaurora.org> --- … [View More]

3 4

[PATCH] drm/msm/dsi: Fix return value check for clk_get_parent
by Sean Paul 07 Aug '19

07 Aug '19

From: Sean Paul <seanpaul(a)chromium.org> clk_get_parent returns an error pointer upon failure, not NULL. So the checks as they exist won't catch a failure. This patch changes the checks and the return values to properly handle an error pointer. Fixes: c4d8cfe516dc ("drm/msm/dsi: add implementation for helper functions") Cc: Sibi Sankar <sibis(a)codeaurora.org> Cc: Sean Paul <seanpaul(a)chromium.org> Cc: Rob Clark <robdclark(a)chromium.org> Cc: <stable(a)vger.kernel.… [View More]

1 0

Re: [PATCH 04/15] mm: remove the pgmap field from struct hmm_vma_walk
by Jason Gunthorpe 07 Aug '19

07 Aug '19

On Tue, Aug 06, 2019 at 07:05:42PM +0300, Christoph Hellwig wrote: > There is only a single place where the pgmap is passed over a function > call, so replace it with local variables in the places where we deal > with the pgmap. > > Signed-off-by: Christoph Hellwig <hch(a)lst.de> > mm/hmm.c | 62 ++++++++++++++++++++++++-------------------------------- > 1 file changed, 27 insertions(+), 35 deletions(-) > > diff --git a/mm/hmm.c b/mm/hmm.c > index … [View More]9a908902e4cc..d66fa29b42e0 100644 > +++ b/mm/hmm.c > @@ -278,7 +278,6 @@ EXPORT_SYMBOL(hmm_mirror_unregister); > > struct hmm_vma_walk { > struct hmm_range *range; > - struct dev_pagemap *pgmap; > unsigned long last; > unsigned int flags; > }; > @@ -475,6 +474,7 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk, > #ifdef CONFIG_TRANSPARENT_HUGEPAGE > struct hmm_vma_walk *hmm_vma_walk = walk->private; > struct hmm_range *range = hmm_vma_walk->range; > + struct dev_pagemap *pgmap = NULL; > unsigned long pfn, npages, i; > bool fault, write_fault; > uint64_t cpu_flags; > @@ -490,17 +490,14 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk, > pfn = pmd_pfn(pmd) + pte_index(addr); > for (i = 0; addr < end; addr += PAGE_SIZE, i++, pfn++) { > if (pmd_devmap(pmd)) { > - hmm_vma_walk->pgmap = get_dev_pagemap(pfn, > - hmm_vma_walk->pgmap); > - if (unlikely(!hmm_vma_walk->pgmap)) > + pgmap = get_dev_pagemap(pfn, pgmap); > + if (unlikely(!pgmap)) > return -EBUSY; Unrelated to this patch, but what is the point of getting checking that the pgmap exists for the page and then immediately releasing it? This code has this pattern in several places. It feels racy > } > pfns[i] = hmm_device_entry_from_pfn(range, pfn) | cpu_flags; > } > - if (hmm_vma_walk->pgmap) { > - put_dev_pagemap(hmm_vma_walk->pgmap); > - hmm_vma_walk->pgmap = NULL; Putting the value in the hmm_vma_walk would have made some sense to me if the pgmap was not set to NULL all over the place. Then the most xa_loads would be eliminated, as I would expect the pgmap tends to be mostly uniform for these use cases. Is there some reason the pgmap ref can't be held across faulting/sleeping? ie like below. Anyhow, I looked over this pretty carefully and the change looks functionally OK, I just don't know why the code is like this in the first place. Reviewed-by: Jason Gunthorpe <jgg(a)mellanox.com> diff --git a/mm/hmm.c b/mm/hmm.c index 9a908902e4cc38..4e30128c23a505 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -497,10 +497,6 @@ static int hmm_vma_handle_pmd(struct mm_walk *walk, } pfns[i] = hmm_device_entry_from_pfn(range, pfn) | cpu_flags; } - if (hmm_vma_walk->pgmap) { - put_dev_pagemap(hmm_vma_walk->pgmap); - hmm_vma_walk->pgmap = NULL; - } hmm_vma_walk->last = end; return 0; #else @@ -604,10 +600,6 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr, return 0; fault: - if (hmm_vma_walk->pgmap) { - put_dev_pagemap(hmm_vma_walk->pgmap); - hmm_vma_walk->pgmap = NULL; - } pte_unmap(ptep); /* Fault any virtual address we were asked to fault */ return hmm_vma_walk_hole_(addr, end, fault, write_fault, walk); @@ -690,16 +682,6 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp, return r; } } - if (hmm_vma_walk->pgmap) { - /* - * We do put_dev_pagemap() here and not in hmm_vma_handle_pte() - * so that we can leverage get_dev_pagemap() optimization which - * will not re-take a reference on a pgmap if we already have - * one. - */ - put_dev_pagemap(hmm_vma_walk->pgmap); - hmm_vma_walk->pgmap = NULL; - } pte_unmap(ptep - 1); hmm_vma_walk->last = addr; @@ -751,10 +733,6 @@ static int hmm_vma_walk_pud(pud_t *pudp, pfns[i] = hmm_device_entry_from_pfn(range, pfn) | cpu_flags; } - if (hmm_vma_walk->pgmap) { - put_dev_pagemap(hmm_vma_walk->pgmap); - hmm_vma_walk->pgmap = NULL; - } hmm_vma_walk->last = end; return 0; } @@ -1026,6 +1004,14 @@ long hmm_range_fault(struct hmm_range *range, unsigned int flags) /* Keep trying while the range is valid. */ } while (ret == -EBUSY && range->valid); + /* + * We do put_dev_pagemap() here so that we can leverage + * get_dev_pagemap() optimization which will not re-take a + * reference on a pgmap if we already have one. + */ + if (hmm_vma_walk->pgmap) + put_dev_pagemap(hmm_vma_walk->pgmap); + if (ret) { unsigned long i; [View Less]

2 1

[pull] amdgpu, amdkfd drm-fixes-5.3
by Alex Deucher 07 Aug '19

07 Aug '19

Hi Dave, Daniel, Fixes for 5.3. Nothing too major bug-wise. I'm reverting the kfd GWS ioctl that was added this cycle. After working with it for a while the kfd team decided it wasn't quite right. I should have been stricter with it in the beginning. Revert it. The following changes since commit 9c8c9c7cdb4c8fb48a2bc70f41a07920f761d2cd: Merge tag 'exynos-drm-fixes-for-v5.3-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos into drm-fixes (2019-08-02 17:10:17 +0200)… [View More] are available in the Git repository at: git://people.freedesktop.org/~agd5f/linux tags/drm-fixes-5.3-2019-08-07 for you to fetch changes up to 4b3e30ed3ec7864e798403a63ff2e96bd0c19ab0: Revert "drm/amdkfd: New IOCTL to allocate queue GWS" (2019-08-07 10:21:38 -0500) ---------------------------------------------------------------- drm-fixes-5.3-2019-08-07: amdgpu: - Fixes VCN to handle the latest navi10 firmware - Fixes for fan control on navi10 - Properly handle SMU metrics table on navi10 - Fix a resume regression on Stoney amdkfd: - Revert new GWS ioctl. It's not ready. ---------------------------------------------------------------- Alex Deucher (1): Revert "drm/amdkfd: New IOCTL to allocate queue GWS" Evan Quan (1): drm/amd/powerplay: correct navi10 vcn powergate Kevin Wang (1): drm/amd/powerplay: honor hw limit on fetching metrics data for navi10 Likun Gao (1): drm/amdgpu: pin the csb buffer on hw init for gfx v8 Marek Olšák (1): Revert "drm/amdgpu: fix transform feedback GDS hang on gfx10 (v2)" Matt Coffin (1): drm/amd/powerplay: Allow changing of fan_control in smu_v11_0 Thong Thai (2): drm/amd/amdgpu/vcn_v2_0: Mark RB commands as KMD commands drm/amd/amdgpu/vcn_v2_0: Move VCN 2.0 specific dec ring test to vcn_v2_0 drivers/gpu/drm/amd/amdgpu/amdgpu_gds.h | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 + drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 12 +--- drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 40 +++++++++++++ drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 44 +++++++++++--- drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 28 --------- drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 4 +- drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 + drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 79 +++++++++++++++++--------- drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 2 +- include/uapi/linux/kfd_ioctl.h | 20 +------ 11 files changed, 138 insertions(+), 94 deletions(-) [View Less]

1 0

Re: hmm cleanups, v2
by Jason Gunthorpe 07 Aug '19

07 Aug '19

On Tue, Aug 06, 2019 at 07:05:38PM +0300, Christoph Hellwig wrote: > > Hi Jérôme, Ben, Felix and Jason, > > below is a series against the hmm tree which cleans up various minor > bits and allows HMM_MIRROR to be built on all architectures. > > Diffstat: > > 11 files changed, 94 insertions(+), 210 deletions(-) > > A git tree is also available at: > > git://git.infradead.org/users/hch/misc.git hmm-cleanups.2 > > Gitweb: > > … [View More]

1 0

Re: [PATCH 07/15] mm: remove the page_shift member from struct hmm_range
by Jason Gunthorpe 07 Aug '19

07 Aug '19

On Tue, Aug 06, 2019 at 07:05:45PM +0300, Christoph Hellwig wrote: > All users pass PAGE_SIZE here, and if we wanted to support single > entries for huge pages we should really just add a HMM_FAULT_HUGEPAGE > flag instead that uses the huge page size instead of having the > caller calculate that size once, just for the hmm code to verify it. > > Signed-off-by: Christoph Hellwig <hch(a)lst.de> > Acked-by: Felix Kuehling <Felix.Kuehling(a)amd.com> > --- > … [View More]drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 - > drivers/gpu/drm/nouveau/nouveau_svm.c | 1 - > include/linux/hmm.h | 22 ------------- > mm/hmm.c | 42 ++++++------------------- > 4 files changed, 9 insertions(+), 57 deletions(-) Having looked at ODP more closley this doesn't seem to match what it needs anyhow. It can keep using its checking algorithm > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > index 71d6e7087b0b..8bf79288c4e2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c > @@ -818,7 +818,6 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages) > 0 : range->flags[HMM_PFN_WRITE]; > range->pfn_flags_mask = 0; > range->pfns = pfns; > - range->page_shift = PAGE_SHIFT; > range->start = start; > range->end = start + ttm->num_pages * PAGE_SIZE; > > diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c > index 41fad4719ac6..668d4bd0c118 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_svm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c > @@ -680,7 +680,6 @@ nouveau_svm_fault(struct nvif_notify *notify) > args.i.p.addr + args.i.p.size, fn - fi); > > /* Have HMM fault pages within the fault window to the GPU. */ > - range.page_shift = PAGE_SHIFT; > range.start = args.i.p.addr; > range.end = args.i.p.addr + args.i.p.size; > range.pfns = args.phys; > diff --git a/include/linux/hmm.h b/include/linux/hmm.h > index c5b51376b453..51e18fbb8953 100644 > --- a/include/linux/hmm.h > +++ b/include/linux/hmm.h > @@ -158,7 +158,6 @@ enum hmm_pfn_value_e { > * @values: pfn value for some special case (none, special, error, ...) > * @default_flags: default flags for the range (write, read, ... see hmm doc) > * @pfn_flags_mask: allows to mask pfn flags so that only default_flags matter > - * @page_shift: device virtual address shift value (should be >= PAGE_SHIFT) > * @pfn_shifts: pfn shift value (should be <= PAGE_SHIFT) > * @valid: pfns array did not change since it has been fill by an HMM function > */ > @@ -172,31 +171,10 @@ struct hmm_range { > const uint64_t *values; > uint64_t default_flags; > uint64_t pfn_flags_mask; > - uint8_t page_shift; > uint8_t pfn_shift; > bool valid; > }; > > -/* > - * hmm_range_page_shift() - return the page shift for the range > - * @range: range being queried > - * Return: page shift (page size = 1 << page shift) for the range > - */ > -static inline unsigned hmm_range_page_shift(const struct hmm_range *range) > -{ > - return range->page_shift; > -} > - > -/* > - * hmm_range_page_size() - return the page size for the range > - * @range: range being queried > - * Return: page size for the range in bytes > - */ > -static inline unsigned long hmm_range_page_size(const struct hmm_range *range) > -{ > - return 1UL << hmm_range_page_shift(range); > -} > - > /* > * hmm_range_wait_until_valid() - wait for range to be valid > * @range: range affected by invalidation to wait on > diff --git a/mm/hmm.c b/mm/hmm.c > index 926735a3aef9..f26d6abc4ed2 100644 > --- a/mm/hmm.c > +++ b/mm/hmm.c > @@ -344,13 +344,12 @@ static int hmm_vma_walk_hole_(unsigned long addr, unsigned long end, > struct hmm_vma_walk *hmm_vma_walk = walk->private; > struct hmm_range *range = hmm_vma_walk->range; > uint64_t *pfns = range->pfns; > - unsigned long i, page_size; > + unsigned long i; > > hmm_vma_walk->last = addr; > - page_size = hmm_range_page_size(range); > - i = (addr - range->start) >> range->page_shift; > + i = (addr - range->start) >> PAGE_SHIFT; > > - for (; addr < end; addr += page_size, i++) { > + for (; addr < end; addr += PAGE_SIZE, i++) { > pfns[i] = range->values[HMM_PFN_NONE]; > if (fault || write_fault) { > int ret; > @@ -772,7 +771,7 @@ static int hmm_vma_walk_hugetlb_entry(pte_t *pte, unsigned long hmask, > struct mm_walk *walk) > { > #ifdef CONFIG_HUGETLB_PAGE > - unsigned long addr = start, i, pfn, mask, size, pfn_inc; > + unsigned long addr = start, i, pfn, mask; > struct hmm_vma_walk *hmm_vma_walk = walk->private; > struct hmm_range *range = hmm_vma_walk->range; > struct vm_area_struct *vma = walk->vma; > @@ -783,24 +782,12 @@ static int hmm_vma_walk_hugetlb_entry(pte_t *pte, unsigned long hmask, > pte_t entry; > int ret = 0; > > - size = huge_page_size(h); > - mask = size - 1; > - if (range->page_shift != PAGE_SHIFT) { > - /* Make sure we are looking at a full page. */ > - if (start & mask) > - return -EINVAL; > - if (end < (start + size)) > - return -EINVAL; > - pfn_inc = size >> PAGE_SHIFT; > - } else { > - pfn_inc = 1; > - size = PAGE_SIZE; > - } > + mask = huge_page_size(h) - 1; > > ptl = huge_pte_lock(hstate_vma(vma), walk->mm, pte); > entry = huge_ptep_get(pte); > > - i = (start - range->start) >> range->page_shift; > + i = (start - range->start) >> PAGE_SHIFT; > orig_pfn = range->pfns[i]; > range->pfns[i] = range->values[HMM_PFN_NONE]; > cpu_flags = pte_to_hmm_pfn_flags(range, entry); > @@ -812,8 +799,8 @@ static int hmm_vma_walk_hugetlb_entry(pte_t *pte, unsigned long hmask, > goto unlock; > } > > - pfn = pte_pfn(entry) + ((start & mask) >> range->page_shift); > - for (; addr < end; addr += size, i++, pfn += pfn_inc) > + pfn = pte_pfn(entry) + ((start & mask) >> PAGE_SHIFT); > + for (; addr < end; addr += PAGE_SIZE, i++, pfn++) > range->pfns[i] = hmm_device_entry_from_pfn(range, pfn) | > cpu_flags; > hmm_vma_walk->last = end; > @@ -850,14 +837,13 @@ static void hmm_pfns_clear(struct hmm_range *range, > */ > int hmm_range_register(struct hmm_range *range, struct hmm_mirror *mirror) > { > - unsigned long mask = ((1UL << range->page_shift) - 1UL); > struct hmm *hmm = mirror->hmm; > unsigned long flags; > > range->valid = false; > range->hmm = NULL; > > - if ((range->start & mask) || (range->end & mask)) > + if ((range->start & (PAGE_SIZE - 1)) || (range->end & (PAGE_SIZE - 1))) > return -EINVAL; PAGE_SIZE-1 == PAGE_MASK ? If yes I can fix it Reviewed-by: Jason Gunthorpe <jgg(a)mellanox.com> Jason [View Less]

1 0

Re: next/master boot: 263 boots: 11 failed, 186 passed with 64 offline, 1 untried/unknown, 1 conflict (next-20190802)
by Mark Brown 07 Aug '19

07 Aug '19

On Fri, Aug 02, 2019 at 05:13:30AM -0700, kernelci.org bot wrote: Today's -next still fails to boot on CM-QS600 with qcom_defconfig: > qcom_defconfig: > gcc-8: > qcom-apq8064-cm-qs600: 1 failed lab This has been going on since June. It crashes initializing the GPU: [ 4.261135] adreno 4300000.adreno-3xx: 4300000.adreno-3xx supply vddcx not found, using dummy regulator [ 4.270254] msm 5100000.mdp: [drm:msm_gpu_init] A320: using IOMMU [ 4.280025] 8… [View More]

1 0

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 1:48 AM Christoph Hellwig <hch(a)lst.de> wrote: > > This goes in the wrong direction. drm_cflush_* are a bad API we need to > get rid of, not add use of it. The reason for that is two-fold: > > a) it doesn't address how cache maintaince actually works in most > platforms. When talking about a cache we three fundamental operations: > > 1) write back - this writes the content of the cache back to the > backing … [View More]memory > 2) invalidate - this remove the content of the cache > 3) write back + invalidate - do both of the above 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. > b) which of the above operation you use when depends on a couple of > factors of what you want to do with the range you do the cache > maintainance operations > > Take a look at the comment in arch/arc/mm/dma.c around line 30 that > explains how this applies to buffer ownership management. Note that > "for device" applies to "for userspace" in the same way, just that > userspace then also needs to follow this protocol. So the whole idea > that random driver code calls random low-level cache maintainance > operations (and use the non-specific term flush to make it all more > confusing) is a bad idea. Fortunately enough we have really good > arch helpers for all non-coherent architectures (this excludes the > magic i915 won't be covered by that, but that is a separate issue > to be addressed later, and the fact that while arm32 did grew them > very recently and doesn't expose them for all configs, which is easily > fixable if needed) with arch_sync_dma_for_device and > arch_sync_dma_for_cpu. So what we need is to figure out where we > have valid cases for buffer ownership transfer outside the DMA > API, and build proper wrappers around the above function for that. > My guess is it should probably be build to go with the iommu API > as that is the only other way to map memory for DMA access, but > if you have a better idea I'd be open to discussion. 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. Exposing the arch_sync_* API and using that directly (bypassing drm_cflush_*) actually seems pretty reasonable and pragmatic. I did have one doubt, as phys_to_virt() is only valid for kernel direct mapped memory (AFAIU), what happens for pages that are not in kernel linear map? Maybe it is ok to ignore those pages, since they won't have an aliased mapping? BR, -R [View Less]

2 6

Re: [PATCH 09/15] mm: don't abuse pte_index() in hmm_vma_handle_pmd
by Jason Gunthorpe 07 Aug '19

07 Aug '19

On Tue, Aug 06, 2019 at 07:05:47PM +0300, Christoph Hellwig wrote: > pte_index is an internal arch helper in various architectures, > without consistent semantics. Open code that calculation of a PMD > index based on the virtual address instead. > > Signed-off-by: Christoph Hellwig <hch(a)lst.de> > --- > mm/hmm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) There sure are a lot of different ways to express this, but this one looks OK to me, at least … [View More]

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel August 2019