On Tue, Jul 27, 2021 at 03:17:35PM +0200, Mark Kettenis wrote:
> > Date: Tue, 27 Jul 2021 11:38:20 +1000
> > From: Jonathan Gray <[email protected]>
> >
> > On Mon, Jul 26, 2021 at 08:45:24AM +0100, Edd Barrett wrote:
> > > Hi,
> > >
> > > I upgraded my snapshot last week and the machine no longer suspends. I
> > > don't know exactly which snap caused this, as I hadn't been upgrading
> > > snaps very frequently.
> > >
> > > Upon running `zzz` from a terminal emulator, the screen goes black, but
> > > the machine never enters sleep state.
> > >
> > > Upon hard-resetting the machine, fsck runs, so the disks can't have been
> > > synced.
> > >
> > > I don't have a serial port on this machine, and I don't think I can
> > > force console output out of a usb-serial adapter (can I?).
> > >
> > > dmesg follows, but please let me know what other info might be useful.
> > >
> > > Thanks.
> >
> > on a t500 running amd64 with
> > radeondrm0: RV635
> >
> > on suspend I see
> > ttm_copy_io_ttm_page: stub
> > [TTM] Buffer eviction failed
> > uhub2 detached
> > ugen0 detached
> > ugen1 detached
> > uhub3 detached
> > uhub4 detached
> > uhub8 detached
> > video0 detached
> > uvideo0 detached
> > uhub0 detached
> > uhub5 detached
> > uhub6 detached
> > uhub7 detached
> > uhub1 detached
> >
> > but it does suspend and resume
> >
> > a t42 running i386 with
> > radeondrm0: RV200
> > does not hit the ttm_copy_io_ttm_page / Buffer eviction failed path
> >
> > with the following diff t500 suspends quicker here does it change
> > anything on your machine?
> >
> > Index: sys/dev/pci/drm/include/linux/highmem.h
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/pci/drm/include/linux/highmem.h,v
> > retrieving revision 1.3
> > diff -u -p -r1.3 highmem.h
> > --- sys/dev/pci/drm/include/linux/highmem.h 14 Jun 2020 15:20:07 -0000
> > 1.3
> > +++ sys/dev/pci/drm/include/linux/highmem.h 27 Jul 2021 01:26:38 -0000
> > @@ -54,6 +54,21 @@ kunmap_atomic(void *addr)
> > #endif
> > }
> >
> > -#endif /* defined(__i386__) || defined(__amd64__) */
> > +#else /* !defined(__i386__) && !defined(__amd64__) */
> > +
> > +static inline void *
> > +kmap_atomic(struct vm_page *pg)
> > +{
> > + return NULL;
> > +}
> > +
> > +static inline void
> > +kunmap_atomic(void *addr)
> > +{
> > +}
> > +
> > +#endif
> > +
> > +#define kmap_atomic_prot(a, p) kmap_atomic(a)
> >
> > #endif
> > Index: sys/dev/pci/drm/ttm/ttm_bo_util.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/pci/drm/ttm/ttm_bo_util.c,v
> > retrieving revision 1.28
> > diff -u -p -r1.28 ttm_bo_util.c
> > --- sys/dev/pci/drm/ttm/ttm_bo_util.c 7 Jul 2021 02:38:37 -0000
> > 1.28
> > +++ sys/dev/pci/drm/ttm/ttm_bo_util.c 27 Jul 2021 00:40:32 -0000
> > @@ -186,9 +186,6 @@ static int ttm_copy_io_ttm_page(struct t
> > unsigned long page,
> > pgprot_t prot)
> > {
> > - STUB();
> > - return -ENOSYS;
> > -#ifdef notyet
> > struct vm_page *d = ttm->pages[page];
> > void *dst;
> >
> > @@ -205,16 +202,12 @@ static int ttm_copy_io_ttm_page(struct t
> > kunmap_atomic(dst);
> >
> > return 0;
> > -#endif
> > }
> >
> > static int ttm_copy_ttm_io_page(struct ttm_tt *ttm, void *dst,
> > unsigned long page,
> > pgprot_t prot)
> > {
> > - STUB();
> > - return -ENOSYS;
> > -#ifdef notyet
> > struct vm_page *s = ttm->pages[page];
> > void *src;
> >
> > @@ -231,7 +224,6 @@ static int ttm_copy_ttm_io_page(struct t
> > kunmap_atomic(src);
> >
> > return 0;
> > -#endif
> > }
> >
> > int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>
> So here is a better diff that does respect the page protections in
> kmap_atomic_prot(). That might be relevant since pages may need to be
> mapped uncached.
>
> I think we can get away with having only a single reserved virtual
> address for kmap_atomic_prot(), at least as long as we run the drm
> code under the kernel lock. But I added some KASSERTs to make sure
> there isn't any concurrent use of the reserved virtual address.
>
> Edd, Alf, can you verifyu that this version fixes the issue as well?
>
Hello,
the new diff works too, thx!
Alf
> Jonathan, this probably needs a bit of testing on inteldrm(4) as well.
>
>
> Index: dev/pci/drm/drm_linux.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/drm/drm_linux.c,v
> retrieving revision 1.81
> diff -u -p -r1.81 drm_linux.c
> --- dev/pci/drm/drm_linux.c 26 Jul 2021 06:24:22 -0000 1.81
> +++ dev/pci/drm/drm_linux.c 27 Jul 2021 13:10:59 -0000
> @@ -571,6 +571,29 @@ kunmap_va(void *addr)
> #endif
> }
>
> +vaddr_t kmap_atomic_va;
> +int kmap_atomic_inuse;
> +
> +void *
> +kmap_atomic_prot(struct vm_page *pg, pgprot_t prot)
> +{
> + KASSERT(!kmap_atomic_inuse);
> +
> + kmap_atomic_inuse = 1;
> + pmap_kenter_pa(kmap_atomic_va, VM_PAGE_TO_PHYS(pg) | prot,
> + PROT_READ | PROT_WRITE);
> + return (void *)kmap_atomic_va;
> +}
> +
> +void
> +kunmap_atomic(void *addr)
> +{
> + KASSERT(kmap_atomic_inuse);
> +
> + pmap_kremove(kmap_atomic_va, PAGE_SIZE);
> + kmap_atomic_inuse = 0;
> +}
> +
> void *
> vmap(struct vm_page **pages, unsigned int npages, unsigned long flags,
> pgprot_t prot)
> @@ -2348,6 +2371,9 @@ drm_linux_init(void)
>
> pool_init(&idr_pool, sizeof(struct idr_entry), 0, IPL_TTY, 0,
> "idrpl", NULL);
> +
> + kmap_atomic_va =
> + (vaddr_t)km_alloc(PAGE_SIZE, &kv_any, &kp_none, &kd_waitok);
> }
>
> void
> Index: dev/pci/drm/include/linux/highmem.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/drm/include/linux/highmem.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 highmem.h
> --- dev/pci/drm/include/linux/highmem.h 14 Jun 2020 15:20:07 -0000
> 1.3
> +++ dev/pci/drm/include/linux/highmem.h 27 Jul 2021 13:11:00 -0000
> @@ -21,39 +21,20 @@
> #include <sys/param.h>
> #include <uvm/uvm_extern.h>
> #include <linux/uaccess.h>
> +#include <asm/pgtable.h>
>
> void *kmap(struct vm_page *);
> -void kunmap_va(void *addr);
> +void kunmap_va(void *addr);
>
> #define kmap_to_page(ptr) (ptr)
>
> -#if defined(__i386__) || defined(__amd64__)
> +void *kmap_atomic_prot(struct vm_page *, pgprot_t);
> +void kunmap_atomic(void *);
>
> static inline void *
> kmap_atomic(struct vm_page *pg)
> {
> - vaddr_t va;
> -
> -#if defined (__HAVE_PMAP_DIRECT)
> - va = pmap_map_direct(pg);
> -#else
> - extern vaddr_t pmap_tmpmap_pa(paddr_t);
> - va = pmap_tmpmap_pa(VM_PAGE_TO_PHYS(pg));
> -#endif
> - return (void *)va;
> + return kmap_atomic_prot(pg, PAGE_KERNEL);
> }
> -
> -static inline void
> -kunmap_atomic(void *addr)
> -{
> -#if defined (__HAVE_PMAP_DIRECT)
> - pmap_unmap_direct((vaddr_t)addr);
> -#else
> - extern void pmap_tmpunmap_pa(void);
> - pmap_tmpunmap_pa();
> -#endif
> -}
> -
> -#endif /* defined(__i386__) || defined(__amd64__) */
>
> #endif
> Index: dev/pci/drm/ttm/ttm_bo_util.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/drm/ttm/ttm_bo_util.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 ttm_bo_util.c
> --- dev/pci/drm/ttm/ttm_bo_util.c 7 Jul 2021 02:38:37 -0000 1.28
> +++ dev/pci/drm/ttm/ttm_bo_util.c 27 Jul 2021 13:11:00 -0000
> @@ -186,9 +186,6 @@ static int ttm_copy_io_ttm_page(struct t
> unsigned long page,
> pgprot_t prot)
> {
> - STUB();
> - return -ENOSYS;
> -#ifdef notyet
> struct vm_page *d = ttm->pages[page];
> void *dst;
>
> @@ -205,16 +202,12 @@ static int ttm_copy_io_ttm_page(struct t
> kunmap_atomic(dst);
>
> return 0;
> -#endif
> }
>
> static int ttm_copy_ttm_io_page(struct ttm_tt *ttm, void *dst,
> unsigned long page,
> pgprot_t prot)
> {
> - STUB();
> - return -ENOSYS;
> -#ifdef notyet
> struct vm_page *s = ttm->pages[page];
> void *src;
>
> @@ -231,7 +224,6 @@ static int ttm_copy_ttm_io_page(struct t
> kunmap_atomic(src);
>
> return 0;
> -#endif
> }
>
> int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>
