[Intel-gfx] [PATCH 1/8] drm/i915: Move locking and unclaimed check into mmio_debug_{suspend, resume}

[Sripada, Radhakrishna]

> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper at intel.com>
> Sent: Monday, August 29, 2022 10:03 AM
> To: intel-gfx at lists.freedesktop.org
> Cc: dri-devel at lists.freedesktop.org; Sripada, Radhakrishna
> <radhakrishna.sripada at intel.com>; Roper, Matthew D
> <matthew.d.roper at intel.com>
> Subject: [PATCH 1/8] drm/i915: Move locking and unclaimed check into
> mmio_debug_{suspend,resume}
> 
> Moving the locking for MMIO debug (and the final check for unclaimed
> accesses when resuming debug after a userspace-initiated forcewake) will
> make it simpler to completely skip MMIO debug handling on uncores that
> don't support it in future patches.
> 
LGTM,
Reviewed-by: Radhakrishna Sripada <radhakrishna.sripada at intel.com>

> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 41 +++++++++++++++--------------
>  1 file changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c
> b/drivers/gpu/drm/i915/intel_uncore.c
> index 9b81b2543ce2..e717ea55484a 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -50,23 +50,33 @@ intel_uncore_mmio_debug_init_early(struct
> intel_uncore_mmio_debug *mmio_debug)
>  	mmio_debug->unclaimed_mmio_check = 1;
>  }
> 
> -static void mmio_debug_suspend(struct intel_uncore_mmio_debug
> *mmio_debug)
> +static void mmio_debug_suspend(struct intel_uncore *uncore)
>  {
> -	lockdep_assert_held(&mmio_debug->lock);
> +	spin_lock(&uncore->debug->lock);
> 
>  	/* Save and disable mmio debugging for the user bypass */
> -	if (!mmio_debug->suspend_count++) {
> -		mmio_debug->saved_mmio_check = mmio_debug-
> >unclaimed_mmio_check;
> -		mmio_debug->unclaimed_mmio_check = 0;
> +	if (!uncore->debug->suspend_count++) {
> +		uncore->debug->saved_mmio_check = uncore->debug-
> >unclaimed_mmio_check;
> +		uncore->debug->unclaimed_mmio_check = 0;
>  	}
> +
> +	spin_unlock(&uncore->debug->lock);
>  }
> 
> -static void mmio_debug_resume(struct intel_uncore_mmio_debug
> *mmio_debug)
> +static bool check_for_unclaimed_mmio(struct intel_uncore *uncore);
> +
> +static void mmio_debug_resume(struct intel_uncore *uncore)
>  {
> -	lockdep_assert_held(&mmio_debug->lock);
> +	spin_lock(&uncore->debug->lock);
> +
> +	if (!--uncore->debug->suspend_count)
> +		uncore->debug->unclaimed_mmio_check = uncore->debug-
> >saved_mmio_check;
> 
> -	if (!--mmio_debug->suspend_count)
> -		mmio_debug->unclaimed_mmio_check = mmio_debug-
> >saved_mmio_check;
> +	if (check_for_unclaimed_mmio(uncore))
> +		drm_info(&uncore->i915->drm,
> +			 "Invalid mmio detected during user access\n");
> +
> +	spin_unlock(&uncore->debug->lock);
>  }
> 
>  static const char * const forcewake_domain_names[] = {
> @@ -677,9 +687,7 @@ void intel_uncore_forcewake_user_get(struct
> intel_uncore *uncore)
>  	spin_lock_irq(&uncore->lock);
>  	if (!uncore->user_forcewake_count++) {
>  		intel_uncore_forcewake_get__locked(uncore,
> FORCEWAKE_ALL);
> -		spin_lock(&uncore->debug->lock);
> -		mmio_debug_suspend(uncore->debug);
> -		spin_unlock(&uncore->debug->lock);
> +		mmio_debug_suspend(uncore);
>  	}
>  	spin_unlock_irq(&uncore->lock);
>  }
> @@ -695,14 +703,7 @@ void intel_uncore_forcewake_user_put(struct
> intel_uncore *uncore)
>  {
>  	spin_lock_irq(&uncore->lock);
>  	if (!--uncore->user_forcewake_count) {
> -		spin_lock(&uncore->debug->lock);
> -		mmio_debug_resume(uncore->debug);
> -
> -		if (check_for_unclaimed_mmio(uncore))
> -			drm_info(&uncore->i915->drm,
> -				 "Invalid mmio detected during user access\n");
> -		spin_unlock(&uncore->debug->lock);
> -
> +		mmio_debug_resume(uncore);
>  		intel_uncore_forcewake_put__locked(uncore,
> FORCEWAKE_ALL);
>  	}
>  	spin_unlock_irq(&uncore->lock);
> --
> 2.37.2