[PATCH] Fix SH DMAC code to handle PVR2 cascade - Kernel

This is a discussion on [PATCH] Fix SH DMAC code to handle PVR2 cascade - Kernel ; 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 ...

+ Reply to Thread
Results 1 to 5 of 5

Thread: [PATCH] Fix SH DMAC code to handle PVR2 cascade

  1. [PATCH] 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)


    --- arch/sh/drivers/dma/dma-sh.c 2007/09/22 18:34:42 1.1
    +++ arch/sh/drivers/dma/dma-sh.c 2007/10/02 20:53:49 1.3
    @@ -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]);
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH] Fix SH DMAC code to handle PVR2 cascade

    On Tue, Oct 02, 2007 at 10:09:27PM +0100, Adrian McMenamin wrote:
    > 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)
    >

    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.
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH] Fix SH DMAC code to handle PVR2 cascade

    On Wed, October 3, 2007 7:18 am, Paul Mundt wrote:
    > On Tue, Oct 02, 2007 at 10:09:27PM +0100, Adrian McMenamin wrote:
    >> 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)
    >>

    > 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.



    From include/asm-sh/cpu-sh4/dma.h

    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.




    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH] Fix SH DMAC code to handle PVR2 cascade

    On Wed, Oct 03, 2007 at 04:41:37PM +0100, Adrian McMenamin wrote:
    > On Wed, October 3, 2007 7:18 am, Paul Mundt wrote:
    > > On Tue, Oct 02, 2007 at 10:09:27PM +0100, Adrian McMenamin wrote:
    > >> 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)
    > >>

    > > 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.

    >
    >
    > >From include/asm-sh/cpu-sh4/dma.h

    >
    > 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:
    >

    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.

    > 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 }
    >

    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!
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH] Fix SH DMAC code to handle PVR2 cascade

    On Thu, October 4, 2007 11:01 am, Paul Mundt wrote:
    > 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.
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread