Re: [PATCH 01/11] drm: add plane support

[Jesse Barnes <jbarnes@virtuousgeek.org> (by way of Jesse Barnes <jbarnes@virtuousgeek.org>)]

On Tue, 25 Oct 2011 13:58:55 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, Oct 25, 2011 at 11:46:56AM +0200, Jesse Barnes wrote:
> > Planes are a bit like half-CRTCs.  They have a location and fb, but
> > don't drive outputs directly.  Add support for handling them to the
> > core KMS code.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> As discussed with Jesse on irc, drm fb handling is fragile. Current
> rules:
> - fbs are not reference counted, hence when destroying we need to
> disable all crtcs (and now also planes) that use them.
> drm_framebuffer_cleanup does that atm
> - drivers that hold onto fbs after the kms core drops the
> corresponding pointer needs to hold a ref onto the underlying backing
> storage (like e.g. for pageflip on the to-be-flipped-out fb as long
> as it might still be scanned out).
> 
> We need proper refcounting for these ... But for now this patch is
> missing the plane cleanup in drm_framebuffer_cleanup.

Ah yeah that's a better place for the disable plane call I currently
have in the intel specific code...  I'll fix up and test.

> Otherwise I think going with just the src and dst rect for set_plane
> is about the only sensible thing given the crazy hw out there. But I
> lack the knowledge about that kind of hw (and video stuff in
> general), so I'll refrain from slapping my r-b on these two.

Yeah, I think the drivers just need to be able to calculate the scaling
level from the params and program them into whatever regs they happen
to have (on Intel fortunately the scaling is figured out by hw when we
program in the source & dest values, subject to some restrictions).


Thanks,
Jesse