Re: [Intel-gfx] [PATCH] drm/i915: do not reset PLANE_SURF on plane disable on older gens

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

On Mon, Sep 05, 2022 at 10:05:00AM +0200, Andrzej Hajda wrote:
> In case of ICL and older generations disabling plane and/or disabling
> async update is always performed on vblank,

It should only be broken on bdw-glk (see. need_async_flip_disable_wa).

> but if async update is enabled
> PLANE_SURF register is updated asynchronously. Writing 0 to PLANE_SURF
> when plane is still enabled can cause DMAR/PIPE errors.
> On the other side PLANE_SURF is used to arm plane registers - we need to
> write to it to trigger update on VBLANK, writting current value should
> be safe - the buffer address is valid till vblank.

I think you're effectively saying that somehow the async
flip disable w/a is not kicking in sometimes.

> 
> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
> ---
>  drivers/gpu/drm/i915/display/skl_universal_plane.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index bcfde81e4d0866..bc9ed60a2d349e 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -615,11 +615,13 @@ skl_plane_disable_arm(struct intel_plane *plane,
>  	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	enum plane_id plane_id = plane->id;
>  	enum pipe pipe = plane->pipe;
> +	u32 plane_surf;
>  
>  	skl_write_plane_wm(plane, crtc_state);
>  
>  	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
> -	intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), 0);
> +	plane_surf = intel_de_read_fw(dev_priv, PLANE_SURF(pipe, plane_id));
> +	intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf);
>  }
>  
>  static void
> @@ -629,6 +631,7 @@ icl_plane_disable_arm(struct intel_plane *plane,
>  	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
>  	enum plane_id plane_id = plane->id;
>  	enum pipe pipe = plane->pipe;
> +	u32 plane_surf;
>  
>  	if (icl_is_hdr_plane(dev_priv, plane_id))
>  		intel_de_write_fw(dev_priv, PLANE_CUS_CTL(pipe, plane_id), 0);
> @@ -637,7 +640,8 @@ icl_plane_disable_arm(struct intel_plane *plane,
>  
>  	intel_psr2_disable_plane_sel_fetch(plane, crtc_state);
>  	intel_de_write_fw(dev_priv, PLANE_CTL(pipe, plane_id), 0);
> -	intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), 0);
> +	plane_surf = intel_de_read_fw(dev_priv, PLANE_SURF(pipe, plane_id));
> +	intel_de_write_fw(dev_priv, PLANE_SURF(pipe, plane_id), plane_surf);
>  }
>  
>  static bool
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel