Re: [PATCH] ALSA: fix emu8000 DRAM sizing for AWE64 Value

From: Takashi Iwai
Date: Fri Jan 09 2015 - 06:09:38 EST


At Thu, 08 Jan 2015 19:49:30 -0500,
David Flater wrote:
>
> Applicable to any kernel since 2013:
>
> The special case added in commit 1338fc97d07a did not handle the possibility
> that the address space on an AWE64 Value would wrap around at 512 KiB. That
> is what it does, so the memory is still not detected on those cards.
>
> Fix that with a logic clean-up that eliminates the need for a special case.
> Also log the amount of memory detected at level INFO and add sufficiently
> verbose debugging to diagnose any additional faults of this kind.
>
> Tested on unexpanded CT4390 (4 MiB), CT4520 (512 KiB), and CT4380 (512 KiB).
> The latter is commonly said to come with 1 MiB of DRAM, but Creative's AWE
> Control app agreed that mine has only 512 KiB. It has the same memory chip
> as the CT4520.
>
> Signed-off-by: David Flater <dave@xxxxxxxxxxxx>
> ---
> History:
> 2015-01-08 v1 patch sent to LKML, Alsa Devel and maintainers.
>
> The affected function first appeared in alsa-driver-0.3.0 and was merged in
> linux-2.5.5. Its somewhat different ancestor was in sound/oss/awe_wave.c.
>
> AFAICT, the manufacturer never disclosed the right way to do it. In the
> AWE32 Developer's Information Pack (1994-1996) the RAM sizing function was
> implemented in object files with a license that prohibited even disassembly.

Unfortunately it's not easy to read what you really changed because
the patch moves the loop in a deeper indentation. Could you rewrite
to keep the original code as much as possible? From what I read, the
necessary change would be something like:

/* If that didn't work, we have no RAM at all */
EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
EMU8000_SMLD_READ(emu); /* discard stale data */
if (EMU8000_SMLD_READ(emu) != UNIQUE_ID1) {
detected_size = 0;
goto memory_detect_end;
}

snd_emu8000_read_wait(emu);
detected_size = 512 * 1024;

while (size < EMU8000_MAX_DRAM) {
....
}

memory_detect_end:
emu->mem_size = detected_size;
emu->dram_checked = 1;

Also, don't change the printk level. A patch should do only one
thing. If you want to increase the printk level for the detected
memory size, do it another patch.


thanks,

Takashi

>
> sound/isa/sb/emu8000.c | 127 +++++++++++++++++++++++++++++--------------------
> 1 file changed, 76 insertions(+), 51 deletions(-)
>
> diff --git a/sound/isa/sb/emu8000.c b/sound/isa/sb/emu8000.c
> index 45fcdff..3aa2250 100644
> --- a/sound/isa/sb/emu8000.c
> +++ b/sound/isa/sb/emu8000.c
> @@ -378,13 +378,13 @@ init_arrays(struct snd_emu8000 *emu)
> static void
> size_dram(struct snd_emu8000 *emu)
> {
> - int i, size, detected_size;
> + int i, size;
> + unsigned short rdback;
>
> if (emu->dram_checked)
> return;
>
> size = 0;
> - detected_size = 0;
>
> /* write out a magic number */
> snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_WRITE);
> @@ -392,55 +392,81 @@ size_dram(struct snd_emu8000 *emu)
> EMU8000_SMALW_WRITE(emu, EMU8000_DRAM_OFFSET);
> EMU8000_SMLD_WRITE(emu, UNIQUE_ID1);
> snd_emu8000_init_fm(emu); /* This must really be here and not 2 lines back even */
> -
> - while (size < EMU8000_MAX_DRAM) {
> -
> - size += 512 * 1024; /* increment 512kbytes */
> -
> - /* Write a unique data on the test address.
> - * if the address is out of range, the data is written on
> - * 0x200000(=EMU8000_DRAM_OFFSET). Then the id word is
> - * changed by this data.
> - */
> - /*snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_WRITE);*/
> - EMU8000_SMALW_WRITE(emu, EMU8000_DRAM_OFFSET + (size>>1));
> - EMU8000_SMLD_WRITE(emu, UNIQUE_ID2);
> - snd_emu8000_write_wait(emu);
> -
> - /*
> - * read the data on the just written DRAM address
> - * if not the same then we have reached the end of ram.
> - */
> - /*snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_READ);*/
> - EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET + (size>>1));
> - /*snd_emu8000_read_wait(emu);*/
> - EMU8000_SMLD_READ(emu); /* discard stale data */
> - if (EMU8000_SMLD_READ(emu) != UNIQUE_ID2)
> - break; /* no memory at this address */
> + snd_emu8000_write_wait(emu);
> +
> + /* If that didn't work, we have no RAM at all. */
> + EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
> + rdback = EMU8000_SMLD_READ(emu);
> + snd_printdd("EMU8000 [0x%lx]: initial discard data = %04hx\n",
> + emu->port1, rdback);
> + rdback = EMU8000_SMLD_READ(emu);
> + snd_printdd("EMU8000 [0x%lx]: initial readback = %04hx\n",
> + emu->port1, rdback);
> + if (rdback == UNIQUE_ID1) {
> snd_emu8000_read_wait(emu);
>
> /*
> - * If it is the same it could be that the address just
> - * wraps back to the beginning; so check to see if the
> - * initial value has been overwritten.
> + * If a write succeeds at the beginning of a 512 KiB page we
> + * assume that the whole page is there.
> */
> - EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
> - EMU8000_SMLD_READ(emu); /* discard stale data */
> - if (EMU8000_SMLD_READ(emu) != UNIQUE_ID1)
> - break; /* we must have wrapped around */
> - snd_emu8000_read_wait(emu);
> -
> - /* Otherwise, it's valid memory. */
> - detected_size = size + 512 * 1024;
> - }
> -
> - /* Distinguish 512 KiB from 0. */
> - if (detected_size == 0) {
> - snd_emu8000_read_wait(emu);
> - EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
> - EMU8000_SMLD_READ(emu); /* discard stale data */
> - if (EMU8000_SMLD_READ(emu) == UNIQUE_ID1)
> - detected_size = 512 * 1024;
> + size = 512 * 1024;
> +
> + while (size < EMU8000_MAX_DRAM) {
> + /*
> + * Write a unique data on the test address. If the
> + * address is out of range, the data is written on
> + * 0x200000(=EMU8000_DRAM_OFFSET). Then the id word
> + * is changed by this data.
> + */
> + EMU8000_SMALW_WRITE(emu, EMU8000_DRAM_OFFSET +
> + (size>>1));
> + EMU8000_SMLD_WRITE(emu, UNIQUE_ID2);
> + snd_emu8000_write_wait(emu);
> +
> + /*
> + * Read the data on the just written DRAM address.
> + * If not the same then we have reached the end of RAM.
> + */
> + EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET +
> + (size>>1));
> + rdback = EMU8000_SMLD_READ(emu);
> + snd_printdd("EMU8000 [0x%lx]: ID2 discard data = %04hx\n",
> + emu->port1, rdback);
> + rdback = EMU8000_SMLD_READ(emu);
> + snd_printdd("EMU8000 [0x%lx]: ID2 readback = %04hx\n",
> + emu->port1, rdback);
> + if (rdback != UNIQUE_ID2) {
> + snd_printdd("EMU8000 [0x%lx]: ID2 break\n",
> + emu->port1);
> + break; /* no memory at this address */
> + }
> + snd_emu8000_read_wait(emu);
> +
> + /*
> + * If it is the same it could be that the address just
> + * wraps back to the beginning, so check to see if the
> + * initial value has been overwritten.
> + */
> + EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
> + rdback = EMU8000_SMLD_READ(emu);
> + snd_printdd("EMU8000 [0x%lx]: ID1 discard data = %04hx\n",
> + emu->port1, rdback);
> + rdback = EMU8000_SMLD_READ(emu);
> + snd_printdd("EMU8000 [0x%lx]: ID1 readback = %04hx\n",
> + emu->port1, rdback);
> + if (rdback != UNIQUE_ID1) {
> + snd_printdd("EMU8000 [0x%lx]: ID1 break\n",
> + emu->port1);
> + break; /* may have wrapped around */
> + }
> + snd_emu8000_read_wait(emu);
> +
> + /* Otherwise, it's valid memory. */
> + size += 512 * 1024;
> + }
> + } else {
> + snd_printdd("EMU8000 [0x%lx]: initial test failed\n",
> + emu->port1);
> }
>
> /* wait until FULL bit in SMAxW register is false */
> @@ -454,10 +480,9 @@ size_dram(struct snd_emu8000 *emu)
> snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_CLOSE);
> snd_emu8000_dma_chan(emu, 1, EMU8000_RAM_CLOSE);
>
> - snd_printdd("EMU8000 [0x%lx]: %d Kb on-board memory detected\n",
> - emu->port1, detected_size/1024);
> -
> - emu->mem_size = detected_size;
> + snd_printk(KERN_INFO "sbawe [0x%lx]: %d B on-board DRAM detected\n",
> + emu->port1, size);
> + emu->mem_size = size;
> emu->dram_checked = 1;
> }
>
> --
> 1.8.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/