Juku E5101/E5104 into working machine status #9946

boamaod · 2022-06-18T02:12:38Z

Added:

Sound (three volume levels)
Graphics modes 384x200, 400x192 (and 256x192)
Text modes 64x20, 80x24
EKDOS 2.29 and 2.30 hashes

Fixed:

Floppy side selection
Broken floppy write routine
Partially mapped memory mode 0
Some descriptions

…ture.

cuavas · 2022-06-28T07:46:53Z

src/mame/juku/juku.cpp

 #include "formats/juku_dsk.h"
 #include "softlist_dev.h"
 #include "screen.h"
+#include "sound/spkrdev.h"
+#include "speaker.h"

+#include "osdcore.h" // osd_printf_*


Please keep headers organised and ordered from most to least dependent. In this case, it should be:

emu.h first, because it’s the prefix header, and hence special.

The headers from src/devices sorted alphabetically.

The headers from src/emu sorted alphabetically.

The headers from src/formats sorted alphabetically.

You don’t need to explicitly include osdcore.h here – it’s guaranteed to come via emu.h

cuavas · 2022-06-28T07:49:14Z

src/mame/juku/juku.cpp

+	static constexpr int screen_320_240_seq[5] = { 0x24, 0x8, 0x72, 0x0, 0x25 };
+	static constexpr int screen_384_200_seq[5] = { 0x16, 0x4, 0x12, 0x1, 0x45 };
+	static constexpr int screen_400_192_seq[5] = { 0x14, 0x3, 0x1a, 0x1, 0x45 };
+	static constexpr int screen_256_192_seq[5] = { 0x32, 0x12, 0x20, 0x1, 0x49 };
+	static constexpr const int *screen_mode_sequences[4] = { screen_320_240_seq, screen_384_200_seq, screen_400_192_seq, screen_256_192_seq };


Why not just use a static contexpr int foo[4][5] = ...; for this and avoid the extra relocations? Also, things that are like constants conventionally use SCREAMING_SNAKE_CASE for their names in MAME.

cuavas · 2022-06-28T07:50:24Z

src/mame/juku/juku.cpp

+	bool m_beep_state;
+	DECLARE_WRITE_LINE_MEMBER(speaker_w);
+	void update_speaker();
+	static constexpr double speaker_levels[3] = { 0, 0.67, 1 };


Please don’t intermix data members and member functions like this. Keep the member variables together, and keep the static data members together as well.

cuavas · 2022-06-28T07:54:36Z

src/mame/juku/juku.cpp

+	m_screen_cur_seq[pos] = (int)data;
+
+	for (int i=0; i<3; i++)
+		if (memcmp(screen_mode_sequences[i], m_screen_cur_seq, pos*sizeof(int)) == 0) {


Please use std::equal from <algorithm> rather than memcmp.

cuavas · 2022-06-28T07:56:05Z

src/mame/juku/juku.cpp

+	for (int i=0; i<3; i++)
+		if (memcmp(screen_mode_sequences[i], m_screen_cur_seq, pos*sizeof(int)) == 0) {
+			if (pos == 4)
+				switch (i) {


This code uses K&R brace positions while other code in the file uses Allman style. Please pick a style and stick with it.

cuavas · 2022-06-28T08:01:41Z

src/mame/juku/juku.cpp

+void juku_state::screen_mode(int x, int y)
+{
+	if (m_screen_x == x && m_screen_y == y)
+		return;
+
+	m_screen_x = x, m_screen_y = y;
+	rectangle v = m_screen->visible_area();
+	v.max_x = x - 1;
+	v.max_y = y - 1;
+
+	m_screen->configure(x, y, v, m_screen->frame_period().attoseconds());
+}


This doesn’t really seem that plausible. Maintaining the same frame period for any combination of H/V counts isn’t something that’s achievable. How is the pixel clock obtained? Also, what kind of screen is this? Unless it’s an LCD, it needs at least some vertical blanking lines – most CRT monitors require three lines of front porch, three lines of vertical sync, and at least a dozen lines of back porch.

cuavas · 2022-06-28T08:02:28Z

src/mame/juku/juku.cpp

-	for (int y = 0; y < 240; y++)
-		for (int x = 0; x < 320; x++)
-			bitmap.pix(y, x) = BIT(m_ram[0xd800 + (y * (320 / 8) + x / 8)], 7 - (x % 8)) ? rgb_t::white() : rgb_t::black();
+	for (int y = 0; y < m_screen_y; y++)
+	{
+		uint32_t *dest = &bitmap.pix(y);
+		for (int x = 0; x < m_screen_x; x++)
+			*dest++ = BIT(m_ram[0xd800 + (y * (m_screen_x / 8) + x / 8)], 7 - (x % 8)) ? rgb_t::white() : rgb_t::black();
+	}


You should restrict drawing to the clipping rectangle, at least vertically.

cuavas · 2022-06-28T08:04:36Z

src/mame/juku/juku.cpp

-			logerror("pio0: c%d = %d\n", i, BIT(data, i));
+			osd_printf_verbose("pio0: c%d = %d\n", i, BIT(data, i));


Using logerror or the LOG macro from logmacro.h is preferred for things like this. You should be able to find plenty of examples to code that’s been migrated to use logmacro.h for debug logging.

cuavas · 2022-06-28T08:05:30Z

src/mame/juku/juku.cpp

-	m_prev_portc = data;
+	if (m_prev_portc != data)
+		m_prev_portc = data;


The test here doesn’t really provide any value – you want m_prev_portc to have the value of data at the end.

startaq · 2022-06-28T14:31:02Z

hash/juku.xml

-		<year>198?</year>
-		<publisher>&lt;unknown&gt;</publisher>
+		<year>1979</year>
+		<publisher>Digital Research</publisher>


Digital Research is the original publisher - but I don't think they published the Juku specific version. Same issue with the year of release.

I think this generic CP/M 2.2 disk was created and published in 2018 at Башкирия-2М/Универсальный эмулятор file section and some online forums as means to test Juku emulation (also the description says original disk is still searched for). Besides disk size, it is not specific to Juku system, using this CP/M release you cannot even fully read Juku disks, which have different sector translation system. I think keeping publisher "unknown" (it's not an official publication) and modifying year to 2018 might be an option. And adding few words on comment field.

startaq · 2022-06-28T14:32:09Z

hash/juku.xml

+		<info name="usage" value="Press t d d" />
+		<info name="release" value="19880501" /><!-- Based on ИTCРВ E5104 руководство по эксплуатации (Таллинн 1988). -->
+		<info name="version" value="2.29" />
+		<info name="programmer" value="Andres Kaukver"/>


Usually we name this field "author".

startaq · 2022-06-28T14:34:36Z

src/mame/juku/juku.cpp

    - КР580ВИ53 x3
    - КР580ВК38
    - КР580ВН59
    - КР580ВВ51A x2
    - КР580ВВ55A x2
+    - КР1818ВГ93 (on all E5104 production models)


Would it make sense to create two drivers, e5101 and e5104, with only the latter including the floppy drive emulation (I assume the 5101 uses a tape)?

I think, yes, however currently there might be not enough information or authentic ROM dumps/DOS disks to differentiate between them. This means the current working machine will be E5104 and will default for test runs for most of people, which seems fine. In fact, even E5102 is mentioned in some documents and the numbering seems similar to CM-1800, which was a computer EKTA worked on previously (BIOS and maybe DOS).

startaq · 2022-06-28T14:38:47Z

src/mame/juku/juku.cpp

@@ -132,17 +164,20 @@ void juku_state::bank_map(address_map &map)

 void juku_state::io_map(address_map &map)
 {
-	map(0x00, 0x01).rw(m_pic, FUNC(pic8259_device::read), FUNC(pic8259_device::write));
+	map(0x00, 0x03).rw(m_pic, FUNC(pic8259_device::read), FUNC(pic8259_device::write));


The 8259 only has one address line (A0), did you mean to mirror the accesses? In that case use mirror(0x02).

startaq · 2022-06-28T14:40:53Z

src/mame/juku/juku.cpp

+	map(0x11,0x11).w(FUNC(juku_state::screen_cmd_a_w));
+	map(0x12,0x12).w(FUNC(juku_state::screen_data_a_w));
+	map(0x15,0x15).w(FUNC(juku_state::screen_cmd_b_w));
+	map(0x16,0x16).w(FUNC(juku_state::screen_data_b_w));


This overwrites the PIT devices. At least a comment why this is done is needed here.

startaq · 2022-06-28T14:43:05Z

src/mame/juku/juku.cpp

+		update_speaker();
+
+	if (m_prev_porta != data)
+		m_prev_porta = data;


The check isn't needed, there is no harm in assigning the same value twice.

startaq · 2022-06-28T14:46:20Z

src/mame/juku/juku.cpp

+	SCREEN(config, m_screen, SCREEN_TYPE_RASTER);
+	m_screen->set_refresh_hz(60);
+	m_screen->set_vblank_time(ATTOSECONDS_IN_USEC(2500)); // not accurate
+	m_screen_x = 320, m_screen_y = 240;


Don't assign variables here - maybe set it at machine_reset time and just use fixed values here.

startaq · 2022-06-28T14:47:14Z

src/mame/juku/juku.cpp

-	KR1818VG93(config, m_fdc, 1000000);
-//  m_fdc->intrq_wr_callback().set(FUNC(juku_state::fdc_intrq_w));
+	// КР1818ВГ93
+	KR1818VG93(config, m_fdc, 1_MHz_XTAL);


Is there really a 1 MHz XTAL or is the value divided down from another value?

startaq · 2022-06-28T14:48:57Z

src/mame/mess.flt

@@ -401,6 +401,7 @@ interton/vc4000.cpp
 intv/intv.cpp
 isc/compucolor.cpp
 jazz/jazz.cpp
+juku/juku.cpp


Should that be ekta/juku.cpp instead? The folders are mostly created by company name.

Yes, this seems reasonable, longer name of company/institute is Arvutustehnika Erikonstrueerimisbüroo, it's related to Estonian Academy of Sciences and EKTA is its trademark from 1980s. It's not completely clear to what extent there were companies in USSR and sometimes research and production had really interesting ways, but I think from computer architecture and reuse of technology point of view putting it under EKTA makes sense (also compared to other institutes in Estonia).

FYI there's a folder for USSR MAME drivers here: https://github.com/mamedev/mame/tree/master/src/mame/cccp

Thanks, @happppp, I did notice, but I think it might be wise to consider it as a temporary category and still opt for separating mature drivers under institutes, product lines or schools of computing (BTW, it seems Специалист currently has escaped the category for some reason). For Juku the case is even more mixed because most kids using this computer at schools in the beginning of 1990s were already doing it in free Estonia and they would consider putting it under USSR label rather insulting. Also the engineers working to localise system for Estonian schools and language were building it precisely because political situation made it possible in the second half of 1980s and this is what individuates/motivates the Juku series. But I think it would be wrong also for more technical reasons to view computer production in USSR as a single category.

@boamaod thanks for the heads up, specialist is now moved to proper location as well

startaq · 2022-06-28T14:50:26Z

src/mame/juku/juku.cpp

+	ROM_REGION(0x8000, "extension", 0)
+	// This is EKTA JBASIC cartridge from 1990, probably version 1.1
+	// from 14.09.1987, there is also version with HEX$ directive
+	// publised for running from EKDOS. E5101 test batch probably had


"published" or "released"?

boamaod added 3 commits June 13, 2022 01:20

Add sound, graphics modes, fix floppy writing/side selection.

4cb7c4f

Fix memory mode 0, add graphics mode 400x192 (= text mode 80x24).

7ce4fad

Add EKDOS 2.29 and 2.30 hashes.

919e128

boamaod mentioned this pull request Jun 20, 2022

Juku taaskogemine uuele tasemele infoaed/juku3000#4

Open

6 tasks

boamaod added 2 commits June 28, 2022 05:43

Merge remote-tracking branch 'upstream/master'

4c65508

Update descriptions, move to juku/juku.cpp to fit new directory struc…

dac36a1

…ture.

cuavas requested changes Jun 28, 2022

View reviewed changes

startaq requested changes Jun 28, 2022

View reviewed changes

boamaod mentioned this pull request Oct 10, 2024

Kuvaparameetrite dünaamiline tuletamine riistvaraportide sisendist infoaed/juku3000#6

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Juku E5101/E5104 into working machine status #9946

Juku E5101/E5104 into working machine status #9946

boamaod commented Jun 18, 2022

cuavas Jun 28, 2022

cuavas Jun 28, 2022

cuavas Jun 28, 2022

cuavas Jun 28, 2022

cuavas Jun 28, 2022

cuavas Jun 28, 2022

cuavas Jun 28, 2022

cuavas Jun 28, 2022

cuavas Jun 28, 2022

startaq Jun 28, 2022

boamaod Jun 30, 2022 •

edited

Loading

startaq Jun 28, 2022

startaq Jun 28, 2022

boamaod Jun 30, 2022

startaq Jun 28, 2022

startaq Jun 28, 2022

startaq Jun 28, 2022

startaq Jun 28, 2022

startaq Jun 28, 2022

startaq Jun 28, 2022

boamaod Jun 30, 2022

happppp Jun 30, 2022

boamaod Jun 30, 2022

mmicko Jul 1, 2022

startaq Jun 28, 2022

		logerror("pio0: c%d = %d\n", i, BIT(data, i));
		osd_printf_verbose("pio0: c%d = %d\n", i, BIT(data, i));

Juku E5101/E5104 into working machine status #9946

Are you sure you want to change the base?

Juku E5101/E5104 into working machine status #9946

Conversation

boamaod commented Jun 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boamaod Jun 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boamaod Jun 30, 2022 •

edited

Loading