Re: DRM scatter/gather mem allocation fix

[Rafal Boni <rafal%pobox.com@localhost>]

On Fri, Apr 03, 2009 at 09:19:09PM -0400, Rafal Boni wrote:
> Attached is my update of the fix for NetBSD bus_dma framework; this does
> fix my T43 1st-X-since-boot issue, however, it turns out that the define
> for the DRM_IOCTL_SG_ALLOC in our xsrc is incorrect -- it's defined as an
> input-only ioctl vs. an in-out ioctl.  This diff works around that by not
> doing the handle-is-valid check in the sg_free ioctl, but I'm not sure if
> that's the best thing to do.

Since I got comments on the drm_scatter.c diff being unreadble, attached is
the whole new drm_scatter.c file (with one change -- I updated the comment
in sg_free to be a bit more parsable).

Thx,
--rafal

-- 
  Time is an illusion; lunchtime, doubly so.     |/\/\|           Rafal Boni
                   -- Ford Prefect               |\/\/|      
rafal%pobox.com@localhost

/* $NetBSD: drm_scatter.c,v 1.7 2008/07/07 00:33:23 mrg Exp $ */

/* drm_scatter.h -- IOCTLs to manage scatter/gather memory -*- linux-c -*-
 * Created: Mon Dec 18 23:20:54 2000 by gareth%valinux.com@localhost */
/*-
 * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
 * All Rights Reserved.
 *
 * Permission is hereby granted, free of charge, to any person obtaining a
 * copy of this software and associated documentation files (the "Software"),
 * to deal in the Software without restriction, including without limitation
 * the rights to use, copy, modify, merge, publish, distribute, sublicense,
 * and/or sell copies of the Software, and to permit persons to whom the
 * Software is furnished to do so, subject to the following conditions:
 *
 * The above copyright notice and this permission notice (including the next
 * paragraph) shall be included in all copies or substantial portions of the
 * Software.
 *
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
 * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
 * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
 * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 * DEALINGS IN THE SOFTWARE.
 *
 * Authors:
 *   Gareth Hughes <gareth%valinux.com@localhost>
 *   Eric Anholt <anholt%FreeBSD.org@localhost>
 *
 */

#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: drm_scatter.c,v 1.7 2008/07/07 00:33:23 mrg Exp $");
/*
__FBSDID("$FreeBSD: src/sys/dev/drm/drm_scatter.c,v 1.3 2006/05/17 06:29:36 
anholt Exp $");
*/

/** @file drm_scatter.c
 * Allocation of memory for scatter-gather mappings by the graphics chip.
 *
 * The memory allocated here is then made into an aperture in the card
 * by drm_ati_pcigart_init().
 */

#include "drmP.h"

int
drm_sg_alloc(struct drm_device *dev, struct drm_scatter_gather *request)
{
        drm_sg_mem_t *entry;
        struct drm_dma_handle *dmah;
        unsigned long pages;
        int nsegs, ret, i;

        entry = malloc(sizeof(*entry), M_DRM, M_ZERO | M_NOWAIT);
        if ( !entry )
                return ENOMEM;

        pages = round_page(request->size) / PAGE_SIZE;
        DRM_DEBUG( "sg size=%ld pages=%ld\n", request->size, pages );

        entry->pages = pages;

        entry->busaddr = malloc(pages * sizeof(*entry->busaddr), M_DRM,
            M_ZERO | M_NOWAIT);
        if ( !entry->busaddr ) {
                free(entry, M_DRM);
                return ENOMEM;
        }

        dmah = malloc(sizeof(struct drm_dma_handle), M_DRM,
            M_ZERO | M_NOWAIT);
        if (dmah == NULL) {
                free(entry->busaddr, M_DRM);
                free(entry, M_DRM);
                return ENOMEM;
        }

        dmah->dmat = dev->pa.pa_dmat;

        ret = bus_dmamem_alloc(dmah->dmat, request->size, PAGE_SIZE, 0,
            dmah->segs, 1, &nsegs, BUS_DMA_NOWAIT);
        if (ret != 0) {
                free(dmah, M_DRM);
                free(entry->busaddr, M_DRM);
                free(entry, M_DRM);
                return ENOMEM;
        }

        ret = bus_dmamem_map(dmah->dmat, dmah->segs, nsegs, request->size,
             &dmah->addr, BUS_DMA_NOWAIT | BUS_DMA_COHERENT);
        if (ret != 0) {
                bus_dmamem_free(dmah->dmat, dmah->segs, nsegs);
                free(dmah, M_DRM);
                free(entry->busaddr, M_DRM);
                free(entry, M_DRM);
                return ENOMEM;
        }

        ret = bus_dmamap_create(dmah->dmat, request->size, pages, PAGE_SIZE,
                0, BUS_DMA_NOWAIT | BUS_DMA_ALLOCNOW, &dmah->map);

        if (ret != 0) {
                bus_dmamem_unmap(dmah->dmat, dmah->addr, request->size);
                bus_dmamem_free(dmah->dmat, dmah->segs, nsegs);
                free(dmah, M_DRM);
                free(entry->busaddr, M_DRM);
                free(entry, M_DRM);
                return ENOMEM;
        }

        ret = bus_dmamap_load(dmah->dmat, dmah->map, dmah->addr, request->size,
            NULL, BUS_DMA_NOWAIT);
        if (ret != 0) {
                bus_dmamem_unmap(dmah->dmat, dmah->addr, request->size);
                bus_dmamem_free(dmah->dmat, dmah->segs, nsegs);
                free(dmah, M_DRM);
                free(entry->busaddr, M_DRM);
                free(entry, M_DRM);
                return ENOMEM;
        }

        DRM_DEBUG( "sg alloc nsegs = %d\n", dmah->map->dm_nsegs);

        for (i = 0; i < pages; i++) {
            entry->busaddr[i] = dmah->map->dm_segs[i].ds_addr;
        }

        entry->handle = (unsigned long)dmah->addr;
        entry->sg_dmah = dmah;
        
        DRM_DEBUG( "sg alloc handle  = %08lx\n", entry->handle );

        entry->virtual = (void *)entry->handle;
        request->handle = entry->handle;

        DRM_LOCK();
        if (dev->sg) {
                DRM_UNLOCK();
                drm_sg_cleanup(entry);
                return EINVAL;
        }
        dev->sg = entry;
        DRM_UNLOCK();

        return 0;
}

int
drm_sg_alloc_ioctl(DRM_IOCTL_ARGS)
{
        DRM_DEVICE;
        drm_scatter_gather_t request;
        int ret;

        if ( dev->sg )
                return EINVAL;

        DRM_DEBUG( "%s\n", __FUNCTION__ );

        DRM_COPY_FROM_USER_IOCTL(request, (drm_scatter_gather_t *)data,
                             sizeof(request) );

        ret = drm_sg_alloc(dev, &request);
        if (ret != 0)
                return ret;

        DRM_COPY_TO_USER_IOCTL( (drm_scatter_gather_t *)data,
                           request,
                           sizeof(request) );

        return 0;
}

void
drm_sg_cleanup(drm_sg_mem_t *entry)
{
        struct drm_dma_handle *dmah = entry->sg_dmah;

        bus_dmamap_unload(dmah->dmat, dmah->map);
        bus_dmamem_unmap(dmah->dmat, dmah->addr, entry->pages << PAGE_SHIFT);
        bus_dmamem_free(dmah->dmat, dmah->segs, 1);
        free(dmah, M_DRM);
        free(entry->busaddr, M_DRM);
        free(entry, M_DRM);
}

int
drm_sg_free_ioctl(DRM_IOCTL_ARGS)
{
        DRM_DEVICE;
        drm_scatter_gather_t request;
        drm_sg_mem_t *entry;

        DRM_COPY_FROM_USER_IOCTL( request, (drm_scatter_gather_t *)data,
                             sizeof(request) );

        DRM_LOCK();
        entry = dev->sg;
        dev->sg = NULL;
        DRM_UNLOCK();

        /*
         * XXX: current Xsrc defines the SG_ALLOC ioctl as _IOR() vs _IOWR(),
         * so the handle created in the sg alloc call never makes it back to
         * userland.
         *
         * Allow the handle to be free'd so we don't leak memory; this should
         * really be done in the locked section above so we don't toss out the
         * devices' sg list unless the handle matches, except for the above
         * caveat.
         */ 
        if ( !entry /* || entry->handle != request.handle */ ) {
                DRM_DEBUG( "sg free: error: %s handle\n", entry ? "invalid" : 
"no allocated");
                return EINVAL;
        }

        DRM_DEBUG( "sg free virtual  = 0x%lx\n", entry->handle );

        drm_sg_cleanup(entry);

        return 0;
}