On 06/14/10 16:18, Dave Airlie wrote:
On Thu, Jun 10, 2010 at 4:02 PM, Henry Zhao<henry.z...@oracle.com>  wrote:
Proposal for improving vgaarb arbitration method

It appears  that after session is up,  in most cases,  drivers only do
non-legacy accesses.
Non-legacy accesses do not need to block each other. Blocking arbitration is
needed
mostly for session initialization and exiting. To improve performance, we
need to treat
differently to legacy and non-legacy accesses, and allow non-legacy accesses
to proceed
concurrently among devices without blocking each other. Non-legacy accesses
is assumed
to be the default for operating functions after initialization. In case
legacy accesses are
necessary for some of them, drivers can redefine them per function group
bases.
Here are some details:

(1) New lock for non-legacy access

  Define another lock, vgadev->locks2 (locks2), for non-legacy access locking
  in addition to vgadev->locks (locks1), currently used for legacy access
  locking.

  Non-legacy access requests from a device that does not have legacy access
  decoding ability should always be honored without a need of acquiring a
lock.
  Non-legacy access requests from a device that has legacy access decoding
  ability needs to acquire locks2 before proceeding.

  Request for locks2 is blocked only when some other device already has
locks1
  (on the same resources).  Request for locks1 is blocked when some other
device
  already has locks1 or locks2 (on the same resource). This means request for
  locks2 should not be blocked just because some other device already has
locks2
  (on the same resources).
Originally I think Ben H had something like this, and I think avoiding
deadlock was really really impossible, so I gave up.

The thing is you have to guarantee that deadlock can't occur when you
combine any set of n userspace drivers, I think the original design
went a step further though and allowed separate io and mem requests
and that might have caused the deadlock situations rather than the
legacy/non-legacy issues.

So I'd like to make sure all the possible locking/unlocking combos are
thought out before going ahead with anything.

  but only two strings for them, "io" and "mem". Add "IO" and "MEM" for non-
  legacy accesses.
You'll notice io+mem is only interpreted in the kernel, separate
io/mem was a deadlock fest, esp when you also consider non-X apps
running at the same time as X. X since it is single threaded can't
really deadlock, but if X has locked the non-legacy io+mem, and
vbetool locks the legacy io+mem, and waits for the non-legacy lock, it
starts to get really messy. As long as you've considered this in the
design I'm fine with it.

Thank you for pointing out potential deadlock problem.  In this
context deadlock could happen when a device is holding a lock,
while requesting another.  These are possible scenarios:

(1) If a device is making both legacy (locks) and non-legacy
    (locks2) requests in one call (such as io+mem+IO+MEM), the
    code in _tryget() guarantees either both or none are granted.
    Safe.

(2) If a device is holding a legacy lock while making request for
    another lock (whether it's legacy or non-legacy) in a subsequent
    call, since legacy lock is exclusive and no other devices can get
    any lock when a legacy lock is being held, the new lock can be
    granted without waiting.  Safe.

(3) A device is holding a non-legacy lock while making request
    for another lock in subsequent call.
    (a) It is safe if the new lock requested is non-legacy lock: when
        a non-legacy lock is being held, no other devices can get any
        legacy lock, therefore the new lock will not be in conflict with
        any locks on other devices (the only locks other devices may
        have are non-legacy, which certainly are not in conflict with
        the one being requested).
    (b) It is unsafe if the new lock requested is a legacy lock:  if two
        devices are both holding non-legacy lock while requesting
        legacy lock, a deadlock occurs: both devices are waiting for
        the non-legacy lock from other device to be removed, which
        never happens.  To solve this problem:

        * Add the resources represented in the non-legacy lock
            (currently being held) to the requesting resource of the
            new lock;
        * Clear non-legacy lock.

        This means the new request will include both non-legacy and
        legacy locks, like the situation in (1).

(4) However neither the current code nor the proposed code allows
    locking on individual attributes. If device "a" is holding lock for
    "io" while requesting a lock for "mem"; device "b" is holding lock
    for "mem" while requesting a lock for "io", deadlock occurs.
    So far I don't see any such requirement.  We certainly could use
    the method similar to what described in (3)/(b) to make this
    possible if there is a need.

The patch for in-kernel driver with (3)/(b) fix is attached.

(2) Function group based resource request

  Need to distinguish between decoding ability and decoding request (resource
  request). Decoding ability is still maintained in struct vga_device of
kernel
  driver with

       unsigned int decodes;

  and a userland copy in dev->vgaarb_rsrc.

  Currently all lock/unlocking mechanism uses resource requests from
  dev->vgaarb_rsrc, which is actually decoding ability. In new design
however,
  this is only the case for xf86VGAarbiterLock() and xf86VGAarbiterUnlock(),
run
  during session initialization and exiting. During normal run, resource
request
  is determined by a resource mask associated with each function.

  Wrapping function are grouped into MAX_VGAARB_OPS_MASK number of
  groups with resource masks assigned to each of them. The default setting of
mask is
  VGA_RSRC_NORMAL_IO|VGA_RSRC_NORMAL_MEM, meaning non-legacy
  access, but drivers can redefine any of them. In an extreme if a driver
redefines all
  masks to
      VGA_RSRC_NORMAL_IO|VGA_RSRC_NORMAL_MEM|
  VGA_RSRC_LEGACY_IO|VGA_RSRC_LEGACY_MEM

  we are returning to old arbitration algorithm.

(3) Other changes

  * pci_device_vgaarb_set_target() is heavily called. Currently it involves
two
   syscalls.  These calls can be saved if the device in question is the same
as
   in the previous call (recorded in pci_sys->vga_target). This contributes
   to major performance improvement.

  * OpenConsole()/CloseConsole() need to be protected by lock and unlock as
they
   may have vga register accesses. Further, OpenConsole()/CloseConsole() is
run
   only on a session with primary device.

In-kernel I'd really like to make some more changes, that I've been
hacking on but really don't have time to fix.

The main one is to use bridge control when possible to switch stuff
on/off, the other was some sort of callback for nvidia (though maybe
they can make a patch).

The callback would be called instead of the current forcing
enable/disable, and the driver in-kernel would be responsible for
doing that, it would allow more flexibility in the non-kms world
(which I don't care about enough to do much more).

[attached initial patch - kernel patch not X.org, also probably broken.

Dave.

I think with the attached patch, my proposed code can be futher
optimized:

If vgadev->bridge_has_one_vga is true, the pci decoding bits on the
device are alway set (never turned off), so the device can always
do non-legacy accesses.  That is, when requesting  non-legacy access
such device does not even need to get a lock (locks2) - it can bypass
arbitration (only want to make sure VGAEnable bit of the bus is off) !!
This also means, if we have two devices sitting on two different buses,
during normal run ("graphics mode"), in most cases no arbitration is
really needed - _tryget() returns immediately.  Arbitration is needed
when one of the devices exits (including VT switch), or a new device
starts.

Suggest to add an entry in vga_arb_read(), to print also VGAEnable
bit of the bus.


-Henry


_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index 441e38c..a22b91e 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -36,11 +36,12 @@ struct vga_device {
 	struct pci_dev *pdev;
 	unsigned int decodes;	/* what does it decodes */
 	unsigned int owns;	/* what does it owns */
-	unsigned int locks;	/* what does it locks */
+	unsigned int locks;	/* what does it locks for legacy access */
+	unsigned int locks2;	/* what does it locks for non-legacy (normal) access */
 	unsigned int io_lock_cnt;	/* legacy IO lock count */
 	unsigned int mem_lock_cnt;	/* legacy MEM lock count */
-	unsigned int io_norm_cnt;	/* normal IO count */
-	unsigned int mem_norm_cnt;	/* normal MEM count */
+	unsigned int io_lock2_cnt;	/* normal IO count */
+	unsigned int mem_lock2_cnt;	/* normal MEM count */
 
 	/* allow IRQ enable/disable hook */
 	void *cookie;
@@ -54,24 +55,28 @@ static bool vga_arbiter_used;
 static DEFINE_SPINLOCK(vga_lock);
 static DECLARE_WAIT_QUEUE_HEAD(vga_wait_queue);
 
-
-static const char *vga_iostate_to_str(unsigned int iostate)
-{
-	/* Ignore VGA_RSRC_IO and VGA_RSRC_MEM */
-	iostate &= VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM;
-	switch (iostate) {
-	case VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM:
-		return "io+mem";
-	case VGA_RSRC_LEGACY_IO:
-		return "io";
-	case VGA_RSRC_LEGACY_MEM:
-		return "mem";
-	}
-	return "none";
-}
+static char *vga_iostate_to_str[] = {
+	"none",
+	"io",
+	"mem",
+	"io+mem",
+	"IO",
+	"io+IO",
+	"mem+IO",
+	"io+mem+IO",
+	"MEM",
+	"io+MEM",
+	"mem+MEM",
+	"io+mem+MEM",
+	"IO+MEM",
+	"io+IO+MEM",
+	"mem+IO+MEM",
+	"io+mem+IO+MEM"
+};
 
 static int vga_str_to_iostate(char *buf, int str_size, int *io_state)
 {
+	unsigned int state = 0;
 	/* we could in theory hand out locks on IO and mem
 	 * separately to userspace but it can cause deadlocks */
 	if (strncmp(buf, "none", 4) == 0) {
@@ -80,15 +85,20 @@ static int vga_str_to_iostate(char *buf, int str_size, int *io_state)
 	}
 
 	/* XXX We're not chekcing the str_size! */
-	if (strncmp(buf, "io+mem", 6) == 0)
-		goto both;
-	else if (strncmp(buf, "io", 2) == 0)
-		goto both;
-	else if (strncmp(buf, "mem", 3) == 0)
-		goto both;
-	return 0;
-both:
-	*io_state = VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM;
+	if (strstr(buf, "io"))
+		state |= VGA_RSRC_LEGACY_IO;
+	if (strstr(buf, "mem"))
+		state |= VGA_RSRC_LEGACY_MEM;
+	if (strstr(buf, "IO"))
+		state |= VGA_RSRC_NORMAL_IO;
+	if (strstr(buf, "MEM"))
+		state |= VGA_RSRC_NORMAL_MEM;
+
+	if (!state)
+		return 0;
+
+	*io_state = state;
+
 	return 1;
 }
 
@@ -142,39 +152,43 @@ static void vga_check_first_use(void)
 static struct vga_device *__vga_tryget(struct vga_device *vgadev,
 				       unsigned int rsrc)
 {
-	unsigned int wants, legacy_wants, match;
+	unsigned int wants, legacy_wants, normal_wants, match;
 	struct vga_device *conflict;
-	unsigned int pci_bits;
-	/* Account for "normal" resources to lock. If we decode the legacy,
-	 * counterpart, we need to request it as well
-	 */
-	if ((rsrc & VGA_RSRC_NORMAL_IO) &&
-	    (vgadev->decodes & VGA_RSRC_LEGACY_IO))
-		rsrc |= VGA_RSRC_LEGACY_IO;
-	if ((rsrc & VGA_RSRC_NORMAL_MEM) &&
-	    (vgadev->decodes & VGA_RSRC_LEGACY_MEM))
-		rsrc |= VGA_RSRC_LEGACY_MEM;
+	unsigned int pci_bits, orsrc;
 
 	pr_devel("%s: %d\n", __func__, rsrc);
 	pr_devel("%s: owns: %d\n", __func__, vgadev->owns);
 
+	/*
+	 * Requests are always honored if the device does not have
+	 * legacy decoding access ability
+	 */
+	if (!(orsrc = rsrc) || !(vgadev->decodes|VGA_RSRC_LEGACY_MASK))
+		return NULL;
+
+	/* We don't need to request a legacy resource, we just enable
+	 * appropriate decoding and go
+	 */
+	if ((legacy_wants = (rsrc & VGA_RSRC_LEGACY_MASK)) && vgadev->locks2) {
+		/* we have a legacy request while holding non-legacy lock:
+		   add locks2 to request, clear locks2, to avoid deadlock */
+		rsrc = orsrc | vgadev->locks2;
+		vgadev->locks2 = 0;
+	}
+
 	/* Check what resources we need to acquire */
 	wants = rsrc & ~vgadev->owns;
 
-	/* We already own everything, just mark locked & bye bye */
-	if (wants == 0)
+	/* already own everything: legal wants still need to grab other devices' owns */
+        if ((wants == 0) && (legacy_wants == 0))
 		goto lock_them;
 
-	/* We don't need to request a legacy resource, we just enable
-	 * appropriate decoding and go
-	 */
-	legacy_wants = wants & VGA_RSRC_LEGACY_MASK;
-	if (legacy_wants == 0)
-		goto enable_them;
+	normal_wants = wants & ~VGA_RSRC_LEGACY_MASK;
 
 	/* Ok, we don't, let's find out how we need to kick off */
 	list_for_each_entry(conflict, &vga_list, list) {
 		unsigned int lwants = legacy_wants;
+		unsigned int nwants = normal_wants, nwants_io, nwants_mem;
 		unsigned int change_bridge = 0;
 
 		/* Don't conflict with myself */
@@ -195,14 +209,31 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev,
 		 */
 		if (vgadev->pdev->bus != conflict->pdev->bus) {
 			change_bridge = 1;
-			lwants = VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM;
+			if (lwants)
+				lwants = VGA_RSRC_LEGACY_IO|VGA_RSRC_LEGACY_MEM;
+			if (nwants)
+				nwants = VGA_RSRC_NORMAL_IO|VGA_RSRC_NORMAL_MEM;
 		}
 
 		/* Check if the guy has a lock on the resource. If he does,
 		 * return the conflicting entry
 		 */
+		/* check conflict for legacy access request */
 		if (conflict->locks & lwants)
 			return conflict;
+		if (((lwants & VGA_RSRC_LEGACY_IO) &&
+			(conflict->locks2 & VGA_RSRC_NORMAL_IO)) || 
+			((lwants & VGA_RSRC_LEGACY_MEM) &&
+			(conflict->locks2 & VGA_RSRC_NORMAL_MEM)))
+			return conflict;
+
+		nwants_io = nwants & VGA_RSRC_NORMAL_IO;
+		nwants_mem = nwants & VGA_RSRC_NORMAL_MEM;
+
+		/* check conflict for non-legacy access request */
+		if ((nwants_io && (conflict->locks & VGA_RSRC_LEGACY_IO)) || 
+			(nwants_mem && (conflict->locks & VGA_RSRC_LEGACY_MEM)))
+			return conflict;
 
 		/* Ok, now check if he owns the resource we want. We don't need
 		 * to check "decodes" since it should be impossible to own
@@ -220,46 +251,54 @@ static struct vga_device *__vga_tryget(struct vga_device *vgadev,
 		vga_irq_set_state(conflict, false);
 
 		pci_bits = 0;
-		if (lwants & (VGA_RSRC_LEGACY_MEM|VGA_RSRC_NORMAL_MEM))
+		if (lwants & VGA_RSRC_LEGACY_MEM) {
 			pci_bits |= PCI_COMMAND_MEMORY;
-		if (lwants & (VGA_RSRC_LEGACY_IO|VGA_RSRC_NORMAL_IO))
+			conflict->owns &= ~(VGA_RSRC_NORMAL_MEM|VGA_RSRC_LEGACY_MEM);
+		}
+		if (lwants & VGA_RSRC_LEGACY_IO) {
 			pci_bits |= PCI_COMMAND_IO;
+			conflict->owns &= ~(VGA_RSRC_NORMAL_IO|VGA_RSRC_LEGACY_IO);
+		}
 
 		pci_set_vga_state(conflict->pdev, false, pci_bits,
 				  change_bridge);
-		conflict->owns &= ~lwants;
-		/* If he also owned non-legacy, that is no longer the case */
-		if (lwants & VGA_RSRC_LEGACY_MEM)
-			conflict->owns &= ~VGA_RSRC_NORMAL_MEM;
-		if (lwants & VGA_RSRC_LEGACY_IO)
-			conflict->owns &= ~VGA_RSRC_NORMAL_IO;
 	}
 
-enable_them:
 	/* ok dude, we got it, everybody conflicting has been disabled, let's
 	 * enable us. Make sure we don't mark a bit in "owns" that we don't
 	 * also have in "decodes". We can lock resources we don't decode but
 	 * not own them.
 	 */
-	pci_bits = 0;
-	if (wants & (VGA_RSRC_LEGACY_MEM|VGA_RSRC_NORMAL_MEM))
-		pci_bits |= PCI_COMMAND_MEMORY;
-	if (wants & (VGA_RSRC_LEGACY_IO|VGA_RSRC_NORMAL_IO))
-		pci_bits |= PCI_COMMAND_IO;
-	pci_set_vga_state(vgadev->pdev, true, pci_bits, !!(wants & VGA_RSRC_LEGACY_MASK));
+	if (wants) {
+		pci_bits = 0;
+		if (wants & (VGA_RSRC_LEGACY_MEM|VGA_RSRC_NORMAL_MEM)) {
+			vgadev->owns |= VGA_RSRC_LEGACY_MEM|VGA_RSRC_NORMAL_MEM;
+			pci_bits |= PCI_COMMAND_MEMORY;
+		}
+		if (wants & (VGA_RSRC_LEGACY_IO|VGA_RSRC_NORMAL_IO)) {
+			vgadev->owns |= VGA_RSRC_LEGACY_IO|VGA_RSRC_NORMAL_IO;
+			pci_bits |= PCI_COMMAND_IO;
+		}
+
+		pci_set_vga_state(vgadev->pdev, true, pci_bits, !!(legacy_wants));
+	}
 
 	vga_irq_set_state(vgadev, true);
-	vgadev->owns |= (wants & vgadev->decodes);
 lock_them:
-	vgadev->locks |= (rsrc & VGA_RSRC_LEGACY_MASK);
-	if (rsrc & VGA_RSRC_LEGACY_IO)
-		vgadev->io_lock_cnt++;
-	if (rsrc & VGA_RSRC_LEGACY_MEM)
-		vgadev->mem_lock_cnt++;
-	if (rsrc & VGA_RSRC_NORMAL_IO)
-		vgadev->io_norm_cnt++;
-	if (rsrc & VGA_RSRC_NORMAL_MEM)
-		vgadev->mem_norm_cnt++;
+	if (rsrc & VGA_RSRC_LEGACY_MASK) {
+		vgadev->locks |= rsrc & VGA_RSRC_LEGACY_MASK;
+		if (rsrc & VGA_RSRC_LEGACY_IO)
+			vgadev->io_lock_cnt++;
+		if (rsrc & VGA_RSRC_LEGACY_MEM)
+			vgadev->mem_lock_cnt++;
+	}
+	if (rsrc & ~VGA_RSRC_LEGACY_MASK) {
+		vgadev->locks2 |= rsrc & ~VGA_RSRC_LEGACY_MASK;
+		if (orsrc & VGA_RSRC_NORMAL_IO)
+			vgadev->io_lock2_cnt++;
+		if (orsrc & VGA_RSRC_NORMAL_MEM)
+			vgadev->mem_lock2_cnt++;
+	}
 
 	return NULL;
 }
@@ -267,22 +306,17 @@ lock_them:
 static void __vga_put(struct vga_device *vgadev, unsigned int rsrc)
 {
 	unsigned int old_locks = vgadev->locks;
+	unsigned int old_locks2 = vgadev->locks2;
 
 	pr_devel("%s\n", __func__);
 
 	/* Update our counters, and account for equivalent legacy resources
 	 * if we decode them
 	 */
-	if ((rsrc & VGA_RSRC_NORMAL_IO) && vgadev->io_norm_cnt > 0) {
-		vgadev->io_norm_cnt--;
-		if (vgadev->decodes & VGA_RSRC_LEGACY_IO)
-			rsrc |= VGA_RSRC_LEGACY_IO;
-	}
-	if ((rsrc & VGA_RSRC_NORMAL_MEM) && vgadev->mem_norm_cnt > 0) {
-		vgadev->mem_norm_cnt--;
-		if (vgadev->decodes & VGA_RSRC_LEGACY_MEM)
-			rsrc |= VGA_RSRC_LEGACY_MEM;
-	}
+	if ((rsrc & VGA_RSRC_NORMAL_IO) && vgadev->io_lock2_cnt > 0)
+		vgadev->io_lock2_cnt--;
+	if ((rsrc & VGA_RSRC_NORMAL_MEM) && vgadev->mem_lock2_cnt > 0)
+		vgadev->mem_lock2_cnt--;
 	if ((rsrc & VGA_RSRC_LEGACY_IO) && vgadev->io_lock_cnt > 0)
 		vgadev->io_lock_cnt--;
 	if ((rsrc & VGA_RSRC_LEGACY_MEM) && vgadev->mem_lock_cnt > 0)
@@ -295,11 +329,15 @@ static void __vga_put(struct vga_device *vgadev, unsigned int rsrc)
 		vgadev->locks &= ~VGA_RSRC_LEGACY_IO;
 	if (vgadev->mem_lock_cnt == 0)
 		vgadev->locks &= ~VGA_RSRC_LEGACY_MEM;
+	if (vgadev->io_lock2_cnt == 0)
+		vgadev->locks2 &= ~VGA_RSRC_NORMAL_IO;
+	if (vgadev->mem_lock2_cnt == 0)
+		vgadev->locks2 &= ~VGA_RSRC_NORMAL_MEM;
 
 	/* Kick the wait queue in case somebody was waiting if we actually
 	 * released something
 	 */
-	if (old_locks != vgadev->locks)
+	if ((old_locks != vgadev->locks) || (old_locks2 != vgadev->locks2))
 		wake_up_all(&vga_wait_queue);
 }
 
@@ -451,9 +489,9 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
 	 */
 	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
 	if (cmd & PCI_COMMAND_IO)
-		vgadev->owns |= VGA_RSRC_LEGACY_IO;
+		vgadev->owns |= VGA_RSRC_LEGACY_IO|VGA_RSRC_NORMAL_IO;
 	if (cmd & PCI_COMMAND_MEMORY)
-		vgadev->owns |= VGA_RSRC_LEGACY_MEM;
+		vgadev->owns |= VGA_RSRC_LEGACY_MEM|VGA_RSRC_NORMAL_MEM;
 
 	/* Check if VGA cycles can get down to us */
 	bus = pdev->bus;
@@ -483,11 +521,12 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
 	/* Add to the list */
 	list_add(&vgadev->list, &vga_list);
 	vga_count++;
-	pr_info("vgaarb: device added: PCI:%s,decodes=%s,owns=%s,locks=%s\n",
+	pr_info("vgaarb: device added: PCI:%s,decodes=%s,owns=%s,locks=%s,locks2=%s\n",
 		pci_name(pdev),
-		vga_iostate_to_str(vgadev->decodes),
-		vga_iostate_to_str(vgadev->owns),
-		vga_iostate_to_str(vgadev->locks));
+		vga_iostate_to_str[vgadev->decodes],
+		vga_iostate_to_str[vgadev->owns],
+		vga_iostate_to_str[vgadev->locks],
+		vga_iostate_to_str[vgadev->locks2]);
 
 	spin_unlock_irqrestore(&vga_lock, flags);
 	return true;
@@ -546,9 +585,9 @@ static inline void vga_update_device_decodes(struct vga_device *vgadev,
 
 	pr_info("vgaarb: device changed decodes: PCI:%s,olddecodes=%s,decodes=%s:owns=%s\n",
 		pci_name(vgadev->pdev),
-		vga_iostate_to_str(old_decodes),
-		vga_iostate_to_str(vgadev->decodes),
-		vga_iostate_to_str(vgadev->owns));
+		vga_iostate_to_str[old_decodes],
+		vga_iostate_to_str[vgadev->decodes],
+		vga_iostate_to_str[vgadev->owns]);
 
 
 	/* if we own the decodes we should move them along to
@@ -577,13 +616,11 @@ static inline void vga_update_device_decodes(struct vga_device *vgadev,
 	}
 }
 
-void __vga_set_legacy_decoding(struct pci_dev *pdev, unsigned int decodes, bool userspace)
+void __vga_set_decoding(struct pci_dev *pdev, unsigned int decodes, bool userspace)
 {
 	struct vga_device *vgadev;
 	unsigned long flags;
 
-	decodes &= VGA_RSRC_LEGACY_MASK;
-
 	spin_lock_irqsave(&vga_lock, flags);
 	vgadev = vgadev_find(pdev);
 	if (vgadev == NULL)
@@ -604,11 +641,11 @@ bail:
 	spin_unlock_irqrestore(&vga_lock, flags);
 }
 
-void vga_set_legacy_decoding(struct pci_dev *pdev, unsigned int decodes)
+void vga_set_decoding(struct pci_dev *pdev, unsigned int decodes)
 {
-	__vga_set_legacy_decoding(pdev, decodes, false);
+	__vga_set_decoding(pdev, decodes, false);
 }
-EXPORT_SYMBOL(vga_set_legacy_decoding);
+EXPORT_SYMBOL(vga_set_decoding);
 
 /* call with NULL to unregister */
 int vga_client_register(struct pci_dev *pdev, void *cookie,
@@ -699,6 +736,8 @@ struct vga_arb_user_card {
 	struct pci_dev *pdev;
 	unsigned int mem_cnt;
 	unsigned int io_cnt;
+	unsigned int mem_cnt2;
+	unsigned int io_cnt2;
 };
 
 struct vga_arb_private {
@@ -776,12 +815,14 @@ static ssize_t vga_arb_read(struct file *file, char __user * buf,
 
 	/* Fill the buffer with infos */
 	len = snprintf(lbuf, 1024,
-		       "count:%d,PCI:%s,decodes=%s,owns=%s,locks=%s(%d:%d)\n",
+		       "count:%d,PCI:%s,decodes=%s,owns=%s,locks=%s(%d:%d),locks2=%s(%d:%d)\n",
 		       vga_decode_count, pci_name(pdev),
-		       vga_iostate_to_str(vgadev->decodes),
-		       vga_iostate_to_str(vgadev->owns),
-		       vga_iostate_to_str(vgadev->locks),
-		       vgadev->io_lock_cnt, vgadev->mem_lock_cnt);
+		       vga_iostate_to_str[vgadev->decodes],
+		       vga_iostate_to_str[vgadev->owns],
+		       vga_iostate_to_str[vgadev->locks],
+		       vgadev->io_lock_cnt, vgadev->mem_lock_cnt,
+		       vga_iostate_to_str[vgadev->locks2],
+		       vgadev->io_lock2_cnt, vgadev->mem_lock2_cnt);
 
 	spin_unlock_irqrestore(&vga_lock, flags);
 done:
@@ -857,6 +898,10 @@ static ssize_t vga_arb_write(struct file *file, const char __user * buf,
 					priv->cards[i].io_cnt++;
 				if (io_state & VGA_RSRC_LEGACY_MEM)
 					priv->cards[i].mem_cnt++;
+				if (io_state & VGA_RSRC_NORMAL_IO)
+					priv->cards[i].io_cnt2++;
+				if (io_state & VGA_RSRC_NORMAL_MEM)
+					priv->cards[i].mem_cnt2++;
 				break;
 			}
 		}
@@ -870,7 +915,8 @@ static ssize_t vga_arb_write(struct file *file, const char __user * buf,
 		pr_devel("client 0x%p called 'unlock'\n", priv);
 
 		if (strncmp(curr_pos, "all", 3) == 0)
-			io_state = VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM;
+			io_state = VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM |
+				VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM;
 		else {
 			if (!vga_str_to_iostate
 			    (curr_pos, remaining, &io_state)) {
@@ -895,14 +941,30 @@ static ssize_t vga_arb_write(struct file *file, const char __user * buf,
 				uc = &priv->cards[i];
 		}
 
-		if (!uc)
-			return -EINVAL;
+		if (!uc) {
+			ret_val = -ENODEV;
+			goto done;
+		}
+
+		if (io_state & VGA_RSRC_LEGACY_IO && uc->io_cnt == 0) {
+			ret_val = -EINVAL;
+			goto done;
+		}
+
+		if (io_state & VGA_RSRC_LEGACY_MEM && uc->mem_cnt == 0) {
+			ret_val = -EINVAL;
+			goto done;
+		}
 
-		if (io_state & VGA_RSRC_LEGACY_IO && uc->io_cnt == 0)
-			return -EINVAL;
+		if (io_state & VGA_RSRC_NORMAL_IO && uc->io_cnt2 == 0) {
+			ret_val = -EINVAL;
+			goto done;
+		}
 
-		if (io_state & VGA_RSRC_LEGACY_MEM && uc->mem_cnt == 0)
-			return -EINVAL;
+		if (io_state & VGA_RSRC_NORMAL_MEM && uc->mem_cnt2 == 0) {
+			ret_val = -EINVAL;
+			goto done;
+		}
 
 		vga_put(pdev, io_state);
 
@@ -910,6 +972,10 @@ static ssize_t vga_arb_write(struct file *file, const char __user * buf,
 			uc->io_cnt--;
 		if (io_state & VGA_RSRC_LEGACY_MEM)
 			uc->mem_cnt--;
+		if (io_state & VGA_RSRC_NORMAL_IO)
+			uc->io_cnt2--;
+		if (io_state & VGA_RSRC_NORMAL_MEM)
+			uc->mem_cnt2--;
 
 		ret_val = count;
 		goto done;
@@ -936,7 +1002,7 @@ static ssize_t vga_arb_write(struct file *file, const char __user * buf,
 			goto done;
 		}
 
-		if (vga_tryget(pdev, io_state)) {
+		if (!vga_tryget(pdev, io_state)) {
 			/* Update the client's locks lists... */
 			for (i = 0; i < MAX_USER_CARDS; i++) {
 				if (priv->cards[i].pdev == pdev) {
@@ -944,6 +1010,10 @@ static ssize_t vga_arb_write(struct file *file, const char __user * buf,
 						priv->cards[i].io_cnt++;
 					if (io_state & VGA_RSRC_LEGACY_MEM)
 						priv->cards[i].mem_cnt++;
+					if (io_state & VGA_RSRC_NORMAL_IO)
+						priv->cards[i].io_cnt2++;
+					if (io_state & VGA_RSRC_NORMAL_MEM)
+						priv->cards[i].mem_cnt2++;
 					break;
 				}
 			}
@@ -1009,6 +1079,8 @@ static ssize_t vga_arb_write(struct file *file, const char __user * buf,
 				priv->cards[i].pdev = pdev;
 				priv->cards[i].io_cnt = 0;
 				priv->cards[i].mem_cnt = 0;
+				priv->cards[i].io_cnt2 = 0;
+				priv->cards[i].mem_cnt2 = 0;
 				break;
 			}
 		}
@@ -1041,7 +1113,7 @@ static ssize_t vga_arb_write(struct file *file, const char __user * buf,
 			goto done;
 		}
 
-		__vga_set_legacy_decoding(pdev, io_state, true);
+		__vga_set_decoding(pdev, io_state, true);
 		ret_val = count;
 		goto done;
 	}
@@ -1089,6 +1161,8 @@ static int vga_arb_open(struct inode *inode, struct file *file)
 	priv->cards[0].pdev = priv->target;
 	priv->cards[0].io_cnt = 0;
 	priv->cards[0].mem_cnt = 0;
+	priv->cards[0].io_cnt2 = 0;
+	priv->cards[0].mem_cnt2 = 0;
 
 
 	return 0;
@@ -1112,12 +1186,16 @@ static int vga_arb_release(struct inode *inode, struct file *file)
 		uc = &priv->cards[i];
 		if (uc->pdev == NULL)
 			continue;
-		pr_devel("uc->io_cnt == %d, uc->mem_cnt == %d\n",
-			 uc->io_cnt, uc->mem_cnt);
+		pr_devel("uc->io_cnt == %d, uc->mem_cnt == %d, uc->io_cnt2 == %d, uc->mem_cnt2 == %d\n",
+			 uc->io_cnt, uc->mem_cnt, uc->io_cnt2, uc->mem_cnt2);
 		while (uc->io_cnt--)
 			vga_put(uc->pdev, VGA_RSRC_LEGACY_IO);
 		while (uc->mem_cnt--)
 			vga_put(uc->pdev, VGA_RSRC_LEGACY_MEM);
+		while (uc->io_cnt2--)
+			vga_put(uc->pdev, VGA_RSRC_NORMAL_IO);
+		while (uc->mem_cnt2--)
+			vga_put(uc->pdev, VGA_RSRC_NORMAL_MEM);
 	}
 	spin_unlock_irqrestore(&vga_user_lock, flags);
 
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to