Re: [PATCH 12/18] drm/i915: Move display w/a #1175

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Jul 19, 2018 at 09:22:08PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move the display w/a #1175 to a better place. That place
> being the new skl+ specific plane->check() hook. This leaves
> the skl_check_plane_surface() stuff to deal with the gtt offset
> and src coordinate stuff as originally envisioned.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 36 +++++-------------------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  3 +--
>  drivers/gpu/drm/i915/intel_sprite.c  | 36 +++++++++++++++++++++++++++++++++++-
>  3 files changed, 41 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2381b762d109..dbcc9a23eefa 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2986,20 +2986,14 @@ static bool skl_check_main_ccs_coordinates(struct intel_plane_state *plane_state
>  	return true;
>  }
>  
> -static int skl_check_main_surface(const struct intel_crtc_state *crtc_state,
> -				  struct intel_plane_state *plane_state)
> +static int skl_check_main_surface(struct intel_plane_state *plane_state)
>  {
> -	struct drm_i915_private *dev_priv =
> -		to_i915(plane_state->base.plane->dev);
>  	const struct drm_framebuffer *fb = plane_state->base.fb;
>  	unsigned int rotation = plane_state->base.rotation;
>  	int x = plane_state->base.src.x1 >> 16;
>  	int y = plane_state->base.src.y1 >> 16;
>  	int w = drm_rect_width(&plane_state->base.src) >> 16;
>  	int h = drm_rect_height(&plane_state->base.src) >> 16;
> -	int dst_x = plane_state->base.dst.x1;
> -	int dst_w = drm_rect_width(&plane_state->base.dst);
> -	int pipe_src_w = crtc_state->pipe_src_w;
>  	int max_width = skl_max_plane_width(fb, 0, rotation);
>  	int max_height = 4096;
>  	u32 alignment, offset, aux_offset = plane_state->color_plane[1].offset;
> @@ -3010,24 +3004,6 @@ static int skl_check_main_surface(const struct intel_crtc_state *crtc_state,
>  		return -EINVAL;
>  	}
>  
> -	/*
> -	 * Display WA #1175: cnl,glk
> -	 * Planes other than the cursor may cause FIFO underflow and display
> -	 * corruption if starting less than 4 pixels from the right edge of
> -	 * the screen.
> -	 * Besides the above WA fix the similar problem, where planes other
> -	 * than the cursor ending less than 4 pixels from the left edge of the
> -	 * screen may cause FIFO underflow and display corruption.
> -	 */
> -	if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> -	    (dst_x + dst_w < 4 || dst_x > pipe_src_w - 4)) {
> -		DRM_DEBUG_KMS("requested plane X %s position %d invalid (valid range %d-%d)\n",
> -			      dst_x + dst_w < 4 ? "end" : "start",
> -			      dst_x + dst_w < 4 ? dst_x + dst_w : dst_x,
> -			      4, pipe_src_w - 4);
> -		return -ERANGE;
> -	}
> -
>  	intel_add_fb_offsets(&x, &y, plane_state, 0);
>  	offset = intel_plane_compute_aligned_offset(&x, &y, plane_state, 0);
>  	alignment = intel_surf_alignment(fb, 0);
> @@ -3089,8 +3065,7 @@ static int skl_check_main_surface(const struct intel_crtc_state *crtc_state,
>  }
>  
>  static int
> -skl_check_nv12_surface(const struct intel_crtc_state *crtc_state,
> -		       struct intel_plane_state *plane_state)
> +skl_check_nv12_surface(struct intel_plane_state *plane_state)
>  {
>  	/* Display WA #1106 */
>  	if (plane_state->base.rotation !=
> @@ -3161,8 +3136,7 @@ static int skl_check_ccs_aux_surface(struct intel_plane_state *plane_state)
>  	return 0;
>  }
>  
> -int skl_check_plane_surface(const struct intel_crtc_state *crtc_state,
> -			    struct intel_plane_state *plane_state)
> +int skl_check_plane_surface(struct intel_plane_state *plane_state)
>  {
>  	const struct drm_framebuffer *fb = plane_state->base.fb;
>  	unsigned int rotation = plane_state->base.rotation;
> @@ -3186,7 +3160,7 @@ int skl_check_plane_surface(const struct intel_crtc_state *crtc_state,
>  	 * the main surface setup depends on it.
>  	 */
>  	if (fb->format->format == DRM_FORMAT_NV12) {
> -		ret = skl_check_nv12_surface(crtc_state, plane_state);
> +		ret = skl_check_nv12_surface(plane_state);
>  		if (ret)
>  			return ret;
>  		ret = skl_check_nv12_aux_surface(plane_state);
> @@ -3203,7 +3177,7 @@ int skl_check_plane_surface(const struct intel_crtc_state *crtc_state,
>  		plane_state->color_plane[1].y = 0;
>  	}
>  
> -	ret = skl_check_main_surface(crtc_state, plane_state);
> +	ret = skl_check_main_surface(plane_state);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 6d57c6a508d4..55f3537ba8f8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1651,8 +1651,7 @@ u32 skl_plane_ctl(const struct intel_crtc_state *crtc_state,
>  u32 glk_color_ctl(const struct intel_plane_state *plane_state);
>  u32 skl_plane_stride(const struct intel_plane_state *plane_state,
>  		     int plane);
> -int skl_check_plane_surface(const struct intel_crtc_state *crtc_state,
> -			    struct intel_plane_state *plane_state);
> +int skl_check_plane_surface(struct intel_plane_state *plane_state);
>  int i9xx_check_plane_surface(struct intel_plane_state *plane_state);
>  int skl_format_to_fourcc(int format, bool rgb_order, bool alpha);
>  unsigned int i9xx_plane_max_stride(struct intel_plane *plane,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 041b8921f4fe..f43884f76212 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1218,6 +1218,36 @@ static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
>  	return 0;
>  }
>  
> +static int skl_plane_check_dst_coordinates(const struct intel_crtc_state *crtc_state,
> +					   const struct intel_plane_state *plane_state)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(plane_state->base.plane->dev);
> +	int crtc_x = plane_state->base.dst.x1;
> +	int crtc_w = drm_rect_width(&plane_state->base.dst);
> +	int pipe_src_w = crtc_state->pipe_src_w;
> +
> +	/*
> +	 * Display WA #1175: cnl,glk
> +	 * Planes other than the cursor may cause FIFO underflow and display
> +	 * corruption if starting less than 4 pixels from the right edge of
> +	 * the screen.
> +	 * Besides the above WA fix the similar problem, where planes other
> +	 * than the cursor ending less than 4 pixels from the left edge of the
> +	 * screen may cause FIFO underflow and display corruption.
> +	 */
> +	if ((IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) &&
> +	    (crtc_x + crtc_w < 4 || crtc_x > pipe_src_w - 4)) {
> +		DRM_DEBUG_KMS("requested plane X %s position %d invalid (valid range %d-%d)\n",
> +			      crtc_x + crtc_w < 4 ? "end" : "start",
> +			      crtc_x + crtc_w < 4 ? crtc_x + crtc_w : crtc_x,
> +			      4, pipe_src_w - 4);
> +		return -ERANGE;
> +	}
> +
> +	return 0;
> +}
> +
>  int skl_plane_check(struct intel_crtc_state *crtc_state,
>  		    struct intel_plane_state *plane_state)
>  {
> @@ -1252,11 +1282,15 @@ int skl_plane_check(struct intel_crtc_state *crtc_state,
>  	if (!plane_state->base.visible)
>  		return 0;
>  
> +	ret = skl_plane_check_dst_coordinates(crtc_state, plane_state);
> +	if (ret)
> +		return ret;
> +
>  	ret = intel_plane_check_src_coordinates(plane_state);
>  	if (ret)
>  		return ret;
>  
> -	ret = skl_check_plane_surface(crtc_state, plane_state);
> +	ret = skl_check_plane_surface(plane_state);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx