diff mbox series

[1/3] drm/i915/hdcp: Do intel_hdcp_component_init() much later during init

Message ID 20231215110933.9188-2-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Eaerly ggtt pinning stuff | expand

Commit Message

Ville Syrjälä Dec. 15, 2023, 11:09 a.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

intel_hdcp_component_init()->...->intel_hdcp_gsc_initialize_message()
will allocate ggtt address space for some hdcp gsc message thing.
That is currently being done way too early as we haven't even
taken over the BIOS fb yet. So this has the potential of corrupting
ggtt PTEs that need to be preserved until the the BIOS fb takover
is done.

Only call intel_hdcp_component_init() once all the BIOS fb takeover,
and full ggtt init (which currently also needs to reserve very
specific ranges of ggtt, thus assuming that no one else has stolen
them yet) is done.

Cc: Suraj Kandpal <suraj.kandpal@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_driver.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Andrzej Hajda Jan. 10, 2024, 3:31 p.m. UTC | #1
On 15.12.2023 12:09, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> intel_hdcp_component_init()->...->intel_hdcp_gsc_initialize_message()
> will allocate ggtt address space for some hdcp gsc message thing.
> That is currently being done way too early as we haven't even
> taken over the BIOS fb yet. So this has the potential of corrupting
> ggtt PTEs that need to be preserved until the the BIOS fb takover
> is done.
> 
> Only call intel_hdcp_component_init() once all the BIOS fb takeover,
> and full ggtt init (which currently also needs to reserve very
> specific ranges of ggtt, thus assuming that no one else has stolen
> them yet) is done.
> 
> Cc: Suraj Kandpal <suraj.kandpal@intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

Regards
Andrzej

> ---
>   drivers/gpu/drm/i915/display/intel_display_driver.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
> index 62f7b10484be..b71338b4d793 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_driver.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
> @@ -319,8 +319,6 @@ int intel_display_driver_probe_nogem(struct drm_i915_private *i915)
>   	intel_display_driver_init_hw(i915);
>   	intel_dpll_update_ref_clks(i915);
>   
> -	intel_hdcp_component_init(i915);
> -
>   	if (i915->display.cdclk.max_cdclk_freq == 0)
>   		intel_update_max_cdclk(i915);
>   
> @@ -360,6 +358,13 @@ int intel_display_driver_probe(struct drm_i915_private *i915)
>   	if (!HAS_DISPLAY(i915))
>   		return 0;
>   
> +	/*
> +	 * This will bind stuff into ggtt, so it needs to be done after
> +	 * the BIOS fb takeover and whatever else magic ggtt reservations
> +	 * happen during gem/ggtt init.
> +	 */
> +	intel_hdcp_component_init(i915);
> +
>   	/*
>   	 * Force all active planes to recompute their states. So that on
>   	 * mode_setcrtc after probe, all the intel_plane_state variables
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index 62f7b10484be..b71338b4d793 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -319,8 +319,6 @@  int intel_display_driver_probe_nogem(struct drm_i915_private *i915)
 	intel_display_driver_init_hw(i915);
 	intel_dpll_update_ref_clks(i915);
 
-	intel_hdcp_component_init(i915);
-
 	if (i915->display.cdclk.max_cdclk_freq == 0)
 		intel_update_max_cdclk(i915);
 
@@ -360,6 +358,13 @@  int intel_display_driver_probe(struct drm_i915_private *i915)
 	if (!HAS_DISPLAY(i915))
 		return 0;
 
+	/*
+	 * This will bind stuff into ggtt, so it needs to be done after
+	 * the BIOS fb takeover and whatever else magic ggtt reservations
+	 * happen during gem/ggtt init.
+	 */
+	intel_hdcp_component_init(i915);
+
 	/*
 	 * Force all active planes to recompute their states. So that on
 	 * mode_setcrtc after probe, all the intel_plane_state variables