Skip to content
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.

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

pbkit: Enable flip stall; expose ramin, print_char, depth buffer internals #628

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

antangelo
Copy link
Contributor

@antangelo antangelo commented Mar 2, 2023

This PR upstreams the pbkit changes required by the pgraph test suite.

They are all combined into one PR, but can be split it if preferred.
CC: @abaire

@@ -2468,7 +2468,7 @@ int pb_finished(void)
p=pb_push1(p,NV20_TCL_PRIMITIVE_3D_WAIT_MAKESPACE,0); //wait/makespace (obtains null status)
p=pb_push1(p,NV20_TCL_PRIMITIVE_3D_PARAMETER_A,pb_back_index); //set param=back buffer index to show up
p=pb_push1(p,NV20_TCL_PRIMITIVE_3D_FIRE_INTERRUPT,PB_FINISHED); //subprogID PB_FINISHED: gets frame ready to show up soon
// p=pb_push1(p,NV20_TCL_PRIMITIVE_3D_STALL_PIPELINE,0); //stall gpu pipeline (not sure it's needed in triple buffering technic)
p=pb_push1(p,NV20_TCL_PRIMITIVE_3D_STALL_PIPELINE,0); //stall gpu pipeline (not sure it's needed in triple buffering technic)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed?

@@ -2630,6 +2631,11 @@ void pb_set_color_format(unsigned int fmt, bool swizzled) {
assert(swizzled == false);
}

void pb_set_fb_size_multiplier(unsigned int multiplier) {
assert(multiplier > 0);
pb_FBSizeMultiplier = multiplier;
Copy link
Member

Choose a reason for hiding this comment

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

When would you use this?

@@ -159,8 +159,6 @@ void pb_create_gr_instance(int ChannelID,
DWORD flags3D,
struct s_CtxDma *pGrObject);

// Exposed so pb_print can be overridden using a version of vsprintf that
// supports floats.
Copy link
Member

Choose a reason for hiding this comment

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

Why was this comment removed?

@@ -144,6 +144,10 @@ void pb_create_gr_ctx( int ChannelID,
struct s_CtxDma *pGrObject );
void pb_bind_channel(struct s_CtxDma *pCtxDmaObject);

uint8_t *pb_depth_stencil_buffer();
DWORD pb_depth_stencil_pitch();
DWORD pb_depth_stencil_size();
Copy link
Member

@JayFoxRox JayFoxRox Mar 4, 2023

Choose a reason for hiding this comment

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

I always thought pbkit had this interface backwards. Currently pbkit is a mix of a low-level and high-level driver.

pbkit can submit pushbuffers, but it also constructs them, some using rather high level operations.
Instead, I believe that pbkit should allow submission of pushbuffers, but it should primarily be the users responsibility to construct them.
pbkit should only control the hardware so it can process commands from the pushbuffers.

More importantly, pbkit also manages buffers internally (sometimes like a singleton), but doesn't allow the user to manually create or control these buffers.
Instead, I believe that pbkit should provide the user with buffer creation / managing functions, but the user should always explicitly create these buffers and keep track of them.

This means I'd prefer to see something like pb_create_depth_stencil_buffer(size, pitch) and then the user should be responsible for creating these buffers.
So, in an ideal world, the application would never even have to query for the buffer address / pitch / size, because it created the buffer itself.

This (explicit buffer creation by application) is also required (or at least helpful) for more advanced applications which want to switch render-targets.

@JayFoxRox
Copy link
Member

JayFoxRox commented Mar 4, 2023

Commit titles don't follow our usual style (<prefix>: <Verb> [...] without any punctuation at the end)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants