[Intel-gfx] [PATCH] [drm/i915] Handle swap buffers in work proc instead of softirq

[Jesse Barnes]

On Friday, October 17, 2008 3:55 pm Keith Packard wrote:
> On Fri, 2008-10-17 at 15:23 -0700, Eric Anholt wrote:
> > On Fri, 2008-10-17 at 00:44 -0700, Keith Packard wrote:
> > > Swap buffers must hold both the DRM lock and the struct_mutex to access
> > > both drawable clip lists and the hardware ring. struct_mutex may only
> > > be acquired in a non-interrupt context. This means that vblank will be
> > > entirely executed outside of IRQ context, which is not ideal, but
> > > should avoid lengthy delays while the ring is full along with making it
> > > actually work reliably.
> >
> > OK, this looks a lot nicer.  I also like moving from planes to pipes in
> > vblank handling code, since it's pipes and not planes that produce
> > vblank events.  But I fear that there's some pipe/plane confusion going
> > on here.
> >
> > > @@ -165,7 +170,7 @@ static void i915_vblank_do_swaps(struct drm_device
> > > *dev) list_for_each_safe(list, tmp, &dev_priv->vbl_swaps.head) {
> > >  		drm_i915_vbl_swap_t *vbl_swap =
> > >  			list_entry(list, drm_i915_vbl_swap_t, head);
> > > -		int pipe = i915_get_pipe(dev, vbl_swap->plane);
> > > +		int pipe = vbl_swap->pipe;
> > >
> > >  		if ((counter[pipe] - vbl_swap->sequence) > (1<<23))
> > >  			continue;
> >
> > OK, so we're getting counter[pipe] = drm_vblank_count(dev, pipe)
> >
> > The prototype of that one is:
> >
> > u32 drm_vblank_count(struct drm_device *dev, int crtc)
> >
> > so you should be in the clear there since a crtc is a pipe not a plane.
>
> Alas, that would be cool, but the object exposed to user space is a
> plane, not a pipe, and hence the crtc in the drm code is a plane as
> well. I don't think we can change this without breaking the 2D driver,
> so we'll need some compatibility mechanism I fear.
>
> Sounds like my code is just broken here, unless we flip DRM from planes
> to pipes, along with the 2D driver. I'm up for that if you are; now that
> 965GM has no FBC support, we should have pipe == plane all the time.

No it's previous chips that have the LVDS on pipe B, FBC only on plane A 
limitation.  But the flipping only ever worked with drm.git master bits, 
which never went upstream.  So if you want to redefine the APIs that's still 
possible.

It doesn't matter which way we go as long as we're consistent.  There are only 
two places where you need to know the mapping:  FBC setup (and therefore 2D 
mode programming) and vblank interrupt time.  The rest of the code in Mesa 
and elsewhere can just deal with either planes or pipes all the time.

-- 
Jesse Barnes, Intel Open Source Technology Center