From 246cb75584af281596b938f898e8a3aedbcdb62a Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Wed, 16 Jul 2014 11:27:58 +0200 Subject: [PATCH] RegisterSet: Simplify code by using structs for register definition instead of unions. --- src/common/register_set.h | 14 +- src/core/hw/gpu.cpp | 100 ++++++------ src/core/hw/gpu.h | 150 ++++++++---------- .../renderer_opengl/renderer_opengl.cpp | 18 +-- 4 files changed, 137 insertions(+), 145 deletions(-) diff --git a/src/common/register_set.h b/src/common/register_set.h index 0418551b34..ba19a2614d 100644 --- a/src/common/register_set.h +++ b/src/common/register_set.h @@ -34,7 +34,7 @@ /* * Standardized way to define a group of registers and corresponding data structures. To define * a new register set, first define struct containing an enumeration called "Id" containing - * all register IDs and a template union called "Struct". Specialize the Struct union for any + * all register IDs and a template struct called "Struct". Specialize the Struct struct for any * register ID which needs to be accessed in a specialized way. You can then declare the object * containing all register values using the RegisterSet type, where * BaseType is the underlying type of each register (e.g. u32). @@ -54,7 +54,7 @@ * * // declare register definition structures * template - * union Struct; + * struct Struct; * }; * * // Define register set object @@ -62,9 +62,11 @@ * * // define register definition structures * template<> - * union Regs::Struct { - * BitField<0, 4, u32> some_field; - * BitField<4, 3, u32> some_other_field; + * struct Regs::Struct { + * union { + * BitField<0, 4, u32> some_field; + * BitField<4, 3, u32> some_other_field; + * }; * }; * * Usage in external code (within SomeNamespace scope): @@ -77,7 +79,7 @@ * * * @tparam BaseType Base type used for storing individual registers, e.g. u32 - * @tparam RegDefinition Class defining an enumeration called "Id" and a template union, as described above. + * @tparam RegDefinition Class defining an enumeration called "Id" and a template struct, as described above. * @note RegDefinition::Id needs to have an enum value called NumIds defining the number of registers to be allocated. */ template diff --git a/src/core/hw/gpu.cpp b/src/core/hw/gpu.cpp index 372e4f4cc9..edffa25c57 100644 --- a/src/core/hw/gpu.cpp +++ b/src/core/hw/gpu.cpp @@ -30,14 +30,14 @@ void SetFramebufferLocation(const FramebufferLocation mode) { auto& framebuffer_top = g_regs.Get(); auto& framebuffer_sub = g_regs.Get(); - framebuffer_top.data.address_left1 = PADDR_TOP_LEFT_FRAME1; - framebuffer_top.data.address_left2 = PADDR_TOP_LEFT_FRAME2; - framebuffer_top.data.address_right1 = PADDR_TOP_RIGHT_FRAME1; - framebuffer_top.data.address_right2 = PADDR_TOP_RIGHT_FRAME2; - framebuffer_sub.data.address_left1 = PADDR_SUB_FRAME1; - //framebuffer_sub.data.address_left2 = unknown; - framebuffer_sub.data.address_right1 = PADDR_SUB_FRAME2; - //framebuffer_sub.data.address_right2 = unknown; + framebuffer_top.address_left1 = PADDR_TOP_LEFT_FRAME1; + framebuffer_top.address_left2 = PADDR_TOP_LEFT_FRAME2; + framebuffer_top.address_right1 = PADDR_TOP_RIGHT_FRAME1; + framebuffer_top.address_right2 = PADDR_TOP_RIGHT_FRAME2; + framebuffer_sub.address_left1 = PADDR_SUB_FRAME1; + //framebuffer_sub.address_left2 = unknown; + framebuffer_sub.address_right1 = PADDR_SUB_FRAME2; + //framebuffer_sub.address_right2 = unknown; break; } @@ -46,14 +46,14 @@ void SetFramebufferLocation(const FramebufferLocation mode) { auto& framebuffer_top = g_regs.Get(); auto& framebuffer_sub = g_regs.Get(); - framebuffer_top.data.address_left1 = PADDR_VRAM_TOP_LEFT_FRAME1; - framebuffer_top.data.address_left2 = PADDR_VRAM_TOP_LEFT_FRAME2; - framebuffer_top.data.address_right1 = PADDR_VRAM_TOP_RIGHT_FRAME1; - framebuffer_top.data.address_right2 = PADDR_VRAM_TOP_RIGHT_FRAME2; - framebuffer_sub.data.address_left1 = PADDR_VRAM_SUB_FRAME1; - //framebuffer_sub.data.address_left2 = unknown; - framebuffer_sub.data.address_right1 = PADDR_VRAM_SUB_FRAME2; - //framebuffer_sub.data.address_right2 = unknown; + framebuffer_top.address_left1 = PADDR_VRAM_TOP_LEFT_FRAME1; + framebuffer_top.address_left2 = PADDR_VRAM_TOP_LEFT_FRAME2; + framebuffer_top.address_right1 = PADDR_VRAM_TOP_RIGHT_FRAME1; + framebuffer_top.address_right2 = PADDR_VRAM_TOP_RIGHT_FRAME2; + framebuffer_sub.address_left1 = PADDR_VRAM_SUB_FRAME1; + //framebuffer_sub.address_left2 = unknown; + framebuffer_sub.address_right1 = PADDR_VRAM_SUB_FRAME2; + //framebuffer_sub.address_right2 = unknown; break; } } @@ -135,14 +135,14 @@ inline void Write(u32 addr, const T data) { const auto& config = g_regs.Get(static_cast(index - 3)); // TODO: Not sure if this check should be done at GSP level instead - if (config.data.address_start) { + if (config.address_start) { // TODO: Not sure if this algorithm is correct, particularly because it doesn't use the size member at all - u32* start = (u32*)Memory::GetPointer(config.data.GetStartAddress()); - u32* end = (u32*)Memory::GetPointer(config.data.GetEndAddress()); + u32* start = (u32*)Memory::GetPointer(config.GetStartAddress()); + u32* end = (u32*)Memory::GetPointer(config.GetEndAddress()); for (u32* ptr = start; ptr < end; ++ptr) - *ptr = bswap32(config.data.value); // TODO: This is just a workaround to missing framebuffer format emulation + *ptr = bswap32(config.value); // TODO: This is just a workaround to missing framebuffer format emulation - DEBUG_LOG(GPU, "MemoryFill from %x to %x", config.data.GetStartAddress(), config.data.GetEndAddress()); + DEBUG_LOG(GPU, "MemoryFill from %x to %x", config.GetStartAddress(), config.GetEndAddress()); } break; } @@ -150,20 +150,20 @@ inline void Write(u32 addr, const T data) { case Regs::DisplayTransfer + 6: { const auto& config = g_regs.Get(); - if (config.data.trigger & 1) { - u8* source_pointer = Memory::GetPointer(config.data.GetPhysicalInputAddress()); - u8* dest_pointer = Memory::GetPointer(config.data.GetPhysicalOutputAddress()); + if (config.trigger & 1) { + u8* source_pointer = Memory::GetPointer(config.GetPhysicalInputAddress()); + u8* dest_pointer = Memory::GetPointer(config.GetPhysicalOutputAddress()); - for (int y = 0; y < config.data.output_height; ++y) { + for (int y = 0; y < config.output_height; ++y) { // TODO: Why does the register seem to hold twice the framebuffer width? - for (int x = 0; x < config.data.output_width / 2; ++x) { + for (int x = 0; x < config.output_width / 2; ++x) { int source[4] = { 0, 0, 0, 0}; // rgba; - switch (config.data.input_format) { + switch (config.input_format) { case Regs::FramebufferFormat::RGBA8: { // TODO: Most likely got the component order messed up. - u8* srcptr = source_pointer + x * 4 + y * config.data.input_width * 4 / 2; + u8* srcptr = source_pointer + x * 4 + y * config.input_width * 4 / 2; source[0] = srcptr[0]; // blue source[1] = srcptr[1]; // green source[2] = srcptr[2]; // red @@ -172,15 +172,15 @@ inline void Write(u32 addr, const T data) { } default: - ERROR_LOG(GPU, "Unknown source framebuffer format %x", config.data.input_format.Value()); + ERROR_LOG(GPU, "Unknown source framebuffer format %x", config.input_format.Value()); break; } - switch (config.data.output_format) { + switch (config.output_format) { /*case Regs::FramebufferFormat::RGBA8: { // TODO: Untested - u8* dstptr = (u32*)(dest_pointer + x * 4 + y * config.data.output_width * 4); + u8* dstptr = (u32*)(dest_pointer + x * 4 + y * config.output_width * 4); dstptr[0] = source[0]; dstptr[1] = source[1]; dstptr[2] = source[2]; @@ -190,7 +190,7 @@ inline void Write(u32 addr, const T data) { case Regs::FramebufferFormat::RGB8: { - u8* dstptr = dest_pointer + x * 3 + y * config.data.output_width * 3 / 2; + u8* dstptr = dest_pointer + x * 3 + y * config.output_width * 3 / 2; dstptr[0] = source[0]; // blue dstptr[1] = source[1]; // green dstptr[2] = source[2]; // red @@ -198,17 +198,17 @@ inline void Write(u32 addr, const T data) { } default: - ERROR_LOG(GPU, "Unknown destination framebuffer format %x", config.data.output_format.Value()); + ERROR_LOG(GPU, "Unknown destination framebuffer format %x", config.output_format.Value()); break; } } } DEBUG_LOG(GPU, "DisplayTriggerTransfer: %x bytes from %x(%xx%x)-> %x(%xx%x), dst format %x", - config.data.output_height * config.data.output_width * 4, - config.data.GetPhysicalInputAddress(), (int)config.data.input_width, (int)config.data.input_height, - config.data.GetPhysicalOutputAddress(), (int)config.data.output_width, (int)config.data.output_height, - config.data.output_format.Value()); + config.output_height * config.output_width * 4, + config.GetPhysicalInputAddress(), (int)config.input_width, (int)config.input_height, + config.GetPhysicalOutputAddress(), (int)config.output_width, (int)config.output_height, + config.output_format.Value()); } break; } @@ -216,10 +216,10 @@ inline void Write(u32 addr, const T data) { case Regs::CommandProcessor + 4: { const auto& config = g_regs.Get(); - if (config.data.trigger & 1) + if (config.trigger & 1) { - // u32* buffer = (u32*)Memory::GetPointer(config.data.address << 3); - ERROR_LOG(GPU, "Beginning %x bytes of commands from address %x", config.data.size, config.data.address << 3); + // u32* buffer = (u32*)Memory::GetPointer(config.address << 3); + ERROR_LOG(GPU, "Beginning %x bytes of commands from address %x", config.size, config.address << 3); // TODO: Process command list! } break; @@ -263,17 +263,17 @@ void Init() { auto& framebuffer_top = g_regs.Get(); auto& framebuffer_sub = g_regs.Get(); // TODO: Width should be 240 instead? - framebuffer_top.data.width = 480; - framebuffer_top.data.height = 400; - framebuffer_top.data.stride = 480*3; - framebuffer_top.data.color_format = Regs::FramebufferFormat::RGB8; - framebuffer_top.data.active_fb = 0; + framebuffer_top.width = 480; + framebuffer_top.height = 400; + framebuffer_top.stride = 480*3; + framebuffer_top.color_format = Regs::FramebufferFormat::RGB8; + framebuffer_top.active_fb = 0; - framebuffer_sub.data.width = 480; - framebuffer_sub.data.height = 400; - framebuffer_sub.data.stride = 480*3; - framebuffer_sub.data.color_format = Regs::FramebufferFormat::RGB8; - framebuffer_sub.data.active_fb = 0; + framebuffer_sub.width = 480; + framebuffer_sub.height = 400; + framebuffer_sub.stride = 480*3; + framebuffer_sub.color_format = Regs::FramebufferFormat::RGB8; + framebuffer_sub.active_fb = 0; NOTICE_LOG(GPU, "initialized OK"); } diff --git a/src/core/hw/gpu.h b/src/core/hw/gpu.h index ce524bd029..4ef0a047fb 100644 --- a/src/core/hw/gpu.h +++ b/src/core/hw/gpu.h @@ -29,7 +29,7 @@ struct Regs { }; template - union Struct; + struct Struct; enum class FramebufferFormat : u32 { RGBA8 = 0, @@ -38,128 +38,118 @@ struct Regs { RGB5A1 = 3, RGBA4 = 4, }; - }; template<> -union Regs::Struct { - struct { - u32 address_start; - u32 address_end; // ? - u32 size; - u32 value; // ? +struct Regs::Struct { + u32 address_start; + u32 address_end; // ? + u32 size; + u32 value; // ? - inline u32 GetStartAddress() const { - return address_start * 8; - } + inline u32 GetStartAddress() const { + return address_start * 8; + } - inline u32 GetEndAddress() const { - return address_end * 8; - } - } data; + inline u32 GetEndAddress() const { + return address_end * 8; + } }; static_assert(sizeof(Regs::Struct) == 0x10, "Structure size and register block length don't match"); template<> -union Regs::Struct { +struct Regs::Struct { using Format = Regs::FramebufferFormat; - struct { - union { - u32 size; + union { + u32 size; - BitField< 0, 16, u32> width; - BitField<16, 16, u32> height; - }; + BitField< 0, 16, u32> width; + BitField<16, 16, u32> height; + }; - u32 pad0[2]; + u32 pad0[2]; - u32 address_left1; - u32 address_left2; + u32 address_left1; + u32 address_left2; - union { - u32 format; + union { + u32 format; - BitField< 0, 3, Format> color_format; - }; + BitField< 0, 3, Format> color_format; + }; - u32 pad1; + u32 pad1; - union { - u32 active_fb; + union { + u32 active_fb; - BitField<0, 1, u32> second_fb_active; - }; + BitField<0, 1, u32> second_fb_active; + }; - u32 pad2[5]; + u32 pad2[5]; - u32 stride; + u32 stride; - u32 address_right1; - u32 address_right2; - } data; + u32 address_right1; + u32 address_right2; }; + template<> -union Regs::Struct { - using Type = decltype(Regs::Struct::data); - Type data; +struct Regs::Struct : public Regs::Struct { }; static_assert(sizeof(Regs::Struct) == 0x40, "Structure size and register block length don't match"); template<> -union Regs::Struct { +struct Regs::Struct { using Format = Regs::FramebufferFormat; - struct { - u32 input_address; - u32 output_address; + u32 input_address; + u32 output_address; - inline u32 GetPhysicalInputAddress() const { - return input_address * 8; - } + inline u32 GetPhysicalInputAddress() const { + return input_address * 8; + } - inline u32 GetPhysicalOutputAddress() const { - return output_address * 8; - } + inline u32 GetPhysicalOutputAddress() const { + return output_address * 8; + } - union { - u32 output_size; + union { + u32 output_size; - BitField< 0, 16, u32> output_width; - BitField<16, 16, u32> output_height; - }; + BitField< 0, 16, u32> output_width; + BitField<16, 16, u32> output_height; + }; - union { - u32 input_size; + union { + u32 input_size; - BitField< 0, 16, u32> input_width; - BitField<16, 16, u32> input_height; - }; + BitField< 0, 16, u32> input_width; + BitField<16, 16, u32> input_height; + }; - union { - u32 flags; + union { + u32 flags; - BitField< 0, 1, u32> flip_data; - BitField< 8, 3, Format> input_format; - BitField<12, 3, Format> output_format; - BitField<16, 1, u32> output_tiled; - }; + BitField< 0, 1, u32> flip_data; + BitField< 8, 3, Format> input_format; + BitField<12, 3, Format> output_format; + BitField<16, 1, u32> output_tiled; + }; - u32 unknown; - u32 trigger; - } data; + u32 unknown; + u32 trigger; }; static_assert(sizeof(Regs::Struct) == 0x1C, "Structure size and register block length don't match"); template<> -union Regs::Struct { - struct { - u32 size; - u32 pad0; - u32 address; - u32 pad1; - u32 trigger; - } data; +struct Regs::Struct { + u32 size; + u32 pad0; + u32 address; + u32 pad1; + u32 trigger; }; static_assert(sizeof(Regs::Struct) == 0x14, "Structure size and register block length don't match"); diff --git a/src/video_core/renderer_opengl/renderer_opengl.cpp b/src/video_core/renderer_opengl/renderer_opengl.cpp index 8d9d61ae82..50f820e4aa 100644 --- a/src/video_core/renderer_opengl/renderer_opengl.cpp +++ b/src/video_core/renderer_opengl/renderer_opengl.cpp @@ -80,17 +80,17 @@ void RendererOpenGL::RenderXFB(const common::Rect& src_rect, const common::Rect& const auto& framebuffer_top = GPU::g_regs.Get(); const auto& framebuffer_sub = GPU::g_regs.Get(); - const u32 active_fb_top = (framebuffer_top.data.active_fb == 1) - ? framebuffer_top.data.address_left2 - : framebuffer_top.data.address_left1; - const u32 active_fb_sub = (framebuffer_sub.data.active_fb == 1) - ? framebuffer_sub.data.address_left2 - : framebuffer_sub.data.address_left1; + const u32 active_fb_top = (framebuffer_top.active_fb == 1) + ? framebuffer_top.address_left2 + : framebuffer_top.address_left1; + const u32 active_fb_sub = (framebuffer_sub.active_fb == 1) + ? framebuffer_sub.address_left2 + : framebuffer_sub.address_left1; DEBUG_LOG(GPU, "RenderXFB: %x bytes from %x(%xx%x), fmt %x", - framebuffer_top.data.stride * framebuffer_top.data.height, - GPU::GetFramebufferAddr(active_fb_top), (int)framebuffer_top.data.width, - (int)framebuffer_top.data.height, (int)framebuffer_top.data.format); + framebuffer_top.stride * framebuffer_top.height, + GPU::GetFramebufferAddr(active_fb_top), (int)framebuffer_top.width, + (int)framebuffer_top.height, (int)framebuffer_top.format); // TODO: This should consider the GPU registers for framebuffer width, height and stride. FlipFramebuffer(GPU::GetFramebufferPointer(active_fb_top), m_xfb_top_flipped);