Module: Mesa Branch: main Commit: 87e7f6abbe195689d7302d0844e5b25d920809a8 URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=87e7f6abbe195689d7302d0844e5b25d920809a8
Author: Karmjit Mahil <[email protected]> Date: Thu Jun 29 15:17:57 2023 +0100 pvr: Remove some magic numbers and increments from km stream - Update and add csbgen definitions to make the content of the geom and frag km stream more obvious. - Replace some of the hard coded constants with defines. - Adds some static assert to make the provenance of definitions more clear as well as making sure things fit properly. Signed-off-by: Karmjit Mahil <[email protected]> Reviewed-by: Frank Binns <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/24138> --- src/imagination/csbgen/rogue_cr.xml | 14 +---- src/imagination/csbgen/rogue_kmd_stream.xml | 37 ++++++++++++ src/imagination/vulkan/pvr_job_render.c | 65 +++++++++++++--------- src/imagination/vulkan/pvr_job_render.h | 15 +++++ src/imagination/vulkan/pvr_job_transfer.c | 24 ++++---- .../vulkan/winsys/pvrsrvkm/pvr_srv_job_transfer.c | 8 +-- 6 files changed, 109 insertions(+), 54 deletions(-) diff --git a/src/imagination/csbgen/rogue_cr.xml b/src/imagination/csbgen/rogue_cr.xml index f7c2d5337e0..8ce85bc3009 100644 --- a/src/imagination/csbgen/rogue_cr.xml +++ b/src/imagination/csbgen/rogue_cr.xml @@ -669,19 +669,7 @@ SOFTWARE. <field name="width" start="0" end="1" type="PIXEL_WIDTH"/> </struct> - <struct name="USC_CLEAR_REGISTER0" length="1"> - <field name="val" start="0" end="31" type="uint"/> - </struct> - - <struct name="USC_CLEAR_REGISTER1" length="1"> - <field name="val" start="0" end="31" type="uint"/> - </struct> - - <struct name="USC_CLEAR_REGISTER2" length="1"> - <field name="val" start="0" end="31" type="uint"/> - </struct> - - <struct name="USC_CLEAR_REGISTER3" length="1"> + <struct name="USC_CLEAR_REGISTER" length="1"> <field name="val" start="0" end="31" type="uint"/> </struct> diff --git a/src/imagination/csbgen/rogue_kmd_stream.xml b/src/imagination/csbgen/rogue_kmd_stream.xml index 0e659a80494..4277c6a4c6c 100644 --- a/src/imagination/csbgen/rogue_kmd_stream.xml +++ b/src/imagination/csbgen/rogue_kmd_stream.xml @@ -31,6 +31,12 @@ TODO: Once the kernel driver is merged upstream, check to see if this comment needs updating. --> +<!-- +TODO: Currently the sizes for fields are in bits. Those should be changed to +bytes. Might want to do this in conjunction with csbgen being changed from +dword to bytes granular. +--> + <csbgen name="ROGUE" prefix="KMD_STREAM"> <struct name="HDR" length="2"> @@ -70,4 +76,35 @@ needs updating. <field name="has_brn49927" start="0" end="0" type="bool"/> </struct> + <enum name="PIXEL_PHANTOM_STATE"> + <value name="DISABLED" value="0x0"/> + <value name="ENABLED" value="0xF"/> + </enum> + + <!-- + Note: if there's an attempt to disable all phantoms, they will all be set + to their default states (i.e. all enabled). + --> + <struct name="PIXEL_PHANTOM" length="1"> + <field name="phantom_1" start="4" end="7" type="PIXEL_PHANTOM_STATE"/> + <field name="phantom_0" start="0" end="3" type="PIXEL_PHANTOM_STATE"/> + </struct> + + <struct name="VIEW_IDX" length="1"> + <field name="idx" start="0" end="7" type="uint"/> + </struct> + + <!-- + Note: this does not depend on + \ref pvr_device_features.has_eight_output_registers . + It's always 8 catering for the largest size. + --> + <!-- + TODO: Instead of defining this, see if we can use something like anvil's + "group" where a field is repeated n times. That would allow us to get the + total length of the whole group instead of the driver calculating it by + multiplying this with the size of the usc reg. + --> + <define name="USC_CLEAR_REGISTER_COUNT" value="8"/> + </csbgen> diff --git a/src/imagination/vulkan/pvr_job_render.c b/src/imagination/vulkan/pvr_job_render.c index f7e1071afc2..35a6bb9a7e0 100644 --- a/src/imagination/vulkan/pvr_job_render.c +++ b/src/imagination/vulkan/pvr_job_render.c @@ -990,9 +990,10 @@ static void pvr_geom_state_stream_init(struct pvr_render_ctx *ctx, } stream_ptr += pvr_cmd_length(VDMCTRL_PDS_STATE0); - /* Set up view_idx to 0 */ - *stream_ptr = 0; - stream_ptr++; + /* clang-format off */ + pvr_csb_pack (stream_ptr, KMD_STREAM_VIEW_IDX, value); + /* clang-format on */ + stream_ptr += pvr_cmd_length(KMD_STREAM_VIEW_IDX); state->fw_stream_len = (uint8_t *)stream_ptr - (uint8_t *)state->fw_stream; assert(state->fw_stream_len <= ARRAY_SIZE(state->fw_stream)); @@ -1196,11 +1197,15 @@ static void pvr_frag_state_stream_init(struct pvr_render_ctx *ctx, stream_ptr += pvr_cmd_length(CR_FB_CDC_ZLS); } - STATIC_ASSERT(ARRAY_SIZE(job->pbe_reg_words) == 8U); - STATIC_ASSERT(ARRAY_SIZE(job->pbe_reg_words[0]) == 3U); +#define DWORDS_PER_U64 2 + + STATIC_ASSERT(ARRAY_SIZE(job->pbe_reg_words) == PVR_MAX_COLOR_ATTACHMENTS); + STATIC_ASSERT(ARRAY_SIZE(job->pbe_reg_words[0]) == + ROGUE_NUM_PBESTATE_REG_WORDS); STATIC_ASSERT(sizeof(job->pbe_reg_words[0][0]) == sizeof(uint64_t)); memcpy(stream_ptr, job->pbe_reg_words, sizeof(job->pbe_reg_words)); - stream_ptr += 8U * 3U * 2U; + stream_ptr += + PVR_MAX_COLOR_ATTACHMENTS * ROGUE_NUM_PBESTATE_REG_WORDS * DWORDS_PER_U64; pvr_csb_pack ((uint64_t *)stream_ptr, CR_TPU_BORDER_COLOUR_TABLE_PDM, @@ -1210,26 +1215,33 @@ static void pvr_frag_state_stream_init(struct pvr_render_ctx *ctx, } stream_ptr += pvr_cmd_length(CR_TPU_BORDER_COLOUR_TABLE_PDM); - STATIC_ASSERT(ARRAY_SIZE(job->pds_bgnd_reg_values) == 3U); + STATIC_ASSERT(ARRAY_SIZE(job->pds_bgnd_reg_values) == + ROGUE_NUM_CR_PDS_BGRND_WORDS); STATIC_ASSERT(sizeof(job->pds_bgnd_reg_values[0]) == sizeof(uint64_t)); memcpy(stream_ptr, job->pds_bgnd_reg_values, sizeof(job->pds_bgnd_reg_values)); - stream_ptr += 3U * 2U; + stream_ptr += ROGUE_NUM_CR_PDS_BGRND_WORDS * DWORDS_PER_U64; - STATIC_ASSERT(ARRAY_SIZE(job->pds_pr_bgnd_reg_values) == 3U); + STATIC_ASSERT(ARRAY_SIZE(job->pds_pr_bgnd_reg_values) == + ROGUE_NUM_CR_PDS_BGRND_WORDS); STATIC_ASSERT(sizeof(job->pds_pr_bgnd_reg_values[0]) == sizeof(uint64_t)); memcpy(stream_ptr, job->pds_pr_bgnd_reg_values, sizeof(job->pds_pr_bgnd_reg_values)); - stream_ptr += 3U * 2U; + stream_ptr += ROGUE_NUM_CR_PDS_BGRND_WORDS * DWORDS_PER_U64; + +#undef DWORDS_PER_U64 - /* Set usc_clear_register array to 0 */ - memset(stream_ptr, 0, 8U * sizeof(uint32_t)); - stream_ptr += 8U; + memset(stream_ptr, + 0, + PVRX(KMD_STREAM_USC_CLEAR_REGISTER_COUNT) * + PVR_DW_TO_BYTES(pvr_cmd_length(CR_USC_CLEAR_REGISTER))); + stream_ptr += PVRX(KMD_STREAM_USC_CLEAR_REGISTER_COUNT) * + pvr_cmd_length(CR_USC_CLEAR_REGISTER); *stream_ptr = pixel_ctl; - stream_ptr++; + stream_ptr += pvr_cmd_length(CR_USC_PIXEL_OUTPUT_CTRL); pvr_csb_pack (stream_ptr, CR_ISP_BGOBJDEPTH, value) { const float depth_clear = job->ds_clear_value.depth; @@ -1301,10 +1313,7 @@ static void pvr_frag_state_stream_init(struct pvr_render_ctx *ctx, stream_ptr += pvr_cmd_length(CR_EVENT_PIXEL_PDS_INFO); if (PVR_HAS_FEATURE(dev_info, cluster_grouping)) { - uint32_t pixel_phantom = 0; - - if (PVR_HAS_FEATURE(dev_info, slc_mcu_cache_controls) && - dev_runtime_info->num_phantoms > 1 && job->frag_uses_atomic_ops) { + pvr_csb_pack (stream_ptr, KMD_STREAM_PIXEL_PHANTOM, value) { /* Each phantom has its own MCU, so atomicity can only be guaranteed * when all work items are processed on the same phantom. This means * we need to disable all USCs other than those of the first @@ -1312,16 +1321,22 @@ static void pvr_frag_state_stream_init(struct pvr_render_ctx *ctx, * for atomic operations in fragment shaders, since hardware * prevents the TA to run on more than one phantom anyway. */ - pixel_phantom = 0xF; + /* Note that leaving all phantoms disabled (as csbgen will do by + * default since it will zero out things) will set them to their + * default state (i.e. enabled) instead of disabling them. + */ + if (PVR_HAS_FEATURE(dev_info, slc_mcu_cache_controls) && + dev_runtime_info->num_phantoms > 1 && job->frag_uses_atomic_ops) { + value.phantom_0 = PVRX(KMD_STREAM_PIXEL_PHANTOM_STATE_ENABLED); + } } - - *stream_ptr = pixel_phantom; - stream_ptr++; + stream_ptr += pvr_cmd_length(KMD_STREAM_PIXEL_PHANTOM); } - /* Set up view_idx to 0 */ - *stream_ptr = 0; - stream_ptr++; + /* clang-format off */ + pvr_csb_pack (stream_ptr, KMD_STREAM_VIEW_IDX, value); + /* clang-format on */ + stream_ptr += pvr_cmd_length(KMD_STREAM_VIEW_IDX); pvr_csb_pack (stream_ptr, CR_EVENT_PIXEL_PDS_DATA, value) { value.addr = PVR_DEV_ADDR(job->pds_pixel_event_data_offset); diff --git a/src/imagination/vulkan/pvr_job_render.h b/src/imagination/vulkan/pvr_job_render.h index b7c25b10d3a..ae21c3e3dd6 100644 --- a/src/imagination/vulkan/pvr_job_render.h +++ b/src/imagination/vulkan/pvr_job_render.h @@ -29,6 +29,7 @@ #include <vulkan/vulkan.h> #include "hwdef/rogue_hw_defs.h" +#include "pvr_csb.h" #include "pvr_limits.h" #include "pvr_types.h" @@ -138,9 +139,23 @@ struct pvr_render_job { */ uint32_t max_tiles_in_flight; + static_assert(pvr_cmd_length(PBESTATE_REG_WORD0) == 2, + "PBESTATE_REG_WORD0 cannot be stored in uint64_t"); + static_assert(pvr_cmd_length(PBESTATE_REG_WORD1) == 2, + "PBESTATE_REG_WORD1 cannot be stored in uint64_t"); + static_assert(ROGUE_NUM_PBESTATE_REG_WORDS >= 2, + "Cannot store both PBESTATE_REG_WORD{0,1}"); uint64_t pbe_reg_words[PVR_MAX_COLOR_ATTACHMENTS] [ROGUE_NUM_PBESTATE_REG_WORDS]; + static_assert(pvr_cmd_length(CR_PDS_BGRND0_BASE) == 2, + "CR_PDS_BGRND0_BASE cannot be stored in uint64_t"); + static_assert(pvr_cmd_length(CR_PDS_BGRND1_BASE) == 2, + "CR_PDS_BGRND1_BASE cannot be stored in uint64_t"); + static_assert(pvr_cmd_length(CR_PDS_BGRND3_SIZEINFO) == 2, + "CR_PDS_BGRND3_SIZEINFO cannot be stored in uint64_t"); + static_assert(ROGUE_NUM_CR_PDS_BGRND_WORDS == 3, + "Cannot store all CR_PDS_BGRND words"); uint64_t pds_bgnd_reg_values[ROGUE_NUM_CR_PDS_BGRND_WORDS]; uint64_t pds_pr_bgnd_reg_values[ROGUE_NUM_CR_PDS_BGRND_WORDS]; }; diff --git a/src/imagination/vulkan/pvr_job_transfer.c b/src/imagination/vulkan/pvr_job_transfer.c index 1dbe9aa1447..6bec19b79e9 100644 --- a/src/imagination/vulkan/pvr_job_transfer.c +++ b/src/imagination/vulkan/pvr_job_transfer.c @@ -2711,19 +2711,19 @@ static VkResult pvr_3d_copy_blit_core(struct pvr_transfer_ctx *ctx, if (result != VK_SUCCESS) return result; - pvr_csb_pack (®s->usc_clear_register0, CR_USC_CLEAR_REGISTER0, reg) { + pvr_csb_pack (®s->usc_clear_register0, CR_USC_CLEAR_REGISTER, reg) { reg.val = packed_color[0U]; } - pvr_csb_pack (®s->usc_clear_register1, CR_USC_CLEAR_REGISTER1, reg) { + pvr_csb_pack (®s->usc_clear_register1, CR_USC_CLEAR_REGISTER, reg) { reg.val = packed_color[1U]; } - pvr_csb_pack (®s->usc_clear_register2, CR_USC_CLEAR_REGISTER2, reg) { + pvr_csb_pack (®s->usc_clear_register2, CR_USC_CLEAR_REGISTER, reg) { reg.val = packed_color[2U]; } - pvr_csb_pack (®s->usc_clear_register3, CR_USC_CLEAR_REGISTER3, reg) { + pvr_csb_pack (®s->usc_clear_register3, CR_USC_CLEAR_REGISTER, reg) { reg.val = packed_color[3U]; } @@ -4043,25 +4043,25 @@ static VkResult pvr_isp_ctrl_stream(const struct pvr_device_info *dev_info, fill_mapping.dst_rect = transfer_cmd->scissor; pvr_csb_pack (®s->usc_clear_register0, - CR_USC_CLEAR_REGISTER0, + CR_USC_CLEAR_REGISTER, reg) { reg.val = packed_color[0U]; } pvr_csb_pack (®s->usc_clear_register1, - CR_USC_CLEAR_REGISTER1, + CR_USC_CLEAR_REGISTER, reg) { reg.val = packed_color[1U]; } pvr_csb_pack (®s->usc_clear_register2, - CR_USC_CLEAR_REGISTER2, + CR_USC_CLEAR_REGISTER, reg) { reg.val = packed_color[2U]; } pvr_csb_pack (®s->usc_clear_register3, - CR_USC_CLEAR_REGISTER3, + CR_USC_CLEAR_REGISTER, reg) { reg.val = packed_color[3U]; } @@ -5694,16 +5694,16 @@ pvr_submit_info_stream_init(struct pvr_transfer_ctx *ctx, stream_ptr += pvr_cmd_length(CR_USC_PIXEL_OUTPUT_CTRL); *stream_ptr = regs->usc_clear_register0; - stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER0); + stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER); *stream_ptr = regs->usc_clear_register1; - stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER1); + stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER); *stream_ptr = regs->usc_clear_register2; - stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER2); + stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER); *stream_ptr = regs->usc_clear_register3; - stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER3); + stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER); *stream_ptr = regs->isp_mtile_size; stream_ptr += pvr_cmd_length(CR_ISP_MTILE_SIZE); diff --git a/src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_job_transfer.c b/src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_job_transfer.c index 35ba01b535b..49c0a7483b5 100644 --- a/src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_job_transfer.c +++ b/src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_job_transfer.c @@ -169,16 +169,16 @@ pvr_srv_transfer_cmd_stream_load(struct rogue_fwif_cmd_transfer *const cmd, stream_ptr += pvr_cmd_length(CR_USC_PIXEL_OUTPUT_CTRL); regs->usc_clear_register0 = *stream_ptr; - stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER0); + stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER); regs->usc_clear_register1 = *stream_ptr; - stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER1); + stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER); regs->usc_clear_register2 = *stream_ptr; - stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER2); + stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER); regs->usc_clear_register3 = *stream_ptr; - stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER3); + stream_ptr += pvr_cmd_length(CR_USC_CLEAR_REGISTER); regs->isp_mtile_size = *stream_ptr; stream_ptr += pvr_cmd_length(CR_ISP_MTILE_SIZE);
