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

Heuristic to switch problematic chipsets to PIO #70

Merged
merged 2 commits into from
Jan 12, 2024

Conversation

jiyunomegami
Copy link
Contributor

Sorry to bother you again with this, but with #68, everything seems to fallback to PIO.

I think this is from mpxplay:
card->codec_vendor_id = snd_hda_param_read(card, AC_NODE_ROOT,AC_PAR_VENDOR_ID);
if(card->codec_vendor_id <=0)
card->codec_vendor_id = snd_hda_param_read(card, AC_NODE_ROOT,AC_PAR_VENDOR_ID);
The first read always times out!

I tested this with 3 motherboards including the problematic D945GCLF2.
With this patch, only the D945GCLF2 will fallback to PIO.

Regarding the comment below, what devices did you run into that did have immediate commands?
"Immediate Commands are optional, some devices don't have it, use CORB"

@crazii
Copy link
Owner

crazii commented Jan 12, 2024

No problem.
I haven't seen any boards that uses PIO yet, the comments is just based on the spec, as you read about.

@crazii crazii merged commit 54f3cdc into crazii:main Jan 12, 2024
1 check passed
@crazii
Copy link
Owner

crazii commented Jan 12, 2024

@volkertb is it possible to publish the build release only by hand, and the auto-build only performs as check workflows? because there might be 'working in progress' commits that doesn't require a release.

@volkertb
Copy link
Collaborator

volkertb commented Jan 14, 2024

@crazii What if I change it so that it will still always build, but only publish when you push a tag?

So only once you're ready to actually "release a new version", you do the following (assuming for example that you want to release a version numbered 1.4):

git tag -a v1.4 -m "Release version 1.4 of SBEMU"
git push origin v1.4

And then GitHub Actions will not only build as usual, but also package and publish the USB image.

Would that work for you?

@crazii
Copy link
Owner

crazii commented Jan 14, 2024

I think it's ok, are the none release build artifacts still being kept, if yes, for how long? Sorry I'm not familiar with GitHub that much. :)

@volkertb
Copy link
Collaborator

No, once we make a distinction and only publish releases when a tag is pushed, then only the artifacts of those releases will be published and thus preserved. The build logs of the non-release builds will still be kept in history, but no artifacts will be stored from the non-release builds. At least that's how I understand it.

@crazii
Copy link
Owner

crazii commented Jan 15, 2024

I read the spec again, the original CORB/RIRB may not properly initialized, I will change the code but I'm not sure if that will conflict your changes.

I'll add a complete init routine (corb_init) for CORB/RIRB since to make sure the first cmd not always fail. I tested with my laptop now it won't fail, so I removed this code in mixer_init, but you need check it on your PCs to see if it is still the same,
I'm going to add this code back to corb_init, but I'm not sure the problem still exist, I can confirm it while I'm adding it back.
sorry for the inconvenience.

@crazii
Copy link
Owner

crazii commented Jan 15, 2024

I just added your code back to int corb_init function, you can check if the changes work.

@jiyunomegami
Copy link
Contributor Author

jiyunomegami commented Jan 15, 2024

Intel D945GCLF2: OK, it switches to PIO like before.
Asus B75M-PLUS: OK, uses CORB like before.

I found another motherboard that switches to PIO:
Jetway MA3-79GDG COMBO-LF (AMD AM2)
In the BIOS, I had to set SATA mode to Legacy IDE, and set SATA as the primary IDE controller.
I also added the /NOINVLPG option to JEMM386.
Otherwise the system with crash soon after running SBEMU.
Now, with HDA , halfway through the Monkey Island theme, the system crashes with some text from JEMM all over the screen.
CMI lasts longer without crashing, but the music turned into a buzz, and then the screen goes black.
This is a problem with JEMM and/or the motherboard right? Do you know if there is anything else I can try to make it stable?

@crazii
Copy link
Owner

crazii commented Jan 16, 2024

There might be IRQ conflicts, although the SBEMU handles shared IRQ but it might have problems, turn off onboard LAN/USB and other onboard devices.
And also there might be bugs in the code, not easy to fix, but will be improved overtime.

@hjnijlunsing
Copy link

Sometimes enabling onboard devices also helps with IRQ issues. My a0751h did not get an IRQ assigned for the HDA chipset until I enabled the onboard LAN controller: Baron-von-Riedesel/VSBHDA#1

@crazii
Copy link
Owner

crazii commented Jan 16, 2024

SBEMU will auto assign IRQ for ICH, VT82xx, but not for HDA(never seen unassigned for HDA). I can add it too.
but the assignment function is not complete yet, there's something more to be done.

@jiyunomegami
Copy link
Contributor Author

jiyunomegami commented Jan 16, 2024

Removing the / from the Jemm386 option improved things, but it still crashes after a few minutes.
Now I am using JEMM386 NOINVLPG X=TEST
I already tried disabling USB/Lan/FDD etc. but it didn't help.

I am trying to debug this using duke3d. Disabling sound effects, and using MIDI for music, it always crashes in the same place, after a few minutes into the demo.
I get this message on the screen:
Exception 0D in ring 0
client CS:EIP=005B:000002A2 SS:ESP=01B7:000006808

Does anyone know how to find what is at that CS:EIP?
For example, is it possible add some debug messages (over serial) of CS:EIP, like DBG_Log("%s:%d CS:EIP: %x:%x", __FILE__, __LINE__, get_cs(), get_eip());

Is there some way to implement get_cs and get_eip?

BTW, in sc_inthda.c, ... EDIT you already fixed that assertion.

@crazii
Copy link
Owner

crazii commented Jan 16, 2024

assert((((uint32_t)card->corb_buffer)&(HDA_CORB_ALIGN-1)) == 0);

I have them fixed, pushed up 5h ago: f50531d
sorry for the inconvenience.

@crazii
Copy link
Owner

crazii commented Jan 16, 2024

Is there some way to implement get_cs and get_eip?

There's a _my_cs() function in DJGPP, #include <sys/segments.h>
For eip, it is tricky, you many do it like this:

DBG_Log("%s:%d CS:EIP: %x:%x", __FILE__, __LINE__, _my_cs(), (uintptr_t)&&label);
label:

The address of label which means next instruction address after DBG_Log, can be get using &&, but it is GCC only.
but the crashed client CS:EIP looks like a real mode one, that means it might not be the direct cause of the crash.

EDIT: You can wrap up this code to a macro which might be convenient.

@crazii
Copy link
Owner

crazii commented Jan 16, 2024

BTW: there's a offline doc for DJGPP, (functions, structs etc, which might help you finding useful things).
There's also a online version of it, if you perfer.

@crazii
Copy link
Owner

crazii commented Jan 16, 2024

Sometimes enabling onboard devices also helps with IRQ issues. My a0751h did not get an IRQ assigned for the HDA chipset until I enabled the onboard LAN controller: Baron-von-Riedesel/VSBHDA#1

The IRQ assignment is improved (d517785), but still it might not work, I tested on my laptop and it works. you can test it too.

@jiyunomegami
Copy link
Contributor Author

This MA3-79GDG, I reset the BIOS settings to default and disabled HDA. Now Duke 3D and Monkey Island are stable with a CMI PCI card.
Doom crashes on the title screen a second or two after the music starts, if sound FX or adlib are enabled. But works with PC speaker/no sound FX and MIDI.
After quitting doom and uploading a file to mTCP's ftp server, I did get a crash at that same address 005B:000002A2, so I guess it is related to SATA/IDE or maybe the NIC.
-> Now after changing the SATA mode to "Legacy IDE" and setting SATA to primary, I cannot reproduce this.

The HDA is still detected, but snd_ihd_mixer_init fails.
The HDA is detected as ATI RS780 (1002960F) -> ATI (1002791A) (max 0kHz/0bit/8ch)
I think you should add a warning, like this:

 int mixer_ok = 0;
 for(i=0;i<AZX_MAX_CODECS;i++){
  if(card->codec_mask&(1<<i)){
   card->codec_index=i;
   if(snd_ihd_mixer_init(card)){
    mixer_ok = 1;
    break;
   }
  }
 }
 if(!mixer_ok){
  printf("Intel HDA: WARNING: Mixer init failed\n");
 }

I think it is better to not disable HDA in this case, since otherwise SBEMU will exit with a "No supported soundcard found!" message.
With just a warning, you can disable HDA in the BIOS and still use MIDI.
Doom works with the sc_inthd driver loaded with sound FX/adlib disabled but MIDI enabled.

Related: is there a "dummy" soundcard driver, that will not try to use any sound card?

@crazii
Copy link
Owner

crazii commented Jan 17, 2024

Related: is there a "dummy" soundcard driver, that will not try to use any sound card?

There was a sc_null.c that was recently deleted.

I think it is better to not disable HDA in this case, since otherwise SBEMU will exit with a "No supported soundcard found!" message.

With an addition condition that P330 is enabled, that will be better?

Can you create the PR for that? Or I just do it?

@hjnijlunsing
Copy link

hjnijlunsing commented Jan 17, 2024

Sometimes enabling onboard devices also helps with IRQ issues. My a0751h did not get an IRQ assigned for the HDA chipset until I enabled the onboard LAN controller: Baron-von-Riedesel/VSBHDA#1

The IRQ assignment is improved (d517785), but still it might not work, I tested on my laptop and it works. you can test it too.

When I now disable network boot in BIOS (thus not getting an IRQ assigned);
SBEMU crashes with: "Invalid Soundcard IRQ: 255, Trying to assign a valid IRQ..."

Still with current SBEMU build; my intel HDA; Quake works; but Doom crashes.

@crazii
Copy link
Owner

crazii commented Jan 17, 2024

SBEMU crashes with: "Invalid Soundcard IRQ: 255, Trying to assign a valid IRQ..."

There's some bugs causing the crash, thanks for testing, I'll try to fix it.

@hjnijlunsing
Copy link

UserBuild_2024.01.17_13-59
[IRQ assignment: bugfix & readability improvement.]

Same issue ;-)

@crazii
Copy link
Owner

crazii commented Jan 17, 2024

SBEMU crashes with: "Invalid Soundcard IRQ: 255, Trying to assign a valid IRQ..."

I updated some fix, hope it will work this time.

@hjnijlunsing
Copy link

I already tried the release before your comment as I noticed from the buildbot a new version was available.

UserBuild_2024.01.17_13-59
[IRQ assignment: bugfix & readability improvement.]

Unfortunately same issue

@crazii
Copy link
Owner

crazii commented Jan 17, 2024

I already tried the release before your comment as I noticed from the buildbot a new version was available.

UserBuild_2024.01.17_13-59 [IRQ assignment: bugfix & readability improvement.]

Unfortunately same issue

I've tried to fix it again, you can test it if it works. if not, you can make a debug build by make DEBUG=1 and then run it to check the logs on screen, I have no clue why it crashes, it might freeze after return but not crash on the call.

@crazii
Copy link
Owner

crazii commented Jan 18, 2024

Still with current SBEMU build; my intel HDA; Quake works; but Doom crashes.

I fixed a interrupt issue for HDA, but not sure it's related, I can try it, thanks.

@crazii
Copy link
Owner

crazii commented Jan 18, 2024

There might be some unhandled corner cases, but it's hard find them, hope we'll fix them by time.

@hjnijlunsing
Copy link

Meanwhile I read the spec and found it requires 1K stack space, but it's possible that HDPMI doesn't preserve enough. the readme says at least 200h bytes. I'll allocate 1K stack for the call. But I'm not able to test it my self.

Still same issue

@crazii
Copy link
Owner

crazii commented Jan 18, 2024

Weird. At least I will get an error on my laptop for this call, or in virtualbox (SET_FAILED on may laptop - there's routing conflicts, FUNC_NOT_SUPPORTED in virtualbox), but no freeze.
Maybe I can try to use 32bit protected mode PCI BIOS call, but still not quite promising.

@jiyunomegami
Copy link
Contributor Author

With an addition condition that P330 is enabled, that will be better?

Can you create the PR for that? Or I just do it?

If there is a selectable null driver, I think it is better to return an error in this case. I can make a PR later.
I want to add back the null driver, for use with cards that only have FM or MPU401. I am making a Yamaha YMF7xx (YMF744) driver, but it only supports FM/MPU401 now.

BTW, pcibios_enable_interrupt is implemented wrong, it only reads and sets one byte.
I don't know if it has anything to do with hjnijlunsing's problem.
This should be correct:
void pcibios_enable_interrupt(pci_config_s* ppkey)
{
unsigned int cmd;
cmd=pcibios_ReadConfig_Word(ppkey, PCIR_PCICMD);
cmd &= ~(1<<10);
pcibios_WriteConfig_Word(ppkey, PCIR_PCICMD, cmd);
}

@jiyunomegami
Copy link
Contributor Author

jiyunomegami commented Jan 18, 2024

pcibios_enable_memmap_set_master etc. are also wrong, I will make a PR

@crazii
Copy link
Owner

crazii commented Jan 18, 2024

I don't know if it has anything to do with hjnijlunsing's problem.

It's unrelated to that. Yes it should be a word write, I just copied the code above not realizing that it should be word.

@crazii
Copy link
Owner

crazii commented Jan 18, 2024

pcibios_enable_memmap_set_master etc. are also wrong, I will make a PR

That code is OK. since the function doesn't alter the higher byte.

@crazii
Copy link
Owner

crazii commented Jan 19, 2024

I want to add back the null driver, for use with cards that only have FM or MPU401. I am making a Yamaha YMF7xx (YMF744) driver, but it only supports FM/MPU401 now.

Doesn't the YMF7x4 works with its own driver? Does the SetupDS/DSDMA require motherboard support? I kinda forget the details.

@crazii
Copy link
Owner

crazii commented Jan 19, 2024

Meanwhile I read the spec and found it requires 1K stack space, but it's possible that HDPMI doesn't preserve enough. the readme says at least 200h bytes. I'll allocate 1K stack for the call. But I'm not able to test it my self.

Still same issue

I've added 32 bit protected call to PCI BIOS, you can check it again. If it still freezes, then it'll need to program the interrupt router, but that's more complicated. I think we need more tests for more PCs, if it works for other PCs, then just leave it.

It works on my laptop with a different code path, since my laptop has conflict IRQ routing options: the sound card options is IRQ 5, another device uses IRQ 10, but they are wired together with the same interrupt link. So PCI_SET_INTERRUPT will naturally fail. But weird that the sound card can works with IRQ 5/10 only changing the PCI config register INT_LINE. I didn't know how the bios setup the interrupt router for it.
The ECHI controller is the same for the same laptop, it has routing conflicts, but the ECHI only works with IRQ 5.

@hjnijlunsing
Copy link

A new crash ;-)
image

But I agree with you; there are issues with higher priority especially if my netbook is the only one. (N=1)
And my netbook has a workaround as well (enabling network boot).

Items I could still help with testing:

  • Doom to work on my A0751h
  • Ali M5455 AC97 my HPT5710 Thin Client
  • Corrupted sound on VIA VT8233 VIA EPIA-M
  • Getting sound to work on CMI8738-PCI-SX

@hjnijlunsing
Copy link

After latest commit:

image

@crazii
Copy link
Owner

crazii commented Jan 19, 2024

Still freezes using PM mode call. That's really frustrating.. :) Just leave it be for now.

@crazii
Copy link
Owner

crazii commented Jan 20, 2024

  • Getting sound to work on CMI8738-PCI-SX

The bug on CMI8738-PCI-SX is fixed, you can try it out. It should also fix for the CMI8338 cards.

@hjnijlunsing
Copy link

hjnijlunsing commented Jan 20, 2024

CMI8738-PCI-SX crashes around 4 seconds after running SBEMU.EXE with a random character on the top-left.
Checked the other CMI (6ch), it was still working in Doom ;-)

image

@crazii
Copy link
Owner

crazii commented Jan 20, 2024

Weird, I didn't change anything important. I'll check the code again.

@hjnijlunsing
Copy link

I also compared the debug output with the working (6ch) card vs the non-working card (PCI-SX).
Both output is identical, including the IRQ

@crazii
Copy link
Owner

crazii commented Jan 20, 2024

I previously removed the code resetting ADC, added it back, if it still doesn't work, I can revert codes line by line to help identifying which line causing the problem. Or I can revert them all except the mixer fix, but without knowing why this happens.

@hjnijlunsing
Copy link

hjnijlunsing commented Jan 20, 2024

userbuild 13-51 still crashes with same behaviour
userbuild 14-05: Same issue

@crazii
Copy link
Owner

crazii commented Jan 20, 2024

can you test the build 2024.01.15_12-46 to confirm whether it crashes? I doubt the commit on 12-46 might have problems.

@hjnijlunsing
Copy link

hjnijlunsing commented Jan 20, 2024

https://github.com/crazii/SBEMU/releases/tag/UserBuild_2024.01.15_12-46
--> No Crash, but also no sound for the PCI-SX (same behavior as last week)

https://github.com/crazii/SBEMU/releases/tag/UserBuild_2024.01.19_13-23
--> As well, no crash

@crazii
Copy link
Owner

crazii commented Jan 20, 2024

Hell no, then I'll continue reverting codes.
What about 2024.01.20_14-30?
It might be the mixer code that causing the problem, but it shouldn't.

@hjnijlunsing
Copy link

2024.01.20_14-30 --> Crash

@crazii
Copy link
Owner

crazii commented Jan 20, 2024

Then I think the problem might be the version number detected incorrectly, I reverted some code and also copy the version detection code from Linux kernel.

If it doesn't crash this time, then it behaves as the original code, and won't have sound either.

@hjnijlunsing
Copy link

@crazii
Copy link
Owner

crazii commented Jan 20, 2024

I commented out the mixer code, we're back to the original point.
But I read it in the data sheet: https://html.alldatasheet.com/html-pdf/181665/CMEDIA/CMI-8738/3785/17/CMI-8738.html
that it have no SB16 mixers, and it says Please do not write any values into reserved registers, but I don't know why it crashes on accessing SBPro mixers. It should only have problem accessing SB16 mixers because they are reserved.

@hjnijlunsing
Copy link

hjnijlunsing commented Jan 20, 2024

https://github.com/crazii/SBEMU/releases/tag/UserBuild_2024.01.20_15-08 --> Confirmed No Crash. No Sound (Same behavior as last week) :)

@crazii
Copy link
Owner

crazii commented Jan 20, 2024

I tried disable SB16 mode, hope it doesn't crash this time.

@hjnijlunsing
Copy link

https://github.com/crazii/SBEMU/releases/tag/UserBuild_2024.01.20_15-36 --> Same behavior as 15-08;
Thus indeed no crash; but also no change.

@crazii
Copy link
Owner

crazii commented Jan 20, 2024

OK at least it doesn't crash this time, I get why the crash happens but the datasheet is so unclear and misleading.
I'll turn to the Linux source code and use the datasheet as guides.

@crazii
Copy link
Owner

crazii commented Jan 20, 2024

There was a bug in interrupt function and I fixed it.
May be we need to open another issue to stop disturbing others. :)

EDIT: maybe this one #55 fits better.

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

4 participants