On Tue, 2012-03-06 at 08:01 -0600, Rob Clark wrote:
> On Tue, Mar 6, 2012 at 7:26 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> > Can there be more than one omapdrm device? I guess not. If so, the id
> > should be -1.
> 
> in the past, we have used multiple devices (using the platform-data to
> divide up the dss resources), although this was sort of a
> special-case.  But in theory you could have multiple.  (Think of a
> multi-seat scenario, where multiple compositors need to run and each
> need to be drm-master of their own device to do mode-setting on their
> corresponding display.)
> 
> I think if we use .id = -1, then we'd end up with a funny looking
> bus-id for the device: "platform:omapdrm:-1"

I don't know about that, but it's the way platform devices are meant to
be used if there can be only one instance. If the case where there are
multiple omapdrm devices is rather theoretical, or only used for certain
debugging scenarios or such, I think having the id as -1 in the mainline
is cleaner.

> >> +};
> >> +
> >> +static int __init omap_init_gpu(void)
> >> +{
> >> +     struct omap_hwmod *oh = NULL;
> >> +     struct platform_device *pdev;
> >> +
> >> +     /* lookup and populate the DMM information, if present - OMAP4+ */
> >> +     oh = omap_hwmod_lookup("dmm");
> >> +
> >> +     if (oh) {
> >> +             pdev = omap_device_build(oh->name, -1, oh, NULL, 0, NULL, 0,
> >> +                                     false);
> >> +             WARN(IS_ERR(pdev), "Could not build omap_device for %s\n",
> >> +                     oh->name);
> >> +     }
> >> +
> >> +     return platform_device_register(&omap_drm_device);
> >> +
> >> +}
> >
> > The function is called omap_init_gpu(), but it doesn't do anything
> > related to the gpu... And aren't DMM and DRM totally separate things,
> > why are they bunched together like that?
> 
> only legacy.. product branches also have sgx initialization here.  But
> I can change that to omap_init_drm()
> 
> DMM is managed by the drm driver (and exposed to userspace as drm/gem
> buffers, shared with other devices via dma-buf, etc).  It is only
> split out to a separate device to play along with hwmod.

I have to say I don't know much about DMM, but my understanding is that
DMM is a bigger entity, of which TILER is only a small part, and DMM
manages all memory accesses.

Can there be other users for the DMM than DRM? I know there could be
other users for the TILER, and I know you want to combine that with the
DRM driver, but I'm wondering about the other parts of DMM than the
TILER.

Somehow having a DMM driver inside omapdrm sounds strange.

> >> +arch_initcall(omap_init_gpu);
> >> +
> >> +void omapdrm_reserve_vram(void)
> >> +{
> >> +#ifdef CONFIG_CMA
> >> +     /*
> >> +      * Create private 32MiB contiguous memory area for omapdrm.0 device
> >> +      * TODO revisit size.. if uc/wc buffers are allocated from CMA pages
> >> +      * then the amount of memory we need goes up..
> >> +      */
> >> +     dma_declare_contiguous(&omap_drm_device.dev, 32 * SZ_1M, 0, 0);
> >
> > What does this actually do? Does it reserve the memory, i.e. it's not
> > usable for others? If so, shouldn't there be a way for the user to
> > configure it?
> 
> It can be re-used by others.. see http://lwn.net/Articles/479297/

The link didn't tell much. I looked at the patches, and
dma_declare_contiguous allocates the memory with memblock_alloc. So is
that allocated memory available for any users? If so, why does the
function want a device pointer as an argument...

Is the memory available for normal kmalloc, or only available via the
CMA functions? Surely there must be some downside to the above? =) And
if so, it should be configurable. You could have a board with no display
at all, and you probably don't want to have 32MB allocated for DRM in
that case.

 Tomi