Re: [PATCH] drm/doc: Document uapi requirements in DRM

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Liviu Dudau" <liviu.dudau@arm.com>,
	"DRI Development" <dri-devel@lists.freedesktop.org>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Thomas Hellstrom" <thellstrom@vmware.com>,
	"Alexey Brodkin" <abrodkin@synopsys.com>,
	"Xinliang Liu" <z.liuxinliang@hisilicon.com>,
	"Tomi Valkeinen" <tomi.valkeinen@ti.com>,
	"Ben Skeggs" <bskeggs@redhat.com>,
	"Russell King" <rmk+kernel@armlinux.org.uk>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Maxime Ripard" <maxime.ripard@free-electrons.com>,
	"Christian König" <christian.koenig@amd.com>
Subject: Re: [PATCH] drm/doc: Document uapi requirements in DRM
Date: Mon, 02 Jan 2017 00:40:59 +0200	[thread overview]
Message-ID: <1733448.yI717seRR3@avalon> (raw)
In-Reply-To: <1471639838-25591-1-git-send-email-daniel.vetter@ffwll.ch>

Hi Daniel,

Thank you for the patch.

On Friday 19 Aug 2016 22:50:38 Daniel Vetter wrote:
> Everyone knows them, except all the new folks joining from the ARM
> side haven't lived through all the pain of the past years and are
> entirely surprised when I raise this. Definitely time to document
> this.
> 
> Last time this was a big discussion was about 6 years ago, when qcom
> tried to land a kernel driver without userspace. Dave Airlie made the
> rules really clear:
> 
> http://airlied.livejournal.com/73115.html
> 
> This write-up here is essentially what I've put into a presentation a
> while ago, which was also reviewed by Dave:
> 
> http://blog.ffwll.ch/2015/05/gfx-kernel-upstreaming-requirements.html
> 
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: Oded Gabbay <oded.gabbay@gmail.com>
> Cc: Russell King <rmk+kernel@armlinux.org.uk>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Thomas Hellstrom <thellstrom@vmware.com>
> Cc: Sinclair Yeh <syeh@vmware.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Mark Yao <mark.yao@rock-chips.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: CK Hu <ck.hu@mediatek.com>
> Cc: Xinliang Liu <z.liuxinliang@hisilicon.com>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Maxime Ripard  <maxime.ripard@free-electrons.com>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Brian Starkey <brian.starkey@arm.com>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> Acked-by: Dave Airlie <airlied@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  Documentation/gpu/drm-uapi.rst | 67 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
> index 94876938aef3..a7e3aa27167d 100644
> --- a/Documentation/gpu/drm-uapi.rst
> +++ b/Documentation/gpu/drm-uapi.rst
> @@ -36,6 +36,73 @@ Primary Nodes, DRM Master and Authentication
>  Open-Source Userspace Requirements
>  ==================================
> 
> +The DRM subsystem has stricter requirements on what the userspace side for
> new +uAPI needs to look like. This section here explains what exactly those
> +requirements are, and why they exist.
> +
> +The short summary is that any addition of DRM uAPI requires corresponding
> +open-sourced userspace patches, and those patches must be reviewed and
> ready for +merging into a suitable and canonical upstream project.
> +
> +GFX devices (both display and render/GPU side) are really complex bits of
> hardware, +with userspace and kernel by necessity having to work together
> really closely. +The interfaces, for rendering and modesetting must be
> extremely wide and +flexible, and therefore it is almost always impossible
> to precisely define them +for every possible corner case. This in turns
> makes it really practically +infeasible to differentiate between behaviour
> that's required by userspace, and +which must not be changed to avoid
> regressions, and behaviour which is only an +accidental artifact of the
> current implementation.

While I agree that this is a proper description of the current state of the 
DRM/KMS subsystem, I don't like how it implies that shipping code is an 
acceptable replacement for documentation. We need to aim at documenting the 
DRM/KMS userspace API to a level of detail that will cover the vast majority 
of cases (be they corner or round), if not all of them.

> +Without access to the full source code of all userspace users that means it
> +becomes impossible to change the implementation details, since userspace
> could +depend upon the accidental behaviour of the current implementation
> in minute +details. And debugging such regressions without access to source
> code is pretty +much impossible. As a consequence this means:
> +
> +- The Linux kernel's "no regression" policy holds in practice only for
> +  open-source userspace of the DRM subsystem. DRM developers are perfectly
> fine +  if closed-source blob drivers in userspace use the same uAPI as the
> open +  drivers,

In which category do you put the open-source userspace code that is not part 
of the mainline version of a major userspace framework (X11, Weston, Android 
hwcomposer, ...) ? I'm thinking about open-source vendor forks of those 
projects, or home-brew implementation of a tailor-made display stack for a 
product (I'm intentionally using the word display instead of GFX here, if a 
full tailor-made GFX stack including 3D rendering is quite unlikely, a display 
stack or even just an application driving the display through the DRM/KMS API 
is much easier to write from scratch).

> but they must do so in the exact same way as the open drivers.
> +  Creative (ab)use of the interfaces will, and in the past routinely has,
> lead +  to breakage.

This in my opinion calls for documentation, otherwise how can userspace 
developers tell what is an abuse without copying open-source code verbatim ?

> +- Any new userspace interface must have an open-source implementation as
> +  demonstration vehicle.
> +
> +The other reason for requiring open-source userspace is uAPI review. Since
> the +kernel and userspace parts of a GFX stack must work together so
> closely, code +review can only assess whether a new interface achieves its
> goals by looking at +both sides. Making sure that the interface indeed
> covers the use-case fully +leads to a few additional requirements:
> +
> +- The open-source userspace must not be a toy/test application, but the
> real +  thing. Specifically it needs to handle all the usual error and
> corner cases. +  These are often the places where new uAPI falls apart and
> hence essential to +  assess the fitness of a proposed interface.
> +
> +- The userspace side must be fully reviewed and tested to the standards of
> that +  userspace project. For e.g. mesa this means piglit testcases and
> review on the +  mailing list. This is again to ensure that the new
> interface actually gets the +  job done.
> +
> +- The userspace patches must be against the canonical upstream, not some
> vendor +  fork. This is to make sure that no one cheats on the review and
> testing +  requirements by doing a quick fork.
> +
> +- The kernel patch can only be merged after all the above requirements are
> met, +  but it **must** be merged **before** the userspace patches land.
> uAPI always flows +  from the kernel, doing things the other way round
> risks divergence of the uAPI +  definitions and header files.
> +
> +These are fairly steep requirements, but have grown out from years of
> shared +pain and experience with uAPI added hastily, and almost always
> regretted about +as fast. GFX devices change really fast, requiring a
> paradigm shift and entire +new set of uAPI interfaces every few years at
> least. Together with the Linux +kernel's guarantee to keep existing
> userspace running for 10+ years this is +already rather painful for the DRM
> subsystem, with multiple different uAPIs for +the same thing co-existing.
> If we'd add a few more complete mistakes into the +mix every year it would
> be entirely unmanagable.
> +
>  Render nodes
>  ============

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel