Re: [Nouveau] [PATCH 04/22] mm: don't clear ->mapping in hmm_devmem_free
On Thu 13-06-19 11:43:07, Christoph Hellwig wrote:
> ->mapping isn't even used by HMM users, and the field at the same offset
> in the zone_device part of the union is declared as pad. (Which btw is
> rather confusing, as DAX uses ->pgmap and ->mapping from two different
> sides of the union, but DAX doesn't use hmm_devmem_free).
>
> Signed-off-by: Christoph Hellwig
I cannot really judge here but setting mapping here without any comment
is quite confusing. So if this is safe to remove then it is certainly an
improvement.
> ---
> mm/hmm.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 0c62426d1257..e1dc98407e7b 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -1347,8 +1347,6 @@ static void hmm_devmem_free(struct page *page, void
> *data)
> {
> struct hmm_devmem *devmem = data;
>
> - page->mapping = NULL;
> -
> devmem->ops->free(devmem, page);
> }
>
> --
> 2.20.1
--
Michal Hocko
SUSE Labs
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [PATCH 03/22] mm: remove hmm_devmem_add_resource
On Thu 13-06-19 11:43:06, Christoph Hellwig wrote:
> This function has never been used since it was first added to the kernel
> more than a year and a half ago, and if we ever grow a consumer of the
> MEMORY_DEVICE_PUBLIC infrastructure it can easily use devm_memremap_pages
> directly now that we've simplified the API for it.
>
> Signed-off-by: Christoph Hellwig
Acked-by: Michal Hocko
> ---
> include/linux/hmm.h | 3 ---
> mm/hmm.c| 54 -
> 2 files changed, 57 deletions(-)
>
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 4867b9da1b6c..5761a39221a6 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -688,9 +688,6 @@ struct hmm_devmem {
> struct hmm_devmem *hmm_devmem_add(const struct hmm_devmem_ops *ops,
> struct device *device,
> unsigned long size);
> -struct hmm_devmem *hmm_devmem_add_resource(const struct hmm_devmem_ops *ops,
> -struct device *device,
> -struct resource *res);
>
> /*
> * hmm_devmem_page_set_drvdata - set per-page driver data field
> diff --git a/mm/hmm.c b/mm/hmm.c
> index ff2598eb7377..0c62426d1257 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -1445,58 +1445,4 @@ struct hmm_devmem *hmm_devmem_add(const struct
> hmm_devmem_ops *ops,
> return devmem;
> }
> EXPORT_SYMBOL_GPL(hmm_devmem_add);
> -
> -struct hmm_devmem *hmm_devmem_add_resource(const struct hmm_devmem_ops *ops,
> -struct device *device,
> -struct resource *res)
> -{
> - struct hmm_devmem *devmem;
> - void *result;
> - int ret;
> -
> - if (res->desc != IORES_DESC_DEVICE_PUBLIC_MEMORY)
> - return ERR_PTR(-EINVAL);
> -
> - dev_pagemap_get_ops();
> -
> - devmem = devm_kzalloc(device, sizeof(*devmem), GFP_KERNEL);
> - if (!devmem)
> - return ERR_PTR(-ENOMEM);
> -
> - init_completion(>completion);
> - devmem->pfn_first = -1UL;
> - devmem->pfn_last = -1UL;
> - devmem->resource = res;
> - devmem->device = device;
> - devmem->ops = ops;
> -
> - ret = percpu_ref_init(>ref, _devmem_ref_release,
> - 0, GFP_KERNEL);
> - if (ret)
> - return ERR_PTR(ret);
> -
> - ret = devm_add_action_or_reset(device, hmm_devmem_ref_exit,
> - >ref);
> - if (ret)
> - return ERR_PTR(ret);
> -
> - devmem->pfn_first = devmem->resource->start >> PAGE_SHIFT;
> - devmem->pfn_last = devmem->pfn_first +
> -(resource_size(devmem->resource) >> PAGE_SHIFT);
> - devmem->page_fault = hmm_devmem_fault;
> -
> - devmem->pagemap.type = MEMORY_DEVICE_PUBLIC;
> - devmem->pagemap.res = *devmem->resource;
> - devmem->pagemap.page_free = hmm_devmem_free;
> - devmem->pagemap.altmap_valid = false;
> - devmem->pagemap.ref = >ref;
> - devmem->pagemap.data = devmem;
> - devmem->pagemap.kill = hmm_devmem_ref_kill;
> -
> - result = devm_memremap_pages(devmem->device, >pagemap);
> - if (IS_ERR(result))
> - return result;
> - return devmem;
> -}
> -EXPORT_SYMBOL_GPL(hmm_devmem_add_resource);
> #endif /* CONFIG_DEVICE_PRIVATE || CONFIG_DEVICE_PUBLIC */
> --
> 2.20.1
--
Michal Hocko
SUSE Labs
Re: [Nouveau] [PATCH 18/22] mm: mark DEVICE_PUBLIC as broken
On Thu 13-06-19 11:43:21, Christoph Hellwig wrote: > The code hasn't been used since it was added to the tree, and doesn't > appear to actually be usable. Mark it as BROKEN until either a user > comes along or we finally give up on it. I would go even further and simply remove all the DEVICE_PUBLIC code. > Signed-off-by: Christoph Hellwig Anyway Acked-by: Michal Hocko > --- > mm/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mm/Kconfig b/mm/Kconfig > index 0d2ba7e1f43e..406fa45e9ecc 100644 > --- a/mm/Kconfig > +++ b/mm/Kconfig > @@ -721,6 +721,7 @@ config DEVICE_PRIVATE > config DEVICE_PUBLIC > bool "Addressable device memory (like GPU memory)" > depends on ARCH_HAS_HMM > + depends on BROKEN > select HMM > select DEV_PAGEMAP_OPS > > -- > 2.20.1 -- Michal Hocko SUSE Labs ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [PATCH 05/22] mm: export alloc_pages_vma
On Thu 13-06-19 11:43:08, Christoph Hellwig wrote: > noveau is currently using this through an odd hmm wrapper, and I plan > to switch it to the real thing later in this series. > > Signed-off-by: Christoph Hellwig > --- > mm/mempolicy.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index 01600d80ae01..f9023b5fba37 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -2098,6 +2098,7 @@ alloc_pages_vma(gfp_t gfp, int order, struct > vm_area_struct *vma, > out: > return page; > } > +EXPORT_SYMBOL_GPL(alloc_pages_vma); All allocator exported symbols are EXPORT_SYMBOL, what is a reason to have this one special? -- Michal Hocko SUSE Labs
[Nouveau] [Bug 75985] [NVC1] HDMI audio device only visible after rescan
https://bugs.freedesktop.org/show_bug.cgi?id=75985 --- Comment #78 from Lukas Wunner --- (In reply to Daniel Drake from comment #77) > MLTF presumably means multifunction and it's exactly the bit we've been > working with. But I haven't yet managed to get _PS0 to run this code. I get > to the GGIV(0x01080001) call, but it returns 0, so the bit doesn't get set. > > I tried understanding what GGIV() does but nothing is clear there. It ends > up reading bit 1 from physical memory address 0xfdac0408 which is under: > pci_bus :00: resource 21 [mem 0xfd00-0xfe7f window] > and I can't immediately spot any ACPI code that writes to that address. GGIV is a method name used by many vendors to read a GPIO pin. "Get GPIO Input Value" or something like that. Quite likely the GPIO pin in question is attached to HPD of an HDMI or DP port. So if an external display is attached, GGIV(0x01080001) should return 1 and the HDA is exposed, else it's hidden. If the GPIO pin in question is on the PCH then you can download the spec for the PCH from Intel's website to verify that the MMIO space at 0x01080001 is a GPIO block. The GPIO pin could also be on the Nvidia card itself, in that case physical address 0x01080001 would belong to a resource of the GPU's PCI device. -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 75985] [NVC1] HDMI audio device only visible after rescan
https://bugs.freedesktop.org/show_bug.cgi?id=75985
--- Comment #77 from Daniel Drake ---
Created attachment 144596
--> https://bugs.freedesktop.org/attachment.cgi?id=144596=edit
Acer Predator G3-572 acpidump
Martin Lopatář, thanks for the analysis above, sorry I missed those details
before!
Checking on my Acer Predator G3-572 (acpidump attached here), I can see what
you're referring to in SSDT5:
OperationRegion (PCNV, SystemMemory, \_SB.PCI0.PEG0.PEGP.EBAS, 0x1000)
Field (PCNV, AnyAcc, NoLock, Preserve)
{
Offset (0x488),
, 25,
MLTF, 1
}
Method (_PS0, 0, NotSerialized) // _PS0: Power State 0
{
If (DGOS)
{
If ((\_SB.PCI0.PEG0.PEGP.DPCS != Zero))
{
\_SB.PCI0.PEG0.PEGP._ON ()
DGOS = Zero
}
}
ElseIf ((\_SB.PCI0.PEG0.DVID != 0x))
{
If ((GGIV (0x01080001) == Zero))
{
MLTF = Zero
}
Else
{
MLTF = One
\_SB.PCI0.PEG0.PEGP.NASV = \_SB.PCI0.PEG0.PEGP.DSSV
}
}
}
MLTF presumably means multifunction and it's exactly the bit we've been working
with. But I haven't yet managed to get _PS0 to run this code. I get to the
GGIV(0x01080001) call, but it returns 0, so the bit doesn't get set.
I looked for some obvious connection to stuff in _DSM but I can't spot
anything. Can you clarify exactly what you saw that links _DSM and _PS0
together, and share your acpidump?
I tried understanding what GGIV() does but nothing is clear there. It ends up
reading bit 1 from physical memory address 0xfdac0408 which is under:
pci_bus :00: resource 21 [mem 0xfd00-0xfe7f window]
and I can't immediately spot any ACPI code that writes to that address.
--
You are receiving this mail because:
You are the assignee for the bug.___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] dev_pagemap related cleanups v2
On Wed, Jun 19, 2019 at 03:19:23PM -0300, Jason Gunthorpe wrote: > > Just make sure that when you backmerge v5.2-rc5 you have a clear > > reason in the merge commit message about why you needed to do it. > > While needless rebasing is top of the pet peeve list, second place, as > > I found out, is mystery merges without explanations. > > Yes, I always describe the merge commits. Linus also particular about > having *good reasons* for merges. > > This is why I can't fix the hmm.git to have rc5 until I have patches > to apply.. > > Probbaly I will just put CH's series on rc5 and merge it with the > cover letter as the merge message. This avoid both rebasing and gives > purposeful merges. Fine with me. My series right now is on top of the rdma/hmm branch. There is a trivial conflict that is solved by doing so, as my series removes documentation that is fixed up there a bit. There is another trivial conflict with your pending series as they remove code next to each other in hmm.git. ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
