Skip to content

Commit

Permalink
staging: udlfb: fix incorrect fb_defio implementation for multiple fr…
Browse files Browse the repository at this point in the history 
…amebuffers

Fix fb_defio bug in udlfb for multiple framebuffers

Previously when using fb_defio with multiple DisplayLink attached displays
udlfb incorrectly used a shared datastructure where it needed to have
one per fb_info struct. This was completely broken and caused all kinds of
problems, including rendering to one framebuffer affecting others.

Signed-off-by: Bernie Thompson <bernie@plugable.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
Bernie Thompson authored and Greg Kroah-Hartman committed Sep 6, 2010
1 parent d5fa133 commit 5bea1fb
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 25 deletions.
81 changes: 56 additions & 25 deletions drivers/staging/udlfb/udlfb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,6 @@ static int dlfb_submit_urb(struct dlfb_data *dev, struct urb * urb, size_t len);
static int dlfb_alloc_urb_list(struct dlfb_data *dev, int count, size_t size);
static void dlfb_free_urb_list(struct dlfb_data *dev);

/* other symbols with dependents */
#ifdef CONFIG_FB_DEFERRED_IO
static struct fb_deferred_io dlfb_defio;
#endif

/*
* All DisplayLink bulk operations start with 0xAF, followed by specific code
* All operations are written to buffers which then later get sent to device
Expand Down Expand Up @@ -481,7 +476,7 @@ static void dlfb_compress_hline(
* (that we can only write to, slowly, and can never read), and (optionally)
* our shadow copy that tracks what's been sent to that hardware buffer.
*/
static void dlfb_render_hline(struct dlfb_data *dev, struct urb **urb_ptr,
static int dlfb_render_hline(struct dlfb_data *dev, struct urb **urb_ptr,
const char *front, char **urb_buf_ptr,
u32 byte_offset, u32 byte_width,
int *ident_ptr, int *sent_ptr)
Expand Down Expand Up @@ -523,18 +518,20 @@ static void dlfb_render_hline(struct dlfb_data *dev, struct urb **urb_ptr,
if (cmd >= cmd_end) {
int len = cmd - (u8 *) urb->transfer_buffer;
if (dlfb_submit_urb(dev, urb, len))
return; /* lost pixels is set */
return 1; /* lost pixels is set */
*sent_ptr += len;
urb = dlfb_get_urb(dev);
if (!urb)
return; /* lost_pixels is set */
return 1; /* lost_pixels is set */
*urb_ptr = urb;
cmd = urb->transfer_buffer;
cmd_end = &cmd[urb->transfer_buffer_length];
}
}

*urb_buf_ptr = cmd;

return 0;
}

int dlfb_handle_damage(struct dlfb_data *dev, int x, int y,
Expand Down Expand Up @@ -571,9 +568,11 @@ int dlfb_handle_damage(struct dlfb_data *dev, int x, int y,
const int line_offset = dev->info->fix.line_length * i;
const int byte_offset = line_offset + (x * BPP);

dlfb_render_hline(dev, &urb, (char *) dev->info->fix.smem_start,
if (dlfb_render_hline(dev, &urb,
(char *) dev->info->fix.smem_start,
&cmd, byte_offset, width * BPP,
&bytes_identical, &bytes_sent);
&bytes_identical, &bytes_sent))
goto error;
}

if (cmd > (char *) urb->transfer_buffer) {
Expand All @@ -584,6 +583,7 @@ int dlfb_handle_damage(struct dlfb_data *dev, int x, int y,
} else
dlfb_urb_completion(urb);

error:
atomic_add(bytes_sent, &dev->bytes_sent);
atomic_add(bytes_identical, &dev->bytes_identical);
atomic_add(width*height*2, &dev->bytes_rendered);
Expand Down Expand Up @@ -722,9 +722,10 @@ static void dlfb_dpy_deferred_io(struct fb_info *info,
/* walk the written page list and render each to device */
list_for_each_entry(cur, &fbdefio->pagelist, lru) {

dlfb_render_hline(dev, &urb, (char *) info->fix.smem_start,
if (dlfb_render_hline(dev, &urb, (char *) info->fix.smem_start,
&cmd, cur->index << PAGE_SHIFT,
PAGE_SIZE, &bytes_identical, &bytes_sent);
PAGE_SIZE, &bytes_identical, &bytes_sent))
goto error;
bytes_rendered += PAGE_SIZE;
}

Expand All @@ -736,6 +737,7 @@ static void dlfb_dpy_deferred_io(struct fb_info *info,
} else
dlfb_urb_completion(urb);

error:
atomic_add(bytes_sent, &dev->bytes_sent);
atomic_add(bytes_identical, &dev->bytes_identical);
atomic_add(bytes_rendered, &dev->bytes_rendered);
Expand Down Expand Up @@ -796,6 +798,16 @@ static int dlfb_ops_ioctl(struct fb_info *info, unsigned int cmd,
/* TODO: Help propose a standard fb.h ioctl to report mmap damage */
if (cmd == DLFB_IOCTL_REPORT_DAMAGE) {

/*
* If we have a damage-aware client, turn fb_defio "off"
* To avoid perf imact of unecessary page fault handling.
* Done by resetting the delay for this fb_info to a very
* long period. Pages will become writable and stay that way.
* Reset to normal value when all clients have closed this fb.
*/
if (info->fbdefio)
info->fbdefio->delay = DL_DEFIO_WRITE_DISABLE;

area = (struct dloarea *)arg;

if (area->x < 0)
Expand Down Expand Up @@ -871,8 +883,18 @@ static int dlfb_ops_open(struct fb_info *info, int user)

#ifdef CONFIG_FB_DEFERRED_IO
if (fb_defio && (info->fbdefio == NULL)) {
/* enable defio */
info->fbdefio = &dlfb_defio;
/* enable defio at last moment if not disabled by client */

struct fb_deferred_io *fbdefio;

fbdefio = kmalloc(GFP_KERNEL, sizeof(struct fb_deferred_io));

if (fbdefio) {
fbdefio->delay = DL_DEFIO_WRITE_DELAY;
fbdefio->deferred_io = dlfb_dpy_deferred_io;
}

info->fbdefio = fbdefio;
fb_deferred_io_init(info);
}
#endif
Expand Down Expand Up @@ -906,6 +928,13 @@ static void dlfb_free(struct kref *kref)
kfree(dev);
}

static void dlfb_release_urb_work(struct work_struct *work)
{
struct urb_node *unode = container_of(work, struct urb_node,
release_urb_work.work);

up(&unode->dev->urbs.limit_sem);
}

static void dlfb_free_framebuffer_work(struct work_struct *work)
{
Expand Down Expand Up @@ -1415,15 +1444,6 @@ static struct device_attribute fb_device_attrs[] = {
__ATTR(metrics_reset, S_IWUGO, NULL, metrics_reset_store),
};

#ifdef CONFIG_FB_DEFERRED_IO

static struct fb_deferred_io dlfb_defio = {
.delay = 5,
.deferred_io = dlfb_dpy_deferred_io,
};

#endif

/*
* This is necessary before we can communicate with the display controller.
*/
Expand Down Expand Up @@ -1730,7 +1750,14 @@ static void dlfb_urb_completion(struct urb *urb)
dev->urbs.available++;
spin_unlock_irqrestore(&dev->urbs.lock, flags);

up(&dev->urbs.limit_sem);
/*
* When using fb_defio, we deadlock if up() is called
* while another is waiting. So queue to another process.
*/
if (fb_defio)
schedule_delayed_work(&unode->release_urb_work, 0);
else
up(&dev->urbs.limit_sem);
}

static void dlfb_free_urb_list(struct dlfb_data *dev)
Expand Down Expand Up @@ -1789,6 +1816,9 @@ static int dlfb_alloc_urb_list(struct dlfb_data *dev, int count, size_t size)
break;
unode->dev = dev;

INIT_DELAYED_WORK(&unode->release_urb_work,
dlfb_release_urb_work);

urb = usb_alloc_urb(0, GFP_KERNEL);
if (!urb) {
kfree(unode);
Expand Down Expand Up @@ -1835,7 +1865,8 @@ static struct urb *dlfb_get_urb(struct dlfb_data *dev)
ret = down_timeout(&dev->urbs.limit_sem, GET_URB_TIMEOUT);
if (ret) {
atomic_set(&dev->lost_pixels, 1);
dl_err("wait for urb interrupted: %x\n", ret);
dl_warn("wait for urb interrupted: %x available: %d\n",
ret, dev->urbs.available);
goto error;
}

Expand Down
4 changes: 4 additions & 0 deletions drivers/staging/udlfb/udlfb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ struct dloarea {
struct urb_node {
struct list_head entry;
struct dlfb_data *dev;
struct delayed_work release_urb_work;
struct urb *urb;
};

Expand Down Expand Up @@ -87,6 +88,9 @@ struct dlfb_data {
#define MIN_RAW_PIX_BYTES 2
#define MIN_RAW_CMD_BYTES (RAW_HEADER_BYTES + MIN_RAW_PIX_BYTES)

#define DL_DEFIO_WRITE_DELAY 5 /* fb_deferred_io.delay in jiffies */
#define DL_DEFIO_WRITE_DISABLE (HZ*60) /* "disable" with long delay */

/* remove these once align.h patch is taken into kernel */
#define DL_ALIGN_UP(x, a) ALIGN(x, a)
#define DL_ALIGN_DOWN(x, a) ALIGN(x-(a-1), a)
Expand Down

0 comments on commit 5bea1fb

Please sign in to comment.