From mboxrd@z Thu Jan 1 00:00:00 1970 From: daniel@ffwll.ch (Daniel Vetter) Date: Thu, 13 Jun 2013 14:58:27 +0200 Subject: [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver In-Reply-To: References: <20130610233611.GK18614@n2100.arm.linux.org.uk> <20130612134845.GQ18614@n2100.arm.linux.org.uk> <20130612164914.GT18614@n2100.arm.linux.org.uk> <20130612170512.GU18614@n2100.arm.linux.org.uk> <20130612194021.GX18614@n2100.arm.linux.org.uk> <20130612230057.GY18614@n2100.arm.linux.org.uk> <20130613111903.GA18614@n2100.arm.linux.org.uk> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, Jun 13, 2013 at 2:52 PM, Rob Clark wrote: > most of the embedded drivers should ignore the old_fb.. the API is a > bit odd but the purpose is to help drivers that need to pin/unpin the > gem objects backing the fb. The ones that do, do something like: > > foo_pin(new_fb); > foo_unpin(old_fb); > > if the two are the same, it works out. > > Note that even for the non error paths, new_fb and old_fb could be the same. > > Possibly something worth clarifying in the docs. One implication this has is that the crtc helper assumes that the driver modeset callbacks only fail before the driver internally has done the pin/unpin dance. If the new fb is already the one which should be unpinned and the modeset fails after that point, then we'll leak stuff. So drivers need to ensure that they undo any pin/unpins (and other fb refcounted resource handling) if they're ->modeset callback fails. In the new shiny drm/i915 modeset helper code we've flipped around those semantics, but imo for the crtc helper it does fit into the larger assumptions of the crtc helper code: - crtc helper code assumes that all ->disable callbacks are stateless - hence it can put the _new_ requested state into the sw tracking structures before the modeset starts so that ->mode_fixup callbacks could e.g. check the pixel format of the new fb. Flipping that around would remove one of the cornerstones of the crtc helpers, so imo doesn't make much sense. But as i915.ko demonstrates with a bit of effort it's no problem to have a completely different modeset sequence driver infrastructure. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH RFC 2/8] DRM: Armada: Add Armada DRM driver Date: Thu, 13 Jun 2013 14:58:27 +0200 Message-ID: References: <20130610233611.GK18614@n2100.arm.linux.org.uk> <20130612134845.GQ18614@n2100.arm.linux.org.uk> <20130612164914.GT18614@n2100.arm.linux.org.uk> <20130612170512.GU18614@n2100.arm.linux.org.uk> <20130612194021.GX18614@n2100.arm.linux.org.uk> <20130612230057.GY18614@n2100.arm.linux.org.uk> <20130613111903.GA18614@n2100.arm.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ie0-f169.google.com (mail-ie0-f169.google.com [209.85.223.169]) by gabe.freedesktop.org (Postfix) with ESMTP id E74E7E60DF for ; Thu, 13 Jun 2013 05:58:27 -0700 (PDT) Received: by mail-ie0-f169.google.com with SMTP id 10so12639730ied.0 for ; Thu, 13 Jun 2013 05:58:27 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Rob Clark Cc: dri-devel , Russell King - ARM Linux , Jason Cooper , "linux-arm-kernel@lists.infradead.org" , Sebastian Hesselbarth List-Id: dri-devel@lists.freedesktop.org On Thu, Jun 13, 2013 at 2:52 PM, Rob Clark wrote: > most of the embedded drivers should ignore the old_fb.. the API is a > bit odd but the purpose is to help drivers that need to pin/unpin the > gem objects backing the fb. The ones that do, do something like: > > foo_pin(new_fb); > foo_unpin(old_fb); > > if the two are the same, it works out. > > Note that even for the non error paths, new_fb and old_fb could be the same. > > Possibly something worth clarifying in the docs. One implication this has is that the crtc helper assumes that the driver modeset callbacks only fail before the driver internally has done the pin/unpin dance. If the new fb is already the one which should be unpinned and the modeset fails after that point, then we'll leak stuff. So drivers need to ensure that they undo any pin/unpins (and other fb refcounted resource handling) if they're ->modeset callback fails. In the new shiny drm/i915 modeset helper code we've flipped around those semantics, but imo for the crtc helper it does fit into the larger assumptions of the crtc helper code: - crtc helper code assumes that all ->disable callbacks are stateless - hence it can put the _new_ requested state into the sw tracking structures before the modeset starts so that ->mode_fixup callbacks could e.g. check the pixel format of the new fb. Flipping that around would remove one of the cornerstones of the crtc helpers, so imo doesn't make much sense. But as i915.ko demonstrates with a bit of effort it's no problem to have a completely different modeset sequence driver infrastructure. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch