> From: Teres Alexis, Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
> Sent: Wednesday, February 1, 2023 2:18 PM
> To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Shankar, Uma <uma.shankar@xxxxxxxxx>; Nautiyal, Ankit K
> <ankit.k.nautiyal@xxxxxxxxx>; Ceraolo Spurio, Daniele
> <daniele.ceraolospurio@xxxxxxxxx>; Gupta, Anshuman
> <anshuman.gupta@xxxxxxxxx>
> Subject: Re: [PATCH v9 5/6] drm/i915/mtl: Add function to send command to
> GSC CS
>
> Conditional Rb with below fix in intel_hdcp_gsc_free_message:
>
> Reviewed-by: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
>
> On Tue, 2023-01-31 at 12:03 +0530, Kandpal, Suraj wrote:
> > Add function that takes care of sending command to gsc cs. We start of
> > with allocation of memory for our command intel_hdcp_gsc_message that
> > contains gsc cs memory header as directed in specs followed by the
> > actual payload hdcp message that we want to send.
> > Spec states that we need to poll pending bit of response header around
> > 20 times each try being 50ms apart hence adding that to current
> > gsc_msg_send function Also we use the same function to take care of
> > both sending and receiving hence no separate function to get the
> > response.
> >
> >
> alan:snip..
>
> > +/*This function helps allocate memory for the command that we will
> > +send to gsc cs */ int intel_hdcp_gsc_hdcp2_init(struct
> > +drm_i915_private *i915) {
> > +
> alan: okay i see this is now being called from intel_hdcp_gsc_init
>
> > +void intel_hdcp_gsc_free_message(struct drm_i915_private *i915) {
> > + struct intel_hdcp_gsc_message *hdcp_message =
> > +
> > +i915->display.hdcp.hdcp_message;
> > + if (hdcp_message->vma)
> > + i915_vma_unpin(fetch_and_zero(&hdcp_message->vma));
> alan: i believe you don't need this extra i915_vma_unpin. Also, take note you
> have a bug above ...
> first code does a "fetch_and_zero" and right after zero-ing out its used to call
> i915_vma_unpin_and_release on the line below. So.. "if (hdcp_message-
> >vma) -> i915_vma_unpin_and_release"
I see we can just skip this part and directly do an i915_vma_unpin_and_release
Regards,
Suraj Kandpal
> > +
> > + i915_vma_unpin_and_release(&hdcp_message->vma,
> > +I915_VMA_RELEASE_MAP);
> > + kfree(hdcp_message);
> > +}
> > +
> >
>
> alan:snip..
> > --
> > 2.25.1
> >
