Re: [PATCH 5/8] drm/i915: Add CCS capability for sprites

Mika Kahola
2017-12-27T11:09:27Z

On Fri, 2017-12-22 at 21:22 +0200, Ville Syrjala 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.
>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: Daniel Stone <daniels@collabora.com>

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

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 25 ++++--------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  drivers/gpu/drm/i915/intel_sprite.c  | 50
> ++++++++++++++++++++++++++----------
>  3 files changed, 44 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 0dd37c854820..df29e33a2355 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3034,6 +3034,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);
>   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;
> @@ -3044,17 +3045,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;
>   }
>
> @@ -13161,18 +13153,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) {
> + 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 = skl_update_plane;
> - primary->disable_plane = skl_disable_plane;
> - primary->get_hw_state = skl_plane_get_hw_state;
> - } 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 30f791f89d64..059cabdcc026 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1932,6 +1932,8 @@ void skl_update_plane(struct intel_plane
> *plane,
>         const struct intel_plane_state *plane_state);
>  void skl_disable_plane(struct intel_plane *plane, struct intel_crtc
> *crtc);
>  bool skl_plane_get_hw_state(struct intel_plane *plane);
> +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 349be8134c76..cb06acff283d 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1161,7 +1161,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,
> @@ -1234,6 +1244,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:
> @@ -1289,6 +1303,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)
> @@ -1315,7 +1346,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;
>
> @@ -1325,18 +1356,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;
> - intel_plane->get_hw_state = skl_plane_get_hw_state;
> -
> - 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