> From: Teres Alexis, Alan Previn <alan.previn.teres.ale...@intel.com> > Sent: Wednesday, February 1, 2023 2:18 PM > To: Kandpal, Suraj <suraj.kand...@intel.com>; intel- > g...@lists.freedesktop.org > Cc: Shankar, Uma <uma.shan...@intel.com>; Nautiyal, Ankit K > <ankit.k.nauti...@intel.com>; Ceraolo Spurio, Daniele > <daniele.ceraolospu...@intel.com>; Gupta, Anshuman > <anshuman.gu...@intel.com> > 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.ale...@intel.com> > > 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 > >