Messages in this thread | | | From | Wei Hu <> | Subject | RE: [PATCH] video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs. | Date | Fri, 25 Oct 2019 08:09:34 +0000 |
| |
Thanks for the review. Please see my response inline.
> > + select DMA_CMA > > Thіs needs to be > > select DMA_CMA if HAVE_DMA_CONTIGUOUS > > > +#include <linux/dma-contiguous.h> > > > + /* Allocate from CMA */ > > + // request_pages = (request_size >> PAGE_SHIFT) + 1; > > + request_pages = (round_up(request_size, PAGE_SIZE) >> PAGE_SHIFT); > > + page = dma_alloc_from_contiguous(NULL, request_pages, 0, false); > > dma_alloc_from_contiguous is an internal helper, you must use it > through dma_alloc_coherent and pass a struct device to that function. >
Can I directly use cma_alloc() and cma_release() in this case? The contiguous memory allocated is just for virtual framebuffer device, not for any DMA operation. I think using dma_alloc_coherent() might be a bit of overkill.
> > + if (!gen2vm) { > > + pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT, > > + PCI_DEVICE_ID_HYPERV_VIDEO, NULL); > > + if (!pdev) { > > + pr_err("Unable to find PCI Hyper-V video\n"); > > + return -ENODEV; > > + } > > + } > > Please actually implement a pci_driver instead of hacks like this. >
I don't quite follow this. What do you mean implementing a pci_driver in this case?
> > + par->need_docopy = false; > > + goto getmem1; > > + } else { > > No need for an else after a goto. Thanks. Will do.
Wei
| |