Re: [PATCH] omap2+: add drm device

["Clark, Rob" <rob@ti.com>]

On Thu, May 24, 2012 at 12:01 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Hi,
>
> On Wed, 2012-05-23 at 15:08 -0500, Andy Gross wrote:
>> Register OMAP DRM/KMS platform device.  DMM is split into a
>> separate device using hwmod.
>>
>> Signed-off-by: Andy Gross <andy.gross@ti.com>
>
> <snip>
>
>> +static int __init omap_init_drm(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);
>> +
>> +}
>
> I still don't like fixing the tiler to drm. I would like to have basic
> tiler support in omapfb also, but with this approach I'll need to
> duplicate the code. And even if we disregard omapfb, wouldn't it be
> architecturally better to have the tiler as a separate independent
> library/driver?

Not easily, at least not if we want to manage to use tiler/dmm in a
more dynamic way, or to enable some additional features which are
still on the roadmap (like reprogramming dmm synchronized w/ scanout,
or some things which are coming if future hw generations).  We need
one place to keep track of which buffers are potentially evictable to
make room for mapping a new buffer.  And if you look at the tricks
that go on with mmap'ing tiled buffers to userspace, you *really*
don't want to duplicate that in N different drivers.

Fortunately with dmabuf there is not really a need for N different
drivers to need to use tiler/dmm directly.  The dmabuf mechanism
provides what they need to import GEM buffers from omapdrm.  That may
not really help omapfb because fbdev doesn't have a concept of
importing buffers.  But OTOH this is unnecessary, because drm provides
an fbdev interface for legacy apps.  The best thing I'd recommend is,
if you miss some features of omapfb in the drm fbdev implementation,
is to send some patches to add this missing features.

>> +struct omap_drm_platform_data {
>> +     struct omap_kms_platform_data *kms_pdata;
>> +};
>
> This one is missing struct omap_dmm_platform_data *dmm_pdata, so you
> didn't just move the struct. Is that on purpose?

the dmm pdata is no longer needed because we get what we need from
hwmod via platform_get_resource()

BR,
-R

>  Tomi
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html