From owner-freebsd-bugs Mon Jan 21 7:40:28 2002 Delivered-To: freebsd-bugs@hub.freebsd.org Received: from freefall.freebsd.org (freefall.FreeBSD.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id 6A32B37B402 for ; Mon, 21 Jan 2002 07:40:01 -0800 (PST) Received: (from gnats@localhost) by freefall.freebsd.org (8.11.6/8.11.6) id g0LFe1m44846; Mon, 21 Jan 2002 07:40:01 -0800 (PST) (envelope-from gnats) Received: from staff.rinet.ru (staff.rinet.ru [195.54.192.46]) by hub.freebsd.org (Postfix) with ESMTP id 6F93E37B417 for ; Mon, 21 Jan 2002 07:38:20 -0800 (PST) Received: (from gvs@localhost) by staff.rinet.ru (8.11.6/8.11.4) id g0LFcG968345; Mon, 21 Jan 2002 18:38:16 +0300 (MSK) (envelope-from gvs) Message-Id: <200201211538.g0LFcG968345@staff.rinet.ru> Date: Mon, 21 Jan 2002 18:38:16 +0300 (MSK) From: Seva Gluschenko To: FreeBSD-gnats-submit@freebsd.org X-Send-Pr-Version: 3.113 Subject: kern/34120: optimized fm801 pcm driver Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org >Number: 34120 >Category: kern >Synopsis: optimized fm801 pcm driver >Confidential: no >Severity: non-critical >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: update >Submitter-Id: current-users >Arrival-Date: Mon Jan 21 07:40:01 PST 2002 >Closed-Date: >Last-Modified: >Originator: Seva Gluschenko >Release: FreeBSD 4.5-PRERELEASE i386 >Organization: Cronyx Plus LLC / ru.rinet LIR >Environment: System: FreeBSD staff.rinet.ru 4.5-PRERELEASE FreeBSD 4.5-PRERELEASE #2: Tue Jan 1 18:04:00 MSK 2002 root@staff.rinet.ru:/usr/obj/usr/src/sys/staff i386 >Description: While doing a number of analysis and changes in /sys/dev/sound/pci for a future submission of pcm_drv.h driver template I've found fm801.c ForteMedia card driver written (transferred from ALSA) in an inaccurate and unoptimized manner. So I reviewed and optimized the code. Actually changes divide for two instances: 1. All subsequent calls to fm801_rd and fm801_wr functions are changed to inline macro substitutions because the named functions just added extra switch {} operator per call with no increased functionality wasting this way both namespace (not meaningful) and execution time (a bit more interesting). 2. The channel info structure got the new members taken from the card info structure to save both the variables space and amount of code generated, because the appropriate change of code allowed to substantially decrease channel processing functions. >How-To-Repeat: Review the code and use some search&replace and other editor commands ;). >Fix: The diff itself follows. Little bit long for not so big source, eh? --- fm801.c.orig Fri Jan 18 15:50:55 2002 +++ fm801.c Sun Jan 20 19:00:33 2002 @@ -1,5 +1,6 @@ /* * Copyright (c) 2000 Dmitry Dicky diwil@dataart.com + * Optimized by GvS (C) 2001 gvs@rinet.ru * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -125,7 +126,7 @@ struct pcm_channel *channel; struct snd_dbuf *buffer; u_int32_t spd, dir, fmt; /* speed, direction, format */ - u_int32_t shift; + u_int32_t shift, flip, start, nextblk, blksize; }; struct fm801_info { @@ -142,55 +143,17 @@ int regtype, regid, irqid; void *ih; - u_int32_t play_flip, - play_nextblk, - play_start, - play_blksize, - play_fmt, - play_shift, - play_size; - - u_int32_t rec_flip, - rec_nextblk, - rec_start, - rec_blksize, - rec_fmt, - rec_shift, - rec_size; - struct fm801_chinfo pch, rch; }; /* Bus Read / Write routines */ -static u_int32_t -fm801_rd(struct fm801_info *fm801, int regno, int size) -{ - switch(size) { - case 1: - return (bus_space_read_1(fm801->st, fm801->sh, regno)); - case 2: - return (bus_space_read_2(fm801->st, fm801->sh, regno)); - case 4: - return (bus_space_read_4(fm801->st, fm801->sh, regno)); - default: - return 0xffffffff; - } -} - -static void -fm801_wr(struct fm801_info *fm801, int regno, u_int32_t data, int size) -{ - switch(size) { - case 1: - return bus_space_write_1(fm801->st, fm801->sh, regno, data); - case 2: - return bus_space_write_2(fm801->st, fm801->sh, regno, data); - case 4: - return bus_space_write_4(fm801->st, fm801->sh, regno, data); - default: - return; - } -} +/* Taken from Maestro3 code */ +#define sc_rd_1(sc, regno) bus_space_read_1(sc->st, sc->sh, regno) +#define sc_rd_2(sc, regno) bus_space_read_2(sc->st, sc->sh, regno) +#define sc_rd_4(sc, regno) bus_space_read_4(sc->st, sc->sh, regno) +#define sc_wr_1(sc, regno, data) bus_space_write_1(sc->st, sc->sh, regno, data) +#define sc_wr_2(sc, regno, data) bus_space_write_2(sc->st, sc->sh, regno, data) +#define sc_wr_4(sc, regno, data) bus_space_write_4(sc->st, sc->sh, regno, data) /* -------------------------------------------------------------------- */ /* @@ -203,7 +166,7 @@ struct fm801_info *fm801 = (struct fm801_info *)devinfo; int i; - for (i = 0; i < TIMO && fm801_rd(fm801,FM_CODEC_CMD,2) & FM_CODEC_CMD_BUSY; i++) { + for (i = 0; i < TIMO && sc_rd_2(fm801,FM_CODEC_CMD) & FM_CODEC_CMD_BUSY; i++) { DELAY(10000); DPRINT("fm801 rdcd: 1 - DELAY\n"); } @@ -212,9 +175,9 @@ return 0; } - fm801_wr(fm801,FM_CODEC_CMD, regno|FM_CODEC_CMD_READ,2); + sc_wr_2(fm801,FM_CODEC_CMD, regno|FM_CODEC_CMD_READ); - for (i = 0; i < TIMO && !(fm801_rd(fm801,FM_CODEC_CMD,2) & FM_CODEC_CMD_VALID); i++) + for (i = 0; i < TIMO && !(sc_rd_2(fm801,FM_CODEC_CMD) & FM_CODEC_CMD_VALID); i++) { DELAY(10000); DPRINT("fm801 rdcd: 2 - DELAY\n"); @@ -224,7 +187,7 @@ return 0; } - return fm801_rd(fm801,FM_CODEC_DATA,2); + return sc_rd_2(fm801,FM_CODEC_DATA); } static int @@ -238,7 +201,7 @@ if(regno == AC97_REG_RECSEL) return; */ /* Poll until codec is ready */ - for (i = 0; i < TIMO && fm801_rd(fm801,FM_CODEC_CMD,2) & FM_CODEC_CMD_BUSY; i++) { + for (i = 0; i < TIMO && sc_rd_2(fm801,FM_CODEC_CMD) & FM_CODEC_CMD_BUSY; i++) { DELAY(10000); DPRINT("fm801 rdcd: 1 - DELAY\n"); } @@ -247,11 +210,11 @@ return -1; } - fm801_wr(fm801,FM_CODEC_DATA,data, 2); - fm801_wr(fm801,FM_CODEC_CMD, regno,2); + sc_wr_2(fm801,FM_CODEC_DATA,data); + sc_wr_2(fm801,FM_CODEC_CMD, regno); /* wait until codec is ready */ - for (i = 0; i < TIMO && fm801_rd(fm801,FM_CODEC_CMD,2) & FM_CODEC_CMD_BUSY; i++) { + for (i = 0; i < TIMO && sc_rd_2(fm801,FM_CODEC_CMD) & FM_CODEC_CMD_BUSY; i++) { DELAY(10000); DPRINT("fm801 wrcd: 2 - DELAY\n"); } @@ -279,40 +242,43 @@ fm801_intr(void *p) { struct fm801_info *fm801 = (struct fm801_info *)p; - u_int32_t intsrc = fm801_rd(fm801, FM_INTSTATUS, 2); + struct fm801_chinfo *ch; + u_int32_t intsrc = sc_rd_2(fm801, FM_INTSTATUS); DPRINT("\nfm801_intr intsrc 0x%x ", intsrc); if(intsrc & FM_INTSTATUS_PLAY) { - fm801->play_flip++; - if(fm801->play_flip & 1) { - fm801_wr(fm801, FM_PLAY_DMABUF1, fm801->play_start,4); + ch = &fm801->pch; + ch->flip++; + if(ch->flip & 1) { + sc_wr_4(fm801, FM_PLAY_DMABUF1, ch->start); } else - fm801_wr(fm801, FM_PLAY_DMABUF2, fm801->play_nextblk,4); + sc_wr_4(fm801, FM_PLAY_DMABUF2, ch->nextblk); chn_intr(fm801->pch.channel); } if(intsrc & FM_INTSTATUS_REC) { - fm801->rec_flip++; - if(fm801->rec_flip & 1) { - fm801_wr(fm801, FM_REC_DMABUF1, fm801->rec_start,4); + ch = &fm801->rch; + ch->flip++; + if(ch->flip & 1) { + sc_wr_4(fm801, FM_REC_DMABUF1, ch->start); } else - fm801_wr(fm801, FM_REC_DMABUF2, fm801->rec_nextblk,4); + sc_wr_4(fm801, FM_REC_DMABUF2, ch->nextblk); chn_intr(fm801->rch.channel); } if ( intsrc & FM_INTSTATUS_MPU ) { /* This is a TODOish thing... */ - fm801_wr(fm801, FM_INTSTATUS, intsrc & FM_INTSTATUS_MPU,2); + sc_wr_2(fm801, FM_INTSTATUS, intsrc & FM_INTSTATUS_MPU); } if ( intsrc & FM_INTSTATUS_VOL ) { /* This is a TODOish thing... */ - fm801_wr(fm801, FM_INTSTATUS, intsrc & FM_INTSTATUS_VOL,2); + sc_wr_2(fm801, FM_INTSTATUS, intsrc & FM_INTSTATUS_VOL); } DPRINT("fm801_intr clear\n\n"); - fm801_wr(fm801, FM_INTSTATUS, intsrc & (FM_INTSTATUS_PLAY | FM_INTSTATUS_REC), 2); + sc_wr_2(fm801, FM_INTSTATUS, intsrc & (FM_INTSTATUS_PLAY | FM_INTSTATUS_REC)); } /* -------------------------------------------------------------------- */ @@ -336,7 +302,6 @@ fm801ch_setformat(kobj_t obj, void *data, u_int32_t format) { struct fm801_chinfo *ch = data; - struct fm801_info *fm801 = ch->parent; DPRINT("fm801ch_setformat 0x%x : %s, %s, %s, %s\n", format, (format & AFMT_STEREO)?"stereo":"mono", @@ -344,17 +309,9 @@ (format & AFMT_SIGNED)? "signed":"unsigned", (format & AFMT_BIGENDIAN)?"bigendiah":"littleendian" ); - if(ch->dir == PCMDIR_PLAY) { - fm801->play_fmt = (format & AFMT_STEREO)? FM_PLAY_STEREO : 0; - fm801->play_fmt |= (format & AFMT_16BIT) ? FM_PLAY_16BIT : 0; - return 0; - } - - if(ch->dir == PCMDIR_REC ) { - fm801->rec_fmt = (format & AFMT_STEREO)? FM_REC_STEREO:0; - fm801->rec_fmt |= (format & AFMT_16BIT) ? FM_PLAY_16BIT : 0; - return 0; - } + ch->fmt = (format & AFMT_STEREO) ? ( (ch->dir == PCMDIR_PLAY) ? + FM_PLAY_STEREO : FM_REC_STEREO ) : 0; + ch->fmt |= (format & AFMT_16BIT) ? FM_PLAY_16BIT : 0; return 0; } @@ -381,44 +338,26 @@ fm801ch_setspeed(kobj_t obj, void *data, u_int32_t speed) { struct fm801_chinfo *ch = data; - struct fm801_info *fm801 = ch->parent; register int i; for (i = 0; i < 10 && fm801_rates[i].limit <= speed; i++) ; - if(ch->dir == PCMDIR_PLAY) { - fm801->pch.spd = fm801_rates[i].rate; - fm801->play_shift = (i<<8); - fm801->play_shift &= FM_PLAY_RATE_MASK; - } - - if(ch->dir == PCMDIR_REC ) { - fm801->rch.spd = fm801_rates[i].rate; - fm801->rec_shift = (i<<8); - fm801->rec_shift &= FM_REC_RATE_MASK; - } - + ch->shift = (i<<8); + ch->shift &= (ch->dir == PCMDIR_PLAY) ? + FM_PLAY_RATE_MASK : FM_REC_RATE_MASK; ch->spd = fm801_rates[i].rate; - return fm801_rates[i].rate; + return ch->spd; } static int fm801ch_setblocksize(kobj_t obj, void *data, u_int32_t blocksize) { struct fm801_chinfo *ch = data; - struct fm801_info *fm801 = ch->parent; - - if(ch->dir == PCMDIR_PLAY) { - if(fm801->play_flip) return fm801->play_blksize; - fm801->play_blksize = blocksize; - } - if(ch->dir == PCMDIR_REC) { - if(fm801->rec_flip) return fm801->rec_blksize; - fm801->rec_blksize = blocksize; - } + if(ch->flip) return ch->blksize; + ch->blksize = blocksize; DPRINT("fm801ch_setblocksize %d (dir %d)\n",blocksize, ch->dir); @@ -439,42 +378,35 @@ return 0; } - if (ch->dir == PCMDIR_PLAY) { - if (go == PCMTRIG_START) { - - fm801->play_start = baseaddr; - fm801->play_nextblk = fm801->play_start + fm801->play_blksize; - fm801->play_flip = 0; - fm801_wr(fm801, FM_PLAY_DMALEN, fm801->play_blksize - 1, 2); - fm801_wr(fm801, FM_PLAY_DMABUF1,fm801->play_start,4); - fm801_wr(fm801, FM_PLAY_DMABUF2,fm801->play_nextblk,4); - fm801_wr(fm801, FM_PLAY_CTL, - FM_PLAY_START | FM_PLAY_STOPNOW | fm801->play_fmt | fm801->play_shift, - 2 ); - } else { - fm801->play_flip = 0; - k1 = fm801_rd(fm801, FM_PLAY_CTL,2); - fm801_wr(fm801, FM_PLAY_CTL, - (k1 & ~(FM_PLAY_STOPNOW | FM_PLAY_START)) | - FM_PLAY_BUF1_LAST | FM_PLAY_BUF2_LAST, 2 ); + ch->flip = 0; + if (go == PCMTRIG_START) { + ch->start = baseaddr; + ch->nextblk = ch->start + ch->blksize; + + if (ch->dir == PCMDIR_PLAY) { + sc_wr_2(fm801, FM_PLAY_DMALEN, ch->blksize - 1); + sc_wr_4(fm801, FM_PLAY_DMABUF1,ch->start); + sc_wr_4(fm801, FM_PLAY_DMABUF2,ch->nextblk); + sc_wr_2(fm801, FM_PLAY_CTL, FM_PLAY_START | + FM_PLAY_STOPNOW | ch->fmt | ch->shift); + } else if(ch->dir == PCMDIR_REC) { + sc_wr_2(fm801, FM_REC_DMALEN, ch->blksize - 1); + sc_wr_4(fm801, FM_REC_DMABUF1,ch->start); + sc_wr_4(fm801, FM_REC_DMABUF2,ch->nextblk); + sc_wr_2(fm801, FM_REC_CTL, FM_REC_START | + FM_REC_STOPNOW | ch->fmt | ch->shift); } - } else if(ch->dir == PCMDIR_REC) { - if (go == PCMTRIG_START) { - fm801->rec_start = baseaddr; - fm801->rec_nextblk = fm801->rec_start + fm801->rec_blksize; - fm801->rec_flip = 0; - fm801_wr(fm801, FM_REC_DMALEN, fm801->rec_blksize - 1, 2); - fm801_wr(fm801, FM_REC_DMABUF1,fm801->rec_start,4); - fm801_wr(fm801, FM_REC_DMABUF2,fm801->rec_nextblk,4); - fm801_wr(fm801, FM_REC_CTL, - FM_REC_START | FM_REC_STOPNOW | fm801->rec_fmt | fm801->rec_shift, - 2 ); - } else { - fm801->rec_flip = 0; - k1 = fm801_rd(fm801, FM_REC_CTL,2); - fm801_wr(fm801, FM_REC_CTL, + } else { + if (ch->dir == PCMDIR_PLAY) { + k1 = sc_rd_2(fm801, FM_PLAY_CTL); + sc_wr_2(fm801, FM_PLAY_CTL, + (k1 & ~(FM_PLAY_STOPNOW | FM_PLAY_START)) | + FM_PLAY_BUF1_LAST | FM_PLAY_BUF2_LAST); + } else if(ch->dir == PCMDIR_REC) { + k1 = sc_rd_2(fm801, FM_REC_CTL); + sc_wr_2(fm801, FM_REC_CTL, (k1 & ~(FM_REC_STOPNOW | FM_REC_START)) | - FM_REC_BUF1_LAST | FM_REC_BUF2_LAST, 2); + FM_REC_BUF1_LAST | FM_REC_BUF2_LAST); } } @@ -487,21 +419,10 @@ { struct fm801_chinfo *ch = data; struct fm801_info *fm801 = ch->parent; - int result = 0; - - if (ch->dir == PCMDIR_PLAY) { - result = fm801_rd(fm801, - (fm801->play_flip&1) ? - FM_PLAY_DMABUF2:FM_PLAY_DMABUF1, 4) - fm801->play_start; - } - - if (ch->dir == PCMDIR_REC) { - result = fm801_rd(fm801, - (fm801->rec_flip&1) ? - FM_REC_DMABUF2:FM_REC_DMABUF1, 4) - fm801->rec_start; - } - return result; + return sc_rd_4(fm801, (ch->dir == PCMDIR_PLAY) ? (ch->flip & 1) ? + FM_PLAY_DMABUF2 : FM_PLAY_DMABUF1 : (ch->flip & 1) ? + FM_REC_DMABUF2 : FM_REC_DMABUF1) - ch->start; } static struct pcmchan_caps * @@ -533,26 +454,26 @@ u_int32_t k1; /* reset codec */ - fm801_wr(fm801, FM_CODEC_CTL, 0x0020,2); + sc_wr_2(fm801, FM_CODEC_CTL, 0x0020); DELAY(100000); - fm801_wr(fm801, FM_CODEC_CTL, 0x0000,2); + sc_wr_2(fm801, FM_CODEC_CTL, 0x0000); DELAY(100000); - fm801_wr(fm801, FM_PCM_VOLUME, 0x0808,2); - fm801_wr(fm801, FM_FM_VOLUME, 0x0808,2); - fm801_wr(fm801, FM_I2S_VOLUME, 0x0808,2); - fm801_wr(fm801, 0x40,0x107f,2); /* enable legacy audio */ + sc_wr_2(fm801, FM_PCM_VOLUME, 0x0808); + sc_wr_2(fm801, FM_FM_VOLUME, 0x0808); + sc_wr_2(fm801, FM_I2S_VOLUME, 0x0808); + sc_wr_2(fm801, 0x40,0x107f); /* enable legacy audio */ - fm801_wr((void *)fm801, FM_RECORD_SOURCE, 0x0000,2); + sc_wr_2(fm801, FM_RECORD_SOURCE, 0x0000); /* Unmask playback, record and mpu interrupts, mask the rest */ - k1 = fm801_rd((void *)fm801, FM_INTMASK,2); - fm801_wr(fm801, FM_INTMASK, + k1 = sc_rd_2(fm801, FM_INTMASK); + sc_wr_2(fm801, FM_INTMASK, (k1 & ~(FM_INTMASK_PLAY | FM_INTMASK_REC | FM_INTMASK_MPU)) | - FM_INTMASK_VOL,2); - fm801_wr(fm801, FM_INTSTATUS, + FM_INTMASK_VOL); + sc_wr_2(fm801, FM_INTSTATUS, FM_INTSTATUS_PLAY | FM_INTSTATUS_REC | FM_INTSTATUS_MPU | - FM_INTSTATUS_VOL,2); + FM_INTSTATUS_VOL); DPRINT("FM801 init Ok\n"); return 0; >Release-Note: >Audit-Trail: >Unformatted: To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message