| Message ID | 20231009094254.653551-2-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor i915 HDCP for XE | expand |
On Mon, 09 Oct 2023, Suraj Kandpal <suraj.kandpal@intel.com> wrote: > Move checks for gsc components required for HDCP 2.2 > to work into intel_hdcp_gsc.c. This will also help > with XE refactor on HDCP's side. > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > Reviewed-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/display/intel_hdcp.c | 8 +------- > drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 14 ++++++++++++++ > drivers/gpu/drm/i915/display/intel_hdcp_gsc.h | 1 + > 3 files changed, 16 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c > index 8cca4793cf92..c89da3568ebd 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -173,14 +173,8 @@ bool intel_hdcp2_capable(struct intel_connector *connector) > > /* If MTL+ make sure gsc is loaded and proxy is setup */ > if (intel_hdcp_gsc_cs_required(i915)) { > - struct intel_gt *gt = i915->media_gt; > - struct intel_gsc_uc *gsc = gt ? >->uc.gsc : NULL; > - > - if (!gsc || !intel_uc_fw_is_running(&gsc->fw)) { > - drm_dbg_kms(&i915->drm, > - "GSC components required for HDCP2.2 are not ready\n"); > + if (!intel_hdcp_gsc_check_status(i915)) Naming nitpick, though not intended to block the patch: I always stop at functions named "check" that return a value. What does the return value mean? The above code reads, "if not check status". So I usually try to name functions so that it's obvious on both sides what it means, when you read it aloud. For example, "if not status ok". Just something to consider for the future. BR, Jani. > return false; > - } > } > > /* MEI/GSC interface is solid depending on which is used */ > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > index d753db3eef15..d355d610bc9f 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > @@ -17,6 +17,20 @@ bool intel_hdcp_gsc_cs_required(struct drm_i915_private *i915) > return DISPLAY_VER(i915) >= 14; > } > > +bool intel_hdcp_gsc_check_status(struct drm_i915_private *i915) > +{ > + struct intel_gt *gt = i915->media_gt; > + struct intel_gsc_uc *gsc = gt ? >->uc.gsc : NULL; > + > + if (!gsc || !intel_uc_fw_is_running(&gsc->fw)) { > + drm_dbg_kms(&i915->drm, > + "GSC components required for HDCP2.2 are not ready\n"); > + return false; > + } > + > + return true; > +} > + > static int > gsc_hdcp_initiate_session(struct device *dev, struct hdcp_port_data *data, > struct hdcp2_ake_init *ake_data) > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h > index cbf96551e534..eba2057c5a9e 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h > @@ -23,5 +23,6 @@ ssize_t intel_hdcp_gsc_msg_send(struct drm_i915_private *i915, u8 *msg_in, > size_t msg_out_len); > int intel_hdcp_gsc_init(struct drm_i915_private *i915); > void intel_hdcp_gsc_fini(struct drm_i915_private *i915); > +bool intel_hdcp_gsc_check_status(struct drm_i915_private *i915); > > #endif /* __INTEL_HDCP_GCS_H__ */
> -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Monday, October 9, 2023 3:37 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH v5 1/2] drm/i915/hdcp: Move checks for gsc > health status > > On Mon, 09 Oct 2023, Suraj Kandpal <suraj.kandpal@intel.com> wrote: > > Move checks for gsc components required for HDCP 2.2 to work into > > intel_hdcp_gsc.c. This will also help with XE refactor on HDCP's side. > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > Reviewed-by: Uma Shankar <uma.shankar@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_hdcp.c | 8 +------- > > drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 14 ++++++++++++++ > > drivers/gpu/drm/i915/display/intel_hdcp_gsc.h | 1 + > > 3 files changed, 16 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > index 8cca4793cf92..c89da3568ebd 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > @@ -173,14 +173,8 @@ bool intel_hdcp2_capable(struct intel_connector > > *connector) > > > > /* If MTL+ make sure gsc is loaded and proxy is setup */ > > if (intel_hdcp_gsc_cs_required(i915)) { > > - struct intel_gt *gt = i915->media_gt; > > - struct intel_gsc_uc *gsc = gt ? >->uc.gsc : NULL; > > - > > - if (!gsc || !intel_uc_fw_is_running(&gsc->fw)) { > > - drm_dbg_kms(&i915->drm, > > - "GSC components required for HDCP2.2 are > not ready\n"); > > + if (!intel_hdcp_gsc_check_status(i915)) > > Naming nitpick, though not intended to block the patch: > > I always stop at functions named "check" that return a value. What does the > return value mean? The above code reads, "if not check status". > > So I usually try to name functions so that it's obvious on both sides what it > means, when you read it aloud. For example, "if not status ok". > > Just something to consider for the future. > > Got it Jani will keep this in mind in future Regards, Suraj Kandpal > BR, > Jani. > > > > > return false; > > - } > > } > > > > /* MEI/GSC interface is solid depending on which is used */ diff > > --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > index d753db3eef15..d355d610bc9f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > @@ -17,6 +17,20 @@ bool intel_hdcp_gsc_cs_required(struct > drm_i915_private *i915) > > return DISPLAY_VER(i915) >= 14; > > } > > > > +bool intel_hdcp_gsc_check_status(struct drm_i915_private *i915) { > > + struct intel_gt *gt = i915->media_gt; > > + struct intel_gsc_uc *gsc = gt ? >->uc.gsc : NULL; > > + > > + if (!gsc || !intel_uc_fw_is_running(&gsc->fw)) { > > + drm_dbg_kms(&i915->drm, > > + "GSC components required for HDCP2.2 are not > ready\n"); > > + return false; > > + } > > + > > + return true; > > +} > > + > > static int > > gsc_hdcp_initiate_session(struct device *dev, struct hdcp_port_data *data, > > struct hdcp2_ake_init *ake_data) diff --git > > a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h > > b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h > > index cbf96551e534..eba2057c5a9e 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h > > @@ -23,5 +23,6 @@ ssize_t intel_hdcp_gsc_msg_send(struct > drm_i915_private *i915, u8 *msg_in, > > size_t msg_out_len); > > int intel_hdcp_gsc_init(struct drm_i915_private *i915); void > > intel_hdcp_gsc_fini(struct drm_i915_private *i915); > > +bool intel_hdcp_gsc_check_status(struct drm_i915_private *i915); > > > > #endif /* __INTEL_HDCP_GCS_H__ */ > > -- > Jani Nikula, Intel
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index 8cca4793cf92..c89da3568ebd 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -173,14 +173,8 @@ bool intel_hdcp2_capable(struct intel_connector *connector) /* If MTL+ make sure gsc is loaded and proxy is setup */ if (intel_hdcp_gsc_cs_required(i915)) { - struct intel_gt *gt = i915->media_gt; - struct intel_gsc_uc *gsc = gt ? >->uc.gsc : NULL; - - if (!gsc || !intel_uc_fw_is_running(&gsc->fw)) { - drm_dbg_kms(&i915->drm, - "GSC components required for HDCP2.2 are not ready\n"); + if (!intel_hdcp_gsc_check_status(i915)) return false; - } } /* MEI/GSC interface is solid depending on which is used */ diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c index d753db3eef15..d355d610bc9f 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c @@ -17,6 +17,20 @@ bool intel_hdcp_gsc_cs_required(struct drm_i915_private *i915) return DISPLAY_VER(i915) >= 14; } +bool intel_hdcp_gsc_check_status(struct drm_i915_private *i915) +{ + struct intel_gt *gt = i915->media_gt; + struct intel_gsc_uc *gsc = gt ? >->uc.gsc : NULL; + + if (!gsc || !intel_uc_fw_is_running(&gsc->fw)) { + drm_dbg_kms(&i915->drm, + "GSC components required for HDCP2.2 are not ready\n"); + return false; + } + + return true; +} + static int gsc_hdcp_initiate_session(struct device *dev, struct hdcp_port_data *data, struct hdcp2_ake_init *ake_data) diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h index cbf96551e534..eba2057c5a9e 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.h @@ -23,5 +23,6 @@ ssize_t intel_hdcp_gsc_msg_send(struct drm_i915_private *i915, u8 *msg_in, size_t msg_out_len); int intel_hdcp_gsc_init(struct drm_i915_private *i915); void intel_hdcp_gsc_fini(struct drm_i915_private *i915); +bool intel_hdcp_gsc_check_status(struct drm_i915_private *i915); #endif /* __INTEL_HDCP_GCS_H__ */