diff mbox

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

Message ID 20160829123522.9532-7-mahesh1.kumar@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kumar, Mahesh Aug. 29, 2016, 12:35 p.m. UTC
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@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 57 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 53 insertions(+), 4 deletions(-)

Comments

Zanoni, Paulo R Aug. 30, 2016, 7:32 p.m. UTC | #1
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@intel.com>

> ---

>  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?

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;

>  }
Zanoni, Paulo R Aug. 31, 2016, 1:47 p.m. UTC | #2
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@intel.com>

> > ---

> >  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?


I kept investigating and it seems that the current recommendation is
still that transition watermarks stay disabled for SKL and KBL.

> 

> 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@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Kumar, Mahesh Sept. 2, 2016, 11:46 a.m. UTC | #3
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@intel.com>
>>> ---
>>>   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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Zanoni, Paulo R Sept. 2, 2016, 12:21 p.m. UTC | #4
Em Sex, 2016-09-02 às 17:16 +0530, Mahesh Kumar escreveu:
> 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@intel.com>

> > > > ---

> > > >   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.


BSpec says transition watermarks should be disabled for GEN9:ALL.

Arthur, can you please comment on this?

> 

> 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@lists.freedesktop.org

> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

>
diff mbox

Patch

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
 
 /*
  * 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;
 }