dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Frank Binns <frank.binns@imgtec.com>
Cc: marcheu@google.com, Chiawen.Lee@mediatek.com,
	puneetster@google.com, scott.chuang@mediatek.com,
	cawa.cheng@mediatek.com, dri-devel@lists.freedesktop.org,
	zelidrag@google.com, alberto@google.com, piman@google.com,
	"Rufus Hamade" <rufus.hamade@imgtec.com>,
	djkurtz@google.com, katierh@google.com, dbehr@google.com,
	Yt.Shen@mediatek.com,
	"\"Eddie Huang (黃智傑)\"" <eddie.huang@mediatek.com>,
	"\"Nathan Chung (仲崇寧)\"" <nathan.chung@mediatek.com>
Subject: Re: RFC on upstreaming of a Mediatek DRM modesetting driver
Date: Tue, 2 Dec 2014 16:10:15 +0100	[thread overview]
Message-ID: <20141202151015.GV32117@phenom.ffwll.local> (raw)
In-Reply-To: <547D84AF.5030102@imgtec.com>

On Tue, Dec 02, 2014 at 09:21:51AM +0000, Frank Binns wrote:
> On 01/12/14 15:28, Daniel Vetter wrote:
> > On Mon, Dec 01, 2014 at 10:01:37AM +0000, Frank Binns wrote:
> >> Hi,
> >>
> >> We are currently in negotiations with one of our customers (Mediatek) on
> >> a strategy that will allow them to push a DRM modesetting driver into
> >> the upstream kernel. We are writing to get people's opinions and
> >> feedback on our proposed approach.
> >>
> >> Currently, our driver is structured in such a way that the display
> >> driver is more tightly integrated with the GPU driver than we would
> >> like. Although our kernel driver has been shipped with a GPL license for
> >> a long time, it is not in a form that would be considered acceptable
> >> upstream. Unfortunately, it is going to be a long process to get this
> >> part of the driver into a reasonable state. However, in the meantime, we
> >> don't want to prevent customer portions of the driver from being
> >> upstreamed. With the work done on recent kernels, and with a willing
> >> partner in Mediatek, we now think that we can restructure our driver in
> >> such a way as to allow this to happen.
> >>
> >> We see two basic approaches to achieving this:
> >> 1) Two independent DRM drivers, i.e. modesetting and render node drivers
> >> 2) A single componentised DRM driver
> >>
> >> Our (IMG's) preferred approach is to have a single componentised DRM
> >> driver. This is due to the following reasons:
> >>
> >> - Existing user space is not fully prepared to handle render nodes.
> >>
> >> - There is concern that any IMG DRM render node driver will need
> >> knowledge about multiple SoCs, each one being from a different vendor.
> >> Would this be deemed acceptable?
> >>
> >> - There is a trend, at least for DRM SoC drivers, towards using the
> >> component interface. Although there appears to be very few (one?)
> >> examples of GPU component drivers.
> >>
> >> To give some high level details on how we expect the componentised DRM
> >> driver model to work, each vendor (in this case Mediatek) will write
> >> their own DRM driver (supporting modesetting, dumb buffers, GEM, prime,
> >> etc) and IMG will provide an almost entirely independent component
> >> driver that adds in GPU support. Until our GPU driver is in a suitable
> >> state this will most likely necessitate a small kernel patch to wire up
> >> support, e.g. GPU specific ioctls.
> >>
> >> Cross-device and cross-process memory allocations will be made using the
> >> DRM driver. In order for this memory to be shared with the GPU component
> >> driver it will be necessary, at least for the time being, to export it
> >> via prime and import it via a GPU ioctl. Synchronisation between the
> >> display and GPU will be performed via reservation objects.
> >>
> >> Does this sound like a sane approach? Questions and/or feedback is very
> >> welcome.
> > Rule of thumb is that if it's an externally licensed IP block it should be
> > a separate driver. Which is the case here. The idea is that the mostly
> > generic IMG driver could be reused on other platforms that ship the same
> > IP-block, while linking up with the respective display controller driver.
> > The end result is 2 drm drivers:
> > - Display block drm driver which expose KMS objects for modesetting, but
> >   only very basic gem (just enough to allocate dumb framebuffers and
> >   import/export dma-bufs).
> > - Full-blown gem driver for the img render IP block.
> >
> > For an example look at the tegra/nouveau combo which can run on TK1.
> >
> > Plugggin in an IMG driver into each display block like it's currently done
> > with all the armsoc stuff on android is imo completely no-go.
> >
> > Note that the component interface is completely irrelevant wrt the
> > interface you expose to userspace. It's just an driver-internal helper
> > library useful in certain situation. Not even the drm core really cares
> > whether you use component helpers or not.
> >
> > Thanks, Daniel
> 
> OK, so it seems the consensus is that IMG should provide a separate
> render-node only DRM driver.
> 
> Having not worked directly on the core DRM code I'm not completely
> familiar with it but it seems to me that the DRIVER_MODESET flag has a
> dual meaning. Firstly it means that the driver supports KMS and secondly
> it means that a lot of the legacy stuff isn't supported. It also changes
> the way in which driver initialisation is performed. Would it make sense
> for the DRIVER_RENDER flag to have a similar effect? In other words,
> should it turn off legacy stuff and use the newer method of driver
> initialisation?

DRIVER_MODESET means you have a modern driver which binds to the device.
We should probably RENAME it to DRIVER_LEGACY and invert it's sense, but
no one has stepped up to the taks.

DRIVER_RENDER just means you support render nodes. And new drm driver
which exposes driver-specific ioctls should set this flag. Again we should
probably rename this to DRIVER_IOCTLS_BROKEN_CANT_DO_RENDER and invert the
sense ;-) Ofc with a better neame.

So for the IMG render-only driver you'd need to set both. Both flags just
hide legacy behaviour if not set, but that's the way you need to do it
without changing every driver.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2014-12-02 15:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-01 10:01 RFC on upstreaming of a Mediatek DRM modesetting driver Frank Binns
2014-12-01 15:28 ` Daniel Vetter
2014-12-01 15:31   ` Alberto Martin
2014-12-02  9:21   ` Frank Binns
2014-12-02 15:10     ` Daniel Vetter [this message]
2014-12-02 20:28       ` Thierry Reding
2014-12-01 15:32 ` Jerome Glisse
2014-12-01 16:20 ` Rob Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141202151015.GV32117@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=Chiawen.Lee@mediatek.com \
    --cc=Yt.Shen@mediatek.com \
    --cc=alberto@google.com \
    --cc=cawa.cheng@mediatek.com \
    --cc=dbehr@google.com \
    --cc=djkurtz@google.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eddie.huang@mediatek.com \
    --cc=frank.binns@imgtec.com \
    --cc=katierh@google.com \
    --cc=marcheu@google.com \
    --cc=nathan.chung@mediatek.com \
    --cc=piman@google.com \
    --cc=puneetster@google.com \
    --cc=rufus.hamade@imgtec.com \
    --cc=scott.chuang@mediatek.com \
    --cc=zelidrag@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox