Re: [Intel-gfx] [PATCH 17/18] drm/i915/pm: Prefer drm_WARN_ON over WARN_ON

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 06 Apr 2020, Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com> wrote:
> struct drm_device specific drm_WARN* macros include device information
> in the backtrace, so we know what device the warnings originate from.
>
> Prefer drm_WARN_ON over WARN_ON.
>
> Conversion is done with below sementic patch:
>
> @@
> identifier func, T;
> @@
> func(...) {
> ...
> struct intel_crtc *T = ...;
> <+...
> -WARN_ON(
> +drm_WARN_ON(T->base.dev,
> ...)
> ...+>
>
> }
>
> @@
> identifier func, T;
> @@
> func(struct intel_crtc_state *T,...) {
> <+...
> -WARN_ON(
> +drm_WARN_ON(T->uapi.crtc->dev,
> ...)
> ...+>
>
> }
>
> Signed-off-by: Pankaj Bharadiya <pankaj.laxminarayan.bharadiya@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 57 ++++++++++++++++++---------------
>  1 file changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8375054ba27d..b2d22fdaf3db 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1464,8 +1464,8 @@ static int g4x_compute_intermediate_wm(struct intel_crtc_state *new_crtc_state)
>  			max(optimal->wm.plane[plane_id],
>  			    active->wm.plane[plane_id]);
>  
> -		WARN_ON(intermediate->wm.plane[plane_id] >
> -			g4x_plane_fifo_size(plane_id, G4X_WM_LEVEL_NORMAL));
> +		drm_WARN_ON(crtc->base.dev, intermediate->wm.plane[plane_id] >
> +			    g4x_plane_fifo_size(plane_id, G4X_WM_LEVEL_NORMAL));
>  	}
>  
>  	intermediate->sr.plane = max(optimal->sr.plane,
> @@ -1482,21 +1482,25 @@ static int g4x_compute_intermediate_wm(struct intel_crtc_state *new_crtc_state)
>  	intermediate->hpll.fbc = max(optimal->hpll.fbc,
>  				     active->hpll.fbc);
>  
> -	WARN_ON((intermediate->sr.plane >
> -		 g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_SR) ||
> -		 intermediate->sr.cursor >
> -		 g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_SR)) &&
> -		intermediate->cxsr);
> -	WARN_ON((intermediate->sr.plane >
> -		 g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_HPLL) ||
> -		 intermediate->sr.cursor >
> -		 g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_HPLL)) &&
> -		intermediate->hpll_en);
> -
> -	WARN_ON(intermediate->sr.fbc > g4x_fbc_fifo_size(1) &&
> -		intermediate->fbc_en && intermediate->cxsr);
> -	WARN_ON(intermediate->hpll.fbc > g4x_fbc_fifo_size(2) &&
> -		intermediate->fbc_en && intermediate->hpll_en);
> +	drm_WARN_ON(crtc->base.dev,
> +		    (intermediate->sr.plane >
> +		     g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_SR) ||
> +		     intermediate->sr.cursor >
> +		     g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_SR)) &&
> +		    intermediate->cxsr);
> +	drm_WARN_ON(crtc->base.dev,
> +		    (intermediate->sr.plane >
> +		     g4x_plane_fifo_size(PLANE_PRIMARY, G4X_WM_LEVEL_HPLL) ||
> +		     intermediate->sr.cursor >
> +		     g4x_plane_fifo_size(PLANE_CURSOR, G4X_WM_LEVEL_HPLL)) &&
> +		    intermediate->hpll_en);
> +
> +	drm_WARN_ON(crtc->base.dev,
> +		    intermediate->sr.fbc > g4x_fbc_fifo_size(1) &&
> +		    intermediate->fbc_en && intermediate->cxsr);
> +	drm_WARN_ON(crtc->base.dev,
> +		    intermediate->hpll.fbc > g4x_fbc_fifo_size(2) &&
> +		    intermediate->fbc_en && intermediate->hpll_en);

Please add a i915 local variable and use &i915->drm.

>  
>  out:
>  	/*
> @@ -1748,11 +1752,11 @@ static int vlv_compute_fifo(struct intel_crtc_state *crtc_state)
>  		fifo_left -= plane_extra;
>  	}
>  
> -	WARN_ON(active_planes != 0 && fifo_left != 0);
> +	drm_WARN_ON(crtc->base.dev, active_planes != 0 && fifo_left != 0);
>  
>  	/* give it all to the first plane if none are active */
>  	if (active_planes == 0) {
> -		WARN_ON(fifo_left != fifo_size);
> +		drm_WARN_ON(crtc->base.dev, fifo_left != fifo_size);
>  		fifo_state->plane[PLANE_PRIMARY] = fifo_left;
>  	}
>  
> @@ -4154,7 +4158,8 @@ skl_plane_downscale_amount(const struct intel_crtc_state *crtc_state,
>  	uint_fixed_16_16_t fp_w_ratio, fp_h_ratio;
>  	uint_fixed_16_16_t downscale_h, downscale_w;
>  
> -	if (WARN_ON(!intel_wm_plane_visible(crtc_state, plane_state)))
> +	if (drm_WARN_ON(crtc_state->uapi.crtc->dev,
> +			!intel_wm_plane_visible(crtc_state, plane_state)))
>  		return u32_to_fixed16(0);
>  
>  	/*
> @@ -4815,7 +4820,7 @@ intel_get_linetime_us(const struct intel_crtc_state *crtc_state)
>  
>  	pixel_rate = crtc_state->pixel_rate;
>  
> -	if (WARN_ON(pixel_rate == 0))
> +	if (drm_WARN_ON(crtc_state->uapi.crtc->dev, pixel_rate == 0))
>  		return u32_to_fixed16(0);
>  
>  	crtc_htotal = crtc_state->hw.adjusted_mode.crtc_htotal;
> @@ -4832,7 +4837,8 @@ skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
>  	uint_fixed_16_16_t downscale_amount;
>  
>  	/* Shouldn't reach here on disabled planes... */
> -	if (WARN_ON(!intel_wm_plane_visible(crtc_state, plane_state)))
> +	if (drm_WARN_ON(crtc_state->uapi.crtc->dev,
> +			!intel_wm_plane_visible(crtc_state, plane_state)))
>  		return 0;
>  
>  	/*
> @@ -5261,9 +5267,10 @@ static int icl_build_plane_wm(struct intel_crtc_state *crtc_state,
>  		const struct drm_framebuffer *fb = plane_state->hw.fb;
>  		enum plane_id y_plane_id = plane_state->planar_linked_plane->id;
>  
> -		WARN_ON(!intel_wm_plane_visible(crtc_state, plane_state));
> -		WARN_ON(!fb->format->is_yuv ||
> -			fb->format->num_planes == 1);
> +		drm_WARN_ON(crtc_state->uapi.crtc->dev,
> +			    !intel_wm_plane_visible(crtc_state, plane_state));
> +		drm_WARN_ON(crtc_state->uapi.crtc->dev, !fb->format->is_yuv ||
> +			    fb->format->num_planes == 1);

Ditto.

BR,
Jani.


>  
>  		ret = skl_build_plane_wm_single(crtc_state, plane_state,
>  						y_plane_id, 0);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx