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

[Ville Syrjälä]

On Thu, Oct 18, 2018 at 04:55:52PM +0300, Ville Syrjälä wrote:
> 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 at linux.intel.com>

And this will definitely conflict with Maarten's NV12 work. Either we
need to duplicate even more stuff or just pass the proper states around.

> 
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni at 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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel