[Intel-gfx] [PATCH 3/4] drm/i915: Add format modifiers for Intel

[Emil Velikov]

On 23 June 2017 at 17:45, Ben Widawsky <ben at bwidawsk.net> wrote:
> This was based on a patch originally by Kristian. It has been modified
> pretty heavily to use the new callbacks from the previous patch.
>
> v2:
>   - Add LINEAR and Yf modifiers to list (Ville)
>   - Combine i8xx and i965 into one list of formats (Ville)
>   - Allow 1010102 formats for Y/Yf tiled (Ville)
>
> v3:
>   - Handle cursor formats (Ville)
>   - Put handling for LINEAR in the mod_support functions (Ville)
>
> v4:
>   - List each modifier explicitly in supported modifiers (Ville)
>   - Handle the CURSOR plane (Ville)
>
> v5:
>   - Split out cursor and sprite handling (Ville)
>
> v6:
>   - Actually use the sprite funcs (Emil)
>   - Use unreachable (Emil)
>
> v7:
>   - Only allow Intel modifiers and LINEAR (Ben)
>
> v8
>   - Fix spite assert introduced in v6 (Daniel)
>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Kristian H. Kristensen <hoegsberg at gmail.com>
> Cc: Emil Velikov <emil.l.velikov at gmail.com>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 136 +++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_sprite.c  |  82 ++++++++++++++++++++-
>  2 files changed, 211 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7d55c5e3df28..877a51514c61 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -72,6 +72,12 @@ static const uint32_t i965_primary_formats[] = {
>         DRM_FORMAT_XBGR2101010,
>  };
>
> +static const uint64_t i9xx_format_modifiers[] = {
> +       I915_FORMAT_MOD_X_TILED,
> +       DRM_FORMAT_MOD_LINEAR,
> +       DRM_FORMAT_MOD_INVALID
> +};
> +
>  static const uint32_t skl_primary_formats[] = {
>         DRM_FORMAT_C8,
>         DRM_FORMAT_RGB565,
> @@ -87,11 +93,24 @@ static const uint32_t skl_primary_formats[] = {
>         DRM_FORMAT_VYUY,
>  };
>
> +static const uint64_t skl_format_modifiers[] = {
> +       I915_FORMAT_MOD_Yf_TILED,
> +       I915_FORMAT_MOD_Y_TILED,
> +       I915_FORMAT_MOD_X_TILED,
> +       DRM_FORMAT_MOD_LINEAR,
> +       DRM_FORMAT_MOD_INVALID
> +};
> +
>  /* Cursor formats */
>  static const uint32_t intel_cursor_formats[] = {
>         DRM_FORMAT_ARGB8888,
>  };
>
> +static const uint64_t cursor_format_modifiers[] = {
> +       DRM_FORMAT_MOD_LINEAR,
> +       DRM_FORMAT_MOD_INVALID
> +};
> +
>  static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
>                                 struct intel_crtc_state *pipe_config);
>  static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> @@ -13810,6 +13829,108 @@ void intel_plane_destroy(struct drm_plane *plane)
>         kfree(to_intel_plane(plane));
>  }
>
> +static bool i8xx_mod_supported(uint32_t format, uint64_t modifier)
> +{
> +       switch (format) {
> +       case DRM_FORMAT_C8:
> +       case DRM_FORMAT_RGB565:
> +       case DRM_FORMAT_XRGB1555:
> +       case DRM_FORMAT_XRGB8888:
> +               return modifier == DRM_FORMAT_MOD_LINEAR ||
> +                       modifier == I915_FORMAT_MOD_X_TILED;
> +       default:
> +               return false;
> +       }
> +}
> +
> +static bool i965_mod_supported(uint32_t format, uint64_t modifier)
> +{
> +       switch (format) {
> +       case DRM_FORMAT_C8:
> +       case DRM_FORMAT_RGB565:
> +       case DRM_FORMAT_XRGB8888:
> +       case DRM_FORMAT_XBGR8888:
> +       case DRM_FORMAT_XRGB2101010:
> +       case DRM_FORMAT_XBGR2101010:
> +               return modifier == DRM_FORMAT_MOD_LINEAR ||
> +                       modifier == I915_FORMAT_MOD_X_TILED;
> +       default:
> +               return false;
> +       }
> +}
> +
> +static bool skl_mod_supported(uint32_t format, uint64_t modifier)
> +{
> +       switch (format) {
> +       case DRM_FORMAT_C8:
> +               switch (modifier) {
> +               case DRM_FORMAT_MOD_LINEAR:
> +               case I915_FORMAT_MOD_X_TILED:
> +               case I915_FORMAT_MOD_Y_TILED:
> +                       return true;
> +               default:
> +                       return false;
> +               }
> +       case DRM_FORMAT_RGB565:
> +       case DRM_FORMAT_XRGB8888:
> +       case DRM_FORMAT_XBGR8888:
> +       case DRM_FORMAT_ARGB8888:
> +       case DRM_FORMAT_ABGR8888:
> +       case DRM_FORMAT_XRGB2101010:
> +       case DRM_FORMAT_XBGR2101010:
> +       case DRM_FORMAT_YUYV:
> +       case DRM_FORMAT_YVYU:
> +       case DRM_FORMAT_UYVY:
> +       case DRM_FORMAT_VYUY:
> +               /* All i915 modifiers are fine */
> +               switch (modifier) {
> +               case DRM_FORMAT_MOD_LINEAR:
> +               case I915_FORMAT_MOD_X_TILED:
> +               case I915_FORMAT_MOD_Y_TILED:
> +               case I915_FORMAT_MOD_Yf_TILED:
> +                       return true;
> +               default:
> +                       return false;
> +               }
> +       default:
> +               return false;
> +       }
> +}
> +
> +static bool intel_primary_plane_format_mod_supported(struct drm_plane *plane,
> +                                                    uint32_t format,
> +                                                    uint64_t modifier)
> +{
> +       struct drm_i915_private *dev_priv = to_i915(plane->dev);
> +
> +       if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID))
> +               return false;
> +
> +       if ((modifier >> 56) != DRM_FORMAT_MOD_VENDOR_INTEL &&
Using ">> 56" seem a bit magical. Alternative solution is to introduce
DRM_FORMAT_MOD_VENDOR_MASK or even
"if (!(modifier & DRM_FORMAT_MOD_VENDOR_INTEL) ..."

Regardless, patch looks good and is
Reviewed-by: Emil Velikov <emil.l.velikov at gmail.com>

-Emil