From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH] ALSA: fix emu8000 DRAM sizing for AWE64 Value Date: Fri, 09 Jan 2015 12:09:31 +0100 Message-ID: References: <54AF259A.3070204@flaterco.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Return-path: In-Reply-To: <54AF259A.3070204@flaterco.com> Sender: linux-kernel-owner@vger.kernel.org To: David Flater Cc: Jaroslav Kysela , LKML , Alsa Devel List-Id: alsa-devel@alsa-project.org 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 > --- > 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 >