Skip to content

Cleanup AMD specific features. #682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
hysw wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Cleanup AMD specific features. #682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions dive_core/data_core.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,16 +390,6 @@ bool CaptureMetadataCreator::OnPacket(const IMemoryManager &mem_manager, uint32_
}

m_capture_metadata.m_event_info.push_back(event_info);

#if defined(ENABLE_CAPTURE_BUFFERS)
// Parse descriptor tables, descriptors, and descriptor contents (ie: textures,
// buffers, etc)
ShaderReflector sr;
SRDCallbacks srd_callbacks;
if (!sr.Process(srd_callbacks, mem_manager, m_state_tracker, m_constant_engine_emu,
submit_index, header.opcode, this))
return false;
#endif
}
return true;
}
Expand Down
32 changes: 0 additions & 32 deletions dive_core/data_core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,28 +117,6 @@ class DataCore
CaptureMetadata m_capture_metadata;
};

#if defined(ENABLE_CAPTURE_BUFFERS)
//--------------------------------------------------------------------------------------------------
// Shader reflector callback class, for use by the CaptureMetadataCreator
class SRDCallbacks : public IShaderReflectorCallbacks
{
public:
// Callbacks on each SRD table used by the shader. The SRD table is a buffer that contains 1 or
// more SRDs, each of which might be a different type
virtual bool OnSRDTable(ShaderStage shader_stage, uint64_t va_addr, uint64_t size,
void *user_ptr)
{
return true;
}

virtual bool OnSRDTable(ShaderStage shader_stage, void *data_ptr, uint64_t va_addr,
uint64_t size, void *user_ptr)
{
return true;
}
};
#endif

//--------------------------------------------------------------------------------------------------
// Handles creation of much of the metadata "info" from the capture
class CaptureMetadataCreator : public EmulateCallbacksBase
Expand Down Expand Up @@ -183,18 +161,8 @@ class CaptureMetadataCreator : public EmulateCallbacksBase
// Map from shader address to shader index (in m_capture_metadata.m_shaders)
std::map<uint64_t, uint32_t> m_shader_addrs;

// Map from buffer address to buffer index (in m_capture_metadata.m_buffers)
std::map<uint64_t, uint32_t> m_buffer_addrs;

CaptureMetadata &m_capture_metadata;
RenderModeType m_current_render_mode = RenderModeType::kUnknown;

#if defined(ENABLE_CAPTURE_BUFFERS)
// SRDCallbacks is a friend class, since it is essentially doing part of
// CaptureMetadataCreator's work and is only a separate class due to the callback nature of SRD
// reflection
friend class SRDCallbacks;
#endif
};

} // namespace Dive
25 changes: 10 additions & 15 deletions ...vent_timing/event_timing_graphics_scene.h → src/dive/ui/features.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
/*
Copyright 2019 Google LLC
Copyright 2025 Google LLC

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -12,20 +15,12 @@
*/

#pragma once
#include <QGraphicsScene>

class EventTimingGraphicsScene : public QGraphicsScene
struct DiveUIBuildFeatures
{
Q_OBJECT
public:
EventTimingGraphicsScene();

public slots:
void onMouseCursorChanged(QPointF new_mouse_pos);

protected:
void drawForeground(QPainter* painter, const QRectF& rect) Q_DECL_OVERRIDE;

private:
QPointF m_mouse_pos;
#ifdef ENABLE_CAPTURE_BUFFERS
Copy link
Collaborator

@wangra-google wangra-google Dec 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think could remove ENABLE_CAPTURE_BUFFERS, is it expensive of enabling it?

Copy link
Collaborator

@shanminchao shanminchao Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ENABLE_CAPTURE_BUFFERS was an AMDVLK-only experimental feature that does not apply for our new Adrenal world. I would remove the entirety of buffers_view.cpp, since it's very old.

Copy link
Collaborator

@wangra-google wangra-google Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is the part i am confused, i saw some later code in this PR when we parse the PM4, we actually get heap types from PM4, and show it in UI when the buffer capture is enabled. and the Heap type seems to be valid for Adreno gpu?

Copy link
Collaborator Author

@hysw hysw Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure we can also infer some basic information from Vulkan capture.

static constexpr bool kEnableBufferView = true;
#else
static constexpr bool kEnableBufferView = false;
#endif
};
1 change: 0 additions & 1 deletion src/dive/ui/forward.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ class DiveTreeView;
class ErrorDialog;
class EventSelection;
class EventStateView;
class EventTimingView;
Copy link
Collaborator

@wangra-google wangra-google Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we could remove event timing for now, but one of the tasks i want @GrantComm to work on is to use the timeline view (not necessarily this one) to replace the gpu timing table that we have now. The old event timing view could be a good reference for Grant.

class FrameTabView;
class GfxrVulkanCommandArgumentsFilterProxyModel;
class GfxrVulkanCommandArgumentsTabView;
Expand Down
12 changes: 0 additions & 12 deletions ui/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,18 +177,6 @@ add_executable(
viewport_stats_model.h
window_scissors_stats_model.cpp
window_scissors_stats_model.h
# Event Timing
event_timing/event_graphics_item.cpp
event_timing/event_graphics_item.h
event_timing/event_timing_graphics_scene.cpp
event_timing/event_timing_graphics_scene.h
event_timing/event_timing_graphics_view.cpp
event_timing/event_timing_graphics_view.h
event_timing/event_timing_view.cpp
event_timing/event_timing_view.h
# SQTT
sqtt/ruler_graphics_item.cpp
sqtt/ruler_graphics_item.h
)

target_include_directories(
Expand Down
85 changes: 30 additions & 55 deletions ui/buffer_view.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,31 @@
#include <QTreeWidget>
#include <QVBoxLayout>

#include "absl/strings/str_format.h"
#include "dive_core/data_core.h"
#include "dive_core/dive_strings.h"

namespace
{
QString GetHeapString(Dive::MemoryAllocationData::GpuHeap heap)
{
switch (heap)
{
case Dive::MemoryAllocationData::GpuHeap::GpuHeapLocal:
return QObject::tr("Local");
case Dive::MemoryAllocationData::GpuHeap::GpuHeapInvisible:
return QObject::tr("Invisible");
case Dive::MemoryAllocationData::GpuHeap::GpuHeapGartUswc:
return QObject::tr("GartUswc");
case Dive::MemoryAllocationData::GpuHeap::GpuHeapGartCacheable:
return QObject::tr("GartCacheable");
case Dive::MemoryAllocationData::GpuHeap::GpuHeapCount:
return QObject::tr("GpuHeap(unknown)");
}
return QObject::tr("GpuHeap(unknown)");
}
} // namespace

// =================================================================================================
// BufferWidgetItem
// =================================================================================================
Expand All @@ -46,15 +68,10 @@ BufferView::BufferView(const Dive::DataCore &data_core)
{
QVBoxLayout *layout = new QVBoxLayout();
m_buffer_list = new QTreeWidget();
m_buffer_list->setColumnCount(9);
m_buffer_list->setColumnCount(4);
m_buffer_list->setHeaderLabels(QStringList() << "Shader Stage"
<< "Address"
<< "Size"
<< "Format"
<< "Dest-X"
<< "Dest-Y"
<< "Dest-Z"
<< "Dest-W"
<< "Preferred Heap");
m_memory_view = new QTableWidget();
m_memory_view->setColumnCount(kNumDwordsPerRow + 1);
Expand Down Expand Up @@ -100,40 +117,14 @@ void BufferView::OnEventSelected(uint32_t event_index)
treeItem->setText(0, tr(kShaderStageStrings[shader_stage]));

// Column 1
constexpr uint32_t kStrBufferSize = 256;
char str_buffer[kStrBufferSize];
snprintf(str_buffer, kStrBufferSize, "%p", (void *)buffer_info.m_addr);
treeItem->setText(1, tr(str_buffer));
treeItem->setText(
1, QString::fromStdString(absl::StrFormat("%" PRIx64, buffer_info.m_addr)));

// Column 2
snprintf(str_buffer, kStrBufferSize, "%" PRIu64, buffer_info.m_size);
treeItem->setText(2, tr(str_buffer));
/*
treeItem->setText(
2, QString::fromStdString(absl::StrFormat("%" PRIu64, buffer_info.m_size)));

// Column 3
snprintf(str_buffer,
kStrBufferSize,
"%s_%s",
kBufferDataFormatStrings[buffer_info.m_data_format],
kBufferNumFormatStrings[buffer_info.m_num_format]);
treeItem->setText(3, tr(str_buffer));

// Column 4
snprintf(str_buffer, kStrBufferSize, "%s",
kSqSelStrings[(uint32_t)buffer_info.m_dst_sel_x]); treeItem->setText(4, tr(str_buffer));

// Column 5
snprintf(str_buffer, kStrBufferSize, "%s",
kSqSelStrings[(uint32_t)buffer_info.m_dst_sel_y]); treeItem->setText(5, tr(str_buffer));

// Column 6
snprintf(str_buffer, kStrBufferSize, "%s",
kSqSelStrings[(uint32_t)buffer_info.m_dst_sel_z]); treeItem->setText(6, tr(str_buffer));

// Column 7
snprintf(str_buffer, kStrBufferSize, "%s",
kSqSelStrings[(uint32_t)buffer_info.m_dst_sel_w]); treeItem->setText(7, tr(str_buffer));
*/
// Column 8
// Find preferred heap
const Dive::Pm4CaptureData &capture_data = m_data_core.GetPm4CaptureData();
const Dive::MemoryManager &memory = capture_data.GetMemoryManager();
Expand All @@ -142,24 +133,8 @@ void BufferView::OnEventSelected(uint32_t event_index)
mem_alloc_ptr =
mem_alloc_info.FindGlobalAllocation(buffer_info.m_addr, buffer_info.m_size);
DIVE_ASSERT(mem_alloc_ptr != nullptr);
switch ((Dive::MemoryAllocationData::GpuHeap)mem_alloc_ptr->m_preferred_heap)
{
case Dive::MemoryAllocationData::GpuHeap::GpuHeapLocal:
snprintf(str_buffer, kStrBufferSize, "Local");
break;
case Dive::MemoryAllocationData::GpuHeap::GpuHeapInvisible:
snprintf(str_buffer, kStrBufferSize, "Invisible");
break;
case Dive::MemoryAllocationData::GpuHeap::GpuHeapGartUswc:
snprintf(str_buffer, kStrBufferSize, "GartUswc");
break;
case Dive::MemoryAllocationData::GpuHeap::GpuHeapGartCacheable:
snprintf(str_buffer, kStrBufferSize, "GartCacheable");
break;
default:
DIVE_ASSERT(false);
}
treeItem->setText(8, tr(str_buffer));
treeItem->setText(3, GetHeapString(static_cast<Dive::MemoryAllocationData::GpuHeap>(
Copy link
Collaborator

@wangra-google wangra-google Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow, i didn't know we can check the heap info. where can i see this heap info? i tried a capture but couldn't find this info in the shader tab or pm4 packets tab.

Copy link
Collaborator

@wangra-google wangra-google Dec 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh i see, need to enable ENABLE_CAPTURE_BUFFERS? i never tried it. can you add a screenshot of the heap info in this PR?

Copy link
Collaborator Author

@hysw hysw Dec 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source information was from AMD open source driver. This view is compile time disabled (if constexpr) at the moment, I removed #if defined so compiler sees the code and enforce syntax correctness.

mem_alloc_ptr->m_preferred_heap)));

// Store the buffer index for retrieval on item selection
m_buffer_indices.push_back(buffer_index);
Expand Down
82 changes: 0 additions & 82 deletions ui/event_timing/event_graphics_item.cpp
View file
Open in desktop

This file was deleted.

Loading
Loading