On Thu, Jun 13, 2013 at 2:52 PM, Rob Clark <robdclark@xxxxxxxxx> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
