lkml.org 
[lkml]   [2016]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] drm/fb_cma_helper: Implement fb_mmap callback
    From
    Date
    On 16/03/16 19:14, Russell King - ARM Linux wrote:
    > On Wed, Mar 16, 2016 at 04:28:25PM +0100, Daniel Vetter wrote:
    >> On Wed, Mar 16, 2016 at 02:57:49PM +0000, Robin Murphy wrote:
    >>> In the absence of an fb_mmap callback, the fbdev code falls back to a
    >>> naive implementation which relies upon the DMA address being the same
    >>> as the physical address, and the buffer being physically contiguous
    >>> from there. Whilst this often holds for standard CMA allocations via
    >>> the platform's regular DMA ops, if the allocation is provided by an
    >>> IOMMU then such assumptions can fall apart spectacularly.
    >>>
    >>> To resolve this, reroute the fb_mmap call to the appropriate DMA API
    >>> implementation, as per the other cma_helper calls.
    >>>
    >>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
    >>> ---
    >>>
    >>> Hi dri-devel,
    >>>
    >>> This is an empirical fix for something I tickled via the newly-added
    >>> ARM HDLCD driver on a Juno platform - I have no idea whatsoever about
    >>> how "proper" it is in terms of the DRM infrastructure, so feel free to
    >>> treat this as a bug report rather than an actual patch if appropriate ;)
    >>
    >> I think the best case would be if we could have a generic fbdev helper
    >> that remaps to dumb mmap support. But that's a bit tricky to pull of:
    >> 1. from fb_info we can get at the fbdev drm_framebuffer.
    >> 2. from a drm_framebuffer we can get at the underlying backing storage
    >> object using fb->funcs->get_handle.
    >> 3. With that handle we could go into the dumb mmap support (using als the
    >> vma) and create the mmap.
    >>
    >> Except that ->get_handle needs a file_priv, and that just exist for the
    >> fbdev emulation kms client. I guess we could fix that by creating a
    >> minimal fake drm file_priv for the fbdev emulation (and treat it more like
    >> any other kms client), but I think that's way too much work when this
    >> simple patch here gets the job done.
    >
    > I think first, a different question needs to be answered:
    >
    > include/uapi/linux/fb.h:
    >
    > struct fb_fix_screeninfo {
    > char id[16]; /* identification string eg "TT Builtin" */
    > unsigned long smem_start; /* Start of frame buffer mem */
    > /* (physical address) */
    >
    > Should a DMA address be exposed through smem_start, rather than a
    > physical address as the long-standing documentation quoted above
    > has stated?
    >
    > Is it, in fact, a driver bug to store something that isn't a physical
    > address there?

    We could also go into whether it's right to store even a physical
    address in something which isn't necessarily big enough...

    After the time I spent in the bowels of the fbdev code figuring out this
    crash, I fear that if we dig too deep we may awaken something in the
    darkness ;)

    Robin.

    \
     
     \ /
      Last update: 2016-03-17 12:41    [W:2.527 / U:0.244 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site