[PATCH] omap2+: add drm device

[Rob Clark]

On Wed, Mar 7, 2012 at 5:59 AM, Tomi Valkeinen <tomi.valkeinen at ti.com> wrote:
> On Tue, 2012-03-06 at 09:29 -0600, Rob Clark wrote:
>> 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..
>
> Hmm, why does the drm core care about it?

Because it is the one generating the bus-id.. see
drivers/gpu/drm/drm_platform.c drm_platform_set_busid()

Anyways, it's just a detail about how libdrm/drmOpen() can
differentiate between multiple instances of the same driver, similar
to how PCI bus-id is used in the desktop world.  It is not difficult
to change in drm_platform_set_busid(), although not sure if that would
be considered an ABI change to userspace.  (Maybe it is less critical,
I'm under the impression that other platform-drm users didn't even
realize we had a bus-id.)

> And generally, I think if the bus id is -1, there is no bus id in a
> sense. For example, with bus id 0 you'll get a device "omapdrm.0". With
> bus id -1 you'll get a device "omapdrm".
>
>> >> >> +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.
>
> Okay, let me ask the other way. Is 32MB enough for everyone? Hardcoding
> a value like that without any possibility to adjust it just sounds like
> a rather bad thing.

The main requirement is that, on omap3 or before (platforms without
DMM) you need enough to allocate all of your scanout buffers.

I'm not against having a bootarg to adjust, although I strongly prefer
sane defaults and not requiring a million bootargs just to boot in
some usable fashion.

BR,
-R

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