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

[Andrzej Hajda <andrzej.hajda@intel.com>]

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