[PATCH] sata_nv: Don't attempt using ADMA for (READ|SET)_MAX commands

2007-04-13 Thread Kyle McMartin
READ_NATIVE_MAX and SET_MAX were causing timeouts on sata_nv controllers.
Disabling ADMA helped, but that is quite a large hammer to use. Reverting
382a6652e91b34d5480cfc0ed840c196650493d4 also helped, but we might as well
fix it right, instead of disabling the performance gain on cache flushes
by using ADMA mode.

Signed-off-by: Kyle McMartin <[EMAIL PROTECTED]>

---

This patch depends on the Host Protected Area patch Alan sent to linux-ide
this week.

diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
index 9d9670a..eaf9b76 100644
--- a/drivers/ata/sata_nv.c
+++ b/drivers/ata/sata_nv.c
@@ -1162,6 +1162,20 @@ static void nv_adma_fill_sg(struct ata_queued_cmd *qc, 
struct nv_adma_cpb *cpb)
cpb->next_aprd = cpu_to_le64(0);
 }
 
+static int nv_blacklist_adma_for_hpa_cmds(struct ata_taskfile *tf)
+{
+   switch(tf->command) {
+   case ATA_CMD_READ_NATIVE_MAX:
+   case ATA_CMD_READ_NATIVE_MAX_EXT:
+   case ATA_CMD_SET_MAX:
+   case ATA_CMD_SET_MAX_EXT:
+   return 1;
+
+   default:
+   return 0;
+   }
+}
+
 static int nv_adma_use_reg_mode(struct ata_queued_cmd *qc)
 {
struct nv_adma_port_priv *pp = qc->ap->private_data;
@@ -1173,8 +1187,12 @@ static int nv_adma_use_reg_mode(struct ata_queued_cmd 
*qc)
return 1;
 
if((qc->flags & ATA_QCFLAG_DMAMAP) ||
-  (qc->tf.protocol == ATA_PROT_NODATA))
-   return 0;
+  (qc->tf.protocol == ATA_PROT_NODATA)) {
+   if (nv_blacklist_adma_for_hpa_cmds(&qc->tf))
+   return 1;   /* (SET|READ)_NATIVE_MAX time out in 
ADMA mdoe */
+   else
+   return 0;
+   }
 
return 1;
 }
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] sata_nv: Don't attempt using ADMA for (READ|SET)_MAX commands

2007-04-13 Thread Alan Cox
On Fri, 13 Apr 2007 13:08:31 -0400
Kyle McMartin <[EMAIL PROTECTED]> wrote:

> READ_NATIVE_MAX and SET_MAX were causing timeouts on sata_nv controllers.
> Disabling ADMA helped, but that is quite a large hammer to use. Reverting
> 382a6652e91b34d5480cfc0ed840c196650493d4 also helped, but we might as well
> fix it right, instead of disabling the performance gain on cache flushes
> by using ADMA mode.

Probably not going to make any performance difference to blacklist all
non-data commands or all but a few like cache-flush. 

Alan
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] sata_nv: Don't attempt using ADMA for (READ|SET)_MAX commands

2007-04-13 Thread Mark Lord

Alan Cox wrote:

On Fri, 13 Apr 2007 13:08:31 -0400
Kyle McMartin <[EMAIL PROTECTED]> wrote:


READ_NATIVE_MAX and SET_MAX were causing timeouts on sata_nv controllers.
Disabling ADMA helped, but that is quite a large hammer to use. Reverting
382a6652e91b34d5480cfc0ed840c196650493d4 also helped, but we might as well
fix it right, instead of disabling the performance gain on cache flushes
by using ADMA mode.


Probably not going to make any performance difference to blacklist all
non-data commands or all but a few like cache-flush. 


I agree.  The Pacific Digital ADMA stuff had some quirks for various non-data
commands as well, and the sensible thing was to just not use ADMA for anything
other than READs/WRITEs and possible CACHE FLUSHes.

Cheers
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] sata_nv: Don't attempt using ADMA for (READ|SET)_MAX commands

2007-04-13 Thread Robert Hancock

Mark Lord wrote:

Alan Cox wrote:

On Fri, 13 Apr 2007 13:08:31 -0400
Kyle McMartin <[EMAIL PROTECTED]> wrote:

READ_NATIVE_MAX and SET_MAX were causing timeouts on sata_nv 
controllers.
Disabling ADMA helped, but that is quite a large hammer to use. 
Reverting
382a6652e91b34d5480cfc0ed840c196650493d4 also helped, but we might as 
well

fix it right, instead of disabling the performance gain on cache flushes
by using ADMA mode.


Probably not going to make any performance difference to blacklist all
non-data commands or all but a few like cache-flush. 


I agree.  The Pacific Digital ADMA stuff had some quirks for various 
non-data
commands as well, and the sensible thing was to just not use ADMA for 
anything

other than READs/WRITEs and possible CACHE FLUSHes.


I'm told by NVidia that non-data commands should be fine in ADMA mode, 
except for those that require reading back a result taskfile (which 
can't be done in ADMA mode). The patch in libata-dev which I mentioned 
in another email will ensure that we don't try to use ADMA for such 
commands, so this patch should not be needed.


--
Robert Hancock  Saskatoon, SK, Canada
To email, remove "nospam" from [EMAIL PROTECTED]
Home Page: http://www.roberthancock.com/

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html