Re: [PATCH] omap2+: add drm device

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 5178 bytes --]

On Thu, 2012-05-24 at 02:35 -0600, Rob Clark wrote:
> On Thu, May 24, 2012 at 1:05 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Thu, 2012-05-24 at 00:27 -0600, Clark, Rob wrote:
> >> 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.
> >
> > So why can't all that code be in a tiler library/driver?
> 
> Possibly the placement stuff could be in a library..  the
> mmap/faulting stuff I think would be harder to split.  With it split
> out in a separate lib, it becomes logistically a bit more of a
> headache to change APIs, etc.  Basically it just makes more work and
> is unnecessary.

Unnecessary for you, but maybe not for those who want to use omapfb.

> >> 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.
> >
> > Well, at least currently omapfb and omapdrm work quite differently, if
> > I've understood right. Can we make a full omapfb layer on top of
> > omapdrm? With multiple framebuffers mapped to one or more overlays,
> > support for all the ioctls, etc?
> 
> Well, there is still room to add your own fb_ioctl() fxn, so I guess
> in principle it should be possible to add whatever custom ioctls are
> required.
> 
> Typically you have a single fbdev device for a single drm device,
> although I suppose nothing prevents creating more.  I'd probably want
> to encourage users more towards using KMS directly for multi-display
> cases because you have a lot more options/flexibility that way.

Sure, but we can't force people to use omapdrm instead of omapfb. And
omapfb is not going to disappear. So obviously we should recommend using
omapdrm, but on the other hand, I don't see any problem in adding new
features to omapfb if they are easily implemented (using, for example, a
separate tiler driver).

> > I guess we'd still need to have omapfb driver to keep the module
> > parameters and behavior the same. Can omapdrm be used from inside the
> > kernel by another driver?
> 
> Hmm, I'm not quite sure what you have in mind, but it sounds a bit
> hacky..  I'd guess if you need 100% backwards compatibility even down
> to kernel cmdline / module params, then you probably want to use
> omapfb.  But there isn't really need to add new features to omapfb in
> that case.

I was thinking of making omapfb use omapdrm, instead of omapdss. I mean,
not planning to do that, just wondering if that would be possible.

> Off the top of my head, I guess that 80-90% compatibility would
> probably be reasonable to add to omapdrm's fbdev..  and that the last
> 10-20% would be too hacky/invasive to justify adding to omapdrm.

I think it should be 99.9% - 100% or nothing. If it's only 80-90%
compatible, then it's not compatible =).

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]