[PATCH] RFCv2: omapdrm DRM/KMS driver for TI OMAP platforms
Daniel Vetter
daniel at ffwll.ch
Sun Sep 18 03:23:55 PDT 2011
On Sat, Sep 17, 2011 at 04:32:09PM -0500, Rob Clark wrote:
> From: Rob Clark <rob at ti.com>
>
> 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 at ffwll.ch> (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 at ti.com>
> + *
> + * 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 at ti.com>
> + *
> + * 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48
More information about the dri-devel
mailing list