[PATCH] omap2+: add drm device

[Rob Clark]

On Tue, Mar 6, 2012 at 8:35 AM, Tomi Valkeinen <tomi.valkeinen at ti.com> wrote:
> 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 at 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.

well, I don't care much one way or another, but need to check if there
is a small patch needed in drm core code that generates the bus-id for
.id = -1..

>> >> +};
>> >> +
>> >> +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.

when most people refer to TILER they actually refer to DMM..

DMM is the piece which is basically a GART, it is what omapdrm is
programming and managing.

> 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.

yes, clearly there are other users.. we pass gem buffers allocated
from omapdrm to (for example, video decoder).  But it is omapdrm which
is managing the allocation, keeping track of which buffers are pinned
and which can be evicted, dealing with the complications of userspace
mmap'ing of tiled buffers, etc.  That stuff you don't want littered
throughout the kernel.

> 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.

Basically the short version is memory from a CMA carveout can be
re-used for unpinnable memory.  Not kmalloc, but it can be used for
anon userspace pages, for example.  Userspace needs memory too.

BR,
-R

>  Tomi
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>