On Sat, Sep 17, 2011 at 04:32:09PM -0500, Rob Clark wrote: > From: Rob Clark <rob@xxxxxx> > > A DRM display driver for TI OMAP platform. Similar to omapfb (fbdev) > and omap_vout (v4l2 display) drivers in the past, this driver uses the > DSS2 driver to access the display hardware, including support for > HDMI, DVI, and various types of LCD panels. And it implements GEM > support for buffer allocation (for KMS as well as offscreen buffers > used by the xf86-video-omap userspace xorg driver). > > The driver maps CRTCs to overlays, encoders to overlay-managers, and > connectors to dssdev's. Note that this arrangement might change slightly > when support for drm_plane overlays is added. > > For GEM support, non-scanout buffers are using the shmem backed pages > provided by GEM core (In drm_gem_object_init()). In the case of scanout > buffers, which need to be physically contiguous, those are allocated > with CMA and use drm_gem_private_object_init(). > > See userspace xorg driver: > git://github.com/robclark/xf86-video-omap.git > > Refer to this link for CMA (Continuous Memory Allocator): > http://lkml.org/lkml/2011/8/19/302 > > Links to previous versions of the patch: > v1: http://lwn.net/Articles/458137/ > > History: > > v2: replace omap_vram with CMA for scanout buffer allocation, remove > unneeded functions, use dma_addr_t for physical addresses, error > handling cleanup, refactor attach/detach pages into common drm > functions, split non-userspace-facing API into omap_priv.h, remove > plugin API > > v1: original This looks good. A few minors things to add to the TODO to be looked at before moving the driver out of staging: - review the dss/kms interface mismatch issues you've noted in the code - review the sync object implementation when an actual gpu side user (not just pageflip) shows up. Also a minor comment on your ioctl interface, see below. A haven't looked too closely at the fbdev stuff, simply because that's way outside my expertise. I don't think there's anything in there that can't be fixed up in staging. Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> (for -staging) > diff --git a/drivers/staging/omapdrm/TODO.txt b/drivers/staging/omapdrm/TODO.txt > new file mode 100644 > index 0000000..af81989 > --- /dev/null > +++ b/drivers/staging/omapdrm/TODO.txt > @@ -0,0 +1,14 @@ > +TODO > +. check error handling/cleanup paths > +. add drm_plane / overlay support > +. add video decode/encode support (via syslink3 + codec-engine) > +. still some rough edges with flipping.. event back to userspace should > + really come after VSYNC interrupt > diff --git a/include/drm/omap_drm.h b/include/drm/omap_drm.h > new file mode 100644 > index 0000000..ea0ae8e > --- /dev/null > +++ b/include/drm/omap_drm.h > @@ -0,0 +1,111 @@ > +/* > + * linux/include/drm/omap_drm.h > + * > + * Copyright (C) 2011 Texas Instruments > + * Author: Rob Clark <rob@xxxxxx> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published by > + * the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + * You should have received a copy of the GNU General Public License along with > + * this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#ifndef __OMAP_DRM_H__ > +#define __OMAP_DRM_H__ > + > +#include "drm.h" > + > +/* Please note that modifications to all structs defined here are > + * subject to backwards-compatibility constraints. > + */ > + > +#define OMAP_PARAM_CHIPSET_ID 1 /* ie. 0x3430, 0x4430, etc */ > + > +struct drm_omap_param { > + uint64_t param; /* in */ > + uint64_t value; /* in (set_param), out (get_param) */ > +}; > + > +struct drm_omap_get_base { > + char plugin_name[64]; /* in */ > + uint32_t ioctl_base; /* out */ > +}; > + > +#define OMAP_BO_SCANOUT 0x00000001 /* scanout capable (phys contiguous) */ > +#define OMAP_BO_CACHE_MASK 0x00000006 /* cache type mask, see cache modes */ > +#define OMAP_BO_TILED_MASK 0x00000f00 /* tiled mapping mask, see tiled modes */ > + > +/* cache modes */ > +#define OMAP_BO_CACHED 0x00000000 /* default */ > +#define OMAP_BO_WC 0x00000002 /* write-combine */ > +#define OMAP_BO_UNCACHED 0x00000004 /* strongly-ordered (uncached) */ > + > +/* tiled modes */ > +#define OMAP_BO_TILED_8 0x00000100 > +#define OMAP_BO_TILED_16 0x00000200 > +#define OMAP_BO_TILED_32 0x00000300 > + > +struct drm_omap_gem_new { > + union { /* in */ > + uint32_t bytes; /* (for non-tiled formats) */ > + struct { > + uint16_t width; > + uint16_t height; > + } tiled; /* (for tiled formats) */ > + } size; > + uint32_t flags; /* in */ > + uint32_t handle; /* out */ > +}; > + > +/* mask of operations: */ > +enum omap_gem_op { > + OMAP_GEM_READ = 0x01, > + OMAP_GEM_WRITE = 0x02, > +}; > + > +struct drm_omap_gem_cpu_prep { > + uint32_t handle; /* buffer handle (in) */ > + uint32_t op; /* mask of omap_gem_op (in) */ > +}; > + > +struct drm_omap_gem_cpu_fini { > + uint32_t handle; /* buffer handle (in) */ > + uint32_t op; /* mask of omap_gem_op (in) */ > + /* TODO maybe here we pass down info about what regions are touched > + * by sw so we can be clever about cache ops? For now a placeholder, > + * set to zero and we just do full buffer flush.. > + */ > + uint32_t nregions; > +}; Note that you cannot extend ioctl structures in an easy way with the drm ioctl infrastructure. So I think you need at least a pointer here, too. > +struct drm_omap_gem_info { > + uint32_t handle; /* buffer handle (in) */ > + uint32_t pad; > + uint64_t offset; /* out */ > +}; > + > +#define DRM_OMAP_GET_PARAM 0x00 > +#define DRM_OMAP_SET_PARAM 0x01 > +#define DRM_OMAP_GET_BASE 0x02 Unused ;-) > +#define DRM_OMAP_GEM_NEW 0x03 > +#define DRM_OMAP_GEM_CPU_PREP 0x04 > +#define DRM_OMAP_GEM_CPU_FINI 0x05 > +#define DRM_OMAP_GEM_INFO 0x06 > +#define DRM_OMAP_NUM_IOCTLS 0x07 > + > +#define DRM_IOCTL_OMAP_GET_PARAM DRM_IOWR(DRM_COMMAND_BASE + DRM_OMAP_GET_PARAM, struct drm_omap_param) > +#define DRM_IOCTL_OMAP_SET_PARAM DRM_IOW (DRM_COMMAND_BASE + DRM_OMAP_SET_PARAM, struct drm_omap_param) > +#define DRM_IOCTL_OMAP_GET_BASE DRM_IOWR(DRM_COMMAND_BASE + DRM_OMAP_GET_BASE, struct drm_omap_get_base) > +#define DRM_IOCTL_OMAP_GEM_NEW DRM_IOWR(DRM_COMMAND_BASE + DRM_OMAP_GEM_NEW, struct drm_omap_gem_new) > +#define DRM_IOCTL_OMAP_GEM_CPU_PREP DRM_IOW (DRM_COMMAND_BASE + DRM_OMAP_GEM_CPU_PREP, struct drm_omap_gem_cpu_prep) > +#define DRM_IOCTL_OMAP_GEM_CPU_FINI DRM_IOW (DRM_COMMAND_BASE + DRM_OMAP_GEM_CPU_FINI, struct drm_omap_gem_cpu_fini) > +#define DRM_IOCTL_OMAP_GEM_INFO DRM_IOWR(DRM_COMMAND_BASE + DRM_OMAP_GEM_INFO, struct drm_omap_gem_info) > + > +#endif /* __OMAP_DRM_H__ */ > diff --git a/include/drm/omap_priv.h b/include/drm/omap_priv.h > new file mode 100644 > index 0000000..ca7d975 > --- /dev/null > +++ b/include/drm/omap_priv.h > @@ -0,0 +1,42 @@ > +/* > + * linux/include/drm/omap_priv.h > + * > + * Copyright (C) 2011 Texas Instruments > + * Author: Rob Clark <rob@xxxxxx> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published by > + * the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + * You should have received a copy of the GNU General Public License along with > + * this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#ifndef __OMAP_PRIV_H__ > +#define __OMAP_PRIV_H__ > + > +/* Non-userspace facing APIs > + */ > + > +/* optional platform data to configure the default configuration of which > + * pipes/overlays/CRTCs are used.. if this is not provided, then instead the > + * first CONFIG_DRM_OMAP_NUM_CRTCS are used, and they are each connected to > + * one manager, with priority given to managers that are connected to > + * detected devices. This should be a good default behavior for most cases, > + * but yet there still might be times when you wish to do something different. > + */ > +struct omap_drm_platform_data { > + int ovl_cnt; > + const int *ovl_ids; > + int mgr_cnt; > + const int *mgr_ids; > + int dev_cnt; > + const char **dev_names; > +}; > + > +#endif /* __OMAP_DRM_H__ */ > -- > 1.7.5.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Mail: daniel@xxxxxxxx Mobile: +41 (0)79 365 57 48 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
