Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

16 participants
3178 discussions

Re: [Linaro-mm-sig] CDF meeting @FOSDEM report

by Stéphane Marchesin

On Tue, Feb 5, 2013 at 2:27 PM, Laurent Pinchart <laurent.pinchart(a)ideasonboard.com> wrote: > Hello, > > We've hosted a CDF meeting at the FOSDEM on Sunday morning. Here's a summary > of the discussions. > > I would like to start with a big thank to UrLab, the ULB university hacker > space, for providing us with a meeting room. > > The meeting would of course not have been successful without the wide range of > participants, so I also want to thank all the people who woke up on Sunday > morning to attend the meeting :-) > > (The CC list is pretty long, please let me know - by private e-mail in order > not to spam the list - if you would like not to receive future CDF-related e- > mails directly) > > 0. Abbreviations > ---------------- > > DBI - Display Bus Interface, a parallel video control and data bus that > transmits data using parallel data, read/write, chip select and address > signals, similarly to 8051-style microcontroller parallel busses. This is a > mixed video control and data bus. > > DPI - Display Pixel Interface, a parallel video data bus that transmits data > using parallel data, h/v sync and clock signals. This is a video data bus > only. > > DSI - Display Serial Interface, a serial video control and data bus that > transmits data using one or more differential serial lines. This is a mixed > video control and data bus. > > DT - Device Tree, a representation of a hardware system as a tree of physical > devices with associated properties. > > SFI - Simple Firmware Interface, a lightweight method for firmware to export > static tables to the operating system. Those tables can contain display device > topology information. > > VBT - Video BIOS Table, a block of data residing in the video BIOS that can > contain display device topology information. > > 1. Goals > -------- > > The meeting started with a brief discussion about the CDF goals. > > Tomi Valkeinin and Tomasz Figa have sent RFC patches to show their views of > what CDF could/should be. Many others have provided very valuable feedback. > Given the early development stage propositions were sometimes contradictory, > and focused on different areas of interest. We have thus started the meeting > with a discussion about what CDF should try to achieve, and what it shouldn't. > > CDF has two main purposes. The original goal was to support display panels in > a platform- and subsystem-independent way. While mostly useful for embedded > systems, the emergence of platforms such as Intel Medfield and ARM-based PCs > that blends the embedded and PC worlds makes panel support useful for the PC > world as well. > > The second purpose is to provide a cross-subsystem interface to support video > encoders. The idea originally came from a generalisation of the original RFC > that supported panels only. While encoder support is considered as lower > priority than display panel support by developers focussed on display > controller driver (Intel, Renesas, ST Ericsson, TI), companies that produce > video encoders (Analog Devices, and likely others) don't share that point of > view and would like to provide a single encoder driver that can be used in > both KMS and V4L2 drivers. > > Both display panels and encoders are thus the target of a lot of attention, > depending on the audience. As long as none of them is forgotten in CDF, the > overall agreement was that focussing on panels first is acceptable. Care shall > be taken in that case to avoid any architecture that would make encoders > support difficult or impossible. > > 2. Subsystems > ------------- > > Display panels are used in conjunction with FBDEV and KMS drivers. There was > to the audience knowledge no V4L2 driver that needs to explicitly handle > display panels. Even though at least one V4L2 output drivers (omap_vout) can > output video to a display panel, it does so in conjunction with the KMS and/or > FBDEV APIs that handle panel configuration. Panels are thus not exposed to > V4L2 drivers. > > Encoders, on the other hand, are widely used in the V4L2 subsystem. Many V4L2 > devices output video in either analog (Composite, S-Video, VGA) or digital > (DVI, HDMI) way. > > Display panel drivers don't need to be shared with the V4L2 subsystem. > Furthermore, as the general opinion during the meeting was that the FBDEV > subsystem should be considered as legacy and deprecate in the future, > restricting panel support to KMS hasn't been considered by anyone as an issue. > KMS will thus be the main target of display panel support in CDF, and FBDEV > will be supported if that doesn't bring any drawback from an architecture > point of view. > > Encoder drivers need to be shared with the V4L2 subsystem. Similarly to panel > drivers, excluding FBDEV support from CDF isn't considered as an issue. > > 3. KMS Extensions > ----------------- > > The usefulness of V4L2 for output devices was questioned, and the possibility > of using KMS for complex video devices usually associated with V4L2 was > raised. The TI DaVinci 8xxx family is an example of chips that could benefit > from KMS support. > > The KMS API is lacking support for deep-pipelining ("framebuffers" that are > sourced from a data stream instead of a memory buffer) today. Extending the > KMS API with deep-pipelining support was considered as a sensible goal that > would mostly require the creation of a new KMS source object. Exposing the > topology of the whole device would then be handled by the Media Controller > API. > > Given that no evidence of this KMS extension being ready in a reasonable time > frame exists, sharing encoder drivers with the V4L2 subsystem hasn't been > seriously questioned. > > 4. Discovery and Initialization > ------------------------------- > > As CDF will split support for complete display devices across different > drivers, the question of physical devices discovery and initialization caused > concern among the audience. > > Topology and connectivity information can come from a wide variety of sources. > Embedded platforms typically provide that information in platform data > supplied by board code or through the device tree. PC platforms usually store > the information in the firmware exposed through ACPI, SFI, VBT or other > interfaces. Pluggable devices (PCI being the most common case) can also store > the information on an on-board non-volatile memory or hardcode it in drivers. > > When using the device tree display entity information are bundled with the > display entity device DT node. The associated driver shall thus extract itself > information from the DT node. In all other cases the display entity driver > shall not parse data from the information source directly, but shall instead > received a platform data structure filled with data parsed by the display > controller driver. In the most complex case a machine driver, similar to ASoC > machine drivers, might be needed, in which case platform data could be > provided by that machine driver. > > Display entity drivers are encouraged to internally fill a platform data > structure from their DT node to reuse the same code path for both platform > data- and DT-based initialization. > > 5. Bus Model > ------------ > > Display panels are connected to a video bus that transmits video data and > optionally to a control bus. Those two busses can be separate physical > interfaces or combined into a single physical interface. > > The Linux device model represents the system as a tree of devices (not to be > confused by the device tree, abreviated as DT). The tree is organized around > control busses, with every device being a child of its control bus master. For > instance an I2C device will be a child of its I2C controller device, which can > itself be a child of its parent PCI device. > > Display panels will be represented as Linux devices. They will have a single > parent from the Linux device model point of view, but will be potentially > connected to multiple physical busses. CDF thus needs to define what bus to > select as the Linux parent bus. > > In theory any physical bus that the device is attached to can be selected as > the parent bus. However, selecting a video data bus would depart from the > traditional Linux device model that uses control busses only. This caused > concern among several people who argued that not presenting the device to the > kernel as attached to its control bus would bring issues in embedded system. > Unlike on PC systems where the control bus master is usually the same physical > device as the data bus master, embedded systems are made of a potentially > complex assembly of completely unrelated devices. Not representing an I2C- > controlled panel as a child of its I2C master in DT was thus frown upon, even > though no clear agreement was reached on the subject. > > Panels can be divided in three categories based on their bus model. > > - No control bus > > Many panels don't offer any control interface. They are usually referred to as > 'dumb panels' as they directly display the data received on their video bus > without any configurable option. Panels in this category often use DPI is > their video bus, but other options such as DSI (using the DSI video mode only) > are possible. > > Panels with no control bus can be represented in the device model as platform > devices, or as being attached to their video bus. In the later case we would > need Linux busses for pure video data interfaces such as DPI or VGA. Nobody > was particularly enthousiastic about this idea. Dumb panels will thus likely > be represented as platform devices. > > - Separate video and control busses > > The typical case is a panel connected to an I2C or SPI bus that receives data > through a DPI video interface or DSI video mode interface. > > Using a mixed control and video bus (such as DSI and DBI) for control only > with a different bus for video data is possible in theory but very unlikely in > practice (although the creativity of hardware developers should never be > underestimated). > > Display panels that use a control bus supported by the Linux kernel should > likely be represented as children of their control bus master. Other options > are possible as mentioned above but were received without enthousiasm by most > embedded kernel developers. > > When the control bus isn't supported by the kernel, a new bus type can be > developed, or the panel can be represented as a platform device. The right > option will likely very depending on the control bus. > > - Combined video and control busses > > When the two busses are combined in a single physical bus the panel device > will obviously be represented as a child of that single physical bus. > > In such cases the control bus could expose video bus control methods. This > would remove the need for a video source as proposed by Tomi Valkeinen in his > CDF model. However, if the bus can be used for video data transfer in > combination with a different control bus, a video source corresponding to the > data bus will be needed. > > No decision has been taken on whether to use a video source in addition to the > control bus in the combined busses case. Experimentation will be needed, and > the right solution might depend on the bus type. > > - Multiple control busses > > One panel was mentioned as being connected to a DSI bus and an I2C bus. The > DSI bus is used for both control and video, and the I2C bus for control only. > configuring the panel requires sending commands through both DSI and I2C. The > opinion on such panels was a large *sigh* followed by a "this should be > handled by the device core, let's ask Greg KH". > > 6. Miscellaneous > ---------------- > > - If the OMAP3 DSS driver is used as a model for the DSI support > implementation, Daniel Vetter requested the DSI bus lock semaphore to be > killed as it prevents lockdep from working correctly (reference needed ;-)). > > - Do we need to support chaining several encoders ? We can come up with > several theoretical use cases, some of them probably exist in real hardware, > but the details are still a bit fuzzy. So, a part which is completely omitted in this thread is how to handle suspend/resume ordering. If you have multiple encoders which need to be turned on/off in a given order at suspend/resume, how do you handle that given the current scheme where they are just separate platform drivers in drivers/video? This problems occurs with drm/exynos in current 3.8 kernels for example. On that platform, the DP driver and the FIMD driver will suspend/resume in random order, and therefore fail resuming half the time. Is there something which could be done in CDF to address that? Stéphane

12 years, 9 months

Re: [Linaro-mm-sig] Fwd: [PATCHv3] gpu: ion: Add support for sharing buffers with dma buf kernel handles

by Johan Mossberg

Hi Rebecca, Is this patch going to be merged to the ion mainline? We (ST-Ericsson) are going to include it in our ion either way as we need it but if you're not planning on including it in mainline ion I'll use the previous version that did not change the api. Don't want to diverge from the standard api unless absolutely necessary. Johan On 01/11/2013 12:38 PM, Johan Mossberg wrote: > Currently ion can only share buffers with dma buf fd:s. Fd:s can not be > used inside the kernel as they are process specific so support for > sharing buffers with dma buf kernel handles is needed to support kernel > only use cases. An example use case could be a GPU driver using ion > that wants to share its output buffers with a 3d party display > controller driver supporting dma buf. > > Signed-off-by: Johan Mossberg<johan.mossberg(a)stericsson.com> > --- > drivers/gpu/ion/ion.c | 26 ++++++++++++++++++++------ > include/linux/ion.h | 12 ++++++++++-- > 2 files changed, 30 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/ion/ion.c b/drivers/gpu/ion/ion.c > index 0fc02fd..8fd61b3 100644 > --- a/drivers/gpu/ion/ion.c > +++ b/drivers/gpu/ion/ion.c > @@ -949,19 +949,19 @@ struct dma_buf_ops dma_buf_ops = { > .kunmap = ion_dma_buf_kunmap, > }; > > -int ion_share_dma_buf(struct ion_client *client, struct ion_handle *handle) > +struct dma_buf *ion_share_dma_buf(struct ion_client *client, > + struct ion_handle *handle) > { > struct ion_buffer *buffer; > struct dma_buf *dmabuf; > bool valid_handle; > - int fd; > > mutex_lock(&client->lock); > valid_handle = ion_handle_validate(client, handle); > mutex_unlock(&client->lock); > if (!valid_handle) { > WARN(1, "%s: invalid handle passed to share.\n", __func__); > - return -EINVAL; > + return ERR_PTR(-EINVAL); > } > > buffer = handle->buffer; > @@ -969,15 +969,29 @@ int ion_share_dma_buf(struct ion_client *client, struct ion_handle *handle) > dmabuf = dma_buf_export(buffer, &dma_buf_ops, buffer->size, O_RDWR); > if (IS_ERR(dmabuf)) { > ion_buffer_put(buffer); > - return PTR_ERR(dmabuf); > + return dmabuf; > } > + > + return dmabuf; > +} > +EXPORT_SYMBOL(ion_share_dma_buf); > + > +int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle) > +{ > + struct dma_buf *dmabuf; > + int fd; > + > + dmabuf = ion_share_dma_buf(client, handle); > + if (IS_ERR(dmabuf)) > + return PTR_ERR(dmabuf); > + > fd = dma_buf_fd(dmabuf, O_CLOEXEC); > if (fd < 0) > dma_buf_put(dmabuf); > > return fd; > } > -EXPORT_SYMBOL(ion_share_dma_buf); > +EXPORT_SYMBOL(ion_share_dma_buf_fd); > > struct ion_handle *ion_import_dma_buf(struct ion_client *client, int fd) > { > @@ -1086,7 +1100,7 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > > if (copy_from_user(&data, (void __user *)arg, sizeof(data))) > return -EFAULT; > - data.fd = ion_share_dma_buf(client, data.handle); > + data.fd = ion_share_dma_buf_fd(client, data.handle); > if (copy_to_user((void __user *)arg, &data, sizeof(data))) > return -EFAULT; > if (data.fd < 0) > diff --git a/include/linux/ion.h b/include/linux/ion.h > index a55d11f..e2503e9 100644 > --- a/include/linux/ion.h > +++ b/include/linux/ion.h > @@ -214,11 +214,19 @@ void *ion_map_kernel(struct ion_client *client, struct ion_handle *handle); > void ion_unmap_kernel(struct ion_client *client, struct ion_handle *handle); > > /** > - * ion_share_dma_buf() - given an ion client, create a dma-buf fd > + * ion_share_dma_buf() - share buffer as dma-buf > * @client: the client > * @handle: the handle > */ > -int ion_share_dma_buf(struct ion_client *client, struct ion_handle *handle); > +struct dma_buf *ion_share_dma_buf(struct ion_client *client, > + struct ion_handle *handle); > + > +/** > + * ion_share_dma_buf_fd() - given an ion client, create a dma-buf fd > + * @client: the client > + * @handle: the handle > + */ > +int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle); > > /** > * ion_import_dma_buf() - given an dma-buf fd from the ion exporter get handle > -- > 1.8.0 > > > _______________________________________________ > Linaro-mm-sig mailing list > Linaro-mm-sig(a)lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-mm-sig > >

12 years, 9 months

Support for high-order pages on ARM

by Mitchel Humpherys

Hi, While working with high-order page allocations (using `alloc_pages') I've encountered some issues* with certain APIs and wanted to get a better understanding of support for those APIs with high-order pages on ARM. In short, I'm trying to give userspace access to those pages by using `vm_insert_page' in an mmap handler. Without further ado, some questions: o vm_insert_page doesn't seem to work with high-order pages (it eventually calls __flush_dcache_page which assumes pages of size PAGE_SIZE). Is this analysis correct or am I missing something? Things work fine if I use `remap_pfn_range' instead of `vm_insert_page'. Things also seem to work if I use `vm_insert_page' with an array of struct page * of size PAGE_SIZE (derived from the high-order pages by picking out the PAGE_SIZE pages with nth_page)... o There's a comment in __dma_alloc (dma-alloc.c) to the effect that __GFP_COMP is not supported on ARM. Is this true? The commit that introduced this comment (ea2e7057) was actually ported from avr32 (3611553ef) so I'm curious about the basis for this claim... I've tried pages of order 8 and order 4. The gfp flags I'm passing to `alloc_pages' are (GFP_KERNEL | __GFP_HIGHMEM | __GFP_COMP). Thanks! * Some issues = in userspace mmap the buffer whose underlying mmap handler is the one mentioned above, memset that to something and then immediately check that the bytes are equal to whatever we just memset. (With huge pages and vm_insert_page this test fails). -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation

12 years, 9 months

[PATCH] lib/scatterlist: add simple page iterator

by Imre Deak

Add an iterator to walk through a scatter list a page at a time starting at a specific page offset. As opposed to the mapping iterator this is meant to be small, performing well even in simple loops like collecting all pages on the scatterlist into an array or setting up an iommu table based on the pages' DMA address. Signed-off-by: Imre Deak <imre.deak(a)intel.com> --- include/linux/scatterlist.h | 48 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) [ Resending with proper email addresses. ] diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 4bd6c06..d22851c 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -231,6 +231,54 @@ size_t sg_copy_to_buffer(struct scatterlist *sgl, unsigned int nents, */ #define SG_MAX_SINGLE_ALLOC (PAGE_SIZE / sizeof(struct scatterlist)) +struct sg_page_iter { + struct scatterlist *sg; + int sg_pgoffset; + struct page *page; +}; + +static inline int +sg_page_cnt(struct scatterlist *sg) +{ + BUG_ON(sg->offset || sg->length & ~PAGE_MASK); + + return sg->length >> PAGE_SHIFT; +} + +static inline void +sg_page_iter_next(struct sg_page_iter *iter) +{ + while (iter->sg && iter->sg_pgoffset >= sg_page_cnt(iter->sg)) { + iter->sg_pgoffset -= sg_page_cnt(iter->sg); + iter->sg = sg_next(iter->sg); + } + + if (iter->sg) { + iter->page = nth_page(sg_page(iter->sg), iter->sg_pgoffset); + iter->sg_pgoffset++; + } +} + +static inline void +sg_page_iter_start(struct sg_page_iter *iter, struct scatterlist *sglist, + unsigned long pgoffset) +{ + iter->sg = sglist; + iter->sg_pgoffset = pgoffset; + iter->page = NULL; + + sg_page_iter_next(iter); +} + +/* + * Simple sg page iterator, starting off at the given page offset. Each entry + * on the sglist must start at offset 0 and can contain only full pages. + * iter->page will point to the current page, iter->sg_pgoffset to the page + * offset within the sg holding that page. + */ +#define for_each_sg_page(sglist, iter, pgoffset) \ + for (sg_page_iter_start((iter), (sglist), (pgoffset)); \ + (iter)->sg; sg_page_iter_next(iter)) /* * Mapping sg iterator -- 1.7.9.5

12 years, 9 months

[PATCH] gpu: ion: Use memblock_remove when reserving carveout memory

by Johan Mossberg

__arm_ioremap, which is used by the carveout heap, refuses to map memory already mapped as lowmem, probably to avoid having multiple mappings with different cache settings pointing at the same memory. By using memblock_remove rather than memblock_reserve the carveout heap's memory will not be included in lowmem i.e no problem. Signed-off-by: Johan Mossberg <johan.mossberg(a)stericsson.com> --- drivers/gpu/ion/ion.c | 35 +++++++++++++++++++---------------- 1 files changed, 19 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/ion/ion.c b/drivers/gpu/ion/ion.c index 0fc02fd..b8b43bb 100644 --- a/drivers/gpu/ion/ion.c +++ b/drivers/gpu/ion/ion.c @@ -1323,29 +1323,32 @@ void __init ion_reserve(struct ion_platform_data *data) int i; for (i = 0; i < data->nr; i++) { + int ret; + if (data->heaps[i].size == 0) continue; if (data->heaps[i].base == 0) { - phys_addr_t paddr; - paddr = memblock_alloc_base(data->heaps[i].size, - data->heaps[i].align, - MEMBLOCK_ALLOC_ANYWHERE); - if (!paddr) { - pr_err("%s: error allocating memblock for " - "heap %d\n", - __func__, i); + data->heaps[i].base = memblock_find_in_range_node( + 0, + MEMBLOCK_ALLOC_ANYWHERE, + data->heaps[i].size, + data->heaps[i].align, + MAX_NUMNODES); + if (data->heaps[i].base == 0) { + pr_err("%s: error allocating memblock for heap %d\n", + __func__, data->heaps[i].id); continue; } - data->heaps[i].base = paddr; - } else { - int ret = memblock_reserve(data->heaps[i].base, - data->heaps[i].size); - if (ret) - pr_err("memblock reserve of %x@%lx failed\n", - data->heaps[i].size, - data->heaps[i].base); } + + ret = memblock_remove(data->heaps[i].base, data->heaps[i].size); + if (ret < 0) { + pr_err("%s: could not remove heap %d memory from kernel\n", + __func__, data->heaps[i].id); + continue; + } + pr_info("%s: %s reserved base %lx size %d\n", __func__, data->heaps[i].name, data->heaps[i].base, -- 1.7.4.3

12 years, 9 months

Re: [Linaro-mm-sig] [Intel-gfx] [PATCH 1/4] drm: add helper to walk a dma scatter list a page at a time

by Daniel Vetter

Hi Imre! On Sat, Feb 09, 2013 at 05:27:33PM +0200, Imre Deak wrote: > Add a helper to walk through a scatter list a page at a time. Needed by > upcoming patches fixing the scatter list walking logic in the i915 driver. Nice patch, but I think this would make a rather nice addition to the common scatterlist api in scatterlist.h, maybe called sg_page_iter. There's already another helper which does cpu mappings, but it has a different use-case (gives you the page mapped, which we don't neeed and can cope with not page-aligned sg tables). With dma-buf using sg tables I expect more users of such a sg page iterator to pop up. Most possible users of this will hang around on linaro-mm-sig, so please also cc that besides the usual suspects. Cheers, Daniel > > Signed-off-by: Imre Deak <imre.deak(a)intel.com> > --- > include/drm/drmP.h | 44 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index fad21c9..0c0c213 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -1578,6 +1578,50 @@ extern int drm_sg_alloc_ioctl(struct drm_device *dev, void *data, > extern int drm_sg_alloc(struct drm_device *dev, struct drm_scatter_gather * request); > extern int drm_sg_free(struct drm_device *dev, void *data, > struct drm_file *file_priv); > +struct drm_sg_iter { > + struct scatterlist *sg; > + int sg_offset; Imo using sg_pfn_offset (i.e. sg_offset >> PAGE_SIZE) would make it clearer that this is all about iterating page-aligned sg tables. > + struct page *page; > +}; > + > +static inline int > +__drm_sg_iter_seek(struct drm_sg_iter *iter) > +{ > + while (iter->sg && iter->sg_offset >= iter->sg->length) { > + iter->sg_offset -= iter->sg->length; > + iter->sg = sg_next(iter->sg); And adding a WARN_ON(sg->legnth & ~PAGE_MASK); here would enforce that. > + } > + > + return iter->sg ? 0 : -1; > +} > + > +static inline struct page * > +drm_sg_iter_next(struct drm_sg_iter *iter) > +{ > + struct page *page; > + > + if (__drm_sg_iter_seek(iter)) > + return NULL; > + > + page = nth_page(sg_page(iter->sg), iter->sg_offset >> PAGE_SHIFT); > + iter->sg_offset = (iter->sg_offset + PAGE_SIZE) & PAGE_MASK; > + > + return page; > +} > + > +static inline struct page * > +drm_sg_iter_start(struct drm_sg_iter *iter, struct scatterlist *sg, > + unsigned long offset) > +{ > + iter->sg = sg; > + iter->sg_offset = offset; > + > + return drm_sg_iter_next(iter); > +} > + > +#define drm_for_each_sg_page(iter, sg, pgoffset) \ > + for ((iter)->page = drm_sg_iter_start((iter), (sg), (pgoffset));\ > + (iter)->page; (iter)->page = drm_sg_iter_next(iter)) Again, for the initialization I'd go with page numbers, not an offset in bytes. > > /* ATI PCIGART support (ati_pcigart.h) */ > extern int drm_ati_pcigart_init(struct drm_device *dev, > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx(a)lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch

12 years, 9 months

[PATCH] gpu: ion: Fix a kmalloc leak in carveout heap

by Nishanth Peethambaran

Free the sg_table allocated. Signed-off-by: Nishanth Peethambaran <nishanth(a)broadcom.com> --- drivers/gpu/ion/ion_carveout_heap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/ion/ion_carveout_heap.c b/drivers/gpu/ion/ion_carveout_heap.c index ce8d311..e8164a6 100644 --- a/drivers/gpu/ion/ion_carveout_heap.c +++ b/drivers/gpu/ion/ion_carveout_heap.c @@ -107,6 +107,7 @@ void ion_carveout_heap_unmap_dma(struct ion_heap *heap, struct ion_buffer *buffer) { sg_free_table(buffer->sg_table); + kfree(buffer->sg_table); } void *ion_carveout_heap_map_kernel(struct ion_heap *heap, -- 1.7.9.5

12 years, 9 months

[PATCH] gpu: ion: Use safe version of list iterator

by Nishanth Peethambaran

On allocation or kmalloc failure in system heap allocate, the exit path iterates over the allocated page infos and frees the allocated pages and page info. The same page info structure is used as loop iterator. Use the safe version of list iterator. Signed-off-by: Nishanth Peethambaran <nishanth(a)broadcom.com> --- drivers/gpu/ion/ion_system_heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/ion/ion_system_heap.c b/drivers/gpu/ion/ion_system_heap.c index c1061a8..d079e2b 100644 --- a/drivers/gpu/ion/ion_system_heap.c +++ b/drivers/gpu/ion/ion_system_heap.c @@ -200,7 +200,7 @@ static int ion_system_heap_allocate(struct ion_heap *heap, err1: kfree(table); err: - list_for_each_entry(info, &pages, list) { + list_for_each_entry_safe(info, tmp_info, &pages, list) { free_buffer_page(sys_heap, buffer, info->page, info->order); kfree(info); } -- 1.7.9.5

12 years, 9 months

[PATCH] ARM: DMA-mapping: fix memory leak in IOMMU dma-mapping implementation

by Marek Szyprowski

This patch removes page_address() usage in IOMMU-aware dma-mapping implementation and replaced it with direct use of the cpu virtual address provided by the caller. page_address() returned incorrect address for pages remapped in atomic pool, what caused memory leak. Reported-by: Hiroshi Doyu <hdoyu(a)nvidia.com> Signed-off-by: Marek Szyprowski <m.szyprowski(a)samsung.com> Tested-by: Hiroshi Doyu <hdoyu(a)nvidia.com> --- arch/arm/mm/dma-mapping.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 6b2fb87..8c38378 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1255,11 +1255,11 @@ err_mapping: return NULL; } -static void __iommu_free_atomic(struct device *dev, struct page **pages, +static void __iommu_free_atomic(struct device *dev, void *cpu_addr, dma_addr_t handle, size_t size) { __iommu_remove_mapping(dev, handle, size); - __free_from_pool(page_address(pages[0]), size); + __free_from_pool(cpu_addr, size); } static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, @@ -1342,7 +1342,7 @@ void arm_iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr, } if (__in_atomic_pool(cpu_addr, size)) { - __iommu_free_atomic(dev, pages, handle, size); + __iommu_free_atomic(dev, cpu_addr, handle, size); return; } -- 1.7.9.5

12 years, 9 months

ashmem - mmap deadlock with mmap_sem & ashmem_mutex

by Shankar Brahadeeswaran

Hi, I'm working on Android Linux Kernel Vesion 3.0.15 and seeing a "deadlock" in the ashmem driver, while handling mmap request. I seek your support in finding the correct fix. The locks that involved in the dead lock are 1) mm->mmap_sem 2) ashmem_mutex The following is the sequence of events that leads to the deadlock. There are two threads A and B that belong to the same process (system_server) and hence share the mm struct. A1) In the A's context an mmap system call is made with an fd of ashmem A2) The system call sys_mmap_pgoff acquires the mmap_sem of the "mm" and sleeps before calling the .mmap of ashmem i.e before calling ashmem_mmap Now the thread B runs and proceeds to do the following B1) In the B's context ashmem ioctl with option ASHMEM_SET_NAME is called. B2) Now the code proceeds to acquire the ashmem_mutex and performs a "copy_from_user" B3) copy_from_user raises a valid exception to copy the data from user space and proceeds to handle it gracefully, do_DataAbort --> do_page_fault B4) In do_page_fault it finds that the mm->mmap_sem is not available (Note A & B share the mm) since A has it and sleeps Now the thread A runs again A3) It proceeds to call ashmem_mmap and tries to acquired ashmem_mutex, which is not available (is with B) and sleeps. Now A has acquired mmap_sem and waits for B to release ashmem_mutex B has acquired ashmem_mutex and waits for the mmap_sem to be available, which is held by A This creates a dead lock in the system. I'm not sure how to use these locks in such a way as to prevent this scenario. Any suggestions would be of great help. Workaround: One possible work around is to replace the mutex_lock call made in the ashmem_mmap with mutex_trylock and if it fails, wait for few milliseconds and try back for few iterations and finally give up after few iterations. This will bring the system out deadlock if this scneario happens. I myself feel that this suggestion is not clean. But I'm unable to think of anything. Is there any suggestion to avoid this scenario. Warm Regards, Shankar

12 years, 9 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig