Re: [PATCH] FOR_UPSTREAM [VPG]: drm/i915/skl+: Implement Transition WM

[Mahesh Kumar <mahesh1.kumar@xxxxxxxxx>]

Hi,

On Wednesday 31 August 2016 07:17 PM, Zanoni, Paulo R wrote:
> Em Ter, 2016-08-30 às 19:32 +0000, Zanoni, Paulo R escreveu:
> > Hi
> >
> > Em Seg, 2016-08-29 às 18:05 +0530, Kumar, Mahesh escreveu:
> > > This patch enables Transition WM for SKL+ platforms.
> > >
> > > Transition WM are used if IPC is enabled, to decide, number of
> > > blocks
> > > to be fetched before reducing the priority of display to fetch from
> > > memory.
> > >
> > > Signed-off-by: Kumar, Mahesh <mahesh1.kumar@xxxxxxxxx>
> > > ---
> > >   drivers/gpu/drm/i915/intel_pm.c | 57
> > > ++++++++++++++++++++++++++++++++++++++---
> > >   1 file changed, 53 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index 9f69050..f4f387f 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -2861,6 +2861,8 @@ bool ilk_disable_lp_wm(struct drm_device
> > > *dev)
> > >   #define SKL_DDB_SIZE		896	/* in blocks */
> > >   #define BXT_DDB_SIZE		512
> > >   #define SKL_SAGV_BLOCK_TIME	30 /* µs */
> > > +#define SKL_TRANS_WM_AMOUNT	10
> > > +#define SKL_TRANS_WM_MIN	14
> > As far as I understood (and maybe I'm wrong), SKL_TRANS_WM_AMOUNT is
> > a
> > tunable value, so it depends on a lot of variable factors. Why does
> > this patch use 10 instead of every other possible value?
This value 10 was driven by experiments in other OS with the help of SV 
team.
I'm trying to get more information regarding this from HW/SV team.
we can update this value later if required, what is your thought on this?
> I kept investigating and it seems that the current recommendation is
> still that transition watermarks stay disabled for SKL and KBL.

My earlier impression from Bspec was transition WM's should be disabled 
in SKL A/B stepping only. But from the latest discussion with HW team,
it seems transition WM must be disabled for all SKL stepping.
SV team recommendation is, It should be enabled with IPC for BXT/APL 
systems, So will update the patch with IS_BXT() check.

thanks,

Regards,
-Mahesh
> > Thanks,
> > Paulo
> >
> >
> > >   
> > >   /*
> > >    * Return the index of a plane in the SKL DDB and wm result
> > > arrays.  Primary
> > > @@ -3579,6 +3581,41 @@ static uint32_t
> > > skl_adjusted_plane_pixel_rate(const struct intel_crtc_state *cst
> > >   	return pixel_rate;
> > >   }
> > >   
> > > +static void skl_compute_plane_trans_wm(const struct
> > > drm_i915_private
> > > *dev_priv,
> > > +					struct skl_pipe_wm
> > > *pipe_wm,
> > > +					struct intel_plane_state
> > > *intel_pstate,
> > > +					uint32_t y_tile_min,
> > > +					bool y_tile)
> > > +{
> > > +	struct drm_plane_state *pstate = &intel_pstate->base;
> > > +	int id = skl_wm_plane_id(to_intel_plane(pstate->plane));
> > > +	uint16_t *out_blocks = &pipe_wm->trans_wm.plane_res_b[id];
> > > +	uint8_t *out_lines = &pipe_wm->trans_wm.plane_res_l[id];
> > > +	uint16_t res_blocks = pipe_wm->wm[0].plane_res_b[id];
> > > +	uint32_t trans_min = 0, trans_offset_blocks;
> > > +	uint16_t trans_y_tile_min = 0;
> > > +	uint16_t trans_res_blocks;
> > > +
> > > +
> > > +	if (IS_GEN9(dev_priv))
> > > +		trans_min = SKL_TRANS_WM_MIN;
> > > +
> > > +	trans_offset_blocks = trans_min + SKL_TRANS_WM_AMOUNT;
> > > +
> > > +	if (y_tile) {
> > > +		trans_y_tile_min = 2 * y_tile_min;
> > > +		trans_res_blocks = max(trans_y_tile_min,
> > > res_blocks)
> > > +
> > > +			trans_offset_blocks;
> > > +	} else {
> > > +		trans_res_blocks = res_blocks +
> > > trans_offset_blocks;
> > > +		/* WA BUG:1938466 add one block for non y-tile
> > > planes */
> > > +		trans_res_blocks += 1;
> > > +	}
> > > +
> > > +	*out_blocks = trans_res_blocks;
> > > +	*out_lines = 0;
> > > +}
> > > +
> > >   static int skl_compute_plane_wm(const struct drm_i915_private
> > > *dev_priv,
> > >   				struct intel_crtc_state *cstate,
> > >   				struct intel_plane_state
> > > *intel_pstate,
> > > @@ -3600,6 +3637,8 @@ static int skl_compute_plane_wm(const struct
> > > drm_i915_private *dev_priv,
> > >   	struct skl_wm_level *result = &pipe_wm->wm[level];
> > >   	uint16_t *out_blocks = &result->plane_res_b[id];
> > >   	uint8_t *out_lines = &result->plane_res_l[id];
> > > +	uint32_t y_tile_minimum = 0;
> > > +	bool y_tile = false;
> > >   
> > >   	if (latency == 0 || !cstate->base.active || !intel_pstate-
> > > > base.visible)
> > >   		return 0;
> > > @@ -3627,7 +3666,6 @@ static int skl_compute_plane_wm(const struct
> > > drm_i915_private *dev_priv,
> > >   	if (fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
> > >   	    fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED) {
> > >   		uint32_t min_scanlines = 4;
> > > -		uint32_t y_tile_minimum;
> > >   		if (intel_rotation_90_or_270(pstate->rotation)) {
> > >   			int cpp = (fb->pixel_format ==
> > > DRM_FORMAT_NV12) ?
> > >   				drm_format_plane_cpp(fb-
> > > > pixel_format, 1) :
> > > @@ -3646,6 +3684,7 @@ static int skl_compute_plane_wm(const struct
> > > drm_i915_private *dev_priv,
> > >   		}
> > >   		y_tile_minimum = plane_blocks_per_line *
> > > min_scanlines;
> > >   		selected_result = max(method2, y_tile_minimum);
> > > +		y_tile = true;
> > >   	} else {
> > >   		linetime_us = DIV_ROUND_UP(width * 1000,
> > >   				skl_pipe_pixel_rate(cstate));
> > > @@ -3669,6 +3708,9 @@ static int skl_compute_plane_wm(const struct
> > > drm_i915_private *dev_priv,
> > >   	*out_blocks = res_blocks;
> > >   	*out_lines = res_lines;
> > >   
> > > +	if (level == 0)
> > > +		skl_compute_plane_trans_wm(dev_priv, pipe_wm,
> > > intel_pstate,
> > > +						y_tile_minimum,
> > > y_tile);
> > >   	return 0;
> > >   }
> > >   
> > > @@ -3738,11 +3780,13 @@ skl_compute_linetime_wm(struct
> > > intel_crtc_state *cstate)
> > >   }
> > >   
> > >   static void skl_compute_transition_wm(struct intel_crtc_state
> > > *cstate,
> > > -				      struct skl_wm_level
> > > *trans_wm
> > > /* out */)
> > > +				      struct skl_wm_level
> > > *trans_wm,
> > > +				      struct skl_ddb_allocation
> > > *ddb)
> > >   {
> > >   	struct drm_crtc *crtc = cstate->base.crtc;
> > >   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > >   	struct intel_plane *intel_plane;
> > > +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> > >   
> > >   	if (!cstate->base.active)
> > >   		return;
> > > @@ -3750,8 +3794,13 @@ static void skl_compute_transition_wm(struct
> > > intel_crtc_state *cstate,
> > >   	/* Until we know more, just disable transition WMs */
> > >   	for_each_intel_plane_on_crtc(crtc->dev, intel_crtc,
> > > intel_plane) {
> > >   		int i = skl_wm_plane_id(intel_plane);
> > > +		uint16_t plane_ddb = skl_ddb_entry_size(&ddb-
> > > > plane[pipe][i]);
> > > +		uint16_t trans_res_blocks = trans_wm-
> > > > plane_res_b[i];
> > >   
> > > -		trans_wm->plane_en[i] = false;
> > > +		if ((plane_ddb == 0) || (trans_res_blocks >
> > > plane_ddb))
> > > +			trans_wm->plane_en[i] = false;
> > > +		else
> > > +			trans_wm->plane_en[i] = true;
> > >   	}
> > >   }
> > >   
> > > @@ -3782,7 +3831,7 @@ static int skl_build_pipe_wm(struct
> > > intel_crtc_state *cstate,
> > >   
> > >   	pipe_wm->linetime = skl_compute_linetime_wm(cstate);
> > >   
> > > -	skl_compute_transition_wm(cstate, &pipe_wm->trans_wm);
> > > +	skl_compute_transition_wm(cstate, &pipe_wm->trans_wm,
> > > ddb);
> > >   
> > >   	return 0;
> > >   }
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx