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
