[Intel-gfx] [PATCH 2/3] drm/i915: Include "ignore lines" in skl+ wm state

Clinton Taylor Clinton.A.Taylor at intel.com
Wed Feb 13 19:44:44 UTC 2019


On 2/13/19 8:54 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> We'll need to poke at the "ignore lines" bit in the skl+
> watermark registers for a w/a. Include that bit in the wm
> state.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h |  1 +
>   drivers/gpu/drm/i915/i915_reg.h |  1 +
>   drivers/gpu/drm/i915/intel_pm.c | 44 +++++++++++++++++++++------------
>   3 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 17fe942eaafa..5c8d0489a1cd 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1126,6 +1126,7 @@ struct skl_wm_level {
>   	u16 plane_res_b;
>   	u8 plane_res_l;
>   	bool plane_en;
> +	bool ignore_lines;
>   };
>   
>   /* Stores plane specific WM parameters */
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f3f0b53c9e0e..7b3c41e9771f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6032,6 +6032,7 @@ enum {
>   #define _CUR_WM_TRANS_A_0	0x70168
>   #define _CUR_WM_TRANS_B_0	0x71168
>   #define   PLANE_WM_EN		(1 << 31)
> +#define   PLANE_WM_IGNORE_LINES	(1 << 30)
>   #define   PLANE_WM_LINES_SHIFT	14
>   #define   PLANE_WM_LINES_MASK	0x1f
>   #define   PLANE_WM_BLOCKS_MASK	0x7ff /* skl+: 10 bits, icl+ 11 bits */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index ea492fff6bf8..7dd2ab0ca21b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5056,11 +5056,12 @@ static void skl_write_wm_level(struct drm_i915_private *dev_priv,
>   {
>   	u32 val = 0;
>   
> -	if (level->plane_en) {
> +	if (level->plane_en)
>   		val |= PLANE_WM_EN;
> -		val |= level->plane_res_b;
> -		val |= level->plane_res_l << PLANE_WM_LINES_SHIFT;
> -	}
> +	if (level->ignore_lines)
> +		val |= PLANE_WM_IGNORE_LINES;
> +	val |= level->plane_res_b;
> +	val |= level->plane_res_l << PLANE_WM_LINES_SHIFT;

Is there a reason to program IGNORE_LINES, plane_res_b, and plane_res_l 
even when PLANE_WM_EN is not set? This is a change to functionality not 
described in the commit message.

Since the WM is not enabled anyway:

Reviewed-by: Clint Taylor <Clinton.A.Taylor at intel.com>

-Clint


>   
>   	I915_WRITE_FW(reg, val);
>   }
> @@ -5126,6 +5127,7 @@ bool skl_wm_level_equals(const struct skl_wm_level *l1,
>   			 const struct skl_wm_level *l2)
>   {
>   	return l1->plane_en == l2->plane_en &&
> +		l1->ignore_lines == l2->ignore_lines &&
>   		l1->plane_res_l == l2->plane_res_l &&
>   		l1->plane_res_b == l2->plane_res_b;
>   }
> @@ -5334,19 +5336,28 @@ skl_print_wm_changes(struct intel_atomic_state *state)
>   				      enast(new_wm->wm[6].plane_en), enast(new_wm->wm[7].plane_en),
>   				      enast(new_wm->trans_wm.plane_en));
>   
> -			DRM_DEBUG_KMS("[PLANE:%d:%s]   lines %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d"
> -				      " -> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d\n",
> +			DRM_DEBUG_KMS("[PLANE:%d:%s]   lines %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d"
> +				      " -> %c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d,%c%3d\n",
>   				      plane->base.base.id, plane->base.name,
> -				      old_wm->wm[0].plane_res_l, old_wm->wm[1].plane_res_l,
> -				      old_wm->wm[2].plane_res_l, old_wm->wm[3].plane_res_l,
> -				      old_wm->wm[4].plane_res_l, old_wm->wm[5].plane_res_l,
> -				      old_wm->wm[6].plane_res_l, old_wm->wm[7].plane_res_l,
> -				      old_wm->trans_wm.plane_res_l,
> -				      new_wm->wm[0].plane_res_l, new_wm->wm[1].plane_res_l,
> -				      new_wm->wm[2].plane_res_l, new_wm->wm[3].plane_res_l,
> -				      new_wm->wm[4].plane_res_l, new_wm->wm[5].plane_res_l,
> -				      new_wm->wm[6].plane_res_l, new_wm->wm[7].plane_res_l,
> -				      new_wm->trans_wm.plane_res_l);
> +				      enast(old_wm->wm[0].ignore_lines), old_wm->wm[0].plane_res_l,
> +				      enast(old_wm->wm[1].ignore_lines), old_wm->wm[1].plane_res_l,
> +				      enast(old_wm->wm[2].ignore_lines), old_wm->wm[2].plane_res_l,
> +				      enast(old_wm->wm[3].ignore_lines), old_wm->wm[3].plane_res_l,
> +				      enast(old_wm->wm[4].ignore_lines), old_wm->wm[4].plane_res_l,
> +				      enast(old_wm->wm[5].ignore_lines), old_wm->wm[5].plane_res_l,
> +				      enast(old_wm->wm[6].ignore_lines), old_wm->wm[6].plane_res_l,
> +				      enast(old_wm->wm[7].ignore_lines), old_wm->wm[7].plane_res_l,
> +				      enast(old_wm->trans_wm.ignore_lines), old_wm->trans_wm.plane_res_l,
> +
> +				      enast(new_wm->wm[0].ignore_lines), new_wm->wm[0].plane_res_l,
> +				      enast(new_wm->wm[1].ignore_lines), new_wm->wm[1].plane_res_l,
> +				      enast(new_wm->wm[2].ignore_lines), new_wm->wm[2].plane_res_l,
> +				      enast(new_wm->wm[3].ignore_lines), new_wm->wm[3].plane_res_l,
> +				      enast(new_wm->wm[4].ignore_lines), new_wm->wm[4].plane_res_l,
> +				      enast(new_wm->wm[5].ignore_lines), new_wm->wm[5].plane_res_l,
> +				      enast(new_wm->wm[6].ignore_lines), new_wm->wm[6].plane_res_l,
> +				      enast(new_wm->wm[7].ignore_lines), new_wm->wm[7].plane_res_l,
> +				      enast(new_wm->trans_wm.ignore_lines), new_wm->trans_wm.plane_res_l);
>   
>   			DRM_DEBUG_KMS("[PLANE:%d:%s]  blocks %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d"
>   				      " -> %4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d,%4d\n",
> @@ -5689,6 +5700,7 @@ static inline void skl_wm_level_from_reg_val(u32 val,
>   					     struct skl_wm_level *level)
>   {
>   	level->plane_en = val & PLANE_WM_EN;
> +	level->ignore_lines = val & PLANE_WM_IGNORE_LINES;
>   	level->plane_res_b = val & PLANE_WM_BLOCKS_MASK;
>   	level->plane_res_l = (val >> PLANE_WM_LINES_SHIFT) &
>   		PLANE_WM_LINES_MASK;


More information about the Intel-gfx mailing list