Re: [PATCH v3 0/9] video: screen_info cleanups
On Mon, Oct 09, 2023 at 11:18:36PM +0200, Arnd Bergmann wrote: > From: Arnd Bergmann > > v3 changelog > > No real changes, just rebased for context changes, and picked up the Acks. > > This now conflicts with the ia64 removal and introduces one new dependency > on IA64, but that is harmless and trivial to deal with later. > > Link: https://lore.kernel.org/lkml/20230719123944.3438363-1-a...@kernel.org/ > --- > v2 changelog > > I refreshed the first four patches that I sent before with very minor > updates, and then added some more to further disaggregate the use > of screen_info: > > - I found that powerpc wasn't using vga16fb any more > > - vgacon can be almost entirely separated from the global >screen_info, except on x86 > > - similarly, the EFI framebuffer initialization can be >kept separate, except on x86. > > I did extensive build testing on arm/arm64/x86 and the normal built bot > testing for the other architectures. > > Which tree should this get merged through? I guess if no one else volunteers (Greg maybe?) I can stuff this into drm-misc ... -Sima > > Link: https://lore.kernel.org/lkml/20230707095415.1449376-1-a...@kernel.org/ > > > Arnd Bergmann (9): > vgacon: rework Kconfig dependencies > vgacon: rework screen_info #ifdef checks > dummycon: limit Arm console size hack to footbridge > vgacon, arch/*: remove unused screen_info definitions > vgacon: remove screen_info dependency > vgacon: clean up global screen_info instances > vga16fb: drop powerpc support > hyperv: avoid dependency on screen_info > efi: move screen_info into efi init code > > arch/alpha/kernel/proto.h | 2 + > arch/alpha/kernel/setup.c | 8 +-- > arch/alpha/kernel/sys_sio.c | 8 ++- > arch/arm/include/asm/setup.h | 5 ++ > arch/arm/kernel/atags_parse.c | 20 +++--- > arch/arm/kernel/efi.c | 6 -- > arch/arm/kernel/setup.c | 11 +-- > arch/arm64/kernel/efi.c | 4 -- > arch/arm64/kernel/image-vars.h| 2 + > arch/csky/kernel/setup.c | 12 > arch/hexagon/kernel/Makefile | 2 - > arch/hexagon/kernel/screen_info.c | 3 - > arch/ia64/kernel/setup.c | 53 --- > arch/loongarch/kernel/efi.c | 3 +- > arch/loongarch/kernel/image-vars.h| 2 + > arch/loongarch/kernel/setup.c | 3 - > arch/mips/jazz/setup.c| 9 --- > arch/mips/kernel/setup.c | 11 --- > arch/mips/mti-malta/malta-setup.c | 4 +- > arch/mips/sibyte/swarm/setup.c| 26 --- > arch/mips/sni/setup.c | 18 ++--- > arch/nios2/kernel/setup.c | 5 -- > arch/powerpc/kernel/setup-common.c| 16 - > arch/riscv/kernel/image-vars.h| 2 + > arch/riscv/kernel/setup.c | 12 > arch/sh/kernel/setup.c| 5 -- > arch/sparc/kernel/setup_32.c | 13 > arch/sparc/kernel/setup_64.c | 13 > arch/x86/kernel/setup.c | 2 +- > arch/xtensa/kernel/setup.c| 12 > drivers/firmware/efi/efi-init.c | 14 +++- > drivers/firmware/efi/libstub/efi-stub-entry.c | 8 ++- > drivers/firmware/pcdp.c | 1 - > drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 7 +- > drivers/hv/vmbus_drv.c| 6 +- > drivers/video/console/Kconfig | 11 +-- > drivers/video/console/dummycon.c | 2 +- > drivers/video/console/vgacon.c| 68 +++ > drivers/video/fbdev/Kconfig | 2 +- > drivers/video/fbdev/hyperv_fb.c | 8 +-- > drivers/video/fbdev/vga16fb.c | 9 +-- > include/linux/console.h | 7 ++ > 42 files changed, 183 insertions(+), 252 deletions(-) > delete mode 100644 arch/hexagon/kernel/screen_info.c > > -- > 2.39.2 > > Cc: "David S. Miller" > Cc: "K. Y. Srinivasan" > Cc: Ard Biesheuvel > Cc: Borislav Petkov > Cc: Brian Cain > Cc: Catalin Marinas > Cc: Christophe Leroy > Cc: Daniel Vetter > Cc: Dave Hansen > Cc: David Airlie > Cc: Deepak Rawat > Cc: Dexuan Cui > Cc: Dinh Nguyen > Cc: Greg Kroah-Hartman > Cc: Guo Ren > Cc: Haiyang Zhang > Cc: Helge Deller > Cc: Huacai Chen > Cc: Ingo Molnar > Cc: Javier Martinez Cani
Re: [PATCH 01/18] fbdev: Prepare generic architecture helpers
On Wed, Apr 05, 2023 at 05:53:03PM +0200, Arnd Bergmann wrote:
> On Wed, Apr 5, 2023, at 17:05, Thomas Zimmermann wrote:
> > Generic implementations of fb_pgprotect() and fb_is_primary_device()
> > have been in the source code for a long time. Prepare the header file
> > to make use of them.
> >
> > Improve the code by using an inline function for fb_pgprotect() and
> > by removing include statements.
> >
> > Symbols are protected by preprocessor guards. Architectures that
> > provide a symbol need to define a preprocessor token of the same
> > name and value. Otherwise the header file will provide a generic
> > implementation. This pattern has been taken from .
> >
> > Signed-off-by: Thomas Zimmermann
>
> Moving this into generic code is good, but I'm not sure
> about the default for fb_pgprotect():
>
> > +
> > +#ifndef fb_pgprotect
> > +#define fb_pgprotect fb_pgprotect
> > +static inline void fb_pgprotect(struct file *file, struct vm_area_struct
> > *vma,
> > + unsigned long off)
> > +{ }
> > +#endif
>
> I think most architectures will want the version we have on
> arc, arm, arm64, loongarch, and sh already:
>
> static inline void fb_pgprotect(struct file *file, struct vm_area_struct *vma,
> unsigned long off)
> {
>vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> }
>
> so I'd suggest making that version the default, and treating the
> empty ones (m68knommu, sparc32) as architecture specific
> workarounds.
Yeah I was about to type the same suggestion :-)
-Daniel
> I see that sparc64 and parisc use pgprot_uncached here, but as
> they don't define a custom pgprot_writecombine, this ends up being
> the same, and they can use the above definition as well.
>
> mips defines pgprot_writecombine but uses pgprot_noncached
> in fb_pgprotect(), which is probably a mistake and should have
> been updated as part of commit 4b050ba7a66c ("MIPS: pgtable.h:
> Implement the pgprot_writecombine function for MIPS").
>
> Arnd
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH 00/19] Introduce __xchg, non-atomic xchg
On Thu, Dec 29, 2022 at 10:54:50AM +0100, Andrzej Hajda wrote: > Forgive me late response - Holidays, > > On 22.12.2022 18:21, Andrew Morton wrote: > > On Thu, 22 Dec 2022 12:46:16 +0100 Andrzej Hajda > > wrote: > > > > > Hi all, > > > > > > I hope there will be place for such tiny helper in kernel. > > > Quick cocci analyze shows there is probably few thousands places > > > where it could be useful. > > So to clarify, the intent here is a simple readability cleanup for > > existing open-coded exchange operations. > > And replace private helpers with common one, see the last patch - the > ultimate goal > would be to replace all occurrences of fetch_and_zero with __xchg. > > > The intent is *not* to > > identify existing xchg() sites which are unnecessarily atomic and to > > optimize them by using the non-atomic version. > > > > Have you considered the latter? > > If you mean some way of (semi-)automatic detection of such cases, then no. > Anyway this could be quite interesting challenge. My take is that unless there is very clear demand for this macro from outside of i915, it's not worth it. All that fetch_and_zero zero achieved is make i915 code a lot more confusing to read for people who don't know this thing. And it replaces 2 entirely standard lines of 0, every often clearing pointers in data structures where you really want the verbosity to have a reminder and thinking about the locking. Plus it smells way too much like the cmpxchg family of atomic functions, addig further to the locking confuion. Imo the right approach is to just open code this macro in i915 and then drop it. Again, unless enough people outside of i915 really really want this, and want to lift this to a kernel idiom. -Daniel > > > > > > I am not sure who is good person to review/ack such patches, > > I can take 'em. > > > > > so I've used my intuition to construct to/cc lists, sorry for mistakes. > > > This is the 2nd approach of the same idea, with comments addressed[0]. > > > > > > The helper is tiny and there are advices we can leave without it, so > > > I want to present few arguments why it would be good to have it: > > > > > > 1. Code readability/simplification/number of lines: > > > > > > Real example from drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c: > > > - previous_min_rate = evport->qos.min_rate; > > > - evport->qos.min_rate = min_rate; > > > + previous_min_rate = __xchg(evport->qos.min_rate, min_rate); > > > > > > For sure the code is more compact, and IMHO more readable. > > > > > > 2. Presence of similar helpers in other somehow related languages/libs: > > > > > > a) Rust[1]: 'replace' from std::mem module, there is also 'take' > > > helper (__xchg(&x, 0)), which is the same as private helper in > > > i915 - fetch_and_zero, see latest patch. > > > b) C++ [2]: 'exchange' from utility header. > > > > > > If the idea is OK there are still 2 qestions to answer: > > > > > > 1. Name of the helper, __xchg follows kernel conventions, > > > but for me Rust names are also OK. > > I like replace(), or, shockingly, exchange(). > > > > But... Can we simply make swap() return the previous value? > > > > previous_min_rate = swap(&evport->qos.min_rate, min_rate); > > As Alexander already pointed out, swap requires 'references' to two > variables, > in contrast to xchg which requires reference to variable and value. > So we cannot use swap for cases: > old_value = __xchg(&x, new_value); > > Regards > Andrzej > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH mm-unstable v1 17/20] drm/exynos: remove FOLL_FORCE usage
On Wed, Nov 16, 2022 at 11:26:56AM +0100, David Hildenbrand wrote:
> FOLL_FORCE is really only for ptrace access. As we unpin the pinned pages
> using unpin_user_pages_dirty_lock(true), the assumption is that all these
> pages are writable.
>
> FOLL_FORCE in this case seems to be a legacy leftover. Let's just remove
> it.
>
> Cc: Inki Dae
> Cc: Seung-Woo Kim
> Cc: Kyungmin Park
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Krzysztof Kozlowski
> Signed-off-by: David Hildenbrand
Reviewed-by: Daniel Vetter
Plus ack for merging through the appropriate non-drm tree.
-Daniel
> ---
> drivers/gpu/drm/exynos/exynos_drm_g2d.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index 471fd6c8135f..e19c2ceb3759 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> @@ -477,7 +477,7 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct
> g2d_data *g2d,
> }
>
> ret = pin_user_pages_fast(start, npages,
> - FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM,
> + FOLL_WRITE | FOLL_LONGTERM,
> g2d_userptr->pages);
> if (ret != npages) {
> DRM_DEV_ERROR(g2d->dev,
> --
> 2.38.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH mm-unstable v1 16/20] mm/frame-vector: remove FOLL_FORCE usage
On Wed, Nov 16, 2022 at 11:26:55AM +0100, David Hildenbrand wrote:
> FOLL_FORCE is really only for ptrace access. According to commit
> 707947247e95 ("media: videobuf2-vmalloc: get_userptr: buffers are always
> writable"), get_vaddr_frames() currently pins all pages writable as a
> workaround for issues with read-only buffers.
>
> FOLL_FORCE, however, seems to be a legacy leftover as it predates
> commit 707947247e95 ("media: videobuf2-vmalloc: get_userptr: buffers are
> always writable"). Let's just remove it.
>
> Once the read-only buffer issue has been resolved, FOLL_WRITE could
> again be set depending on the DMA direction.
>
> Cc: Hans Verkuil
> Cc: Marek Szyprowski
> Cc: Tomasz Figa
> Cc: Marek Szyprowski
> Cc: Mauro Carvalho Chehab
> Signed-off-by: David Hildenbrand
Also code I looked at while looking at follow_pfn stuff
Reviewed-by: Daniel Vetter
> ---
> drivers/media/common/videobuf2/frame_vector.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/common/videobuf2/frame_vector.c
> b/drivers/media/common/videobuf2/frame_vector.c
> index 542dde9d2609..062e98148c53 100644
> --- a/drivers/media/common/videobuf2/frame_vector.c
> +++ b/drivers/media/common/videobuf2/frame_vector.c
> @@ -50,7 +50,7 @@ int get_vaddr_frames(unsigned long start, unsigned int
> nr_frames,
> start = untagged_addr(start);
>
> ret = pin_user_pages_fast(start, nr_frames,
> - FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM,
> + FOLL_WRITE | FOLL_LONGTERM,
> (struct page **)(vec->ptrs));
> if (ret > 0) {
> vec->got_ref = true;
> --
> 2.38.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH mm-unstable v1 14/20] drm/etnaviv: remove FOLL_FORCE usage
On Wed, Nov 16, 2022 at 11:26:53AM +0100, David Hildenbrand wrote:
> GUP now supports reliable R/O long-term pinning in COW mappings, such
> that we break COW early. MAP_SHARED VMAs only use the shared zeropage so
> far in one corner case (DAXFS file with holes), which can be ignored
> because GUP does not support long-term pinning in fsdax (see
> check_vma_flags()).
>
> commit cd5297b0855f ("drm/etnaviv: Use FOLL_FORCE for userptr")
> documents that FOLL_FORCE | FOLL_WRITE was really only used for reliable
> R/O pinning.
>
> Consequently, FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM is no longer required
> for reliable R/O long-term pinning: FOLL_LONGTERM is sufficient. So stop
> using FOLL_FORCE, which is really only for ptrace access.
>
> Cc: Daniel Vetter
> Cc: Lucas Stach
> Cc: Russell King
> Cc: Christian Gmeiner
> Cc: David Airlie
> Signed-off-by: David Hildenbrand
Reviewed-by: Daniel Vetter
Also ack for merging through whatever tree suits best, since I guess this
should all land together.
-Daniel
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gem.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index cc386f8a7116..efe2240945d0 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -638,6 +638,7 @@ static int etnaviv_gem_userptr_get_pages(struct
> etnaviv_gem_object *etnaviv_obj)
> struct page **pvec = NULL;
> struct etnaviv_gem_userptr *userptr = &etnaviv_obj->userptr;
> int ret, pinned = 0, npages = etnaviv_obj->base.size >> PAGE_SHIFT;
> + unsigned int gup_flags = FOLL_LONGTERM;
>
> might_lock_read(¤t->mm->mmap_lock);
>
> @@ -648,14 +649,15 @@ static int etnaviv_gem_userptr_get_pages(struct
> etnaviv_gem_object *etnaviv_obj)
> if (!pvec)
> return -ENOMEM;
>
> + if (!userptr->ro)
> + gup_flags |= FOLL_WRITE;
> +
> do {
> unsigned num_pages = npages - pinned;
> uint64_t ptr = userptr->ptr + pinned * PAGE_SIZE;
> struct page **pages = pvec + pinned;
>
> - ret = pin_user_pages_fast(ptr, num_pages,
> - FOLL_WRITE | FOLL_FORCE |
> FOLL_LONGTERM,
> - pages);
> + ret = pin_user_pages_fast(ptr, num_pages, gup_flags, pages);
> if (ret < 0) {
> unpin_user_pages(pvec, pinned);
> kvfree(pvec);
> --
> 2.38.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH mm-unstable v1 13/20] media: videobuf-dma-sg: remove FOLL_FORCE usage
On Wed, Nov 16, 2022 at 11:26:52AM +0100, David Hildenbrand wrote:
> GUP now supports reliable R/O long-term pinning in COW mappings, such
> that we break COW early. MAP_SHARED VMAs only use the shared zeropage so
> far in one corner case (DAXFS file with holes), which can be ignored
> because GUP does not support long-term pinning in fsdax (see
> check_vma_flags()).
>
> Consequently, FOLL_FORCE | FOLL_WRITE | FOLL_LONGTERM is no longer required
> for reliable R/O long-term pinning: FOLL_LONGTERM is sufficient. So stop
> using FOLL_FORCE, which is really only for ptrace access.
>
> Cc: Mauro Carvalho Chehab
> Signed-off-by: David Hildenbrand
I looked at this a while ago when going through some of the follow_pfn
stuff, so
Reviewed-by: Daniel Vetter
> ---
> drivers/media/v4l2-core/videobuf-dma-sg.c | 14 +-
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c
> b/drivers/media/v4l2-core/videobuf-dma-sg.c
> index f75e5eedeee0..234e9f647c96 100644
> --- a/drivers/media/v4l2-core/videobuf-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
> @@ -151,17 +151,16 @@ static void videobuf_dma_init(struct videobuf_dmabuf
> *dma)
> static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
> int direction, unsigned long data, unsigned long size)
> {
> + unsigned int gup_flags = FOLL_LONGTERM;
> unsigned long first, last;
> - int err, rw = 0;
> - unsigned int flags = FOLL_FORCE;
> + int err;
>
> dma->direction = direction;
> switch (dma->direction) {
> case DMA_FROM_DEVICE:
> - rw = READ;
> + gup_flags |= FOLL_WRITE;
> break;
> case DMA_TO_DEVICE:
> - rw = WRITE;
> break;
> default:
> BUG();
> @@ -177,14 +176,11 @@ static int videobuf_dma_init_user_locked(struct
> videobuf_dmabuf *dma,
> if (NULL == dma->pages)
> return -ENOMEM;
>
> - if (rw == READ)
> - flags |= FOLL_WRITE;
> -
> dprintk(1, "init user [0x%lx+0x%lx => %lu pages]\n",
> data, size, dma->nr_pages);
>
> - err = pin_user_pages(data & PAGE_MASK, dma->nr_pages,
> - flags | FOLL_LONGTERM, dma->pages, NULL);
> + err = pin_user_pages(data & PAGE_MASK, dma->nr_pages, gup_flags,
> + dma->pages, NULL);
>
> if (err != dma->nr_pages) {
> dma->nr_pages = (err >= 0) ? err : 0;
> --
> 2.38.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH mm-unstable v1 09/20] mm/gup: reliable R/O long-term pinning in COW mappings
On Wed, Nov 16, 2022 at 11:26:48AM +0100, David Hildenbrand wrote:
> We already support reliable R/O pinning of anonymous memory. However,
> assume we end up pinning (R/O long-term) a pagecache page or the shared
> zeropage inside a writable private ("COW") mapping. The next write access
> will trigger a write-fault and replace the pinned page by an exclusive
> anonymous page in the process page tables to break COW: the pinned page no
> longer corresponds to the page mapped into the process' page table.
>
> Now that FAULT_FLAG_UNSHARE can break COW on anything mapped into a
> COW mapping, let's properly break COW first before R/O long-term
> pinning something that's not an exclusive anon page inside a COW
> mapping. FAULT_FLAG_UNSHARE will break COW and map an exclusive anon page
> instead that can get pinned safely.
>
> With this change, we can stop using FOLL_FORCE|FOLL_WRITE for reliable
> R/O long-term pinning in COW mappings.
>
> With this change, the new R/O long-term pinning tests for non-anonymous
> memory succeed:
> # [RUN] R/O longterm GUP pin ... with shared zeropage
> ok 151 Longterm R/O pin is reliable
> # [RUN] R/O longterm GUP pin ... with memfd
> ok 152 Longterm R/O pin is reliable
> # [RUN] R/O longterm GUP pin ... with tmpfile
> ok 153 Longterm R/O pin is reliable
> # [RUN] R/O longterm GUP pin ... with huge zeropage
> ok 154 Longterm R/O pin is reliable
> # [RUN] R/O longterm GUP pin ... with memfd hugetlb (2048 kB)
> ok 155 Longterm R/O pin is reliable
> # [RUN] R/O longterm GUP pin ... with memfd hugetlb (1048576 kB)
> ok 156 Longterm R/O pin is reliable
> # [RUN] R/O longterm GUP-fast pin ... with shared zeropage
> ok 157 Longterm R/O pin is reliable
> # [RUN] R/O longterm GUP-fast pin ... with memfd
> ok 158 Longterm R/O pin is reliable
> # [RUN] R/O longterm GUP-fast pin ... with tmpfile
> ok 159 Longterm R/O pin is reliable
> # [RUN] R/O longterm GUP-fast pin ... with huge zeropage
> ok 160 Longterm R/O pin is reliable
> # [RUN] R/O longterm GUP-fast pin ... with memfd hugetlb (2048 kB)
> ok 161 Longterm R/O pin is reliable
> # [RUN] R/O longterm GUP-fast pin ... with memfd hugetlb (1048576 kB)
> ok 162 Longterm R/O pin is reliable
>
> Note 1: We don't care about short-term R/O-pinning, because they have
> snapshot semantics: they are not supposed to observe modifications that
> happen after pinning.
>
> As one example, assume we start direct I/O to read from a page and store
> page content into a file: modifications to page content after starting
> direct I/O are not guaranteed to end up in the file. So even if we'd pin
> the shared zeropage, the end result would be as expected -- getting zeroes
> stored to the file.
>
> Note 2: For shared mappings we'll now always fallback to the slow path to
> lookup the VMA when R/O long-term pining. While that's the necessary price
> we have to pay right now, it's actually not that bad in practice: most
> FOLL_LONGTERM users already specify FOLL_WRITE, for example, along with
> FOLL_FORCE because they tried dealing with COW mappings correctly ...
>
> Note 3: For users that use FOLL_LONGTERM right now without FOLL_WRITE,
> such as VFIO, we'd now no longer pin the shared zeropage. Instead, we'd
> populate exclusive anon pages that we can pin. There was a concern that
> this could affect the memlock limit of existing setups.
>
> For example, a VM running with VFIO could run into the memlock limit and
> fail to run. However, we essentially had the same behavior already in
> commit 17839856fd58 ("gup: document and work around "COW can break either
> way" issue") which got merged into some enterprise distros, and there were
> not any such complaints. So most probably, we're fine.
>
> Signed-off-by: David Hildenbrand
I don't think my ack is any good for the implementation, but for the
driver side semantics this sounds like what we want :-)
Acked-by: Daniel Vetter
> ---
> include/linux/mm.h | 27 ---
> mm/gup.c | 10 +-
> mm/huge_memory.c | 2 +-
> mm/hugetlb.c | 7 ---
> 4 files changed, 34 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 6bd2ee5872dd..e8cc838f42f9 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3095,8 +3095,12 @@ static inline int vm_fault_to_errno(vm_fault_t
> vm_fault, int foll_flags)
> * Must be called with the (sub)page that's actually referenced via the
> * page table entry, which might not necessarily be the head page for a
> * PTE-mapped THP.
> + *
&g
Re: [PATCH v2 04/10] drm/simpledrm: Compute framebuffer stride if not set
On Thu, Aug 11, 2022 at 08:27:42PM +0200, Thomas Zimmermann wrote: > > > Am 11.08.22 um 20:26 schrieb Thomas Zimmermann: > > Hi Daniel > > > > Am 11.08.22 um 19:23 schrieb Daniel Vetter: > > > On Wed, 27 Jul 2022 at 09:53, Thomas Zimmermann > > > wrote: > > > > > > > > Hi > > > > > > > > Am 25.07.22 um 17:13 schrieb Javier Martinez Canillas: > > > > > Hello Geert, > > > > > > > > > > On 7/21/22 16:46, Geert Uytterhoeven wrote: > > > > > > Hi Thomas, > > > > > > > > > > > > On Wed, Jul 20, 2022 at 4:27 PM Thomas Zimmermann > > > > > > wrote: > > > > > > > Compute the framebuffer's scanline stride length if not given by > > > > > > > the simplefb data. > > > > > > > > > > > > > > Signed-off-by: Thomas Zimmermann > > > > > > > > > > > > Thanks for your patch! > > > > > > > > > > > > > --- a/drivers/gpu/drm/tiny/simpledrm.c > > > > > > > +++ b/drivers/gpu/drm/tiny/simpledrm.c > > > > > > > @@ -743,6 +743,9 @@ static struct simpledrm_device > > > > > > > *simpledrm_device_create(struct drm_driver *drv, > > > > > > > drm_err(dev, "no simplefb configuration > > > > > > > found\n"); > > > > > > > return ERR_PTR(-ENODEV); > > > > > > > } > > > > > > > + if (!stride) > > > > > > > + stride = format->cpp[0] * width; > > > > > > > > > > > > DIV_ROUND_UP(drm_format_info_bpp(format) * width, 8) > > > > > > > > > > > > > > > > I think you meant here: > > > > > > > > > > DIV_ROUND_UP(drm_format_info_bpp(format, 0) * width, 8) ? > > > > > > > > I guess, that's the right function. My original code is correct, but cpp > > > > is also deprecated. > > > > > > You all mean drm_format_info_min_pitch(). > > > > Thanks a lot. I wasn't even aware of this function, but I had almost > > written my own implementation of it. I'll update the patch accordingly. > > Arghh, too late. I merged that patch already. Reviewed-by: Daniel Vetter Preemptively, if you can do the fixup patch (and it's not yet merged)? -Daniel > > > > > Best regards > > Thomas > > > > > > > > I really don't want drivers to go grab any of the legacy format info > > > fields like bpp or depth. switch() statements on the fourcc code for > > > programming registers, or one of the real helper functions in > > > drm_fourcc.c (there might be some gaps), but not ever going through > > > legacy concepts. Anything else just leads to subtle bugs when new > > > formats get added and oops suddenly the assumptions don't hold. > > > > > > Those should be strictly limited to legacy (i.e. not drm_fourcc aware) > > > interfaces. Heck I think even fbdev emulation should completely switch > > > over to drm_fourcc/drm_format_info, but alas that's a pile of work and > > > not much payoff. > > > > > > I'm trying to volunteer Same to add a legacy_bpp tag to the above > > > helper and appropriately limit it, I think limiting to formats with > > > depth!=0 is probably the right thing. And then we should probably > > > remove a pile of the cargo-culted depth!=0 entries too. > > > -Daniel > > > > > > > > > > > Best regards > > > > Thomas > > > > > > > > > > > > > > With that change, > > > > > > > > > > Acked-by: Javier Martinez Canillas > > > > > > > > > > > > > -- > > > > Thomas Zimmermann > > > > Graphics Driver Developer > > > > SUSE Software Solutions Germany GmbH > > > > Maxfeldstr. 5, 90409 Nürnberg, Germany > > > > (HRB 36809, AG Nürnberg) > > > > Geschäftsführer: Ivo Totev > > > > > > > > > > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Ivo Totev -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v2 04/10] drm/simpledrm: Compute framebuffer stride if not set
On Wed, 27 Jul 2022 at 09:53, Thomas Zimmermann wrote: > > Hi > > Am 25.07.22 um 17:13 schrieb Javier Martinez Canillas: > > Hello Geert, > > > > On 7/21/22 16:46, Geert Uytterhoeven wrote: > >> Hi Thomas, > >> > >> On Wed, Jul 20, 2022 at 4:27 PM Thomas Zimmermann > >> wrote: > >>> Compute the framebuffer's scanline stride length if not given by > >>> the simplefb data. > >>> > >>> Signed-off-by: Thomas Zimmermann > >> > >> Thanks for your patch! > >> > >>> --- a/drivers/gpu/drm/tiny/simpledrm.c > >>> +++ b/drivers/gpu/drm/tiny/simpledrm.c > >>> @@ -743,6 +743,9 @@ static struct simpledrm_device > >>> *simpledrm_device_create(struct drm_driver *drv, > >>> drm_err(dev, "no simplefb configuration found\n"); > >>> return ERR_PTR(-ENODEV); > >>> } > >>> + if (!stride) > >>> + stride = format->cpp[0] * width; > >> > >> DIV_ROUND_UP(drm_format_info_bpp(format) * width, 8) > >> > > > > I think you meant here: > > > > DIV_ROUND_UP(drm_format_info_bpp(format, 0) * width, 8) ? > > I guess, that's the right function. My original code is correct, but cpp > is also deprecated. You all mean drm_format_info_min_pitch(). I really don't want drivers to go grab any of the legacy format info fields like bpp or depth. switch() statements on the fourcc code for programming registers, or one of the real helper functions in drm_fourcc.c (there might be some gaps), but not ever going through legacy concepts. Anything else just leads to subtle bugs when new formats get added and oops suddenly the assumptions don't hold. Those should be strictly limited to legacy (i.e. not drm_fourcc aware) interfaces. Heck I think even fbdev emulation should completely switch over to drm_fourcc/drm_format_info, but alas that's a pile of work and not much payoff. I'm trying to volunteer Same to add a legacy_bpp tag to the above helper and appropriately limit it, I think limiting to formats with depth!=0 is probably the right thing. And then we should probably remove a pile of the cargo-culted depth!=0 entries too. -Daniel > > Best regards > Thomas > > > > > With that change, > > > > Acked-by: Javier Martinez Canillas > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Ivo Totev -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 2/2] fbdev: Remove hot-unplug workaround for framebuffers without device
On Wed, Apr 13, 2022 at 12:50:50PM +0200, Javier Martinez Canillas wrote:
> On 4/13/22 11:24, Thomas Zimmermann wrote:
> > A workaround makes fbdev hot-unplugging work for framebuffers without
> > device. The only user for this feature was offb. As each OF framebuffer
> > now has an associated platform device, the workaround is no longer
> > needed. Remove it. Effectively reverts commit 0f525289ff0d ("fbdev: Fix
> > unregistering of framebuffers without device").
> >
> > Signed-off-by: Thomas Zimmermann
> > ---
> > drivers/video/fbdev/core/fbmem.c | 9 +
> > 1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/core/fbmem.c
> > b/drivers/video/fbdev/core/fbmem.c
> > index bc6ed750e915..bdd00d381bbc 100644
> > --- a/drivers/video/fbdev/core/fbmem.c
> > +++ b/drivers/video/fbdev/core/fbmem.c
> > @@ -1579,14 +1579,7 @@ static void
> > do_remove_conflicting_framebuffers(struct apertures_struct *a,
> > * If it's not a platform device, at least print a
> > warning. A
> > * fix would add code to remove the device from the
> > system.
> > */
> > - if (!device) {
> > - /* TODO: Represent each OF framebuffer as its
> > own
> > -* device in the device hierarchy. For now, offb
> > -* doesn't have such a device, so unregister the
> > -* framebuffer as before without warning.
> > -*/
> > - do_unregister_framebuffer(registered_fb[i]);
>
> Maybe we could still keep this for a couple of releases but with a big
> warning that's not supported in case there are out-of-tree drivers out
> there that still do this ?
>
> Or at least a warning if the do_unregister_framebuffer() call is removed.
Yeah dying while holding console_lock isn't fun, and not having a WARN_ON
+ bail-out code pretty much forces bug reporters to do a bisect here to
give us something more than "machine dies at boot with no messages".
I'd just outright keep the WARN_ON here for 1-2 years even to really make
sure we got all the bug reports, since often these older machines only
update onto LTS releases.
And it needs to be a WARN_ON + bail out since BUG_ON is as bad as just
oopsing.
-Daniel
>
> Regardless of what you chose to do, the patch looks good to me.
>
> Reviewed-by: Javier Martinez Canillas
>
> --
> Best regards,
>
> Javier Martinez Canillas
> Linux Engineering
> Red Hat
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: remove the nvlink2 pci_vfio subdriver v2
On Tue, May 04, 2021 at 12:53:27PM -0300, Jason Gunthorpe wrote: > On Tue, May 04, 2021 at 04:23:40PM +0200, Daniel Vetter wrote: > > > Just my 2cents from drm (where we deprecate old gunk uapi quite often): > > Imo it's best to keep the uapi headers as-is, but exchange the > > documentation with a big "this is removed, never use again" warning: > > We in RDMA have been doing the opposite, the uapi headers are supposed > to reflect the current kernel. This helps make the kernel > understandable. > > When userspace needs backwards compat to ABI that the current kernel > doesn't support then userspace has distinct copies of that information > in some compat location. It has happened a few times over the last 15 > years. > > We keep full copies of the current kernel headers in the userspace > source tree, when the kernel headers change in a compile incompatible > way we fix everything while updating to the new kernel headers. Yeah we do the same since forever (it's either from libdrm package, or directly in the corresponding userspace header). So largely include/uapi is for documentation > > - it's good to know which uapi numbers (like parameter extensions or > > whatever they are in this case) are defacto reserved, because there are > > binaries (qemu in this) that have code acting on them out there. > > Numbers and things get marked reserved or the like > > > Anyway feel free to ignore since this might be different than drivers/gpu. > > AFAIK drives/gpu has a lot wider userspace, rdma manages this OK > because we only have one library package that provides the user/kernel > interface. But since we have some many projects we've started asking all the userspace projects to directly take the kernel ones (after the make step to filter them) so that there's only one source of truth. And also to make sure they don't merge stuff before the kernel side is reviewed&landed. Which also means we can't ditch anything userspace might still need on older trees and stuff. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: remove the nvlink2 pci_vfio subdriver v2
On Tue, May 04, 2021 at 03:20:34PM +0200, Greg Kurz wrote: > On Tue, 4 May 2021 14:59:07 +0200 > Greg Kroah-Hartman wrote: > > > On Tue, May 04, 2021 at 02:22:36PM +0200, Greg Kurz wrote: > > > On Fri, 26 Mar 2021 07:13:09 +0100 > > > Christoph Hellwig wrote: > > > > > > > Hi all, > > > > > > > > the nvlink2 vfio subdriver is a weird beast. It supports a hardware > > > > feature without any open source component - what would normally be > > > > the normal open source userspace that we require for kernel drivers, > > > > although in this particular case user space could of course be a > > > > kernel driver in a VM. It also happens to be a complete mess that > > > > does not properly bind to PCI IDs, is hacked into the vfio_pci driver > > > > and also pulles in over 1000 lines of code always build into powerpc > > > > kernels that have Power NV support enabled. Because of all these > > > > issues and the lack of breaking userspace when it is removed I think > > > > the best idea is to simply kill. > > > > > > > > Changes since v1: > > > > - document the removed subtypes as reserved > > > > - add the ACK from Greg > > > > > > > > Diffstat: > > > > arch/powerpc/platforms/powernv/npu-dma.c | 705 > > > > --- > > > > b/arch/powerpc/include/asm/opal.h|3 > > > > b/arch/powerpc/include/asm/pci-bridge.h |1 > > > > b/arch/powerpc/include/asm/pci.h |7 > > > > b/arch/powerpc/platforms/powernv/Makefile|2 > > > > b/arch/powerpc/platforms/powernv/opal-call.c |2 > > > > b/arch/powerpc/platforms/powernv/pci-ioda.c | 185 --- > > > > b/arch/powerpc/platforms/powernv/pci.c | 11 > > > > b/arch/powerpc/platforms/powernv/pci.h | 17 > > > > b/arch/powerpc/platforms/pseries/pci.c | 23 > > > > b/drivers/vfio/pci/Kconfig |6 > > > > b/drivers/vfio/pci/Makefile |1 > > > > b/drivers/vfio/pci/vfio_pci.c| 18 > > > > b/drivers/vfio/pci/vfio_pci_private.h| 14 > > > > b/include/uapi/linux/vfio.h | 38 - > > > > > > > > > Hi Christoph, > > > > > > FYI, these uapi changes break build of QEMU. > > > > What uapi changes? > > > > All macros and structure definitions that are being removed > from include/uapi/linux/vfio.h by patch 1. Just my 2cents from drm (where we deprecate old gunk uapi quite often): Imo it's best to keep the uapi headers as-is, but exchange the documentation with a big "this is removed, never use again" warning: - it occasionally serves as a good lesson for how to not do uapi (whatever the reasons really are in the specific case) - it's good to know which uapi numbers (like parameter extensions or whatever they are in this case) are defacto reserved, because there are binaries (qemu in this) that have code acting on them out there. The only exception where we completely nuke the structs and #defines is when uapi has been only used by testcases. Which we know, since we defacto limit our stable uapi guarantee to the canonical open&upstream userspace drivers only (for at least the driver-specific stuff, the cross-driver interfaces are hopeless). Anyway feel free to ignore since this might be different than drivers/gpu. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 02/13] drm: remove drm_fb_helper_modinit
On Thu, Jan 28, 2021 at 07:14:10PM +0100, Christoph Hellwig wrote:
> drm_fb_helper_modinit has a lot of boilerplate for what is not very
> simple functionality. Just open code it in the only caller using
> IS_ENABLED and IS_MODULE, and skip the find_module check as a
> request_module is harmless if the module is already loaded (and not
> other caller has this find_module check either).
>
> Signed-off-by: Christoph Hellwig
Hm I thought I've acked this one already somewhere for merging through
your tree.
Acked-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_crtc_helper_internal.h | 10 -
> drivers/gpu/drm/drm_fb_helper.c| 21 --
> drivers/gpu/drm/drm_kms_helper_common.c| 25 +++---
> 3 files changed, 12 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h
> b/drivers/gpu/drm/drm_crtc_helper_internal.h
> index 25ce42e799952c..61e09f8a8d0ff0 100644
> --- a/drivers/gpu/drm/drm_crtc_helper_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
> @@ -32,16 +32,6 @@
> #include
> #include
>
> -/* drm_fb_helper.c */
> -#ifdef CONFIG_DRM_FBDEV_EMULATION
> -int drm_fb_helper_modinit(void);
> -#else
> -static inline int drm_fb_helper_modinit(void)
> -{
> - return 0;
> -}
> -#endif
> -
> /* drm_dp_aux_dev.c */
> #ifdef CONFIG_DRM_DP_AUX_CHARDEV
> int drm_dp_aux_dev_init(void);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 4b81195106875d..0b9f1ae1b7864c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2499,24 +2499,3 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
> drm_client_register(&fb_helper->client);
> }
> EXPORT_SYMBOL(drm_fbdev_generic_setup);
> -
> -/* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
> - * but the module doesn't depend on any fb console symbols. At least
> - * attempt to load fbcon to avoid leaving the system without a usable
> console.
> - */
> -int __init drm_fb_helper_modinit(void)
> -{
> -#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
> - const char name[] = "fbcon";
> - struct module *fbcon;
> -
> - mutex_lock(&module_mutex);
> - fbcon = find_module(name);
> - mutex_unlock(&module_mutex);
> -
> - if (!fbcon)
> - request_module_nowait(name);
> -#endif
> - return 0;
> -}
> -EXPORT_SYMBOL(drm_fb_helper_modinit);
> diff --git a/drivers/gpu/drm/drm_kms_helper_common.c
> b/drivers/gpu/drm/drm_kms_helper_common.c
> index 221a8528c9937a..f933da1656eb52 100644
> --- a/drivers/gpu/drm/drm_kms_helper_common.c
> +++ b/drivers/gpu/drm/drm_kms_helper_common.c
> @@ -64,19 +64,18 @@ MODULE_PARM_DESC(edid_firmware,
>
> static int __init drm_kms_helper_init(void)
> {
> - int ret;
> -
> - /* Call init functions from specific kms helpers here */
> - ret = drm_fb_helper_modinit();
> - if (ret < 0)
> - goto out;
> -
> - ret = drm_dp_aux_dev_init();
> - if (ret < 0)
> - goto out;
> -
> -out:
> - return ret;
> + /*
> + * The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
> + * but the module doesn't depend on any fb console symbols. At least
> + * attempt to load fbcon to avoid leaving the system without a usable
> + * console.
> + */
> + if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) &&
> + IS_MODULE(CONFIG_FRAMEBUFFER_CONSOLE) &&
> + !IS_ENABLED(CONFIG_EXPERT))
> + request_module_nowait("fbcon");
> +
> + return drm_dp_aux_dev_init();
> }
>
> static void __exit drm_kms_helper_exit(void)
> --
> 2.29.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit
On Thu, Jan 21, 2021 at 9:28 AM Christoph Hellwig wrote: > > On Thu, Jan 21, 2021 at 09:25:40AM +0100, Daniel Vetter wrote: > > On Thu, Jan 21, 2021 at 8:55 AM Christoph Hellwig wrote: > > > > > > drm_fb_helper_modinit has a lot of boilerplate for what is not very > > > simple functionality. Just open code it in the only caller using > > > IS_ENABLED and IS_MODULE. > > > > > > Signed-off-by: Christoph Hellwig > > > > I didn't spot any dependencies with your series, should I just merge > > this through drm trees? Or do you want an ack? > > I'd prefer an ACK - module_loaded() is only introduced earlier in this > series. I was looking for that but didn't find the hunk touching drm somehow ... Acked-by: Daniel Vetter Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 08/13] drm: remove drm_fb_helper_modinit
On Thu, Jan 21, 2021 at 8:55 AM Christoph Hellwig wrote:
>
> drm_fb_helper_modinit has a lot of boilerplate for what is not very
> simple functionality. Just open code it in the only caller using
> IS_ENABLED and IS_MODULE.
>
> Signed-off-by: Christoph Hellwig
I didn't spot any dependencies with your series, should I just merge
this through drm trees? Or do you want an ack?
-Daniel
> ---
> drivers/gpu/drm/drm_crtc_helper_internal.h | 10 -
> drivers/gpu/drm/drm_fb_helper.c| 16 -
> drivers/gpu/drm/drm_kms_helper_common.c| 26 +++---
> 3 files changed, 13 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h
> b/drivers/gpu/drm/drm_crtc_helper_internal.h
> index 25ce42e799952c..61e09f8a8d0ff0 100644
> --- a/drivers/gpu/drm/drm_crtc_helper_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_helper_internal.h
> @@ -32,16 +32,6 @@
> #include
> #include
>
> -/* drm_fb_helper.c */
> -#ifdef CONFIG_DRM_FBDEV_EMULATION
> -int drm_fb_helper_modinit(void);
> -#else
> -static inline int drm_fb_helper_modinit(void)
> -{
> - return 0;
> -}
> -#endif
> -
> /* drm_dp_aux_dev.c */
> #ifdef CONFIG_DRM_DP_AUX_CHARDEV
> int drm_dp_aux_dev_init(void);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index ce6d63ca75c32a..0b9f1ae1b7864c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2499,19 +2499,3 @@ void drm_fbdev_generic_setup(struct drm_device *dev,
> drm_client_register(&fb_helper->client);
> }
> EXPORT_SYMBOL(drm_fbdev_generic_setup);
> -
> -/* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
> - * but the module doesn't depend on any fb console symbols. At least
> - * attempt to load fbcon to avoid leaving the system without a usable
> console.
> - */
> -int __init drm_fb_helper_modinit(void)
> -{
> -#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE) && !defined(CONFIG_EXPERT)
> - const char name[] = "fbcon";
> -
> - if (!module_loaded(name))
> - request_module_nowait(name);
> -#endif
> - return 0;
> -}
> -EXPORT_SYMBOL(drm_fb_helper_modinit);
> diff --git a/drivers/gpu/drm/drm_kms_helper_common.c
> b/drivers/gpu/drm/drm_kms_helper_common.c
> index 221a8528c9937a..b694a7da632eae 100644
> --- a/drivers/gpu/drm/drm_kms_helper_common.c
> +++ b/drivers/gpu/drm/drm_kms_helper_common.c
> @@ -64,19 +64,19 @@ MODULE_PARM_DESC(edid_firmware,
>
> static int __init drm_kms_helper_init(void)
> {
> - int ret;
> -
> - /* Call init functions from specific kms helpers here */
> - ret = drm_fb_helper_modinit();
> - if (ret < 0)
> - goto out;
> -
> - ret = drm_dp_aux_dev_init();
> - if (ret < 0)
> - goto out;
> -
> -out:
> - return ret;
> + /*
> +* The Kconfig DRM_KMS_HELPER selects FRAMEBUFFER_CONSOLE (if !EXPERT)
> +* but the module doesn't depend on any fb console symbols. At least
> +* attempt to load fbcon to avoid leaving the system without a usable
> +* console.
> +*/
> + if (IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION) &&
> + IS_MODULE(CONFIG_FRAMEBUFFER_CONSOLE) &&
> + !IS_ENABLED(CONFIG_EXPERT) &&
> + !module_loaded("fbcon"))
> + request_module_nowait("fbcon");
> +
> + return drm_dp_aux_dev_init();
> }
>
> static void __exit drm_kms_helper_exit(void)
> --
> 2.29.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH RFC PKS/PMEM 09/58] drivers/gpu: Utilize new kmap_thread()
On Fri, Oct 09, 2020 at 12:49:44PM -0700, ira.we...@intel.com wrote:
> From: Ira Weiny
>
> These kmap() calls in the gpu stack are localized to a single thread.
> To avoid the over head of global PKRS updates use the new kmap_thread()
> call.
>
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Patrik Jakobsson
> Signed-off-by: Ira Weiny
I'm guessing the entire pile goes in through some other tree. If so:
Acked-by: Daniel Vetter
If you want this to land through maintainer trees, then we need a
per-driver split (since aside from amdgpu and radeon they're all different
subtrees).
btw the two kmap calls in drm you highlight in the cover letter should
also be convertible to kmap_thread. We only hold vmalloc mappings for a
longer time (or it'd be quite a driver bug). So if you want maybe throw
those two as two additional patches on top, and we can do some careful
review & testing for them.
-Daniel
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 ++--
> drivers/gpu/drm/gma500/gma_display.c | 4 ++--
> drivers/gpu/drm/gma500/mmu.c | 10 +-
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c| 4 ++--
> .../gpu/drm/i915/gem/selftests/i915_gem_context.c| 4 ++--
> drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 8
> drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 4 ++--
> drivers/gpu/drm/i915/gt/intel_gtt.c | 4 ++--
> drivers/gpu/drm/i915/gt/shmem_utils.c| 4 ++--
> drivers/gpu/drm/i915/i915_gem.c | 8
> drivers/gpu/drm/i915/i915_gpu_error.c| 4 ++--
> drivers/gpu/drm/i915/selftests/i915_perf.c | 4 ++--
> drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++--
> 13 files changed, 37 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 978bae731398..bd564bccb7a3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -2437,11 +2437,11 @@ static ssize_t amdgpu_ttm_gtt_read(struct file *f,
> char __user *buf,
>
> page = adev->gart.pages[p];
> if (page) {
> - ptr = kmap(page);
> + ptr = kmap_thread(page);
> ptr += off;
>
> r = copy_to_user(buf, ptr, cur_size);
> - kunmap(adev->gart.pages[p]);
> + kunmap_thread(adev->gart.pages[p]);
> } else
> r = clear_user(buf, cur_size);
>
> @@ -2507,9 +2507,9 @@ static ssize_t amdgpu_iomem_read(struct file *f, char
> __user *buf,
> if (p->mapping != adev->mman.bdev.dev_mapping)
> return -EPERM;
>
> - ptr = kmap(p);
> + ptr = kmap_thread(p);
> r = copy_to_user(buf, ptr + off, bytes);
> - kunmap(p);
> + kunmap_thread(p);
> if (r)
> return -EFAULT;
>
> @@ -2558,9 +2558,9 @@ static ssize_t amdgpu_iomem_write(struct file *f, const
> char __user *buf,
> if (p->mapping != adev->mman.bdev.dev_mapping)
> return -EPERM;
>
> - ptr = kmap(p);
> + ptr = kmap_thread(p);
> r = copy_from_user(ptr + off, buf, bytes);
> - kunmap(p);
> + kunmap_thread(p);
> if (r)
> return -EFAULT;
>
> diff --git a/drivers/gpu/drm/gma500/gma_display.c
> b/drivers/gpu/drm/gma500/gma_display.c
> index 3df6d6e850f5..35f4e55c941f 100644
> --- a/drivers/gpu/drm/gma500/gma_display.c
> +++ b/drivers/gpu/drm/gma500/gma_display.c
> @@ -400,9 +400,9 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc,
> /* Copy the cursor to cursor mem */
> tmp_dst = dev_priv->vram_addr + cursor_gt->offset;
> for (i = 0; i < cursor_pages; i++) {
> - tmp_src = kmap(gt->pages[i]);
> + tmp_src = kmap_thread(gt->pages[i]);
> memcpy(tmp_dst, tmp_src, PAGE_SIZE);
> - kunmap(gt->pages[i]);
> + kunmap_thread(gt->pages[i]);
> tmp_dst += PAGE_SIZE;
> }
>
> diff --git a/drivers/gpu/drm/gma500/mmu.c b/drivers/gpu/drm/gma500/mmu.c
> index 505044c9a673..fba7a3a461fd 100644
> --- a/drivers/gpu/drm/gma500/mmu.c
> +++ b/drivers/gpu/drm/gma500/mmu.c
> @@ -192,20 +192,20 @@ struct psb_mmu_pd *psb_mmu_alloc_pd(struct
>
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Sun, Sep 20, 2020 at 08:23:26AM +0200, Thomas Gleixner wrote:
> On Sat, Sep 19 2020 at 12:37, Daniel Vetter wrote:
> > On Sat, Sep 19, 2020 at 12:35 PM Daniel Vetter wrote:
> >> I think it should be the case, but I want to double check: Will
> >> copy_*_user be allowed within a kmap_temporary section? This would
> >> allow us to ditch an absolute pile of slowpaths.
> >
> > (coffee just kicked in) copy_*_user is ofc allowed, but if you hit a
> > page fault you get a short read/write. This looks like it would remove
> > the need to handle these in a slowpath, since page faults can now be
> > served in this new kmap_temporary sections. But this sounds too good
> > to be true, so I'm wondering what I'm missing.
>
> In principle we could allow pagefaults, but not with the currently
> proposed interface which can be called from any context. Obviously if
> called from atomic context it can't handle user page faults.
Yeah that's clear, but does the implemention need to disable pagefaults
unconditionally?
> In theory we could make a variant which does not disable pagefaults, but
> that's what kmap() already provides.
Currently we have a bunch of code which roughly does
kmap_atomic();
copy_*_user();
kunmap_atomic();
if (short_copy_user) {
kmap();
copy_*_user(remaining_stuff);
kunmap();
}
And the only reason is that kmap is a notch slower, hence the fastpath. If
we get a kmap which is fast and allows pagefaults (only in contexts that
allow pagefaults already ofc) then we can ditch a pile of kmap users.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Sat, Sep 19, 2020 at 12:35 PM Daniel Vetter wrote: > > On Sat, Sep 19, 2020 at 11:50 AM Thomas Gleixner wrote: > > > > First of all, sorry for the horribly big Cc list! > > > > Following up to the discussion in: > > > > https://lore.kernel.org/r/20200914204209.256266...@linutronix.de > > > > this provides a preemptible variant of kmap_atomic & related > > interfaces. This is achieved by: > > > > - Consolidating all kmap atomic implementations in generic code > > > > - Switching from per CPU storage of the kmap index to a per task storage > > > > - Adding a pteval array to the per task storage which contains the ptevals > >of the currently active temporary kmaps > > > > - Adding context switch code which checks whether the outgoing or the > >incoming task has active temporary kmaps. If so, the outgoing task's > >kmaps are removed and the incoming task's kmaps are restored. > > > > - Adding new interfaces k[un]map_temporary*() which are not disabling > >preemption and can be called from any context (except NMI). > > > >Contrary to kmap() which provides preemptible and "persistant" mappings, > >these interfaces are meant to replace the temporary mappings provided by > >kmap_atomic*() today. > > > > This allows to get rid of conditional mapping choices and allows to have > > preemptible short term mappings on 64bit which are today enforced to be > > non-preemptible due to the highmem constraints. It clearly puts overhead on > > the highmem users, but highmem is slow anyway. > > > > This is not a wholesale conversion which makes kmap_atomic magically > > preemptible because there might be usage sites which rely on the implicit > > preempt disable. So this needs to be done on a case by case basis and the > > call sites converted to kmap_temporary. > > > > Note, that this is only lightly tested on X86 and completely untested on > > all other architectures. > > > > The lot is also available from > > > >git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git highmem > > I think it should be the case, but I want to double check: Will > copy_*_user be allowed within a kmap_temporary section? This would > allow us to ditch an absolute pile of slowpaths. (coffee just kicked in) copy_*_user is ofc allowed, but if you hit a page fault you get a short read/write. This looks like it would remove the need to handle these in a slowpath, since page faults can now be served in this new kmap_temporary sections. But this sounds too good to be true, so I'm wondering what I'm missing. -Daniel > > > > > Thanks, > > > > tglx > > --- > > a/arch/arm/mm/highmem.c | 121 - > > a/arch/microblaze/mm/highmem.c| 78 - > > a/arch/nds32/mm/highmem.c | 48 > > a/arch/powerpc/mm/highmem.c | 67 --- > > a/arch/sparc/mm/highmem.c | 115 > > arch/arc/Kconfig |1 > > arch/arc/include/asm/highmem.h|8 + > > arch/arc/mm/highmem.c | 44 --- > > arch/arm/Kconfig |1 > > arch/arm/include/asm/highmem.h| 30 +++-- > > arch/arm/mm/Makefile |1 > > arch/csky/Kconfig |1 > > arch/csky/include/asm/highmem.h |4 > > arch/csky/mm/highmem.c| 75 - > > arch/microblaze/Kconfig |1 > > arch/microblaze/include/asm/highmem.h |6 - > > arch/microblaze/mm/Makefile |1 > > arch/microblaze/mm/init.c |6 - > > arch/mips/Kconfig |1 > > arch/mips/include/asm/highmem.h |4 > > arch/mips/mm/highmem.c| 77 - > > arch/mips/mm/init.c |3 > > arch/nds32/Kconfig.cpu|1 > > arch/nds32/include/asm/highmem.h | 21 ++- > > arch/nds32/mm/Makefile|1 > > arch/powerpc/Kconfig |1 > > arch/powerpc/include/asm/highmem.h|6 - > > arch/powerpc/mm/Makefile |1 > > arch/powerpc/mm/mem.c |7 - > > arch/sparc/Kconfig|1 > > arch/sparc/include/asm/highmem.h |7 - > > arch/sparc/mm/Makefile|3 > > arch/sparc/mm/srmmu.c |2 > > arch/x86/include/asm/fixmap.h
Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends
On Sat, Sep 19, 2020 at 11:50 AM Thomas Gleixner wrote: > > First of all, sorry for the horribly big Cc list! > > Following up to the discussion in: > > https://lore.kernel.org/r/20200914204209.256266...@linutronix.de > > this provides a preemptible variant of kmap_atomic & related > interfaces. This is achieved by: > > - Consolidating all kmap atomic implementations in generic code > > - Switching from per CPU storage of the kmap index to a per task storage > > - Adding a pteval array to the per task storage which contains the ptevals >of the currently active temporary kmaps > > - Adding context switch code which checks whether the outgoing or the >incoming task has active temporary kmaps. If so, the outgoing task's >kmaps are removed and the incoming task's kmaps are restored. > > - Adding new interfaces k[un]map_temporary*() which are not disabling >preemption and can be called from any context (except NMI). > >Contrary to kmap() which provides preemptible and "persistant" mappings, >these interfaces are meant to replace the temporary mappings provided by >kmap_atomic*() today. > > This allows to get rid of conditional mapping choices and allows to have > preemptible short term mappings on 64bit which are today enforced to be > non-preemptible due to the highmem constraints. It clearly puts overhead on > the highmem users, but highmem is slow anyway. > > This is not a wholesale conversion which makes kmap_atomic magically > preemptible because there might be usage sites which rely on the implicit > preempt disable. So this needs to be done on a case by case basis and the > call sites converted to kmap_temporary. > > Note, that this is only lightly tested on X86 and completely untested on > all other architectures. > > The lot is also available from > >git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git highmem I think it should be the case, but I want to double check: Will copy_*_user be allowed within a kmap_temporary section? This would allow us to ditch an absolute pile of slowpaths. -Daniel > > Thanks, > > tglx > --- > a/arch/arm/mm/highmem.c | 121 - > a/arch/microblaze/mm/highmem.c| 78 - > a/arch/nds32/mm/highmem.c | 48 > a/arch/powerpc/mm/highmem.c | 67 --- > a/arch/sparc/mm/highmem.c | 115 > arch/arc/Kconfig |1 > arch/arc/include/asm/highmem.h|8 + > arch/arc/mm/highmem.c | 44 --- > arch/arm/Kconfig |1 > arch/arm/include/asm/highmem.h| 30 +++-- > arch/arm/mm/Makefile |1 > arch/csky/Kconfig |1 > arch/csky/include/asm/highmem.h |4 > arch/csky/mm/highmem.c| 75 - > arch/microblaze/Kconfig |1 > arch/microblaze/include/asm/highmem.h |6 - > arch/microblaze/mm/Makefile |1 > arch/microblaze/mm/init.c |6 - > arch/mips/Kconfig |1 > arch/mips/include/asm/highmem.h |4 > arch/mips/mm/highmem.c| 77 - > arch/mips/mm/init.c |3 > arch/nds32/Kconfig.cpu|1 > arch/nds32/include/asm/highmem.h | 21 ++- > arch/nds32/mm/Makefile|1 > arch/powerpc/Kconfig |1 > arch/powerpc/include/asm/highmem.h|6 - > arch/powerpc/mm/Makefile |1 > arch/powerpc/mm/mem.c |7 - > arch/sparc/Kconfig|1 > arch/sparc/include/asm/highmem.h |7 - > arch/sparc/mm/Makefile|3 > arch/sparc/mm/srmmu.c |2 > arch/x86/include/asm/fixmap.h |1 > arch/x86/include/asm/highmem.h| 12 +- > arch/x86/include/asm/iomap.h | 29 +++-- > arch/x86/mm/highmem_32.c | 59 -- > arch/x86/mm/init_32.c | 15 -- > arch/x86/mm/iomap_32.c| 57 -- > arch/xtensa/Kconfig |1 > arch/xtensa/include/asm/highmem.h |9 + > arch/xtensa/mm/highmem.c | 44 --- > b/arch/x86/Kconfig|3 > include/linux/highmem.h | 141 +++- > include/linux/io-mapping.h |2 > include/linux/sched.h |9 + > kernel/sched/core.c | 10 + > mm/Kconfig|3 > mm/highmem.c | 192 > -- > 49 files changed, 422 insertions(+), 909 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 06/20] Documentation: gpu/komeda-kms: eliminate duplicated word
This and next patch merged to drm-misc-next, thanks. On Wed, Jul 08, 2020 at 01:08:21PM +0800, james qian wang (Arm Technology China) wrote: > Hi Randy > > On Tue, Jul 07, 2020 at 11:04:00AM -0700, Randy Dunlap wrote: > > Drop the doubled word "and". > > > > Signed-off-by: Randy Dunlap > > Cc: Jonathan Corbet > > Cc: linux-...@vger.kernel.org > > Cc: James (Qian) Wang > > Cc: Liviu Dudau > > Cc: Mihail Atanassov > > Cc: Mali DP Maintainers > > --- > > Documentation/gpu/komeda-kms.rst |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > --- linux-next-20200701.orig/Documentation/gpu/komeda-kms.rst > > +++ linux-next-20200701/Documentation/gpu/komeda-kms.rst > > @@ -41,7 +41,7 @@ Compositor blends multiple layers or pix > > frame. its output frame can be fed into post image processor for showing > > it on > > the monitor or fed into wb_layer and written to memory at the same time. > > user can also insert a scaler between compositor and wb_layer to down scale > > -the display frame first and and then write to memory. > > +the display frame first and then write to memory. > > Thank you for the patch. > > Reviewed-by: James Qian Wang James, for simple patches like this just go ahead and merge them. You're the maintainer for this, just slapping an r-b onto a patch and no indiciation whether you will pick it up only confuses people and increases the risk that patches get lost. So either pick up right away, or state clearly that you will pick it up later, or that you expect someone else to merge this. Thanks, Daniel > > > Writeback Layer (wb_layer) > > -- -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 06/20] Documentation: gpu/komeda-kms: eliminate duplicated word
On Tue, Jul 14, 2020 at 12:10:05PM +0200, Daniel Vetter wrote: > This and next patch merged to drm-misc-next, thanks. Oops strike this, I just noticed that Jon said he's picked it all up. Oh well, the confusion, I managed to stop the script before it published anything at least :-) -Daniel > > On Wed, Jul 08, 2020 at 01:08:21PM +0800, james qian wang (Arm Technology > China) wrote: > > Hi Randy > > > > On Tue, Jul 07, 2020 at 11:04:00AM -0700, Randy Dunlap wrote: > > > Drop the doubled word "and". > > > > > > Signed-off-by: Randy Dunlap > > > Cc: Jonathan Corbet > > > Cc: linux-...@vger.kernel.org > > > Cc: James (Qian) Wang > > > Cc: Liviu Dudau > > > Cc: Mihail Atanassov > > > Cc: Mali DP Maintainers > > > --- > > > Documentation/gpu/komeda-kms.rst |2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > --- linux-next-20200701.orig/Documentation/gpu/komeda-kms.rst > > > +++ linux-next-20200701/Documentation/gpu/komeda-kms.rst > > > @@ -41,7 +41,7 @@ Compositor blends multiple layers or pix > > > frame. its output frame can be fed into post image processor for showing > > > it on > > > the monitor or fed into wb_layer and written to memory at the same time. > > > user can also insert a scaler between compositor and wb_layer to down > > > scale > > > -the display frame first and and then write to memory. > > > +the display frame first and then write to memory. > > > > Thank you for the patch. > > > > Reviewed-by: James Qian Wang > > James, for simple patches like this just go ahead and merge them. You're > the maintainer for this, just slapping an r-b onto a patch and no > indiciation whether you will pick it up only confuses people and increases > the risk that patches get lost. > > So either pick up right away, or state clearly that you will pick it up > later, or that you expect someone else to merge this. > > Thanks, Daniel > > > > > Writeback Layer (wb_layer) > > > -- > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH V2 11/11] drm: Remove drm specific kmap_atomic code
On Mon, May 4, 2020 at 3:09 AM wrote:
>
> From: Ira Weiny
>
> kmap_atomic_prot() is now exported by all architectures. Use this
> function rather than open coding a driver specific kmap_atomic.
>
> Reviewed-by: Christian König
> Reviewed-by: Christoph Hellwig
> Signed-off-by: Ira Weiny
I'm assuming this lands through some other tree or a topic branch or whatever.
Acked-by: Daniel Vetter
Cheers, Daniel
> ---
> drivers/gpu/drm/ttm/ttm_bo_util.c| 56 ++--
> drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 16
> include/drm/ttm/ttm_bo_api.h | 4 --
> 3 files changed, 12 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 52d2b71f1588..f09b096ba4fd 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -257,54 +257,6 @@ static int ttm_copy_io_page(void *dst, void *src,
> unsigned long page)
> return 0;
> }
>
> -#ifdef CONFIG_X86
> -#define __ttm_kmap_atomic_prot(__page, __prot) kmap_atomic_prot(__page,
> __prot)
> -#define __ttm_kunmap_atomic(__addr) kunmap_atomic(__addr)
> -#else
> -#define __ttm_kmap_atomic_prot(__page, __prot) vmap(&__page, 1, 0, __prot)
> -#define __ttm_kunmap_atomic(__addr) vunmap(__addr)
> -#endif
> -
> -
> -/**
> - * ttm_kmap_atomic_prot - Efficient kernel map of a single page with
> - * specified page protection.
> - *
> - * @page: The page to map.
> - * @prot: The page protection.
> - *
> - * This function maps a TTM page using the kmap_atomic api if available,
> - * otherwise falls back to vmap. The user must make sure that the
> - * specified page does not have an aliased mapping with a different caching
> - * policy unless the architecture explicitly allows it. Also mapping and
> - * unmapping using this api must be correctly nested. Unmapping should
> - * occur in the reverse order of mapping.
> - */
> -void *ttm_kmap_atomic_prot(struct page *page, pgprot_t prot)
> -{
> - if (pgprot_val(prot) == pgprot_val(PAGE_KERNEL))
> - return kmap_atomic(page);
> - else
> - return __ttm_kmap_atomic_prot(page, prot);
> -}
> -EXPORT_SYMBOL(ttm_kmap_atomic_prot);
> -
> -/**
> - * ttm_kunmap_atomic_prot - Unmap a page that was mapped using
> - * ttm_kmap_atomic_prot.
> - *
> - * @addr: The virtual address from the map.
> - * @prot: The page protection.
> - */
> -void ttm_kunmap_atomic_prot(void *addr, pgprot_t prot)
> -{
> - if (pgprot_val(prot) == pgprot_val(PAGE_KERNEL))
> - kunmap_atomic(addr);
> - else
> - __ttm_kunmap_atomic(addr);
> -}
> -EXPORT_SYMBOL(ttm_kunmap_atomic_prot);
> -
> static int ttm_copy_io_ttm_page(struct ttm_tt *ttm, void *src,
> unsigned long page,
> pgprot_t prot)
> @@ -316,13 +268,13 @@ static int ttm_copy_io_ttm_page(struct ttm_tt *ttm,
> void *src,
> return -ENOMEM;
>
> src = (void *)((unsigned long)src + (page << PAGE_SHIFT));
> - dst = ttm_kmap_atomic_prot(d, prot);
> + dst = kmap_atomic_prot(d, prot);
> if (!dst)
> return -ENOMEM;
>
> memcpy_fromio(dst, src, PAGE_SIZE);
>
> - ttm_kunmap_atomic_prot(dst, prot);
> + kunmap_atomic(dst);
>
> return 0;
> }
> @@ -338,13 +290,13 @@ static int ttm_copy_ttm_io_page(struct ttm_tt *ttm,
> void *dst,
> return -ENOMEM;
>
> dst = (void *)((unsigned long)dst + (page << PAGE_SHIFT));
> - src = ttm_kmap_atomic_prot(s, prot);
> + src = kmap_atomic_prot(s, prot);
> if (!src)
> return -ENOMEM;
>
> memcpy_toio(dst, src, PAGE_SIZE);
>
> - ttm_kunmap_atomic_prot(src, prot);
> + kunmap_atomic(src);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> index bb46ca0c458f..94d456a1d1a9 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
> @@ -374,12 +374,12 @@ static int vmw_bo_cpu_blit_line(struct
> vmw_bo_blit_line_data *d,
> copy_size = min_t(u32, copy_size, PAGE_SIZE -
> src_page_offset);
>
> if (unmap_src) {
> - ttm_kunmap_atomic_prot(d->src_addr, d->src_prot);
> + kunmap_atomic(d->src_addr);
> d->src_addr = NULL;
> }
>
> if (unmap_dst) {
> - ttm_kunmap
Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
On Fri, Apr 10, 2020 at 12:57 AM Benjamin Herrenschmidt wrote: > > On Thu, 2020-04-09 at 11:41 +0200, Daniel Vetter wrote: > > Now if these boxes didn't ever have agp then I think we can get away > > with deleting this, since we've already deleted the legacy radeon > > driver. And that one used vmalloc for everything. The new kms one does > > use the dma-api if the gpu isn't connected through agp > > Definitely no AGP there. Ah in that case I think we can be sure that this code is dead. Acked-by: Daniel Vetter Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
On Thu, Apr 9, 2020 at 4:19 PM Alex Deucher wrote:
>
> On Thu, Apr 9, 2020 at 5:41 AM Daniel Vetter wrote:
> >
> > On Thu, Apr 9, 2020 at 10:54 AM Benjamin Herrenschmidt
> > wrote:
> > >
> > > On Wed, 2020-04-08 at 14:25 +0200, Daniel Vetter wrote:
> > > > On Wed, Apr 08, 2020 at 01:59:17PM +0200, Christoph Hellwig wrote:
> > > > > If this code was broken for non-coherent caches a crude powerpc hack
> > > > > isn't going to help anyone else. Remove the hack as it is the last
> > > > > user of __vmalloc passing a page protection flag other than
> > > > > PAGE_KERNEL.
> > > >
> > > > Well Ben added this to make stuff work on ppc, ofc the home grown dma
> > > > layer in drm from back then isn't going to work in other places. I guess
> > > > should have at least an ack from him, in case anyone still cares about
> > > > this on ppc. Adding Ben to cc.
> > >
> > > This was due to some drivers (radeon ?) trying to use vmalloc pages for
> > > coherent DMA, which means on those 4xx powerpc's need to be non-cached.
> > >
> > > There were machines using that (440 based iirc), though I honestly
> > > can't tell if anybody still uses any of it.
> >
> > agp subsystem still seems to happily do that (vmalloc memory for
> > device access), never having been ported to dma apis (or well
> > converted to iommu drivers, which they kinda are really). So I think
> > this all still works exactly as back then, even with the kms radeon
> > drivers. Question really is whether we have users left, and I have no
> > clue about that either.
> >
> > Now if these boxes didn't ever have agp then I think we can get away
> > with deleting this, since we've already deleted the legacy radeon
> > driver. And that one used vmalloc for everything. The new kms one does
> > use the dma-api if the gpu isn't connected through agp.
>
> All radeons have a built in remapping table to handle non-AGP systems.
> On the earlier radeons it wasn't quite as performant as AGP, but it
> was always more reliable because AGP is AGP. Maybe it's time to let
> AGP go?
I'd be very much in favour of that, if we can just use the integrated
gart and drop agp fast writes wobbliness on the floor. I think the
only other modern driver using agp would be nouveau at that point.
-Daniel
>
> Alex
>
> > -Daniel
> >
> > > Cheers,
> > > Ben.
> > >
> > > > -Daniel
> > > >
> > > > >
> > > > > Signed-off-by: Christoph Hellwig
> > > > > ---
> > > > > drivers/gpu/drm/drm_scatter.c | 11 +--
> > > > > 1 file changed, 1 insertion(+), 10 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/drm_scatter.c
> > > > > b/drivers/gpu/drm/drm_scatter.c
> > > > > index ca520028b2cb..f4e6184d1877 100644
> > > > > --- a/drivers/gpu/drm/drm_scatter.c
> > > > > +++ b/drivers/gpu/drm/drm_scatter.c
> > > > > @@ -43,15 +43,6 @@
> > > > >
> > > > > #define DEBUG_SCATTER 0
> > > > >
> > > > > -static inline void *drm_vmalloc_dma(unsigned long size)
> > > > > -{
> > > > > -#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE)
> > > > > - return __vmalloc(size, GFP_KERNEL,
> > > > > pgprot_noncached_wc(PAGE_KERNEL));
> > > > > -#else
> > > > > - return vmalloc_32(size);
> > > > > -#endif
> > > > > -}
> > > > > -
> > > > > static void drm_sg_cleanup(struct drm_sg_mem * entry)
> > > > > {
> > > > > struct page *page;
> > > > > @@ -126,7 +117,7 @@ int drm_legacy_sg_alloc(struct drm_device *dev,
> > > > > void *data,
> > > > > return -ENOMEM;
> > > > > }
> > > > >
> > > > > - entry->virtual = drm_vmalloc_dma(pages << PAGE_SHIFT);
> > > > > + entry->virtual = vmalloc_32(pages << PAGE_SHIFT);
> > > > > if (!entry->virtual) {
> > > > > kfree(entry->busaddr);
> > > > > kfree(entry->pagelist);
> > > > > --
> > > > > 2.25.1
> > > > >
> > > >
> > > >
> > >
> >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
On Thu, Apr 9, 2020 at 10:54 AM Benjamin Herrenschmidt
wrote:
>
> On Wed, 2020-04-08 at 14:25 +0200, Daniel Vetter wrote:
> > On Wed, Apr 08, 2020 at 01:59:17PM +0200, Christoph Hellwig wrote:
> > > If this code was broken for non-coherent caches a crude powerpc hack
> > > isn't going to help anyone else. Remove the hack as it is the last
> > > user of __vmalloc passing a page protection flag other than PAGE_KERNEL.
> >
> > Well Ben added this to make stuff work on ppc, ofc the home grown dma
> > layer in drm from back then isn't going to work in other places. I guess
> > should have at least an ack from him, in case anyone still cares about
> > this on ppc. Adding Ben to cc.
>
> This was due to some drivers (radeon ?) trying to use vmalloc pages for
> coherent DMA, which means on those 4xx powerpc's need to be non-cached.
>
> There were machines using that (440 based iirc), though I honestly
> can't tell if anybody still uses any of it.
agp subsystem still seems to happily do that (vmalloc memory for
device access), never having been ported to dma apis (or well
converted to iommu drivers, which they kinda are really). So I think
this all still works exactly as back then, even with the kms radeon
drivers. Question really is whether we have users left, and I have no
clue about that either.
Now if these boxes didn't ever have agp then I think we can get away
with deleting this, since we've already deleted the legacy radeon
driver. And that one used vmalloc for everything. The new kms one does
use the dma-api if the gpu isn't connected through agp.
-Daniel
> Cheers,
> Ben.
>
> > -Daniel
> >
> > >
> > > Signed-off-by: Christoph Hellwig
> > > ---
> > > drivers/gpu/drm/drm_scatter.c | 11 +--
> > > 1 file changed, 1 insertion(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_scatter.c b/drivers/gpu/drm/drm_scatter.c
> > > index ca520028b2cb..f4e6184d1877 100644
> > > --- a/drivers/gpu/drm/drm_scatter.c
> > > +++ b/drivers/gpu/drm/drm_scatter.c
> > > @@ -43,15 +43,6 @@
> > >
> > > #define DEBUG_SCATTER 0
> > >
> > > -static inline void *drm_vmalloc_dma(unsigned long size)
> > > -{
> > > -#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE)
> > > - return __vmalloc(size, GFP_KERNEL, pgprot_noncached_wc(PAGE_KERNEL));
> > > -#else
> > > - return vmalloc_32(size);
> > > -#endif
> > > -}
> > > -
> > > static void drm_sg_cleanup(struct drm_sg_mem * entry)
> > > {
> > > struct page *page;
> > > @@ -126,7 +117,7 @@ int drm_legacy_sg_alloc(struct drm_device *dev, void
> > > *data,
> > > return -ENOMEM;
> > > }
> > >
> > > - entry->virtual = drm_vmalloc_dma(pages << PAGE_SHIFT);
> > > + entry->virtual = vmalloc_32(pages << PAGE_SHIFT);
> > > if (!entry->virtual) {
> > > kfree(entry->busaddr);
> > > kfree(entry->pagelist);
> > > --
> > > 2.25.1
> > >
> >
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
On Wed, Apr 08, 2020 at 01:59:17PM +0200, Christoph Hellwig wrote:
> If this code was broken for non-coherent caches a crude powerpc hack
> isn't going to help anyone else. Remove the hack as it is the last
> user of __vmalloc passing a page protection flag other than PAGE_KERNEL.
Well Ben added this to make stuff work on ppc, ofc the home grown dma
layer in drm from back then isn't going to work in other places. I guess
should have at least an ack from him, in case anyone still cares about
this on ppc. Adding Ben to cc.
-Daniel
>
> Signed-off-by: Christoph Hellwig
> ---
> drivers/gpu/drm/drm_scatter.c | 11 +--
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_scatter.c b/drivers/gpu/drm/drm_scatter.c
> index ca520028b2cb..f4e6184d1877 100644
> --- a/drivers/gpu/drm/drm_scatter.c
> +++ b/drivers/gpu/drm/drm_scatter.c
> @@ -43,15 +43,6 @@
>
> #define DEBUG_SCATTER 0
>
> -static inline void *drm_vmalloc_dma(unsigned long size)
> -{
> -#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE)
> - return __vmalloc(size, GFP_KERNEL, pgprot_noncached_wc(PAGE_KERNEL));
> -#else
> - return vmalloc_32(size);
> -#endif
> -}
> -
> static void drm_sg_cleanup(struct drm_sg_mem * entry)
> {
> struct page *page;
> @@ -126,7 +117,7 @@ int drm_legacy_sg_alloc(struct drm_device *dev, void
> *data,
> return -ENOMEM;
> }
>
> - entry->virtual = drm_vmalloc_dma(pages << PAGE_SHIFT);
> + entry->virtual = vmalloc_32(pages << PAGE_SHIFT);
> if (!entry->virtual) {
> kfree(entry->busaddr);
> kfree(entry->pagelist);
> --
> 2.25.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH 3/7] docs: fix broken references to text files
On Sat, Feb 22, 2020 at 10:00:03AM +0100, Mauro Carvalho Chehab wrote: > Several references got broken due to txt to ReST conversion. > > Several of them can be automatically fixed with: > > scripts/documentation-file-ref-check --fix > > Signed-off-by: Mauro Carvalho Chehab > --- > Documentation/admin-guide/kernel-parameters.txt | 10 +- > Documentation/filesystems/cifs/cifsroot.txt | 2 +- > Documentation/memory-barriers.txt| 2 +- > Documentation/process/submit-checklist.rst | 2 +- > .../translations/it_IT/process/submit-checklist.rst | 2 +- > Documentation/translations/ko_KR/memory-barriers.txt | 2 +- > .../translations/zh_CN/filesystems/sysfs.txt | 2 +- > .../translations/zh_CN/process/submit-checklist.rst | 2 +- > Documentation/virt/kvm/arm/pvtime.rst| 2 +- > Documentation/virt/kvm/devices/vcpu.rst | 2 +- > Documentation/virt/kvm/hypercalls.rst| 4 ++-- > arch/powerpc/include/uapi/asm/kvm_para.h | 2 +- > drivers/gpu/drm/Kconfig | 2 +- > drivers/gpu/drm/drm_ioctl.c | 2 +- These two look very correct. The patch that moved edid.rst seems to have not updated a lot of references :-/ Acked-by: Daniel Vetter > drivers/hwtracing/coresight/Kconfig | 2 +- > fs/fat/Kconfig | 8 > fs/fuse/Kconfig | 2 +- > fs/fuse/dev.c| 2 +- > fs/nfs/Kconfig | 2 +- > fs/overlayfs/Kconfig | 6 +++--- > include/linux/mm.h | 4 ++-- > include/uapi/linux/ethtool_netlink.h | 2 +- > include/uapi/rdma/rdma_user_ioctl_cmds.h | 2 +- > mm/gup.c | 12 ++-- > net/ipv4/Kconfig | 6 +++--- > net/ipv4/ipconfig.c | 2 +- > virt/kvm/arm/vgic/vgic-mmio-v3.c | 2 +- > virt/kvm/arm/vgic/vgic.h | 4 ++-- > 28 files changed, 47 insertions(+), 47 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt > b/Documentation/admin-guide/kernel-parameters.txt > index 8be1d0bbfd16..e0fe9f70d22b 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -957,7 +957,7 @@ > edid/1680x1050.bin, or edid/1920x1080.bin is given > and no file with the same name exists. Details and > instructions how to build your own EDID data are > - available in Documentation/driver-api/edid.rst. An EDID > + available in Documentation/admin-guide/edid.rst. An EDID > data set will only be used for a particular connector, > if its name and a colon are prepended to the EDID > name. Each connector may use a unique EDID data > @@ -1884,7 +1884,7 @@ > No delay > > ip= [IP_PNP] > - See Documentation/filesystems/nfs/nfsroot.txt. > + See Documentation/admin-guide/nfs/nfsroot.rst. > > ipcmni_extend [KNL] Extend the maximum number of unique System V > IPC identifiers from 32,768 to 16,777,216. > @@ -2863,13 +2863,13 @@ > Default value is 0. > > nfsaddrs= [NFS] Deprecated. Use ip= instead. > - See Documentation/filesystems/nfs/nfsroot.txt. > + See Documentation/admin-guide/nfs/nfsroot.rst. > > nfsroot=[NFS] nfs root filesystem for disk-less boxes. > - See Documentation/filesystems/nfs/nfsroot.txt. > + See Documentation/admin-guide/nfs/nfsroot.rst. > > nfsrootdebug[NFS] enable nfsroot debugging messages. > - See Documentation/filesystems/nfs/nfsroot.txt. > + See Documentation/admin-guide/nfs/nfsroot.rst. > > nfs.callback_nr_threads= > [NFSv4] set the total number of threads that the > diff --git a/Documentation/filesystems/cifs/cifsroot.txt > b/Documentation/filesystems/cifs/cifsroot.txt > index 0fa1a2c36a40..947b7ec6ce9e 100644 > --- a/Documentation/filesystems/cifs/cifsroot.txt > +++ b/Documentation/filesystems/cifs/cifsroot.txt > @@ -13,7 +13,7 @@ network by utilizing SMB or CIFS protocol. > > I
Re: [PATCH v3 00/23] mm/gup: track dma-pinned pages: FOLL_PIN, FOLL_LONGTERM
On Wed, Nov 13, 2019 at 11:12:10AM +0100, Jan Kara wrote: > On Wed 13-11-19 01:02:02, John Hubbard wrote: > > On 11/13/19 12:22 AM, Daniel Vetter wrote: > > ... > > > > > Why are we doing this? I think things got confused here someplace, as > > > > > > > > > > > > Because: > > > > > > > > a) These need put_page() calls, and > > > > > > > > b) there is no put_pages() call, but there is a release_pages() call > > > > that > > > > is, arguably, what put_pages() would be. > > > > > > > > > > > > > the comment still says: > > > > > > > > > > /** > > > > > * put_user_page() - release a gup-pinned page > > > > > * @page:pointer to page to be released > > > > > * > > > > > * Pages that were pinned via get_user_pages*() must be released via > > > > > * either put_user_page(), or one of the put_user_pages*() routines > > > > > * below. > > > > > > > > > > > > Ohhh, I missed those comments. They need to all be changed over to > > > > say "pages that were pinned via pin_user_pages*() or > > > > pin_longterm_pages*() must be released via put_user_page*()." > > > > > > > > The get_user_pages*() pages must still be released via put_page. > > > > > > > > The churn is due to a fairly significant change in strategy, whis > > > > is: instead of changing all get_user_pages*() sites to call > > > > put_user_page(), change selected sites to call pin_user_pages*() or > > > > pin_longterm_pages*(), plus put_user_page(). > > > > > > Can't we call this unpin_user_page then, for some symmetry? Or is that > > > even more churn? > > > > > > Looking from afar the naming here seems really confusing. > > > > > > That look from afar is valuable, because I'm too close to the problem to see > > how the naming looks. :) > > > > unpin_user_page() sounds symmetrical. It's true that it would cause more > > churn (which is why I started off with a proposal that avoids changing the > > names of put_user_page*() APIs). But OTOH, the amount of churn is > > proportional > > to the change in direction here, and it's really only 10 or 20 lines > > changed, > > in the end. > > > > So I'm open to changing to that naming. It would be nice to hear what others > > prefer, too... > > FWIW I'd find unpin_user_page() also better than put_user_page() as a > counterpart to pin_user_pages(). One more point from afar on pin/unpin: We use that a lot in graphics for permanently pinned graphics buffer objects. Which really only should be used for scanout. So at least graphics folks should have an appropriate mindset and try to make sure we don't overuse this stuff. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v3 00/23] mm/gup: track dma-pinned pages: FOLL_PIN, FOLL_LONGTERM
On Tue, Nov 12, 2019 at 10:10 PM John Hubbard wrote: > > On 11/12/19 12:38 PM, Jason Gunthorpe wrote: > > On Mon, Nov 11, 2019 at 04:06:37PM -0800, John Hubbard wrote: > >> Hi, > >> > >> The cover letter is long, so the more important stuff is first: > >> > >> * Jason, if you or someone could look at the the VFIO cleanup (patch 8) > >> and conversion to FOLL_PIN (patch 18), to make sure it's use of > >> remote and longterm gup matches what we discussed during the review > >> of v2, I'd appreciate it. > >> > >> * Also for Jason and IB: as noted below, in patch 11, I am (too?) boldly > >> converting from put_user_pages() to release_pages(). > > > > Why are we doing this? I think things got confused here someplace, as > > > Because: > > a) These need put_page() calls, and > > b) there is no put_pages() call, but there is a release_pages() call that > is, arguably, what put_pages() would be. > > > > the comment still says: > > > > /** > > * put_user_page() - release a gup-pinned page > > * @page:pointer to page to be released > > * > > * Pages that were pinned via get_user_pages*() must be released via > > * either put_user_page(), or one of the put_user_pages*() routines > > * below. > > > Ohhh, I missed those comments. They need to all be changed over to > say "pages that were pinned via pin_user_pages*() or > pin_longterm_pages*() must be released via put_user_page*()." > > The get_user_pages*() pages must still be released via put_page. > > The churn is due to a fairly significant change in strategy, whis > is: instead of changing all get_user_pages*() sites to call > put_user_page(), change selected sites to call pin_user_pages*() or > pin_longterm_pages*(), plus put_user_page(). Can't we call this unpin_user_page then, for some symmetry? Or is that even more churn? Looking from afar the naming here seems really confusing. -Daniel > That allows incrementally converting the kernel over to using the > new pin APIs, without taking on the huge risk of a big one-shot > conversion. > > So, I've ended up with one place that actually needs to get reverted > back to get_user_pages(), and that's the IB ODP code. > > > > > I feel like if put_user_pages() is not the correct way to undo > > get_user_pages() then it needs to be deleted. > > > > Yes, you're right. I'll fix the put_user_page comments() as described. > > > thanks, > > John Hubbard > NVIDIA -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH 09/19] drm/via: set FOLL_PIN via pin_user_pages_fast()
On Mon, Nov 04, 2019 at 11:20:38AM -0800, John Hubbard wrote: > On 11/4/19 10:10 AM, Daniel Vetter wrote: > > On Thu, Oct 31, 2019 at 04:36:28PM -0700, Ira Weiny wrote: > >> On Wed, Oct 30, 2019 at 03:49:20PM -0700, John Hubbard wrote: > >>> Convert drm/via to use the new pin_user_pages_fast() call, which sets > >>> FOLL_PIN. Setting FOLL_PIN is now required for code that requires > >>> tracking of pinned pages, and therefore for any code that calls > >>> put_user_page(). > >>> > >> > >> Reviewed-by: Ira Weiny > > > > No one's touching the via driver anymore, so feel free to merge this > > through whatever tree suits best (aka I'll drop this on the floor and > > forget about it now). > > > > Acked-by: Daniel Vetter > > > > OK, great. Yes, in fact, I'm hoping Andrew can just push the whole series > in through the mm tree, because that would allow it to be done in one > shot, in 5.5 btw is there more? We should have a bunch more userptr stuff in various drivers, so was really surprised that drm/via is the only thing in your series. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH 09/19] drm/via: set FOLL_PIN via pin_user_pages_fast()
On Thu, Oct 31, 2019 at 04:36:28PM -0700, Ira Weiny wrote: > On Wed, Oct 30, 2019 at 03:49:20PM -0700, John Hubbard wrote: > > Convert drm/via to use the new pin_user_pages_fast() call, which sets > > FOLL_PIN. Setting FOLL_PIN is now required for code that requires > > tracking of pinned pages, and therefore for any code that calls > > put_user_page(). > > > > Reviewed-by: Ira Weiny No one's touching the via driver anymore, so feel free to merge this through whatever tree suits best (aka I'll drop this on the floor and forget about it now). Acked-by: Daniel Vetter > > > Signed-off-by: John Hubbard > > --- > > drivers/gpu/drm/via/via_dmablit.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/via/via_dmablit.c > > b/drivers/gpu/drm/via/via_dmablit.c > > index 3db000aacd26..37c5e572993a 100644 > > --- a/drivers/gpu/drm/via/via_dmablit.c > > +++ b/drivers/gpu/drm/via/via_dmablit.c > > @@ -239,7 +239,7 @@ via_lock_all_dma_pages(drm_via_sg_info_t *vsg, > > drm_via_dmablit_t *xfer) > > vsg->pages = vzalloc(array_size(sizeof(struct page *), vsg->num_pages)); > > if (NULL == vsg->pages) > > return -ENOMEM; > > - ret = get_user_pages_fast((unsigned long)xfer->mem_addr, > > + ret = pin_user_pages_fast((unsigned long)xfer->mem_addr, > > vsg->num_pages, > > vsg->direction == DMA_FROM_DEVICE ? FOLL_WRITE : 0, > > vsg->pages); > > -- > > 2.23.0 > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v2 02/24] drivers/video/fbdev: use ioremap_wc/wt() instead of __ioremap()
On Wed, Sep 12, 2018 at 03:58:17PM +, Christophe Leroy wrote:
> _PAGE_NO_CACHE is a platform specific flag. In addition, this flag
> is misleading because one would think it requests a noncached page
> whereas a noncached page is _PAGE_NO_CACHE | _PAGE_GUARDED
>
> _PAGE_NO_CACHE alone means write combined noncached page, so lets
> use ioremap_wc() instead.
>
> _PAGE_WRITETHRU is also platform specific flag. Use ioremap_wt()
> instead.
>
> Signed-off-by: Christophe Leroy
I wondered why this all showed up in my gfx inbox, then spotted this patch
here.
I trust you way more on the _PAGE_ flags than me, but this looks
reasonable.
Acked-by: Daniel Vetter
> ---
> drivers/video/fbdev/chipsfb.c| 3 +--
> drivers/video/fbdev/controlfb.c | 5 +
> drivers/video/fbdev/platinumfb.c | 5 +
> drivers/video/fbdev/valkyriefb.c | 12 ++--
> 4 files changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c
> index f103665cad43..40182ed85648 100644
> --- a/drivers/video/fbdev/chipsfb.c
> +++ b/drivers/video/fbdev/chipsfb.c
> @@ -27,7 +27,6 @@
> #include
> #include
> #include
> -#include
>
> #ifdef CONFIG_PMAC_BACKLIGHT
> #include
> @@ -401,7 +400,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const
> struct pci_device_id *ent)
> #endif /* CONFIG_PMAC_BACKLIGHT */
>
> #ifdef CONFIG_PPC
> - p->screen_base = __ioremap(addr, 0x20, _PAGE_NO_CACHE);
> + p->screen_base = ioremap_wc(addr, 0x20);
> #else
> p->screen_base = ioremap(addr, 0x20);
> #endif
> diff --git a/drivers/video/fbdev/controlfb.c b/drivers/video/fbdev/controlfb.c
> index 8d14b29aafea..9cb0ef7ac29e 100644
> --- a/drivers/video/fbdev/controlfb.c
> +++ b/drivers/video/fbdev/controlfb.c
> @@ -48,9 +48,7 @@
> #include
> #include
> #include
> -#include
> #include
> -#include
> #include
>
> #include "macmodes.h"
> @@ -715,8 +713,7 @@ static int __init control_of_init(struct device_node *dp)
> goto error_out;
> }
> /* map at most 8MB for the frame buffer */
> - p->frame_buffer = __ioremap(p->frame_buffer_phys, 0x80,
> - _PAGE_WRITETHRU);
> + p->frame_buffer = ioremap_wt(p->frame_buffer_phys, 0x80);
>
> if (!p->control_regs_phys ||
> !request_mem_region(p->control_regs_phys, p->control_regs_size,
> diff --git a/drivers/video/fbdev/platinumfb.c
> b/drivers/video/fbdev/platinumfb.c
> index 377d3399a3ad..bf6b7fb83cf4 100644
> --- a/drivers/video/fbdev/platinumfb.c
> +++ b/drivers/video/fbdev/platinumfb.c
> @@ -32,9 +32,7 @@
> #include
> #include
> #include
> -#include
> #include
> -#include
>
> #include "macmodes.h"
> #include "platinumfb.h"
> @@ -577,8 +575,7 @@ static int platinumfb_probe(struct platform_device* odev)
>
> /* frame buffer - map only 4MB */
> pinfo->frame_buffer_phys = pinfo->rsrc_fb.start;
> - pinfo->frame_buffer = __ioremap(pinfo->rsrc_fb.start, 0x40,
> - _PAGE_WRITETHRU);
> + pinfo->frame_buffer = ioremap_wt(pinfo->rsrc_fb.start, 0x40);
> pinfo->base_frame_buffer = pinfo->frame_buffer;
>
> /* registers */
> diff --git a/drivers/video/fbdev/valkyriefb.c
> b/drivers/video/fbdev/valkyriefb.c
> index 275fb98236d3..d51c3a8009cb 100644
> --- a/drivers/video/fbdev/valkyriefb.c
> +++ b/drivers/video/fbdev/valkyriefb.c
> @@ -54,13 +54,11 @@
> #include
> #include
> #include
> -#include
> #ifdef CONFIG_MAC
> #include
> #else
> #include
> #endif
> -#include
>
> #include "macmodes.h"
> #include "valkyriefb.h"
> @@ -318,7 +316,7 @@ static void __init valkyrie_choose_mode(struct
> fb_info_valkyrie *p)
> int __init valkyriefb_init(void)
> {
> struct fb_info_valkyrie *p;
> - unsigned long frame_buffer_phys, cmap_regs_phys, flags;
> + unsigned long frame_buffer_phys, cmap_regs_phys;
> int err;
> char *option = NULL;
>
> @@ -337,7 +335,6 @@ int __init valkyriefb_init(void)
> /* Hardcoded addresses... welcome to 68k Macintosh country :-) */
> frame_buffer_phys = 0xf900;
> cmap_regs_phys = 0x50f24000;
> - flags = IOMAP_NOCACHE_SER; /* IOMAP_WRITETHROUGH?? */
> #else /* ppc (!CONFIG_MAC) */
> {
> struct device_node *dp;
> @@ -354,7 +351,6 @@ int __init valkyriefb_init(void)
>
> frame_buffer_phys = r.start;
> cma
Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
On Wed, Sep 12, 2018 at 5:08 PM, Arnd Bergmann wrote:
> The .ioctl and .compat_ioctl file operations have the same prototype so
> they can both point to the same function, which works great almost all
> the time when all the commands are compatible.
>
> One exception is the s390 architecture, where a compat pointer is only
> 31 bit wide, and converting it into a 64-bit pointer requires calling
> compat_ptr(). Most drivers here will ever run in s390, but since we now
> have a generic helper for it, it's easy enough to use it consistently.
>
> I double-checked all these drivers to ensure that all ioctl arguments
> are used as pointers or are ignored, but are not interpreted as integer
> values.
>
> Signed-off-by: Arnd Bergmann
Acked-by: Daniel Vetter
At least for the drm and dma-buf bits.
-Daniel
> ---
> drivers/android/binder.c| 2 +-
> drivers/crypto/qat/qat_common/adf_ctl_drv.c | 2 +-
> drivers/dma-buf/dma-buf.c | 4 +---
> drivers/dma-buf/sw_sync.c | 2 +-
> drivers/dma-buf/sync_file.c | 2 +-
> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c| 2 +-
> drivers/hid/hidraw.c| 4 +---
> drivers/iio/industrialio-core.c | 2 +-
> drivers/infiniband/core/uverbs_main.c | 4 ++--
> drivers/media/rc/lirc_dev.c | 4 +---
> drivers/mfd/cros_ec_dev.c | 4 +---
> drivers/misc/vmw_vmci/vmci_host.c | 2 +-
> drivers/nvdimm/bus.c| 4 ++--
> drivers/nvme/host/core.c| 2 +-
> drivers/pci/switch/switchtec.c | 2 +-
> drivers/platform/x86/wmi.c | 2 +-
> drivers/rpmsg/rpmsg_char.c | 4 ++--
> drivers/sbus/char/display7seg.c | 2 +-
> drivers/sbus/char/envctrl.c | 4 +---
> drivers/scsi/3w-.c | 4 +---
> drivers/scsi/cxlflash/main.c| 2 +-
> drivers/scsi/esas2r/esas2r_main.c | 2 +-
> drivers/scsi/pmcraid.c | 4 +---
> drivers/staging/android/ion/ion.c | 4 +---
> drivers/staging/vme/devices/vme_user.c | 2 +-
> drivers/tee/tee_core.c | 2 +-
> drivers/usb/class/cdc-wdm.c | 2 +-
> drivers/usb/class/usbtmc.c | 4 +---
> drivers/video/fbdev/ps3fb.c | 2 +-
> drivers/virt/fsl_hypervisor.c | 2 +-
> fs/btrfs/super.c| 2 +-
> fs/ceph/dir.c | 2 +-
> fs/ceph/file.c | 2 +-
> fs/fuse/dev.c | 2 +-
> fs/notify/fanotify/fanotify_user.c | 2 +-
> fs/userfaultfd.c| 2 +-
> net/rfkill/core.c | 2 +-
> 37 files changed, 40 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index d58763b6b009..d2464f5759f8 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5576,7 +5576,7 @@ static const struct file_operations binder_fops = {
> .owner = THIS_MODULE,
> .poll = binder_poll,
> .unlocked_ioctl = binder_ioctl,
> - .compat_ioctl = binder_ioctl,
> + .compat_ioctl = generic_compat_ioctl_ptrarg,
> .mmap = binder_mmap,
> .open = binder_open,
> .flush = binder_flush,
> diff --git a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> index abc7a7f64d64..8ff77a70addc 100644
> --- a/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> +++ b/drivers/crypto/qat/qat_common/adf_ctl_drv.c
> @@ -68,7 +68,7 @@ static long adf_ctl_ioctl(struct file *fp, unsigned int
> cmd, unsigned long arg);
> static const struct file_operations adf_ctl_ops = {
> .owner = THIS_MODULE,
> .unlocked_ioctl = adf_ctl_ioctl,
> - .compat_ioctl = adf_ctl_ioctl,
> + .compat_ioctl = generic_compat_ioctl_ptrarg,
> };
>
> struct adf_ctl_drv_info {
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 13884474d158..a6d7dc4cf7e9 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -325,9 +325,7 @@ static const struct file_operations dma_buf_fops = {
> .llseek = dma_buf_llseek,
> .poll = dma_buf_poll,
> .unlocked_ioctl = dma_buf_ioctl,
> -#ifdef CONFIG_COMPAT
> - .compat_ioctl = dma_buf_ioctl,
> -#endif
> + .compat_ioctl = generic_compat_ioctl_ptrarg,
> };
>
> /*
> diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
> index 53c1d6d36a64..bc8105
Re: [PATCH] [RFC v2] Drop all 00-INDEX files from Documentation/
On Thu, Sep 6, 2018 at 6:01 PM, Steven Rostedt wrote: > On Thu, 6 Sep 2018 09:58:04 -0600 > Jonathan Corbet wrote: > >> Thanks, >> >> jon (who is increasingly inclined to apply this patch) > > As Colin Kaepernick now says... "Just do it!" > > ;-) +1 But I'm biased, I'm part of the party that is responsible for the new shiny documentation system ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH] fix double ;;s in code
t; > - aspace = NULL;;
> > + aspace = NULL;
> > }
> > pm_runtime_put_sync(&pdev->dev);
> > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > index 2c18996..0d95888 100644
> > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
> > @@ -461,7 +461,7 @@ void drm_sched_hw_job_reset(struct drm_gpu_scheduler
> > *sched, struct drm_sched_jo
> > {
> > struct drm_sched_job *s_job;
> > struct drm_sched_entity *entity, *tmp;
> > - int i;;
> > + int i;
> > spin_lock(&sched->job_list_lock);
> > list_for_each_entry_reverse(s_job, &sched->ring_mirror_list, node) {
> > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> > index 35a408d..99bc9bd 100644
> > --- a/drivers/iommu/intel-svm.c
> > +++ b/drivers/iommu/intel-svm.c
> > @@ -205,7 +205,7 @@ static void intel_flush_svm_range_dev (struct intel_svm
> > *svm, struct intel_svm_d
> > * for example, an "address" value of 0x12345f000 will
> > * flush from 0x12344 to 0x12347 (256KiB). */
> > unsigned long last = address + ((unsigned long)(pages -
> > 1) << VTD_PAGE_SHIFT);
> > - unsigned long mask = __rounddown_pow_of_two(address ^
> > last);;
> > + unsigned long mask = __rounddown_pow_of_two(address ^
> > last);
> > desc.high = QI_DEV_EIOTLB_ADDR((address & ~mask) |
> > (mask - 1)) | QI_DEV_EIOTLB_SIZE;
> > } else {
> > diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> > index b2eae33..f978edd 100644
> > --- a/drivers/md/raid1.c
> > +++ b/drivers/md/raid1.c
> > @@ -1108,7 +1108,7 @@ static void alloc_behind_master_bio(struct r1bio
> > *r1_bio,
> > bio_copy_data(behind_bio, bio);
> > skip_copy:
> > - r1_bio->behind_master_bio = behind_bio;;
> > + r1_bio->behind_master_bio = behind_bio;
> > set_bit(R1BIO_BehindIO, &r1_bio->state);
> > return;
> > diff --git a/drivers/soc/imx/gpc.c b/drivers/soc/imx/gpc.c
> > index 53f7275..cfb42f5 100644
> > --- a/drivers/soc/imx/gpc.c
> > +++ b/drivers/soc/imx/gpc.c
> > @@ -348,7 +348,7 @@ static int imx_gpc_old_dt_init(struct device *dev,
> > struct regmap *regmap,
> > if (i == 1) {
> > domain->supply = devm_regulator_get(dev, "pu");
> > if (IS_ERR(domain->supply))
> > - return PTR_ERR(domain->supply);;
> > + return PTR_ERR(domain->supply);
> > ret = imx_pgc_get_clocks(dev, domain);
> > if (ret)
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v2 2/2] vgaarb: Factor out EFI and fallback default device selection
On Wed, Oct 18, 2017 at 11:24:43AM +1100, Daniel Axtens wrote:
> Hi Daniel,
>
> >> Initially I wondered if this info printk could be moved into
> >> vga_arbiter_check_bridge_sharing(), but it's been separated out since
> >> 3448a19da479b ("vgaarb: use bridges to control VGA routing where
> >> possible."), and upon closer examination, it seems you can't be sure a
> >> device doesn't share a bridge until the end of the process, so this is
> >> indeed correct.
> >>
> >> Everything else also looks good to me.
> >>
> >> Reviewed-by: Daniel Axtens
> >
> > R-b for both patches? And ok with everyone if I pull this into drm-misc
> > for 4.15 (deadline is end of this week for feature-y stuff)?
>
> I had only intended it for patch 2, but I've now read over patch 1 to my
> satisfaction, so it too is:
>
> Reviewed-by: Daniel Axtens
Both applied for 4.15, thanks.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v2 2/2] vgaarb: Factor out EFI and fallback default device selection
On Tue, Oct 17, 2017 at 01:03:46PM +1100, Daniel Axtens wrote:
> Bjorn Helgaas writes:
>
> > The default VGA device is normally set in vga_arbiter_add_pci_device() when
> > we call it for the first enabled device that can be accessed with the
> > legacy VGA resources ([mem 0xa-0xb], etc.)
> >
> > That default device can be overridden by an EFI device that owns the boot
> > framebuffer. As a fallback, we can also select a VGA device that can't be
> > accessed via legacy VGA resources, or a VGA device that isn't even enabled.
> >
> > Factor out this EFI and fallback selection from vga_arb_device_init() into
> > a separate vga_arb_select_default_device() function. This doesn't change
> > any behavior, but it untangles the "bridge control possible" checking and
> > messages from the default device selection.
> >
> > Tested-by: Zhou Wang # D05 Hisi Hip07, Hip08
> > Signed-off-by: Bjorn Helgaas
> > ---
> > drivers/gpu/vga/vgaarb.c | 57
> > --
> > 1 file changed, 35 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> > index 8035e38d5110..d35d6d271f3f 100644
> > --- a/drivers/gpu/vga/vgaarb.c
> > +++ b/drivers/gpu/vga/vgaarb.c
> > @@ -1402,29 +1402,14 @@ static struct miscdevice vga_arb_device = {
> > MISC_DYNAMIC_MINOR, "vga_arbiter", &vga_arb_device_fops
> > };
> >
> > -static int __init vga_arb_device_init(void)
> > +static void __init vga_arb_select_default_device(void)
> > {
> > - int rc;
> > struct pci_dev *pdev;
> > struct vga_device *vgadev;
> >
> > - rc = misc_register(&vga_arb_device);
> > - if (rc < 0)
> > - pr_err("error %d registering device\n", rc);
> > -
> > - bus_register_notifier(&pci_bus_type, &pci_notifier);
> > -
> > - /* We add all pci devices satisfying vga class in the arbiter by
> > -* default */
> > - pdev = NULL;
> > - while ((pdev =
> > - pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> > - PCI_ANY_ID, pdev)) != NULL)
> > - vga_arbiter_add_pci_device(pdev);
> > -
> > +#if defined(CONFIG_X86) || defined(CONFIG_IA64)
> > list_for_each_entry(vgadev, &vga_list, list) {
> > struct device *dev = &vgadev->pdev->dev;
> > -#if defined(CONFIG_X86) || defined(CONFIG_IA64)
> > /*
> > * Override vga_arbiter_add_pci_device()'s I/O based detection
> > * as it may take the wrong device (e.g. on Apple system under
> > @@ -1461,12 +1446,8 @@ static int __init vga_arb_device_init(void)
> > vgaarb_info(dev, "overriding boot device\n");
> > vga_set_default_device(vgadev->pdev);
> > }
> > -#endif
> > - if (vgadev->bridge_has_one_vga)
> > - vgaarb_info(dev, "bridge control possible\n");
> > - else
> > - vgaarb_info(dev, "no bridge control possible\n");
> > }
> > +#endif
> >
> > if (!vga_default_device()) {
> > list_for_each_entry(vgadev, &vga_list, list) {
> > @@ -1492,6 +1473,38 @@ static int __init vga_arb_device_init(void)
> > vga_set_default_device(vgadev->pdev);
> > }
> > }
> > +}
> > +
> > +static int __init vga_arb_device_init(void)
> > +{
> > + int rc;
> > + struct pci_dev *pdev;
> > + struct vga_device *vgadev;
> > +
> > + rc = misc_register(&vga_arb_device);
> > + if (rc < 0)
> > + pr_err("error %d registering device\n", rc);
> > +
> > + bus_register_notifier(&pci_bus_type, &pci_notifier);
> > +
> > + /* We add all PCI devices satisfying VGA class in the arbiter by
> > +* default */
> > + pdev = NULL;
> > + while ((pdev =
> > + pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> > + PCI_ANY_ID, pdev)) != NULL)
> > + vga_arbiter_add_pci_device(pdev);
> > +
> > + list_for_each_entry(vgadev, &vga_list, list) {
> > + struct device *dev = &vgadev->pdev->dev;
> > +
> > + if (vgadev->bridge_has_one_vga)
> > + vgaarb_info(dev, "bridge control possible\n");
> > + else
> > + vgaarb_info(dev, "no bridge control possible\n");
> > + }
>
> Initially I wondered if this info printk could be moved into
> vga_arbiter_check_bridge_sharing(), but it's been separated out since
> 3448a19da479b ("vgaarb: use bridges to control VGA routing where
> possible."), and upon closer examination, it seems you can't be sure a
> device doesn't share a bridge until the end of the process, so this is
> indeed correct.
>
> Everything else also looks good to me.
>
> Reviewed-by: Daniel Axtens
R-b for both patches? And ok with everyone if I pull this into drm-misc
for 4.15 (deadline is end of this week for feature-y stuff)?
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v3 0/3] Split default display handling out from VGA arbiter
On Mon, Sep 18, 2017 at 7:49 AM, Daniel Axtens wrote: > Hi all, > > The merge window is well over by now - is there anything else I can do > to get this series to a mergeable state? I'm not particularly across the > rules for drm-misc, so please let me know if there are things I need to > be doing. drm-misc is always open for feature merging, but the patch series lacks full r-b by someone with clue on this stuff (i.e. probably not me). Pls find someone who can do the in-depth review. Just an Ack feels a bit thin. -Daniel > > Regards, > Daniel > > Daniel Axtens writes: > >> This patch set: >> >> - splits the default display handling out from VGA arbiter, into its >>own file and behind its own Kconfig option (and gives the functions >>better names). >> >> - adds extra detection of default devices. To be nominated, the vga >>arbiter and platform hooks must not have nominated a default. A >>card will then only be nominated if it has a driver attached and >>has IO or memory decoding enabled. >> >> - adds relevant documentation. >> >> The practical impact of this is improved X autoconfiguration on some >> arm64 systems. >> >> Changes in v3: >> >> - Add documentation - thanks Daniel Vetter for pointing it out. >> >> - Clarify explanations. Thanks to everyone for continuing to bear >>with my incomplete understanding of PCI and provide some clarity. >> >> - Split refactoring and adding functionality. >> >> Changes in v2: https://www.spinics.net/lists/linux-pci/msg64007.html >> >> Drop all the powerpc patches. [explanation snipped] >> >> v1: https://www.spinics.net/lists/linux-pci/msg63581.html >> >> Regards, >> Daniel >> >> Daniel Axtens (3): >> drm: split default display handler out of VGA arbiter >> drm: add fallback default device detection >> drm: documentation for default display device >> >> Documentation/gpu/default_display.rst | 93 +++ >> Documentation/gpu/index.rst | 1 + >> arch/ia64/pci/fixup.c | 6 +- >> arch/powerpc/kernel/pci-common.c | 6 +- >> arch/x86/pci/fixup.c | 6 +- >> arch/x86/video/fbdev.c| 4 +- >> drivers/gpu/vga/Kconfig | 12 +++ >> drivers/gpu/vga/Makefile | 1 + >> drivers/gpu/vga/default_display.c | 163 >> ++ >> drivers/gpu/vga/vga_switcheroo.c | 8 +- >> drivers/gpu/vga/vgaarb.c | 61 +++-- >> drivers/pci/pci-sysfs.c | 4 +- >> include/linux/default_display.h | 44 + >> include/linux/vgaarb.h| 15 >> 14 files changed, 344 insertions(+), 80 deletions(-) >> create mode 100644 Documentation/gpu/default_display.rst >> create mode 100644 drivers/gpu/vga/default_display.c >> create mode 100644 include/linux/default_display.h >> >> -- >> 2.11.0 > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH v2 1/1] Split VGA default device handler out of VGA arbiter
info(&pdev->dev, "setting as boot VGA device\n");
> vga_set_default_device(pdev);
> @@ -704,7 +669,7 @@ static bool vga_arbiter_del_pci_device(struct pci_dev
> *pdev)
> goto bail;
> }
>
> - if (vga_default == pdev)
> + if (vga_default_device() == pdev)
> vga_set_default_device(NULL);
>
> if (vgadev->decodes & (VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM))
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 2f3780b50723..c174b427ea2b 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -27,7 +27,7 @@
> #include
> #include
> #include
> -#include
> +#include
> #include
> #include
> #include "pci.h"
> diff --git a/include/linux/vga_default.h b/include/linux/vga_default.h
> new file mode 100644
> index ..78df6a2a194c
> --- /dev/null
> +++ b/include/linux/vga_default.h
> @@ -0,0 +1,44 @@
> +/*
> + * vga_default.h: What is the default/boot PCI VGA device?
> + *
> + * (C) Copyright 2005 Benjamin Herrenschmidt
> + * (C) Copyright 2007 Paulo R. Zanoni
> + * (C) Copyright 2007, 2009 Tiago Vignatti
> + * (C) Copyright 2017 Canonical Ltd. (Author: Daniel Axtens
> )
> + *
> + * (License from vgaarb.h)
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#ifndef LINUX_VGA_DEFAULT_H
> +#define LINUX_VGA_DEFAULT_H
> +
> +struct pci_dev;
> +
> +#ifdef CONFIG_VGA_DEFAULT
> +extern struct pci_dev *vga_default_device(void);
> +extern void vga_set_default_device(struct pci_dev *pdev);
> +#else
> +static inline struct pci_dev *vga_default_device(void) { return NULL; };
> +static inline void vga_set_default_device(struct pci_dev *pdev) { };
> +#endif
> +
> +#endif
> diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
> index ee162e3e879b..953e1955efbe 100644
> --- a/include/linux/vgaarb.h
> +++ b/include/linux/vgaarb.h
> @@ -42,12 +42,6 @@
> #define VGA_RSRC_NORMAL_IO 0x04
> #define VGA_RSRC_NORMAL_MEM0x08
>
> -/* Passing that instead of a pci_dev to use the system "default"
> - * device, that is the one used by vgacon. Archs will probably
> - * have to provide their own vga_default_device();
> - */
> -#define VGA_DEFAULT_DEVICE (NULL)
> -
> struct pci_dev;
>
> /* For use by clients */
> @@ -122,14 +116,6 @@ extern void vga_put(struct pci_dev *pdev, unsigned int
> rsrc);
> #endif
>
>
> -#ifdef CONFIG_VGA_ARB
> -extern struct pci_dev *vga_default_device(void);
> -extern void vga_set_default_device(struct pci_dev *pdev);
> -#else
> -static inline struct pci_dev *vga_default_device(void) { return NULL; };
> -static inline void vga_set_default_device(struct pci_dev *pdev) { };
> -#endif
> -
> /*
> * Architectures should define this if they have several
> * independent PCI domains that can afford concurrent VGA
> --
> 2.11.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: clean up and modularize arch dma_mapping interface
On Thu, Jun 08, 2017 at 03:25:25PM +0200, Christoph Hellwig wrote: > Hi all, > > for a while we have a generic implementation of the dma mapping routines > that call into per-arch or per-device operations. But right now there > still are various bits in the interfaces where don't clearly operate > on these ops. This series tries to clean up a lot of those (but not all > yet, but the series is big enough). It gets rid of the DMA_ERROR_CODE > way of signaling failures of the mapping routines from the > implementations to the generic code (and cleans up various drivers that > were incorrectly using it), and gets rid of the ->set_dma_mask routine > in favor of relying on the ->dma_capable method that can be used in > the same way, but which requires less code duplication. > > Btw, we don't seem to have a tree every-growing amount of common dma > mapping code, and given that I have a fair amount of all over the tree > work in that area in my plate I'd like to start one. Any good reason > to that? Anyone willing to volunteer as co maintainer? > > The whole series is also available in git: > > git://git.infradead.org/users/hch/misc.git dma-map Ack for the 2 drm patches, but I can also pick them up through drm-misc if you prefer that (but then it'll be 4.14). -Daniel > > Gitweb: > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-map > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v2 7/7] uapi: export all headers under uapi directories
On Fri, Jan 06, 2017 at 10:43:59AM +0100, Nicolas Dichtel wrote: > Regularly, when a new header is created in include/uapi/, the developer > forgets to add it in the corresponding Kbuild file. This error is usually > detected after the release is out. > > In fact, all headers under uapi directories should be exported, thus it's > useless to have an exhaustive list. > > After this patch, the following files, which were not exported, are now > exported (with make headers_install_all): > asm-unicore32/shmparam.h > asm-unicore32/ucontext.h > asm-hexagon/shmparam.h > asm-mips/ucontext.h > asm-mips/hwcap.h > asm-mips/reg.h > drm/vgem_drm.h > drm/armada_drm.h > drm/omap_drm.h > drm/etnaviv_drm.h > asm-tile/shmparam.h > asm-blackfin/shmparam.h > asm-blackfin/ucontext.h > asm-powerpc/perf_regs.h > rdma/qedr-abi.h > asm-parisc/kvm_para.h > asm-openrisc/shmparam.h > asm-nios2/kvm_para.h > asm-nios2/ucontext.h > asm-sh/kvm_para.h > asm-sh/ucontext.h > asm-xtensa/kvm_para.h > asm-avr32/kvm_para.h > asm-m32r/kvm_para.h > asm-h8300/shmparam.h > asm-h8300/ucontext.h > asm-metag/kvm_para.h > asm-metag/shmparam.h > asm-metag/ucontext.h > asm-m68k/kvm_para.h > asm-m68k/shmparam.h > linux/bcache.h > linux/kvm.h > linux/kvm_para.h > linux/kfd_ioctl.h > linux/cryptouser.h > linux/kcm.h > linux/kcov.h > linux/seg6_iptunnel.h > linux/stm.h > linux/genwqe > linux/genwqe/.install > linux/genwqe/genwqe_card.h > linux/genwqe/..install.cmd > linux/seg6.h > linux/cifs > linux/cifs/.install > linux/cifs/cifs_mount.h > linux/cifs/..install.cmd > linux/auto_dev-ioctl.h > > Thanks to Julien Floret for the tip to get all > subdirs with a pure makefile command. > > Signed-off-by: Nicolas Dichtel Makes lots of sense. Acked-by: Daniel Vetter > --- > Documentation/kbuild/makefiles.txt | 41 ++- > arch/alpha/include/uapi/asm/Kbuild | 41 --- > arch/arc/include/uapi/asm/Kbuild| 3 - > arch/arm/include/uapi/asm/Kbuild| 17 - > arch/arm64/include/uapi/asm/Kbuild | 18 -- > arch/avr32/include/uapi/asm/Kbuild | 20 -- > arch/blackfin/include/uapi/asm/Kbuild | 17 - > arch/c6x/include/uapi/asm/Kbuild| 8 - > arch/cris/include/uapi/arch-v10/arch/Kbuild | 5 - > arch/cris/include/uapi/arch-v32/arch/Kbuild | 3 - > arch/cris/include/uapi/asm/Kbuild | 43 +-- > arch/frv/include/uapi/asm/Kbuild| 33 -- > arch/h8300/include/uapi/asm/Kbuild | 28 -- > arch/hexagon/include/asm/Kbuild | 3 - > arch/hexagon/include/uapi/asm/Kbuild| 13 - > arch/ia64/include/uapi/asm/Kbuild | 45 --- > arch/m32r/include/uapi/asm/Kbuild | 31 -- > arch/m68k/include/uapi/asm/Kbuild | 24 -- > arch/metag/include/uapi/asm/Kbuild | 8 - > arch/microblaze/include/uapi/asm/Kbuild | 32 -- > arch/mips/include/uapi/asm/Kbuild | 37 --- > arch/mn10300/include/uapi/asm/Kbuild| 32 -- > arch/nios2/include/uapi/asm/Kbuild | 4 +- > arch/openrisc/include/asm/Kbuild| 3 - > arch/openrisc/include/uapi/asm/Kbuild | 8 - > arch/parisc/include/uapi/asm/Kbuild | 28 -- > arch/powerpc/include/uapi/asm/Kbuild| 45 --- > arch/s390/include/uapi/asm/Kbuild | 52 --- > arch/score/include/asm/Kbuild | 4 - > arch/score/include/uapi/asm/Kbuild | 32 -- > arch/sh/include/uapi/asm/Kbuild | 23 -- > arch/sparc/include/uapi/asm/Kbuild | 48 --- > arch/tile/include/asm/Kbuild| 3 - > arch/tile/include/uapi/arch/Kbuild | 17 - > arch/tile/include/uapi/asm/Kbuild | 19 +- > arch/unicore32/include/uapi/asm/Kbuild | 6 - > arch/x86/include/uapi/asm/Kbuild| 59 > arch/xtensa/include/uapi/asm/Kbuild | 23 -- > include/Kbuild | 2 - > include/asm-generic/Kbuild.asm | 1 - > include/scsi/fc/Kbuild | 0 > include/uapi/Kbuild | 15 - > include/uapi/asm-generic/Kbuild | 36 --- > include/uapi/asm-generic/Kbuild.asm | 62 ++-- > include/uapi/drm/Kbuild | 22 -- > include/uapi/linux/Kbuild | 482 > > include/uapi/linux/android/Kbuild | 2 - > include/uapi/linux/byteorder/Kbuild | 3 - > include/uapi/linux/caif/Kbuild | 3 - > include/uapi/linux/can/Kbuild | 6 - > include/uapi/linux/dvb/Kbuild | 9 - > include/u
Re: [PATCH v2 2/5] firmware: annotate thou shalt not request fw on init or probe
Cutting down since a lot of this is probably better discussed at ks/lpc. Aside, if you want to check out Chris Wilson's work on our new depency handling, it's called kfence. https://lkml.org/lkml/2016/7/17/37 On Thu, Aug 25, 2016 at 9:41 PM, Luis R. Rodriguez wrote: >> > So .. I agree, let's avoid the hacks. Patches welcomed. >> >> Hm, this is a definite change of tack - back when I discussed this >> with Greg about 2 ks ago it sounded like "don't do this". The only >> thing we need is some way to wait for rootfs before we do the >> request_firmware. Everything else we handle already in the kernel. > > OK so lets just get this userspace event done, and we're set. Ah great. As long as that special wait-for-rootfs-pls firmware request is still allowed, i915&gfx folks will be happy. We will call it from probe though ;-) but all from our own workers. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH v2 2/5] firmware: annotate thou shalt not request fw on init or probe
On Wed, Aug 24, 2016 at 10:39 PM, Luis R. Rodriguez wrote: > On Wed, Aug 24, 2016 at 08:55:55AM +0200, Daniel Vetter wrote: >> On Fri, Jun 17, 2016 at 12:54 AM, Luis R. Rodriguez >> wrote: >> > Thou shalt not make firmware calls early on init or probe. > > <-- snip --> > >> > There are 4 offenders at this time: >> > >> > mcgrof@ergon ~/linux-next (git::20160609)$ export >> > COCCI=scripts/coccinelle/api/request_firmware.cocci >> > mcgrof@ergon ~/linux-next (git::20160609)$ make coccicheck MODE=report >> > >> > drivers/fmc/fmc-fakedev.c: ERROR: driver call request firmware call on its >> > init routine on line 321. >> > drivers/fmc/fmc-write-eeprom.c: ERROR: driver call request firmware call >> > on its probe routine on line 136. >> > drivers/tty/serial/rp2.c: ERROR: driver call request firmware call on its >> > probe routine on line 796. >> > drivers/tty/serial/ucc_uart.c: ERROR: driver call request firmware call on >> > its probe routine on line 1246. >> >> Plus all gpu drivers which need firmware. And yes we must load them at >> probe > > Do you have an upstream driver in mind that does this ? Is it on device > drier module probe or a DRM subsystem specific probe call of some sort ? i915 is the one I care about for obvious reasons ;-) It's all from the pci device probe function, but nested really deeply. >> because people are generally pissed when they boot their machine >> and the screen goes black. On top of that a lot of people want their >> gpu drivers to be built-in, but can't ship the firmware blobs in the >> kernel image because gpl. Yep, there's a bit a contradiction there ... > > Can they use initramfs for this ? Apparently that's also uncool with the embedded folks. Tbh I don't know exactly why. Also I thought initramfs is available only after built-in drivers have finished loading, but maybe that changed. > Also just curious -- as with other subsystems, is it possible to load > a generic driver first, say vesa, and then a more enhanced one later ? > I am not saying this is ideal or am I suggesting this, I'd just like > to know the feasibility of this. Some users want a fully running gfx stack 2s after power-on. There's not even enough time to load an uefi or vga driver first. i915 directly initializes the gpu from power-on state on those. >> I think what would work is loading the different subsystems of the >> driver in parallel (we already do that largely) > > Init level stuff is actually pretty synchronous, and in fact both > init and probe are called serially. There are a few subsystems which > have been doing things a bit differently, but these are exceptions. > > When you say we already do this largely, can you describe a bit more > precisely what you mean ? Oh, this isn't subsystems as in linux device/driver model, but different parts within the driver. We fire up a bunch of struct work to get various bits done asynchronously. >>, and then if one >> firmware blob isn't there yet simply stall that async worker until it >> shows up. > > Is this an existing framework or do you mean if we add something > generic to do this async loading of subsystems ? normal workers, and flush_work and friends to sync up. We have some really fancy ideas for essentially async init tasks that can declare their depencies systemd-unit file style, but that's only in the prototype stage. We need this (eventually) since handling the ordering correctly is getting unwieldy. But again just struct work launched from the main pci probe function. >> But the answers I've gotten thus far from request_firmware() >> folks (well at least Greg) is don't do that ... > > Well in this patch set I'm adding myself as a MAINTAINER and I've > been extending the firmware API recently to help with a few new > features I want, I've been wanting to hear more feedback from > other's needs and I had actually not gotten much -- except > only recently with the usermode helper and reasons why some > folks thought they could not use direct firmware loading from > the fs. I'm keen to hear or more use cases and needs specially if > they have to do with improving boot time and asynchronous boot. > >> Is that problem still somewhere on the radar? > > Not mine. > >> Atm there's various >> wait_for_rootfs hacks out there floating in vendor/product trees. > > This one I've heard about recently, and I suggested two possible > solutions, with a preference to a simply notification of when > rootfs is available from userspace. > >> "Avoid at all costs"
Re: [PATCH v2 2/5] firmware: annotate thou shalt not request fw on init or probe
On Fri, Jun 17, 2016 at 12:54 AM, Luis R. Rodriguez wrote: > Thou shalt not make firmware calls early on init or probe. > > systemd already ripped support out for the usermode helper > a while ago, there are still users that require the usermode helper, > however systemd's use of the usermode helper exacerbated a long > lasting issue of the fact that we have many drivers that load > firmware on init or probe. Independent of the usermode helper > loading firmware on init or probe is a bad idea for a few reasons. > > When the firmware is read directly from the filesystem by the kernel, > if the driver is built-in to the kernel the firmware may not yet be > available, for such uses one could use initramfs and stuff the firmware > inside, or one also use CONFIG_EXTRA_FIRMWARE; however not all distributions > use this, as such generally one cannot count on this. There is another > corner cases to consider, since we are accessing the firmware directly folks > cannot expect new found firmware on a filesystem after we switch off from > an initramfs with pivot_root(). > > Supporting such situations is possible today but fixing it for good is > really complex due to the large amount of variablity in the boot up > process. > > Instead just document the expectations properly and add a grammar rule to > enable folks to check / validate / police if drivers are using the request > firmware API early on init or probe. > > The SmPL rule used to check for the probe routine is loose and is > currently defined through a regexp, that can easily be extended to any > other known bus probe routine names. It also uses the new Python > iteration support which allows us to backtrack from a request_firmware*() > call back to a possible probe or init, iteratively. Without iteration > we would only be able to get reports for callers who directly use the > request_firmware*() API on the initial probe or init routine. > > There are 4 offenders at this time: > > mcgrof@ergon ~/linux-next (git::20160609)$ export > COCCI=scripts/coccinelle/api/request_firmware.cocci > mcgrof@ergon ~/linux-next (git::20160609)$ make coccicheck MODE=report > > drivers/fmc/fmc-fakedev.c: ERROR: driver call request firmware call on its > init routine on line 321. > drivers/fmc/fmc-write-eeprom.c: ERROR: driver call request firmware call on > its probe routine on line 136. > drivers/tty/serial/rp2.c: ERROR: driver call request firmware call on its > probe routine on line 796. > drivers/tty/serial/ucc_uart.c: ERROR: driver call request firmware call on > its probe routine on line 1246. Plus all gpu drivers which need firmware. And yes we must load them at probe because people are generally pissed when they boot their machine and the screen goes black. On top of that a lot of people want their gpu drivers to be built-in, but can't ship the firmware blobs in the kernel image because gpl. Yep, there's a bit a contradiction there ... I think what would work is loading the different subsystems of the driver in parallel (we already do that largely), and then if one firmware blob isn't there yet simply stall that async worker until it shows up. But the answers I've gotten thus far from request_firmware() folks (well at least Greg) is don't do that ... Is that problem still somewhere on the radar? Atm there's various wait_for_rootfs hacks out there floating in vendor/product trees. "Avoid at all costs" sounds like upstream prefers to not care about android/cros in those case (yes I know most arm socs don't need firmware, which would make it a problem fo just be a subset of all devices). -Daniel > I checked and verified all these are valid reports. This list also matches > the same list as in 20150805, so we have fortunately not gotten worse. > Let's keep it that way and also fix these drivers. > > v2: changes from v1 [0]: > > o This now supports iteration, this extends our coverage on the report > > o Update documentation and commit log to accept the fate of not being > able to remove the usermode helper. > > [0] > https://lkml.kernel.org/r/1440811107-861-1-git-send-email-mcg...@do-not-panic.com > > Cc: Alessandro Rubini > Cc: Kevin Cernekee > Cc: Daniel Vetter > Cc: Mimi Zohar > Cc: David Woodhouse > Cc: Kees Cook > Cc: Dmitry Torokhov > Cc: Ming Lei > Cc: Jonathan Corbet > Cc: Julia Lawall > Cc: Gilles Muller > Cc: Nicolas Palix > Cc: Thierry Martinez > Cc: Michal Marek > Cc: co...@systeme.lip6.fr > Cc: Alessandro Rubini > Cc: Kevin Cernekee > Cc: Greg Kroah-Hartman > Cc: Jiri Slaby > Cc: linux-ser...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: linux-ser...@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Signed-
Re: [PATCH v5 00/44] dma-mapping: Use unsigned long for dma_attrs
On Thu, Jun 30, 2016 at 10:23:39AM +0200, Krzysztof Kozlowski wrote:
> Hi,
>
>
> This is fifth approach for replacing struct dma_attrs with unsigned
> long.
>
> The main patch (1/44) doing the change is split into many subpatches
> for easier review (2-42). They should be squashed together when
> applying.
For all the drm driver patches:
Acked-by: Daniel Vetter
Should I pull these in through drm-misc, or do you prefer to merge them
through a special topic branch (with everything else) instead on your own?
-Daniel
>
>
> Rebased on v4.7-rc5.
>
> For easier testing the patchset is available here:
> repo: https://github.com/krzk/linux
> branch: for-next/dma-attrs-const-v5
>
>
> Changes since v4
>
> 1. Collect some acks. Still need more.
> 2. Minor fixes pointed by Robin Murphy.
> 3. Applied changes from Bart Van Assche's comment.
> 4. More tests and builds (using https://www.kernel.org/pub/tools/crosstool/).
>
>
> Changes since v3
>
> 1. Collect some acks.
> 2. Drop wrong patch 1/45 ("powerpc: dma-mapping: Don't hard-code
>the value of DMA_ATTR_WEAK_ORDERING").
> 3. Minor fix pointed out by Michael Ellerman.
>
>
> Changes since v2
>
> 1. Follow Christoph Hellwig's comments (don't use BIT add
>documentation, remove dma_get_attr).
>
>
> Rationale
> =
> The dma-mapping core and the implementations do not change the
> DMA attributes passed by pointer. Thus the pointer can point to const
> data. However the attributes do not have to be a bitfield. Instead
> unsigned long will do fine:
>
> 1. This is just simpler. Both in terms of reading the code and setting
>attributes. Instead of initializing local attributes on the stack
>and passing pointer to it to dma_set_attr(), just set the bits.
>
> 2. It brings safeness and checking for const correctness because the
>attributes are passed by value.
>
>
> Best regards,
> Krzysztof
>
>
> Krzysztof Kozlowski (44):
> dma-mapping: Use unsigned long for dma_attrs
> alpha: dma-mapping: Use unsigned long for dma_attrs
> arc: dma-mapping: Use unsigned long for dma_attrs
> ARM: dma-mapping: Use unsigned long for dma_attrs
> arm64: dma-mapping: Use unsigned long for dma_attrs
> avr32: dma-mapping: Use unsigned long for dma_attrs
> blackfin: dma-mapping: Use unsigned long for dma_attrs
> c6x: dma-mapping: Use unsigned long for dma_attrs
> cris: dma-mapping: Use unsigned long for dma_attrs
> frv: dma-mapping: Use unsigned long for dma_attrs
> drm/exynos: dma-mapping: Use unsigned long for dma_attrs
> drm/mediatek: dma-mapping: Use unsigned long for dma_attrs
> drm/msm: dma-mapping: Use unsigned long for dma_attrs
> drm/nouveau: dma-mapping: Use unsigned long for dma_attrs
> drm/rockship: dma-mapping: Use unsigned long for dma_attrs
> infiniband: dma-mapping: Use unsigned long for dma_attrs
> iommu: dma-mapping: Use unsigned long for dma_attrs
> [media] dma-mapping: Use unsigned long for dma_attrs
> xen: dma-mapping: Use unsigned long for dma_attrs
> swiotlb: dma-mapping: Use unsigned long for dma_attrs
> powerpc: dma-mapping: Use unsigned long for dma_attrs
> video: dma-mapping: Use unsigned long for dma_attrs
> x86: dma-mapping: Use unsigned long for dma_attrs
> iommu: intel: dma-mapping: Use unsigned long for dma_attrs
> h8300: dma-mapping: Use unsigned long for dma_attrs
> hexagon: dma-mapping: Use unsigned long for dma_attrs
> ia64: dma-mapping: Use unsigned long for dma_attrs
> m68k: dma-mapping: Use unsigned long for dma_attrs
> metag: dma-mapping: Use unsigned long for dma_attrs
> microblaze: dma-mapping: Use unsigned long for dma_attrs
> mips: dma-mapping: Use unsigned long for dma_attrs
> mn10300: dma-mapping: Use unsigned long for dma_attrs
> nios2: dma-mapping: Use unsigned long for dma_attrs
> openrisc: dma-mapping: Use unsigned long for dma_attrs
> parisc: dma-mapping: Use unsigned long for dma_attrs
> misc: mic: dma-mapping: Use unsigned long for dma_attrs
> s390: dma-mapping: Use unsigned long for dma_attrs
> sh: dma-mapping: Use unsigned long for dma_attrs
> sparc: dma-mapping: Use unsigned long for dma_attrs
> tile: dma-mapping: Use unsigned long for dma_attrs
> unicore32: dma-mapping: Use unsigned long for dma_attrs
> xtensa: dma-mapping: Use unsigned long for dma_attrs
> dma-mapping: Remove dma_get_attr
> dma-mapping: Document the DMA attributes next to the declaration
>
> Documentation/DMA-API.txt | 33 +++---
> Documentation/DMA-attributes.txt | 2 +-
> arch/alpha/inclu
Re: [PATCH v9 2/3] kernel.h: add to_user_ptr()
On Thu, Mar 17, 2016 at 02:33:50PM -0700, Joe Perches wrote:
> On Thu, 2016-03-17 at 18:19 -0300, Gustavo Padovan wrote:
> > 2016-03-17 Joe Perches :
> > > On Thu, 2016-03-17 at 16:50 -0400, Rob Clark wrote:
> > > > On Thu, Mar 17, 2016 at 4:40 PM, Joe Perches wrote:
> > > []
> > > > > It's a name that seems like it should be a straightforward
> > > > > cast of a kernel pointer to a __user pointer like:
> > > > >
> > > > > static inline void __user *to_user_ptr(void *p)
> > > > > {
> > > > > return (void __user *)p;
> > > > > }
> > > > ahh, ok. I guess I was used to using it in the context of ioctl
> > > > structs.. in that context u64 -> (void __user *) made more sense.
> > > >
> > > > Maybe uapi_to_ptr()? (ok, not super-creative.. maybe someone has a
> > > > better idea)
> > > Maybe u64_to_user_ptr?
> > That is a good name. If everyone agrees I can resend this patch
> > changing it to u64_to_user_ptr. Then should we still keep it on
> > kernel.h?
>
> I've no particular opinion about location,
> but maybe compat.h might be appropriate.
>
> Maybe add all variants:
>
> void __user *u32_to_user_ptr(u32 val)
> void __user *u64_to_user_ptr(u64 val)
> u32 user_ptr_to_u32(void __user *p)
> u64 user_ptr_to_u64(void __user *p)
>
> Maybe there's something about 32 bit userspace on
> 64 OS that should be done too.
Tbh I really don't think we should add 32bit variants and encourage the
mispractice of having 32bit user ptrs in ioctl structs and stuff. Anyway,
just my bikeshed on top ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] drivers: depend on instead of select BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO
On Wed, Oct 22, 2014 at 11:02:26AM +0300, Tomi Valkeinen wrote: > On 18/10/14 00:13, Jani Nikula wrote: > > Documentation/kbuild/kconfig-language.txt warns to use select with care, > > and in general use select only for non-visible symbols and for symbols > > with no dependencies, because select will force a symbol to a value > > without visiting the dependencies. > > > > Select has become particularly problematic, interdependently, with the > > BACKLIGHT_CLASS_DEVICE and ACPI_VIDEO configs. For example: > > > > scripts/kconfig/conf --randconfig Kconfig > > KCONFIG_SEED=0x48312B00 > > warning: (DRM_RADEON && DRM_NOUVEAU && DRM_I915 && DRM_GMA500 && > > DRM_SHMOBILE && DRM_TILCDC && FB_BACKLIGHT && FB_MX3 && USB_APPLEDISPLAY > > && FB_OLPC_DCON && ASUS_LAPTOP && SONY_LAPTOP && THINKPAD_ACPI && > > EEEPC_LAPTOP && ACPI_CMPC && SAMSUNG_Q10) selects BACKLIGHT_CLASS_DEVICE > > which has unmet direct dependencies (HAS_IOMEM && BACKLIGHT_LCD_SUPPORT) > > warning: (DRM_RADEON && DRM_NOUVEAU && DRM_I915 && DRM_GMA500 && > > DRM_SHMOBILE && DRM_TILCDC && FB_BACKLIGHT && FB_MX3 && USB_APPLEDISPLAY > > && FB_OLPC_DCON && ASUS_LAPTOP && SONY_LAPTOP && THINKPAD_ACPI && > > EEEPC_LAPTOP && ACPI_CMPC && SAMSUNG_Q10) selects BACKLIGHT_CLASS_DEVICE > > which has unmet direct dependencies (HAS_IOMEM && BACKLIGHT_LCD_SUPPORT) > > > > With tristates it's possible to end up selecting FOO=y depending on > > BAR=m in the config, which gets discovered at build time, not config > > time, like reported in the thread referenced below. > > > > Do the following to fix the dependencies: > > > > * Depend on instead of select BACKLIGHT_CLASS_DEVICE everywhere. Drop > > select BACKLIGHT_LCD_SUPPORT in such cases, as it's a dependency of > > BACKLIGHT_CLASS_DEVICE. > > > > * Remove config FB_BACKLIGHT altogether, and replace it with a > > dependency on BACKLIGHT_CLASS_DEVICE. All configs selecting > > FB_BACKLIGHT select or depend on FB anyway, so we can simplify. > > > > * Depend on (ACPI && ACPI_VIDEO) || ACPI=n in several places instead of > > selecting ACPI_VIDEO and a number of its dependencies if ACPI is > > enabled. This is tied to backlight, as ACPI_VIDEO depends on > > BACKLIGHT_CLASS_DEVICE. > > > > * Replace a couple of select INPUT/VT with depends as it seemed to be > > necessary. > > While doing 'depends on' instead of 'select' is an "easy" fix for this, > I do dislike it quite a bit. It's a major pain to go around the kernel > config, trying to find all the dependencies that a particular driver > wants. If I need fb-foobar, I should just be able to enable it, instead > of first searching and selecting its minor dependencies individually. > > So, not a NACK, but a "isn't there an another way to fix this?". > > Looking at backlight... BACKLIGHT_LCD_SUPPORT seems to be a "meta" > option, it only enables a Kconfig submenu. > > So I think we could just remove the whole BACKLIGHT_LCD_SUPPORT option. > But if we do that, all the items in drivers/video/backlight/Kconfig with > default 'y' or 'm' would get enabled by default, so I think we should > remove the 'default's from that file. That makes sense in any case, as I > don't see why "HP Jornada 700 series LCD Driver" should be "default y". > > BACKLIGHT_CLASS_DEVICE doesn't depend on anything except > BACKLIGHT_LCD_SUPPORT, so after removing BACKLIGHT_LCD_SUPPORT it should > be safe to 'select' BACKLIGHT_CLASS_DEVICE. > > BACKLIGHT_CLASS_DEVICE could be made a hidden option, and the drivers in > drivers/video/backlight/Kconfig which are under BACKLIGHT_CLASS_DEVICE > could be made to select BACKLIGHT_CLASS_DEVICE instead. > > That doesn't exactly fix anything, but I think it makes sense as > BACKLIGHT_CLASS_DEVICE is something that's selected from all around the > kernel, so it should be a selectable "library" instead of a Kconfig menu > option. > > I didn't look at the ACPI_VIDEO side, so no idea how messy that is. If we want to make BACKLIGHT_CLASS_DEVICE into a library thing then I guess we could do that, but we must then also drag it out of all the other meta options to make sure it's always available. No need I think to ditch the entire BACKLIGHT_LCD_SUPPORT meta option. And then everyone could select it. The problem is if you mix depends and select Kconfig falls over and starts to see loops where there are none. So you really can't ever mix them for the same symbol. I think if we get the BACKLIGHT_CLASS_DEVICE out of the picture with that ACPI_VIDEO should be fixable with some select ACPI_VIDEO if ACPI or so. Currently that doesn't work because of the backlight class knob and select being broken. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: TTM placement & caching issue/questions
On Thu, Sep 04, 2014 at 09:44:04AM +0200, Thomas Hellstrom wrote: > Last time I tested, (and it seems like Michel is on the same track), > writing with the CPU to write-combined memory was substantially faster > than writing to cached memory, with the additional side-effect that CPU > caches are left unpolluted. > > Moreover (although only tested on Intel's embedded chipsets), texturing > from cpu-cache-coherent PCI memory was a real GPU performance hog > compared to texturing from non-snooped memory. Hence, whenever a buffer > could be classified as GPU-read-only (or almost at least), it should be > placed in write-combined memory. Just a quick comment since this explicitly referes to intel chips: On desktop/laptop chips with the big shared l3/l4 caches it's the other way round. Cached uploads are substantially faster than wc and not using coherent access is a severe perf hit for texturing. I guess the hw guys worked really hard to hide the snooping costs so that the gpu can benefit from the massive bandwidth these caches can provide. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
