drm: create drm_framebuffer_lookup
And replace all fb lookups with it. Also add a WARN to
drm_mode_object_find since that is now no longer the blessed interface
to look up an fb. And add kerneldoc to both functions.
This only updates all callsites, but immediately drops the acquired
refence again. Hence all callers still rely on the fact that a mode fb
can't disappear while they're holding the struct mutex. Subsequent
patches will instate proper use of refcounts, and then rework the rmfb
and unref code to no longer serialize fb destruction with the
mode_config lock. We don't want that since otherwise a compositor
might end up stalling for a few frames in rmfb.
v2: Don't use kref_get_unless_zero - Greg KH doesn't like that kind of
interface.
Reviewed-by: Rob Clark <rob@ti.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 13a3d34..f2ccda8 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -294,11 +294,24 @@
mutex_unlock(&dev->mode_config.idr_mutex);
}
+/**
+ * drm_mode_object_find - look up a drm object with static lifetime
+ * @dev: drm device
+ * @id: id of the mode object
+ * @type: type of the mode object
+ *
+ * Note that framebuffers cannot be looked up with this functions - since those
+ * are reference counted, they need special treatment.
+ */
struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
uint32_t id, uint32_t type)
{
struct drm_mode_object *obj = NULL;
+ /* Framebuffers are reference counted and need their own lookup
+ * function.*/
+ WARN_ON(type == DRM_MODE_OBJECT_FB);
+
mutex_lock(&dev->mode_config.idr_mutex);
obj = idr_find(&dev->mode_config.crtc_idr, id);
if (!obj || (obj->type != type) || (obj->id != id))
@@ -359,6 +372,40 @@
}
/**
+ * drm_framebuffer_lookup - look up a drm framebuffer and grab a reference
+ * @dev: drm device
+ * @id: id of the fb object
+ *
+ * If successful, this grabs an additional reference to the framebuffer -
+ * callers need to make sure to eventually unreference the returned framebuffer
+ * again.
+ */
+struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev,
+ uint32_t id)
+{
+ struct drm_mode_object *obj = NULL;
+ struct drm_framebuffer *fb;
+
+ mutex_lock(&dev->mode_config.fb_lock);
+
+ mutex_lock(&dev->mode_config.idr_mutex);
+ obj = idr_find(&dev->mode_config.crtc_idr, id);
+ if (!obj || (obj->type != DRM_MODE_OBJECT_FB) || (obj->id != id))
+ fb = NULL;
+ else
+ fb = obj_to_fb(obj);
+ mutex_unlock(&dev->mode_config.idr_mutex);
+
+ if (fb)
+ kref_get(&fb->refcount);
+
+ mutex_unlock(&dev->mode_config.fb_lock);
+
+ return fb;
+}
+EXPORT_SYMBOL(drm_framebuffer_lookup);
+
+/**
* drm_framebuffer_unreference - unref a framebuffer
* @fb: framebuffer to unref
*
@@ -1788,17 +1835,15 @@
}
crtc = obj_to_crtc(obj);
- mutex_lock(&dev->mode_config.fb_lock);
- obj = drm_mode_object_find(dev, plane_req->fb_id,
- DRM_MODE_OBJECT_FB);
- mutex_unlock(&dev->mode_config.fb_lock);
- if (!obj) {
+ fb = drm_framebuffer_lookup(dev, plane_req->fb_id);
+ if (!fb) {
DRM_DEBUG_KMS("Unknown framebuffer ID %d\n",
plane_req->fb_id);
ret = -ENOENT;
goto out;
}
- fb = obj_to_fb(obj);
+ /* fb is protect by the mode_config lock, so drop the ref immediately */
+ drm_framebuffer_unreference(fb);
/* Check whether this plane supports the fb pixel format. */
for (i = 0; i < plane->format_count; i++)
@@ -1933,17 +1978,16 @@
}
fb = crtc->fb;
} else {
- mutex_lock(&dev->mode_config.fb_lock);
- obj = drm_mode_object_find(dev, crtc_req->fb_id,
- DRM_MODE_OBJECT_FB);
- mutex_unlock(&dev->mode_config.fb_lock);
- if (!obj) {
+ fb = drm_framebuffer_lookup(dev, crtc_req->fb_id);
+ if (!fb) {
DRM_DEBUG_KMS("Unknown FB ID%d\n",
crtc_req->fb_id);
ret = -EINVAL;
goto out;
}
- fb = obj_to_fb(obj);
+ /* fb is protect by the mode_config lock, so drop the
+ * ref immediately */
+ drm_framebuffer_unreference(fb);
}
mode = drm_mode_create(dev);
@@ -2392,7 +2436,6 @@
int drm_mode_rmfb(struct drm_device *dev,
void *data, struct drm_file *file_priv)
{
- struct drm_mode_object *obj;
struct drm_framebuffer *fb = NULL;
struct drm_framebuffer *fbl = NULL;
uint32_t *id = data;
@@ -2403,16 +2446,13 @@
return -EINVAL;
drm_modeset_lock_all(dev);
- mutex_lock(&dev->mode_config.fb_lock);
- obj = drm_mode_object_find(dev, *id, DRM_MODE_OBJECT_FB);
- /* TODO check that we really get a framebuffer back. */
- if (!obj) {
- mutex_unlock(&dev->mode_config.fb_lock);
+ fb = drm_framebuffer_lookup(dev, *id);
+ if (!fb) {
ret = -EINVAL;
goto out;
}
- fb = obj_to_fb(obj);
- mutex_unlock(&dev->mode_config.fb_lock);
+ /* fb is protect by the mode_config lock, so drop the ref immediately */
+ drm_framebuffer_unreference(fb);
mutex_lock(&file_priv->fbs_lock);
list_for_each_entry(fbl, &file_priv->fbs, filp_head)
@@ -2451,7 +2491,6 @@
void *data, struct drm_file *file_priv)
{
struct drm_mode_fb_cmd *r = data;
- struct drm_mode_object *obj;
struct drm_framebuffer *fb;
int ret = 0;
@@ -2459,14 +2498,13 @@
return -EINVAL;
drm_modeset_lock_all(dev);
- mutex_lock(&dev->mode_config.fb_lock);
- obj = drm_mode_object_find(dev, r->fb_id, DRM_MODE_OBJECT_FB);
- mutex_unlock(&dev->mode_config.fb_lock);
- if (!obj) {
+ fb = drm_framebuffer_lookup(dev, r->fb_id);
+ if (!fb) {
ret = -EINVAL;
goto out;
}
- fb = obj_to_fb(obj);
+ /* fb is protect by the mode_config lock, so drop the ref immediately */
+ drm_framebuffer_unreference(fb);
r->height = fb->height;
r->width = fb->width;
@@ -2489,7 +2527,6 @@
struct drm_clip_rect __user *clips_ptr;
struct drm_clip_rect *clips = NULL;
struct drm_mode_fb_dirty_cmd *r = data;
- struct drm_mode_object *obj;
struct drm_framebuffer *fb;
unsigned flags;
int num_clips;
@@ -2499,14 +2536,13 @@
return -EINVAL;
drm_modeset_lock_all(dev);
- mutex_lock(&dev->mode_config.fb_lock);
- obj = drm_mode_object_find(dev, r->fb_id, DRM_MODE_OBJECT_FB);
- mutex_unlock(&dev->mode_config.fb_lock);
- if (!obj) {
+ fb = drm_framebuffer_lookup(dev, r->fb_id);
+ if (!fb) {
ret = -EINVAL;
goto out_err1;
}
- fb = obj_to_fb(obj);
+ /* fb is protect by the mode_config lock, so drop the ref immediately */
+ drm_framebuffer_unreference(fb);
num_clips = r->num_clips;
clips_ptr = (struct drm_clip_rect __user *)(unsigned long)r->clips_ptr;
@@ -3586,12 +3622,11 @@
if (crtc->funcs->page_flip == NULL)
goto out;
- mutex_lock(&dev->mode_config.fb_lock);
- obj = drm_mode_object_find(dev, page_flip->fb_id, DRM_MODE_OBJECT_FB);
- mutex_unlock(&dev->mode_config.fb_lock);
- if (!obj)
+ fb = drm_framebuffer_lookup(dev, page_flip->fb_id);
+ if (!fb)
goto out;
- fb = obj_to_fb(obj);
+ /* fb is protect by the mode_config lock, so drop the ref immediately */
+ drm_framebuffer_unreference(fb);
hdisplay = crtc->mode.hdisplay;
vdisplay = crtc->mode.vdisplay;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
index 0d6a161..1b8f428 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
@@ -131,7 +131,7 @@
struct vmw_master *vmaster = vmw_master(file_priv->master);
struct drm_vmw_rect __user *clips_ptr;
struct drm_vmw_rect *clips = NULL;
- struct drm_mode_object *obj;
+ struct drm_framebuffer *fb;
struct vmw_framebuffer *vfb;
struct vmw_resource *res;
uint32_t num_clips;
@@ -165,15 +165,15 @@
drm_modeset_lock_all(dev);
- mutex_lock(&dev->mode_config.fb_lock);
- obj = drm_mode_object_find(dev, arg->fb_id, DRM_MODE_OBJECT_FB);
- mutex_unlock(&dev->mode_config.fb_lock);
- if (!obj) {
+ fb = drm_framebuffer_lookup(dev, arg->fb_id);
+ if (!fb) {
DRM_ERROR("Invalid framebuffer id.\n");
ret = -EINVAL;
goto out_no_fb;
}
- vfb = vmw_framebuffer_to_vfb(obj_to_fb(obj));
+ /* fb is protect by the mode_config lock, so drop the ref immediately */
+ drm_framebuffer_unreference(fb);
+ vfb = vmw_framebuffer_to_vfb(fb);
ret = ttm_read_lock(&vmaster->lock, true);
if (unlikely(ret != 0))
@@ -217,7 +217,7 @@
struct vmw_master *vmaster = vmw_master(file_priv->master);
struct drm_vmw_rect __user *clips_ptr;
struct drm_vmw_rect *clips = NULL;
- struct drm_mode_object *obj;
+ struct drm_framebuffer *fb;
struct vmw_framebuffer *vfb;
uint32_t num_clips;
int ret;
@@ -250,16 +250,16 @@
drm_modeset_lock_all(dev);
- mutex_lock(&dev->mode_config.fb_lock);
- obj = drm_mode_object_find(dev, arg->fb_id, DRM_MODE_OBJECT_FB);
- mutex_unlock(&dev->mode_config.fb_lock);
- if (!obj) {
+ fb = drm_framebuffer_lookup(dev, arg->fb_id);
+ if (!fb) {
DRM_ERROR("Invalid framebuffer id.\n");
ret = -EINVAL;
goto out_no_fb;
}
+ /* fb is protect by the mode_config lock, so drop the ref immediately */
+ drm_framebuffer_unreference(fb);
- vfb = vmw_framebuffer_to_vfb(obj_to_fb(obj));
+ vfb = vmw_framebuffer_to_vfb(fb);
if (!vfb->dmabuf) {
DRM_ERROR("Framebuffer not dmabuf backed.\n");
ret = -EINVAL;