From 661c2ebc99100f9552dc8680f5861c1761fa3471 Mon Sep 17 00:00:00 2001 From: John Bauman Date: Tue, 20 Jun 2023 22:22:23 +0000 Subject: [PATCH] [magma] Convert msd_icd_info_t to a C++ type Rename it to MsdIcdInfo to match the style guide. Convert the component_url member to be an std::string to save space in the common case. Change-Id: I0ee93d0c3ab7d5a10a4140f96bc7ae117c86914f Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/872668 Reviewed-by: Craig Stout Fuchsia-Auto-Submit: John Bauman Commit-Queue: Auto-Submit --- .../drivers/msd-arm-mali/src/msd_arm_device.cc | 9 ++++----- .../drivers/msd-arm-mali/src/msd_arm_device.h | 2 +- .../msd-intel-gen/src/msd_intel_device.cc | 16 ++++++---------- .../drivers/msd-intel-gen/src/msd_intel_device.h | 2 +- .../drivers/msd-vsi-vip/src/msd_vsi_device.cc | 8 +++----- .../drivers/msd-vsi-vip/src/msd_vsi_device.h | 2 +- src/graphics/lib/magma/include/msd/msd_cc.h | 2 +- src/graphics/lib/magma/include/msd/msd_defs.h | 13 +++++++------ .../lib/magma/src/sys_driver/magma_device_impl.h | 2 +- .../lib/magma/src/sys_driver/magma_driver_base.h | 2 +- .../magma/src/sys_driver/magma_system_device.cc | 2 +- .../magma/src/sys_driver/magma_system_device.h | 2 +- .../sys_driver/tests/test_magma_system_device.cc | 4 ++-- src/graphics/lib/magma/tests/mock/mock_msd_cc.cc | 6 +++--- src/graphics/lib/magma/tests/mock/mock_msd_cc.h | 2 +- 15 files changed, 34 insertions(+), 40 deletions(-) diff --git a/src/graphics/drivers/msd-arm-mali/src/msd_arm_device.cc b/src/graphics/drivers/msd-arm-mali/src/msd_arm_device.cc index 657c2ec6d69..c1ca395b89b 100644 --- a/src/graphics/drivers/msd-arm-mali/src/msd_arm_device.cc +++ b/src/graphics/drivers/msd-arm-mali/src/msd_arm_device.cc @@ -1714,7 +1714,7 @@ magma_status_t MsdArmDevice::Query(uint64_t id, zx::vmo* result_buffer_out, uint void MsdArmDevice::DumpStatus(uint32_t dump_flags) { DumpStatusToLog(); } -magma_status_t MsdArmDevice::GetIcdList(std::vector* icd_info_out) { +magma_status_t MsdArmDevice::GetIcdList(std::vector* icd_info_out) { struct variant { const char* suffix; const char* url; @@ -1725,10 +1725,9 @@ magma_status_t MsdArmDevice::GetIcdList(std::vector* icd_in icd_info.clear(); icd_info.resize(std::size(kVariants)); for (uint32_t i = 0; i < std::size(kVariants); i++) { - strcpy(icd_info[i].component_url, - StringPrintf("fuchsia-pkg://%s/libvulkan_arm_mali_%lx%s#meta/vulkan.cm", - kVariants[i].url, GpuId(), kVariants[i].suffix) - .c_str()); + icd_info[i].component_url = + StringPrintf("fuchsia-pkg://%s/libvulkan_arm_mali_%lx%s#meta/vulkan.cm", kVariants[i].url, + GpuId(), kVariants[i].suffix); icd_info[i].support_flags = msd::ICD_SUPPORT_FLAG_VULKAN; } return MAGMA_STATUS_OK; diff --git a/src/graphics/drivers/msd-arm-mali/src/msd_arm_device.h b/src/graphics/drivers/msd-arm-mali/src/msd_arm_device.h index 156765cb53d..f9a5e47443f 100644 --- a/src/graphics/drivers/msd-arm-mali/src/msd_arm_device.h +++ b/src/graphics/drivers/msd-arm-mali/src/msd_arm_device.h @@ -54,7 +54,7 @@ class MsdArmDevice : public msd::Device, // msd::Device impl. void SetMemoryPressureLevel(msd::MagmaMemoryPressureLevel level) override; magma_status_t Query(uint64_t id, zx::vmo* result_buffer_out, uint64_t* result_out) override; - magma_status_t GetIcdList(std::vector* icd_info_out) override; + magma_status_t GetIcdList(std::vector* icd_info_out) override; void DumpStatus(uint32_t dump_flags) override; std::unique_ptr Open(msd::msd_client_id_t client_id) override; diff --git a/src/graphics/drivers/msd-intel-gen/src/msd_intel_device.cc b/src/graphics/drivers/msd-intel-gen/src/msd_intel_device.cc index 671a7f35418..6458a86a719 100644 --- a/src/graphics/drivers/msd-intel-gen/src/msd_intel_device.cc +++ b/src/graphics/drivers/msd-intel-gen/src/msd_intel_device.cc @@ -1280,28 +1280,24 @@ void MsdIntelDevice::CheckEngines() { void MsdIntelDevice::DumpStatus(uint32_t dump_flags) { DumpStatusToLog(); } -magma_status_t MsdIntelDevice::GetIcdList(std::vector* icd_info_out) { +magma_status_t MsdIntelDevice::GetIcdList(std::vector* icd_info_out) { const char* kSuffixes[] = {"_test", ""}; constexpr uint32_t kMediaIcdCount = 1; constexpr uint32_t kTotalIcdCount = std::size(kSuffixes) + kMediaIcdCount; - std::vector icd_info; + std::vector icd_info; icd_info.resize(kTotalIcdCount); for (uint32_t i = 0; i < std::size(kSuffixes); i++) { - strncpy(icd_info[i].component_url, - fbl::StringPrintf("fuchsia-pkg://fuchsia.com/libvulkan_intel_gen%s#meta/vulkan.cm", - kSuffixes[i]) - .c_str(), - sizeof(icd_info[i].component_url) - 1); + icd_info[i].component_url = fbl::StringPrintf( + "fuchsia-pkg://fuchsia.com/libvulkan_intel_gen%s#meta/vulkan.cm", kSuffixes[i]); icd_info[i].support_flags = msd::ICD_SUPPORT_FLAG_VULKAN; } { size_t media_index = std::size(kSuffixes); - strncpy(icd_info[media_index].component_url, - "fuchsia-pkg://fuchsia.com/codec_runner_intel_gen#meta/codec_runner_intel_gen.cm", - sizeof(icd_info[media_index].component_url) - 1); + icd_info[media_index].component_url = + "fuchsia-pkg://fuchsia.com/codec_runner_intel_gen#meta/codec_runner_intel_gen.cm"; icd_info[media_index].support_flags = msd::ICD_SUPPORT_FLAG_MEDIA_CODEC_FACTORY; } diff --git a/src/graphics/drivers/msd-intel-gen/src/msd_intel_device.h b/src/graphics/drivers/msd-intel-gen/src/msd_intel_device.h index ddcf5522cca..46cd6e56138 100644 --- a/src/graphics/drivers/msd-intel-gen/src/msd_intel_device.h +++ b/src/graphics/drivers/msd-intel-gen/src/msd_intel_device.h @@ -56,7 +56,7 @@ class MsdIntelDevice : public msd::Device, // msd::Device impl. magma_status_t Query(uint64_t id, zx::vmo* result_buffer_out, uint64_t* result_out) override; - magma_status_t GetIcdList(std::vector* icd_info_out) override; + magma_status_t GetIcdList(std::vector* icd_info_out) override; void DumpStatus(uint32_t dump_flags) override; std::unique_ptr Open(msd::msd_client_id_t client_id) override; diff --git a/src/graphics/drivers/msd-vsi-vip/src/msd_vsi_device.cc b/src/graphics/drivers/msd-vsi-vip/src/msd_vsi_device.cc index 1333884855a..ca18c45be90 100644 --- a/src/graphics/drivers/msd-vsi-vip/src/msd_vsi_device.cc +++ b/src/graphics/drivers/msd-vsi-vip/src/msd_vsi_device.cc @@ -1537,16 +1537,14 @@ magma_status_t MsdVsiDevice::Query(uint64_t id, zx::vmo* result_buffer_out, uint void MsdVsiDevice::DumpStatus(uint32_t dump_type) { DumpStatusToLog(); } -magma_status_t MsdVsiDevice::GetIcdList(std::vector* icd_info_out) { +magma_status_t MsdVsiDevice::GetIcdList(std::vector* icd_info_out) { const char* kSuffixes[] = {"_test", ""}; auto& icd_info = *icd_info_out; icd_info.clear(); icd_info.resize(std::size(kSuffixes)); for (uint32_t i = 0; i < std::size(kSuffixes); i++) { - strcpy(icd_info[i].component_url, - fbl::StringPrintf("fuchsia-pkg://fuchsia.com/libopencl_vsi_vip%s#meta/opencl.cm", - kSuffixes[i]) - .c_str()); + icd_info[i].component_url = fbl::StringPrintf( + "fuchsia-pkg://fuchsia.com/libopencl_vsi_vip%s#meta/opencl.cm", kSuffixes[i]); icd_info[i].support_flags = msd::ICD_SUPPORT_FLAG_OPENCL; } return MAGMA_STATUS_OK; diff --git a/src/graphics/drivers/msd-vsi-vip/src/msd_vsi_device.h b/src/graphics/drivers/msd-vsi-vip/src/msd_vsi_device.h index 8698d110ca0..47226ab267a 100644 --- a/src/graphics/drivers/msd-vsi-vip/src/msd_vsi_device.h +++ b/src/graphics/drivers/msd-vsi-vip/src/msd_vsi_device.h @@ -77,7 +77,7 @@ class MsdVsiDevice : public msd::Device, bool StopRingbuffer(); magma_status_t Query(uint64_t id, zx::vmo* result_buffer_out, uint64_t* result_out) override; - magma_status_t GetIcdList(std::vector* icd_info_out) override; + magma_status_t GetIcdList(std::vector* icd_info_out) override; void DumpStatus(uint32_t dump_flags) override; std::unique_ptr Open(msd::msd_client_id_t client_id) override; diff --git a/src/graphics/lib/magma/include/msd/msd_cc.h b/src/graphics/lib/magma/include/msd/msd_cc.h index 34f9cc09c8e..e2520686485 100644 --- a/src/graphics/lib/magma/include/msd/msd_cc.h +++ b/src/graphics/lib/magma/include/msd/msd_cc.h @@ -71,7 +71,7 @@ class Device { } // Outputs a list of ICD components. - virtual magma_status_t GetIcdList(std::vector* icd_info_out) { + virtual magma_status_t GetIcdList(std::vector* icd_info_out) { return MAGMA_STATUS_UNIMPLEMENTED; } diff --git a/src/graphics/lib/magma/include/msd/msd_defs.h b/src/graphics/lib/magma/include/msd/msd_defs.h index cc5d9a6428e..4eb22c3215b 100644 --- a/src/graphics/lib/magma/include/msd/msd_defs.h +++ b/src/graphics/lib/magma/include/msd/msd_defs.h @@ -8,6 +8,8 @@ #include #include +#include + namespace msd { #define MSD_DRIVER_CONFIG_TEST_NO_DEVICE_THREAD 1 @@ -16,17 +18,16 @@ namespace msd { typedef uint64_t msd_client_id_t; -enum IcdSupportFlags { +enum IcdSupportFlags : uint32_t { ICD_SUPPORT_FLAG_VULKAN = 1, ICD_SUPPORT_FLAG_OPENCL = 2, ICD_SUPPORT_FLAG_MEDIA_CODEC_FACTORY = 4, }; -typedef struct msd_icd_info_t { - // Same length as fuchsia.url.MAX_URL_LENGTH. - char component_url[4096]; - uint32_t support_flags; -} msd_icd_info_t; +struct MsdIcdInfo { + std::string component_url; + IcdSupportFlags support_flags; +}; enum MagmaMemoryPressureLevel { MAGMA_MEMORY_PRESSURE_LEVEL_NORMAL = 1, diff --git a/src/graphics/lib/magma/src/sys_driver/magma_device_impl.h b/src/graphics/lib/magma/src/sys_driver/magma_device_impl.h index c72833c2f70..f6395e27186 100644 --- a/src/graphics/lib/magma/src/sys_driver/magma_device_impl.h +++ b/src/graphics/lib/magma/src/sys_driver/magma_device_impl.h @@ -179,7 +179,7 @@ class MagmaDeviceImpl : public ddk::Messageable::Mixin, if (!CheckSystemDevice(completer)) return; fidl::Arena allocator; - std::vector msd_icd_infos; + std::vector msd_icd_infos; magma_system_device_->GetIcdList(&msd_icd_infos); std::vector icd_infos; for (auto& item : msd_icd_infos) { diff --git a/src/graphics/lib/magma/src/sys_driver/magma_driver_base.h b/src/graphics/lib/magma/src/sys_driver/magma_driver_base.h index 635b8df7cce..e8526e7994c 100644 --- a/src/graphics/lib/magma/src/sys_driver/magma_driver_base.h +++ b/src/graphics/lib/magma/src/sys_driver/magma_driver_base.h @@ -220,7 +220,7 @@ class MagmaDriverBase : public fdf::DriverBase, if (!CheckSystemDevice(completer)) return; fidl::Arena allocator; - std::vector msd_icd_infos; + std::vector msd_icd_infos; magma_system_device_->GetIcdList(&msd_icd_infos); std::vector icd_infos; for (auto& item : msd_icd_infos) { diff --git a/src/graphics/lib/magma/src/sys_driver/magma_system_device.cc b/src/graphics/lib/magma/src/sys_driver/magma_system_device.cc index bc164ea5b9d..eb0588dcf37 100644 --- a/src/graphics/lib/magma/src/sys_driver/magma_system_device.cc +++ b/src/graphics/lib/magma/src/sys_driver/magma_system_device.cc @@ -105,7 +105,7 @@ magma::Status MagmaSystemDevice::Query(uint64_t id, magma_handle_t* result_buffe return status; } -magma_status_t MagmaSystemDevice::GetIcdList(std::vector* icd_list_out) { +magma_status_t MagmaSystemDevice::GetIcdList(std::vector* icd_list_out) { return msd_dev()->GetIcdList(icd_list_out); } diff --git a/src/graphics/lib/magma/src/sys_driver/magma_system_device.h b/src/graphics/lib/magma/src/sys_driver/magma_system_device.h index d553818d29f..0f8eff1576f 100644 --- a/src/graphics/lib/magma/src/sys_driver/magma_system_device.h +++ b/src/graphics/lib/magma/src/sys_driver/magma_system_device.h @@ -63,7 +63,7 @@ class MagmaSystemDevice { magma::Status Query(uint64_t id, uint64_t* value_out) { return Query(id, nullptr, value_out); } - magma_status_t GetIcdList(std::vector* icd_list_out); + magma_status_t GetIcdList(std::vector* icd_list_out); void SetMemoryPressureLevel(MagmaMemoryPressureLevel level); diff --git a/src/graphics/lib/magma/src/sys_driver/tests/test_magma_system_device.cc b/src/graphics/lib/magma/src/sys_driver/tests/test_magma_system_device.cc index 40ec7c7a0ee..625ab407c9d 100644 --- a/src/graphics/lib/magma/src/sys_driver/tests/test_magma_system_device.cc +++ b/src/graphics/lib/magma/src/sys_driver/tests/test_magma_system_device.cc @@ -52,11 +52,11 @@ TEST(MagmaSystemDevice, GetIcdList) { auto msd_dev = std::make_unique(); auto device = MagmaSystemDevice::Create(msd_drv.get(), std::move(msd_dev)); - std::vector icds; + std::vector icds; magma_status_t status = device->GetIcdList(&icds); EXPECT_EQ(MAGMA_STATUS_OK, status); EXPECT_EQ(2u, icds.size()); - EXPECT_EQ(std::string(icds[0].component_url), "a"); + EXPECT_EQ(icds[0].component_url, "a"); } } // namespace msd diff --git a/src/graphics/lib/magma/tests/mock/mock_msd_cc.cc b/src/graphics/lib/magma/tests/mock/mock_msd_cc.cc index 2d3c74c695f..7f5bc50c123 100644 --- a/src/graphics/lib/magma/tests/mock/mock_msd_cc.cc +++ b/src/graphics/lib/magma/tests/mock/mock_msd_cc.cc @@ -40,14 +40,14 @@ magma_status_t MsdMockDevice::Query(uint64_t id, zx::vmo* result_buffer_out, uin return MAGMA_STATUS_OK; } -magma_status_t MsdMockDevice::GetIcdList(std::vector* icd_info_out) { +magma_status_t MsdMockDevice::GetIcdList(std::vector* icd_info_out) { icd_info_out->clear(); // Hardcode results. const char* kResults[] = {"a", "b"}; for (uint32_t i = 0; i < std::size(kResults); i++) { - msd::msd_icd_info_t info{}; - strcpy(info.component_url, kResults[i]); + msd::MsdIcdInfo info{}; + info.component_url = kResults[i]; info.support_flags = msd::ICD_SUPPORT_FLAG_VULKAN; icd_info_out->push_back(info); } diff --git a/src/graphics/lib/magma/tests/mock/mock_msd_cc.h b/src/graphics/lib/magma/tests/mock/mock_msd_cc.h index f7f3b567426..6af3b8e2d52 100644 --- a/src/graphics/lib/magma/tests/mock/mock_msd_cc.h +++ b/src/graphics/lib/magma/tests/mock/mock_msd_cc.h @@ -149,7 +149,7 @@ class MsdMockDevice : public msd::Device { void SetMemoryPressureLevel(msd::MagmaMemoryPressureLevel level) override; magma_status_t Query(uint64_t id, zx::vmo* result_buffer_out, uint64_t* result_out) override; - magma_status_t GetIcdList(std::vector* icd_info_out) override; + magma_status_t GetIcdList(std::vector* icd_info_out) override; std::unique_ptr Open(msd::msd_client_id_t client_id) override { return std::make_unique(); }