[Intel-gfx] [PATCH 10/15] drm/i915: Make sure CCS YUV semiplanar format checks work

Kahola, Mika mika.kahola at intel.com
Thu Dec 19 14:14:48 UTC 2019


On Wed, 2019-12-18 at 18:11 +0200, Imre Deak wrote:
> For CCS formats, the current DRM core check for YUV semiplanar
> formats
> doesn't work; use an i915 specific function for that.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>

There is still one call for drm_format_info_is_yuv_semiplanar() in
intel_pm.c. For consistency reasons, maybe we could update that one too
to use i915 specific function? Anyway,

Reviewed-by: Mika Kahola <mika.kahola at intel.com>

> ---
>  .../gpu/drm/i915/display/intel_atomic_plane.c |  2 +-
>  drivers/gpu/drm/i915/display/intel_display.c  | 25 ++++++++++++++---
> --
>  drivers/gpu/drm/i915/display/intel_display.h  |  4 +++
>  drivers/gpu/drm/i915/display/intel_sprite.c   |  8 +++---
>  drivers/gpu/drm/i915/intel_pm.c               |  7 +++---
>  5 files changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 9429b8e17270..3e97af682b1b 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -250,7 +250,7 @@ int intel_plane_atomic_check_with_state(const
> struct intel_crtc_state *old_crtc_
>  		new_crtc_state->active_planes |= BIT(plane->id);
>  
>  	if (new_plane_state->uapi.visible &&
> -	    drm_format_info_is_yuv_semiplanar(fb->format))
> +	    intel_format_info_is_yuv_semiplanar(fb->format, fb-
> >modifier))
>  		new_crtc_state->nv12_planes |= BIT(plane->id);
>  
>  	if (new_plane_state->uapi.visible &&
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index e350f1d40b88..8b36c33bb63e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1985,6 +1985,14 @@ intel_main_to_aux_plane(const struct
> drm_framebuffer *fb, int main_plane)
>  	return 1;
>  }
>  
> +bool
> +intel_format_info_is_yuv_semiplanar(const struct drm_format_info
> *info,
> +				    uint64_t modifier)
> +{
> +	return info->is_yuv &&
> +	       info->num_planes == (is_ccs_modifier(modifier) ? 4 : 2);
> +}
> +
>  static unsigned int
>  intel_tile_width_bytes(const struct drm_framebuffer *fb, int
> color_plane)
>  {
> @@ -3813,7 +3821,8 @@ int skl_check_plane_surface(struct
> intel_plane_state *plane_state)
>  	 * Handle the AUX surface first since
>  	 * the main surface setup depends on it.
>  	 */
> -	if (drm_format_info_is_yuv_semiplanar(fb->format)) {
> +	if (intel_format_info_is_yuv_semiplanar(fb->format,
> +					        fb->modifier)) {
>  		ret = skl_check_nv12_aux_surface(plane_state);
>  		if (ret)
>  			return ret;
> @@ -5742,7 +5751,8 @@ static int
>  skl_update_scaler(struct intel_crtc_state *crtc_state, bool
> force_detach,
>  		  unsigned int scaler_user, int *scaler_id,
>  		  int src_w, int src_h, int dst_w, int dst_h,
> -		  const struct drm_format_info *format, bool
> need_scaler)
> +		  const struct drm_format_info *format,
> +		  uint64_t modifier, bool need_scaler)
>  {
>  	struct intel_crtc_scaler_state *scaler_state =
>  		&crtc_state->scaler_state;
> @@ -5796,7 +5806,7 @@ skl_update_scaler(struct intel_crtc_state
> *crtc_state, bool force_detach,
>  		return 0;
>  	}
>  
> -	if (format && drm_format_info_is_yuv_semiplanar(format) &&
> +	if (format && intel_format_info_is_yuv_semiplanar(format,
> modifier) &&
>  	    (src_h < SKL_MIN_YUV_420_SRC_H || src_w <
> SKL_MIN_YUV_420_SRC_W)) {
>  		DRM_DEBUG_KMS("Planar YUV: src dimensions not met\n");
>  		return -EINVAL;
> @@ -5848,7 +5858,8 @@ int skl_update_scaler_crtc(struct
> intel_crtc_state *state)
>  				 &state->scaler_state.scaler_id,
>  				 state->pipe_src_w, state->pipe_src_h,
>  				 adjusted_mode->crtc_hdisplay,
> -				 adjusted_mode->crtc_vdisplay, NULL,
> need_scaler);
> +				 adjusted_mode->crtc_vdisplay, NULL, 0,
> +				 need_scaler);
>  }
>  
>  /**
> @@ -5873,7 +5884,7 @@ static int skl_update_scaler_plane(struct
> intel_crtc_state *crtc_state,
>  
>  	/* Pre-gen11 and SDR planes always need a scaler for planar
> formats. */
>  	if (!icl_is_hdr_plane(dev_priv, intel_plane->id) &&
> -	    fb && drm_format_info_is_yuv_semiplanar(fb->format))
> +	    fb && intel_format_info_is_yuv_semiplanar(fb->format, fb-
> >modifier))
>  		need_scaler = true;
>  
>  	ret = skl_update_scaler(crtc_state, force_detach,
> @@ -5883,7 +5894,9 @@ static int skl_update_scaler_plane(struct
> intel_crtc_state *crtc_state,
>  				drm_rect_height(&plane_state->uapi.src) 
> >> 16,
>  				drm_rect_width(&plane_state->uapi.dst),
>  				drm_rect_height(&plane_state-
> >uapi.dst),
> -				fb ? fb->format : NULL, need_scaler);
> +				fb ? fb->format : NULL,
> +				fb ? fb->modifier : 0,
> +				need_scaler);
>  
>  	if (ret || plane_state->scaler_id < 0)
>  		return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_display.h
> b/drivers/gpu/drm/i915/display/intel_display.h
> index ff496cfbd4ab..0fef9263cddc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.h
> +++ b/drivers/gpu/drm/i915/display/intel_display.h
> @@ -601,6 +601,10 @@ intel_display_capture_error_state(struct
> drm_i915_private *dev_priv);
>  void intel_display_print_error_state(struct drm_i915_error_state_buf
> *e,
>  				     struct intel_display_error_state
> *error);
>  
> +bool
> +intel_format_info_is_yuv_semiplanar(const struct drm_format_info
> *info,
> +				    uint64_t modifier);
> +
>  /* modesetting */
>  void intel_modeset_init_hw(struct drm_i915_private *i915);
>  int intel_modeset_init(struct drm_i915_private *i915);
> diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> b/drivers/gpu/drm/i915/display/intel_sprite.c
> index 67a90059900f..b7f3a1b3358f 100644
> --- a/drivers/gpu/drm/i915/display/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> @@ -417,7 +417,7 @@ skl_program_scaler(struct intel_plane *plane,
>  				      0, INT_MAX);
>  
>  	/* TODO: handle sub-pixel coordinates */
> -	if (drm_format_info_is_yuv_semiplanar(fb->format) &&
> +	if (intel_format_info_is_yuv_semiplanar(fb->format, fb-
> >modifier) &&
>  	    !icl_is_hdr_plane(dev_priv, plane->id)) {
>  		y_hphase = skl_scaler_calc_phase(1, hscale, false);
>  		y_vphase = skl_scaler_calc_phase(1, vscale, false);
> @@ -2151,7 +2151,7 @@ static int skl_plane_check_nv12_rotation(const
> struct intel_plane_state *plane_s
>  	int src_w = drm_rect_width(&plane_state->uapi.src) >> 16;
>  
>  	/* Display WA #1106 */
> -	if (drm_format_info_is_yuv_semiplanar(fb->format) && src_w & 3
> &&
> +	if (intel_format_info_is_yuv_semiplanar(fb->format, fb-
> >modifier) && src_w & 3 &&
>  	    (rotation == DRM_MODE_ROTATE_270 ||
>  	     rotation == (DRM_MODE_REFLECT_X | DRM_MODE_ROTATE_90))) {
>  		DRM_DEBUG_KMS("src width must be multiple of 4 for
> rotated planar YUV\n");
> @@ -2171,7 +2171,7 @@ static int skl_plane_max_scale(struct
> drm_i915_private *dev_priv,
>  	 * FIXME need to properly check this later.
>  	 */
>  	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv) ||
> -	    !drm_format_info_is_yuv_semiplanar(fb->format))
> +	    !intel_format_info_is_yuv_semiplanar(fb->format, fb-
> >modifier))
>  		return 0x30000 - 1;
>  	else
>  		return 0x20000 - 1;
> @@ -2233,7 +2233,7 @@ static int skl_plane_check(struct
> intel_crtc_state *crtc_state,
>  		plane_state->color_ctl =
> glk_plane_color_ctl(crtc_state,
>  							     plane_stat
> e);
>  
> -	if (drm_format_info_is_yuv_semiplanar(fb->format) &&
> +	if (intel_format_info_is_yuv_semiplanar(fb->format, fb-
> >modifier) &&
>  	    icl_is_hdr_plane(dev_priv, plane->id))
>  		/* Enable and use MPEG-2 chroma siting */
>  		plane_state->cus_ctl = PLANE_CUS_ENABLE |
> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> b/drivers/gpu/drm/i915/intel_pm.c
> index 7cdca06be3bd..31ec82337e4f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4135,7 +4135,7 @@ skl_plane_relative_data_rate(const struct
> intel_crtc_state *crtc_state,
>  		return 0;
>  
>  	if (color_plane == 1 &&
> -	    !drm_format_info_is_yuv_semiplanar(fb->format))
> +	    !intel_format_info_is_yuv_semiplanar(fb->format, fb-
> >modifier))
>  		return 0;
>  
>  	/*
> @@ -4559,7 +4559,8 @@ skl_compute_wm_params(const struct
> intel_crtc_state *crtc_state,
>  	u32 interm_pbpl;
>  
>  	/* only planar format has two planes */
> -	if (color_plane == 1 &&
> !drm_format_info_is_yuv_semiplanar(format)) {
> +	if (color_plane == 1 &&
> +	    !intel_format_info_is_yuv_semiplanar(format, modifier)) {
>  		DRM_DEBUG_KMS("Non planar format have single plane\n");
>  		return -EINVAL;
>  	}
> @@ -4571,7 +4572,7 @@ skl_compute_wm_params(const struct
> intel_crtc_state *crtc_state,
>  	wp->x_tiled = modifier == I915_FORMAT_MOD_X_TILED;
>  	wp->rc_surface = modifier == I915_FORMAT_MOD_Y_TILED_CCS ||
>  			 modifier == I915_FORMAT_MOD_Yf_TILED_CCS;
> -	wp->is_planar = drm_format_info_is_yuv_semiplanar(format);
> +	wp->is_planar = intel_format_info_is_yuv_semiplanar(format,
> modifier);
>  
>  	wp->width = width;
>  	if (color_plane == 1 && wp->is_planar)


More information about the Intel-gfx mailing list