Skip to content

Commit

Permalink
drm: create drm_framebuffer_lookup
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Daniel Vetter committed Jan 20, 2013
1 parent 4b096ac commit 786b99e
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 49 deletions.
109 changes: 72 additions & 37 deletions drivers/gpu/drm/drm_crtc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,11 +294,24 @@ static void drm_mode_object_put(struct drm_device *dev,
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))
Expand Down Expand Up @@ -358,6 +371,40 @@ static void drm_framebuffer_free(struct kref *kref)
fb->funcs->destroy(fb);
}

/**
* 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
Expand Down Expand Up @@ -1788,17 +1835,15 @@ int drm_mode_setplane(struct drm_device *dev, void *data,
}
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++)
Expand Down Expand Up @@ -1933,17 +1978,16 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
}
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);
Expand Down Expand Up @@ -2392,7 +2436,6 @@ int drm_mode_addfb2(struct drm_device *dev,
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;
Expand All @@ -2403,16 +2446,13 @@ int drm_mode_rmfb(struct drm_device *dev,
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)
Expand Down Expand Up @@ -2451,22 +2491,20 @@ int drm_mode_getfb(struct drm_device *dev,
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;

if (!drm_core_check_feature(dev, DRIVER_MODESET))
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;
Expand All @@ -2489,7 +2527,6 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
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;
Expand All @@ -2499,14 +2536,13 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
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;
Expand Down Expand Up @@ -3586,12 +3622,11 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
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;
Expand Down
24 changes: 12 additions & 12 deletions drivers/gpu/drm/vmwgfx/vmwgfx_ioctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,
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;
Expand Down Expand Up @@ -165,15 +165,15 @@ int vmw_present_ioctl(struct drm_device *dev, void *data,

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))
Expand Down Expand Up @@ -217,7 +217,7 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void *data,
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;
Expand Down Expand Up @@ -250,16 +250,16 @@ int vmw_present_readback_ioctl(struct drm_device *dev, void *data,

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;
Expand Down
2 changes: 2 additions & 0 deletions include/drm/drm_crtc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,8 @@ extern void drm_framebuffer_set_object(struct drm_device *dev,
extern int drm_framebuffer_init(struct drm_device *dev,
struct drm_framebuffer *fb,
const struct drm_framebuffer_funcs *funcs);
extern struct drm_framebuffer *drm_framebuffer_lookup(struct drm_device *dev,
uint32_t id);
extern void drm_framebuffer_unreference(struct drm_framebuffer *fb);
extern void drm_framebuffer_reference(struct drm_framebuffer *fb);
extern void drm_framebuffer_remove(struct drm_framebuffer *fb);
Expand Down

0 comments on commit 786b99e

Please sign in to comment.