Re: [PATCH 08/11] drm/i915: reorganize the error message for invalid watermarks

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Oct 16, 2018 at 03:01:30PM -0700, Paulo Zanoni wrote:
> Print a more generic "failed to compute watermark levels" whenever any
> of skl_compute_wm_levels() fail, and print only the specific error
> message for the specific cases. This allows us to stop passing pstate
> everywhere, making the watermarks computation code a little less
> dependent on random intel state structs.

Nothing random about those structs. They are *the* state.
Using them directly rather than duplicating informationa in the
wm_params struct would probably make the code look a bit less alien.

But given that the information is duplicated I guess we don't have to
pass in the plane state. So the patch seems fine in that sense.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 4053f4a68657..1290efc64869 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4635,13 +4635,11 @@ skl_compute_plane_wm_params(const struct drm_i915_private *dev_priv,
>  
>  static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  				struct intel_crtc_state *cstate,
> -				const struct intel_plane_state *intel_pstate,
>  				int level,
>  				const struct skl_wm_params *wp,
>  				const struct skl_wm_level *result_prev,
>  				struct skl_wm_level *result /* out */)
>  {
> -	const struct drm_plane_state *pstate = &intel_pstate->base;
>  	uint32_t latency = dev_priv->wm.skl_latency[level];
>  	uint_fixed_16_16_t method1, method2;
>  	uint_fixed_16_16_t selected_result;
> @@ -4763,11 +4761,7 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  		if (level) {
>  			return 0;
>  		} else {
> -			struct drm_plane *plane = pstate->plane;
> -
> -			DRM_DEBUG_KMS("Requested display configuration exceeds system watermark limitations\n");
> -			DRM_DEBUG_KMS("[PLANE:%d:%s] blocks required = %u/%u, lines required = %u/31\n",
> -				      plane->base.id, plane->name,
> +			DRM_DEBUG_KMS("Requested display configuration exceeds system watermark limitations: blocks required = %u/%u, lines required = %u/31\n",
>  				      res_blocks, wp->ddb_blocks, res_lines);
>  			return -EINVAL;
>  		}
> @@ -4795,7 +4789,6 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  static int
>  skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
>  		      struct intel_crtc_state *cstate,
> -		      const struct intel_plane_state *intel_pstate,
>  		      const struct skl_wm_params *wm_params,
>  		      struct skl_plane_wm *wm,
>  		      int plane_id)
> @@ -4816,7 +4809,6 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
>  
>  		ret = skl_compute_plane_wm(dev_priv,
>  					   cstate,
> -					   intel_pstate,
>  					   level,
>  					   wm_params,
>  					   result_prev,
> @@ -4951,10 +4943,13 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
>  		if (!wm_params.plane_visible)
>  			continue;
>  
> -		ret = skl_compute_wm_levels(dev_priv, cstate,
> -					    intel_pstate, &wm_params, wm, 0);
> -		if (ret)
> +		ret = skl_compute_wm_levels(dev_priv, cstate, &wm_params, wm,
> +					    0);
> +		if (ret) {
> +			DRM_DEBUG_KMS("[PLANE:%d:%s] failed to compute watermark levels\n",
> +				      plane->base.id, plane->name);
>  			return ret;
> +		}
>  
>  		skl_compute_transition_wm(cstate, &wm_params, &wm->wm[0],
>  					  &wm->trans_wm);
> @@ -4968,10 +4963,12 @@ static int skl_build_pipe_wm(struct intel_crtc_state *cstate,
>  				return ret;
>  
>  			ret = skl_compute_wm_levels(dev_priv, cstate,
> -						    intel_pstate, &wm_params,
> -						    wm, 1);
> -			if (ret)
> +						    &wm_params, wm, 1);
> +			if (ret) {
> +				DRM_DEBUG_KMS("[PLANE:%d:%s] failed to compute planar watermark levels\n",
> +					      plane->base.id, plane->name);
>  				return ret;
> +			}
>  		}
>  	}
>  
> -- 
> 2.14.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx