On Tue, 18 Aug 2009 16:24:14 +1000
Dave Airlie <[email protected]> wrote:
> > +#undef set_base
> > +
> > struct drm_prop_enum_list {
> > int type;
> > char *name;
> > @@ -342,6 +344,34 @@ void drm_framebuffer_cleanup(struct
> > drm_framebuffer *fb) EXPORT_SYMBOL(drm_framebuffer_cleanup);
> >
> > /**
> > + * drm_crtc_async_flip - do a set_base call from a work queue
> > + * @work: work struct
> > + *
> > + * Called when a set_base call is queued by the page flip code.
> > This
> > + * allows the flip ioctl itself to return immediately and allow
> > userspace
> > + * to continue working.
> > + */
> > +static void drm_crtc_async_flip(struct work_struct *work)
> > +{
> > + struct drm_crtc *crtc = container_of(work, struct drm_crtc,
> > async_flip);
> > + struct drm_device *dev = crtc->dev;
> > + struct drm_pending_flip *pending;
> > +
> > + BUG_ON(crtc->pending_flip == NULL);
> > +
> > + mutex_lock(&dev->struct_mutex);
>
> I'm not sure locking mode objects here will help though.
>
> I assume drm_crtc can't go away while this is scheduled.
>
>
> > + crtc->funcs->set_base(crtc, crtc->x, crtc->y, NULL);
> > +
> > + pending = crtc->pending_flip;
> > + crtc->pending_flip = NULL;
> > +
> > + pending->frame = drm_vblank_count(dev, crtc->pipe);
> > + list_add_tail(&pending->link, &dev->flip_list);
> > +
> > + mutex_unlock(&dev->struct_mutex);
> > +}
>
> <snipped>
> > + */
> > +int drm_mode_page_flip_ioctl(struct drm_device *dev, void *data,
> > + struct drm_file *file_priv)
> > +{
> > + struct drm_pending_flip *pending;
> > + struct drm_mode_page_flip *flip_data = data;
> > + struct drm_mode_object *drm_obj, *fb_obj;
> > + struct drm_crtc *crtc;
> > + int ret = 0;
> > +
> > + if (!(drm_core_check_feature(dev, DRIVER_MODESET)))
> > + return -ENODEV;
> > +
> > + /*
> > + * Reject unknown flags so future userspace knows what we
> > (don't)
> > + * support
> > + */
> > + if (flip_data->flags & (~DRM_MODE_PAGE_FLIP_FLAGS_MASK)) {
> > + DRM_DEBUG("bad page flip flags\n");
> > + return -EINVAL;
> > + }
> > +
> > + pending = kzalloc(sizeof *pending, GFP_KERNEL);
> > + if (pending == NULL)
> > + return -ENOMEM;
> > +
> > + mutex_lock(&dev->struct_mutex);
>
> I suspect this should be locking dev->mode_config.mutex
> which is what is need to protect mode objects, not struct_mutex.
Arg I think you're right... Maybe this is what Thomas was concerned
about.
--
Jesse Barnes, Intel Open Source Technology Center
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
--
_______________________________________________
Dri-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel
