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

Add bdm hdd support #1365

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add bdm hdd support #1365

wants to merge 4 commits into from

Conversation

KrahJohlito
Copy link
Member

Pull Request checklist

Note: these are not necessarily requirements

  • I reformatted the code with clang-format
  • I checked to make sure my submission worked
  • I am the author of submission or have permission from the original author
  • Requires update of the PS2SDK or other dependencies
  • Others (please specify below)

Pull Request description

This adds bdm hdd support adapted from @grimdoomer fork. Only additions are if one hdd option is ON the other will be disabled to try avoid conflicts.

@BatRastard
Copy link
Contributor

Got a test binary?

@KrahJohlito
Copy link
Member Author

Got a test binary?

CI builds pending prs if you ever want to try them before merge

https://github.com/ps2homebrew/Open-PS2-Loader/actions/runs/11513539409

src/bdmsupport.c Outdated Show resolved Hide resolved
src/bdmsupport.c Outdated Show resolved Hide resolved
src/bdmsupport.c Outdated
else {
dmaType = 0x40;
#ifdef ATA_UDMA_PLUS
dmaMode = pDeviceData->ataHighestUDMAMode;
Copy link
Member

Choose a reason for hiding this comment

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

Chinese IDE-to-SD adapters are faking this value in adapter specs. If the user still can set UDMA mode manually it is ok, but I am not sure if settings for "classic" HDD (HD Loader) will be taken in place in this case. Also some adapters sets this value to zero, while actually supports UDMA4.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what to do here about that? Any suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

Huh, I am not sure too, maybe read udma mode from HDD apa settings somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

but that's what it's doing already? on device connection it calls the cmd to retrieve the highest udma mode here

pDeviceData->ataHighestUDMAMode = fileXioDevctl("xhdd0:", ATA_DEVCTL_GET_HIGHEST_UDMA_MODE, NULL, 0, NULL, 0);

case ATA_DEVCTL_GET_HIGHEST_UDMA_MODE: {

and store the value in pDeviceData->ataHighestUDMAMode then at launch its just applying it in this line we're quoting.

Or I'm just misunderstanding your latest comment and you mean read a manually defined udma user setting?

Copy link
Member Author

Choose a reason for hiding this comment

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

Huh, I am not sure too, maybe read udma mode from HDD apa settings somehow?

Oh wait apa settings.. I didn’t read this properly.. what about this? If ataHighestUDMAMode > 0 use it, otherwise use the user defined setting from the cfg?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I was talking about user defined value that are per game.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I was talking about user defined value that are per game.

ok now it works very similar to apa hdd, if the automatically resolved udma is <= 0 it will use the user defined value.. we could have it always use the user defined value but then the whole ataHighestUDMAMode is pointless and unused..

so now we check ataHighestUDMAMode is greater than 0 if its not due to a faked value we use the user defined value

src/hddsupport.c Show resolved Hide resolved
@CosmicScale
Copy link

CosmicScale commented Nov 3, 2024

I've been testing the CI build and I've found a couple of bugs.

  1. OPL settings are always saved to the HDD's exFAT partition no matter what. On launch OPL does not read the settings config from the HDD's exFAT partition.
  2. The 'autoLaunchBDMGame' function is now completely broken again.

I tried to compile this PR with the latest build and bdm hdd support doesn't work at all.

I did a little debugging on KrahJohlito's gd2 build trying to find the issue with 'autoLaunchBDMGame'. It gets as far as reading the game's config file from the exFAT partition on the HDD but then just crashed when trying to launch the game.

@KrahJohlito
Copy link
Member Author

KrahJohlito commented Nov 3, 2024

I've been testing the CI build and I've found a couple of bugs.

  1. OPL settings are always saved to the HDD's exFAT partition no matter what. On launch OPL does not read the settings config from the HDD's exFAT partition.
  2. The 'autoLaunchBDMGame' function is now completely broken again.

I tried to compile this PR with the latest build and bdm hdd support doesn't work at all.

I did a little debugging on KrahJohlito's gd2 build trying to find the issue with 'autoLaunchBDMGame'. It gets as far as reading the game's config file from the exFAT partition on the HDD but then just crashed when trying to launch the game.

2 is likely not “completely broken” but broken for hdd as I forgot to add the loading of bdm hdd modules for auto launching, that’s probably all it is

Edit: ok now I'm actually on pc, no its more than that haha i'll try push an update if you can give it a quick test

@CosmicScale
Copy link

Edit: ok now I'm actually on pc, no its more than that haha i'll try push an update if you can give it a quick test

I can test it whenever you're ready :)

@KrahJohlito
Copy link
Member Author

KrahJohlito commented Nov 4, 2024

Edit: ok now I'm actually on pc, no its more than that haha i'll try push an update if you can give it a quick test

I can test it whenever you're ready :)

I think both issues 1 & 2 should work properly now.. tbh the whole cfg saving/loading system needs a rewrite (probably a more static approach like maximus32 has suggested multiple times is the best way to go) but I personally don't have time to it at the moment.

I'll rebase if those problems are solved.

@CosmicScale
Copy link

Edit: ok now I'm actually on pc, no its more than that haha i'll try push an update if you can give it a quick test

I can test it whenever you're ready :)

I think both issues 1 & 2 should work properly now.. tbh the whole cfg saving/loading system needs a rewrite (probably a more static approach like maximus32 has suggested multiple times is the best way to go) but I personally don't have time to it at the moment.

I'll rebase if those problems are solved.

OPL settings now save to memory card and are loaded at launch however, OPL always starts up on the menu instead of the games list.

BDM Auto Launch didn't working on HDD or USB but I did some debugging. opl.c line 1846, change the delay to 10 and it works!

@KrahJohlito
Copy link
Member Author

Edit: ok now I'm actually on pc, no its more than that haha i'll try push an update if you can give it a quick test

I can test it whenever you're ready :)

I think both issues 1 & 2 should work properly now.. tbh the whole cfg saving/loading system needs a rewrite (probably a more static approach like maximus32 has suggested multiple times is the best way to go) but I personally don't have time to it at the moment.
I'll rebase if those problems are solved.

OPL settings now save to memory card and are loaded at launch however, OPL always starts up on the menu instead of the games list.

BDM Auto Launch didn't working on HDD or USB but I did some debugging. opl.c line 1846, change the delay to 10 and it works!

I’m going to drop the cfg changes commit, the whole system is the problem and needs a rewrite.. the cfg entry to know whether or not to load the module is in the cfg.. but how can we read it from the cfg if the module isn’t loaded :-s .. chicken and the egg situation.

ok so we gotta increase the delay now thanks for trying it, I’ll try make it more dynamic so it’s not just a set x delay, we’ll just delay as long as we need to until the device is found that way it’s not longer than it needs to be.

@KrahJohlito
Copy link
Member Author

@CosmicScale I've just set a delay loop if you're free to test the auto launching again?

The cfg issues will need to be addressed in a separate pull request imo as a rewrite to the system will require some big changes.

@CosmicScale
Copy link

@CosmicScale I've just set a delay loop if you're free to test the auto launching again?

The new loop with delay(2) fixed loading from HDD but USB was still broken. I experimented with delay lengths, the shortest delay that reliably worked with both was 6.

@KrahJohlito
Copy link
Member Author

force pushed

@Mooninites11
Copy link

I could not get this to run i just get a black screen

@CosmicScale
Copy link

I could not get this to run i just get a black screen

Working perfectly for me. Could be the way you formatted your drive. I assume, just like Grimdoomer's build, the HDD must have a sector size of 512. I remember people have issues when using certain USB enclosures. It's best to connect the HDD directly to your PC then format it.

@Mooninites11
Copy link

Mooninites11 commented Nov 8, 2024

I could not get this to run i just get a black screen

Working perfectly for me. Could be the way you formatted your drive. I assume, just like Grimdoomer's build, the HDD must have a sector size of 512. I remember people have issues when using certain USB enclosures. It's best to connect the HDD directly to your PC then format it.

i was talking about opl not even launching its in a boot loop and if unplug the hard drive opl works i just reformatted me drive and made sure it was 512 and its still doing it

@CosmicScale
Copy link

i was talking about opl not even launching its in a boot loop and if unplug the hard drive opl works i just reformatted me drive and made sure it was 512 and its still doing it

Try deleting your OPL config files.

@KrahJohlito
Copy link
Member Author

KrahJohlito commented Nov 8, 2024

I could not get this to run i just get a black screen

likely due to this #1380 which imo is from a ps2sdk update, if you tried the latest version of this build; I rebased which used a newer sdk to compile.. try this one before that sdk update and see if the issue persists https://github.com/ps2homebrew/Open-PS2-Loader/actions/runs/11694352998 .. after the recent sdk update I get random spam messages in pcsx2 logs Unknown Device 'host' .. something seems to be causing issues

@Mooninites11
Copy link

I could not get this to run i just get a black screen

likely due to this #1380 which imo is from a ps2sdk update, if you tried the latest version of this build; I rebased which used a newer sdk to compile.. try this one before that sdk update and see if the issue persists https://github.com/ps2homebrew/Open-PS2-Loader/actions/runs/11694352998 .. after the recent sdk update I get random spam messages in pcsx2 logs Unknown Device 'host' .. something seems to be causing issues

Yep its working now

@mystyq
Copy link

mystyq commented Nov 10, 2024

Happy to help test this. 8TB HDD exFAT. Grimdoomer's build works.

The current PR build switches to GPT and then is quite laggy. Loader icon just keeps spinning and inputs are don't register unless on a specific frame

@362053534
Copy link

362053534 commented Nov 11, 2024

I could not get this to run i just get a black screen

likely due to this #1380 which imo is from a ps2sdk update, if you tried the latest version of this build; I rebased which used a newer sdk to compile.. try this one before that sdk update and see if the issue persists https://github.com/ps2homebrew/Open-PS2-Loader/actions/runs/11694352998 .. after the recent sdk update I get random spam messages in pcsx2 logs Unknown Device 'host' .. something seems to be causing issues

Hello! I tested this build too. And I found that loading the HDD game list is too slow,but Grimdoomer's build works good.
I don't know why ,I guess maybe hdd or bdm cache doesn't work.

@KrahJohlito
Copy link
Member Author

KrahJohlito commented Nov 13, 2024

I could not get this to run i just get a black screen

likely due to this #1380 which imo is from a ps2sdk update, if you tried the latest version of this build; I rebased which used a newer sdk to compile.. try this one before that sdk update and see if the issue persists https://github.com/ps2homebrew/Open-PS2-Loader/actions/runs/11694352998 .. after the recent sdk update I get random spam messages in pcsx2 logs Unknown Device 'host' .. something seems to be causing issues

Hello! I tested this build too. And I found that loading the HDD game list is too slow,but Grimdoomer's build works good. I don't know why ,I guess maybe hdd or bdm cache doesn't work.

Happy to help test this. 8TB HDD exFAT. Grimdoomer's build works.

The current PR build switches to GPT and then is quite laggy. Loader icon just keeps spinning and inputs are don't register unless on a specific frame

Hi guys, unfortunately these differences appear to be the result of a newer build environment because the code here is almost exactly the same as grimdoomer builds (can’t really do anything about that in this pr, may need to be addressed in the sdk idk).

https://github.com/KrahJohlito/Open-PS2-Loader/actions/runs/11811553194

This version that I’m linking above is the same as this PR build except the build environment used is very similar to OPL revision 2049 rather than “latest” and seems to have much better performance; conclusion this PR is not causing the issues but there can still be improvements made whether it be in the build environment or OPL itself. I do not intended to try rectify any further issues in this PR (with the exception of current pending reviews) as I don’t believe the problem to be here.

Best Regards

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.

7 participants