lkml.org 
[lkml]   [2007]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [alsa-devel] [PATCH] ALSA AICA sound on SEGA Dreamcast - fix behaviour in poor resource conditions
    At Fri, 6 Jul 2007 10:46:59 +0100,
    Adrian McMenamin wrote:
    >
    > On 06/07/07, Takashi Iwai <tiwai@suse.de> wrote:
    > > At Thu, 5 Jul 2007 23:07:44 +0100,
    > > Adrian McMenamin wrote:
    > > > @@ -402,17 +396,10 @@
    > > > static int snd_aicapcm_pcm_trigger(struct snd_pcm_substream
    > > > *substream, int cmd)
    > > > {
    > > > - struct snd_card_aica *dreamcastcard;
    > > > switch (cmd) {
    > > > case SNDRV_PCM_TRIGGER_START:
    > > > spu_begin_dma(substream);
    > > > break;
    > > > - case SNDRV_PCM_TRIGGER_STOP:
    > > > - dreamcastcard = substream->pcm->private_data;
    > > > - if (dreamcastcard->timer.data)
    > > > - del_timer(&dreamcastcard->timer);
    > > > - aica_chn_halt();
    > > > - break;
    > > > default:
    > > > return -EINVAL;
    > > > }
    > >
    > > Is this a correct change? Then you'd have no control for stopping the
    > > stream.
    > >
    >
    > The shutdown is all done in the close() now, partly because
    > snd_aicapcm_pcm_trigger cannot sleep and close() can. Is there a
    > pressing reason to put code in snd_aicapcm_pcm_trigger?

    I guess you shoud call del_timer() there at least. Otherwise the
    timer handler (aica_period_elapsed()) might be called even if the
    stream is not really running.

    The below is some coding style fixes, BTW. I'm not sure whether
    queue_work() is intended in the block if "if (dma_check == 0)" or
    not.


    Takashi

    --- aica.c.old 2007-07-06 11:51:18.000000000 +0200
    +++ aica.c 2007-07-06 11:52:10.000000000 +0200
    @@ -256,7 +256,8 @@
    buffer_size = frames_to_bytes(runtime, runtime->buffer_size);
    if (runtime->channels > 1)
    dreamcastcard->channel->flags |= 0x01;
    - aica_dma_transfer(runtime->channels, buffer_size, dreamcastcard->substream);
    + aica_dma_transfer(runtime->channels, buffer_size,
    + dreamcastcard->substream);
    startup_aica(dreamcastcard);
    dreamcastcard->clicks =
    buffer_size / (AICA_PERIOD_SIZE * runtime->channels);
    @@ -268,9 +269,7 @@
    snd_pcm_period_elapsed(dreamcastcard->substream);
    dreamcastcard->clicks++;
    if (unlikely(dreamcastcard->clicks >= AICA_PERIOD_NUMBER))
    - {
    dreamcastcard->clicks %= AICA_PERIOD_NUMBER;
    - }
    mod_timer(&dreamcastcard->timer, jiffies + 1);
    }
    }
    @@ -300,8 +299,7 @@
    dreamcastcard->current_period = play_period;
    if (unlikely(dreamcastcard->dma_check == 0))
    dreamcastcard->dma_check = 1;
    - queue_work(aica_queue, &(dreamcastcard->spu_dma_work));
    -
    + queue_work(aica_queue, &(dreamcastcard->spu_dma_work));
    }

    static void spu_begin_dma(struct snd_pcm_substream *substream)
    @@ -311,7 +309,7 @@
    struct snd_pcm_runtime *runtime;
    runtime = substream->runtime;
    dreamcastcard = substream->pcm->private_data;
    - //get the queue to do the work
    + /* get the queue to do the work */
    queue_work(aica_queue, &(dreamcastcard->spu_dma_work));
    /* Timer may already be running */
    if (unlikely(dreamcastcard->timer.data)) {
    @@ -597,7 +595,7 @@
    strcpy(dreamcastcard->card->shortname, SND_AICA_DRIVER);
    strcpy(dreamcastcard->card->longname,
    "Yamaha AICA Super Intelligent Sound Processor for SEGA Dreamcast");
    - //start the worker thread
    + /* start the worker thread */
    INIT_WORK(&(dreamcastcard->spu_dma_work), run_spu_dma);
    /* Load the PCM 'chip' */
    err = snd_aicapcmchip(dreamcastcard, 0);
    @@ -631,7 +629,8 @@
    .probe = snd_aica_probe,
    .remove = snd_aica_remove,
    .driver = {
    - .name = SND_AICA_DRIVER},
    + .name = SND_AICA_DRIVER
    + },
    };

    static int __init aica_init(void)
    -
    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/
    \
     
     \ /
      Last update: 2007-07-06 11:59    [W:0.028 / U:29.436 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site