On Tue, 2022-12-20 at 07:19 +0000, Kandpal, Suraj wrote: > > > > On Wed, 2022-12-14 at 14:37 +0530, Suraj Kandpal wrote: > > > > > > Alan: See my review comment on patch #1 - i believe most of this function above > > (intel_hdcp_gsc_msg_send) could go into a common > > intel_gsc_engine_send_hecipkt function (in a new gsc-heci specific file, > > i.e. intel_gsc_heci.c) that lives in the uc/gsc domain, not here in display. In fact > > the "struct intel_hdcp_gsc_message" also also be renamed to be "struct > > intel_gsc_heci_pkt_info" and its definition moved over to (and included from) a > > header in the uc/gsc domain. > > I believe it make sense for the caller to allocate the objects but the common > > header to have the structure definition and the common function can do the > > cmd-prep, submission, waiting (and eventually checking of pending-bit). > > > I can move a lot of these functions to intel_gsc_fw.c > But I still don’t see the merit in adding more functions and files in just for more readability > > Regards, > Suraj Kandpal > Replying back after the offline meeting. So just to recap, my concerns include: 1. In hindsight, "readability" wasn't the correct term - i wanted to highlight the importance of how we organize the code hierarchy in accordance to the HW architecture so that if changes in future hw or workarounds are required, we can eventually can maintain as much common-hw changes (agnostic to the internals of the heci-pkt) to a single layer 2. Also, with regards to the reviewing process, we know that 3 sets of series are emerging over these coming weeks - gsc-sw-proxy, hdcp and pxp and all of them are going to be adding patches for the mtl-gsc-mem-header structure population and the gsc-cs heci-load-pkt command submission. So it might be better for reviewers and ourselves if we can maintain roughly the same function names/locations/responsibilities as these coming series add-on or modify those common codes/.. So as per this mornings, meeting that included Daniele, here is the summary proposal we agreed on - that adds some common helpers while keeping other common-hw resposibilities as caller-handled (to cater for different e2e flows): Create new common utility files with common functions: 1. gt/uc/intel_gsc_uc_heci_cmd_submit.h 1. define the mtl-mem-header as per now 2. Add the "host-session-handle-rules" we need to add BIT-MASKS for different usages for host_session_handle uint64_t: (details of these bitmasks can be discussed thru review but here is what we concluded from mtg: - BITMASKS [63...60] (0000 = hdcp, 00001 = pxp-single, (downstrm-only 00002 = pxp-multi), future...) - caller must obey - verified by intel_gsc_uc_heci_cmd_emit_mtl_memhdr - BITMASKS [59...50] (caller-mask) - caller can fill this with anything it wants - PXP would use this arb-session-rolling-counter, hdcp could use this for pipe differentiation - BITMASKS [49...00] (randomly generated number) - caller to fill this with anything it wants but really should be randomly generated * must be unique per calling process + usage. 2. gt/uc/intel_gsc_uc_heci_cmd_submit.c a. helper -- intel_gsc_uc_heci_cmd_emit_mtl_memhdr params: - virt-ptr to the mem header (pinned object) - message_address_type - params = size - flags (after - host-session = (has to verify in the rules of "@HSH)" based on message_address_type) b. common = intel_gsc_uc_heci_cmd_submit_packet (does the same thing as the current code but an independant function, NOT overloading the gsc-load or heci-submission based on "pkt" as current patch) - mostly the same code - caller will do mtl-header pre-population using the helper 2-a - takes in the objects (from caller), creates the request internally - no pending bit management - caller handles that the checking of "pending-bit" + "gsc_message_handle" echo c. for PXP arb session - dont use priveleged instruction, use global instruction. - need to verify if this works - testing was done to use GGTT like multi-session - expected to work. - if works, we can use the same 2-b submission helper
