Re: sata_mv query

On 12-08-07 01:16 PM, Mark Lord wrote:
..

mv_wait_for_edma_empty_idle() is called from mv_stop_edma(),
which happens whenever we transition between sending NCQ commands
(eg. read/write) and non-NCQ commands (eg. flush cache).

The mv_qc_defer() function is supposed to ensure that things are
more or less idle before it even gets to mv_stop_edma().
So we'd expect the mv_wait_for_edma_empty_idle() function to be quick,
which is a big part of why we're willing to tolerate an inline busy-wait.

But here it seems to be waiting MUCH longer, as if it has to wait for
some IO-in-progress to complete before continuing.  Which means that
the mv_qc_defer() function isn't being as effective as it should be.

The idea is, before any new command is queued, libata invokes the defer
function to see if the driver is ready to accept the new command.
The sata_mv driver normally says "yes" (go ahead) for a new NCQ command
whenever it currently has existing NCQ commands in-flight.
Otherwise it always says "no" (please defer) unless completely idle.
That's how mv_qc_defer() is supposed to work, and that should minimize
the time spent inside mv_wait_for_edma_empty_idle() by ensuring the
edma engine is already idle whenever that gets called.

Say, here's a thought:  We could get rid of (or relocate to upper layers)
the busy-wait by having something similar get called from ata_qc_defer().

If you are feeling adventurous, here is a 100% untested patch to do just that.
It looks correct to me, it compiles, but that's all I can say.

The attached copy is the better one to use -- my mailer mangles inline patches.

--- linux-3.4.4/drivers/ata/sata_mv.c	2012-06-22 14:37:50.000000000 -0400
+++ linux/drivers/ata/sata_mv.c	2012-08-07 14:14:43.554503157 -0400

@@ -1388,6 +1388,17 @@
 	}
 }

+static int mv_check_for_edma_empty_idle(struct ata_port *ap)
+{
+	void __iomem *port_mmio = mv_ap_base(ap);
+	const u32 empty_idle = (EDMA_STATUS_CACHE_EMPTY | EDMA_STATUS_IDLE);
+
+	u32 edma_stat = readl(port_mmio + EDMA_STATUS);
+	if ((edma_stat & empty_idle) == empty_idle)
+		return ATA_DEFER_PORT;
+	return 0;
+}
+
 static int mv_qc_defer(struct ata_queued_cmd *qc)
 {
 	struct ata_link *link = qc->dev->link;

@@ -1423,7 +1434,7 @@
 	 * If the port is completely idle, then allow the new qc.
 	 */
 	if (ap->nr_active_links == 0)
-		return 0;
+		return mv_check_for_edma_empty_idle(ap);

 	/*
 	 * The port is operating in host queuing mode (EDMA) with NCQ