Fix SH DMAC code to handle PVR2 cascade

Fix SH DMAC code to correctly handle PVR2 cascade DMA.

This updates http://lkml.org/lkml/2007/10/2/276

(I decided it was better to have the true size of the transfer put in via the API and refactor this here. And calc_xmit_shift(chan) should return 5 but only returns 3 so I've not used it here)

@@ -150,6 +150,13 @@ static void sh_dmac_disable_dma(struct d
 
 static int sh_dmac_xfer_dma(struct dma_channel *chan)
 {
+	/* Handle Dreamcast PVR cascade */
+	if (mach_is_dreamcast() && chan->chan == PVR2_CASCADE_CHAN) {
+		ctrl_outl(chan->sar, SAR[chan->chan]);
+		/* Transfer in 32 byte blocks */
+		ctrl_outl((chan->count) >> 5, DMATCR[chan->chan]);
+		return 0;
+	}
 	/*
 	 * If we haven't pre-configured the channel with special flags, use
 	 * the defaults.

@@ -159,26 +166,9 @@ static int sh_dmac_xfer_dma(struct dma_c
 
 	sh_dmac_disable_dma(chan);
 
-	/*
-	 * Single-address mode usage note!
-	 *
-	 * It's important that we don't accidentally write any value to SAR/DAR
-	 * (this includes 0) that hasn't been directly specified by the user if
-	 * we're in single-address mode.
-	 *
-	 * In this case, only one address can be defined, anything else will
-	 * result in a DMA address error interrupt (at least on the SH-4),
-	 * which will subsequently halt the transfer.
-	 *
-	 * Channel 2 on the Dreamcast is a special case, as this is used for
-	 * cascading to the PVR2 DMAC. In this case, we still need to write
-	 * SAR and DAR, regardless of value, in order for cascading to work.
-	 */
-	if (chan->sar || (mach_is_dreamcast() &&
-			  chan->chan == PVR2_CASCADE_CHAN))
+	if (chan->sar)
 		ctrl_outl(chan->sar, SAR[chan->chan]);
-	if (chan->dar || (mach_is_dreamcast() &&
-			  chan->chan == PVR2_CASCADE_CHAN))
+	if (chan->dar)
 		ctrl_outl(chan->dar, DAR[chan->chan]);
 
 	ctrl_outl(chan->count >> calc_xmit_shift(chan), DMATCR[chan->chan]);

It would be helpful to know why calc_xmit_shift() is broken here rather than just coding around it, as this will have implications for the other DMA channels on SH7091/SH7750.

Now that you've completely bypassed the rest of the SH-DMAC ->xfer_dma() op, it's clear that the existing infrastructure needs a bit of rework for dealing with the cascaded DMACs (especially for single-address mode only, unidirectionally). It would be nice to get the mach-specific kludges for cascade out of dma-sh entirely.

This can certainly be fixed for 2.6.24, though a larger overhaul is 2.6.25 material at this point.

53 /*
54 * The DMA count is defined as the number of bytes to transfer. 55 */
56 static unsigned int ts_shift[] __maybe_unused = {

57         [XMIT_SZ_64BIT]         = 3,
58         [XMIT_SZ_8BIT]          = 0,
59         [XMIT_SZ_16BIT]         = 1,
60         [XMIT_SZ_32BIT]         = 2,
61         [XMIT_SZ_256BIT]        = 5,

62 };
63 #endif

ie ts_shift returns the number of bytes per transfer, but is then used as a bit shift:

45 /*

46  * We determine the correct shift size based off of the CHCR transmit size
47  * for the given channel. Since we know that it will take:
48  *
49  *      info->count >> ts_shift[transmit_size]
50  *
51  * iterations to complete the transfer.
52  */

53 static inline unsigned int calc_xmit_shift(struct dma_channel *chan) 54 {
55 u32 chcr = ctrl_inl(CHCR[chan->chan]);
56
57 return ts_shift[(chcr & CHCR_TS_MASK)>>CHCR_TS_SHIFT];
58 }

(From arch/sh/drivers/dma/dma-sh.c)

I'm not anywhere where I can fix this at the moment, but i am sure it could be patched quite trivally.

Er, no it doesn't, try again. ts_shift returns the transfer size _shift_. The comment refers to the DMA count, which is a different matter. Your 32-byte transfer where you want the >> 5 shift == ts_shift[XMIT_SZ_256BIT], and there's nothing wrong with that. So the issue becomes why you don't get a ts_shift[] offset of XMIT_SZ_256BIT for that channel, and that's the bug that has to be fixed.

So for PVR2 cascade, what does the CHCR value work out to? Both CHCR_TS_MASK and CHCR_TS_SHIFT haven't changed for SH7750, so this suggests that either the CHCR value is just wrong or we've had a long-standing bug with the CHCR.TS mask.

Either way, we are not going to hardcode a ts_shift value when the CHCR has all of the information encoded in it!

There *is* a long standing bug in CHCR_TS_MASK. It is meant to mask out all but bits 6:4 but in fact only masks bits 5 and 4. If it is set to 0x70 and not 0x30 this should work - though as usual I cannot make a patch to check this out at the moment.