Re: [PATCH 08/12] drm/i915: Add CCS capability for sprites

[Mika Kahola <mika.kahola@intel.com>]

On Thu, 2017-08-24 at 22:10 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Allow sprites to scan out compressed framebuffers.
> 
> Since different platforms have a different set of planes that
> support CCS let's add a small helper to determine whether a
> specific plane supports CCS or not. Currently that information
> is spread around in many places, and not all the pieces of
> code even agree with each other.
> 
> In addition to allowing sprites to scan out compressed fbs,
> the other fix here is that we stop rejecting them on pipe C
> on CNL.
Unfortunately, this patch didn't apply cleanly on top of the latest
drm-tip. Overall, the patch looks good to me. 

> 
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Daniel Stone <daniels@collabora.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 24 ++++--------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_sprite.c  | 49
> +++++++++++++++++++++++++++---------
>  3 files changed, 44 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index ba351cbef93b..d52a5e01aaf9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3120,6 +3120,7 @@ static int skl_check_nv12_aux_surface(struct
> intel_plane_state *plane_state)
>  static int skl_check_ccs_aux_surface(struct intel_plane_state
> *plane_state)
>  {
>  	struct intel_plane *plane = to_intel_plane(plane_state-
> >base.plane);
> +	struct drm_i915_private *dev_priv = to_i915(plane-
> >base.dev);
Seems that this addition is not really needed.

>  	struct intel_crtc *crtc = to_intel_crtc(plane_state-
> >base.crtc);
>  	const struct drm_framebuffer *fb = plane_state->base.fb;
>  	int src_x = plane_state->base.src.x1 >> 16;
> @@ -3130,17 +3131,8 @@ static int skl_check_ccs_aux_surface(struct
> intel_plane_state *plane_state)
>  	int y = src_y / vsub;
>  	u32 offset;
>  
> -	switch (plane->id) {
> -	case PLANE_PRIMARY:
> -	case PLANE_SPRITE0:
> -		break;
> -	default:
> -		DRM_DEBUG_KMS("RC support only on plane 1 and 2\n");
> -		return -EINVAL;
> -	}
> -
> -	if (crtc->pipe == PIPE_C) {
> -		DRM_DEBUG_KMS("No RC support on pipe C\n");
> +	if (!skl_plane_has_ccs(dev_priv, crtc->pipe, plane->id)) {
> +		DRM_DEBUG_KMS("No RC support on %s\n", plane-
> >base.name);
>  		return -EINVAL;
>  	}
>  
> @@ -13279,17 +13271,11 @@ intel_primary_plane_create(struct
> drm_i915_private *dev_priv, enum pipe pipe)
>  	primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe);
>  	primary->check_plane = intel_check_primary_plane;
>  
> -	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> +	if (INTEL_GEN(dev_priv) >= 9) {
>  		intel_primary_formats = skl_primary_formats;
>  		num_formats = ARRAY_SIZE(skl_primary_formats);
> -		modifiers = skl_format_modifiers_ccs;
>  
> -		primary->update_plane =
> skylake_update_primary_plane;
> -		primary->disable_plane =
> skylake_disable_primary_plane;
> -	} else if (INTEL_GEN(dev_priv) >= 9) {
> -		intel_primary_formats = skl_primary_formats;
> -		num_formats = ARRAY_SIZE(skl_primary_formats);
> -		if (pipe < PIPE_C)
> +		if (skl_plane_has_ccs(dev_priv, pipe,
> PLANE_PRIMARY))
>  			modifiers = skl_format_modifiers_ccs;
>  		else
>  			modifiers = skl_format_modifiers_noccs;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index f60995fe455c..b2bec0b2e582 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1890,6 +1890,8 @@ int intel_sprite_set_colorkey(struct drm_device
> *dev, void *data,
>  			      struct drm_file *file_priv);
>  void intel_pipe_update_start(struct intel_crtc *crtc);
>  void intel_pipe_update_end(struct intel_crtc *crtc);
> +bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
> +		       enum pipe pipe, enum plane_id plane_id);
>  
>  /* intel_tv.c */
>  void intel_tv_init(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 7610f84f3525..345873be14c3 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1079,7 +1079,17 @@ static uint32_t skl_plane_formats[] = {
>  	DRM_FORMAT_VYUY,
>  };
>  
> -static const uint64_t skl_plane_format_modifiers[] = {
> +static const uint64_t skl_plane_format_modifiers_noccs[] = {
> +	I915_FORMAT_MOD_Yf_TILED,
> +	I915_FORMAT_MOD_Y_TILED,
> +	I915_FORMAT_MOD_X_TILED,
> +	DRM_FORMAT_MOD_LINEAR,
> +	DRM_FORMAT_MOD_INVALID
> +};
> +
> +static const uint64_t skl_plane_format_modifiers_ccs[] = {
> +	I915_FORMAT_MOD_Yf_TILED_CCS,
> +	I915_FORMAT_MOD_Y_TILED_CCS,
>  	I915_FORMAT_MOD_Yf_TILED,
>  	I915_FORMAT_MOD_Y_TILED,
>  	I915_FORMAT_MOD_X_TILED,
> @@ -1152,6 +1162,10 @@ static bool skl_mod_supported(uint32_t format,
> uint64_t modifier)
>  	case DRM_FORMAT_XBGR8888:
>  	case DRM_FORMAT_ARGB8888:
>  	case DRM_FORMAT_ABGR8888:
> +		if (modifier == I915_FORMAT_MOD_Yf_TILED_CCS ||
> +		    modifier == I915_FORMAT_MOD_Y_TILED_CCS)
> +			return true;
> +		/* fall through */
>  	case DRM_FORMAT_RGB565:
>  	case DRM_FORMAT_XRGB2101010:
>  	case DRM_FORMAT_XBGR2101010:
> @@ -1207,6 +1221,23 @@ static const struct drm_plane_funcs
> intel_sprite_plane_funcs = {
>          .format_mod_supported =
> intel_sprite_plane_format_mod_supported,
>  };
>  
> +bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
> +		       enum pipe pipe, enum plane_id plane_id)
> +{
> +	if (plane_id == PLANE_CURSOR)
> +		return false;
> +
> +	if (INTEL_GEN(dev_priv) >= 10)
> +		return true;
> +
> +	if (IS_GEMINILAKE(dev_priv))
> +		return pipe != PIPE_C;
> +
> +	return pipe != PIPE_C &&
> +		(plane_id == PLANE_PRIMARY ||
> +		 plane_id == PLANE_SPRITE0);
> +}
> +
>  struct intel_plane *
>  intel_sprite_plane_create(struct drm_i915_private *dev_priv,
>  			  enum pipe pipe, int plane)
> @@ -1233,7 +1264,7 @@ intel_sprite_plane_create(struct
> drm_i915_private *dev_priv,
>  	}
>  	intel_plane->base.state = &state->base;
>  
> -	if (INTEL_GEN(dev_priv) >= 10) {
> +	if (INTEL_GEN(dev_priv) >= 9) {
>  		intel_plane->can_scale = true;
>  		state->scaler_id = -1;
>  
> @@ -1242,17 +1273,11 @@ intel_sprite_plane_create(struct
> drm_i915_private *dev_priv,
>  
>  		plane_formats = skl_plane_formats;
>  		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> -		modifiers = skl_plane_format_modifiers;
> -	} else if (INTEL_GEN(dev_priv) >= 9) {
> -		intel_plane->can_scale = true;
> -		state->scaler_id = -1;
> -
> -		intel_plane->update_plane = skl_update_plane;
> -		intel_plane->disable_plane = skl_disable_plane;
>  
> -		plane_formats = skl_plane_formats;
> -		num_plane_formats = ARRAY_SIZE(skl_plane_formats);
> -		modifiers = skl_plane_format_modifiers;
> +		if (skl_plane_has_ccs(dev_priv, pipe, PLANE_SPRITE0
> + plane))
> +			modifiers = skl_plane_format_modifiers_ccs;
> +		else
> +			modifiers =
> skl_plane_format_modifiers_noccs;
>  	} else if (IS_VALLEYVIEW(dev_priv) ||
> IS_CHERRYVIEW(dev_priv)) {
>  		intel_plane->can_scale = false;
>  		intel_plane->max_downscale = 1;
-- 
Mika Kahola - Intel OTC

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx