On Fri, 23 Oct 2009, Juan Quintela wrote:
> malc <av1...@comtv.ru> wrote:
> > On Thu, 22 Oct 2009, Juan Quintela wrote:
> >
>
> Hi
>
> >> - es1370: the best working with migration.
> >> - adlib: I am not able to get sound out of it on any recent Fedora :(
> >
> > It's an FM chip, trying to play PCM with it just not gonna fly.
>
> That could expain it :)
>
> >> I disabled dma_running before loading state. malc can you take a look
> >> here?
>
> > Can you please elaborate on what is exactly the problem.
>
> What we have here:
>
> On save_state, we save a state field:
> val = s->dma_running; qemu_put_be32s (f, &val);
>
> Now on load state, we read it to one local variable:
> qemu_get_be32s (f, &dma_running);
>
> My understanding of the code here is that you should never assign
> directly s->dma_running, that you should set/clean it through
> cs_reset_voices()
> (setting this value has _side-effects_
Okay, i've looked at the code dma_running is basically a flag which is
sortof redundant but for the lack of better interface tells us whether
we have DREQ asserted, nothing more nothing less.
>
> So far so good.
>
> Now, the whole point of vmstate is:
> - state is described declaratively
> - and load/save are "kind" of inverse functions. One save
> the value of one field to the state, and the other takes the value
> from the state and put it back in the field.
> - Later (post_load), we do any side-effect that we need to do with the
> new loaded state.
>
> Here we can't do this. How would I do it in any other driver, i.e. (no
> audio). Easy, you just declare dma_running_vmstate in the state, and
> in post_load(), you just call cs_reset_voices depending on that value.
>
> You already ruled out that solution on ac97. Didn't even try here.
>
> No problem, I just read it on s->dma_running(), and on post_load I do:
>
> if (s->dma_running && (s->dregs[Interface_Configuration] & PEN)) {
> s->dma_running = 0;
> cs_reset_voices (s, s->dregs[FS_And_Playback_Data_Format]);
> }
>
> And call it a day :).
>
> But then I thought of what happens if you call loadvm from the monitor
> while s->dma_running = 1.
>
> And decided that the proper thing to do is to do:
>
> in pre_load()
> - we set dma_running to 0 if if it 1, but do it like in
> cs_reset_voices()
>
> if (s->dma_running) {
> DMA_release_DREQ (s->dma);
> AUD_set_active_out (s->voice, 0);
> }
> s->dma_running = 0;
>
> And now, in post_load, we know that s->dma_runing is what was comes from
> migration, and we can set it to this value. Do you think that my
> explanation is clear?
Sorry, i haven't slept in quite a while, so nothing makes any sense
currently, i'll try to do a mental exercise of running it solely
inside my brain sometime later.
> Discussing this with Anthony on irc, he thinks that dma_running is the
> same that (s->dregs[Interface_Configuration] & PEN), and that we could
> remove it from the state, but I am not sure, nad decided not to go that
> route (because I am not sure, not because it is not the same. I don't know).
It's not quite the same as Iconf & PEN (see for instance PPIO
handling, or lack thereof)
[..snip..]
--
mailto:av1...@comtv.ru
