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

[Keith Packard]

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.

> Here you've got count[pipe] = drm_vblank_count(dev, plane).

I think that's right (unfortunately).

I'll fix the first one.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20081017/f20152dc/attachment.sig>