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    [W:0.035 / U:1.796 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site