On Thursday, February 19, 2009 9:46:06 pm Zou, Nanhai wrote:
> >+ if ((pipe = flip_data->pipe) > 1) {
> >+ DRM_ERROR("bad pipe\n");
> >+ return -EINVAL;
> >+ }
>
> pipe should be unsigned int, if it is int, this check should be if
> (pipe >
> 1 || pipe < 0)
Yeah, fixed (the ioctl arg is already unsigned, so it should match).
> >+ spin_lock_irqsave(&dev_priv->vblank_lock, flags);
> >+
> >+ BEGIN_LP_RING(4);
>
> This could be sleep in atomic because BEGIN_LP_RING may sleep.
Ok, I'll pull that bit out... but doing that introduces a small race. If we
lose the race (the flip happens immediately and we get interrupted before
adding the object to the vblank queue) the object will be stalled for one
extra frame.
> >+ obj_priv = object_list[i]->driver_private;
> >+ if (i915_gem_flip_pending(object_list[i]))
> >+ wait_for_completion(&obj_priv->vblank);
>
> I am wondering if this will block other clients execbuffer to run thus
> hurt their performance because struct_mutex is hold.
Yeah, that's a good point. I'll see if we can drop it here and restart the
execbuffer if that happens.
Thanks for checking it out; new patch on its way soon.
--
Jesse Barnes, Intel Open Source Technology Center
------------------------------------------------------------------------------
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
--
_______________________________________________
Dri-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel
