History

drm: refcnt drm_framebuffer (v4.1)

This simplifies drm fb lifetime, and if the crtc/plane needs to hold a ref to the fb when disabling a pipe until the next vblank, this avoids the need to make disabling an overlay synchronous. This is a problem that shows up when userspace is using a drm plane to implement a hw cursor.. making overlay disable synchronous causes a performance problem when x11 is rapidly enabling/disabling the hw cursor. But not making it synchronous opens up a race condition for crashing if userspace turns around and immediately deletes the fb. Refcnt'ing the fb makes it possible to solve this problem. v1: original v2: add drm_framebuffer_remove() which is called in all paths where fb->funcs->destroy() was directly called before. This cleans up the CRTCs/planes that the fb was attached to. You should only directly use drm_framebuffer_unreference() if you are also using drm_framebuffer_reference() to keep a ref to the fb. v3: add comment explaining the fb refcount v4: remove duplicate 'list_del(&fb->filp_head)' [airlied: v4.1: fix local rejection]

Rob Clark committed 14 years ago

Commit f7eff601 parent 33cce6e4 files · +85 −16Browse files

Unified · Split

4 files
  1. drivers/gpu/drm/drm_crtc.c +67 −12
  2. drivers/gpu/drm/exynos/exynos_drm_fbdev.c +2 −2
  3. drivers/staging/omapdrm/omap_fbdev.c +2 −2
  4. include/drm/drm_crtc.h +14 −0
drivers/gpu/drm/drm_crtc.c+67−12
@@ -294,6 +294,8 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb, · 293 unmodified lines
294294 {
295295 int ret;
296296
297+ kref_init(&fb->refcount);
298+
297299 ret = drm_mode_object_get(dev, &fb->base, DRM_MODE_OBJECT_FB);
298300 if (ret)
299301 return ret;
@@ -307,6 +309,38 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb, · 7 unmodified lines
307309 }
308310 EXPORT_SYMBOL(drm_framebuffer_init);
309311
312+static void drm_framebuffer_free(struct kref *kref)
313+{
314+ struct drm_framebuffer *fb =
315+ container_of(kref, struct drm_framebuffer, refcount);
316+ fb->funcs->destroy(fb);
317+}
318+
319+/**
320+ * drm_framebuffer_unreference - unref a framebuffer
321+ *
322+ * LOCKING:
323+ * Caller must hold mode config lock.
324+ */
325+void drm_framebuffer_unreference(struct drm_framebuffer *fb)
326+{
327+ struct drm_device *dev = fb->dev;
328+ DRM_DEBUG("FB ID: %d\n", fb->base.id);
329+ WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
330+ kref_put(&fb->refcount, drm_framebuffer_free);
331+}
332+EXPORT_SYMBOL(drm_framebuffer_unreference);
333+
334+/**
335+ * drm_framebuffer_reference - incr the fb refcnt
336+ */
337+void drm_framebuffer_reference(struct drm_framebuffer *fb)
338+{
339+ DRM_DEBUG("FB ID: %d\n", fb->base.id);
340+ kref_get(&fb->refcount);
341+}
342+EXPORT_SYMBOL(drm_framebuffer_reference);
343+
310344 /**
311345 * drm_framebuffer_cleanup - remove a framebuffer object
312346 * @fb: framebuffer to remove
@@ -318,6 +352,32 @@ EXPORT_SYMBOL(drm_framebuffer_init); · 5 unmodified lines
318352 * it, setting it to NULL.
319353 */
320354 void drm_framebuffer_cleanup(struct drm_framebuffer *fb)
355+{
356+ struct drm_device *dev = fb->dev;
357+ /*
358+ * This could be moved to drm_framebuffer_remove(), but for
359+ * debugging is nice to keep around the list of fb's that are
360+ * no longer associated w/ a drm_file but are not unreferenced
361+ * yet. (i915 and omapdrm have debugfs files which will show
362+ * this.)
363+ */
364+ drm_mode_object_put(dev, &fb->base);
365+ list_del(&fb->head);
366+ dev->mode_config.num_fb--;
367+}
368+EXPORT_SYMBOL(drm_framebuffer_cleanup);
369+
370+/**
371+ * drm_framebuffer_remove - remove and unreference a framebuffer object
372+ * @fb: framebuffer to remove
373+ *
374+ * LOCKING:
375+ * Caller must hold mode config lock.
376+ *
377+ * Scans all the CRTCs and planes in @dev's mode_config. If they're
378+ * using @fb, removes it, setting it to NULL.
379+ */
380+void drm_framebuffer_remove(struct drm_framebuffer *fb)
321381 {
322382 struct drm_device *dev = fb->dev;
323383 struct drm_crtc *crtc;
@@ -350,11 +410,11 @@ void drm_framebuffer_cleanup(struct drm_framebuffer *fb) · 26 unmodified lines
350410 }
351411 }
352412
353− drm_mode_object_put(dev, &fb->base);
354− list_del(&fb->head);
355− dev->mode_config.num_fb--;
413+ list_del(&fb->filp_head);
414+
415+ drm_framebuffer_unreference(fb);
356416 }
357−EXPORT_SYMBOL(drm_framebuffer_cleanup);
417+EXPORT_SYMBOL(drm_framebuffer_remove);
358418
359419 /**
360420 * drm_crtc_init - Initialise a new CRTC object
@@ -1031,7 +1091,7 @@ void drm_mode_config_cleanup(struct drm_device *dev) · 670 unmodified lines
10311091 }
10321092
10331093 list_for_each_entry_safe(fb, fbt, &dev->mode_config.fb_list, head) {
1034− fb->funcs->destroy(fb);
1094+ drm_framebuffer_remove(fb);
10351095 }
10361096
10371097 list_for_each_entry_safe(plane, plt, &dev->mode_config.plane_list,
@@ -2337,11 +2397,7 @@ int drm_mode_rmfb(struct drm_device *dev, · 1299 unmodified lines
23372397 goto out;
23382398 }
23392399
2340− /* TODO release all crtc connected to the framebuffer */
2341− /* TODO unhock the destructor from the buffer object */
2342−
2343− list_del(&fb->filp_head);
2344− fb->funcs->destroy(fb);
2400+ drm_framebuffer_remove(fb);
23452401
23462402 out:
23472403 mutex_unlock(&dev->mode_config.mutex);
@@ -2491,8 +2547,7 @@ void drm_fb_release(struct drm_file *priv) · 143 unmodified lines
24912547
24922548 mutex_lock(&dev->mode_config.mutex);
24932549 list_for_each_entry_safe(fb, tfb, &priv->fbs, filp_head) {
2494− list_del(&fb->filp_head);
2495− fb->funcs->destroy(fb);
2550+ drm_framebuffer_remove(fb);
24962551 }
24972552 mutex_unlock(&dev->mode_config.mutex);
24982553 }
drivers/gpu/drm/exynos/exynos_drm_fbdev.c+2−2
@@ -266,8 +266,8 @@ static void exynos_drm_fbdev_destroy(struct drm_device *dev, · 265 unmodified lines
266266 /* release drm framebuffer and real buffer */
267267 if (fb_helper->fb && fb_helper->fb->funcs) {
268268 fb = fb_helper->fb;
269− if (fb && fb->funcs->destroy)
270− fb->funcs->destroy(fb);
269+ if (fb)
270+ drm_framebuffer_remove(fb);
271271 }
272272
273273 /* release linux framebuffer */
drivers/staging/omapdrm/omap_fbdev.c+2−2
@@ -276,7 +276,7 @@ fail: · 275 unmodified lines
276276 if (fbi)
277277 framebuffer_release(fbi);
278278 if (fb)
279− fb->funcs->destroy(fb);
279+ drm_framebuffer_remove(fb);
280280 }
281281
282282 return ret;
@@ -401,7 +401,7 @@ void omap_fbdev_free(struct drm_device *dev) · 118 unmodified lines
401401
402402 /* this will free the backing object */
403403 if (fbdev->fb)
404− fbdev->fb->funcs->destroy(fbdev->fb);
404+ drm_framebuffer_remove(fbdev->fb);
405405
406406 kfree(fbdev);
407407
include/drm/drm_crtc.h+14−0
@@ -218,6 +218,7 @@ struct drm_display_info { · 217 unmodified lines
218218 };
219219
220220 struct drm_framebuffer_funcs {
221+ /* note: use drm_framebuffer_remove() */
221222 void (*destroy)(struct drm_framebuffer *framebuffer);
222223 int (*create_handle)(struct drm_framebuffer *fb,
223224 struct drm_file *file_priv,
@@ -242,6 +243,16 @@ struct drm_framebuffer_funcs { · 18 unmodified lines
242243
243244 struct drm_framebuffer {
244245 struct drm_device *dev;
246+ /*
247+ * Note that the fb is refcounted for the benefit of driver internals,
248+ * for example some hw, disabling a CRTC/plane is asynchronous, and
249+ * scanout does not actually complete until the next vblank. So some
250+ * cleanup (like releasing the reference(s) on the backing GEM bo(s))
251+ * should be deferred. In cases like this, the driver would like to
252+ * hold a ref to the fb even though it has already been removed from
253+ * userspace perspective.
254+ */
255+ struct kref refcount;
245256 struct list_head head;
246257 struct drm_mode_object base;
247258 const struct drm_framebuffer_funcs *funcs;
@@ -919,6 +930,9 @@ extern void drm_framebuffer_set_object(struct drm_device *dev, · 671 unmodified lines
919930 extern int drm_framebuffer_init(struct drm_device *dev,
920931 struct drm_framebuffer *fb,
921932 const struct drm_framebuffer_funcs *funcs);
933+extern void drm_framebuffer_unreference(struct drm_framebuffer *fb);
934+extern void drm_framebuffer_reference(struct drm_framebuffer *fb);
935+extern void drm_framebuffer_remove(struct drm_framebuffer *fb);
922936 extern void drm_framebuffer_cleanup(struct drm_framebuffer *fb);
923937 extern int drmfb_probe(struct drm_device *dev, struct drm_crtc *crtc);
924938 extern int drmfb_remove(struct drm_device *dev, struct drm_framebuffer *fb);