From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 01/19] drm/doc: Clarify the dumb object interfaces Date: Thu, 23 Jan 2014 14:46:40 +0100 Message-ID: <20140123134640.GX9772@phenom.ffwll.local> References: <1390467164-951-1-git-send-email-daniel.vetter@ffwll.ch> <1755306.LJ2EgIi7Xi@avalon> <20140123124731.GV9772@phenom.ffwll.local> <1805347.h4MtSGFZv1@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f172.google.com (mail-we0-f172.google.com [74.125.82.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 7F46511D39B for ; Thu, 23 Jan 2014 05:46:45 -0800 (PST) Received: by mail-we0-f172.google.com with SMTP id q58so1188135wes.31 for ; Thu, 23 Jan 2014 05:46:44 -0800 (PST) Content-Disposition: inline In-Reply-To: <1805347.h4MtSGFZv1@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: Laurent Pinchart Cc: Daniel Vetter , DRI Development List-Id: dri-devel@lists.freedesktop.org On Thu, Jan 23, 2014 at 01:56:51PM +0100, Laurent Pinchart wrote: > > > > > > > > Drivers must first validate the requested frame buffer > > > > parameters passed > > > > @@ -1052,6 +998,71 @@ int max_width, max_height; > > > > drm_framebuffer_unregister_private. > > > > > > > > > > > > + Dumb GEM Objects > > > > + > > > > + The KMS API doesn't standardize backing storage object creation and > > > > > > Strictly speaking isn't it the DRM API that's responsible for memory > > > management, not the KMS API ? > > > > The driver's private api is responsible for memory management, but the > > crucial thing here is that the KMS ioctls don't mandate anything specific > > (beyong that it needs to use uint32_t for handles). > > Sure, but my point was that I believe memory management is the responsibility > of DRM, not KMS. It of course needs to be driver-specific. Well imo the dumb ioctls are part of kms. DRM itself doesn't have any memory management interfaces (at least if you ignore all the horror stories around legacy ums/dri1 drivers). My reasons are: - If you implement a kms driver you really should implement the dumb interfaces. Even when you have almost no memory management like the simpledrm driver. - If you have a driver with memory management and command submission but no KMS, there's no reason at all to implement the dumb interfaces. - ARM people abused dumb buffers for accelaration "because it worked", so imo moving it's documentation as far away as possible from the memory management section is a feature. So the dumb stuff is a KMS interface to abstract away the lowest common denominator between all kms drivers. Actually memory manament needs a real interface, and is obviously separate. > > > > + leaves it to driver-specific ioctls. Furthermore actually creating a > > > > + buffer object even for GEM-based drivers is done through a > > > > + driver-specific ioctl - GEM only has a common userspace interface for > > > > + sharing and destroying objects. While not an issue for full-fledged > > > > + graphics stacks that include device-specific userspace components (in > > > > + libdrm for instance), this limit makes DRM-based early boot graphics > > > > + unnecessarily complex. > > > > + > > > > > > This feels a bit out of place, can't we leave the section where it was ? > > > > Imo the justification for why we have the dumb ioctls should be here. And > > I wanted to mention both that KMS doesn't mandate a particular bo > > interface like GEM and that on top GEM wouldn't even provide a common > > allocation function anyway. > > I agree that a justification here is a good idea, but I would just add one > paragraph that references the dumb GEM objects section instead of scattering > GEM documentation around. I've pretty much removed all mention of dumb gem objects ;-) There's one mention of dumb_create left in the GEM/memory management section, but that one is just an example for the lifetime and reference counting rules. So not relevant. > > > But besides that I think there's some room for improvement in the GEM > > section to clarify what is the actual core interfaces, what is more helper > > library in nature and what in GEM is more just a common design pattern for > > driver ioctls but not specified in a mandatory way anywhere. E.g. atm all > > drivers which implement a GEM interface (radeon, i915, ...) have a mostly > > implicitly synchronizing buffer access interface, but there's nothing in > > GEM mandating this. Or the usual confusing between TTM directly exposed to > > userspace and TTM hidden behind a GEM-based ioctl interface. > > I agree, the GEM section should be improved, and TTM documentation would be > nice as well. I'm lacking time though (as well as knowledge about TTM). I have a few ideas, but I think I'll postpone this until I get around to documenting the i915 GEM code a bit. Having a concrete driver to talk about should help greatly to separate common concepts from artifacts of the i915 implementation. I guess that review will also lead to some abi cleanups to remove i915-ism from core gem. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch