[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 <cstout@google.com> Fuchsia-Auto-Submit: John Bauman <jbauman@google.com> Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
gnoliyil · Jun 20, 2023 · 661c2eb · 661c2eb
1 parent a968aaa
commit 661c2eb
Show file tree

Hide file tree

Showing 15 changed files with 34 additions and 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
@@ -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<msd::msd_icd_info_t>* icd_info_out) {
+magma_status_t MsdArmDevice::GetIcdList(std::vector<msd::MsdIcdInfo>* icd_info_out) {
   struct variant {
     const char* suffix;
     const char* url;
@@ -1725,10 +1725,9 @@ magma_status_t MsdArmDevice::GetIcdList(std::vector<msd::msd_icd_info_t>* 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
@@ -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<msd::msd_icd_info_t>* icd_info_out) override;
+  magma_status_t GetIcdList(std::vector<msd::MsdIcdInfo>* icd_info_out) override;
   void DumpStatus(uint32_t dump_flags) override;
   std::unique_ptr<msd::Connection> 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
@@ -1280,28 +1280,24 @@ void MsdIntelDevice::CheckEngines() {
 
 void MsdIntelDevice::DumpStatus(uint32_t dump_flags) { DumpStatusToLog(); }
 
-magma_status_t MsdIntelDevice::GetIcdList(std::vector<msd::msd_icd_info_t>* icd_info_out) {
+magma_status_t MsdIntelDevice::GetIcdList(std::vector<msd::MsdIcdInfo>* icd_info_out) {
   const char* kSuffixes[] = {"_test", ""};
   constexpr uint32_t kMediaIcdCount = 1;
   constexpr uint32_t kTotalIcdCount = std::size(kSuffixes) + kMediaIcdCount;
 
-  std::vector<msd::msd_icd_info_t> icd_info;
+  std::vector<msd::MsdIcdInfo> 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
@@ -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<msd::msd_icd_info_t>* icd_info_out) override;
+  magma_status_t GetIcdList(std::vector<msd::MsdIcdInfo>* icd_info_out) override;
   void DumpStatus(uint32_t dump_flags) override;
   std::unique_ptr<msd::Connection> 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
@@ -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<msd::msd_icd_info_t>* icd_info_out) {
+magma_status_t MsdVsiDevice::GetIcdList(std::vector<msd::MsdIcdInfo>* 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
@@ -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<msd::msd_icd_info_t>* icd_info_out) override;
+  magma_status_t GetIcdList(std::vector<msd::MsdIcdInfo>* icd_info_out) override;
   void DumpStatus(uint32_t dump_flags) override;
   std::unique_ptr<msd::Connection> 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
@@ -71,7 +71,7 @@ class Device {
   }
 
   // Outputs a list of ICD components.
-  virtual magma_status_t GetIcdList(std::vector<msd_icd_info_t>* icd_info_out) {
+  virtual magma_status_t GetIcdList(std::vector<MsdIcdInfo>* 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
@@ -8,6 +8,8 @@
 #include <lib/magma/magma_common_defs.h>
 #include <stdint.h>
 
+#include <string>
+
 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
@@ -179,7 +179,7 @@ class MagmaDeviceImpl : public ddk::Messageable<DeviceType>::Mixin<D>,
     if (!CheckSystemDevice(completer))
       return;
     fidl::Arena allocator;
-    std::vector<msd_icd_info_t> msd_icd_infos;
+    std::vector<msd::MsdIcdInfo> msd_icd_infos;
     magma_system_device_->GetIcdList(&msd_icd_infos);
     std::vector<fuchsia_gpu_magma::wire::IcdInfo> 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
@@ -220,7 +220,7 @@ class MagmaDriverBase : public fdf::DriverBase,
     if (!CheckSystemDevice(completer))
       return;
     fidl::Arena allocator;
-    std::vector<msd_icd_info_t> msd_icd_infos;
+    std::vector<msd::MsdIcdInfo> msd_icd_infos;
     magma_system_device_->GetIcdList(&msd_icd_infos);
     std::vector<fuchsia_gpu_magma::wire::IcdInfo> 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
@@ -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<msd_icd_info_t>* icd_list_out) {
+magma_status_t MagmaSystemDevice::GetIcdList(std::vector<MsdIcdInfo>* 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
@@ -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<msd_icd_info_t>* icd_list_out);
+  magma_status_t GetIcdList(std::vector<MsdIcdInfo>* 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
@@ -52,11 +52,11 @@ TEST(MagmaSystemDevice, GetIcdList) {
   auto msd_dev = std::make_unique<MsdMockDevice>();
   auto device = MagmaSystemDevice::Create(msd_drv.get(), std::move(msd_dev));
 
-  std::vector<msd_icd_info_t> icds;
+  std::vector<msd::MsdIcdInfo> 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
@@ -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<msd::msd_icd_info_t>* icd_info_out) {
+magma_status_t MsdMockDevice::GetIcdList(std::vector<msd::MsdIcdInfo>* 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
@@ -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<msd::msd_icd_info_t>* icd_info_out) override;
+  magma_status_t GetIcdList(std::vector<msd::MsdIcdInfo>* icd_info_out) override;
   std::unique_ptr<msd::Connection> Open(msd::msd_client_id_t client_id) override {
     return std::make_unique<MsdMockConnection>();
   }