Multiple issues with omapdrm

[Rob Clark]

On Wed, Jun 5, 2013 at 8:16 AM, Tomi Valkeinen <tomi.valkeinen at ti.com> wrote:
> On 05/06/13 14:52, Rob Clark wrote:
>> On Wed, Jun 5, 2013 at 7:35 AM, Tomi Valkeinen <tomi.valkeinen at ti.com> wrote:
>>> On 05/06/13 13:57, Rob Clark wrote:
>>>
>>>> 1) drmOpen("omap") will try to modprobe "omap", not "omapdrm" so we
>>>> need to rename the .ko
>>>
>>> Has something been changed that broke that? Or was "omapdrm" just a
>>> badly chosen name from the start? If drmOpen("omapdrm") works now,
>>> doesn't changing the module name break userspace compatibility?
>>
>> it was broken all along, because you need to drmOpen("omap") so the
>> module should have been called omap.ko from the beginning
>
> But it wasn't, so wouldn't changing it break userspace compatibility?

could be udev or something else was triggering the module to load?
Not 100% sure offhand.

> Why do you "need" drmOpen("omap")? Is it just a convention to use that
> kind of name, instead of "omapdrm" style name?

all of the /dev/dri/cardN get opened, and then DRM_IOCTL_VERSION ioctl
to get the driver name/version, which is compared against the name
passed in to drmOpen().  So drmOpen("omapdrm") shouldn't really work

>>> I had a quick look at libdrm. It calls server_info->load_module() but I
>>> couldn't figure out where that call actually goes...
>>
>> it's registered from xserver, fyi
>
> Ah ok. So in my testing, running without X, there's actually no module
> loading happening. The test tools print something like "trying to load
> module omapdrm...success." but I guess that only means that the module
> is already there.

right

>>>> 2) sorting out the modprobe of panel drivers..  although with the
>>>> current structure of omapdrm+omapdss I can't think of any clean way to
>>>> handle this.  I suppose we could do a hack with a bunch of
>>>> request_module()s
>>>
>>> If omapdrm and omapdss were merged, what would be the clean way be? Or
>>> did you mean some other structure?
>>
>> well, then we'd just build it all into one .ko
>
> So you didn't actually mean omapdrm+omapdss, but omapdrm + omapdss +
> all-the-panel-drivers? And then have some kind of forced method to probe
> the panels at omapdrm start?

oh, yeah.  Probably whenever I say 'omapdss' I actually mean
'everything under drivers/video/omap2 except omapfb'

> That would be quite a horrible solution, in my opinion =). It would also
> be even worse when we get platform independent panel drivers, as all the
> drm drivers using those panel drivers would include all the panel drivers.

yeah, it isn't really ideal.. really we'd want them as separate
modules but some sort of dependency to ensure that the panel drivers
actually get loaded.

>>> I'm no expert on this, but my understanding is that udev (or such)
>>> should load the modules for the devices that the board has. If it's a
>>> requirement that the drm drivers are loaded only by a call to drmOpen()
>>> (why is that?), then maybe udev could load only the panel drivers.
>>
>> I'm not quite sure the entire history offhand.. maybe drmOpen()
>> pre-dated that?  At any rate, the main issue is just ensuring the
>> panel modules (if they are split out into different .ko's) are loaded
>> first
>
> I think there should be a core display facility in the kernel to which
> the panel drivers register the displays in probe().
>
> At boot time udev would go through /sys, find the panel devices somehow
> (how? modalias is probably somehow involved..), and load their
> respective modules.
>
> Later, when X is started, omapdrm and omapdss are loaded, and at that
> point all the panels are already there.

yeah, as long as we get the panels (and encoders/bridges/etc) loaded
first, I think it should be ok..

I'm not hugely fond of having it split out of drm, since I think that
just results extra layering and glue, and it makes it harder to
iteratively evolve the infrastructure for sharing panel stuff via
different trees from drm.  But that is a different discussion and is
mostly about where the code lives.

BR,
-R

>  Tomi
>
>