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

From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
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>,
	"alexander.deucher@amd.com" <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, 2 Jan 2017 09:09:32 +0100	[thread overview]
Message-ID: <CAKMK7uFogorPp559eK8UA2AyMOyO=-vEXKzV=e=P8qT0KV-7ow@mail.gmail.com> (raw)
In-Reply-To: <1733448.yI717seRR3@avalon>

On Sun, Jan 1, 2017 at 11:40 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> 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.

I agree that this is a good idea, the trouble is in convincing
everyone else, and more important, that this is important enough that
they should spent time typing these docs. I think the only way to do
that is to 1) type those docs for existing ioctls yourself, and then
start and RFC to make it a standard. For properties I'm already
starting with at least some not-too-formal docs within the existing
kernel-doc stuff. Not there yet either. I somewhat plan to get around
to ioctl in the next few years, but would be real awesome if someone
beats me to it ;-)

>> +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).

I think if it's just a vendor fork of an existing open-source project,
then that existing open-source project should be the target. The idea
behind requiring review&ready for acceptance by the userspace side is
to make sure it'll be useful for more than just 1 vendor, just
applying it to a vendor fork somewhat defeats that idea. Best option
would be to either reintegrate the vendor for, or if that's not
possible, have some shared library or other means of sharing code.
Afaiui that's the entire point of libweston, so assuming your vendor
fork is tracking upstream libweston (it really should) this shouldn't
be a problem. If there's no direct code sharing, but at least common
history, porting the patches to upstream as demonstration vehicle also
shouldn't be that bad a burden.

An entirely different case is 2 independent codebases that solve the
exact same problem. We have no precedence for that, but I expect we'll
have that real soon with the community radv vulkan driver vs. the one
from amd (that should be released as open source any minute now).
Personally I'm leaning towards "why don't you just collaborate" and
trying to look cute&cuddly, but no sure where that one will fall tbh.
We will see soon.

>> 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 ?

If there's a regression, and code disagrees with the documentation,
the code wins. I agree that good docs would help, but the reality is
that very often you get to review 10+ years of git history in a bunch
of projects. I & others have done this plenty of times, and we
generally get things mostly right. But again not having tons of forks
and different implementations helps a lot with this, so "pls
collaborate" is a good idea (for the community overall, and that's the
point here) too. We have a bunch of forks and independent
reimplemenations (e.g. the gallium intel driver), but most often those
forks tend to lack in features so much that they're not hitting any of
the uapi corner cases that might break. I'll be interesting when this
happens for the first time for real, but personally I really hope this
doesn't happen, and that everyone understands the long-term benefits
of collaborating.

Cheers, 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
https://lists.freedesktop.org/mailman/listinfo/dri-devel