| Message ID | 20241022072920.102033-1-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/xe/hdcp: Add check to remove hdcp2 compatibility | expand |
On Tue, 22 Oct 2024, Suraj Kandpal <suraj.kandpal@intel.com> wrote: > Add check to remove HDCP2 compatibility from BMG as it does not > have GSC which ends up causing warning when we try to get reference > of GSC FW. > > Fixes: 89d030804831 ("drm/xe/hdcp: Fix condition for hdcp gsc cs requirement") > Fixes: 883631771038 ("drm/i915/mtl: Add HDCP GSC interface") > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> > --- > drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++- > drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 4 +++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > index 55965844d829..2c1d0ee8cec2 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > @@ -21,7 +21,8 @@ struct intel_hdcp_gsc_message { > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) > { > - return DISPLAY_VER(display) >= 14; > + return DISPLAY_VER(display) >= 14 && > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > } > > bool intel_hdcp_gsc_check_status(struct intel_display *display) > diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > index 231677129a35..efa3441c249c 100644 > --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > @@ -8,6 +8,7 @@ > #include <linux/delay.h> > > #include "abi/gsc_command_header_abi.h" > +#include "i915_drv.h" Hrmh, xe code should not include i915_drv.h. > #include "intel_hdcp_gsc.h" > #include "intel_hdcp_gsc_message.h" > #include "xe_bo.h" > @@ -32,7 +33,8 @@ struct intel_hdcp_gsc_message { > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) > { > - return DISPLAY_VER(display) >= 14; > + return DISPLAY_VER(display) >= 14 && > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > } > > bool intel_hdcp_gsc_check_status(struct intel_display *display)
On 22-10-2024 12:59, Suraj Kandpal wrote: > Add check to remove HDCP2 compatibility from BMG as it does not > have GSC which ends up causing warning when we try to get reference > of GSC FW. > > Fixes: 89d030804831 ("drm/xe/hdcp: Fix condition for hdcp gsc cs requirement") > Fixes: 883631771038 ("drm/i915/mtl: Add HDCP GSC interface") > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> > --- > drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++- > drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 4 +++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > index 55965844d829..2c1d0ee8cec2 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > @@ -21,7 +21,8 @@ struct intel_hdcp_gsc_message { > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) > { > - return DISPLAY_VER(display) >= 14; > + return DISPLAY_VER(display) >= 14 && > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > } > > bool intel_hdcp_gsc_check_status(struct intel_display *display) > diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > index 231677129a35..efa3441c249c 100644 > --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > @@ -8,6 +8,7 @@ > #include <linux/delay.h> > > #include "abi/gsc_command_header_abi.h" > +#include "i915_drv.h" i915 header ? > #include "intel_hdcp_gsc.h" > #include "intel_hdcp_gsc_message.h" > #include "xe_bo.h" > @@ -32,7 +33,8 @@ struct intel_hdcp_gsc_message { > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) > { > - return DISPLAY_VER(display) >= 14; > + return DISPLAY_VER(display) >= 14 && > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > } > > bool intel_hdcp_gsc_check_status(struct intel_display *display)
> -----Original Message----- > From: Jani Nikula <jani.nikula@linux.intel.com> > Sent: Tuesday, October 22, 2024 1:17 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-xe@lists.freedesktop.org; > intel-gfx@lists.freedesktop.org > Cc: Kandpal, Suraj <suraj.kandpal@intel.com>; Nautiyal, Ankit K > <ankit.k.nautiyal@intel.com>; Ghimiray, Himal Prasad > <himal.prasad.ghimiray@intel.com> > Subject: Re: [PATCH] drm/xe/hdcp: Add check to remove hdcp2 compatibility > > On Tue, 22 Oct 2024, Suraj Kandpal <suraj.kandpal@intel.com> wrote: > > Add check to remove HDCP2 compatibility from BMG as it does not have > > GSC which ends up causing warning when we try to get reference of GSC > > FW. > > > > Fixes: 89d030804831 ("drm/xe/hdcp: Fix condition for hdcp gsc cs > > requirement") > > Fixes: 883631771038 ("drm/i915/mtl: Add HDCP GSC interface") > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > > Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++- > > drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 4 +++- > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > index 55965844d829..2c1d0ee8cec2 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > @@ -21,7 +21,8 @@ struct intel_hdcp_gsc_message { > > > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) { > > - return DISPLAY_VER(display) >= 14; > > + return DISPLAY_VER(display) >= 14 && > > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > > } > > > > bool intel_hdcp_gsc_check_status(struct intel_display *display) diff > > --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > index 231677129a35..efa3441c249c 100644 > > --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > @@ -8,6 +8,7 @@ > > #include <linux/delay.h> > > > > #include "abi/gsc_command_header_abi.h" > > +#include "i915_drv.h" > > Hrmh, xe code should not include i915_drv.h. The issue is without this IP_VER used below ends up throwing an error Regards, Suraj Kandpal > > > #include "intel_hdcp_gsc.h" > > #include "intel_hdcp_gsc_message.h" > > #include "xe_bo.h" > > @@ -32,7 +33,8 @@ struct intel_hdcp_gsc_message { > > > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) { > > - return DISPLAY_VER(display) >= 14; > > + return DISPLAY_VER(display) >= 14 && > > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > > } > > > > bool intel_hdcp_gsc_check_status(struct intel_display *display) > > -- > Jani Nikula, Intel
On Tue, Oct 22, 2024 at 12:59:20PM +0530, Suraj Kandpal wrote: > Add check to remove HDCP2 compatibility from BMG as it does not > have GSC which ends up causing warning when we try to get reference > of GSC FW. Why do you say BMG doesn't have GSC? I don't see anything in the bspec indicating it wouldn't have it. If you're missing the GSC firmware, then we'll disable the engine, but that's true on any platform. Matt > > Fixes: 89d030804831 ("drm/xe/hdcp: Fix condition for hdcp gsc cs requirement") > Fixes: 883631771038 ("drm/i915/mtl: Add HDCP GSC interface") > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> > --- > drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++- > drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 4 +++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > index 55965844d829..2c1d0ee8cec2 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > @@ -21,7 +21,8 @@ struct intel_hdcp_gsc_message { > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) > { > - return DISPLAY_VER(display) >= 14; > + return DISPLAY_VER(display) >= 14 && > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > } > > bool intel_hdcp_gsc_check_status(struct intel_display *display) > diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > index 231677129a35..efa3441c249c 100644 > --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > @@ -8,6 +8,7 @@ > #include <linux/delay.h> > > #include "abi/gsc_command_header_abi.h" > +#include "i915_drv.h" > #include "intel_hdcp_gsc.h" > #include "intel_hdcp_gsc_message.h" > #include "xe_bo.h" > @@ -32,7 +33,8 @@ struct intel_hdcp_gsc_message { > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) > { > - return DISPLAY_VER(display) >= 14; > + return DISPLAY_VER(display) >= 14 && > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > } > > bool intel_hdcp_gsc_check_status(struct intel_display *display) > -- > 2.34.1 >
> -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@intel.com> > Sent: Thursday, October 24, 2024 1:15 AM > To: Kandpal, Suraj <suraj.kandpal@intel.com> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Nautiyal, > Ankit K <ankit.k.nautiyal@intel.com>; Ghimiray, Himal Prasad > <himal.prasad.ghimiray@intel.com> > Subject: Re: [PATCH] drm/xe/hdcp: Add check to remove hdcp2 compatibility > > On Tue, Oct 22, 2024 at 12:59:20PM +0530, Suraj Kandpal wrote: > > Add check to remove HDCP2 compatibility from BMG as it does not have > > GSC which ends up causing warning when we try to get reference of GSC > > FW. > > Why do you say BMG doesn't have GSC? I don't see anything in the bspec > indicating it wouldn't have it. > > If you're missing the GSC firmware, then we'll disable the engine, but that's > true on any platform. > We are missing the GSC firmware here since the decision was taken not to code in GSC for this Platform Daniele can provide more information here Regards, Suraj Kandpal > > Matt > > > > > Fixes: 89d030804831 ("drm/xe/hdcp: Fix condition for hdcp gsc cs > > requirement") > > Fixes: 883631771038 ("drm/i915/mtl: Add HDCP GSC interface") > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > > Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++- > > drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 4 +++- > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > index 55965844d829..2c1d0ee8cec2 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > @@ -21,7 +21,8 @@ struct intel_hdcp_gsc_message { > > > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) { > > - return DISPLAY_VER(display) >= 14; > > + return DISPLAY_VER(display) >= 14 && > > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > > } > > > > bool intel_hdcp_gsc_check_status(struct intel_display *display) diff > > --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > index 231677129a35..efa3441c249c 100644 > > --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > @@ -8,6 +8,7 @@ > > #include <linux/delay.h> > > > > #include "abi/gsc_command_header_abi.h" > > +#include "i915_drv.h" > > #include "intel_hdcp_gsc.h" > > #include "intel_hdcp_gsc_message.h" > > #include "xe_bo.h" > > @@ -32,7 +33,8 @@ struct intel_hdcp_gsc_message { > > > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) { > > - return DISPLAY_VER(display) >= 14; > > + return DISPLAY_VER(display) >= 14 && > > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > > } > > > > bool intel_hdcp_gsc_check_status(struct intel_display *display) > > -- > > 2.34.1 > > > > -- > Matt Roper > Graphics Software Engineer > Linux GPU Platform Enablement > Intel Corporation
On Tue, 22 Oct 2024, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote: >> -----Original Message----- >> From: Jani Nikula <jani.nikula@linux.intel.com> >> Sent: Tuesday, October 22, 2024 1:17 PM >> To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-xe@lists.freedesktop.org; >> intel-gfx@lists.freedesktop.org >> Cc: Kandpal, Suraj <suraj.kandpal@intel.com>; Nautiyal, Ankit K >> <ankit.k.nautiyal@intel.com>; Ghimiray, Himal Prasad >> <himal.prasad.ghimiray@intel.com> >> Subject: Re: [PATCH] drm/xe/hdcp: Add check to remove hdcp2 compatibility >> >> On Tue, 22 Oct 2024, Suraj Kandpal <suraj.kandpal@intel.com> wrote: >> > Add check to remove HDCP2 compatibility from BMG as it does not have >> > GSC which ends up causing warning when we try to get reference of GSC >> > FW. >> > >> > Fixes: 89d030804831 ("drm/xe/hdcp: Fix condition for hdcp gsc cs >> > requirement") >> > Fixes: 883631771038 ("drm/i915/mtl: Add HDCP GSC interface") >> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> >> > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> > Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> >> > --- >> > drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++- >> > drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 4 +++- >> > 2 files changed, 5 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c >> > b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c >> > index 55965844d829..2c1d0ee8cec2 100644 >> > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c >> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c >> > @@ -21,7 +21,8 @@ struct intel_hdcp_gsc_message { >> > >> > bool intel_hdcp_gsc_cs_required(struct intel_display *display) { >> > - return DISPLAY_VER(display) >= 14; >> > + return DISPLAY_VER(display) >= 14 && >> > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); >> > } >> > >> > bool intel_hdcp_gsc_check_status(struct intel_display *display) diff >> > --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c >> > b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c >> > index 231677129a35..efa3441c249c 100644 >> > --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c >> > +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c >> > @@ -8,6 +8,7 @@ >> > #include <linux/delay.h> >> > >> > #include "abi/gsc_command_header_abi.h" >> > +#include "i915_drv.h" >> >> Hrmh, xe code should not include i915_drv.h. > > The issue is without this IP_VER used below ends up throwing an error Yeah, we'll need to fix that. Ack as a temporary measure, but please follow through with Matt's review comments before merging. BR, Jani. > > Regards, > Suraj Kandpal >> >> > #include "intel_hdcp_gsc.h" >> > #include "intel_hdcp_gsc_message.h" >> > #include "xe_bo.h" >> > @@ -32,7 +33,8 @@ struct intel_hdcp_gsc_message { >> > >> > bool intel_hdcp_gsc_cs_required(struct intel_display *display) { >> > - return DISPLAY_VER(display) >= 14; >> > + return DISPLAY_VER(display) >= 14 && >> > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); >> > } >> > >> > bool intel_hdcp_gsc_check_status(struct intel_display *display) >> >> -- >> Jani Nikula, Intel
On 10/22/2024 12:29 AM, Suraj Kandpal wrote: > Add check to remove HDCP2 compatibility from BMG as it does not > have GSC which ends up causing warning when we try to get reference > of GSC FW. > > Fixes: 89d030804831 ("drm/xe/hdcp: Fix condition for hdcp gsc cs requirement") > Fixes: 883631771038 ("drm/i915/mtl: Add HDCP GSC interface") > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> > --- > drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++- > drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 4 +++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > index 55965844d829..2c1d0ee8cec2 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > @@ -21,7 +21,8 @@ struct intel_hdcp_gsc_message { > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) > { > - return DISPLAY_VER(display) >= 14; > + return DISPLAY_VER(display) >= 14 && > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > } > > bool intel_hdcp_gsc_check_status(struct intel_display *display) > diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > index 231677129a35..efa3441c249c 100644 > --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > @@ -8,6 +8,7 @@ > #include <linux/delay.h> > > #include "abi/gsc_command_header_abi.h" > +#include "i915_drv.h" > #include "intel_hdcp_gsc.h" > #include "intel_hdcp_gsc_message.h" > #include "xe_bo.h" > @@ -32,7 +33,8 @@ struct intel_hdcp_gsc_message { > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) > { > - return DISPLAY_VER(display) >= 14; > + return DISPLAY_VER(display) >= 14 && > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); I don't think this is the correct check or the correct location. BMG does require the GSC for HDCP, so intel_hdcp_gsc_cs_required() should still return true; it's just that we've decided not to support GSC FW loading on the platform, so we can't support HDCP2.x. Also note that the this might change and/or it might apply to other platform in the future, so any check needs to be done based on GSC support and not platform/display ID. IMO when intel_hdcp_gsc_cs_required() returns true, the caller should check if the GSC FW is defined (or if the GSCCS is available) and if it is not return that hdcp2 is not supported due to unmet prerequsites and fallback to 1.4 without printing any errors. Daniele > } > > bool intel_hdcp_gsc_check_status(struct intel_display *display)
> -----Original Message----- > From: Ceraolo Spurio, Daniele <daniele.ceraolospurio@intel.com> > Sent: Thursday, October 24, 2024 9:03 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-xe@lists.freedesktop.org; > intel-gfx@lists.freedesktop.org > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Ghimiray, Himal Prasad > <himal.prasad.ghimiray@intel.com> > Subject: Re: [PATCH] drm/xe/hdcp: Add check to remove hdcp2 compatibility > > > > > On 10/22/2024 12:29 AM, Suraj Kandpal wrote: > > Add check to remove HDCP2 compatibility from BMG as it does not have > > GSC which ends up causing warning when we try to get reference of GSC > > FW. > > > > Fixes: 89d030804831 ("drm/xe/hdcp: Fix condition for hdcp gsc cs > > requirement") > > Fixes: 883631771038 ("drm/i915/mtl: Add HDCP GSC interface") > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > > Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++- > > drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 4 +++- > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > index 55965844d829..2c1d0ee8cec2 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > > @@ -21,7 +21,8 @@ struct intel_hdcp_gsc_message { > > > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) > > { > > - return DISPLAY_VER(display) >= 14; > > + return DISPLAY_VER(display) >= 14 && > > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > > } > > > > bool intel_hdcp_gsc_check_status(struct intel_display *display) diff > > --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > index 231677129a35..efa3441c249c 100644 > > --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > > @@ -8,6 +8,7 @@ > > #include <linux/delay.h> > > > > #include "abi/gsc_command_header_abi.h" > > +#include "i915_drv.h" > > #include "intel_hdcp_gsc.h" > > #include "intel_hdcp_gsc_message.h" > > #include "xe_bo.h" > > @@ -32,7 +33,8 @@ struct intel_hdcp_gsc_message { > > > > bool intel_hdcp_gsc_cs_required(struct intel_display *display) > > { > > - return DISPLAY_VER(display) >= 14; > > + return DISPLAY_VER(display) >= 14 && > > + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > > I don't think this is the correct check or the correct location. BMG does > require the GSC for HDCP, so intel_hdcp_gsc_cs_required() should still return > true; it's just that we've decided not to support GSC FW loading on the > platform, so we can't support HDCP2.x. Also note that the this might change > and/or it might apply to other platform in the future, so any check needs to > be done based on GSC support and not platform/display ID. > > IMO when intel_hdcp_gsc_cs_required() returns true, the caller should check > if the GSC FW is defined (or if the GSCCS is available) and if it is not return > that hdcp2 is not supported due to unmet prerequsites and fallback to 1.4 > without printing any errors. > Here is the thing before this I thought that should have worked too but after hdcp_gsc_cs_required() We call intel_hdcp_gsc_check_status() which has the following check if (!gsc && !xe_uc_fw_is_enabled(&gsc->fw)) { drm_dbg_kms(&xe->drm, "GSC Components not ready for HDCP2.x\n"); return false; } And this should have returned from here but it does not it goes ahead and tries to get a xe_pm_runtime() Which causes it to shout out loud which is currently causing a lot of noise in CI Regards, Suraj Kandpal > Daniele > > > } > > > > bool intel_hdcp_gsc_check_status(struct intel_display *display)
On 10/24/2024 6:21 PM, Kandpal, Suraj wrote: > >> -----Original Message----- >> From: Ceraolo Spurio, Daniele <daniele.ceraolospurio@intel.com> >> Sent: Thursday, October 24, 2024 9:03 PM >> To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-xe@lists.freedesktop.org; >> intel-gfx@lists.freedesktop.org >> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Ghimiray, Himal Prasad >> <himal.prasad.ghimiray@intel.com> >> Subject: Re: [PATCH] drm/xe/hdcp: Add check to remove hdcp2 compatibility >> >> >> >> >> On 10/22/2024 12:29 AM, Suraj Kandpal wrote: >>> Add check to remove HDCP2 compatibility from BMG as it does not have >>> GSC which ends up causing warning when we try to get reference of GSC >>> FW. >>> >>> Fixes: 89d030804831 ("drm/xe/hdcp: Fix condition for hdcp gsc cs >>> requirement") >>> Fixes: 883631771038 ("drm/i915/mtl: Add HDCP GSC interface") >>> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> >>> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >>> Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> >>> --- >>> drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++- >>> drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 4 +++- >>> 2 files changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c >>> b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c >>> index 55965844d829..2c1d0ee8cec2 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c >>> +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c >>> @@ -21,7 +21,8 @@ struct intel_hdcp_gsc_message { >>> >>> bool intel_hdcp_gsc_cs_required(struct intel_display *display) >>> { >>> - return DISPLAY_VER(display) >= 14; >>> + return DISPLAY_VER(display) >= 14 && >>> + DISPLAY_VER_FULL(display) != IP_VER(14, 1); >>> } >>> >>> bool intel_hdcp_gsc_check_status(struct intel_display *display) diff >>> --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c >>> b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c >>> index 231677129a35..efa3441c249c 100644 >>> --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c >>> +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c >>> @@ -8,6 +8,7 @@ >>> #include <linux/delay.h> >>> >>> #include "abi/gsc_command_header_abi.h" >>> +#include "i915_drv.h" >>> #include "intel_hdcp_gsc.h" >>> #include "intel_hdcp_gsc_message.h" >>> #include "xe_bo.h" >>> @@ -32,7 +33,8 @@ struct intel_hdcp_gsc_message { >>> >>> bool intel_hdcp_gsc_cs_required(struct intel_display *display) >>> { >>> - return DISPLAY_VER(display) >= 14; >>> + return DISPLAY_VER(display) >= 14 && >>> + DISPLAY_VER_FULL(display) != IP_VER(14, 1); >> I don't think this is the correct check or the correct location. BMG does >> require the GSC for HDCP, so intel_hdcp_gsc_cs_required() should still return >> true; it's just that we've decided not to support GSC FW loading on the >> platform, so we can't support HDCP2.x. Also note that the this might change >> and/or it might apply to other platform in the future, so any check needs to >> be done based on GSC support and not platform/display ID. >> >> IMO when intel_hdcp_gsc_cs_required() returns true, the caller should check >> if the GSC FW is defined (or if the GSCCS is available) and if it is not return >> that hdcp2 is not supported due to unmet prerequsites and fallback to 1.4 >> without printing any errors. >> > Here is the thing before this I thought that should have worked too but after hdcp_gsc_cs_required() > We call intel_hdcp_gsc_check_status() which has the following check > > if (!gsc && !xe_uc_fw_is_enabled(&gsc->fw)) { This check seems incorrect to me. Shouldn't it be an OR instead of an AND? It is an OR in the i915 code. > drm_dbg_kms(&xe->drm, > "GSC Components not ready for HDCP2.x\n"); > return false; > } > > And this should have returned from here but it does not it goes ahead and tries to get a xe_pm_runtime() > Which causes it to shout out loud which is currently causing a lot of noise in CI See comment above about possible issue. But even if that is not the bug, if this function should return and it is not then we should fix this, not hack the intel_hdcp_gsc_cs_required() function. Daniele > > Regards, > Suraj Kandpal > >> Daniele >> >>> } >>> >>> bool intel_hdcp_gsc_check_status(struct intel_display *display)
> -----Original Message----- > From: Ceraolo Spurio, Daniele <daniele.ceraolospurio@intel.com> > Sent: Friday, October 25, 2024 8:35 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel-xe@lists.freedesktop.org; > intel-gfx@lists.freedesktop.org > Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Ghimiray, Himal Prasad > <himal.prasad.ghimiray@intel.com> > Subject: Re: [PATCH] drm/xe/hdcp: Add check to remove hdcp2 compatibility > > > > > On 10/24/2024 6:21 PM, Kandpal, Suraj wrote: > > > >> -----Original Message----- > >> From: Ceraolo Spurio, Daniele <daniele.ceraolospurio@intel.com> > >> Sent: Thursday, October 24, 2024 9:03 PM > >> To: Kandpal, Suraj <suraj.kandpal@intel.com>; > >> intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > >> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Ghimiray, Himal > >> Prasad <himal.prasad.ghimiray@intel.com> > >> Subject: Re: [PATCH] drm/xe/hdcp: Add check to remove hdcp2 > >> compatibility > >> > >> > >> > >> > >> On 10/22/2024 12:29 AM, Suraj Kandpal wrote: > >>> Add check to remove HDCP2 compatibility from BMG as it does not have > >>> GSC which ends up causing warning when we try to get reference of > >>> GSC FW. > >>> > >>> Fixes: 89d030804831 ("drm/xe/hdcp: Fix condition for hdcp gsc cs > >>> requirement") > >>> Fixes: 883631771038 ("drm/i915/mtl: Add HDCP GSC interface") > >>> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > >>> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > >>> Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> > >>> --- > >>> drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++- > >>> drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 4 +++- > >>> 2 files changed, 5 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > >>> b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > >>> index 55965844d829..2c1d0ee8cec2 100644 > >>> --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > >>> +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c > >>> @@ -21,7 +21,8 @@ struct intel_hdcp_gsc_message { > >>> > >>> bool intel_hdcp_gsc_cs_required(struct intel_display *display) > >>> { > >>> - return DISPLAY_VER(display) >= 14; > >>> + return DISPLAY_VER(display) >= 14 && > >>> + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > >>> } > >>> > >>> bool intel_hdcp_gsc_check_status(struct intel_display *display) > >>> diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > >>> b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > >>> index 231677129a35..efa3441c249c 100644 > >>> --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > >>> +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c > >>> @@ -8,6 +8,7 @@ > >>> #include <linux/delay.h> > >>> > >>> #include "abi/gsc_command_header_abi.h" > >>> +#include "i915_drv.h" > >>> #include "intel_hdcp_gsc.h" > >>> #include "intel_hdcp_gsc_message.h" > >>> #include "xe_bo.h" > >>> @@ -32,7 +33,8 @@ struct intel_hdcp_gsc_message { > >>> > >>> bool intel_hdcp_gsc_cs_required(struct intel_display *display) > >>> { > >>> - return DISPLAY_VER(display) >= 14; > >>> + return DISPLAY_VER(display) >= 14 && > >>> + DISPLAY_VER_FULL(display) != IP_VER(14, 1); > >> I don't think this is the correct check or the correct location. BMG > >> does require the GSC for HDCP, so intel_hdcp_gsc_cs_required() should > >> still return true; it's just that we've decided not to support GSC FW > >> loading on the platform, so we can't support HDCP2.x. Also note that > >> the this might change and/or it might apply to other platform in the > >> future, so any check needs to be done based on GSC support and not > platform/display ID. > >> > >> IMO when intel_hdcp_gsc_cs_required() returns true, the caller should > >> check if the GSC FW is defined (or if the GSCCS is available) and if > >> it is not return that hdcp2 is not supported due to unmet > >> prerequsites and fallback to 1.4 without printing any errors. > >> > > Here is the thing before this I thought that should have worked too > > but after hdcp_gsc_cs_required() We call intel_hdcp_gsc_check_status() > > which has the following check > > > > if (!gsc && !xe_uc_fw_is_enabled(&gsc->fw)) { > > This check seems incorrect to me. Shouldn't it be an OR instead of an AND? It > is an OR in the i915 code. Yes this could be it will float a new version Regards, Suraj Kandpal > > > drm_dbg_kms(&xe->drm, > > "GSC Components not ready for HDCP2.x\n"); > > return false; > > } > > > > And this should have returned from here but it does not it goes ahead > > and tries to get a xe_pm_runtime() Which causes it to shout out loud > > which is currently causing a lot of noise in CI > > See comment above about possible issue. But even if that is not the bug, if > this function should return and it is not then we should fix this, not hack the > intel_hdcp_gsc_cs_required() function. > > Daniele > > > > > Regards, > > Suraj Kandpal > > > >> Daniele > >> > >>> } > >>> > >>> bool intel_hdcp_gsc_check_status(struct intel_display *display)
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c index 55965844d829..2c1d0ee8cec2 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c @@ -21,7 +21,8 @@ struct intel_hdcp_gsc_message { bool intel_hdcp_gsc_cs_required(struct intel_display *display) { - return DISPLAY_VER(display) >= 14; + return DISPLAY_VER(display) >= 14 && + DISPLAY_VER_FULL(display) != IP_VER(14, 1); } bool intel_hdcp_gsc_check_status(struct intel_display *display) diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c index 231677129a35..efa3441c249c 100644 --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c @@ -8,6 +8,7 @@ #include <linux/delay.h> #include "abi/gsc_command_header_abi.h" +#include "i915_drv.h" #include "intel_hdcp_gsc.h" #include "intel_hdcp_gsc_message.h" #include "xe_bo.h" @@ -32,7 +33,8 @@ struct intel_hdcp_gsc_message { bool intel_hdcp_gsc_cs_required(struct intel_display *display) { - return DISPLAY_VER(display) >= 14; + return DISPLAY_VER(display) >= 14 && + DISPLAY_VER_FULL(display) != IP_VER(14, 1); } bool intel_hdcp_gsc_check_status(struct intel_display *display)
Add check to remove HDCP2 compatibility from BMG as it does not have GSC which ends up causing warning when we try to get reference of GSC FW. Fixes: 89d030804831 ("drm/xe/hdcp: Fix condition for hdcp gsc cs requirement") Fixes: 883631771038 ("drm/i915/mtl: Add HDCP GSC interface") Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> Reviewed-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com> --- drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 3 ++- drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-)