Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AC97: Add support for SIS7012 (Fixes #27) #29

Merged
merged 1 commit into from
Nov 18, 2023
Merged

Conversation

thp
Copy link
Collaborator

@thp thp commented Sep 30, 2023

This is based on information from the Linux and FreeBSD AC97/ICH sound drivers, with some local testing and trial'n'error to recover from DMA underruns.

This is based on information from the Linux and FreeBSD
AC97/ICH sound drivers, with some local testing and
trial'n'error to recover from DMA underruns.
@thp thp changed the title AC97: Add support for SIS7012 AC97: Add support for SIS7012 (Fixes #27) Sep 30, 2023
@thp thp mentioned this pull request Sep 30, 2023
Comment on lines +74 to +77
#define ICH_DMABUF_MAX_PERIODS 32 // number of entries in the Buffer Descriptor List
#define ICH_DMABUF_PERIODS 4 // number of "used" entries in the Buffer Descriptor List
#define ICH_BDL_ENTRY_SIZE (2 * sizeof(uint32_t)) // size of one entry in the Buffer Descriptor List
#define ICH_DMABUF_ALIGN (ICH_DMABUF_MAX_PERIODS*ICH_BDL_ENTRY_SIZE) // 256
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of unrelated to SIS7012 support, but during reading of the AC97 docs on OSDev, I found that some of these things look mislabeled (it works because 2 * sizeof(uint32_t) is the same as 2 * 4, even though the "2" doesn't have anything to do with channels. Those values are used for the Buffer Descriptor List.

Comment on lines +187 to +198
size_t buffer_descriptor_list_size = ICH_DMABUF_MAX_PERIODS*ICH_BDL_ENTRY_SIZE;

// Allocate Buffer Descriptor List + PCM output buffer in a single allocation
card->dm = MDma_alloc_cardmem(buffer_descriptor_list_size + card->pcmout_bufsize);

// buffer descriptor list requires ICH_BDL_ENTRY_SIZE alignment,
// but dos-allocmem gives 16 byte align (so we don't need alignment correction)
card->virtualpagetable = (uint32_t *)card->dm->linearptr;
card->pcmout_buffer = card->dm->linearptr + buffer_descriptor_list_size;

// DMA buffer written by MDma_writedata() and MDma_clearbuf()
aui->card_DMABUFF = card->pcmout_buffer;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just like above, this is unrelated to SIS7012, but documents this part a little better.

@@ -322,7 +351,6 @@ static void snd_intel_prepare_playback(struct intel_card_s *card,struct mpxplay_
snd_intel_write_32(card,ICH_PO_BDBAR_REG,(uint32_t)pds_cardmem_physicalptr(card->dm,table_base));

snd_intel_write_8(card,ICH_PO_LVI_REG,(ICH_DMABUF_PERIODS-1)); // set last index
snd_intel_write_8(card,ICH_PO_CIV_REG,0); // reset current index
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CIV register is read-only in AC97, so I removed writes to it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I saw this too, but I didn't remove it, thought it wouldn't hurt. And yes remove it also wouldn't hurt.

@@ -598,23 +616,22 @@ static long INTELICH_getbufpos(struct mpxplay_audioout_info_s *aui)
if(retry>1)
continue;
MDma_clearbuf(aui);
snd_intel_write_8(card,ICH_PO_LVI_REG,(ICH_DMABUF_PERIODS-1));
snd_intel_write_8(card,ICH_PO_CIV_REG,0);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CIV register is read-only in AC97, so I removed writes to it.

@sezero
Copy link

sezero commented Oct 2, 2023

Is this tested in mpxplay itself? (I guess a backport would be needed for it.)

@thp
Copy link
Collaborator Author

thp commented Oct 3, 2023

Is this tested in mpxplay itself? (I guess a backport would be needed for it.)

Haven't backported it or tested it in MPXPlay. Should be easy to backport, although some #ifdef SBEMU lines make it a bit more difficult (just a little bit more effort, not impossible). Would be great to get some feedback from other users using SIS7012 if it actually works on their machines first :)

@crazii crazii merged commit 960d02f into crazii:main Nov 18, 2023
@thp thp deleted the sis7012-only branch November 19, 2023 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants