lkml.org 
[lkml]   [2009]   [Jul]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] vga: implements VGA arbitration on Linux
> +#ifndef __ARCH_HAS_VGA_ENABLE_RESOURCES
> +static inline void vga_enable_resources(struct pci_dev *pdev,
> + unsigned int rsrc)
> +{
> + struct pci_bus *bus;
> + struct pci_dev *bridge;
> + u16 cmd;
> +
> +#ifdef DEBUG
> + printk(KERN_DEBUG "%s\n", __func__);
> +#endif
> + pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> + if (rsrc & (VGA_RSRC_LEGACY_IO | VGA_RSRC_NORMAL_IO))
> + cmd |= PCI_COMMAND_IO;
> + if (rsrc & (VGA_RSRC_LEGACY_MEM | VGA_RSRC_NORMAL_MEM))
> + cmd |= PCI_COMMAND_MEMORY;
> + pci_write_config_word(pdev, PCI_COMMAND, cmd);

Locking question - what locks this lot against hotplug also touching
bridge settings ?

> +
> + if (!(rsrc & VGA_RSRC_LEGACY_MASK))
> + return;
> +
> + bus = pdev->bus;
> + while (bus) {
> + bridge = bus->self;
> + if (bridge) {
> + pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
> + &cmd);
> + if (!(cmd & PCI_BRIDGE_CTL_VGA)) {
> + cmd |= PCI_BRIDGE_CTL_VGA;
> + pci_write_config_word(bridge,
> + PCI_BRIDGE_CONTROL,
> + cmd);
> + }
> + }
> + bus = bus->parent;
> + }
> +}
> +#endif


> + /* The one who calls us should check for this, but lets be sure... */
> + if (pdev == NULL)
> + pdev = vga_default_device();

What if the BIOS provided device was hot unplugged ?

> + conflict = __vga_tryget(vgadev, rsrc);
> + spin_unlock_irqrestore(&vga_lock, flags);
> + if (conflict == NULL)
> + break;
> +
> +
> + /* We have a conflict, we wait until somebody kicks the
> + * work queue. Currently we have one work queue that we

If two drivers own half the resources and both are waiting for the rest
what handles the deadlock

> + * kick each time some resources are released, but it would
> + * be fairly easy to have a per device one so that we only
> + * need to attach to the conflicting device
> + */
> + init_waitqueue_entry(&wait, current);
> + add_wait_queue(&vga_wait_queue, &wait);
> + set_current_state(interruptible ?
> + TASK_INTERRUPTIBLE :
> + TASK_UNINTERRUPTIBLE);
> + if (signal_pending(current)) {
> + rc = -EINTR;
> + break;
> + }
> + schedule();
> + remove_wait_queue(&vga_wait_queue, &wait);
> + set_current_state(TASK_RUNNING);

Seems a very long winded way to write

wait_event_interruptible(...)


> + /* Allocate structure */
> + vgadev = kmalloc(sizeof(struct vga_device), GFP_KERNEL);
> + if (vgadev == NULL) {
> + /* What to do on allocation failure ? For now, let's
> + * just do nothing, I'm not sure there is anything saner
> + * to be done
> + */

If this is an "oh dear" moment then at least printk something

>
> + /* Set the client' lists of locks */
> + priv->target = vga_default_device(); /* Maybe this is still null! */

PCI device refcounting ?



\
 
 \ /
  Last update: 2009-07-14 16:37    [from the cache]
©2003-2011 Jasper Spaans