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

msx/msx.cpp: Small MSX2+ related updates #12239

Merged
merged 4 commits into from
Apr 13, 2024
Merged

Conversation

wilbertpol
Copy link
Contributor

  • Move MSX2+ machines to msx/msx2p.cpp.
  • Add support for Kanji level 2 I/O ports.
  • Add support for MSX2+ boot flags register.
  • Hook up msx2p_cart and msxr_cart software lists.

- Move MSX2+ machines to msx/msx2p.cpp.
- Add support Kanji level 2 I/O ports.
- Add support for MSX2+ boot flags register.
- Hook up msx2p_cart and msxr_cart software lists.
Comment on lines 22 to 34
void set_ym2413_tag(const char *tag) { m_ym2413_tag = tag; }
template <class ObjectClass, bool Required>
void set_ym2413(const device_finder<ObjectClass, Required> &finder) {
m_ym2413_base = &finder.finder_target().first;
m_ym2413_tag = finder.finder_target().second;
}

protected:
virtual void device_start() override;

private:
device_t *m_ym2413_base;
ym2413_device *m_ym2413;
const char *m_ym2413_tag;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason it can’t just use a required_device<ym2413_device> rather than this hackery? It looks like leftovers from when someone didn’t understand object finders.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using required_device does indeed make it a lot simpler.

Comment on lines 458 to 474
u8 msx_state::kanji2_r(offs_t offset)
{
u8 result = 0xff;

if (m_region_kanji && m_region_kanji->bytes() > 0x20000)
{
// TODO: Are there one or two latches in a system?
u32 latch = m_kanji_fsa1fx ? bitswap<17>(m_kanji_latch, 4, 3, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 2, 1, 0) : m_kanji_latch;
result = m_region_kanji->as_u8(0x20000 | latch);

if (!machine().side_effects_disabled())
{
m_kanji_latch = (m_kanji_latch & ~0x1f) | ((m_kanji_latch + 1) & 0x1f);
}
}
return result;
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you turn m_kanji_region into an optional_region_ptr<u8> and index it directly (plus get some more validation) rather than needing as_u8?

Should you take the address_space & parameter and return space.unmap() if the kanji ROM is not present or too small, or is it guaranteed to read high?

Or alternatively, should these handlers only be installed if the kanji ROM is present and large enough (e.g. install them on start after doing the check)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The handlers will now only be installed when a kanji region is present.

, m_ym2413(nullptr)
, m_ym2413_tag(nullptr)
, m_ym2413(*this, "^ym2413")
Copy link
Member

Choose a reason for hiding this comment

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

Please initialise this with m_ym2413(*this, finder_base::DUMMY_TAG) and let the user configure it. Making assumptions about the host system never ends well – we’re trying to eliminate devices that construct object finders with root or parent references in tags.

@cuavas cuavas merged commit 6fa5b13 into mamedev:master Apr 13, 2024
2 of 5 checks passed
@wilbertpol wilbertpol deleted the msx2p branch April 13, 2024 19:55
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

2 participants