Re: [Intel-gfx] [PATCH 6/6] drm/i915: Pimp icl+ sagv pre/post update

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

On Mon, Feb 14, 2022 at 12:00:11PM +0200, Lisovskiy, Stanislav wrote:
> On Mon, Feb 14, 2022 at 11:18:11AM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Add some debugs on what exactly we're doing to the QGV point mask
> > in the icl+ sagv pre/post plane update hooks. Currently we're just
> > guessing.
> > 
> > Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 37 ++++++++++++++++-----------------
> >  1 file changed, 18 insertions(+), 19 deletions(-)
> 
> Weird I think, I had those debugs initially. Definitely remember
> there was something similar. Was it kinda removed later?

Can't immediately see any such debugs being added or removed
by any commit.

> 
> Stan
> 
> Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>  
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 8b70cdc3b58b..5d1f1a9988bb 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3818,26 +3818,22 @@ static void icl_sagv_pre_plane_update(struct intel_atomic_state *state)
> >  		intel_atomic_get_old_bw_state(state);
> >  	const struct intel_bw_state *new_bw_state =
> >  		intel_atomic_get_new_bw_state(state);
> > -	u32 new_mask;
> > +	u32 old_mask, new_mask;
> >  
> >  	if (!new_bw_state)
> >  		return;
> >  
> > -	/*
> > -	 * Nothing to mask
> > -	 */
> > -	if (new_bw_state->qgv_points_mask == old_bw_state->qgv_points_mask)
> > -		return;
> > -
> > +	old_mask = old_bw_state->qgv_points_mask;
> >  	new_mask = old_bw_state->qgv_points_mask | new_bw_state->qgv_points_mask;
> >  
> > -	/*
> > -	 * If new mask is zero - means there is nothing to mask,
> > -	 * we can only unmask, which should be done in unmask.
> > -	 */
> > -	if (!new_mask)
> > +	if (old_mask == new_mask)
> >  		return;
> >  
> > +	WARN_ON(!new_bw_state->base.changed);
> > +
> > +	drm_dbg_kms(&dev_priv->drm, "Restricting QGV points: 0x%x -> 0x%x\n",
> > +		    old_mask, new_mask);
> > +
> >  	/*
> >  	 * Restrict required qgv points before updating the configuration.
> >  	 * According to BSpec we can't mask and unmask qgv points at the same
> > @@ -3854,19 +3850,22 @@ static void icl_sagv_post_plane_update(struct intel_atomic_state *state)
> >  		intel_atomic_get_old_bw_state(state);
> >  	const struct intel_bw_state *new_bw_state =
> >  		intel_atomic_get_new_bw_state(state);
> > -	u32 new_mask = 0;
> > +	u32 old_mask, new_mask;
> >  
> >  	if (!new_bw_state)
> >  		return;
> >  
> > -	/*
> > -	 * Nothing to unmask
> > -	 */
> > -	if (new_bw_state->qgv_points_mask == old_bw_state->qgv_points_mask)
> > -		return;
> > -
> > +	old_mask = old_bw_state->qgv_points_mask | new_bw_state->qgv_points_mask;
> >  	new_mask = new_bw_state->qgv_points_mask;
> >  
> > +	if (old_mask == new_mask)
> > +		return;
> > +
> > +	WARN_ON(!new_bw_state->base.changed);
> > +
> > +	drm_dbg_kms(&dev_priv->drm, "Relaxing QGV points: 0x%x -> 0x%x\n",
> > +		    old_mask, new_mask);
> > +
> >  	/*
> >  	 * Allow required qgv points after updating the configuration.
> >  	 * According to BSpec we can't mask and unmask qgv points at the same
> > -- 
> > 2.34.1
> > 

-- 
Ville Syrjälä
Intel