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.syrj...@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.zan...@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

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

Reply via email to