Implement stub version of the Wii AV Encoder

[Pokechu22]

This PR implements a stub version of the audio/video encoder (AVE-RVL) found on the Wii. This is accessed via I²C using two GPIO pins, which involves some awkward bitbanging on the PPC side, and is even more awkward for us. All I've done is implement this I²C interface and respond to it appropriately, specifically only for writes to the address of the AV encoder (reads are unhandled, and games don't seem to use them, though symbol maps mention __VIGetVenderID, VIGetCGMS, and VIGetWSS so this might be an issue). (Note that the way I²C acknowledges a byte is by holding the data line low, and since the previous implementation never set AVE_SDA for the input it always was acknowledged; now we don't acknowledge it in some cases.)

The logging occurs under WII_IPC since that's where the GPIOs are handled currently (and very little else is logged there).

This is mainly intended to help with debugging gamma issues (e.g. 12974).

[Warepire]

The i2c bus addressing is 7 bits (or 10 bits for extended addressing, but that doesn't seem to be needed here), with the last bit of that byte determining if it's a read or write operation.

There's a pretty good explanation of the i2c bus here: https://www.robot-electronics.co.uk/i2c-tutorial

So address would be: ((byte >> 1) & 0x7F)
And read_op (which you call read_i2c_address) would be: (byte & 0x1)

[Pokechu22]

Yes, I have that implemented already; it detects read operations and does not acknowledge them:

dolphin/Source/Core/Core/HW/WII_IPC.cpp

Lines 360 to 384 in b551c8b

	if (!i2c_state.read_i2c_address)
	{
	i2c_state.read_i2c_address = true;
	if ((i2c_state.current_byte >> 1) == 0x70)
	{
	i2c_state.is_correct_i2c_address = true;
	}
	else
	{
	WARN_LOG_FMT(WII_IPC, "AVE: Wrong I2C address: {:02x}", i2c_state.current_byte >> 1);
	i2c_state.acknowledge = false;
	i2c_state.is_correct_i2c_address = false;
	}
	if ((i2c_state.current_byte & 1) == 0)
	{
	i2c_state.is_read = false;
	}
	else
	{
	WARN_LOG_FMT(WII_IPC, "AVE: Reads aren't implemented yet");
	i2c_state.is_read = true;
	i2c_state.acknowledge = false; // until reads are implemented
	}
	}

I just haven't implemented sending back data if a read operation is performed, as no games I have seem to use it.

To clarify, the AV encoder is located at I2C address 0x70 (0b1110000) which is apparently used for "AV colour space converters" typically, which does mean that it's written using the byte 0xe0 and read using 0xe1. There isn't anything else on the I2C bus that I'm aware of.

[Warepire]

Although current_address is just assigned a byte directly from the i2c stream so it's missing the drop of the read/write bit?

[Pokechu22]

current_address is the address within the audio/video encoder itself, sent after the i2c address. The flow is that you first write the i2c address (0x70<<1 or 0xe0), then you write the address within the device (e.g. 0x10 for the gamma coefficients), then you keep on writing bytes and the address increments each time (e.g. the first byte writes to 0x10, then to 0x11, then to 0x12), and then it's done when the i2c stop signal occurs.

Since there's only one device, I never actually store the i2c address; it's only checked if it is 0x70 (is_correct_i2c_address).

(Though now I'm not sure how a read would even work, as you would need to write current_address first and then read the response - I haven't seen any examples of reads anywhere.)

[Warepire]

Now I understand, I expected current_address to be the address of the currently addressed device. I suspect at this time it's no use to divide the struct up between bus and device, but perhaps a comment about what is bus state and what is device state would help clarify?

For reads to work the chip's registers would need to be somewhat properly emulated (default register states) and knowing what writes would end up looking like (as sometimes software is lazy and writes RO bits), and carry the entire address data around in structs / arrays. Some devices (unfortunately, mostly video decoders) are paged, so the address space then is the number of pages * 256.

[Pokechu22]

I think the device itself has a 256-byte address space without paging (it might be a 128-byte address space even, as the highest known register is 0x7D). All listed registers are marked writable (though Wiibrew doesn't say whether they're all readable - I'm not sure if that's a case of reading not having been tested, or if they read back as 0 or something.

After looking more closely at the symbol map, a __VIReceiveI2CData seems to exist (though I don't know of any games that actually have it). However, the Wiibrew article mentions a 3rd-party hardware mod that does get attached to the I2C bus, which is used by @Aurelio92's RVLoader (from here), and lives at I2C address 0x20 or 0x21. This isn't something we need to emulate itself (Dolphin is already perfectly capable of running games), but it does mean that Dolphin's I2C implementation should be a bit more extensible, and that homebrew should be usable as a test case for reads.

(The article you linked also explained how reads work: you send start, then the 7-bit address and a 0 bit (for write), then the internal address, then another start, then the 7-bit address and a 1-bit (for read), then start reading, then send stop. The second start is something I wasn't aware of.)

[iwubcode]

Commented on the issue ticket but yes, the logging does show the gamma values changing when changed in game:

45:52:481 Core\HW\WII_IPC.cpp:401 I[WII_IPC]: AVE: Wrote 80 to 1b (Gamma coefficients)
45:52:481 Core\HW\WII_IPC.cpp:401 I[WII_IPC]: AVE: Wrote 19 to 25 (Gamma coefficients)
45:52:481 Core\HW\WII_IPC.cpp:401 I[WII_IPC]: AVE: Wrote 2f to 27 (Gamma coefficients)
45:52:481 Core\HW\WII_IPC.cpp:401 I[WII_IPC]: AVE: Wrote 80 to 28 (Gamma coefficients)
45:52:481 Core\HW\WII_IPC.cpp:401 I[WII_IPC]: AVE: Wrote 4f to 29 (Gamma coefficients)
45:52:481 Core\HW\WII_IPC.cpp:401 I[WII_IPC]: AVE: Wrote c0 to 2a (Gamma coefficients)
45:52:481 Core\HW\WII_IPC.cpp:401 I[WII_IPC]: AVE: Wrote 7a to 2b (Gamma coefficients)
45:52:481 Core\HW\WII_IPC.cpp:401 I[WII_IPC]: AVE: Wrote ad to 2d (Gamma coefficients)
45:52:481 Core\HW\WII_IPC.cpp:401 I[WII_IPC]: AVE: Wrote c0 to 2e (Gamma coefficients)

[iwubcode]

Sorry, I think I missed this before. Since this is creating a temporary string, I don't think we can return a std::string_view. Either we'd need a static value that we fill out or just return a std::string

[Rumi-Larry]

Seems like this was meant to be the left channel volume, otherwise it seems really weird that the right channel appears twice

[Pokechu22]

Yeah, probably it should be left, though the wiki is a bit ambiguous: https://wiibrew.org/wiki/Hardware/AV_Encoder#Registers_71h_.26_72h