[PATCH 1/7] ALSA: emu10k1: drop redundant snd_emu10k1_efx_playback_hw_free()

[PATCH 1/7] ALSA: emu10k1: drop redundant snd_emu10k1_efx_playback_hw_free()

[Oswald Buddenhagen]

Or actually, replace snd_emu10k1_playback_hw_free() with it, as that is
a subset.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 sound/pci/emu10k1/emupcm.c | 32 +-------------------------------
 1 file changed, 1 insertion(+), 31 deletions(-)

diff --git a/sound/pci/emu10k1/emupcm.c b/sound/pci/emu10k1/emupcm.c
index 48af77ae8020..b89382aa4db6 100644
--- a/sound/pci/emu10k1/emupcm.c
+++ b/sound/pci/emu10k1/emupcm.c
@@ -429,36 +429,6 @@ static int snd_emu10k1_playback_hw_params(struct snd_pcm_substream *substream,
 }
 
 static int snd_emu10k1_playback_hw_free(struct snd_pcm_substream *substream)
-{
-	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct snd_emu10k1_pcm *epcm;
-
-	if (runtime->private_data == NULL)
-		return 0;
-	epcm = runtime->private_data;
-	if (epcm->extra) {
-		snd_emu10k1_voice_free(epcm->emu, epcm->extra);
-		epcm->extra = NULL;
-	}
-	if (epcm->voices[1]) {
-		snd_emu10k1_voice_free(epcm->emu, epcm->voices[1]);
-		epcm->voices[1] = NULL;
-	}
-	if (epcm->voices[0]) {
-		snd_emu10k1_voice_free(epcm->emu, epcm->voices[0]);
-		epcm->voices[0] = NULL;
-	}
-	if (epcm->memblk) {
-		snd_emu10k1_free_pages(emu, epcm->memblk);
-		epcm->memblk = NULL;
-		epcm->start_addr = 0;
-	}
-	snd_pcm_lib_free_pages(substream);
-	return 0;
-}
-
-static int snd_emu10k1_efx_playback_hw_free(struct snd_pcm_substream *substream)
 {
 	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
 	struct snd_pcm_runtime *runtime = substream->runtime;
@@ -1372,7 +1342,7 @@ static const struct snd_pcm_ops snd_emu10k1_efx_playback_ops = {
 	.open =			snd_emu10k1_efx_playback_open,
 	.close =		snd_emu10k1_efx_playback_close,
 	.hw_params =		snd_emu10k1_playback_hw_params,
-	.hw_free =		snd_emu10k1_efx_playback_hw_free,
+	.hw_free =		snd_emu10k1_playback_hw_free,
 	.prepare =		snd_emu10k1_efx_playback_prepare,
 	.trigger =		snd_emu10k1_efx_playback_trigger,
 	.pointer =		snd_emu10k1_efx_playback_pointer,
-- 
2.40.0.152.g15d061e6df

[PATCH 2/7] ALSA: emu10k1: drop redundant snd_emu10k1_efx_playback_pointer()

[Oswald Buddenhagen]

It's just an (outdated) copy of snd_emu10k1_playback_pointer().

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 sound/pci/emu10k1/emupcm.c | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/sound/pci/emu10k1/emupcm.c b/sound/pci/emu10k1/emupcm.c
index b89382aa4db6..b7830fd5c2b4 100644
--- a/sound/pci/emu10k1/emupcm.c
+++ b/sound/pci/emu10k1/emupcm.c
@@ -76,23 +76,6 @@ static void snd_emu10k1_pcm_efx_interrupt(struct snd_emu10k1 *emu,
 	snd_pcm_period_elapsed(emu->pcm_capture_efx_substream);
 }	 
 
-static snd_pcm_uframes_t snd_emu10k1_efx_playback_pointer(struct snd_pcm_substream *substream)
-{
-	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
-	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct snd_emu10k1_pcm *epcm = runtime->private_data;
-	unsigned int ptr;
-
-	if (!epcm->running)
-		return 0;
-	ptr = snd_emu10k1_ptr_read(emu, CCCA, epcm->voices[0]->number) & 0x00ffffff;
-	ptr += runtime->buffer_size;
-	ptr -= epcm->ccca_start_addr;
-	ptr %= runtime->buffer_size;
-
-	return ptr;
-}
-
 static int snd_emu10k1_pcm_channel_alloc(struct snd_emu10k1_pcm * epcm, int voices)
 {
 	int err, i;
@@ -1345,7 +1328,7 @@ static const struct snd_pcm_ops snd_emu10k1_efx_playback_ops = {
 	.hw_free =		snd_emu10k1_playback_hw_free,
 	.prepare =		snd_emu10k1_efx_playback_prepare,
 	.trigger =		snd_emu10k1_efx_playback_trigger,
-	.pointer =		snd_emu10k1_efx_playback_pointer,
+	.pointer =		snd_emu10k1_playback_pointer,
 };
 
 int snd_emu10k1_pcm(struct snd_emu10k1 *emu, int device)
-- 
2.40.0.152.g15d061e6df

[PATCH 3/7] ALSA: emu10k1: remove obsolete card type variable and defines

[Oswald Buddenhagen]

The use of the variable was removed in commit 2b637da5a1b ("clean up
card features"). That commit also broke user space (the ioctl
structure), at which point the defines became meaningless, so I don't
think purging them is a problem.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 include/sound/emu10k1.h      | 1 -
 include/uapi/sound/emu10k1.h | 3 ---
 2 files changed, 4 deletions(-)

diff --git a/include/sound/emu10k1.h b/include/sound/emu10k1.h
index 39787fecc8d9..9a6ede9ac18a 100644
--- a/include/sound/emu10k1.h
+++ b/include/sound/emu10k1.h
@@ -1691,7 +1691,6 @@ struct snd_emu10k1 {
 	unsigned int revision;			/* chip revision */
 	unsigned int serial;			/* serial number */
 	unsigned short model;			/* subsystem id */
-	unsigned int card_type;			/* EMU10K1_CARD_* */
 	unsigned int ecard_ctrl;		/* ecard control bits */
 	unsigned int address_mode;		/* address mode */
 	unsigned long dma_mask;			/* PCI DMA mask */
diff --git a/include/uapi/sound/emu10k1.h b/include/uapi/sound/emu10k1.h
index 1c1f1dd44611..c2414bd5aecd 100644
--- a/include/uapi/sound/emu10k1.h
+++ b/include/uapi/sound/emu10k1.h
@@ -15,9 +15,6 @@
  * ---- FX8010 ----
  */
 
-#define EMU10K1_CARD_CREATIVE			0x00000000
-#define EMU10K1_CARD_EMUAPS			0x00000001
-
 #define EMU10K1_FX8010_PCM_COUNT		8
 
 /*
-- 
2.40.0.152.g15d061e6df

[PATCH 4/7] ALSA: emu10k1: remove unused `resume` parameter from snd_emu10k1_init()

[Oswald Buddenhagen]

This was unnecessarily added in commit 09668b441da.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 sound/pci/emu10k1/emu10k1_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/pci/emu10k1/emu10k1_main.c b/sound/pci/emu10k1/emu10k1_main.c
index 3880f359e688..cbfaa4bc70a2 100644
--- a/sound/pci/emu10k1/emu10k1_main.c
+++ b/sound/pci/emu10k1/emu10k1_main.c
@@ -140,7 +140,7 @@ static const unsigned int i2c_adc_init[][2] = {
 	{ 0x15, ADC_MUX_2 },  /* ADC Mixer control. Mic for A2ZS Notebook */
 };
 
-static int snd_emu10k1_init(struct snd_emu10k1 *emu, int enable_ir, int resume)
+static int snd_emu10k1_init(struct snd_emu10k1 *emu, int enable_ir)
 {
 	unsigned int silent_page;
 	int ch;
@@ -1975,7 +1975,7 @@ int snd_emu10k1_create(struct snd_card *card,
 		emu->voices[idx].number = idx;
 	}
 
-	err = snd_emu10k1_init(emu, enable_ir, 0);
+	err = snd_emu10k1_init(emu, enable_ir);
 	if (err < 0)
 		return err;
 #ifdef CONFIG_PM_SLEEP
@@ -2068,7 +2068,7 @@ void snd_emu10k1_resume_init(struct snd_emu10k1 *emu)
 		snd_emu10k1_emu1010_init(emu);
 	else
 		snd_emu10k1_ptr_write(emu, AC97SLOT, 0, AC97SLOT_CNTR|AC97SLOT_LFE);
-	snd_emu10k1_init(emu, emu->enable_ir, 1);
+	snd_emu10k1_init(emu, emu->enable_ir);
 }
 
 void snd_emu10k1_resume_regs(struct snd_emu10k1 *emu)
-- 
2.40.0.152.g15d061e6df

Re: [PATCH 4/7] ALSA: emu10k1: remove unused `resume` parameter from snd_emu10k1_init()

[Takashi Iwai]

On Fri, 21 Apr 2023 16:10:03 +0200,
Oswald Buddenhagen wrote:
> 
> This was unnecessarily added in commit 09668b441da.

Please put the commit summary, too.


thanks,

Takashi

[PATCH 5/7] ALSA: emu10k1: remove unused snd_emu10k1_voice.emu field

[Oswald Buddenhagen]

It was written, but never read from. Its value is available via the epcm
field.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 include/sound/emu10k1.h          | 1 -
 sound/pci/emu10k1/emu10k1_main.c | 4 +---
 sound/pci/emu10k1/p16v.c         | 2 --
 3 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/sound/emu10k1.h b/include/sound/emu10k1.h
index 9a6ede9ac18a..d1bae032c05f 100644
--- a/include/sound/emu10k1.h
+++ b/include/sound/emu10k1.h
@@ -1485,7 +1485,6 @@ enum {
 struct snd_emu10k1;
 
 struct snd_emu10k1_voice {
-	struct snd_emu10k1 *emu;
 	int number;
 	unsigned int use: 1,
 	    pcm: 1,
diff --git a/sound/pci/emu10k1/emu10k1_main.c b/sound/pci/emu10k1/emu10k1_main.c
index cbfaa4bc70a2..b862f6e7163b 100644
--- a/sound/pci/emu10k1/emu10k1_main.c
+++ b/sound/pci/emu10k1/emu10k1_main.c
@@ -1970,10 +1970,8 @@ int snd_emu10k1_create(struct snd_card *card,
 		pgtbl[idx] = cpu_to_le32(silent_page | idx);
 
 	/* set up voice indices */
-	for (idx = 0; idx < NUM_G; idx++) {
-		emu->voices[idx].emu = emu;
+	for (idx = 0; idx < NUM_G; idx++)
 		emu->voices[idx].number = idx;
-	}
 
 	err = snd_emu10k1_init(emu, enable_ir);
 	if (err < 0)
diff --git a/sound/pci/emu10k1/p16v.c b/sound/pci/emu10k1/p16v.c
index 18a1b0740e6b..f5e0972187a7 100644
--- a/sound/pci/emu10k1/p16v.c
+++ b/sound/pci/emu10k1/p16v.c
@@ -181,7 +181,6 @@ static int snd_p16v_pcm_open_playback_channel(struct snd_pcm_substream *substrea
   
 	runtime->hw = snd_p16v_playback_hw;
 
-        channel->emu = emu;
         channel->number = channel_id;
 
         channel->use=1;
@@ -230,7 +229,6 @@ static int snd_p16v_pcm_open_capture_channel(struct snd_pcm_substream *substream
   
 	runtime->hw = snd_p16v_capture_hw;
 
-	channel->emu = emu;
 	channel->number = channel_id;
 
 	channel->use=1;
-- 
2.40.0.152.g15d061e6df

[PATCH 6/7] ALSA: emu10k1: remove unused emu->pcm_playback_efx_substream field

[Oswald Buddenhagen]

Amends commit 27ae958cf6.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---
 include/sound/emu10k1.h    | 1 -
 sound/pci/emu10k1/emupcm.c | 2 --
 2 files changed, 3 deletions(-)

diff --git a/include/sound/emu10k1.h b/include/sound/emu10k1.h
index d1bae032c05f..84ea53e9c83f 100644
--- a/include/sound/emu10k1.h
+++ b/include/sound/emu10k1.h
@@ -1762,7 +1762,6 @@ struct snd_emu10k1 {
 	struct snd_pcm_substream *pcm_capture_substream;
 	struct snd_pcm_substream *pcm_capture_mic_substream;
 	struct snd_pcm_substream *pcm_capture_efx_substream;
-	struct snd_pcm_substream *pcm_playback_efx_substream;
 
 	struct snd_timer *timer;
 
diff --git a/sound/pci/emu10k1/emupcm.c b/sound/pci/emu10k1/emupcm.c
index b7830fd5c2b4..c2749ad59e10 100644
--- a/sound/pci/emu10k1/emupcm.c
+++ b/sound/pci/emu10k1/emupcm.c
@@ -1044,8 +1044,6 @@ static int snd_emu10k1_efx_playback_open(struct snd_pcm_substream *substream)
 	epcm->type = PLAYBACK_EFX;
 	epcm->substream = substream;
 	
-	emu->pcm_playback_efx_substream = substream;
-
 	runtime->private_data = epcm;
 	runtime->private_free = snd_emu10k1_pcm_free_substream;
 	runtime->hw = snd_emu10k1_efx_playback;
-- 
2.40.0.152.g15d061e6df

Re: [PATCH 6/7] ALSA: emu10k1: remove unused emu->pcm_playback_efx_substream field

[Takashi Iwai]

On Fri, 21 Apr 2023 16:10:05 +0200,
Oswald Buddenhagen wrote:
> 
> Amends commit 27ae958cf6.

I couldn't find this commit ID in the upstream.
Also put the commit summary in text, not only the SHA1 ID, too.


thanks,

Takashi

Re: [PATCH 6/7] ALSA: emu10k1: remove unused emu->pcm_playback_efx_substream field

[Oswald Buddenhagen]

On Fri, Apr 21, 2023 at 04:54:21PM +0200, Takashi Iwai wrote:
>On Fri, 21 Apr 2023 16:10:05 +0200, Oswald Buddenhagen wrote:
>> 
>> Amends commit 27ae958cf6.
>
>I couldn't find this commit ID in the upstream.
>
it's from the historic repo. i'll amend it to mention that.

>Also put the commit summary in text, not only the SHA1 ID, too.
>
right.

[PATCH 7/7] ALSA: emu10k1: clean up P16V part somewhat

[Oswald Buddenhagen]

Detach it better from the main PCM driver, which it really doesn't have
much in common with.

In particular, this moves the interrupt handler implementation into
p16v.c, and makes it access the substream runtime status more directly,
so it doesn't need to abuse structs snd_emu10k1_pcm and
snd_emu10k1_voice any more.

We don't need private pcm runtime data at all, as the only thing it was
used for (except the back-link to the substream) was the `running` flag.
So store that directly in runtime->private_data.

This somewhat radical strip-down shows that this driver contains some
complexity that was never actually utilized. I suppose the right way to
fully utilize the hardware in a simple way would be introducing more
substreams. This wouldn't require any of the removed code.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

---

FIXME: only build-tested. it would require someone with an Audigy 2/4
card to runtime-test it.

i don't mind if this is deferred until someone tests it.
---
 include/sound/emu10k1.h  |   3 +-
 sound/pci/emu10k1/irq.c  |  32 ++---------
 sound/pci/emu10k1/p16v.c | 111 ++++++++++++++++-----------------------
 3 files changed, 50 insertions(+), 96 deletions(-)

diff --git a/include/sound/emu10k1.h b/include/sound/emu10k1.h
index 84ea53e9c83f..4b4026dfde5e 100644
--- a/include/sound/emu10k1.h
+++ b/include/sound/emu10k1.h
@@ -1737,8 +1737,6 @@ struct snd_emu10k1 {
 	spinlock_t i2c_lock; /* serialises access to i2c port */
 
 	struct snd_emu10k1_voice voices[NUM_G];
-	struct snd_emu10k1_voice p16v_voices[4];
-	struct snd_emu10k1_voice p16v_capture_voice;
 	int p16v_device_offset;
 	u32 p16v_capture_source;
 	u32 p16v_capture_channel;
@@ -1758,6 +1756,7 @@ struct snd_emu10k1 {
 	void (*capture_efx_interrupt)(struct snd_emu10k1 *emu, unsigned int status);
 	void (*spdif_interrupt)(struct snd_emu10k1 *emu, unsigned int status);
 	void (*dsp_interrupt)(struct snd_emu10k1 *emu);
+	void (*p16v_interrupt)(struct snd_emu10k1 *emu);
 
 	struct snd_pcm_substream *pcm_capture_substream;
 	struct snd_pcm_substream *pcm_capture_mic_substream;
diff --git a/sound/pci/emu10k1/irq.c b/sound/pci/emu10k1/irq.c
index ebb2275efb6c..dfb44e5e69a7 100644
--- a/sound/pci/emu10k1/irq.c
+++ b/sound/pci/emu10k1/irq.c
@@ -18,7 +18,7 @@
 irqreturn_t snd_emu10k1_interrupt(int irq, void *dev_id)
 {
 	struct snd_emu10k1 *emu = dev_id;
-	unsigned int status, status2, orig_status, orig_status2;
+	unsigned int status, orig_status;
 	int handled = 0;
 	int timeout = 0;
 
@@ -139,32 +139,10 @@ irqreturn_t snd_emu10k1_interrupt(int irq, void *dev_id)
 			status &= ~IPR_FXDSP;
 		}
 		if (status & IPR_P16V) {
-			while ((status2 = inl(emu->port + IPR2)) != 0) {
-				u32 mask = INTE2_PLAYBACK_CH_0_LOOP;  /* Full Loop */
-				struct snd_emu10k1_voice *pvoice = &(emu->p16v_voices[0]);
-				struct snd_emu10k1_voice *cvoice = &(emu->p16v_capture_voice);
-
-				/* dev_dbg(emu->card->dev, "status2=0x%x\n", status2); */
-				orig_status2 = status2;
-				if(status2 & mask) {
-					if(pvoice->use) {
-						snd_pcm_period_elapsed(pvoice->epcm->substream);
-					} else { 
-						dev_err(emu->card->dev,
-							"p16v: status: 0x%08x, mask=0x%08x, pvoice=%p, use=%d\n",
-							status2, mask, pvoice,
-							pvoice->use);
-					}
-				}
-				if(status2 & 0x110000) {
-					/* dev_info(emu->card->dev, "capture int found\n"); */
-					if(cvoice->use) {
-						/* dev_info(emu->card->dev, "capture period_elapsed\n"); */
-						snd_pcm_period_elapsed(cvoice->epcm->substream);
-					}
-				}
-				outl(orig_status2, emu->port + IPR2); /* ack all */
-			}
+			if (emu->p16v_interrupt)
+				emu->p16v_interrupt(emu);
+			else
+				outl(0, emu->port + INTE2);
 			status &= ~IPR_P16V;
 		}
 
diff --git a/sound/pci/emu10k1/p16v.c b/sound/pci/emu10k1/p16v.c
index f5e0972187a7..ce4d3450959c 100644
--- a/sound/pci/emu10k1/p16v.c
+++ b/sound/pci/emu10k1/p16v.c
@@ -149,125 +149,69 @@ static const struct snd_pcm_hardware snd_p16v_capture_hw = {
 	.fifo_size =		0,
 };
 
-static void snd_p16v_pcm_free_substream(struct snd_pcm_runtime *runtime)
-{
-	struct snd_emu10k1_pcm *epcm = runtime->private_data;
-
-	kfree(epcm);
-}
-
 /* open_playback callback */
 static int snd_p16v_pcm_open_playback_channel(struct snd_pcm_substream *substream, int channel_id)
 {
-	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
-        struct snd_emu10k1_voice *channel = &(emu->p16v_voices[channel_id]);
-	struct snd_emu10k1_pcm *epcm;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	int err;
 
-	epcm = kzalloc(sizeof(*epcm), GFP_KERNEL);
-	/* dev_dbg(emu->card->dev, "epcm kcalloc: %p\n", epcm); */
-
-	if (epcm == NULL)
-		return -ENOMEM;
-	epcm->emu = emu;
-	epcm->substream = substream;
 	/*
 	dev_dbg(emu->card->dev, "epcm device=%d, channel_id=%d\n",
 		   substream->pcm->device, channel_id);
 	*/
-	runtime->private_data = epcm;
-	runtime->private_free = snd_p16v_pcm_free_substream;
   
 	runtime->hw = snd_p16v_playback_hw;
 
-        channel->number = channel_id;
-
-        channel->use=1;
 #if 0 /* debug */
 	dev_dbg(emu->card->dev,
 		   "p16v: open channel_id=%d, channel=%p, use=0x%x\n",
 		   channel_id, channel, channel->use);
 	dev_dbg(emu->card->dev, "open:channel_id=%d, chip=%p, channel=%p\n",
 	       channel_id, chip, channel);
 #endif /* debug */
 	/* channel->interrupt = snd_p16v_pcm_channel_interrupt; */
-	channel->epcm = epcm;
 	err = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
 	if (err < 0)
                 return err;
 
 	runtime->sync.id32[0] = substream->pcm->card->number;
 	runtime->sync.id32[1] = 'P';
 	runtime->sync.id32[2] = 16;
 	runtime->sync.id32[3] = 'V';
 
 	return 0;
 }
+
 /* open_capture callback */
 static int snd_p16v_pcm_open_capture_channel(struct snd_pcm_substream *substream, int channel_id)
 {
-	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
-	struct snd_emu10k1_voice *channel = &(emu->p16v_capture_voice);
-	struct snd_emu10k1_pcm *epcm;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	int err;
 
-	epcm = kzalloc(sizeof(*epcm), GFP_KERNEL);
-	/* dev_dbg(emu->card->dev, "epcm kcalloc: %p\n", epcm); */
-
-	if (epcm == NULL)
-		return -ENOMEM;
-	epcm->emu = emu;
-	epcm->substream = substream;
 	/*
 	dev_dbg(emu->card->dev, "epcm device=%d, channel_id=%d\n",
 		   substream->pcm->device, channel_id);
 	*/
-	runtime->private_data = epcm;
-	runtime->private_free = snd_p16v_pcm_free_substream;
   
 	runtime->hw = snd_p16v_capture_hw;
 
-	channel->number = channel_id;
-
-	channel->use=1;
-#if 0 /* debug */
-	dev_dbg(emu->card->dev,
-		   "p16v: open channel_id=%d, channel=%p, use=0x%x\n",
-		   channel_id, channel, channel->use);
-	dev_dbg(emu->card->dev, "open:channel_id=%d, chip=%p, channel=%p\n",
-	       channel_id, chip, channel);
-#endif /* debug */
-	/* channel->interrupt = snd_p16v_pcm_channel_interrupt; */
-	channel->epcm = epcm;
 	err = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
 	if (err < 0)
 		return err;
 
 	return 0;
 }
 
 
 /* close callback */
 static int snd_p16v_pcm_close_playback(struct snd_pcm_substream *substream)
 {
-	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
-	//struct snd_pcm_runtime *runtime = substream->runtime;
-	//struct snd_emu10k1_pcm *epcm = runtime->private_data;
-	emu->p16v_voices[substream->pcm->device - emu->p16v_device_offset].use = 0;
-	/* FIXME: maybe zero others */
 	return 0;
 }
 
 /* close callback */
 static int snd_p16v_pcm_close_capture(struct snd_pcm_substream *substream)
 {
-	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
-	//struct snd_pcm_runtime *runtime = substream->runtime;
-	//struct snd_emu10k1_pcm *epcm = runtime->private_data;
-	emu->p16v_capture_voice.use = 0;
-	/* FIXME: maybe zero others */
 	return 0;
 }
 
@@ -409,13 +353,48 @@ static void snd_p16v_intr_disable(struct snd_emu10k1 *emu, unsigned int intrenb)
 	spin_unlock_irqrestore(&emu->emu_lock, flags);
 }
 
+static void snd_p16v_interrupt(struct snd_emu10k1 *emu)
+{
+	unsigned int status;
+
+	while ((status = inl(emu->port + IPR2)) != 0) {
+		u32 mask = INTE2_PLAYBACK_CH_0_LOOP;  /* Full Loop */
+
+		/* dev_dbg(emu->card->dev, "p16v status=0x%x\n", status); */
+		if (status & mask) {
+			struct snd_pcm_substream *substream =
+					emu->pcm_p16v->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
+			struct snd_pcm_runtime *runtime = substream->runtime;
+
+			if (runtime && runtime->private_data) {
+				snd_pcm_period_elapsed(substream);
+			} else {
+				dev_err(emu->card->dev,
+					"p16v: status: 0x%08x, mask=0x%08x\n",
+					status, mask);
+			}
+		}
+		if (status & 0x110000) {
+			struct snd_pcm_substream *substream =
+					emu->pcm_p16v->streams[SNDRV_PCM_STREAM_CAPTURE].substream;
+			struct snd_pcm_runtime *runtime = substream->runtime;
+
+			/* dev_info(emu->card->dev, "capture int found\n"); */
+			if (runtime && runtime->private_data) {
+				/* dev_info(emu->card->dev, "capture period_elapsed\n"); */
+				snd_pcm_period_elapsed(substream);
+			}
+		}
+		outl(status, emu->port + IPR2); /* ack all */
+	}
+}
+
 /* trigger_playback callback */
 static int snd_p16v_pcm_trigger_playback(struct snd_pcm_substream *substream,
 				    int cmd)
 {
 	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
 	struct snd_pcm_runtime *runtime;
-	struct snd_emu10k1_pcm *epcm;
 	int channel;
 	int result = 0;
         struct snd_pcm_substream *s;
@@ -437,10 +416,9 @@ static int snd_p16v_pcm_trigger_playback(struct snd_pcm_substream *substream,
 		    s->stream != SNDRV_PCM_STREAM_PLAYBACK)
 			continue;
 		runtime = s->runtime;
-		epcm = runtime->private_data;
 		channel = substream->pcm->device-emu->p16v_device_offset;
 		/* dev_dbg(emu->card->dev, "p16v channel=%d\n", channel); */
-		epcm->running = running;
+		runtime->private_data = (void *)(ptrdiff_t)running;
 		basic |= (0x1<<channel);
 		inte |= (INTE2_PLAYBACK_CH_0_LOOP<<channel);
                 snd_pcm_trigger_done(s, substream);
@@ -469,22 +447,21 @@ static int snd_p16v_pcm_trigger_capture(struct snd_pcm_substream *substream,
 {
 	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct snd_emu10k1_pcm *epcm = runtime->private_data;
 	int channel = 0;
 	int result = 0;
 	u32 inte = INTE2_CAPTURE_CH_0_LOOP | INTE2_CAPTURE_CH_0_HALF_LOOP;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 		snd_p16v_intr_enable(emu, inte);
 		snd_emu10k1_ptr20_write(emu, BASIC_INTERRUPT, 0, snd_emu10k1_ptr20_read(emu, BASIC_INTERRUPT, 0)|(0x100<<channel));
-		epcm->running = 1;
+		runtime->private_data = (void *)1;
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 		snd_emu10k1_ptr20_write(emu, BASIC_INTERRUPT, 0, snd_emu10k1_ptr20_read(emu, BASIC_INTERRUPT, 0) & ~(0x100<<channel));
 		snd_p16v_intr_disable(emu, inte);
 		//snd_emu10k1_ptr20_write(emu, EXTENDED_INT_MASK, 0, snd_emu10k1_ptr20_read(emu, EXTENDED_INT_MASK, 0) & ~(0x110000<<channel));
-		epcm->running = 0;
+		runtime->private_data = NULL;
 		break;
 	default:
 		result = -EINVAL;
@@ -499,10 +476,10 @@ snd_p16v_pcm_pointer_playback(struct snd_pcm_substream *substream)
 {
 	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct snd_emu10k1_pcm *epcm = runtime->private_data;
 	snd_pcm_uframes_t ptr, ptr1, ptr2,ptr3,ptr4 = 0;
 	int channel = substream->pcm->device - emu->p16v_device_offset;
-	if (!epcm->running)
+
+	if (!runtime->private_data)
 		return 0;
 
 	ptr3 = snd_emu10k1_ptr20_read(emu, PLAYBACK_LIST_PTR, channel);
@@ -524,11 +501,10 @@ snd_p16v_pcm_pointer_capture(struct snd_pcm_substream *substream)
 {
 	struct snd_emu10k1 *emu = snd_pcm_substream_chip(substream);
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct snd_emu10k1_pcm *epcm = runtime->private_data;
 	snd_pcm_uframes_t ptr, ptr1, ptr2 = 0;
 	int channel = 0;
 
-	if (!epcm->running)
+	if (!runtime->private_data)
 		return 0;
 
 	ptr1 = snd_emu10k1_ptr20_read(emu, CAPTURE_POINTER, channel);
@@ -589,6 +565,7 @@ int snd_p16v_pcm(struct snd_emu10k1 *emu, int device)
 	pcm->dev_subclass = SNDRV_PCM_SUBCLASS_GENERIC_MIX;
 	strcpy(pcm->name, "p16v");
 	emu->pcm_p16v = pcm;
+	emu->p16v_interrupt = snd_p16v_interrupt;
 
 	for(substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream; 
 	    substream; 
-- 
2.40.0.152.g15d061e6df

Re: [PATCH 1/7] ALSA: emu10k1: drop redundant snd_emu10k1_efx_playback_hw_free()

[Takashi Iwai]

On Fri, 21 Apr 2023 16:10:00 +0200,
Oswald Buddenhagen wrote:
> 
> Or actually, replace snd_emu10k1_playback_hw_free() with it, as that is
> a subset.
> 
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

The code changes in the whole patch series look OK, but still a few
minor corrections in the change logs (missing commit summary, etc) are
needed.

Also, when you submit this size of patch series, please make a cover
letter and explain what the series are intended.


thanks,

Takashi

Re: [PATCH 1/7] ALSA: emu10k1: drop redundant snd_emu10k1_efx_playback_hw_free()

[Oswald Buddenhagen]

On Fri, Apr 21, 2023 at 04:56:52PM +0200, Takashi Iwai wrote:
>Also, when you submit this size of patch series, please make a cover
>letter and explain what the series are intended.
>
i've actually pondered that and concluded that it would be kinda 
pointless, because the theme "cleanups" is rather obvious.

for the other series it would be "fpga access fixes & cleanups", which 
also seemed obvious enough.

anyway, i'll lower the "threshold of noteworthiness" for the future.

regards

Re: [PATCH 1/7] ALSA: emu10k1: drop redundant snd_emu10k1_efx_playback_hw_free()

[Takashi Iwai]

On Fri, 21 Apr 2023 16:56:52 +0200,
Takashi Iwai wrote:
> 
> On Fri, 21 Apr 2023 16:10:00 +0200,
> Oswald Buddenhagen wrote:
> > 
> > Or actually, replace snd_emu10k1_playback_hw_free() with it, as that is
> > a subset.
> > 
> > Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> 
> The code changes in the whole patch series look OK, but still a few
> minor corrections in the change logs (missing commit summary, etc) are
> needed.
> 
> Also, when you submit this size of patch series, please make a cover
> letter and explain what the series are intended.

I applied applicable ones in the series (patches 1, 2, 3, 5, 7) now.
Please resubmit patches 4 and 6.


thanks,

Takashi